diff --git a/bin/version b/bin/version index 0835db2b4015497bf11a336789550d6b7f5e8c1c..16925539d1d2754a760a7ecd800f875624ffc3e8 100755 --- a/bin/version +++ b/bin/version @@ -1,11 +1,12 @@ #!/usr/bin/env bash -# Return the version string used to describe this version of Metabase. +VERSION="v0.15.0-snapshot" -TAG=$(git name-rev --tags --name-only HEAD) -TAG=${TAG%^0} +# dynamically pull more interesting stuff from latest git commit HASH=$(git show-ref --head --hash=7 head) # first 7 letters of hash should be enough; that's what GitHub uses BRANCH=$(git rev-parse --abbrev-ref HEAD) DATE=$(git log -1 --pretty=%ad --date=short) -echo "$TAG $HASH $BRANCH $DATE" + +# Return the version string used to describe this version of Metabase. +echo "$VERSION $HASH $BRANCH $DATE" diff --git a/circle.yml b/circle.yml index 40f37811b53b8f3ff274638959429a6e7e5ce19c..8302f70e66f24551eb382b0bda4cb9e0f86c13cd 100644 --- a/circle.yml +++ b/circle.yml @@ -25,11 +25,11 @@ test: # 0) runs unit tests w/ H2 local DB. Runs against H2, Mongo, MySQL # 1) runs unit tests w/ Postgres local DB. Runs against H2, SQL Server # 2) runs unit tests w/ MySQL local DB. Runs against H2, Postgres, SQLite - # 3) runs unit tests w/ H2 local DB. Runs against H2, Redshift + # 3) runs unit tests w/ H2 local DB. Runs against H2, Redshift, Druid # 4) runs Eastwood linter & Bikeshed linter && ./bin/reflection-linter # 5) runs JS linter + JS test # 6) runs lein uberjar. (We don't run bin/build because we're not really concerned about `npm install` (etc) in this test, which runs elsewhere) - - case $CIRCLE_NODE_INDEX in 0) ENGINES=h2,mongo,mysql lein test ;; 1) ENGINES=h2,sqlserver MB_DB_TYPE=postgres MB_DB_DBNAME=circle_test MB_DB_PORT=5432 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 2) ENGINES=h2,postgres,sqlite MB_DB_TYPE=mysql MB_DB_DBNAME=circle_test MB_DB_PORT=3306 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 3) ENGINES=h2,redshift lein test ;; 4) lein eastwood 2>&1 | grep -v Reflection && lein bikeshed 2>&1 | grep -v Reflection && ./bin/reflection-linter ;; 5) npm install && npm run lint && npm run build && npm run test ;; 6) lein uberjar ;; esac: + - case $CIRCLE_NODE_INDEX in 0) ENGINES=h2,mongo,mysql lein test ;; 1) ENGINES=h2,sqlserver MB_DB_TYPE=postgres MB_DB_DBNAME=circle_test MB_DB_PORT=5432 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 2) ENGINES=h2,postgres,sqlite MB_DB_TYPE=mysql MB_DB_DBNAME=circle_test MB_DB_PORT=3306 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 3) ENGINES=h2,redshift,druid lein test ;; 4) lein eastwood 2>&1 | grep -v Reflection && lein bikeshed 2>&1 | grep -v Reflection && ./bin/reflection-linter ;; 5) npm install && npm run lint && npm run build && npm run test ;; 6) lein uberjar ;; esac: parallel: true deployment: master: diff --git a/docs/operations-guide/running-metabase-on-heroku.md b/docs/operations-guide/running-metabase-on-heroku.md index f19ab1a977c302849714bc59c1fb08c71ea4b75b..faafe6b2f296fda10aaf2b980135eb9931025d9b 100644 --- a/docs/operations-guide/running-metabase-on-heroku.md +++ b/docs/operations-guide/running-metabase-on-heroku.md @@ -20,3 +20,25 @@ This will launch a Heroku deployment using a github repository that Metabase mai * If you don’t access the application for a while Heroku will sleep your Metabase environment. This prevents things like Pulses and Metabase background tasks from running when scheduled and at times makes the app appear to be slow when really it's just Heroku reloading your app. Now that you’ve installed Metabase, it’s time to [set it up and connect it to your database](../setting-up-metabase.md). + + +# Deploying New Versions of Metabase + +Upgrading to the next version of Metabase is a simple process where you will grab the latest version of [metabase-deploy](https://github.com/metabase/metabase-deploy) and push it to Heroku. + +Here's each step: + +* Clone the latest version to your local machine: +``` +git clone https://github.com/metabase/metabase-deploy.git +cd metabase-deploy +``` +* Add a git remote with your metabase setup: +``` +git remote add heroku https://git.heroku.com/your-metabase-app.git +``` +* Force push the new version to Heroku: +``` +git push -f heroku master +``` +* Wait for the deploy to finish diff --git a/frontend/src/admin/datamodel/components/database/ColumnItem.jsx b/frontend/src/admin/datamodel/components/database/ColumnItem.jsx index 2dccb88d78e785bb8a9159bfa8a8918a31a85db6..f540070c6ccf7f460f0bf366a6b562e32acb4530 100644 --- a/frontend/src/admin/datamodel/components/database/ColumnItem.jsx +++ b/frontend/src/admin/datamodel/components/database/ColumnItem.jsx @@ -3,7 +3,7 @@ import React, { Component, PropTypes } from "react"; import Input from "metabase/components/Input.jsx"; import Select from "metabase/components/Select.jsx"; -import MetabaseCore from "metabase/lib/core"; +import * as MetabaseCore from "metabase/lib/core"; import _ from "underscore"; diff --git a/frontend/src/admin/people/components/UserRoleSelect.jsx b/frontend/src/admin/people/components/UserRoleSelect.jsx index 0f73ee9b0e7e320e01a3ec837f1ed5b1ede83d7a..1012295c00e388f5f37adde5be437a36f21a4342 100644 --- a/frontend/src/admin/people/components/UserRoleSelect.jsx +++ b/frontend/src/admin/people/components/UserRoleSelect.jsx @@ -3,7 +3,7 @@ import cx from "classnames"; import ColumnarSelector from "metabase/components/ColumnarSelector.jsx"; import Icon from "metabase/components/Icon.jsx"; -import MetabaseCore from "metabase/lib/core"; +import * as MetabaseCore from "metabase/lib/core"; import PopoverWithTrigger from "metabase/components/PopoverWithTrigger.jsx"; diff --git a/frontend/src/card/card.charting.js b/frontend/src/card/card.charting.js index 68a2c132cd9aeb1070d7fa370bf525fd1e1db28d..232f70d0dbc364ff61551181ef53472f785860bf 100644 --- a/frontend/src/card/card.charting.js +++ b/frontend/src/card/card.charting.js @@ -85,16 +85,6 @@ function dimensionIsTimeseries(result) { return (hasDateField || isDateFirstVal); } -/// return the Element ID that should be used to find chart with a given chartId -function chartElementIdForId(chartId) { - return 'card-inner--' + chartId; -} - -/// return the DOM element where chart with CHARTID -function chartElementForId(chartId) { - return document.getElementById(chartElementIdForId(chartId)); -} - // return computed property of element or element with ID. Returns null if element is not found function getComputedProperty(prop, elementOrId) { if (typeof elementOrId === 'string') elementOrId = document.getElementById(elementOrId); @@ -133,14 +123,14 @@ function getDcjsChartType(cardType) { } } -function initializeChart(card, elementId, defaultWidth, defaultHeight, chartType) { +function initializeChart(card, element, defaultWidth, defaultHeight, chartType) { chartType = (chartType) ? chartType : getDcjsChartType(card.display); // create the chart - var chart = dc[chartType]('#' + chartElementIdForId(elementId)); + var chart = dc[chartType](element); // set width and height - chart = applyChartBoundary(chart, card, elementId, defaultWidth, defaultHeight); + chart = applyChartBoundary(chart, card, element, defaultWidth, defaultHeight); // specify legend chart = applyChartLegend(chart, card); @@ -148,19 +138,16 @@ function initializeChart(card, elementId, defaultWidth, defaultHeight, chartType // disable animations chart.transitionDuration(0); - // set card title - setCardTitle(card, elementId); - return chart; } -function applyChartBoundary(dcjsChart, card, elementId, defaultWidth, defaultHeight) { - return dcjsChart - .width(CardRenderer.getAvailableCanvasWidth(elementId, card) || defaultWidth) - .height(CardRenderer.getAvailableCanvasHeight(elementId, card) || defaultHeight); +function applyChartBoundary(chart, card, element, defaultWidth, defaultHeight) { + return chart + .width(CardRenderer.getAvailableCanvasWidth(element, card) || defaultWidth) + .height(CardRenderer.getAvailableCanvasHeight(element, card) || defaultHeight); } -function applyChartLegend(dcjsChart, card) { +function applyChartLegend(chart, card) { // ENABLE LEGEND IF SPECIFIED IN VISUALIZATION SETTINGS // I'm sure it made sense to somebody at some point to make this setting live in two different places depending on the type of chart. var settings = card.visualization_settings, @@ -173,21 +160,9 @@ function applyChartLegend(dcjsChart, card) { } if (legendEnabled) { - return dcjsChart.legend(dc.legend()); + return chart.legend(dc.legend()); } else { - return dcjsChart; - } -} - -function setCardTitle(card, elementId) { - // SET THE CARD TITLE if applicable (probably not, since there's no UI to set this AFAIK) - var settings = card.visualization_settings, - chartTitle = settings.global.title; - if (chartTitle) { - var titleElement = document.getElementById('card-title--' + elementId); - if (titleElement) { - titleElement.innerText = chartTitle; - } + return chart; } } @@ -382,23 +357,23 @@ function applyChartYAxis(chart, card, coldefs, data, minPixelsPerTick) { } } -function applyChartColors(dcjsChart, card) { +function applyChartColors(chart, card) { // Set the color for the bar/line let settings = card.visualization_settings; let chartColor = (card.display === 'bar') ? settings.bar.color : settings.line.lineColor; let colorList = (card.display === 'bar') ? settings.bar.colors : settings.line.colors; // dedup colors list to ensure stacked charts don't have the same color let uniqueColors = _.uniq([chartColor].concat(colorList)); - return dcjsChart.ordinalColors(uniqueColors); + return chart.ordinalColors(uniqueColors); } -function applyChartTooltips(dcjsChart, id, card, cols) { - dcjsChart.on('renderlet', function(chart) { +function applyChartTooltips(chart, element, card, cols) { + chart.on('renderlet', function(chart) { // Remove old tooltips which are sometimes not removed due to chart being rerendered while tip is visible - Array.prototype.forEach.call(document.querySelectorAll('.ChartTooltip--'+id), (t) => t.parentNode.removeChild(t)); + Array.prototype.forEach.call(document.querySelectorAll('.ChartTooltip--'+element.id), (t) => t.parentNode.removeChild(t)); var tip = d3.tip() - .attr('class', 'ChartTooltip ChartTooltip--'+id) + .attr('class', 'ChartTooltip ChartTooltip--'+element.id) .direction('n') .offset([-10, 0]) .html(function(d) { @@ -416,7 +391,7 @@ function applyChartTooltips(dcjsChart, id, card, cols) { .on('mouseover.tip', function(slice) { tip.show.apply(tip, arguments); if (card.display === "pie") { - var tooltip = d3.select('.ChartTooltip--'+id); + var tooltip = d3.select('.ChartTooltip--'+element.id); let tooltipOffset = getTooltipOffset(tooltip); let sliceCentroid = getPieSliceCentroid(this, slice); tooltip.style({ @@ -466,9 +441,9 @@ function getTooltipOffset(tooltip) { }; } -function lineAndBarOnRender(dcjsChart, card) { +function lineAndBarOnRender(chart, card) { // once chart has rendered and we can access the SVG, do customizations to axis labels / etc that you can't do through dc.js - var svg = dcjsChart.svg(), + var svg = chart.svg(), settings = card.visualization_settings, x = settings.xAxis, y = settings.yAxis; @@ -524,8 +499,8 @@ function lineAndBarOnRender(dcjsChart, card) { } catch (e) {} // adjust the margins to fit the Y-axis tick label sizes, and rerender - dcjsChart.margins().left = dcjsChart.select(".axis.y")[0][0].getBBox().width + 20; - dcjsChart.render(); + chart.margins().left = chart.select(".axis.y")[0][0].getBBox().width + 20; + chart.render(); } @@ -543,13 +518,13 @@ function lineAndBarOnRender(dcjsChart, card) { /// 2) Code in ChartRenderer(...) runs and does setup common across all charts /// 3) Code in GeoHeatmapChartRenderer(...) runs and does setup common across the charts /// 4) Further customizations specific to bar charts take place in .customize() -function ChartRenderer(id, card, result, chartType) { +function ChartRenderer(element, card, result, chartType) { // ------------------------------ CONSTANTS ------------------------------ // var DEFAULT_CARD_WIDTH = 900, DEFAULT_CARD_HEIGHT = 500; // ------------------------------ PROPERTIES ------------------------------ // - this.id = id; + this.element = element; this.card = card; this.result = result; this.chartType = chartType; @@ -613,17 +588,16 @@ function ChartRenderer(id, card, result, chartType) { /// determine what width we should use for the chart - we can look at size of the card header / footer and match that this._getWidth = function() { - return CardRenderer.getAvailableCanvasWidth(this.id, this.card) || DEFAULT_CARD_WIDTH; + return CardRenderer.getAvailableCanvasWidth(this.element, this.card) || DEFAULT_CARD_WIDTH; }; /// height available to card for the chart, if available. Equal to height of card minus heights of header + footer. this._getHeight = function() { - return CardRenderer.getAvailableCanvasHeight(this.id, this.card) || DEFAULT_CARD_HEIGHT; + return CardRenderer.getAvailableCanvasHeight(this.element, this.card) || DEFAULT_CARD_HEIGHT; }; // ------------------------------ INITIALIZATION ------------------------------ // - this.chartFn = dc[this.chartType]; // e.g. dc['geoChoroplethChart] - this.chart = this.chartFn('#' + chartElementIdForId(this.id)) + this.chart = dc[this.chartType](this.element) .width(this._getWidth()) .height(this._getHeight()); @@ -631,18 +605,9 @@ function ChartRenderer(id, card, result, chartType) { // I'm sure it made sense to somebody at some point to make this setting live in two different places depending on the type of chart. var legendEnabled = chartType === 'pieChart' ? this.settings.pie.legend_enabled : this.settings.chart.legend_enabled; if (legendEnabled) this.chart.legend(dc.legend()); - - // SET THE CARD TITLE if applicable (probably not, since there's no UI to set this AFAIK) - var chartTitle = this.settings.global.title; - if (chartTitle) { - var titleElement = document.getElementById('card-title--' + id); - if (titleElement) { - titleElement.innerText = chartTitle; - } - } } -function GeoHeatmapChartRenderer(id, card, result) { +function GeoHeatmapChartRenderer(element, card, result) { // ------------------------------ CONSTANTS ------------------------------ // /// various shades that should be used in State + World Heatmaps /// TODO - These colors are from the dc.js examples and aren't the same ones we used on highcharts. Do we want custom Metabase colors? @@ -662,7 +627,7 @@ function GeoHeatmapChartRenderer(id, card, result) { HEAT_MAP_ZERO_COLOR = '#CCC'; // ------------------------------ SUPERCLASS INIT ------------------------------ // - ChartRenderer.call(this, id, card, result, 'geoChoroplethChart'); + ChartRenderer.call(this, element, card, result, 'geoChoroplethChart'); // ------------------------------ METHODS ------------------------------ // this.superSetData = this.setData; @@ -734,96 +699,47 @@ export var CardRenderer = { }, /// height available for rendering the card - getAvailableCanvasHeight: function(id, cardOrDimension) { + getAvailableCanvasHeight: function(element, cardOrDimension) { var sizeSettings = CardRenderer._getSizeSettings(cardOrDimension), initialHeight = sizeSettings ? sizeSettings.initialHeight : undefined; if (typeof cardOrDimension === "number") { initialHeight = cardOrDimension; } - - // if we already have size settings subtract height of header + footer and return if (typeof initialHeight !== "undefined") { - var headerHeight = getComputedHeight(id + '_heading'); - return initialHeight - headerHeight - 5; // why the magic number :/ + return initialHeight; } - // if we can find the chart element in the DOM then max width is parent element - parent x padding - var chartElement = chartElementForId(id); - if (chartElement) { - var parent = chartElement.parentElement, - parentHeight = getComputedHeight(parent), - parentPaddingTop = getComputedSizeProperty('padding-top', parent), - parentPaddingBottom = getComputedSizeProperty('padding-bottom', parent); - - // NOTE: if this magic number is not 3 we can get into infinite re-render loops - return parentHeight - parentPaddingTop - parentPaddingBottom - 3; // why the magic number :/ - } + var parent = element.parentElement, + parentHeight = getComputedHeight(parent), + parentPaddingTop = getComputedSizeProperty('padding-top', parent), + parentPaddingBottom = getComputedSizeProperty('padding-bottom', parent); - return null; + // NOTE: if this magic number is not 3 we can get into infinite re-render loops + return parentHeight - parentPaddingTop - parentPaddingBottom - 3; // why the magic number :/ }, /// width available for rendering the card - getAvailableCanvasWidth: function(id, cardOrDimension) { + getAvailableCanvasWidth: function(element, cardOrDimension) { var sizeSettings = CardRenderer._getSizeSettings(cardOrDimension), initialWidth = sizeSettings ? sizeSettings.initialWidth : undefined; if (typeof cardOrDimension === "number") { initialWidth = cardOrDimension; } - if (typeof initalWidth !== 'undefined') return initialWidth; - - // if we can find the chart element in the DOM then max width is parent element - parent x padding - var chartElement = chartElementForId(id); - if (chartElement) { - var parent = chartElement.parentElement, - parentWidth = getComputedWidth(parent), - parentPaddingLeft = getComputedSizeProperty('padding-left', parent), - parentPaddingRight = getComputedSizeProperty('padding-right', parent); - - return parentWidth - parentPaddingLeft - parentPaddingRight; + if (typeof initialWidth !== 'undefined') { + return initialWidth; } - // otherwise try to return the width of #CHARTID_heading or null if that fails - return getComputedWidth(id + '_heading'); - }, - - /// set the size of chart/table - /// called every time a table is displayed, or when charts are resized on a Dashboard - setSize: function(id, height, width) { - height = CardRenderer.getAvailableCanvasHeight(id, height); - width = CardRenderer.getAvailableCanvasWidth(id, width); - if (height === null) { - height = 450; - } - var el = document.getElementById(id); - if (el) { - el.style.height = height + "px"; - if (width !== null) { - el.style.width = width + "px"; - } - el.dispatchEvent(new Event("cardrenderer-card-resized")); - } + var parent = element.parentElement, + parentWidth = getComputedWidth(parent), + parentPaddingLeft = getComputedSizeProperty('padding-left', parent), + parentPaddingRight = getComputedSizeProperty('padding-right', parent); - // dynamically resize the chart if applicable - var chartRootElement = chartElementForId(id); - if (chartRootElement) { - // find the dc.js Chart instance for that corresponds to this element - var chart = _.filter(dc.chartRegistry.list(), function(dcChart) { - return dcChart.root()[0][0] === chartRootElement; - })[0]; - // update size, width then re-render the chart - if (height) chart.height(height); - if (width) chart.width(width); - - // HACK! pie chart doesn't resize properly unless you also update radius - if (typeof chart.radius !== 'undefined') chart.radius(Math.min(height, width) / 2); - - chart.render(); - } + return parentWidth - parentPaddingLeft - parentPaddingRight; }, - pie: function(id, card, result) { + pie: function(element, card, result) { var settings = card.visualization_settings, data = _.map(result.rows, function(row) { return { @@ -845,7 +761,7 @@ export var CardRenderer = { group = dimension.group().reduceSum(function(d) { return d.value; }), - chart = initializeChart(card, id, DEFAULT_CARD_WIDTH, DEFAULT_CARD_HEIGHT) + chart = initializeChart(card, element, DEFAULT_CARD_WIDTH, DEFAULT_CARD_HEIGHT) .dimension(dimension) .group(group) .colors(settings.pie.colors) @@ -861,12 +777,12 @@ export var CardRenderer = { // disables ability to select slices chart.filter = function() {}; - applyChartTooltips(chart, id, card, result.cols); + applyChartTooltips(chart, element, card, result.cols); chart.render(); }, - bar: function(id, card, result) { + bar: function(element, card, result) { var isTimeseries = (dimensionIsTimeseries(result)) ? true : false; var isMultiSeries = (result.cols !== undefined && result.cols.length > 2) ? true : false; @@ -890,7 +806,7 @@ export var CardRenderer = { group = dimension.group().reduceSum(function(d) { return d[1]; }), - chart = initializeChart(card, id, DEFAULT_CARD_WIDTH, DEFAULT_CARD_HEIGHT) + chart = initializeChart(card, element, DEFAULT_CARD_WIDTH, DEFAULT_CARD_HEIGHT) .dimension(dimension) .group(group) .valueAccessor(function(d) { @@ -924,7 +840,7 @@ export var CardRenderer = { // TODO: if we are multi-series this could be split axis applyChartYAxis(chart, card, result.cols, data, MIN_PIXELS_PER_TICK.y); - applyChartTooltips(chart, id, card, result.cols); + applyChartTooltips(chart, element, card, result.cols); applyChartColors(chart, card); // if the chart supports 'brushing' (brush-based range filter), disable this since it intercepts mouse hovers which means we can't see tooltips @@ -940,7 +856,7 @@ export var CardRenderer = { lineAndBarOnRender(chart, card); }, - line: function(id, card, result, isAreaChart, isTimeseries) { + line: function(element, card, result, isAreaChart, isTimeseries) { isAreaChart = typeof isAreaChart === undefined ? false : isAreaChart; isTimeseries = ((typeof isAreaChart !== undefined && isTimeseries) || dimensionIsTimeseries(result)) ? true : false; @@ -966,7 +882,7 @@ export var CardRenderer = { group = dimension.group().reduceSum(function(d) { return d[1]; }), - chart = initializeChart(card, id, DEFAULT_CARD_WIDTH, DEFAULT_CARD_HEIGHT) + chart = initializeChart(card, element, DEFAULT_CARD_WIDTH, DEFAULT_CARD_HEIGHT) .dimension(dimension) .group(group) .valueAccessor(function(d) { @@ -1001,7 +917,7 @@ export var CardRenderer = { // TODO: if we are multi-series this could be split axis applyChartYAxis(chart, card, result.cols, data, MIN_PIXELS_PER_TICK.y); - applyChartTooltips(chart, id, card, result.cols); + applyChartTooltips(chart, element, card, result.cols); applyChartColors(chart, card); // if the chart supports 'brushing' (brush-based range filter), disable this since it intercepts mouse hovers which means we can't see tooltips @@ -1019,17 +935,11 @@ export var CardRenderer = { /// Area Chart is just a Line Chart that we called renderArea(true) on /// Defer to CardRenderer.line() and pass param area = true - area: function(id, card, result) { - return CardRenderer.line(id, card, result, true); - }, - - /// TimeSeries is really just a Line Chart where the x-axis is time, so - /// Defer to CardRendered.line() and be explicit that we know timeseries = true - timeseries: function(id, card, result) { - return CardRenderer.line(id, card, result, false, true); + area: function(element, card, result) { + return CardRenderer.line(element, card, result, true); }, - state: function(id, card, result) { + state: function(element, card, result) { var chartData = _.map(result.rows, function(value) { return { stateCode: value[0], @@ -1037,7 +947,7 @@ export var CardRenderer = { }; }); - var chartRenderer = new GeoHeatmapChartRenderer(id, card, result) + var chartRenderer = new GeoHeatmapChartRenderer(element, card, result) .setData(chartData, 'stateCode', 'value') .setJson('/app/charts/us-states.json', function(d) { return d.properties.name; @@ -1054,7 +964,7 @@ export var CardRenderer = { return chartRenderer; }, - country: function(id, card, result) { + country: function(element, card, result) { var chartData = _.map(result.rows, function(value) { // Does this actually make sense? If country is > 2 characters just use the first 2 letters as the country code ?? (WTF) var countryCode = value[0]; @@ -1068,7 +978,7 @@ export var CardRenderer = { }; }); - var chartRenderer = new GeoHeatmapChartRenderer(id, card, result) + var chartRenderer = new GeoHeatmapChartRenderer(element, card, result) .setData(chartData, 'code', 'value') .setJson('/app/charts/world.json', function(d) { return d.properties.ISO_A2; // 2-letter country code @@ -1084,49 +994,7 @@ export var CardRenderer = { return chartRenderer; }, - ll_heatmap: function(id, card, result) { - var mapOptions = { - zoom: 2, - center: new google.maps.LatLng(result.average_latitude, result.average_longitude), - mapTypeId: google.maps.MapTypeId.MAP - }; - - var map = new google.maps.Map(document.getElementById(id), mapOptions); - - var southWest = new google.maps.LatLng(result.min_latitude, result.min_longitude), - northEast = new google.maps.LatLng(result.max_latitude, result.max_longitude), - average = new google.maps.LatLng(result.average_latitude, result.average_longitude); - - var bounds = new google.maps.LatLngBounds(); - bounds.extend(southWest); - bounds.extend(northEast); - bounds.extend(average); - map.fitBounds(bounds); - - var pointData = []; - for (var i = 0; i < result.rows.length; i++) { - pointData.push(new google.maps.LatLng(result.rows[i][0], result.rows[i][1])); - } - var pointArray = new google.maps.MVCArray(pointData); - - var heatmap = new google.maps.visualization.HeatmapLayer({ - data: pointArray, - radius: 16, - maxIntensity: 4 - }); - - heatmap.setMap(map); - var height = CardRenderer.getAvailableCanvasHeight(id, card), - width = CardRenderer.getAvailableCanvasWidth(id, card); - if (!height) height = 450; - - document.getElementById(id).style.height = height + "px"; - if (width !== null) { - document.getElementById(id).style.width = width + "px"; - } - }, - - pin_map: function(id, card, updateMapCenter, updateMapZoom) { + pin_map: function(element, card, updateMapCenter, updateMapZoom) { var query = card.dataset_query, vs = card.visualization_settings, latitude_dataset_col_index = vs.map.latitude_dataset_col_index, @@ -1160,17 +1028,18 @@ export var CardRenderer = { tileSize: new google.maps.Size(256, 256) }); - var height = CardRenderer.getAvailableCanvasHeight(id, card), - width = CardRenderer.getAvailableCanvasWidth(id, card); + var height = CardRenderer.getAvailableCanvasHeight(element, card); + var width = CardRenderer.getAvailableCanvasWidth(element, card); - if (height === null) height = 450; - document.getElementById(id).style.height = height + "px"; + if (height !== null) { + element.style.height = height + "px"; + } if (width !== null) { - document.getElementById(id).style.width = width + "px"; + element.style.width = width + "px"; } - var map = new google.maps.Map(document.getElementById(id), mapOptions); + var map = new google.maps.Map(element, mapOptions); map.overlayMapTypes.push(markerImageMapType); @@ -1196,18 +1065,8 @@ export var CardRenderer = { //listen for resize event (internal to CardRenderer) //to let google maps api know about the resize //(see https://developers.google.com/maps/documentation/javascript/reference) - document.getElementById(id).addEventListener('cardrenderer-card-resized', function() { + element.addEventListener('cardrenderer-card-resized', function() { google.maps.event.trigger(map, 'resize'); }); - }, - - table: function(id, card) { - // set the size of the table's container to the initial - // size prescribed by gridster (if applicable). - if (card.render_settings !== undefined && - card.render_settings.size.initialHeight !== 'undefined' && - card.render_settings.size.initialWidth !== 'undefined') { - CardRenderer.setSize(id, card.render_settings.size.initialHeight, card.render_settings.size.initialWidth); - } } }; diff --git a/frontend/src/card/card.controllers.js b/frontend/src/card/card.controllers.js index 87d1d503bedf300c063ab6420f070388147f13e7..a900ed8adf29c10623a552a87e2100c459d16224 100644 --- a/frontend/src/card/card.controllers.js +++ b/frontend/src/card/card.controllers.js @@ -14,12 +14,14 @@ import SavedQuestionsApp from './containers/SavedQuestionsApp.jsx'; import { createStore, combineReducers } from "metabase/lib/redux"; import _ from "underscore"; -import MetabaseAnalytics from '../lib/analytics'; +import MetabaseAnalytics from "metabase/lib/analytics"; import DataGrid from "metabase/lib/data_grid"; - import Query from "metabase/lib/query"; import { serializeCardForUrl, deserializeCardFromUrl, cleanCopyCard, urlForCardState } from "metabase/lib/card"; import { loadTable } from "metabase/lib/table"; +import { getDefaultColor } from "metabase/lib/visualization_settings"; + +import NotFound from "metabase/components/NotFound.jsx"; import * as reducers from './reducers'; @@ -40,8 +42,8 @@ CardControllers.controller('CardList', ['$scope', '$location', function($scope, }]); CardControllers.controller('CardDetail', [ - '$rootScope', '$scope', '$route', '$routeParams', '$location', '$q', '$window', '$timeout', 'Card', 'Dashboard', 'Metabase', 'VisualizationSettings', 'Revision', - function($rootScope, $scope, $route, $routeParams, $location, $q, $window, $timeout, Card, Dashboard, Metabase, VisualizationSettings, Revision) { + '$rootScope', '$scope', '$route', '$routeParams', '$location', '$q', '$window', '$timeout', 'Card', 'Dashboard', 'Metabase', 'Revision', + function($rootScope, $scope, $route, $routeParams, $location, $q, $window, $timeout, Card, Dashboard, Metabase, Revision) { // promise helper $q.resolve = function(object) { var deferred = $q.defer(); @@ -81,6 +83,7 @@ CardControllers.controller('CardDetail', [ tableForeignKeyReferences = null, isRunning = false, isObjectDetail = false, + isShowingTutorial = $routeParams.tutorial, card = { name: null, public_perms: 0, @@ -171,7 +174,6 @@ CardControllers.controller('CardDetail', [ }; var visualizationModel = { - visualizationSettingsApi: VisualizationSettings, card: null, result: null, databases: null, @@ -186,7 +188,7 @@ CardControllers.controller('CardDetail', [ var vizSettings = card.visualization_settings; // if someone picks the default color then clear any color settings - if (color === VisualizationSettings.getDefaultColor()) { + if (color === getDefaultColor()) { // NOTE: this only works if setting color is the only option we allow card.visualization_settings = {}; @@ -332,7 +334,7 @@ CardControllers.controller('CardDetail', [ // ensure rendering model is up to date editorModel.isRunning = isRunning; editorModel.isShowingDataReference = $scope.isShowingDataReference; - editorModel.isShowingTutorial = !!$routeParams.tutorial; + editorModel.isShowingTutorial = isShowingTutorial; editorModel.databases = databases; editorModel.tableMetadata = tableMetadata; editorModel.tableForeignKeys = tableForeignKeys; @@ -373,19 +375,24 @@ CardControllers.controller('CardDetail', [ let tutorialModel = { onClose: () => { - $routeParams.tutorial = false; + isShowingTutorial = false; updateUrl(); renderAll(); } } function renderTutorial() { - tutorialModel.isShowingTutorial = !!$routeParams.tutorial; + tutorialModel.isShowingTutorial = isShowingTutorial; React.render( <span>{tutorialModel.isShowingTutorial && <QueryBuilderTutorial {...tutorialModel} /> }</span> , document.getElementById('react_qb_tutorial')); } + function renderNotFound() { + tutorialModel.isShowingTutorial = isShowingTutorial; + React.render(<NotFound></NotFound>, document.getElementById('react_qb_viz')); + } + var renderAll = _.debounce(function() { renderHeader(); renderEditor(); @@ -526,6 +533,13 @@ CardControllers.controller('CardDetail', [ tables = null; tableMetadata = null; + let db = _.findWhere(databases, { id: databaseId }); + if (db && db.tables) { + tables = db.tables; + renderAll(); + return; + } + // get tables for db Metabase.db_tables({ 'dbId': databaseId @@ -717,17 +731,17 @@ CardControllers.controller('CardDetail', [ return card; } - async function loadCard() { - if ($routeParams.cardId != undefined) { - var card = await loadSavedCard($routeParams.cardId); - if ($routeParams.serializedCard) { - let serializedCard = await loadSerializedCard($routeParams.serializedCard); - return _.extend(card, serializedCard); + async function loadCard(cardId, serializedCard) { + if (cardId != undefined) { + var card = await loadSavedCard(cardId); + if (serializedCard) { + let deserializedCard = await loadSerializedCard(serializedCard); + return _.extend(card, deserializedCard); } else { return card; } - } else if ($routeParams.serializedCard != undefined) { - return loadSerializedCard($routeParams.serializedCard); + } else if (serializedCard) { + return loadSerializedCard(serializedCard); } else { return loadNewCard(); } @@ -765,7 +779,10 @@ CardControllers.controller('CardDetail', [ // meant to be called once on controller startup async function loadAndSetCard() { try { - let card = await loadCard(); + const cardId = $routeParams.cardId; + const serializedCard = _.isEmpty($location.hash()) ? null : $location.hash(); + + let card = await loadCard(cardId, serializedCard); if ($routeParams.clone) { delete card.id; card.isDirty = true; @@ -775,9 +792,8 @@ CardControllers.controller('CardDetail', [ delete card.isDirty; return setCard(card, { setDirty: isDirty, resetDirty: !isDirty, replaceState: true }); } catch (error) { - if (error.status == 404) { - // TODO() - we should redirect to the card builder with no query instead of / - $location.path('/'); + if (error.status === 404) { + renderNotFound(); } } } @@ -802,6 +818,7 @@ CardControllers.controller('CardDetail', [ function reloadCard() { delete $routeParams.serializedCard; + $location.hash(null); loadAndSetCard(); } @@ -813,7 +830,7 @@ CardControllers.controller('CardDetail', [ // needs to be performed asynchronously otherwise we get weird infinite recursion var updateUrl = (replaceState) => setTimeout(function() { // don't update the URL if we're currently showing the tutorial - if (!!$routeParams.tutorial) { + if (isShowingTutorial) { return; } @@ -867,8 +884,12 @@ CardControllers.controller('CardDetail', [ React.unmountComponentAtNode(document.getElementById('react_data_reference')); }); + // prevent angular route change when we manually update the url + // NOTE: we tried listening on $locationChangeStart and simply canceling that, but doing so prevents the history and everything + // and ideally we'd simply listen on $routeChangeStart and cancel that when it's the same controller, but that doesn't work :( - // mildly hacky way to prevent reloading controllers as the URL changes + // mildly hacky way to prevent reloading controllers as the URL changes + // this works by setting the new route to the old route and manually moving over params var route = $route.current; $scope.$on('$locationChangeSuccess', function (event) { var newParams = $route.current.params; @@ -877,7 +898,9 @@ CardControllers.controller('CardDetail', [ // reload the controller if: // 1. not CardDetail // 2. both serializedCard and cardId are not set (new card) - if ($route.current.$$route.controller === 'CardDetail' && (newParams.serializedCard || newParams.cardId)) { + // TODO: is there really ever a reason to reload this route if we are going to the same place? + const serializedCard = _.isEmpty($location.hash()) ? null : $location.hash(); + if ($route.current.$$route.controller === 'CardDetail' && (serializedCard || newParams.cardId)) { $route.current = route; angular.forEach(oldParams, function(value, key) { @@ -891,19 +914,9 @@ CardControllers.controller('CardDetail', [ } }); - // TODO: while we wait for the databases list we should put something on screen - // grab our database list, then handle the rest - async function loadDatabasesAndTables() { - let dbs = await Metabase.db_list().$promise; - return await * dbs.map(async function(db) { - db.tables = await Metabase.db_tables({ dbId: db.id }).$promise; - return db; - }); - } - async function init() { try { - databases = await loadDatabasesAndTables(); + databases = await Metabase.db_list_with_tables().$promise; if (databases.length < 1) { // TODO: some indication that setting up a db is required @@ -913,7 +926,7 @@ CardControllers.controller('CardDetail', [ // finish initializing our page and render await loadAndSetCard(); - if (!!$routeParams.tutorial) { + if (isShowingTutorial) { setSampleDataset(); } } catch (error) { diff --git a/frontend/src/card/card.module.js b/frontend/src/card/card.module.js index 30e6c58530d38ccc5dbed4f1a281945f9c6396df..82ddc6da672aa70d634721e8b9b7abb77c6493db 100644 --- a/frontend/src/card/card.module.js +++ b/frontend/src/card/card.module.js @@ -5,7 +5,6 @@ var Card = angular.module('metabase.card', [ 'metabase.filters', 'metabase.directives', 'metabase.services', - 'metabase.card.services', 'metabase.card.controllers', 'metabase.card.directives' ]); @@ -15,17 +14,21 @@ Card.config(['$routeProvider', function($routeProvider) { templateUrl: '/app/card/partials/card_detail.html', controller: 'CardDetail' }); - $routeProvider.when('/q/:serializedCard', { - templateUrl: '/app/card/partials/card_detail.html', - controller: 'CardDetail' - }); $routeProvider.when('/card/:cardId', { templateUrl: '/app/card/partials/card_detail.html', controller: 'CardDetail' }); + + // redirect old urls to new ones with hashes + $routeProvider.when('/q/:serializedCard', { + redirectTo: function (routeParams, path, search) { + return "/q#"+routeParams.serializedCard; + } + }); $routeProvider.when('/card/:cardId/:serializedCard', { - templateUrl: '/app/card/partials/card_detail.html', - controller: 'CardDetail' + redirectTo: function (routeParams, path, search) { + return "/card/"+routeParams.cardId+"#"+routeParams.serializedCard; + } }); $routeProvider.when('/card/', { diff --git a/frontend/src/card/card.services.js b/frontend/src/card/card.services.js deleted file mode 100644 index 7dd5696c01498c49b17e2c788c25005fe35d61a9..0000000000000000000000000000000000000000 --- a/frontend/src/card/card.services.js +++ /dev/null @@ -1,359 +0,0 @@ -import _ from "underscore"; - -import { normal, harmony } from 'metabase/lib/colors' - -// Card Services -var CardServices = angular.module('metabase.card.services', ['ngResource', 'ngCookies']); - -CardServices.service('VisualizationUtils', [function() { - this.visualizationTypes = { - scalar: { - display: 'scalar', - label: 'Scalar', - available: false, - notAvailableReasons: [] - }, - table: { - display: 'table', - label: 'Table', - available: false, - notAvailableReasons: [] - }, - pie: { - display: 'pie', - label: 'Pie Chart', - available: false, - notAvailableReasons: [] - }, - bar: { - display: 'bar', - label: 'Bar Chart', - available: false, - notAvailableReasons: [] - }, - line: { - display: 'line', - label: 'Line Chart', - available: false, - notAvailableReasons: [] - }, - area: { - display: 'area', - label: 'Area Chart', - available: false, - notAvailableReasons: [] - }, - timeseries: { - display: 'timeseries', - label: 'Time Series', - available: false, - notAvailableReasons: [] - }, - pin_map: { - display: 'pin_map', - label: 'Pin Map', - available: false, - notAvailableReasons: [] - }, - state: { - display: 'state', - label: 'State Heatmap', - available: false, - notAvailableReasons: [] - }, - country: { - display: 'country', - label: 'World Heatmap', - available: false, - notAvailableReasons: [] - } - }; - - this.zoomTypes = [{ - 'label': 'Disabled', - 'value': null - }, { - 'label': 'X', - 'value': 'x' - }, { - 'label': 'Y', - 'value': 'y' - }, { - 'label': 'XY', - 'value': 'xy' - }]; -}]); - - -CardServices.service('VisualizationSettings', [function() { - - var DEFAULT_COLOR_HARMONY = Object.values(normal); - var DEFAULT_COLOR = DEFAULT_COLOR_HARMONY[0]; - - var EXPANDED_COLOR_HARMONY = harmony; - - /* *** visualization settings *** - * - * This object defines default settings for card visualizations (i.e. charts, maps, etc). - * Each visualization type can be associated with zero or more top-level settings groups defined in this object - * (i.e. line charts may use 'chart', 'xAxis', 'yAxis', 'line'), depending on the settings that are appropriate - * for the particular visualization type (the associations are defined below in groupsForVisualizations). - * - * Before a card renders, the default settings from the appropriate groups are first copied from this object, - * creating an in-memory default settings object for that rendering. - * Then, a settings object stored in the card's record in the database is read and any attributes defined there - * are applied to that in-memory default settings object (using _.defaults()). - * The resulting in-memory settings object is made available to the card renderer at the time - * visualization is rendered. - * - * The settings object stored in the DB is 'sparse': only settings that differ from the defaults - * (at the time the settings were set) are recorded in the DB. This allows us to easily change the appearance of - * visualizations globally, except in cases where the user has explicitly changed the default setting. - * - * Some settings accept aribtrary numbers or text (i.e. titles) and some settings accept only certain values - * (i.e. *_enabled settings must be one of true or false). However, this object does not define the constraints. - * Instead, the controller that presents the UI to change the settings is currently responsible for enforcing the - * appropriate contraints for each setting. - * - * Search for '*** visualization settings ***' in card.controllers.js to find the objects that contain - * choices for the settings that require them. - * Additional constraints are enforced by the input elements in the views for each settings group - * (see app/card/partials/settings/*.html). - * - */ - var settings = { - 'global': { - 'title': null - }, - 'columns': { - 'dataset_column_titles': [] //allows the user to define custom titles for each column in the resulting dataset. Each item in this array corresponds to a column in the dataset's data.columns array. - }, - 'chart': { - 'plotBackgroundColor': '#FFFFFF', - 'borderColor': '#528ec5', - 'zoomType': 'x', - 'panning': true, - 'panKey': 'shift', - 'export_menu_enabled': false, - 'legend_enabled': false - }, - 'xAxis': { - 'title_enabled': true, - 'title_text': null, - 'title_text_default_READONLY': 'Values', //copied into title_text when re-enabling title from disabled state; user will be expected to change title_text - 'title_color': "#707070", - 'title_font_size': 12, //in pixels - 'min': null, - 'max': null, - 'gridLine_enabled': false, - 'gridLineColor': '#999999', - 'gridLineWidth': 0, - 'gridLineWidth_default_READONLY': 1, //copied into gridLineWidth when re-enabling grid lines from disabled state - 'tickInterval': null, - 'labels_enabled': true, - 'labels_step': null, - 'labels_staggerLines': null - }, - 'yAxis': { - 'title_enabled': true, - 'title_text': null, - 'title_text_default_READONLY': 'Values', //copied into title_text when re-enabling title from disabled state; user will be expected to change title_text - 'title_color': "#707070", - 'title_font_size': 12, //in pixels - 'min': null, - 'max': null, - 'gridLine_enabled': true, - 'gridLineColor': '#999999', - 'gridLineWidth': 1, - 'gridLineWidth_default_READONLY': 1, //copied into gridLineWidth when re-enabling grid lines from disabled state - 'tickInterval': null, - 'labels_enabled': true, - 'labels_step': null - }, - 'line': { - 'lineColor': DEFAULT_COLOR, - 'colors': DEFAULT_COLOR_HARMONY, - 'lineWidth': 2, - 'step': false, - 'marker_enabled': true, - 'marker_fillColor': '#528ec5', - 'marker_lineColor': '#FFFFFF', - 'marker_radius': 2, - 'xAxis_column': null, - 'yAxis_columns': [] - }, - 'area': { - 'fillColor': DEFAULT_COLOR, - 'fillOpacity': 0.75 - }, - 'pie': { - 'legend_enabled': true, - 'dataLabels_enabled': false, - 'dataLabels_color': '#777', - 'connectorColor': '#999', - 'colors': EXPANDED_COLOR_HARMONY - }, - 'bar': { - 'colors': DEFAULT_COLOR_HARMONY, - 'color': DEFAULT_COLOR - }, - 'map': { - 'latitude_source_table_field_id': null, - 'longitude_source_table_field_id': null, - 'latitude_dataset_col_index': null, - 'longitude_dataset_col_index': null, - 'zoom': 10, - 'center_latitude': 37.7577, //defaults to SF ;-) - 'center_longitude': -122.4376 - } - }; - - var groupsForVisualizations = { - 'scalar': ['global'], - 'table': ['global', 'columns'], - 'pie': ['global', 'chart', 'pie'], - 'bar': ['global', 'columns', 'chart', 'xAxis', 'yAxis', 'bar'], - 'line': ['global', 'columns', 'chart', 'xAxis', 'yAxis', 'line'], - 'area': ['global', 'columns', 'chart', 'xAxis', 'yAxis', 'line', 'area'], - 'timeseries': ['global', 'columns', 'chart', 'xAxis', 'yAxis', 'line'], - 'country': ['global', 'columns', 'chart', 'map'], - 'state': ['global', 'columns', 'chart', 'map'], - 'pin_map': ['global', 'columns', 'chart', 'map'] - }; - - this.getDefaultColor = function() { - return DEFAULT_COLOR; - }; - - this.getDefaultColorHarmony = function() { - return DEFAULT_COLOR_HARMONY; - }; - - this.getSettingsForGroup = function(dbSettings, groupName) { - if (typeof dbSettings != "object") { - dbSettings = {}; - } - - if (typeof settings[groupName] == "undefined") { - return dbSettings; - } - - if (typeof dbSettings[groupName] == "undefined") { - dbSettings[groupName] = {}; - } - //make a deep copy of default settings, otherwise default settings that are objects - //will not be recognized as 'dirty' after changing the value in the UI, because - //_.defaults make a shallow copy, so objects / arrays are copied by reference, - //so changing the settings in the UI would change the default settings. - var newSettings = _.defaults(dbSettings[groupName], angular.copy(settings[groupName])); - - return newSettings; - }; - - this.getSettingsForGroups = function(dbSettings, groups) { - var newSettings = {}; - for (var i = 0; i < groups.length; i++) { - var groupName = groups[i]; - newSettings[groupName] = this.getSettingsForGroup(dbSettings, groupName); - } - return newSettings; - }; - - this.getSettingsGroupsForVisualization = function(visualization) { - var groups = ['global']; - if (typeof groupsForVisualizations[visualization] != "undefined") { - groups = groupsForVisualizations[visualization]; - } - return groups; - }; - - this.getSettingsForVisualization = function(dbSettings, visualization) { - var settings = angular.copy(dbSettings); - var groups = _.union(_.keys(settings), this.getSettingsGroupsForVisualization(visualization)); - return this.getSettingsForGroups(settings, groups); - }; - - //Clean visualization settings to only keep the settings that are "dirty". - //This is determined by comparing the state of the current settings model to the - //defaults provided by this service. - this.cleanUserSettings = function(userSettings, visualization) { - var defaultSettings = {}; - var cleanSettings = {}; - var groups = _.union(_.keys(userSettings), this.getSettingsGroupsForVisualization(visualization)); - for (var i = 0; i < groups.length; i++) { - var groupName = groups[i]; - defaultSettings[groupName] = settings[groupName]; - } - - _.each(userSettings, function(settings, category) { - var truncatedSettings = _.omit(userSettings[category], function(value, key) { - - if ((typeof defaultSettings[category] == "undefined") || (typeof defaultSettings[category][key] == "undefined")) { - return false; - } - return _.isEqual(defaultSettings[category][key], value); - - }); - if (_.keys(truncatedSettings).length > 0) { - cleanSettings[category] = truncatedSettings; - } - }); - - return cleanSettings; - }; - - this.getDefaultSettingsForVisualization = function(visualization) { - var groups = this.getSettingsGroupsForVisualization(visualization); - var defaults = {}; - for (var i = 0; i < groups.length; i++) { - var groupName = groups[i]; - if (typeof settings[groupName] != "undefined") { - defaults[groupName] = settings[groupName]; - } else { - console.log("WARN: no settings for " + groupName); - } - } - - return defaults; - }; - - this.setLatitudeAndLongitude = function(settings, columnDefs) { - // latitude - var latitudeColumn, - latitudeColumnIndex; - columnDefs.forEach(function(col, index) { - if (col.special_type && - col.special_type === "latitude" && - latitudeColumn === undefined) { - latitudeColumn = col; - latitudeColumnIndex = index; - } - }); - - // longitude - var longitudeColumn, - longitudeColumnIndex; - columnDefs.forEach(function(col, index) { - if (col.special_type && - col.special_type === "longitude" && - longitudeColumn === undefined) { - longitudeColumn = col; - longitudeColumnIndex = index; - } - }); - - if (latitudeColumn && longitudeColumn) { - var settingsWithLatAndLon = angular.copy(settings); - - settingsWithLatAndLon.map.latitude_source_table_field_id = latitudeColumn.id; - settingsWithLatAndLon.map.latitude_dataset_col_index = latitudeColumnIndex; - settingsWithLatAndLon.map.longitude_source_table_field_id = longitudeColumn.id; - settingsWithLatAndLon.map.longitude_dataset_col_index = longitudeColumnIndex; - - return settingsWithLatAndLon; - } else { - return settings; - } - }; - -}]); diff --git a/frontend/src/components/ColumnarSelector.jsx b/frontend/src/components/ColumnarSelector.jsx index a1aaf0ea75246e299220a891e9ea3da1085bf3db..fecf8e1de5227029f48524d22f05ead15ac4fb74 100644 --- a/frontend/src/components/ColumnarSelector.jsx +++ b/frontend/src/components/ColumnarSelector.jsx @@ -32,7 +32,7 @@ export default class ColumnarSelector extends Component { } return ( <li key={rowIndex}> - <a className={itemClasses} href="#" onClick={column.itemSelectFn.bind(null, item)}> + <a className={itemClasses} onClick={column.itemSelectFn.bind(null, item)}> {checkIcon} <div className="flex flex-column"> {column.itemTitleFn(item)} diff --git a/frontend/src/components/CreateDashboardModal.jsx b/frontend/src/components/CreateDashboardModal.jsx index 5dfe474f6a7abd2abc8b3f70751144d3ac2b93b2..6343714fee5a676a7f3eb5bfb68780a09eb36ec2 100644 --- a/frontend/src/components/CreateDashboardModal.jsx +++ b/frontend/src/components/CreateDashboardModal.jsx @@ -109,7 +109,7 @@ export default class CreateDashboardModal extends Component { <div className="Form-actions"> {createButton} - <span className="px1">or</span><a href="#" className="no-decoration text-brand text-bold" onClick={this.props.closeFn}>Cancel</a> + <span className="px1">or</span><a className="no-decoration text-brand text-bold" onClick={this.props.closeFn}>Cancel</a> {formError} </div> </form> diff --git a/frontend/src/components/DashboardsDropdown.jsx b/frontend/src/components/DashboardsDropdown.jsx index 8713a9ead5d5077f4242ab60a1da2fc6ac293d81..6d7a09c2a3d5dfeff61b723512706bea927ee97b 100644 --- a/frontend/src/components/DashboardsDropdown.jsx +++ b/frontend/src/components/DashboardsDropdown.jsx @@ -103,7 +103,7 @@ export default class DashboardsDropdown extends Component { <div className="px2 py1 text-bold">You don’t have any dashboards yet.</div> <div className="px2 pb2">Dashboards group visualizations for frequent questions in a single handy place.</div> <div className="border-top border-light"> - <a className="Dropdown-item block text-white no-decoration" href="#" onClick={this.toggleModal}>Create your first dashboard</a> + <a className="Dropdown-item block text-white no-decoration" onClick={this.toggleModal}>Create your first dashboard</a> </div> </div> : @@ -123,7 +123,7 @@ export default class DashboardsDropdown extends Component { </li> )} <li className="block border-top border-light"> - <a data-metabase-event={"Navbar;Dashboard Dropdown;Create Dashboard"} className="Dropdown-item block text-white no-decoration" href="#" onClick={this.toggleModal}>Create a new dashboard</a> + <a data-metabase-event={"Navbar;Dashboard Dropdown;Create Dashboard"} className="Dropdown-item block text-white no-decoration" onClick={this.toggleModal}>Create a new dashboard</a> </li> </ul> } diff --git a/frontend/src/components/ExplicitSize.jsx b/frontend/src/components/ExplicitSize.jsx new file mode 100644 index 0000000000000000000000000000000000000000..8a5c3262ee52b011ef0a8e3f0e072dc1c6671bde --- /dev/null +++ b/frontend/src/components/ExplicitSize.jsx @@ -0,0 +1,49 @@ +import React, { Component, PropTypes } from "react"; + +export default ComposedComponent => class extends Component { + constructor(props, context) { + super(props, context); + this.state = { + width: null, + height: null + }; + } + + static propTypes = { + className: PropTypes.any, + style: PropTypes.object + }; + + static defaultProps = { + className: "" + }; + + componentDidMount() { + this.componentDidUpdate(); + } + + componentDidUpdate() { + const { width, height } = React.findDOMNode(this).getBoundingClientRect() + if (this.state.width !== width || this.state.height !== height) { + this.setState({ width, height }); + } + } + + render() { + const { className, style } = this.props; + const { width, height } = this.state; + return ( + <div className={className} style={{ position: "relative", ...style }}> + <div style={{ position: "absolute", top: 0, left: 0, width: width, height: height }}> + { width != null && height != null && + <ComposedComponent + width={width} + height={height} + {...this.props} + /> + } + </div> + </div> + ); + } +} diff --git a/frontend/src/components/ModalContent.jsx b/frontend/src/components/ModalContent.jsx index ace5630ca4e4136e5ad5efdf1816b1550e22d8ff..27ec7b9047a1db871b035a7cdcb45059a89c2fd5 100644 --- a/frontend/src/components/ModalContent.jsx +++ b/frontend/src/components/ModalContent.jsx @@ -17,7 +17,7 @@ export default class ModalContent extends Component { <div className={this.props.className}> <div className="Modal-header Form-header flex align-center"> <h2 className="flex-full">{this.props.title}</h2> - <a href="#" className="text-grey-3 p1" onClick={this.props.closeFn}> + <a className="text-grey-3 p1" onClick={this.props.closeFn}> <Icon name='close' width="16px" height="16px"/> </a> </div> diff --git a/frontend/src/components/NotFound.jsx b/frontend/src/components/NotFound.jsx new file mode 100644 index 0000000000000000000000000000000000000000..08338513869a3f4fb12a48b912b4d7e77cf235a4 --- /dev/null +++ b/frontend/src/components/NotFound.jsx @@ -0,0 +1,29 @@ +import React, { Component, PropTypes } from "react"; + + +export default class NotFound extends Component { + render() { + return ( + <div className="layout-centered flex full"> + <div className="p4 text-bold"> + <h1 className="text-brand text-light mb3">We're a little lost...</h1> + <p className="h4 mb1">The page you asked for couldn't be found.</p> + <p className="h4">You might've been tricked by a ninja, but in all likelihood, you were just given a bad link.</p> + <p className="h4 my4">You can always:</p> + <div className="flex align-center"> + <a className="Button Button--primary" href="/q"> + <div className="p1">Ask a new question.</div> + </a> + <span className="mx2">or</span> + <a className="Button Button--withIcon" target="_blank" href="http://tv.giphy.com/kitten"> + <div className="p1 flex align-center relative"> + <span className="h2">😸</span> + <span className="ml1">Take a kitten break.</span> + </div> + </a> + </div> + </div> + </div> + ); + } +} \ No newline at end of file diff --git a/frontend/src/components/OnClickOutsideWrapper.jsx b/frontend/src/components/OnClickOutsideWrapper.jsx index bdd7a03850ac0fb2c469c9d05336b18ede62ecac..bb54a93ec688c65361181df4b92e0234cfa2d7fd 100644 --- a/frontend/src/components/OnClickOutsideWrapper.jsx +++ b/frontend/src/components/OnClickOutsideWrapper.jsx @@ -9,22 +9,14 @@ import ClickOutComponent from 'react-onclickout'; var popoverStack = []; export default class OnClickOutsideWrapper extends ClickOutComponent { - constructor(props, context) { - super(props, context); - this.state = this.state || {}; - this.state.ready = false; - } - - componentWillMount() { - popoverStack.push(this); - } - componentDidMount() { super.componentDidMount(); - // HACK: set the z-index of the parent element to ensure it's always on top - React.findDOMNode(this).parentNode.style.zIndex = popoverStack.length + 2; // HACK: add 2 to ensure it's in front of main and nav elements // necessary to ignore click events that fire immediately, causing modals/popovers to close prematurely - setTimeout(() => this.setState({ ready: true }), 10); + setTimeout(() => { + popoverStack.push(this); + // HACK: set the z-index of the parent element to ensure it's always on top + React.findDOMNode(this).parentNode.style.zIndex = popoverStack.length + 2; // HACK: add 2 to ensure it's in front of main and nav elements + }, 10); } componentWillUnmount() { @@ -36,10 +28,10 @@ export default class OnClickOutsideWrapper extends ClickOutComponent { } } - onClickOut(...args) { + onClickOut(e) { // only propagate event for the popover on top of the stack - if (this.state.ready && this === popoverStack[popoverStack.length - 1]) { - this.props.handleClickOutside(...args); + if (this === popoverStack[popoverStack.length - 1]) { + this.props.handleClickOutside(e); } } diff --git a/frontend/src/components/PasswordReveal.jsx b/frontend/src/components/PasswordReveal.jsx index 2c4135d0e03f8f22b032ad566e8517ca041ef376..3ea272222ea85c75e4c3fe6c4751353f4d797adf 100644 --- a/frontend/src/components/PasswordReveal.jsx +++ b/frontend/src/components/PasswordReveal.jsx @@ -52,9 +52,9 @@ export default class PasswordReveal extends Component { } { visible ? - <a href="#" className="link text-bold" onClick={this.onToggleVisibility.bind(this)}>Hide</a> + <a className="link text-bold" onClick={this.onToggleVisibility.bind(this)}>Hide</a> : - <a href="#" className="link text-bold" onClick={this.onToggleVisibility.bind(this)}>Show</a> + <a className="link text-bold" onClick={this.onToggleVisibility.bind(this)}>Show</a> } </div> ); diff --git a/frontend/src/components/QuestionSavedModal.jsx b/frontend/src/components/QuestionSavedModal.jsx index d12c82381ec83af66c3c6715e82a8f0582d79733..573073705e4186f36fa3ad783644cdc7af01b10c 100644 --- a/frontend/src/components/QuestionSavedModal.jsx +++ b/frontend/src/components/QuestionSavedModal.jsx @@ -23,7 +23,7 @@ export default class QuestionSavedModal extends Component { </a> </li> <li> - <a className="no-decoration flex align-center border-bottom py1 pb2" href="#" onClick={this.props.addToDashboardFn}> + <a className="no-decoration flex align-center border-bottom py1 pb2" onClick={this.props.addToDashboardFn}> <img className="" style={{height: "32px"}} src="/app/components/icons/assets/illustration_dashboard.png" /> <span className="h3 ml2 text-bold text-brand-hover">Add to a dashboard</span> </a> diff --git a/frontend/src/components/RadioSelect.jsx b/frontend/src/components/RadioSelect.jsx index 6daa364b63a42791fa83a63e3158a13a382e5676..93a0071bed700afe15ec087e9468f47f924b18d1 100644 --- a/frontend/src/components/RadioSelect.jsx +++ b/frontend/src/components/RadioSelect.jsx @@ -26,7 +26,7 @@ export default class RadioButtons extends Component { var classes = cx("h3", "text-bold", "text-brand-hover", "no-decoration", { "text-brand": this.props.value === value }); return ( <li className="mr3" key={key}> - <a className={classes} href="#" onClick={this.props.onChange.bind(null, value)}>{name}</a> + <a className={classes} onClick={this.props.onChange.bind(null, value)}>{name}</a> </li> ); }); diff --git a/frontend/src/components/SaveQuestionModal.jsx b/frontend/src/components/SaveQuestionModal.jsx index 8efb2e04a5ea4b57313d599fc7427bd20f57f597..a61ffc27afd4733d1b53fd001f7b44d14b849d30 100644 --- a/frontend/src/components/SaveQuestionModal.jsx +++ b/frontend/src/components/SaveQuestionModal.jsx @@ -118,7 +118,7 @@ export default React.createClass({ <button className={buttonClasses}> Save </button> - <span className="px1">or</span><a href="#" className="no-decoration text-brand text-bold" onClick={this.props.closeFn}>Cancel</a> + <span className="px1">or</span><a className="no-decoration text-brand text-bold" onClick={this.props.closeFn}>Cancel</a> {formError} </div> </form> diff --git a/frontend/src/components/SortableItemList.jsx b/frontend/src/components/SortableItemList.jsx index bea8d6913457b3b56e18b9d699f923bf8f502277..2110834f48234f0254ecb250784f8b75be13df9f 100644 --- a/frontend/src/components/SortableItemList.jsx +++ b/frontend/src/components/SortableItemList.jsx @@ -45,7 +45,7 @@ export default class SortableItemList extends Component { <ul className="SortableItemList-list px2 pb2"> {items.map(item => <li key={item.id} className="border-row-divider"> - <a className="no-decoration flex p2" href="#" onClick={() => this.onClickItem(item)}> + <a className="no-decoration flex p2" onClick={() => this.onClickItem(item)}> <div className="flex align-center flex-full mr2"> {this.props.showIcons ? <div className="mr2"><Icon name={'illustration-'+item.display} width={48} height={48} /></div> diff --git a/frontend/src/components/Toggle.jsx b/frontend/src/components/Toggle.jsx index 0298d406635d3469be648f0442876066847c4194..6966eecc377c88acc1cf25ba9da0e1de6768e43c 100644 --- a/frontend/src/components/Toggle.jsx +++ b/frontend/src/components/Toggle.jsx @@ -22,7 +22,6 @@ export default class Toggle extends Component { render() { return ( <a - href="#" className={cx("Toggle", "no-decoration", { selected: this.props.value }) + " " + (this.props.className||"")} style={{color: this.props.color || null}} onClick={this.onClick} diff --git a/frontend/src/components/Triggerable.jsx b/frontend/src/components/Triggerable.jsx index 1ad52a15f8f600bafcc822f3b12e86516379efd7..bc16a3c670d54d38ec3285aba831d0041beba960 100644 --- a/frontend/src/components/Triggerable.jsx +++ b/frontend/src/components/Triggerable.jsx @@ -42,7 +42,7 @@ export default ComposedComponent => class extends Component { render() { return ( - <a ref="trigger" href="#" onClick={() => this.toggle()} className={cx("no-decoration", this.props.triggerClasses)}> + <a ref="trigger" onClick={() => this.toggle()} className={cx("no-decoration", this.props.triggerClasses)}> {this.props.triggerElement} <ComposedComponent {...this.props} diff --git a/frontend/src/components/dashboard/dashboard.css b/frontend/src/components/dashboard/dashboard.css index f2657b66c2014246843b0e4e528e482b88091657..9b3aa3d352bb2c812f499ba43abfa46ba1645cd3 100644 --- a/frontend/src/components/dashboard/dashboard.css +++ b/frontend/src/components/dashboard/dashboard.css @@ -39,7 +39,8 @@ .DashCard .Card-outer { overflow: hidden; - flex: 1; + width: 100%; + height: 100%; } .DashCard .DashCard-actions { diff --git a/frontend/src/css/core/base.css b/frontend/src/css/core/base.css index 9c1b0c4600a4d1de9b765f05f8af1bec15f7e5c6..3ab5f12f98474f30f5ffaa842aeb4069487fe862 100644 --- a/frontend/src/css/core/base.css +++ b/frontend/src/css/core/base.css @@ -44,6 +44,7 @@ button { a { color: inherit; + cursor: pointer; } input, diff --git a/frontend/src/css/query_builder.css b/frontend/src/css/query_builder.css index 87183c0b92d1d4af4195f790affede2de6e01b08..0e1c4305cdb9191182d3c8475ec76408285d2b9c 100644 --- a/frontend/src/css/query_builder.css +++ b/frontend/src/css/query_builder.css @@ -229,6 +229,7 @@ .Visualization .Card-outer { width: 100%; + height: 100%; display: flex; align-items: center; justify-content: flex-start; @@ -743,3 +744,21 @@ font-size: 13px; font-weight: bold; } + + +.FilterRemove-field { + border-radius: 99px; + opacity: 0; + width: 20px; + height: 20px; + display: flex; + align-items: center; + justify-content: center; + background-color: var(--purple-color); + border: 1px solid var(--purple-color); + transition: opacity .3s ease-out; +} + +.FilterInput:hover .FilterRemove-field { + opacity: 1; +} diff --git a/frontend/src/dashboard/components/DashCard.jsx b/frontend/src/dashboard/components/DashCard.jsx index f7361d743506b5cd2c285753fe03aab99bc123f1..5804eafc3280e0c086d798b67e1bc0768f194750 100644 --- a/frontend/src/dashboard/components/DashCard.jsx +++ b/frontend/src/dashboard/components/DashCard.jsx @@ -2,10 +2,7 @@ import React, { Component, PropTypes } from "react"; -import ScalarCard from "./cards/ScalarCard.jsx"; -import TableCard from "./cards/TableCard.jsx"; -import ChartCard from "./cards/ChartCard.jsx"; - +import Visualization from "metabase/visualizations/Visualization.jsx"; import LoadingSpinner from "metabase/components/LoadingSpinner.jsx"; import { fetchDashCardData, markNewCardSeen } from "../actions"; @@ -20,8 +17,7 @@ export default class DashCard extends Component { static propTypes = { dispatch: PropTypes.func.isRequired, - dashcard: PropTypes.object.isRequired, - visualizationSettingsApi: PropTypes.object.isRequired + dashcard: PropTypes.object.isRequired }; async componentDidMount() { @@ -62,11 +58,7 @@ export default class DashCard extends Component { } if (card && data) { - switch (card.display) { - case "table": return <TableCard className="flex-full" card={card} data={data} visualizationSettingsApi={this.props.visualizationSettingsApi} />; - case "scalar": return <ScalarCard className="flex-full" card={card} data={data} visualizationSettingsApi={this.props.visualizationSettingsApi} />; - default: return <ChartCard className="flex-full" card={card} data={data} visualizationSettingsApi={this.props.visualizationSettingsApi} />; - } + return <Visualization className="flex-full" card={card} data={data} isDashboard={true} />; } return ( diff --git a/frontend/src/dashboard/components/Dashboard.jsx b/frontend/src/dashboard/components/Dashboard.jsx index 4a93cbceb026b109098309cf1a96585d7fddb9f9..58ee3b8647c2d3abeb4dcb8353cb1d399412761a 100644 --- a/frontend/src/dashboard/components/Dashboard.jsx +++ b/frontend/src/dashboard/components/Dashboard.jsx @@ -16,8 +16,7 @@ export default class Dashboard extends Component { static propTypes = { dispatch: PropTypes.func.isRequired, onChangeLocation: PropTypes.func.isRequired, - onDashboardDeleted: PropTypes.func.isRequired, - visualizationSettingsApi: PropTypes.object.isRequired + onDashboardDeleted: PropTypes.func.isRequired }; async componentDidMount() { diff --git a/frontend/src/dashboard/components/DashboardGrid.jsx b/frontend/src/dashboard/components/DashboardGrid.jsx index cbc814697d2811212093aafc37bb551a55c76a0e..0cd43577153f7d1253d1ee983f53401dc83df2f3 100644 --- a/frontend/src/dashboard/components/DashboardGrid.jsx +++ b/frontend/src/dashboard/components/DashboardGrid.jsx @@ -30,7 +30,6 @@ export default class DashboardGrid extends Component { dispatch: PropTypes.func.isRequired, isEditing: PropTypes.bool.isRequired, dashboard: PropTypes.object.isRequired, - visualizationSettingsApi: PropTypes.object.isRequired, onChangeLocation: PropTypes.func.isRequired }; @@ -174,7 +173,6 @@ export default class DashboardGrid extends Component { key={dc.id} dashcard={dc} dispatch={this.props.dispatch} - visualizationSettingsApi={this.props.visualizationSettingsApi} /> <div className="DashCard-actions absolute top right text-brand p1"> <a href="#" onClick={() => this.onEditDashCard(dc)}> diff --git a/frontend/src/dashboard/components/cards/ChartCard.jsx b/frontend/src/dashboard/components/cards/ChartCard.jsx deleted file mode 100644 index 8badc07da8d38a99a2356d91fb620ce9711b3dc6..0000000000000000000000000000000000000000 --- a/frontend/src/dashboard/components/cards/ChartCard.jsx +++ /dev/null @@ -1,25 +0,0 @@ -import React, { Component, PropTypes } from "react"; - -import QueryVisualizationChart from "metabase/query_builder/QueryVisualizationChart.jsx"; - -export default class ChartCard extends Component { - static propTypes = { - card: PropTypes.object.isRequired, - data: PropTypes.object.isRequired, - visualizationSettingsApi: PropTypes.object.isRequired - }; - - static defaultProps = { - className: "" - }; - - render() { - return ( - <QueryVisualizationChart - card={this.props.card} - data={this.props.data} - visualizationSettingsApi={this.props.visualizationSettingsApi} - /> - ); - } -} diff --git a/frontend/src/dashboard/components/cards/ScalarCard.jsx b/frontend/src/dashboard/components/cards/ScalarCard.jsx deleted file mode 100644 index 8ff0bc9f8a0c85e2e4eeb8e5560d19287b7a39b3..0000000000000000000000000000000000000000 --- a/frontend/src/dashboard/components/cards/ScalarCard.jsx +++ /dev/null @@ -1,21 +0,0 @@ -import React, { Component, PropTypes } from "react"; - -import { formatScalar } from "metabase/lib/formatting"; - -export default class ScalarCard extends Component { - static propTypes = { - data: PropTypes.object.isRequired - }; - - static defaultProps = { - className: "" - }; - - render() { - return ( - <div className={"Card--scalar " + this.props.className}> - <h1 className="Card-scalarValue text-normal">{formatScalar(this.props.data.rows[0][0])}</h1> - </div> - ); - } -} diff --git a/frontend/src/dashboard/dashboard.controllers.js b/frontend/src/dashboard/dashboard.controllers.js index 8b8dd2fa96c0d9c02afd1327afa0bd1f82f86d10..0339b716d9bb12fa027c7fe753d7c2ebc7f2ec70 100644 --- a/frontend/src/dashboard/dashboard.controllers.js +++ b/frontend/src/dashboard/dashboard.controllers.js @@ -8,10 +8,9 @@ const reducer = combineReducers(reducers); // Dashboard Controllers var DashboardControllers = angular.module('metabase.dashboard.controllers', []); -DashboardControllers.controller('Dashboard', ['$scope', '$rootScope', '$routeParams', '$location', 'VisualizationSettings', function($scope, $rootScope, $routeParams, $location, VisualizationSettings) { +DashboardControllers.controller('Dashboard', ['$scope', '$rootScope', '$routeParams', '$location', function($scope, $rootScope, $routeParams, $location) { $scope.Component = DashboardApp; $scope.props = { - visualizationSettingsApi: VisualizationSettings, onChangeLocation: function(url) { $scope.$apply(() => $location.url(url)); }, diff --git a/frontend/src/dashboard/dashboard.module.js b/frontend/src/dashboard/dashboard.module.js index 2f543640283a2dae68800079070a0710f27ff7a2..4d7e4db6888acecf374a06a6862ca7010245fa6a 100644 --- a/frontend/src/dashboard/dashboard.module.js +++ b/frontend/src/dashboard/dashboard.module.js @@ -3,8 +3,7 @@ var Dashboard = angular.module('metabase.dashboard', [ 'ngRoute', 'metabase.directives', 'metabase.services', - 'metabase.dashboard.controllers', - 'metabase.card.services' + 'metabase.dashboard.controllers' ]); Dashboard.config(['$routeProvider', function ($routeProvider) { diff --git a/frontend/src/lib/card.js b/frontend/src/lib/card.js index bcca7dbd2117eab6af5dc917d4ee25c0cde2e8df..e63a17df1d10952a2b7a45d875beb77a3bf9e7f5 100644 --- a/frontend/src/lib/card.js +++ b/frontend/src/lib/card.js @@ -44,7 +44,7 @@ export function urlForCardState(state, dirty) { url = "/q"; } if (state.serializedCard && (!state.cardId || dirty)) { - url += "/" + state.serializedCard; + url += "#" + state.serializedCard; } return url; } diff --git a/frontend/src/lib/core.js b/frontend/src/lib/core.js index ae682f522f0b837aa86640216da9d782fd800b2e..65282e395134d781503b8fe418e1a6c3a4ae8586 100644 --- a/frontend/src/lib/core.js +++ b/frontend/src/lib/core.js @@ -1,266 +1,119 @@ -/*global exports*/ -import _ from "underscore"; - -(function() { - - this.user_roles = [{ - 'id': 'user', - 'name': 'User', - 'description': 'Can do everything except access the Admin Panel.' - }, { - 'id': 'admin', - 'name': 'Admin', - 'description': "Can access the Admin Panel to add or remove users and modify database settings." - }]; - - this.perms = [{ - 'id': 0, - 'name': 'Private' - }, { - 'id': 1, - 'name': 'Public (others can read)' - }]; - - this.permName = function(permId) { - if (permId >= 0 && permId <= (this.perms.length - 1)) { - return this.perms[permId].name; - } - return null; - }; - - this.charts = [{ - 'id': 'scalar', - 'name': 'Scalar' - }, { - 'id': 'table', - 'name': 'Table' - }, { - 'id': 'pie', - 'name': 'Pie Chart' - }, { - 'id': 'bar', - 'name': 'Bar Chart' - }, { - 'id': 'line', - 'name': 'Line Chart' - }, { - 'id': 'area', - 'name': 'Area Chart' - }, { - 'id': 'timeseries', - 'name': 'Time Series' - }, { - 'id': 'pin_map', - 'name': 'Pin Map' - }, { - 'id': 'country', - 'name': 'World Heatmap' - }, { - 'id': 'state', - 'name': 'State Heatmap' - }]; - - this.chartName = function(chartId) { - for (var i = 0; i < this.charts.length; i++) { - if (this.charts[i].id == chartId) { - return this.charts[i].name; - } - } - return null; - }; - - this.table_entity_types = [{ - 'id': null, - 'name': 'None' - }, { - 'id': 'person', - 'name': 'Person' - }, { - 'id': 'event', - 'name': 'Event' - }, { - 'id': 'photo', - 'name': 'Photo' - }, { - 'id': 'place', - 'name': 'Place' - }, { - 'id': 'evt-cohort', - 'name': 'Cohorts-compatible Event' - }]; - - this.tableEntityType = function(typeId) { - for (var i = 0; i < this.table_entity_types.length; i++) { - if (this.table_entity_types[i].id == typeId) { - return this.table_entity_types[i].name; - } - } - return null; - }; - - this.field_special_types = [{ - 'id': 'id', - 'name': 'Entity Key', - 'section': 'Overall Row', - 'description': 'The primary key for this table.' - }, { - 'id': 'name', - 'name': 'Entity Name', - 'section': 'Overall Row', - 'description': 'The "name" of each record. Usually a column called "name", "title", etc.' - }, { - 'id': 'fk', - 'name': 'Foreign Key', - 'section': 'Overall Row', - 'description': 'Points to another table to make a connection.' - }, { - 'id': 'avatar', - 'name': 'Avatar Image URL', - 'section': 'Common' - }, { - 'id': 'category', - 'name': 'Category', - 'section': 'Common' - }, { - 'id': 'city', - 'name': 'City', - 'section': 'Common' - }, { - 'id': 'country', - 'name': 'Country', - 'section': 'Common' - }, { - 'id': 'desc', - 'name': 'Description', - 'section': 'Common' - }, { - 'id': 'image', - 'name': 'Image URL', - 'section': 'Common' - }, { - 'id': 'json', - 'name': 'Field containing JSON', - 'section': 'Common' - }, { - 'id': 'latitude', - 'name': 'Latitude', - 'section': 'Common' - }, { - 'id': 'longitude', - 'name': 'Longitude', - 'section': 'Common' - }, { - 'id': 'number', - 'name': 'Number', - 'section': 'Common' - }, { - 'id': 'state', - 'name': 'State', - 'section': 'Common' - }, { - id: 'timestamp_seconds', - name: 'UNIX Timestamp (Seconds)', - 'section': 'Common' - }, { - id: 'timestamp_milliseconds', - name: 'UNIX Timestamp (Milliseconds)', - 'section': 'Common' - }, { - 'id': 'url', - 'name': 'URL', - 'section': 'Common' - }, { - 'id': 'zip_code', - 'name': 'Zip Code', - 'section': 'Common' - }]; - - this.field_field_types = [{ - 'id': 'info', - 'name': 'Information', - 'description': 'Non-numerical value that is not meant to be used.' - }, { - 'id': 'metric', - 'name': 'Metric', - 'description': 'A number that can be added, graphed, etc.' - }, { - 'id': 'dimension', - 'name': 'Dimension', - 'description': 'A high or low-cardinality numerical string value that is meant to be used as a grouping.' - }, { - 'id': 'sensitive', - 'name': 'Sensitive Information', - 'description': 'A field that should never be shown anywhere.' - }]; - - this.field_visibility_types = [{ - 'id': 'everywhere', - 'name': 'Everywhere', - 'description': 'The default setting. This field will be displayed normally in tables and charts.' - }, { - 'id': 'detail_views', - 'name': 'Only in Detail Views', - 'description': "This field will only be displayed when viewing the details of a single record. Use this for information that's lengthy or that isn't useful in a table or chart." - }, { - 'id': 'do_not_include', - 'name': 'Do Not Include', - 'description': 'Metabase will never retrieve this field. Use this for sensitive or irrelevant information.' - }]; - - this.boolean_types = [{ - 'id': true, - 'name': 'Yes' - }, { - 'id': false, - 'name': 'No' - }, ]; - - this.fieldSpecialType = function(typeId) { - for (var i = 0; i < this.field_special_types.length; i++) { - if (this.field_special_types[i].id == typeId) { - return this.field_special_types[i].name; - } - } - return null; - }; - - this.builtinToChart = { - 'latlong_heatmap': 'll_heatmap' - }; - - this.getTitleForBuiltin = function(viewtype, field1Name, field2Name) { - var builtinToTitleMap = { - 'state': 'State Heatmap', - 'country': 'Country Heatmap', - 'pin_map': 'Pin Map', - 'heatmap': 'Heatmap', - 'cohorts': 'Cohorts', - 'latlong_heatmap': 'Lat/Lon Heatmap' - }; - - var title = builtinToTitleMap[viewtype]; - if (field1Name) { - title = title.replace("{0}", field1Name); - } - if (field2Name) { - title = title.replace("{1}", field2Name); - } - - return title; - }; - - this.createLookupTables = function(table) { - // Create lookup tables (ported from ExploreTableDetailData) - - table.fields_lookup = {}; - _.each(table.fields, function(field) { - table.fields_lookup[field.id] = field; - field.operators_lookup = {}; - _.each(field.valid_operators, function(operator) { - field.operators_lookup[operator.name] = operator; - }); - }); - }; - -}).apply(exports); +export const user_roles = [{ + 'id': 'user', + 'name': 'User', + 'description': 'Can do everything except access the Admin Panel.' +}, { + 'id': 'admin', + 'name': 'Admin', + 'description': "Can access the Admin Panel to add or remove users and modify database settings." +}]; + +export const field_special_types = [{ + 'id': 'id', + 'name': 'Entity Key', + 'section': 'Overall Row', + 'description': 'The primary key for this table.' +}, { + 'id': 'name', + 'name': 'Entity Name', + 'section': 'Overall Row', + 'description': 'The "name" of each record. Usually a column called "name", "title", etc.' +}, { + 'id': 'fk', + 'name': 'Foreign Key', + 'section': 'Overall Row', + 'description': 'Points to another table to make a connection.' +}, { + 'id': 'avatar', + 'name': 'Avatar Image URL', + 'section': 'Common' +}, { + 'id': 'category', + 'name': 'Category', + 'section': 'Common' +}, { + 'id': 'city', + 'name': 'City', + 'section': 'Common' +}, { + 'id': 'country', + 'name': 'Country', + 'section': 'Common' +}, { + 'id': 'desc', + 'name': 'Description', + 'section': 'Common' +}, { + 'id': 'image', + 'name': 'Image URL', + 'section': 'Common' +}, { + 'id': 'json', + 'name': 'Field containing JSON', + 'section': 'Common' +}, { + 'id': 'latitude', + 'name': 'Latitude', + 'section': 'Common' +}, { + 'id': 'longitude', + 'name': 'Longitude', + 'section': 'Common' +}, { + 'id': 'number', + 'name': 'Number', + 'section': 'Common' +}, { + 'id': 'state', + 'name': 'State', + 'section': 'Common' +}, { + id: 'timestamp_seconds', + name: 'UNIX Timestamp (Seconds)', + 'section': 'Common' +}, { + id: 'timestamp_milliseconds', + name: 'UNIX Timestamp (Milliseconds)', + 'section': 'Common' +}, { + 'id': 'url', + 'name': 'URL', + 'section': 'Common' +}, { + 'id': 'zip_code', + 'name': 'Zip Code', + 'section': 'Common' +}]; + +export const field_field_types = [{ + 'id': 'info', + 'name': 'Information', + 'description': 'Non-numerical value that is not meant to be used.' +}, { + 'id': 'metric', + 'name': 'Metric', + 'description': 'A number that can be added, graphed, etc.' +}, { + 'id': 'dimension', + 'name': 'Dimension', + 'description': 'A high or low-cardinality numerical string value that is meant to be used as a grouping.' +}, { + 'id': 'sensitive', + 'name': 'Sensitive Information', + 'description': 'A field that should never be shown anywhere.' +}]; + +export const field_visibility_types = [{ + 'id': 'everywhere', + 'name': 'Everywhere', + 'description': 'The default setting. This field will be displayed normally in tables and charts.' +}, { + 'id': 'detail_views', + 'name': 'Only in Detail Views', + 'description': "This field will only be displayed when viewing the details of a single record. Use this for information that's lengthy or that isn't useful in a table or chart." +}, { + 'id': 'do_not_include', + 'name': 'Do Not Include', + 'description': 'Metabase will never retrieve this field. Use this for sensitive or irrelevant information.' +}]; diff --git a/frontend/src/lib/schema_metadata.js b/frontend/src/lib/schema_metadata.js index ca0f84708a6572056712260c5424aefef685b13b..df40d9871a9c424e4de39015070540093521301a 100644 --- a/frontend/src/lib/schema_metadata.js +++ b/frontend/src/lib/schema_metadata.js @@ -40,11 +40,9 @@ const TYPES = { [LOCATION]: { special: ["city", "country", "state", "zip_code"] }, - [ENTITY]: { special: ["fk", "id", "name"] }, - [SUMMABLE]: { include: [NUMBER], exclude: [ENTITY, LOCATION, DATE_TIME] @@ -99,6 +97,7 @@ export function getFieldType(field) { export const isDate = isFieldType.bind(null, DATE_TIME); export const isNumeric = isFieldType.bind(null, NUMBER); export const isBoolean = isFieldType.bind(null, BOOLEAN); +export const isString = isFieldType.bind(null, STRING); export const isSummable = isFieldType.bind(null, SUMMABLE); export const isCategory = isFieldType.bind(null, CATEGORY); export const isDimension = isFieldType.bind(null, DIMENSION); @@ -150,7 +149,7 @@ function equivalentArgument(field, table) { } if (isCategory(field)) { - if (field.id in table.field_values && table.field_values[field.id].length > 0) { + if (table.field_values && field.id in table.field_values && table.field_values[field.id].length > 0) { let validValues = table.field_values[field.id]; // this sort function works for both numbers and strings: validValues.sort((a, b) => a === b ? 0 : (a < b ? -1 : 1)); @@ -237,57 +236,64 @@ const OPERATORS = { }, "CONTAINS": { validArgumentsFilters: [freeformArgument] + }, + "DOES_NOT_CONTAIN": { + validArgumentsFilters: [freeformArgument] } }; // ordered list of operators and metadata per type const OPERATORS_BY_TYPE_ORDERED = { [NUMBER]: [ - { name: "=", verboseName: "Equal" }, - { name: "!=", verboseName: "Not equal" }, - { name: ">", verboseName: "Greater than" }, - { name: "<", verboseName: "Less than" }, - { name: "BETWEEN", verboseName: "Between" }, - { name: ">=", verboseName: "Greater than or equal to", advanced: true }, - { name: "<=", verboseName: "Less than or equal to", advanced: true }, - { name: "IS_NULL", verboseName: "Is empty", advanced: true }, - { name: "NOT_NULL",verboseName: "Not empty", advanced: true } + { name: "=", verboseName: "Equal" }, + { name: "!=", verboseName: "Not equal" }, + { name: ">", verboseName: "Greater than" }, + { name: "<", verboseName: "Less than" }, + { name: "BETWEEN", verboseName: "Between" }, + { name: ">=", verboseName: "Greater than or equal to", advanced: true }, + { name: "<=", verboseName: "Less than or equal to", advanced: true }, + { name: "IS_NULL", verboseName: "Is empty", advanced: true }, + { name: "NOT_NULL", verboseName: "Not empty", advanced: true } ], [STRING]: [ - { name: "=", verboseName: "Is" }, - { name: "!=", verboseName: "Is not" }, - { name: "IS_NULL", verboseName: "Is empty", advanced: true }, - { name: "NOT_NULL",verboseName: "Not empty", advanced: true } + { name: "=", verboseName: "Is" }, + { name: "!=", verboseName: "Is not" }, + { name: "CONTAINS", verboseName: "Contains"}, + { name: "DOES_NOT_CONTAIN", verboseName: "Does not contain"}, + { name: "IS_NULL", verboseName: "Is empty", advanced: true }, + { name: "NOT_NULL", verboseName: "Not empty", advanced: true }, + { name: "STARTS_WITH", verboseName: "Starts with", advanced: true}, + { name: "ENDS_WITH", verboseName: "Ends with", advanced: true} ], [DATE_TIME]: [ - { name: "=", verboseName: "Is" }, - { name: "<", verboseName: "Before" }, - { name: ">", verboseName: "After" }, - { name: "BETWEEN", verboseName: "Between" }, - { name: "IS_NULL", verboseName: "Is empty", advanced: true }, - { name: "NOT_NULL",verboseName: "Not empty", advanced: true } + { name: "=", verboseName: "Is" }, + { name: "<", verboseName: "Before" }, + { name: ">", verboseName: "After" }, + { name: "BETWEEN", verboseName: "Between" }, + { name: "IS_NULL", verboseName: "Is empty", advanced: true }, + { name: "NOT_NULL", verboseName: "Not empty", advanced: true } ], [LOCATION]: [ - { name: "=", verboseName: "Is" }, - { name: "!=", verboseName: "Is not" }, - { name: "IS_NULL", verboseName: "Is empty", advanced: true }, - { name: "NOT_NULL",verboseName: "Not empty", advanced: true } + { name: "=", verboseName: "Is" }, + { name: "!=", verboseName: "Is not" }, + { name: "IS_NULL", verboseName: "Is empty", advanced: true }, + { name: "NOT_NULL", verboseName: "Not empty", advanced: true } ], [COORDINATE]: [ - { name: "=", verboseName: "Is" }, - { name: "!=", verboseName: "Is not" }, - { name: "INSIDE", verboseName: "Inside" } + { name: "=", verboseName: "Is" }, + { name: "!=", verboseName: "Is not" }, + { name: "INSIDE", verboseName: "Inside" } ], [BOOLEAN]: [ - { name: "=", verboseName: "Is", multi: false, defaults: [true] }, - { name: "IS_NULL", verboseName: "Is empty" }, - { name: "NOT_NULL",verboseName: "Not empty" } + { name: "=", verboseName: "Is", multi: false, defaults: [true] }, + { name: "IS_NULL", verboseName: "Is empty" }, + { name: "NOT_NULL", verboseName: "Not empty" } ], [UNKNOWN]: [ - { name: "=", verboseName: "Is" }, - { name: "!=", verboseName: "Is not" }, - { name: "IS_NULL", verboseName: "Is empty", advanced: true }, - { name: "NOT_NULL",verboseName: "Not empty", advanced: true } + { name: "=", verboseName: "Is" }, + { name: "!=", verboseName: "Is not" }, + { name: "IS_NULL", verboseName: "Is empty", advanced: true }, + { name: "NOT_NULL", verboseName: "Not empty", advanced: true } ] }; @@ -300,7 +306,7 @@ const MORE_VERBOSE_NAMES = { "less than": "is less than", "greater than": "is greater than", "less than or equal to": "is less than or equal to", - "greater than or equal to": "is greater than or equal to", + "greater than or equal to": "is greater than or equal to" } function getOperators(field, table) { diff --git a/frontend/src/lib/visualization_settings.js b/frontend/src/lib/visualization_settings.js new file mode 100644 index 0000000000000000000000000000000000000000..b15bcca504a406f156884630818ec1236d64daeb --- /dev/null +++ b/frontend/src/lib/visualization_settings.js @@ -0,0 +1,228 @@ + +import { normal, harmony } from 'metabase/lib/colors' + +import _ from "underscore"; + +const DEFAULT_COLOR_HARMONY = Object.values(normal); +const DEFAULT_COLOR = DEFAULT_COLOR_HARMONY[0]; + +const EXPANDED_COLOR_HARMONY = harmony; + +/* *** visualization settings *** + * + * This object defines default settings for card visualizations (i.e. charts, maps, etc). + * Each visualization type can be associated with zero or more top-level settings groups defined in this object + * (i.e. line charts may use 'chart', 'xAxis', 'yAxis', 'line'), depending on the settings that are appropriate + * for the particular visualization type (the associations are defined below in groupsForVisualizations). + * + * Before a card renders, the default settings from the appropriate groups are first copied from this object, + * creating an in-memory default settings object for that rendering. + * Then, a settings object stored in the card's record in the database is read and any attributes defined there + * are applied to that in-memory default settings object (using _.defaults()). + * The resulting in-memory settings object is made available to the card renderer at the time + * visualization is rendered. + * + * The settings object stored in the DB is 'sparse': only settings that differ from the defaults + * (at the time the settings were set) are recorded in the DB. This allows us to easily change the appearance of + * visualizations globally, except in cases where the user has explicitly changed the default setting. + * + * Some settings accept aribtrary numbers or text (i.e. titles) and some settings accept only certain values + * (i.e. *_enabled settings must be one of true or false). However, this object does not define the constraints. + * Instead, the controller that presents the UI to change the settings is currently responsible for enforcing the + * appropriate contraints for each setting. + * + * Search for '*** visualization settings ***' in card.controllers.js to find the objects that contain + * choices for the settings that require them. + * Additional constraints are enforced by the input elements in the views for each settings group + * (see app/card/partials/settings/*.html). + * + */ +var settings = { + 'global': { + 'title': null + }, + 'columns': { + 'dataset_column_titles': [] //allows the user to define custom titles for each column in the resulting dataset. Each item in this array corresponds to a column in the dataset's data.columns array. + }, + 'chart': { + 'plotBackgroundColor': '#FFFFFF', + 'borderColor': '#528ec5', + 'zoomType': 'x', + 'panning': true, + 'panKey': 'shift', + 'export_menu_enabled': false, + 'legend_enabled': false + }, + 'xAxis': { + 'title_enabled': true, + 'title_text': null, + 'title_text_default_READONLY': 'Values', //copied into title_text when re-enabling title from disabled state; user will be expected to change title_text + 'title_color': "#707070", + 'title_font_size': 12, //in pixels + 'min': null, + 'max': null, + 'gridLine_enabled': false, + 'gridLineColor': '#999999', + 'gridLineWidth': 0, + 'gridLineWidth_default_READONLY': 1, //copied into gridLineWidth when re-enabling grid lines from disabled state + 'tickInterval': null, + 'labels_enabled': true, + 'labels_step': null, + 'labels_staggerLines': null + }, + 'yAxis': { + 'title_enabled': true, + 'title_text': null, + 'title_text_default_READONLY': 'Values', //copied into title_text when re-enabling title from disabled state; user will be expected to change title_text + 'title_color': "#707070", + 'title_font_size': 12, //in pixels + 'min': null, + 'max': null, + 'gridLine_enabled': true, + 'gridLineColor': '#999999', + 'gridLineWidth': 1, + 'gridLineWidth_default_READONLY': 1, //copied into gridLineWidth when re-enabling grid lines from disabled state + 'tickInterval': null, + 'labels_enabled': true, + 'labels_step': null + }, + 'line': { + 'lineColor': DEFAULT_COLOR, + 'colors': DEFAULT_COLOR_HARMONY, + 'lineWidth': 2, + 'step': false, + 'marker_enabled': true, + 'marker_fillColor': '#528ec5', + 'marker_lineColor': '#FFFFFF', + 'marker_radius': 2, + 'xAxis_column': null, + 'yAxis_columns': [] + }, + 'area': { + 'fillColor': DEFAULT_COLOR, + 'fillOpacity': 0.75 + }, + 'pie': { + 'legend_enabled': true, + 'dataLabels_enabled': false, + 'dataLabels_color': '#777', + 'connectorColor': '#999', + 'colors': EXPANDED_COLOR_HARMONY + }, + 'bar': { + 'colors': DEFAULT_COLOR_HARMONY, + 'color': DEFAULT_COLOR + }, + 'map': { + 'latitude_source_table_field_id': null, + 'longitude_source_table_field_id': null, + 'latitude_dataset_col_index': null, + 'longitude_dataset_col_index': null, + 'zoom': 10, + 'center_latitude': 37.7577, //defaults to SF ;-) + 'center_longitude': -122.4376 + } +}; + +var groupsForVisualizations = { + 'scalar': ['global'], + 'table': ['global', 'columns'], + 'pie': ['global', 'chart', 'pie'], + 'bar': ['global', 'columns', 'chart', 'xAxis', 'yAxis', 'bar'], + 'line': ['global', 'columns', 'chart', 'xAxis', 'yAxis', 'line'], + 'area': ['global', 'columns', 'chart', 'xAxis', 'yAxis', 'line', 'area'], + 'country': ['global', 'columns', 'chart', 'map'], + 'state': ['global', 'columns', 'chart', 'map'], + 'pin_map': ['global', 'columns', 'chart', 'map'] +}; + +export function getDefaultColor() { + return DEFAULT_COLOR; +} + +export function getDefaultColorHarmony() { + return DEFAULT_COLOR_HARMONY; +} + +function getSettingsForGroup(dbSettings, groupName) { + if (typeof dbSettings != "object") { + dbSettings = {}; + } + + if (typeof settings[groupName] == "undefined") { + return dbSettings; + } + + if (typeof dbSettings[groupName] == "undefined") { + dbSettings[groupName] = {}; + } + //make a deep copy of default settings, otherwise default settings that are objects + //will not be recognized as 'dirty' after changing the value in the UI, because + //_.defaults make a shallow copy, so objects / arrays are copied by reference, + //so changing the settings in the UI would change the default settings. + var newSettings = _.defaults(dbSettings[groupName], angular.copy(settings[groupName])); + + return newSettings; +} + +function getSettingsForGroups(dbSettings, groups) { + var newSettings = {}; + for (var i = 0; i < groups.length; i++) { + var groupName = groups[i]; + newSettings[groupName] = getSettingsForGroup(dbSettings, groupName); + } + return newSettings; +} + +function getSettingsGroupsForVisualization(visualization) { + var groups = ['global']; + if (typeof groupsForVisualizations[visualization] != "undefined") { + groups = groupsForVisualizations[visualization]; + } + return groups; +} + +export function getSettingsForVisualization(dbSettings, visualization) { + var settings = angular.copy(dbSettings); + var groups = _.union(_.keys(settings), getSettingsGroupsForVisualization(visualization)); + return getSettingsForGroups(settings, groups); +} + +export function setLatitudeAndLongitude(settings, columnDefs) { + // latitude + var latitudeColumn, + latitudeColumnIndex; + columnDefs.forEach(function(col, index) { + if (col.special_type && + col.special_type === "latitude" && + latitudeColumn === undefined) { + latitudeColumn = col; + latitudeColumnIndex = index; + } + }); + + // longitude + var longitudeColumn, + longitudeColumnIndex; + columnDefs.forEach(function(col, index) { + if (col.special_type && + col.special_type === "longitude" && + longitudeColumn === undefined) { + longitudeColumn = col; + longitudeColumnIndex = index; + } + }); + + if (latitudeColumn && longitudeColumn) { + var settingsWithLatAndLon = angular.copy(settings); + + settingsWithLatAndLon.map.latitude_source_table_field_id = latitudeColumn.id; + settingsWithLatAndLon.map.latitude_dataset_col_index = latitudeColumnIndex; + settingsWithLatAndLon.map.longitude_source_table_field_id = longitudeColumn.id; + settingsWithLatAndLon.map.longitude_dataset_col_index = longitudeColumnIndex; + + return settingsWithLatAndLon; + } else { + return settings; + } +} diff --git a/frontend/src/query_builder/Calendar.jsx b/frontend/src/query_builder/Calendar.jsx index 482401be182902b6841c9136fbeb294fe8b3e84f..c26e7f4be19820e644a914d7e6182fe7904a7152 100644 --- a/frontend/src/query_builder/Calendar.jsx +++ b/frontend/src/query_builder/Calendar.jsx @@ -136,14 +136,14 @@ export default class Calendar extends Component { {this.props.onBeforeClick && <div className="absolute top left z2" style={{marginTop: "-12px", marginLeft: "-12px"}}> <Tooltip tooltipElement={"Everything before " + this.props.selected.format("MMMM Do, YYYY")}> - <a className="circle-button" onClick={this.props.onBeforeClick}>«</a> + <a className="circle-button cursor-pointer" onClick={this.props.onBeforeClick}>«</a> </Tooltip> </div> } {this.props.onAfterClick && <div className="absolute bottom right z2" style={{marginBottom: "-12px", marginRight: "-12px"}}> <Tooltip tooltipElement={"Everything after " + this.props.selected.format("MMMM Do, YYYY")}> - <a className="circle-button" onClick={this.props.onAfterClick}>»</a> + <a className="circle-button cursor-pointer" onClick={this.props.onAfterClick}>»</a> </Tooltip> </div> } diff --git a/frontend/src/query_builder/CardFavoriteButton.jsx b/frontend/src/query_builder/CardFavoriteButton.jsx index d9dafdd0bb7348de6979425f232f8e65cda7f6e6..ca5ba843b6fe5bf6868d1a7fbd1594898c0c759e 100644 --- a/frontend/src/query_builder/CardFavoriteButton.jsx +++ b/frontend/src/query_builder/CardFavoriteButton.jsx @@ -85,7 +85,7 @@ export default class CardFavoriteButton extends Component { }); return ( - <a className={iconClasses} href="#" onClick={this.toggleFavorite} title="Favorite this question"> + <a className={iconClasses} onClick={this.toggleFavorite} title="Favorite this question"> <Icon name="star" width="16px" height="16px"></Icon> </a> ); diff --git a/frontend/src/query_builder/DataReference.jsx b/frontend/src/query_builder/DataReference.jsx index db9fd9cf69e53f862402305eda2e2bc00608afb2..4b7a31bbb01e2541e11400ffa8484f9fea6ad96f 100644 --- a/frontend/src/query_builder/DataReference.jsx +++ b/frontend/src/query_builder/DataReference.jsx @@ -69,7 +69,7 @@ export default class DataReference extends Component { var backButton; if (this.state.stack.length > 0) { backButton = ( - <a href="#" className="flex align-center mb2 text-default text-brand-hover no-decoration" onClick={this.back}> + <a className="flex align-center mb2 text-default text-brand-hover no-decoration" onClick={this.back}> <Icon name="chevronleft" width="18px" height="18px" /> <span className="text-uppercase">Back</span> </a> @@ -77,7 +77,7 @@ export default class DataReference extends Component { } var closeButton = ( - <a href="#" className="flex-align-right text-default text-brand-hover no-decoration" onClick={this.close}> + <a className="flex-align-right text-default text-brand-hover no-decoration" onClick={this.close}> <Icon name="close" width="18px" height="18px" /> </a> ); diff --git a/frontend/src/query_builder/DataReferenceField.jsx b/frontend/src/query_builder/DataReferenceField.jsx index a20295ad8ccff0ff7e3e4bf29d3c96290d07425f..09a2eba79bdde967d0dab89d2c06c23a188be0bf 100644 --- a/frontend/src/query_builder/DataReferenceField.jsx +++ b/frontend/src/query_builder/DataReferenceField.jsx @@ -127,7 +127,7 @@ export default class DataReferenceField extends Component { useForCurrentQuestion.push( <li key="filter-by" className="mt1"> - <a className="Button Button--white text-default text-brand-hover border-brand-hover no-decoration" href="#" onClick={this.filterBy}> + <a className="Button Button--white text-default text-brand-hover border-brand-hover no-decoration" onClick={this.filterBy}> <Icon className="mr1" name="add" width="12px" height="12px"/> Filter by {name} </a> </li> @@ -135,7 +135,7 @@ export default class DataReferenceField extends Component { if (validBreakout) { useForCurrentQuestion.push( <li key="group-by" className="mt1"> - <a className="Button Button--white text-default text-brand-hover border-brand-hover no-decoration" href="#" onClick={this.groupBy}> + <a className="Button Button--white text-default text-brand-hover border-brand-hover no-decoration" onClick={this.groupBy}> <Icon className="mr2" name="add" width="12px" height="12px" /> Group by {name} </a> </li> diff --git a/frontend/src/query_builder/DataReferenceMain.jsx b/frontend/src/query_builder/DataReferenceMain.jsx index c2d275e233787784b19b66bd004d99876ed714c2..5730a6775351d0546ab97c8d10e2213764a0f98b 100644 --- a/frontend/src/query_builder/DataReferenceMain.jsx +++ b/frontend/src/query_builder/DataReferenceMain.jsx @@ -5,15 +5,8 @@ import { isQueryable } from "metabase/lib/table"; import inflection from 'inflection'; import cx from "classnames"; -export default class DataReferenceMain extends Component { - constructor(props, context) { - super(props, context); - this.state = { - databases: {}, - tables: {} - }; - } +export default class DataReferenceMain extends Component { static propTypes = { Metabase: PropTypes.func.isRequired, @@ -21,34 +14,23 @@ export default class DataReferenceMain extends Component { }; render() { - var databases; + let databases; if (this.props.databases) { databases = this.props.databases.map((database) => { - var dbTables = this.state.databases[database.id]; - if (dbTables === undefined) { - this.state.databases[database.id] = null; // null indicates loading - this.props.Metabase.db_tables({ - 'dbId': database.id - }).$promise.then((tables) => { - this.state.databases[database.id] = tables.filter(isQueryable); - this.setState({ databases: this.state.databases }); - }); - } - var tables; - var tableCount; - if (dbTables && dbTables.length > 0) { - tableCount = dbTables.length + " " + inflection.inflect("table", dbTables.length); - tables = dbTables.map((table, index) => { - var classes = cx({ + if (database.tables && database.tables.length > 0) { + const tableCount = database.tables.length + " " + inflection.inflect("table", database.tables.length); + const tables = database.tables.filter(isQueryable).map((table, index) => { + let classes = cx({ 'p1' : true, - 'border-bottom': index !== dbTables.length - 1 + 'border-bottom': index !== database.tables.length - 1 }) return ( <li key={table.id} className={classes}> - <a className="text-brand text-brand-darken-hover no-decoration" href="#" onClick={this.props.showTable.bind(null, table)}>{table.display_name}</a> + <a className="text-brand text-brand-darken-hover no-decoration" onClick={this.props.showTable.bind(null, table)}>{table.display_name}</a> </li> ); }); + return ( <li key={database.id}> <div className="my2"> diff --git a/frontend/src/query_builder/DataReferenceQueryButton.jsx b/frontend/src/query_builder/DataReferenceQueryButton.jsx index d43b4cd01a5c9edcbae1be9f8974ddd379cc94ed..e213a29e56198009f617d1a298329b48ebc003c7 100644 --- a/frontend/src/query_builder/DataReferenceQueryButton.jsx +++ b/frontend/src/query_builder/DataReferenceQueryButton.jsx @@ -12,7 +12,7 @@ export default class DataReferenceQueryButton extends Component { render(page) { return ( <div className={this.props.className}> - <a className="DataRefererenceQueryButton flex align-center no-decoration py1" href="#" onClick={this.props.onClick} > + <a className="DataRefererenceQueryButton flex align-center no-decoration py1" onClick={this.props.onClick} > <Icon name={this.props.icon} /> <span className="DataRefererenceQueryButton-text mx2 text-default text-brand-hover">{this.props.text}</span> <span className="DataRefererenceQueryButton-circle flex-align-right text-brand"> diff --git a/frontend/src/query_builder/DataReferenceTable.jsx b/frontend/src/query_builder/DataReferenceTable.jsx index db61104edfa8af5ea64870b8c40b5aaadd434c53..173a6f46446ff0b87cdb82711f972e4772414a2e 100644 --- a/frontend/src/query_builder/DataReferenceTable.jsx +++ b/frontend/src/query_builder/DataReferenceTable.jsx @@ -77,7 +77,7 @@ export default class DataReferenceTable extends Component { 'Button--active': name === this.state.pane }); return ( - <a key={name} className={classes} href="#" onClick={this.showPane.bind(null, name)}> + <a key={name} className={classes} onClick={this.showPane.bind(null, name)}> <span className="DataReference-paneCount">{count}</span><span>{inflection.inflect(name, count)}</span> </a> ); @@ -88,7 +88,7 @@ export default class DataReferenceTable extends Component { var fields = table.fields.map((field, index) => { return ( <li key={field.id} className="p1 border-row-divider"> - <a className="text-brand text-brand-darken-hover no-decoration" href="#" onClick={this.props.showField.bind(null, field)}>{field.display_name}</a> + <a className="text-brand text-brand-darken-hover no-decoration" onClick={this.props.showField.bind(null, field)}>{field.display_name}</a> </li> ); }); @@ -103,7 +103,7 @@ export default class DataReferenceTable extends Component { return ( <li key={fk.id} className="p1 border-row-divider"> - <a className="text-brand text-brand-darken-hover no-decoration" href="#" onClick={this.props.showField.bind(null, fk.origin)}>{fk.origin.table.display_name}{via}</a> + <a className="text-brand text-brand-darken-hover no-decoration" onClick={this.props.showField.bind(null, fk.origin)}>{fk.origin.table.display_name}{via}</a> </li> ); }); diff --git a/frontend/src/query_builder/FieldName.jsx b/frontend/src/query_builder/FieldName.jsx index d0d81c33f6d03d91c7798a8c58aa668558fecae2..0fc039457090279110140acc19393335c73aa9f7 100644 --- a/frontend/src/query_builder/FieldName.jsx +++ b/frontend/src/query_builder/FieldName.jsx @@ -68,7 +68,7 @@ export default class FieldName extends Component { var removeButton; if (this.props.removeField) { removeButton = ( - <a className="text-grey-2 no-decoration pr1 flex align-center" href="#" onClick={this.props.removeField}> + <a className="text-grey-2 no-decoration pr1 flex align-center" onClick={this.props.removeField}> <Icon name='close' width="14px" height="14px" /> </a> ) diff --git a/frontend/src/query_builder/NativeQueryEditor.jsx b/frontend/src/query_builder/NativeQueryEditor.jsx index 26610aa0ac6d8e317696d4d89375aeb47bfcd094..2dfe80b6867f2aa8f2aa01ddea5e7081010c3404 100644 --- a/frontend/src/query_builder/NativeQueryEditor.jsx +++ b/frontend/src/query_builder/NativeQueryEditor.jsx @@ -157,7 +157,7 @@ export default class NativeQueryEditor extends Component { <div className="NativeQueryEditor bordered rounded shadowed"> <div className="flex"> {dbSelector} - <a href="#" className="Query-label no-decoration flex-align-right flex align-center px2" onClick={this.toggleEditor}> + <a className="Query-label no-decoration flex-align-right flex align-center px2" onClick={this.toggleEditor}> <span className="mx2">{toggleEditorText}</span> <Icon name={toggleEditorIcon} width="20" height="20"/> </a> diff --git a/frontend/src/query_builder/QueryHeader.jsx b/frontend/src/query_builder/QueryHeader.jsx index 401db29f1f20bb4c4d422e3ba0954c7f1a055d01..ee3d28fb5b4fdc790858d397c9bb9c7885d23417 100644 --- a/frontend/src/query_builder/QueryHeader.jsx +++ b/frontend/src/query_builder/QueryHeader.jsx @@ -180,7 +180,7 @@ export default React.createClass({ 'text-brand-hover': !this.state.isShowingDataReference }); buttonSections[1].push( - <a key="dataReference" href="#" className={dataReferenceButtonClasses} title="Get help on what data means"> + <a key="dataReference" className={dataReferenceButtonClasses} title="Get help on what data means"> <Icon name='reference' width="16px" height="16px" onClick={this.toggleDataReference}></Icon> </a> ); diff --git a/frontend/src/query_builder/QueryVisualization.jsx b/frontend/src/query_builder/QueryVisualization.jsx index f9956a647222badd7bff8d38d73ffa7f42fb754b..cc9178e1b20c72853fcda4f103ae6e75865ce631 100644 --- a/frontend/src/query_builder/QueryVisualization.jsx +++ b/frontend/src/query_builder/QueryVisualization.jsx @@ -2,12 +2,12 @@ import React, { Component, PropTypes } from "react"; import Icon from "metabase/components/Icon.jsx"; import LoadingSpinner from 'metabase/components/LoadingSpinner.jsx'; -import QueryVisualizationTable from './QueryVisualizationTable.jsx'; -import QueryVisualizationChart from './QueryVisualizationChart.jsx'; import QueryVisualizationObjectDetailTable from './QueryVisualizationObjectDetailTable.jsx'; import RunButton from './RunButton.jsx'; import VisualizationSettings from './VisualizationSettings.jsx'; +import Visualization from "metabase/visualizations/Visualization.jsx"; + import MetabaseSettings from "metabase/lib/settings"; import Modal from "metabase/components/Modal.jsx"; import ModalWithTrigger from "metabase/components/ModalWithTrigger.jsx"; @@ -27,7 +27,6 @@ export default class QueryVisualization extends Component { } static propTypes = { - visualizationSettingsApi: PropTypes.object.isRequired, card: PropTypes.object.isRequired, result: PropTypes.object, databases: PropTypes.array, @@ -176,7 +175,7 @@ export default class QueryVisualization extends Component { } else { if (window.OSX) { return ( - <a classname="mx1" href="#" title="Download this data" onClick={function() { + <a classname="mx1" title="Download this data" onClick={function() { window.OSX.saveCSV(downloadLink); }}> <Icon name='download' width="16px" height="16px" /> @@ -311,57 +310,18 @@ export default class QueryVisualization extends Component { </div> ); - } else if (this.props.card.display === "scalar") { - var scalarValue; - if (this.props.result.data.rows && - this.props.result.data.rows.length > 0 && - this.props.result.data.rows[0].length > 0) { - scalarValue = this.props.result.data.rows[0][0]; - } - - viz = ( - <div className="Visualization--scalar flex full layout-centered"> - <span>{scalarValue}</span> - </div> - ); - - } else if (this.props.card.display === "table") { - - var pivotTable = false, - cellClickable = this.props.cellIsClickableFn, - sortFunction = this.props.setSortFn, - sort = (this.props.card.dataset_query.query && this.props.card.dataset_query.query.order_by) ? - this.props.card.dataset_query.query.order_by : null; - - // check if the data is pivotable (2 groupings + 1 agg != 'rows') - if (Query.isStructured(this.props.card.dataset_query) && - !Query.isBareRowsAggregation(this.props.card.dataset_query.query) && - this.props.result.data.cols.length === 3) { - pivotTable = true; - sortFunction = undefined; - cellClickable = function() { return false; }; - } - + } else { viz = ( - <QueryVisualizationTable + <Visualization + card={this.props.card} data={this.props.result.data} - pivot={pivotTable} - maxRows={this.props.maxTableRows} - setSortFn={sortFunction} - sort={sort} - cellIsClickableFn={cellClickable} + + // Table: + setSortFn={this.props.setSortFn} + cellIsClickableFn={this.props.cellIsClickableFn} cellClickedFn={this.props.cellClickedFn} /> ); - - } else { - // assume that anything not a table is a chart - viz = ( - <QueryVisualizationChart - visualizationSettingsApi={this.props.visualizationSettingsApi} - card={this.props.card} - data={this.props.result.data} /> - ); } } } @@ -371,7 +331,7 @@ export default class QueryVisualization extends Component { 'flex-column': !this.props.isObjectDetail }); - var visualizationClasses = cx('flex flex-full Visualization z1', { + var visualizationClasses = cx('flex flex-full Visualization z1 px1', { 'Visualization--errors': (this.props.result && this.props.result.error), 'Visualization--loading': this.props.isRunning }); diff --git a/frontend/src/query_builder/QueryVisualizationChart.jsx b/frontend/src/query_builder/QueryVisualizationChart.jsx deleted file mode 100644 index 352a6ae834ea4a0b9b9bc12e1837d39fd262ae98..0000000000000000000000000000000000000000 --- a/frontend/src/query_builder/QueryVisualizationChart.jsx +++ /dev/null @@ -1,159 +0,0 @@ -import React, { Component, PropTypes } from "react"; - -import { CardRenderer } from '../card/card.charting'; - -import { hasLatitudeAndLongitudeColumns } from "metabase/lib/schema_metadata"; - -export default class QueryVisualizationChart extends Component { - constructor(props, context) { - super(props, context); - - this.state = { - chartId: Math.floor((Math.random() * 698754) + 1), - width: 0, - height: 0, - }; - } - - static propTypes = { - visualizationSettingsApi: PropTypes.object.isRequired, - card: PropTypes.object.isRequired, - data: PropTypes.object - }; - - shouldComponentUpdate(nextProps, nextState) { - this.calculateSizing(nextState); - // a chart only needs re-rendering when the result itself changes OR the chart type is different - // NOTE: we are purposely doing an identity comparison here with props.result and NOT a value comparison - if (this.state.error === nextState.error && - this.props.data == nextProps.data && - this.props.card.display === nextProps.card.display && - JSON.stringify(this.props.card.visualization_settings) === JSON.stringify(nextProps.card.visualization_settings) && - this.state.width === nextState.width && this.state.height === nextState.height - ) { - return false; - } else { - return true; - } - } - - componentDidMount() { - this.calculateSizing(this.state); - this.renderChart(); - } - - componentDidUpdate() { - this.renderChart(); - } - - calculateSizing(prevState) { - var width = CardRenderer.getAvailableCanvasWidth(this.state.chartId); - var height = CardRenderer.getAvailableCanvasHeight(this.state.chartId); - if (width !== prevState.width || height !== prevState.height) { - this.setState({ width, height }); - } - } - - renderChart() { - if (this.props.data) { - // validate the shape of the data against our chosen display and if we don't have appropriate data - // then lets inform the user that this isn't going to work so they can do something better - var dataError; - if (this.props.card.display === "pin_map" && - !hasLatitudeAndLongitudeColumns(this.props.data.cols)) { - dataError = "Bummer. We can't actually do a pin map for this data because we require both a latitude and longitude column."; - } else if (this.props.data.columns && this.props.data.columns.length < 2) { - dataError = "Doh! The data from your query doesn't fit the chosen display choice. This visualization requires at least 2 columns of data."; - - } else if ((this.props.card.display === "line" || this.props.card.display === "area") && - this.props.data.rows && this.props.data.rows.length < 2) { - dataError = "No dice. We only have 1 data point to show and that's not enough for a line chart."; - } - - if (dataError) { - this.setState({ - error: dataError - }); - return; - } else { - this.setState({ - error: null - }); - } - - try { - // always ensure we have the most recent visualization settings to use for rendering - var vizSettings = this.props.visualizationSettingsApi.getSettingsForVisualization(this.props.card.visualization_settings, this.props.card.display); - - // be as immutable as possible and build a card like structure used for charting - var cardIsh = { - name: this.props.card.name, - display: this.props.card.display, - dataset_query: this.props.card.dataset_query, - visualization_settings: vizSettings - }; - - if (this.props.card.display === "pin_map") { - // call signature is (elementId, card, updateMapCenter (callback), updateMapZoom (callback)) - - // identify the lat/lon columns from our data and make them part of the viz settings so we can render maps - cardIsh.visualization_settings = this.props.visualizationSettingsApi.setLatitudeAndLongitude(cardIsh.visualization_settings, this.props.data.cols); - - // these are example callback functions that could be passed into the renderer - // var updateMapCenter = function(lat, lon) { - // scope.card.visualization_settings.map.center_latitude = lat; - // scope.card.visualization_settings.map.center_longitude = lon; - // scope.$apply(); - // }; - - // var updateMapZoom = function(zoom) { - // scope.card.visualization_settings.map.zoom = zoom; - // scope.$apply(); - // }; - - var no_op = function(a, b) { - // do nothing for now - }; - - CardRenderer[this.props.card.display](this.state.chartId, cardIsh, no_op, no_op); - } else { - // TODO: it would be nicer if this didn't require the whole card - CardRenderer[this.props.card.display](this.state.chartId, cardIsh, this.props.data); - } - } catch (err) { - console.error(err); - this.setState({ - error: (err.message || err) - }); - } - } - } - - render() { - // rendering a chart of some type - var titleId = 'card-title--'+this.state.chartId; - var innerId = 'card-inner--'+this.state.chartId; - - var errorMessage; - if (this.state.error) { - errorMessage = ( - <div className="QueryError flex full align-center text-error"> - <div className="QueryError-iconWrapper"> - <svg className="QueryError-icon" viewBox="0 0 32 32" width="64" height="64" fill="currentcolor"> - <path d="M4 8 L8 4 L16 12 L24 4 L28 8 L20 16 L28 24 L24 28 L16 20 L8 28 L4 24 L12 16 z "></path> - </svg> - </div> - <span className="QueryError-message">{this.state.error}</span> - </div> - ); - } - - return ( - <div className={"Card--" + this.props.card.display + " Card-outer px1"} id={this.state.chartId}> - <div id={titleId} className="text-centered"></div> - <div id={innerId} className="card-inner"></div> - {errorMessage} - </div> - ); - } -} diff --git a/frontend/src/query_builder/QueryVisualizationObjectDetailTable.jsx b/frontend/src/query_builder/QueryVisualizationObjectDetailTable.jsx index 71933278f21bf3117bdffbd130527c182b62993e..0f41ce2ff3616470ca11628fe454e63714635736 100644 --- a/frontend/src/query_builder/QueryVisualizationObjectDetailTable.jsx +++ b/frontend/src/query_builder/QueryVisualizationObjectDetailTable.jsx @@ -67,7 +67,7 @@ export default class QueryVisualizationObjectDetailTable extends Component { // NOTE: that the values to our function call look off, but that's because we are un-pivoting them if (this.props.cellIsClickableFn(0, rowIndex)) { - return (<div key={key}><a className="link" href="#" onClick={this.cellClicked.bind(null, 0, rowIndex)}>{cellValue}</a></div>); + return (<div key={key}><a className="link" onClick={this.cellClicked.bind(null, 0, rowIndex)}>{cellValue}</a></div>); } else { return (<div key={key}>{cellValue}</div>); } diff --git a/frontend/src/query_builder/SelectionModule.jsx b/frontend/src/query_builder/SelectionModule.jsx index 66d46e491bf0fcbe37e197a601bac8daf530cb6f..aae7b29a2d06bed74afb882477290ff7e670eb04 100644 --- a/frontend/src/query_builder/SelectionModule.jsx +++ b/frontend/src/query_builder/SelectionModule.jsx @@ -226,7 +226,7 @@ export default class SelectionModule extends Component { if(this.props.remove) { remove = ( - <a className="text-grey-2 no-decoration pr1 flex align-center" href="#" onClick={this.props.remove.bind(null, this.props.index)}> + <a className="text-grey-2 no-decoration pr1 flex align-center" onClick={this.props.remove.bind(null, this.props.index)}> <Icon name='close' width="14px" height="14px" /> </a> ); diff --git a/frontend/src/query_builder/SortWidget.jsx b/frontend/src/query_builder/SortWidget.jsx index 98f9a8177e55f7867207640b970981c9bb8349d2..329693c9308a1d03a2478919e268fc451247dc71 100644 --- a/frontend/src/query_builder/SortWidget.jsx +++ b/frontend/src/query_builder/SortWidget.jsx @@ -31,6 +31,13 @@ export default class SortWidget extends Component { }); } + componentWillUnmount() { + // Remove partially completed sort if the widget is removed + if (this.state.field == null || this.state.direction == null) { + this.props.removeSort(); + } + } + setField(value) { if (this.state.field !== value) { this.props.updateSort([value, this.state.direction]); diff --git a/frontend/src/query_builder/VisualizationSettings.jsx b/frontend/src/query_builder/VisualizationSettings.jsx index 67db79fad71bd15af703559e5cfed91b6e92a779..0e2b8940aa01ef513d531a3d867032977d866bb8 100644 --- a/frontend/src/query_builder/VisualizationSettings.jsx +++ b/frontend/src/query_builder/VisualizationSettings.jsx @@ -3,22 +3,11 @@ import React, { Component, PropTypes } from "react"; import Icon from "metabase/components/Icon.jsx"; import PopoverWithTrigger from "metabase/components/PopoverWithTrigger.jsx"; -import { hasLatitudeAndLongitudeColumns } from "metabase/lib/schema_metadata"; +import visualizations from "metabase/visualizations"; +import { getDefaultColor, getDefaultColorHarmony } from "metabase/lib/visualization_settings"; import cx from "classnames"; -const VISUALIZATION_TYPE_NAMES = { - 'scalar': { displayName: 'Number', iconName: 'number' }, - 'table': { displayName: 'Table', iconName: 'table' }, - 'line': { displayName: 'Line', iconName: 'line' }, - 'bar': { displayName: 'Bar', iconName: 'bar' }, - 'pie': { displayName: 'Pie', iconName: 'pie' }, - 'area': { displayName: 'Area', iconName: 'area' }, - 'state': { displayName: 'State map', iconName: 'statemap' }, - 'country': { displayName: 'Country map', iconName: 'countrymap' }, - 'pin_map': { displayName: 'Pin map', iconName: 'pinmap' } -}; - export default class VisualizationSettings extends React.Component { constructor(props, context) { super(props, context); @@ -27,27 +16,12 @@ export default class VisualizationSettings extends React.Component { } static propTypes = { - visualizationSettingsApi: PropTypes.object.isRequired, card: PropTypes.object.isRequired, result: PropTypes.object, setDisplayFn: PropTypes.func.isRequired, setChartColorFn: PropTypes.func.isRequired }; - static defaultProps = { - visualizationTypes: [ - 'scalar', - 'table', - 'line', - 'bar', - 'pie', - 'area', - 'state', - 'country', - 'pin_map' - ] - }; - setDisplay(type) { // notify our parent about our change this.props.setDisplayFn(type); @@ -60,72 +34,41 @@ export default class VisualizationSettings extends React.Component { this.refs.colorPopover.toggle(); } - isSensibleChartDisplay(display) { - var data = (this.props.result) ? this.props.result.data : null; - switch (display) { - case "table": - // table is always appropriate - return true; - case "scalar": - // a 1x1 data set is appropriate for a scalar - return (data && data.rows && data.rows.length === 1 && data.cols && data.cols.length === 1); - case "pin_map": - // when we have a latitude and longitude a pin map is cool - return (data && hasLatitudeAndLongitudeColumns(data.cols)); - case "line": - case "area": - // if we have 2x2 or more then that's enough to make a line/area chart - return (data && data.rows && data.rows.length > 1 && data.cols && data.cols.length > 1); - case "country": - case "state": - return (data && data.cols && data.cols.length > 1 && data.cols[0].base_type === "TextField"); - case "bar": - case "pie": - default: - // general check for charts is that they require 2 columns - return (data && data.cols && data.cols.length > 1); - } - } - renderChartTypePicker() { - var iconName = VISUALIZATION_TYPE_NAMES[this.props.card.display].iconName; + let { result, card } = this.props; + let visualization = visualizations.get(card.display); + var triggerElement = ( <span className="px2 py1 text-bold cursor-pointer text-default flex align-center"> - <Icon name={iconName} width="24px" height="24px"/> - {VISUALIZATION_TYPE_NAMES[this.props.card.display].displayName} + <Icon name={visualization.iconName} width="24px" height="24px"/> + {visualization.displayName} <Icon className="ml1" name="chevrondown" width="8px" height="8px"/> </span> - ) - - var displayOptions = this.props.visualizationTypes.map((type, index) => { - var classes = cx({ - 'p2': true, - 'flex': true, - 'align-center': true, - 'cursor-pointer': true, - 'bg-brand-hover': true, - 'text-white-hover': true, - 'ChartType--selected': type === this.props.card.display, - 'ChartType--notSensible': !this.isSensibleChartDisplay(type), - }); - var displayName = VISUALIZATION_TYPE_NAMES[type].displayName; - var iconName = VISUALIZATION_TYPE_NAMES[type].iconName; - return ( - <li className={classes} key={index} onClick={this.setDisplay.bind(null, type)}> - <Icon name={iconName} width="24px" height="24px"/> - <span className="ml1">{displayName}</span> - </li> - ); - }); + ); + return ( <div className="relative"> <span className="GuiBuilder-section-label Query-label">Visualization</span> - <PopoverWithTrigger ref="displayPopover" - className="ChartType-popover" - triggerElement={triggerElement} - triggerClasses="flex align-center"> + <PopoverWithTrigger + ref="displayPopover" + className="ChartType-popover" + triggerElement={triggerElement} + triggerClasses="flex align-center" + > <ul className="pt1 pb1"> - {displayOptions} + { Array.from(visualizations).map(([vizType, viz], index) => + <li + key={index} + className={cx('p2 flex align-center cursor-pointer bg-brand-hover text-white-hover', { + 'ChartType--selected': vizType === card.display, + 'ChartType--notSensible': !(result && result.data && viz.isSensible(result.data.cols, result.data.rows)) + })} + onClick={this.setDisplay.bind(null, vizType)} + > + <Icon name={viz.iconName} width="24px" height="24px"/> + <span className="ml1">{viz.displayName}</span> + </li> + )} </ul> </PopoverWithTrigger> </div> @@ -134,7 +77,7 @@ export default class VisualizationSettings extends React.Component { renderChartColorPicker() { if (this.props.card.display === "line" || this.props.card.display === "area" || this.props.card.display === "bar") { - var colors = this.props.visualizationSettingsApi.getDefaultColorHarmony(); + var colors = getDefaultColorHarmony(); var colorItems = []; for (var i=0; i < colors.length; i++) { var color = colors[i]; @@ -148,7 +91,7 @@ export default class VisualizationSettings extends React.Component { } // TODO: currently we set all chart type colors to the same value so bar color always works - var currentColor = this.props.card.visualization_settings.bar && this.props.card.visualization_settings.bar.color || this.props.visualizationSettingsApi.getDefaultColor(); + var currentColor = this.props.card.visualization_settings.bar && this.props.card.visualization_settings.bar.color || getDefaultColor(); var triggerElement = ( <span className="px2 py1 text-bold cursor-pointer text-default flex align-center"> <div className="ColorWell rounded bordered" style={{backgroundColor:currentColor}}></div> diff --git a/frontend/src/query_builder/filters/FilterWidget.jsx b/frontend/src/query_builder/filters/FilterWidget.jsx index 87ec2167878202641079e4980b8231f3aa7f3366..380678ac5c4a8ea8ca023f973a1a9322a9b0e654 100644 --- a/frontend/src/query_builder/filters/FilterWidget.jsx +++ b/frontend/src/query_builder/filters/FilterWidget.jsx @@ -143,7 +143,7 @@ export default class FilterWidget extends Component { {this.renderPopover()} </div> { this.props.removeFilter && - <a className="text-grey-2 no-decoration px1 flex align-center" href="#" onClick={this.removeFilter}> + <a className="text-grey-2 no-decoration px1 flex align-center" onClick={this.removeFilter}> <Icon name='close' width="14px" height="14px" /> </a> } diff --git a/frontend/src/query_builder/filters/pickers/TextPicker.jsx b/frontend/src/query_builder/filters/pickers/TextPicker.jsx index d363c3324dee70e7fa703e588aa545a91d0da2f8..999dd46cf5f4e51282a7b0cbf8e444c0373861d9 100644 --- a/frontend/src/query_builder/filters/pickers/TextPicker.jsx +++ b/frontend/src/query_builder/filters/pickers/TextPicker.jsx @@ -42,7 +42,7 @@ export default class TextPicker extends Component { <div> <ul> {values.map((value, index) => - <li className="px1 pt1 relative"> + <li className="FilterInput px1 pt1 relative"> <input className={cx("input block full border-purple", { "border-error": validations[index] === false })} type="text" @@ -52,8 +52,8 @@ export default class TextPicker extends Component { autoFocus={true} /> { index > 0 ? - <span className="absolute top right"> - <Icon name="close" className="cursor-pointer" width="16" height="16" onClick={() => this.removeValue(index)}/> + <span className="FilterRemove-field absolute top right"> + <Icon name="close" className="cursor-pointer text-white" width="12" height="12" onClick={() => this.removeValue(index)}/> </span> : null } </li> diff --git a/frontend/src/services.js b/frontend/src/services.js index 0969945c4c4c3e70ef6e3b838e69ee47de80e68e..fe39e21d792d682adb5dbae3459243981cd62952 100644 --- a/frontend/src/services.js +++ b/frontend/src/services.js @@ -2,7 +2,7 @@ import _ from "underscore"; import MetabaseAnalytics from 'metabase/lib/analytics'; import MetabaseCookies from 'metabase/lib/cookies'; -import MetabaseCore from 'metabase/lib/core'; +import * as MetabaseCore from 'metabase/lib/core'; import MetabaseSettings from 'metabase/lib/settings'; @@ -413,6 +413,14 @@ CoreServices.factory('Metabase', ['$resource', '$cookies', 'MetabaseCore', funct method: 'GET', isArray: true }, + db_list_with_tables: { + method: 'GET', + url: '/api/database/', + params: { + include_tables: 'true' + }, + isArray: true + }, db_create: { url: '/api/database/', method: 'POST' diff --git a/frontend/src/visualizations/AreaChart.jsx b/frontend/src/visualizations/AreaChart.jsx new file mode 100644 index 0000000000000000000000000000000000000000..25182f2aaf26fe35d11131e0d680678a238d9cf3 --- /dev/null +++ b/frontend/src/visualizations/AreaChart.jsx @@ -0,0 +1,26 @@ +import React, { Component, PropTypes } from "react"; + +import { MinColumnsError, MinRowsError } from "./errors"; + +import CardRenderer from "./CardRenderer.jsx"; + +export default class AreaChart extends Component { + static displayName = "Area"; + static identifier = "area"; + static iconName = "area"; + + static isSensible(cols, rows) { + return rows.length > 1 && cols.length > 1; + } + + static checkRenderable(cols, rows) { + if (cols.length < 2) { throw new MinColumnsError(2, cols.length); } + if (rows.length < 2) { throw new MinRowsError(2, rows.length); } + } + + render() { + return ( + <CardRenderer className="flex-full" {...this.props} /> + ); + } +} diff --git a/frontend/src/visualizations/BarChart.jsx b/frontend/src/visualizations/BarChart.jsx new file mode 100644 index 0000000000000000000000000000000000000000..c3cc09b790804124831c3473520dd9c0bbf32470 --- /dev/null +++ b/frontend/src/visualizations/BarChart.jsx @@ -0,0 +1,25 @@ +import React, { Component, PropTypes } from "react"; + +import CardRenderer from "./CardRenderer.jsx"; + +import { MinColumnsError } from "./errors"; + +export default class BarChart extends Component { + static displayName = "Bar"; + static identifier = "bar"; + static iconName = "bar"; + + static isSensible(cols, rows) { + return cols.length > 1; + } + + static checkRenderable(cols, rows) { + if (cols.length < 2) { throw new MinColumnsError(2, cols.length); } + } + + render() { + return ( + <CardRenderer className="flex-full" {...this.props} /> + ); + } +} diff --git a/frontend/src/visualizations/CardRenderer.jsx b/frontend/src/visualizations/CardRenderer.jsx new file mode 100644 index 0000000000000000000000000000000000000000..9c307e8ed20568f222d38c0c9094a57b4379f5e6 --- /dev/null +++ b/frontend/src/visualizations/CardRenderer.jsx @@ -0,0 +1,93 @@ +import React, { Component, PropTypes } from "react"; + +import ExplicitSize from "metabase/components/ExplicitSize.jsx"; + +import * as charting from '../card/card.charting'; + +import { getSettingsForVisualization, setLatitudeAndLongitude } from "metabase/lib/visualization_settings"; + +@ExplicitSize +export default class CardRenderer_ extends Component { + static propTypes = { + card: PropTypes.object.isRequired, + data: PropTypes.object + }; + + shouldComponentUpdate(nextProps, nextState) { + // a chart only needs re-rendering when the result itself changes OR the chart type is different + // NOTE: we are purposely doing an identity comparison here with props.result and NOT a value comparison + if (this.props.data == nextProps.data && + this.props.card.display === nextProps.card.display && + JSON.stringify(this.props.card.visualization_settings) === JSON.stringify(nextProps.card.visualization_settings) && + this.props.width === nextProps.width && + this.props.height === nextProps.height + ) { + return false; + } else { + return true; + } + } + + componentDidMount() { + this.renderChart(); + } + + componentDidUpdate() { + this.renderChart(); + } + + renderChart() { + let element = React.findDOMNode(this.refs.chart) + if (this.props.data) { + try { + // always ensure we have the most recent visualization settings to use for rendering + var vizSettings = getSettingsForVisualization(this.props.card.visualization_settings, this.props.card.display); + + var card = { + ...this.props.card, + visualization_settings: vizSettings + }; + + if (this.props.card.display === "pin_map") { + // call signature is (elementId, card, updateMapCenter (callback), updateMapZoom (callback)) + + // identify the lat/lon columns from our data and make them part of the viz settings so we can render maps + card.visualization_settings = setLatitudeAndLongitude(card.visualization_settings, this.props.data.cols); + + // these are example callback functions that could be passed into the renderer + // var updateMapCenter = function(lat, lon) { + // scope.card.visualization_settings.map.center_latitude = lat; + // scope.card.visualization_settings.map.center_longitude = lon; + // scope.$apply(); + // }; + + // var updateMapZoom = function(zoom) { + // scope.card.visualization_settings.map.zoom = zoom; + // scope.$apply(); + // }; + + var no_op = function(a, b) { + // do nothing for now + }; + + + charting.CardRenderer[this.props.card.display](element, card, no_op, no_op); + } else { + // TODO: it would be nicer if this didn't require the whole card + charting.CardRenderer[this.props.card.display](element, card, this.props.data); + } + } catch (err) { + console.error(err); + this.props.onRenderError(err.message || err); + } + } + } + + render() { + return ( + <div className="Card-outer px1"> + <div ref="chart"></div> + </div> + ); + } +} diff --git a/frontend/src/visualizations/LineChart.jsx b/frontend/src/visualizations/LineChart.jsx new file mode 100644 index 0000000000000000000000000000000000000000..3a765af74d22585639de8a1b144260120367b172 --- /dev/null +++ b/frontend/src/visualizations/LineChart.jsx @@ -0,0 +1,26 @@ +import React, { Component, PropTypes } from "react"; + +import CardRenderer from "./CardRenderer.jsx"; + +import { MinColumnsError, MinRowsError } from "./errors"; + +export default class LineChart extends Component { + static displayName = "Line"; + static identifier = "line"; + static iconName = "line"; + + static isSensible(cols, rows) { + return rows.length > 1 && cols.length > 1; + } + + static checkRenderable(cols, rows) { + if (cols.length < 2) { throw new MinColumnsError(2, cols.length); } + if (rows.length < 2) { throw new MinRowsError(2, rows.length); } + } + + render() { + return ( + <CardRenderer className="flex-full" {...this.props} /> + ); + } +} diff --git a/frontend/src/visualizations/ObjectDetail.jsx b/frontend/src/visualizations/ObjectDetail.jsx new file mode 100644 index 0000000000000000000000000000000000000000..a9f7153e05ce577a2cffed971e2dcc599fcf41bd --- /dev/null +++ b/frontend/src/visualizations/ObjectDetail.jsx @@ -0,0 +1,9 @@ +import React, { Component, PropTypes } from "react"; + +export default class ObjectDetail extends Component { + render() { + return ( + <div>Object Detail</div> + ); + } +} diff --git a/frontend/src/visualizations/PieChart.jsx b/frontend/src/visualizations/PieChart.jsx new file mode 100644 index 0000000000000000000000000000000000000000..8a1344b8c9bd6afc3def82ba6356ecfc83744d56 --- /dev/null +++ b/frontend/src/visualizations/PieChart.jsx @@ -0,0 +1,25 @@ +import React, { Component, PropTypes } from "react"; + +import CardRenderer from "./CardRenderer.jsx"; + +import { MinColumnsError } from "./errors"; + +export default class PieChart extends Component { + static displayName = "Pie"; + static identifier = "pie"; + static iconName = "pie"; + + static isSensible(cols, rows) { + return cols.length > 1; + } + + static checkRenderable(cols, rows) { + if (cols.length < 2) { throw new MinColumnsError(2, cols.length); } + } + + render() { + return ( + <CardRenderer className="flex-full" {...this.props} /> + ); + } +} diff --git a/frontend/src/visualizations/PinMap.jsx b/frontend/src/visualizations/PinMap.jsx new file mode 100644 index 0000000000000000000000000000000000000000..481e1832e68282a42118f3bac91f8ea4c6a6d40e --- /dev/null +++ b/frontend/src/visualizations/PinMap.jsx @@ -0,0 +1,27 @@ +import React, { Component, PropTypes } from "react"; + +import CardRenderer from "./CardRenderer.jsx"; + +import { hasLatitudeAndLongitudeColumns } from "metabase/lib/schema_metadata"; + +import { LatitudeLongitudeError } from "./errors"; + +export default class PinMap extends Component { + static displayName = "Pin Map"; + static identifier = "pin_map"; + static iconName = "pinmap"; + + static isSensible(cols, rows) { + return hasLatitudeAndLongitudeColumns(cols); + } + + static checkRenderable(cols, rows) { + if (!hasLatitudeAndLongitudeColumns(cols)) { throw new LatitudeLongitudeError(); } + } + + render() { + return ( + <CardRenderer className="flex-full" {...this.props} /> + ); + } +} diff --git a/frontend/src/visualizations/Scalar.jsx b/frontend/src/visualizations/Scalar.jsx new file mode 100644 index 0000000000000000000000000000000000000000..61a2763bb0fe70058b3ffc64818a1fe00095fc62 --- /dev/null +++ b/frontend/src/visualizations/Scalar.jsx @@ -0,0 +1,36 @@ +import React, { Component, PropTypes } from "react"; + +import { formatScalar } from "metabase/lib/formatting"; + +export default class Scalar extends Component { + static displayName = "Number"; + static identifier = "scalar"; + static iconName = "number"; + + static isSensible(cols, rows) { + return rows.length === 1 && cols.length === 1; + } + + static checkRenderable(cols, rows) { + // scalar can always be rendered, nothing needed here + } + + render() { + let { data, isDashboard } = this.props; + let formattedScalarValue = formatScalar(data && data.rows && data.rows[0] && data.rows[0][0] || ""); + + if (isDashboard) { + return ( + <div className={"Card--scalar " + this.props.className}> + <h1 className="Card-scalarValue text-normal">{formattedScalarValue}</h1> + </div> + ); + } else { + return ( + <div className="Visualization--scalar flex full layout-centered"> + <span>{formattedScalarValue}</span> + </div> + ); + } + } +} diff --git a/frontend/src/visualizations/Table.jsx b/frontend/src/visualizations/Table.jsx new file mode 100644 index 0000000000000000000000000000000000000000..12f51b762f4015b4d7abe17434e53574631a06a1 --- /dev/null +++ b/frontend/src/visualizations/Table.jsx @@ -0,0 +1,51 @@ +import React, { Component, PropTypes } from "react"; + +import TableInteractive from "./TableInteractive.jsx"; +import TableSimple from "./TableSimple.jsx"; + +import Query from "metabase/lib/query"; + +export default class Bar extends Component { + static displayName = "Table"; + static identifier = "table"; + static iconName = "table"; + + static isSensible(cols, rows) { + return true; + } + + static checkRenderable(cols, rows) { + // scalar can always be rendered, nothing needed here + } + + render() { + let { card, data, cellClickedFn, setSortFn } = this.props; + + if (this.props.isDashboard) { + return <TableSimple {...this.props} />; + } else { + let pivot = false; + let sort = card.dataset_query.query && card.dataset_query.query.order_by || null; + + // check if the data is pivotable (2 groupings + 1 agg != 'rows') + if (Query.isStructured(card.dataset_query) && + !Query.isBareRowsAggregation(card.dataset_query.query) && + data.cols.length === 3 + ) { + pivot = true; + setSortFn = null; + cellClickedFn = () => false; + } + + return ( + <TableInteractive + {...this.props} + pivot={pivot} + sort={sort} + setSortFn={setSortFn} + cellClickedFn={cellClickedFn} + /> + ); + } + } +} diff --git a/frontend/src/query_builder/QueryVisualizationTable.jsx b/frontend/src/visualizations/TableInteractive.jsx similarity index 92% rename from frontend/src/query_builder/QueryVisualizationTable.jsx rename to frontend/src/visualizations/TableInteractive.jsx index 75565e28333bbf26bc62a95adfec39261b04ad59..a85849f8a8f8d0cfd6dcc2f37b130dfac8fe6d3e 100644 --- a/frontend/src/query_builder/QueryVisualizationTable.jsx +++ b/frontend/src/visualizations/TableInteractive.jsx @@ -1,10 +1,11 @@ import React, { Component, PropTypes } from "react"; -import { Table, Column } from 'fixed-data-table'; +import { Table, Column } from "fixed-data-table"; + import Icon from "metabase/components/Icon.jsx"; import Popover from "metabase/components/Popover.jsx"; -import MetabaseAnalytics from '../lib/analytics'; +import MetabaseAnalytics from "metabase/lib/analytics"; import DataGrid from "metabase/lib/data_grid"; import { formatValue, capitalize } from "metabase/lib/formatting"; @@ -18,7 +19,7 @@ const QUICK_FILTERS = [ { name: ">", value: ">" } ]; -export default class QueryVisualizationTable extends Component { +export default class TableInteractive extends Component { constructor(props, context) { super(props, context); @@ -38,16 +39,12 @@ export default class QueryVisualizationTable extends Component { } static propTypes = { - data: PropTypes.object, - sort: PropTypes.array, - setSortFn: PropTypes.func, - isCellClickableFn: PropTypes.func, - cellClickedFn: PropTypes.func - }; - - static defaultProps = { - maxRows: 2000, - minColumnWidth: 75 + data: PropTypes.object.isRequired, + sort: PropTypes.array.isRequired, + setSortFn: PropTypes.func.isRequired, + isCellClickableFn: PropTypes.func.isRequired, + cellClickedFn: PropTypes.func.isRequired, + pivot: PropTypes.bool.isRequired }; componentWillMount() { @@ -79,10 +76,13 @@ export default class QueryVisualizationTable extends Component { // if size changes don't update yet because state will change in a moment this.calculateSizing(nextState); - // compare props and state to determine if we should re-render + // compare props (excluding card) and state to determine if we should re-render // NOTE: this is essentially the same as React.addons.PureRenderMixin but // we currently need to recalculate the container size here. - return !_.isEqual(this.props, nextProps) || !_.isEqual(this.state, nextState); + return ( + !_.isEqual({ ...this.props, card: null }, { ...nextProps, card: null }) || + !_.isEqual(this.state, nextState) + ); } componentDidUpdate() { @@ -183,7 +183,7 @@ export default class QueryVisualizationTable extends Component { var key = 'cl'+rowIndex+'_'+cellDataKey; if (this.props.cellIsClickableFn(rowIndex, cellDataKey)) { return ( - <a key={key} className="link cellData" href="#" onClick={this.cellClicked.bind(this, rowIndex, cellDataKey)}>{cellData}</a> + <a key={key} className="link cellData" onClick={this.cellClicked.bind(this, rowIndex, cellDataKey)}>{cellData}</a> ); } else { var popover = null; diff --git a/frontend/src/dashboard/components/cards/TableCard.jsx b/frontend/src/visualizations/TableSimple.jsx similarity index 97% rename from frontend/src/dashboard/components/cards/TableCard.jsx rename to frontend/src/visualizations/TableSimple.jsx index f9be17c3325672ceb94078ead575c464ddadde8d..0dd13a4d3a8d7c5b351d871742756870bf07a64b 100644 --- a/frontend/src/dashboard/components/cards/TableCard.jsx +++ b/frontend/src/visualizations/TableSimple.jsx @@ -1,6 +1,6 @@ import React, { Component, PropTypes } from "react"; -export default class TableCard extends Component { +export default class TableSimple extends Component { static propTypes = { data: PropTypes.object.isRequired diff --git a/frontend/src/visualizations/USStateMap.jsx b/frontend/src/visualizations/USStateMap.jsx new file mode 100644 index 0000000000000000000000000000000000000000..58d975bd617ddf3d3efc6c61de8a3aa991bf2c7e --- /dev/null +++ b/frontend/src/visualizations/USStateMap.jsx @@ -0,0 +1,27 @@ +import React, { Component, PropTypes } from "react"; + +import CardRenderer from "./CardRenderer.jsx"; + +import { isString } from "metabase/lib/schema_metadata"; + +import { MinColumnsError } from "./errors"; + +export default class USStateMap extends Component { + static displayName = "US State Map"; + static identifier = "state"; + static iconName = "statemap"; + + static isSensible(cols, rows) { + return cols.length > 1 && isString(cols[0]); + } + + static checkRenderable(cols, rows) { + if (cols.length < 2) { throw new MinColumnsError(2, cols.length); } + } + + render() { + return ( + <CardRenderer className="flex-full" {...this.props} /> + ); + } +} diff --git a/frontend/src/visualizations/Visualization.jsx b/frontend/src/visualizations/Visualization.jsx new file mode 100644 index 0000000000000000000000000000000000000000..22ac3004c2684d542074da473618016a52bf6db9 --- /dev/null +++ b/frontend/src/visualizations/Visualization.jsx @@ -0,0 +1,78 @@ +import React, { Component, PropTypes } from "react"; + +import visualizations from "."; + +import _ from "underscore"; + +export default class Visualization extends Component { + constructor(props, context) { + super(props, context) + + this.state = { + error: null + }; + + _.bindAll(this, "onRenderError"); + } + + static propTypes = { + card: PropTypes.object.isRequired, + data: PropTypes.object.isRequired, + isDashboard: PropTypes.bool, + + // used by TableInteractive + setSortFn: PropTypes.func, + cellIsClickableFn: PropTypes.func, + cellClickedFn: PropTypes.func + }; + + static defaultProps = { + isDashboard: false + }; + + componentWillMount() { + this.componentWillReceiveProps(this.props); + } + + componentWillReceiveProps(newProps) { + if (!newProps.data) { + this.setState({ error: "No data (TODO)" }); + } else if (!newProps.card.display) { + this.setState({ error: "Chart type not set" }); + } else { + let visualization = visualizations.get(newProps.card.display); + try { + if (visualization.checkRenderable) { + visualization.checkRenderable(newProps.data.cols, newProps.data.rows); + } + this.setState({ error: null }); + } catch (e) { + this.setState({ error: e.message || "Missing error message (TODO)" }); + } + } + } + + onRenderError(error) { + this.setState({ error }) + } + + render() { + let error = this.props.error || this.state.error; + if (error) { + return ( + <div className="QueryError flex full align-center text-error"> + <div className="QueryError-iconWrapper"> + <svg className="QueryError-icon" viewBox="0 0 32 32" width="64" height="64" fill="currentcolor"> + <path d="M4 8 L8 4 L16 12 L24 4 L28 8 L20 16 L28 24 L24 28 L16 20 L8 28 L4 24 L12 16 z "></path> + </svg> + </div> + <span className="QueryError-message">{error}</span> + </div> + ); + } else { + let { card } = this.props; + let CardVisualization = visualizations.get(card.display); + return <CardVisualization {...this.props} onRenderError={this.onRenderError} />; + } + } +} diff --git a/frontend/src/visualizations/WorldMap.jsx b/frontend/src/visualizations/WorldMap.jsx new file mode 100644 index 0000000000000000000000000000000000000000..1ca723b1d552a30fe72a38c1ad2891bbf4b52784 --- /dev/null +++ b/frontend/src/visualizations/WorldMap.jsx @@ -0,0 +1,27 @@ +import React, { Component, PropTypes } from "react"; + +import CardRenderer from "./CardRenderer.jsx"; + +import { isString } from "metabase/lib/schema_metadata"; + +import { MinColumnsError } from "./errors"; + +export default class WorldMap extends Component { + static displayName = "World Map"; + static identifier = "country"; + static iconName = "countrymap"; + + static isSensible(cols, rows) { + return cols.length > 1 && isString(cols[0]); + } + + static checkRenderable(cols, rows) { + if (cols.length < 2) { throw new MinColumnsError(2, cols.length); } + } + + render() { + return ( + <CardRenderer className="flex-full" {...this.props} /> + ); + } +} diff --git a/frontend/src/visualizations/errors.js b/frontend/src/visualizations/errors.js new file mode 100644 index 0000000000000000000000000000000000000000..4088c8b300590949f8e1c4f5b18dd5441b04f742 --- /dev/null +++ b/frontend/src/visualizations/errors.js @@ -0,0 +1,18 @@ + +export class MinColumnsError { + constructor(minColumns, actualColumns) { + this.message = "Doh! The data from your query doesn't fit the chosen display choice. This visualization requires at least " + minColumns + " columns of data."; + } +} + +export class MinRowsError { + constructor(minRows, actualRows) { + this.message = "No dice. We only have 1 data point to show and that's not enough for this visualization."; + } +} + +export class LatitudeLongitudeError { + constructor(minRows, actualRows) { + this.message = "Bummer. We can't actually do a pin map for this data because we require both a latitude and longitude column."; + } +} diff --git a/frontend/src/visualizations/index.js b/frontend/src/visualizations/index.js new file mode 100644 index 0000000000000000000000000000000000000000..307fd95b6852d9ae4cc76b2ccade1c267b2e524f --- /dev/null +++ b/frontend/src/visualizations/index.js @@ -0,0 +1,35 @@ + +import Scalar from "./Scalar.jsx"; +import Table from "./Table.jsx"; +import LineChart from "./LineChart.jsx"; +import BarChart from "./BarChart.jsx"; +import PieChart from "./PieChart.jsx"; +import AreaChart from "./AreaChart.jsx"; +import USStateMap from "./USStateMap.jsx"; +import WorldMap from "./WorldMap.jsx"; +import PinMap from "./PinMap.jsx"; + +const visualizations = new Map(); + +export function registerVisualization(visualization) { + let identifier = visualization.identifier; + if (identifier == null) { + throw new Error("Visualization must define an 'identifier' static variable: " + visualization.name); + } + if (visualizations.has(identifier)) { + throw new Error("Visualization with that identifier is already registered: " + visualization.name); + } + visualizations.set(identifier, visualization); +} + +registerVisualization(Scalar); +registerVisualization(Table); +registerVisualization(LineChart); +registerVisualization(BarChart); +registerVisualization(PieChart); +registerVisualization(AreaChart); +registerVisualization(USStateMap); +registerVisualization(WorldMap); +registerVisualization(PinMap); + +export default visualizations; diff --git a/src/metabase/api/common.clj b/src/metabase/api/common.clj index 773cdd112e15e7ef08599c98582f6caf7dbe6907..4ce9a11826647a8100a6f02571bec8683d4e881b 100644 --- a/src/metabase/api/common.clj +++ b/src/metabase/api/common.clj @@ -176,33 +176,33 @@ ;; #### GENERIC 400 RESPONSE HELPERS (def ^:private ^:const generic-400 [400 "Invalid Request."]) -(defn check-400 [tst] (check tst generic-400)) -(defmacro let-400 [& args] `(api-let ~generic-400 ~@args)) -(defmacro ->400 [& args] `(api-> ~generic-400 ~@args)) -(defmacro ->>400 [& args] `(api->> ~generic-400 ~@args)) +(defn check-400 "Throw a 400 if TEST is false." [tst] (check tst generic-400)) +(defmacro let-400 "Bind a form as with `let`; throw a 400 if it is `nil` or `false`." [& body] `(api-let ~generic-400 ~@body)) +(defmacro ->400 "If form is `nil` or `false`, throw a 400; otherwise thread it through BODY via `->`." [& body] `(api-> ~generic-400 ~@body)) +(defmacro ->>400 "If form is `nil` or `false`, throw a 400; otherwise thread it through BODY via `->>`." [& body] `(api->> ~generic-400 ~@body)) ;; #### GENERIC 404 RESPONSE HELPERS (def ^:private ^:const generic-404 [404 "Not found."]) -(defn check-404 [tst] (check tst generic-404)) -(defmacro let-404 [& args] `(api-let ~generic-404 ~@args)) -(defmacro ->404 [& args] `(api-> ~generic-404 ~@args)) -(defmacro ->>404 [& args] `(api->> ~generic-404 ~@args)) +(defn check-404 "Throw a 404 if TEST is false." [tst] (check tst generic-404)) +(defmacro let-404 "Bind a form as with `let`; throw a 404 if it is `nil` or `false`." [& body] `(api-let ~generic-404 ~@body)) +(defmacro ->404 "If form is `nil` or `false`, throw a 404; otherwise thread it through BODY via `->`." [& body] `(api-> ~generic-404 ~@body)) +(defmacro ->>404 "If form is `nil` or `false`, throw a 404; otherwise thread it through BODY via `->>`." [& body] `(api->> ~generic-404 ~@body)) ;; #### GENERIC 403 RESPONSE HELPERS ;; If you can't be bothered to write a custom error message (def ^:private ^:const generic-403 [403 "You don't have permissions to do that."]) -(defn check-403 [tst] (check tst generic-403)) -(defmacro let-403 [& args] `(api-let ~generic-403 ~@args)) -(defmacro ->403 [& args] `(api-> ~generic-403 ~@args)) -(defmacro ->>403 [& args] `(api->> ~generic-403 ~@args)) +(defn check-403 "Throw a 403 if TEST is false." [tst] (check tst generic-403)) +(defmacro let-403 "Bind a form as with `let`; throw a 403 if it is `nil` or `false`." [& body] `(api-let ~generic-403 ~@body)) +(defmacro ->403 "If form is `nil` or `false`, throw a 403; otherwise thread it through BODY via `->`." [& body] `(api-> ~generic-403 ~@body)) +(defmacro ->>403 "If form is `nil` or `false`, throw a 403; otherwise thread it through BODY via `->>`." [& body] `(api->> ~generic-403 ~@body)) ;; #### GENERIC 500 RESPONSE HELPERS ;; For when you don't feel like writing something useful (def ^:private ^:const generic-500 [500 "Internal server error."]) -(defn check-500 [tst] (check tst generic-500)) -(defmacro let-500 [& args] `(api-let ~generic-500 ~@args)) -(defmacro ->500 [& args] `(api-> ~generic-500 ~@args)) -(defmacro ->>500 [& args] `(api->> ~generic-500 ~@args)) +(defn check-500 "Throw a 500 if TEST is false." [tst] (check tst generic-500)) +(defmacro let-500 "Bind a form as with `let`; throw a 500 if it is `nil` or `false`." [& body] `(api-let ~generic-500 ~@body)) +(defmacro ->500 "If form is `nil` or `false`, throw a 500; otherwise thread it through BODY via `->`." [& body] `(api-> ~generic-500 ~@body)) +(defmacro ->>500 "If form is `nil` or `false`, throw a 500; otherwise thread it through BODY via `->>`." [& body] `(api->> ~generic-500 ~@body)) ;;; ## DEFENDPOINT AND RELATED FUNCTIONS @@ -210,15 +210,14 @@ ;;; ### Arg annotation fns -(defmulti -arg-annotation-fn - "*Internal* - don't use this directly. +(defmulti ^{:doc "*Internal* - don't use this directly. - Multimethod used internally to dispatch arg annotation functions. - Dispatches on the arg annotation as a keyword. + Multimethod used internally to dispatch arg annotation functions. + Dispatches on the arg annotation as a keyword. - {id Required} - -> ((-arg-annotation-fn :Required) 'id id) - -> (annotation:Required 'id id)" + {id Required} + -> ((-arg-annotation-fn :Required) 'id id) + -> (annotation:Required 'id id)"} -arg-annotation-fn ; for some reason supplying a docstr the normal way doesn't assoc it with the metadata like we'd expect (fn [annotation-kw] {:pre [(keyword? annotation-kw)]} annotation-kw)) @@ -440,3 +439,6 @@ obj) ([entity id] (check-403 (models/can-write? entity id)))) + + +(u/require-dox-in-this-namespace) diff --git a/src/metabase/api/common/internal.clj b/src/metabase/api/common/internal.clj index f0ba141f32dea7d0ff37b7bc6508fd682c34a120..16afc89d1d999403321642a201a9580f9159e970 100644 --- a/src/metabase/api/common/internal.clj +++ b/src/metabase/api/common/internal.clj @@ -260,7 +260,7 @@ [[annotation-kw arg-symb]] ; dispatch-fn passed as a param to avoid circular dependencies {:pre [(keyword? annotation-kw) (symbol? arg-symb)]} - `[~arg-symb (~((eval 'metabase.api.common/-arg-annotation-fn) annotation-kw) '~arg-symb ~arg-symb)]) + `[~arg-symb (~((resolve 'metabase.api.common/-arg-annotation-fn) annotation-kw) '~arg-symb ~arg-symb)]) (defn process-arg-annotations [annotations-map] {:pre [(or (nil? annotations-map) diff --git a/src/metabase/api/database.clj b/src/metabase/api/database.clj index 6ee98898d33d46d4e65f4093720a252a7406ad1c..9f791d3604bf6dc7d327881e16a6ab09addf198a 100644 --- a/src/metabase/api/database.clj +++ b/src/metabase/api/database.clj @@ -51,8 +51,13 @@ (defendpoint GET "/" "Fetch all `Databases`." - [] - (sel :many Database (k/order :name))) + [include_tables] + (let [dbs (sel :many Database (k/order :name))] + (if-not include_tables + dbs + (let [db-id->tables (group-by :db_id (sel :many Table, :active true))] + (for [db dbs] + (assoc db :tables (sort-by :name (get db-id->tables (:id db) [])))))))) (defendpoint POST "/" "Add a new `Database`." diff --git a/src/metabase/api/dataset.clj b/src/metabase/api/dataset.clj index 3151fe88d54c45328c00ae0e10b80f59afc7dcd8..0504ee587e03bd18bb53f53383cf9577d7c325b4 100644 --- a/src/metabase/api/dataset.clj +++ b/src/metabase/api/dataset.clj @@ -36,13 +36,13 @@ columns (map name columns)] ; turn keywords into strings, otherwise we get colons in our output (if (= status :completed) ;; successful query, send CSV file - {:status 200 - :body (with-out-str - (csv/write-csv *out* (into [columns] rows))) + {:status 200 + :body (with-out-str + (csv/write-csv *out* (into [columns] rows))) :headers {"Content-Type" "text/csv" "Content-Disposition" (str "attachment; filename=\"query_result_" (u/date->iso-8601) ".csv\"")}} ;; failed query, send error message {:status 500 - :body response}))) + :body (:error response)}))) (define-routes) diff --git a/src/metabase/config.clj b/src/metabase/config.clj index dd1aea33c32543a0934eb373fa83f91bdd18d8e2..460a4ba18628b5bceab52c4b099ea82fba24c256 100644 --- a/src/metabase/config.clj +++ b/src/metabase/config.clj @@ -85,24 +85,23 @@ :date date})) (defn- version-info-from-properties-file [] - (with-open [reader (io/reader (io/resource "version.properties"))] - (let [props (java.util.Properties.)] - (.load props reader) - (into {} (for [[k v] props] - [(keyword k) v]))))) - -(defn mb-version-info - "Return information about the current version of Metabase. + (when-let [props-file (io/resource "version.properties")] + (with-open [reader (io/reader props-file)] + (let [props (java.util.Properties.)] + (.load props reader) + (into {} (for [[k v] props] + [(keyword k) v])))))) + +(def ^:const mb-version-info + "Information about the current version of Metabase. This comes from `resources/version.properties` for prod builds and is fetched from `git` via the `./bin/version` script for dev. - (mb-version) -> {:tag: \"v0.11.1\", :hash: \"afdf863\", :branch: \"about_metabase\", :date: \"2015-10-05\"}" - [] + mb-version-info -> {:tag: \"v0.11.1\", :hash: \"afdf863\", :branch: \"about_metabase\", :date: \"2015-10-05\"}" (if (is-prod?) (version-info-from-properties-file) (version-info-from-shell-script))) -(defn mb-version-string - "Return a formatted version string representing the currently running application." - [] - (let [{:keys [tag hash branch date]} (mb-version-info)] +(def ^:const mb-version-string + "A formatted version string representing the currently running application." + (let [{:keys [tag hash branch date]} mb-version-info] (format "%s (%s %s)" tag hash branch))) diff --git a/src/metabase/core.clj b/src/metabase/core.clj index 923ec5a4fb4b3aa9a44f2b9eeed244f8daa774cd..b53ab8cae14cbe05d41726db81dbd5dc6249ffb7 100644 --- a/src/metabase/core.clj +++ b/src/metabase/core.clj @@ -56,19 +56,19 @@ "The primary entry point to the HTTP server" (-> routes/routes (mb-middleware/log-api-call) - mb-middleware/add-security-headers ; [METABASE] Add HTTP headers to API responses to prevent them from being cached - (wrap-json-body ; extracts json POST body and makes it avaliable on request + mb-middleware/add-security-headers ; Add HTTP headers to API responses to prevent them from being cached + (wrap-json-body ; extracts json POST body and makes it avaliable on request {:keywords? true}) - wrap-json-response ; middleware to automatically serialize suitable objects as JSON in responses - wrap-keyword-params ; converts string keys in :params to keyword keys - wrap-params ; parses GET and POST params as :query-params/:form-params and both as :params - mb-middleware/bind-current-user ; Binds *current-user* and *current-user-id* if :metabase-user-id is non-nil - mb-middleware/wrap-current-user-id ; looks for :metabase-session-id and sets :metabase-user-id if Session ID is valid - mb-middleware/wrap-api-key ; looks for a Metabase API Key on the request and assocs as :metabase-api-key - mb-middleware/wrap-session-id ; looks for a Metabase Session ID and assoc as :metabase-session-id - wrap-cookies ; Parses cookies in the request map and assocs as :cookies - wrap-session ; reads in current HTTP session and sets :session/key - wrap-gzip)) ; GZIP response if client can handle it + wrap-json-response ; middleware to automatically serialize suitable objects as JSON in responses + wrap-keyword-params ; converts string keys in :params to keyword keys + wrap-params ; parses GET and POST params as :query-params/:form-params and both as :params + mb-middleware/bind-current-user ; Binds *current-user* and *current-user-id* if :metabase-user-id is non-nil + mb-middleware/wrap-current-user-id ; looks for :metabase-session-id and sets :metabase-user-id if Session ID is valid + mb-middleware/wrap-api-key ; looks for a Metabase API Key on the request and assocs as :metabase-api-key + mb-middleware/wrap-session-id ; looks for a Metabase Session ID and assoc as :metabase-session-id + wrap-cookies ; Parses cookies in the request map and assocs as :cookies + wrap-session ; reads in current HTTP session and sets :session/key + wrap-gzip)) ; GZIP response if client can handle it ;;; ## ---------------------------------------- LIFECYCLE ---------------------------------------- @@ -115,7 +115,7 @@ (defn init! "General application initialization function which should be run once at application startup." [] - (log/info (format "Starting Metabase version %s..." (config/mb-version-string))) + (log/info (format "Starting Metabase version %s..." config/mb-version-string)) (reset! metabase-initialization-progress 0.1) ;; First of all, lets register a shutdown hook that will tidy things up for us on app exit @@ -176,12 +176,12 @@ :key-password (config/config-str :mb-jetty-ssl-keystore-password) :truststore (config/config-str :mb-jetty-ssl-truststore) :trust-password (config/config-str :mb-jetty-ssl-truststore-password)}) - jetty-config (cond-> (m/filter-vals identity {:port (config/config-int :mb-jetty-port) - :host (config/config-str :mb-jetty-host) - :max-threads (config/config-int :mb-jetty-maxthreads) - :min-threads (config/config-int :mb-jetty-minthreads) - :max-queued (config/config-int :mb-jetty-maxqueued) - :max-idle-time (config/config-int :mb-jetty-maxidletime)}) + jetty-config (cond-> (m/filter-vals identity {:port (config/config-int :mb-jetty-port) + :host (config/config-str :mb-jetty-host) + :max-threads (config/config-int :mb-jetty-maxthreads) + :min-threads (config/config-int :mb-jetty-minthreads) + :max-queued (config/config-int :mb-jetty-maxqueued) + :max-idle-time (config/config-int :mb-jetty-maxidletime)}) (config/config-str :mb-jetty-daemon) (assoc :daemon? (config/config-bool :mb-jetty-daemon)) (config/config-str :mb-jetty-ssl) (-> (assoc :ssl? true) (merge jetty-ssl-config)))] diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj index d63b97958a538795ae593d55f9a809dc89217e32..d81dfe2234547a246c9859eb35e4aed24f08ebea 100644 --- a/src/metabase/driver.clj +++ b/src/metabase/driver.clj @@ -13,7 +13,7 @@ [metabase.util :as u]) (:import clojure.lang.Keyword)) -(declare -dataset-query query-fail query-complete save-query-execution) +(declare query-fail query-complete save-query-execution) ;;; ## INTERFACE + CONSTANTS @@ -203,14 +203,16 @@ more))))) (defn default-field-percent-urls - "Default implementation for optional driver fn `:field-percent-urls` that calculates percentage in Clojure-land." + "Default implementation for optional driver fn `field-percent-urls` that calculates percentage in Clojure-land." [driver field] (->> (field-values-lazy-seq driver field) (filter identity) (take max-sync-lazy-seq-results) percent-valid-urls)) -(defn default-field-avg-length [driver field] +(defn default-field-avg-length + "Default implementation of optional driver fn `field-avg-length` that calculates the average length in Clojure-land via `field-values-lazy-seq`." + [driver field] (let [field-values (->> (field-values-lazy-seq driver field) (filter identity) (take max-sync-lazy-seq-results)) @@ -264,10 +266,10 @@ "Search Classpath for namespaces that start with `metabase.driver.`, then `require` them and look for the `driver-init` function which provides a uniform way for Driver initialization to be done." [] + (log/info "find-and-load-drivers!") (doseq [namespce (filter (fn [ns-symb] (re-matches #"^metabase\.driver\.[a-z0-9_]+$" (name ns-symb))) (ns-find/find-namespaces (classpath/classpath)))] - (log/info "loading driver namespace: " namespce) (require namespce))) (defn is-engine? @@ -462,6 +464,7 @@ (merge query-result))) (defn save-query-execution + "Save (or update) a `QueryExecution`." [{:keys [id] :as query-execution}] (if id ;; execution has already been saved, so update it @@ -470,3 +473,6 @@ query-execution) ;; first time saving execution, so insert it (m/mapply ins QueryExecution query-execution))) + + +(u/require-dox-in-this-namespace) diff --git a/src/metabase/driver/druid.clj b/src/metabase/driver/druid.clj index 0dee1f6baef5dbabc59bb3f0dba605e72d9316d4..e3c6e7b82fa1b5b799e6ed0e97570a5739b62767 100644 --- a/src/metabase/driver/druid.clj +++ b/src/metabase/driver/druid.clj @@ -48,8 +48,6 @@ ([_ details] (can-connect? details)) ([details] - ;; TODO - add a timeout to this (?) - ;; TODO - maybe check :version in the response? (= 200 (:status (http/get (details->url details "/status")))))) @@ -60,12 +58,11 @@ (try (vec (POST (details->url details "/druid/v2"), :body query)) (catch Throwable e ;; try to print the error - ;; TODO - log/error - (try (println (u/format-color 'red "Error running query:\n %s" - (:error (json/parse-string (:body (:object (ex-data e))) keyword)))) ; TODO - log/error + (try (log/error (u/format-color 'red "Error running query:\n %s" + (:error (json/parse-string (:body (:object (ex-data e))) keyword)))) ; TODO - log/error (catch Throwable _)) - ;; TODO - re-throw exception - #_(throw e)))) + ;; re-throw exception either way + (throw e)))) (defn- process-structured diff --git a/src/metabase/driver/druid/query_processor.clj b/src/metabase/driver/druid/query_processor.clj index c4610f23ba7cfd59a0cb82495e4523b970c7e502..15a1a3293d76f7ed721cb6f89a5ef495b39e5a1d 100644 --- a/src/metabase/driver/druid/query_processor.clj +++ b/src/metabase/driver/druid/query_processor.clj @@ -44,7 +44,9 @@ DateTimeField (->rvalue [this] (->rvalue (:field this))) Value (->rvalue [this] (:value this)) DateTimeValue (->rvalue [{{unit :unit} :field, value :value}] (u/date->iso-8601 (u/date-trunc-or-extract unit value))) - RelativeDateTimeValue (->rvalue [{:keys [unit amount]}] (u/date->iso-8601 (u/date-trunc-or-extract unit (u/relative-date unit amount))))) + RelativeDateTimeValue (->rvalue [{:keys [unit amount], :as this}] (if (zero? amount) + (u/date->iso-8601) + (u/date->iso-8601 (u/date-trunc-or-extract unit (u/relative-date unit amount)))))) (defprotocol ^:private IDimensionOrMetric (^:private dimension-or-metric? [this] @@ -61,7 +63,7 @@ (def ^:private ^:const query-type->default-query (let [defaults {:intervals ["-5000/5000"] :granularity :all - :context {:timeout 5000}}] + :context {:timeout 60000}}] {::select (merge defaults {:queryType :select :pagingSpec {:threshold 100 #_qp/absolute-max-results}}) ::timeseries (merge defaults {:queryType :timeseries}) @@ -207,7 +209,10 @@ ;;; ### handle-filter (defn- filter:not [filter] - {:type :not, :field filter}) + {:pre [filter]} + (if (= (:type filter) :not) ; it looks like "two nots don't make an identity" with druid + (:field filter) + {:type :not, :field filter})) (defn- filter:= [field value] {:type :selector @@ -231,6 +236,7 @@ (throw (IllegalArgumentException. (u/format-color 'red "WARNING: Filtering only works on dimensions! '%s' is a metric. Ignoring %s filter." (->rvalue field) filter-type)))))) (defn- parse-filter-subclause:filter [{:keys [filter-type field value] :as filter}] + {:pre [filter]} ;; We'll handle :timestamp separately. It needs to go in :intervals instead (when-not (instance? DateTimeField field) (try (when field @@ -266,12 +272,14 @@ (catch Throwable e (log/warn (.getMessage e)))))) -(defn- parse-filter-clause:filter [{:keys [compound-type subclauses], :as clause}] - (if-not compound-type - (parse-filter-subclause:filter clause) - (let [subclauses (filterv identity (map parse-filter-clause:filter subclauses))] - (when (seq subclauses) - {:type compound-type, :fields subclauses})))) +(defn- parse-filter-clause:filter [{:keys [compound-type subclauses subclause], :as clause}] + {:pre [clause]} + (case compound-type + :and {:type :and, :fields (filterv identity (map parse-filter-clause:filter subclauses))} + :or {:type :or, :fields (filterv identity (map parse-filter-clause:filter subclauses))} + :not (when-let [subclause (parse-filter-subclause:filter subclause)] + (filter:not subclause)) + nil (parse-filter-subclause:filter clause))) (defn- make-intervals [min max & more] @@ -316,7 +324,9 @@ "Ignoring these intervals: %s") (rest subclauses)))) [(first subclauses)]) ;; Ok to specify multiple intervals for OR - :or subclauses))))) + :or subclauses + ;; We should never get to this point since the all non-string negations should get automatically rewritten by the query expander. + :not (log/warn (u/format-color 'red "WARNING: Don't know how to negate: %s" clause))))))) (defn- handle-filter [_ {filter-clause :filter} druid-query] diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj index 0c18ec223a80a1d39d2ad6339db0404683313e93..440b7a0dc7c462be74a51cd88702935719026bd2 100644 --- a/src/metabase/driver/generic_sql/query_processor.clj +++ b/src/metabase/driver/generic_sql/query_processor.clj @@ -15,7 +15,9 @@ [metabase.driver.query-processor :as qp] metabase.driver.query-processor.interface [metabase.util :as u] - [metabase.util.korma-extensions :as kx]) + [metabase.util.korma-extensions :as kx] + [korma.sql.utils :as kutils] + [korma.sql.engine :as kengine]) (:import java.sql.Timestamp java.util.Date (metabase.driver.query_processor.interface AgFieldRef @@ -127,8 +129,9 @@ (defn- filter-subclause->predicate "Given a filter SUBCLAUSE, return a Korma filter predicate form for use in korma `where`." - [{:keys [filter-type], :as filter}] - (let [field (formatted (:field filter)) + [{:keys [filter-type field], :as filter}] + {:pre [(map? filter) field]} + (let [field (formatted field) value (some-> filter :value formatted)] (case filter-type :between {field ['between [(formatted (:min-val filter)) (formatted (:max-val filter))]]} @@ -144,10 +147,12 @@ := {field ['= value]} :!= {field ['not= value]}))) -(defn- filter-clause->predicate [{:keys [compound-type subclauses], :as clause}] +(defn- filter-clause->predicate [{:keys [compound-type subclause subclauses], :as clause}] + {:pre [(map? clause)]} (case compound-type :and (apply kfns/pred-and (map filter-clause->predicate subclauses)) :or (apply kfns/pred-or (map filter-clause->predicate subclauses)) + :not (kfns/pred-not (kengine/pred-map (filter-subclause->predicate subclause))) nil (filter-subclause->predicate clause))) (defn apply-filter [_ korma-query {clause :filter}] diff --git a/src/metabase/driver/mongo/query_processor.clj b/src/metabase/driver/mongo/query_processor.clj index 21ab79ca21352808e92b6d1e557d27b3776fa4af..b373ca7c1f99941c45bb0a0ebcc910c17360bc82 100644 --- a/src/metabase/driver/mongo/query_processor.clj +++ b/src/metabase/driver/mongo/query_processor.clj @@ -40,7 +40,7 @@ (defn- log-monger-form [form] (when-not qp/*disable-qp-logging* - (log/debug (u/format-color 'blue "\nMONGO AGGREGATION PIPELINE:\n%s\n" + (log/debug (u/format-color 'green "\nMONGO AGGREGATION PIPELINE:\n%s\n" (->> form (walk/postwalk #(if (symbol? %) (symbol (name %)) %)) ; strip namespace qualifiers from Monger form u/pprint-to-str) "\n")))) @@ -233,29 +233,33 @@ ;;; ### filter -(defn- parse-filter-subclause [{:keys [filter-type field value] :as filter}] +(defn- parse-filter-subclause [{:keys [filter-type field value] :as filter} & [negate?]] (let [field (when field (->lvalue field)) - value (when value (->rvalue value))] - (case filter-type - :between {field {$gte (->rvalue (:min-val filter)) - $lte (->rvalue (:max-val filter))}} - :is-null {field {$exists false}} - :not-null {field {$exists true}} - :contains {field (re-pattern value)} - :starts-with {field (re-pattern (str \^ value))} - :ends-with {field (re-pattern (str value \$))} - := {field value} - :!= {field {$ne value}} - :< {field {$lt value}} - :> {field {$gt value}} - :<= {field {$lte value}} - :>= {field {$gte value}}))) - -(defn- parse-filter-clause [{:keys [compound-type subclauses], :as clause}] - (cond - (= compound-type :and) {$and (mapv parse-filter-clause subclauses)} - (= compound-type :or) {$or (mapv parse-filter-clause subclauses)} - :else (parse-filter-subclause clause))) + value (when value (->rvalue value)) + v (case filter-type + :between {$gte (->rvalue (:min-val filter)) + $lte (->rvalue (:max-val filter))} + :is-null {$exists false} + :not-null {$exists true} + :contains (re-pattern value) + :starts-with (re-pattern (str \^ value)) + :ends-with (re-pattern (str value \$)) + := {"$eq" value} + :!= {$ne value} + :< {$lt value} + :> {$gt value} + :<= {$lte value} + :>= {$gte value})] + {field (if negate? + {$not v} + v)})) + +(defn- parse-filter-clause [{:keys [compound-type subclause subclauses], :as clause}] + (case compound-type + :and {$and (mapv parse-filter-clause subclauses)} + :or {$or (mapv parse-filter-clause subclauses)} + :not (parse-filter-subclause subclause :negate) + nil (parse-filter-subclause clause))) (defn- handle-filter [{filter-clause :filter} pipeline] (when filter-clause diff --git a/src/metabase/driver/query_processor/expand.clj b/src/metabase/driver/query_processor/expand.clj index cb8bc17cbf5e307d2ceeeb373e17ae8b8fc044ad..db411063a017cdfd7d255c4e9692cbd4e5a873c5 100644 --- a/src/metabase/driver/query_processor/expand.clj +++ b/src/metabase/driver/query_processor/expand.clj @@ -1,7 +1,7 @@ (ns metabase.driver.query-processor.expand "Converts a Query Dict as recieved by the API into an *expanded* one that contains extra information that will be needed to construct the appropriate native Query, and perform various post-processing steps such as Field ordering." - (:refer-clojure :exclude [< <= > >= = != and or filter count distinct sum]) + (:refer-clojure :exclude [< <= > >= = != and or not filter count distinct sum]) (:require (clojure [core :as core] [string :as str]) [clojure.tools.logging :as log] @@ -18,6 +18,7 @@ CompoundFilter EqualityFilter FieldPlaceholder + NotFilter RelativeDatetime StringFilter ValuePlaceholder))) @@ -99,14 +100,15 @@ (relative-datetime -31 :day)" ([n] (s/validate (s/eq :current) (normalize-token n)) (relative-datetime 0 nil)) - ([n :- s/Int, unit] (i/map->RelativeDatetime {:amount n, :unit (when-not (zero? n) + ([n :- s/Int, unit] (i/map->RelativeDatetime {:amount n, :unit (if (zero? n) + :day ; give :unit a default value so we can simplify the schema a bit and require a :unit (normalize-token unit))}))) ;;; ## aggregation (s/defn ^:private ^:always-validate ag-with-field :- i/Aggregation [ag-type f] - {:aggregation-type ag-type, :field (field f)}) + (i/strict-map->AggregationWithField {:aggregation-type ag-type, :field (field f)})) (def ^:ql ^{:arglists '([f])} avg "Aggregation clause. Return the average value of F." (partial ag-with-field :avg)) (def ^:ql ^{:arglists '([f])} distinct "Aggregation clause. Return the number of distinct values of F." (partial ag-with-field :distinct)) @@ -121,7 +123,7 @@ (s/defn ^:ql ^:always-validate count :- i/Aggregation "Aggregation clause. Return total row count (e.g., `COUNT(*)`). If F is specified, only count rows where F is non-null (e.g. `COUNT(f)`)." - ([] {:aggregation-type :count}) + ([] (i/strict-map->AggregationWithoutField {:aggregation-type :count})) ([f] (ag-with-field :count f))) (defn ^:ql ^:deprecated rows @@ -141,11 +143,13 @@ ;; Handle :aggregation top-level clauses ([query ag :- (s/maybe (s/pred map?))] - (if ag - (let [ag (update ag :aggregation-type normalize-token)] + (if-not ag + query + (let [ag ((if (:field ag) + i/map->AggregationWithField + i/map->AggregationWithoutField) (update ag :aggregation-type normalize-token))] (s/validate i/Aggregation ag) - (assoc query :aggregation ag)) - ag))) + (assoc query :aggregation ag))))) ;;; ## breakout & fields @@ -165,7 +169,7 @@ subclause) ([compound-type, subclause :- i/Filter, & more :- [i/Filter]] - (i/map->CompoundFilter {:compound-type compound-type, :subclauses (cons subclause more)}))) + (i/map->CompoundFilter {:compound-type compound-type, :subclauses (vec (cons subclause more))}))) (def ^:ql ^{:arglists '([& subclauses])} and "Filter subclause. Return results that satisfy *all* SUBCLAUSES." (partial compound-filter :and)) (def ^:ql ^{:arglists '([& subclauses])} or "Filter subclause. Return results that satisfy *any* of the SUBCLAUSES." (partial compound-filter :or)) @@ -191,8 +195,8 @@ (!= f v1 v2) ; same as (and (!= f v1) (!= f v2))" (partial equality-filter :!= and)) -(defn ^:ql is-null [f] (= f nil)) -(defn ^:ql not-null [f] (!= f nil)) +(defn ^:ql is-null "Filter subclause. Return results where F is `nil`." [f] (= f nil)) ; TODO - Should we deprecate these? They're syntactic sugar, and not particualarly useful. +(defn ^:ql not-null "Filter subclause. Return results where F is not `nil`." [f] (!= f nil)) ; not-null is doubly unnecessary since you could just use `not` instead. (s/defn ^:private ^:always-validate comparison-filter :- ComparisonFilter [filter-type f v] (i/map->ComparisonFilter {:filter-type filter-type, :field (field f), :value (value f v)})) @@ -217,14 +221,44 @@ (s/defn ^:private ^:always-validate string-filter :- StringFilter [filter-type f s] (i/map->StringFilter {:filter-type filter-type, :field (field f), :value (value f s)})) -(def ^:ql ^{:arglists '([f s])} starts-with "Filter subclause. Return results where F starts with the string V." (partial string-filter :starts-with)) -(def ^:ql ^{:arglists '([f s])} contains "Filter subclause. Return results where F contains the string V." (partial string-filter :contains)) -(def ^:ql ^{:arglists '([f s])} ends-with "Filter subclause. Return results where F ends with with the string V." (partial string-filter :ends-with)) +(def ^:ql ^{:arglists '([f s])} starts-with "Filter subclause. Return results where F starts with the string S." (partial string-filter :starts-with)) +(def ^:ql ^{:arglists '([f s])} contains "Filter subclause. Return results where F contains the string S." (partial string-filter :contains)) +(def ^:ql ^{:arglists '([f s])} ends-with "Filter subclause. Return results where F ends with with the string S." (partial string-filter :ends-with)) + +(s/defn ^:ql ^:always-validate not :- i/Filter + "Filter subclause. Return results that do *not* satisfy SUBCLAUSE. + + For the sake of simplifying driver implementation, `not` automatically translates its argument to a simpler, logically equivalent form whenever possible: + + (not (and x y)) -> (or (not x) (not y)) + (not (not x)) -> x + (not (= x y) -> (!= x y)" + {:added "0.15.0"} + [{:keys [compound-type subclause subclauses], :as clause} :- i/Filter] + (case compound-type + :and (apply or (mapv not subclauses)) + :or (apply and (mapv not subclauses)) + :not subclause + nil (let [{:keys [field value filter-type]} clause] + (case filter-type + := (!= field value) + :!= (= field value) + :< (>= field value) + :> (<= field value) + :<= (> field value) + :>= (< field value) + :between (let [{:keys [min-val max-val]} clause] + (or (< field min-val) + (> field max-val))) + (i/strict-map->NotFilter {:compound-type :not, :subclause clause}))))) + +(def ^:ql ^{:arglists '([f s]), :added "0.15.0"} does-not-contain "Filter subclause. Return results where F does not start with the string S." (comp not contains)) (s/defn ^:ql ^:always-validate time-interval :- i/Filter "Filter subclause. Syntactic sugar for specifying a specific time interval. - (filter {} (time-interval 100 :current :day)) ; return rows where datetime Field 100's value is in the current day" + ;; return rows where datetime Field 100's value is in the current day + (filter {} (time-interval (field-id 100) :current :day)) " [f n unit] (if-not (integer? n) (let [n (normalize-token n)] @@ -240,7 +274,7 @@ (core/< n -1) (between f (value f (relative-datetime (dec n) unit)) (value f (relative-datetime -1 unit))) (core/> n 1) (between f (value f (relative-datetime 1 unit)) - (value f (relative-datetime (inc n) unit))))))) + (value f (relative-datetime (inc n) unit))))))) (s/defn ^:ql ^:always-validate filter "Filter the results returned by the query. @@ -336,7 +370,7 @@ (core/or (token->ql-fn token) (throw (Exception. (str "Illegal clause (no matching fn found): " token)))))) -(s/defn ^:always-validate expand-ql-sexpr +(s/defn ^:always-validate ^:private expand-ql-sexpr "Expand a QL bracketed S-expression by dispatching to the appropriate `^:ql` function. If SEXPR is not a QL S-expression (the first item isn't a token), it is returned as-is. @@ -419,3 +453,6 @@ {:style/indent 0} [& body] `(run-query* (query ~@body))) + + +(u/require-dox-in-this-namespace) diff --git a/src/metabase/driver/query_processor/interface.clj b/src/metabase/driver/query_processor/interface.clj index bd47fc0a4a61d2020bdcefa09260457ab347b5cb..e4dcfbf2fce90c0d67466be317eca43e98c8fa54 100644 --- a/src/metabase/driver/query_processor/interface.clj +++ b/src/metabase/driver/query_processor/interface.clj @@ -3,7 +3,6 @@ This namespace should just contain definitions of various protocols and record types; associated logic should go in `metabase.driver.query-processor.expand`." (:require [schema.core :as s] - [metabase.driver :as driver] [metabase.models.field :as field] [metabase.util :as u]) (:import clojure.lang.Keyword @@ -19,7 +18,7 @@ "When `*driver*` is bound, assert that is supports keyword FEATURE." [feature] (when *driver* - (when-not (contains? (driver/features *driver*) feature) + (when-not (contains? ((resolve 'metabase.driver/features) *driver*) feature) (throw (Exception. (str (name feature) " is not supported by this driver.")))))) ;; Expansion Happens in a Few Stages: @@ -62,7 +61,7 @@ special-type :- (s/maybe (apply s/enum field/special-types)) table-id :- IntGreaterThanZero schema-name :- (s/maybe s/Str) - table-name :- s/Str + table-name :- (s/maybe s/Str) ; TODO - Why is this `maybe` ? position :- (s/maybe s/Int) ; TODO - int >= 0 description :- (s/maybe s/Str) parent-id :- (s/maybe IntGreaterThanZero) @@ -103,7 +102,7 @@ ;; Value is the expansion of a value within a QL clause ;; Information about the associated Field is included for convenience (s/defrecord Value [value :- (s/maybe (s/cond-pre s/Bool s/Num s/Str)) - field :- Field]) + field :- Field]) ;; TODO - Value doesn't need the whole field, just the relevant type info / units ;; e.g. an absolute point in time (literal) (s/defrecord DateTimeValue [value :- Timestamp @@ -147,7 +146,7 @@ (s/defrecord RelativeDatetime [amount :- s/Int - unit :- (s/maybe DatetimeValueUnit)]) + unit :- DatetimeValueUnit]) (def LiteralDatetimeString (s/constrained s/Str u/date-string? "Valid ISO-8601 datetime string literal")) (def LiteralDatetime (s/named (s/cond-pre java.sql.Date LiteralDatetimeString) "Valid datetime literal (must be ISO-8601 string or java.sql.Date)")) @@ -170,14 +169,13 @@ ;;; # ------------------------------------------------------------ CLAUSE SCHEMAS ------------------------------------------------------------ +(s/defrecord AggregationWithoutField [aggregation-type :- (s/eq :count)]) + +(s/defrecord AggregationWithField [aggregation-type :- (s/named (s/enum :avg :count :cumulative-sum :distinct :stddev :sum) "Valid aggregation type") + field :- FieldPlaceholder]) + (def Aggregation (s/constrained - (s/constrained - {:aggregation-type (s/named (s/enum :avg :count :cumulative-sum :distinct :stddev :sum) "Valid aggregation type") - (s/optional-key :field) FieldPlaceholder} - (fn [{:keys [aggregation-type field]}] - (or (= aggregation-type :count) - field)) - "Missing :field.") + (s/cond-pre AggregationWithField AggregationWithoutField) (fn [{:keys [aggregation-type]}] (when (= aggregation-type :stddev) (assert-driver-supports :standard-deviation-aggregations)) @@ -202,12 +200,19 @@ field :- FieldPlaceholder value :- StringValuePlaceholder]) -(def SimpleFilter (s/cond-pre EqualityFilter ComparisonFilter BetweenFilter StringFilter)) +(def SimpleFilterClause (s/named (s/cond-pre EqualityFilter ComparisonFilter BetweenFilter StringFilter) + "Simple filter clause")) + +(s/defrecord NotFilter [compound-type :- (s/eq :not) + subclause :- SimpleFilterClause]) + +(declare Filter) (s/defrecord CompoundFilter [compound-type :- (s/enum :and :or) - subclauses :- [(s/named (s/cond-pre SimpleFilter CompoundFilter) "Valid filter subclause in compound (and/or) filter")]]) + subclauses :- [(s/recursive #'Filter)]]) -(def Filter (s/named (s/cond-pre SimpleFilter CompoundFilter) "Valid filter clause")) +(def Filter (s/named (s/cond-pre SimpleFilterClause NotFilter CompoundFilter) + "Valid filter clause")) (def OrderBy (s/named {:field FieldPlaceholderOrAgRef diff --git a/src/metabase/driver/query_processor/parse.clj b/src/metabase/driver/query_processor/parse.clj deleted file mode 100644 index bcc0b0d52cf4d4ce5717841a71647ac44d37847e..0000000000000000000000000000000000000000 --- a/src/metabase/driver/query_processor/parse.clj +++ /dev/null @@ -1,31 +0,0 @@ -(ns metabase.driver.query-processor.parse - "Logic relating to parsing values associated with different Query Processor `Field` types." - (:require [metabase.driver.query-processor.interface :refer :all] - [metabase.util :as u]) - (:import (metabase.driver.query_processor.interface DateTimeField - Field - RelativeDatetime))) - -(defprotocol IParseValueForField - (parse-value [this value] - "Parse a value for a given type of `Field`.")) - -(extend-protocol IParseValueForField - Field - (parse-value [this value] - (map->Value {:field this, :value value})) - - DateTimeField - (parse-value [this value] - (cond - (u/date-string? value) - (map->DateTimeValue {:field this, :value (u/->Timestamp value)}) - - (instance? RelativeDatetime value) - (map->RelativeDateTimeValue {:field this, :amount (:amount value), :unit (:unit value)}) - - (nil? value) - nil - - :else - (throw (Exception. (format "Invalid value '%s': expected a DateTime." value)))))) diff --git a/src/metabase/driver/query_processor/resolve.clj b/src/metabase/driver/query_processor/resolve.clj index 82448d0be22b2d11575fd105594f603fea04abd2..df147e955709d149a035e7657eb693b3dd88946b 100644 --- a/src/metabase/driver/query_processor/resolve.clj +++ b/src/metabase/driver/query_processor/resolve.clj @@ -4,16 +4,21 @@ (:require (clojure [set :as set] [walk :as walk]) [medley.core :as m] + [schema.core :as s] [metabase.db :refer [sel]] - (metabase.driver.query-processor [interface :refer :all] - [parse :as parse]) + [metabase.driver.query-processor.interface :refer :all] (metabase.models [database :refer [Database]] [field :as field] [foreign-key :refer [ForeignKey]] - [table :refer [Table]])) + [table :refer [Table]]) + [metabase.util :as u]) (:import (metabase.driver.query_processor.interface DateTimeField + DateTimeValue Field FieldPlaceholder + RelativeDatetime + RelativeDateTimeValue + Value ValuePlaceholder))) ;; # ---------------------------------------------------------------------- UTIL FNS ------------------------------------------------------------ @@ -111,11 +116,36 @@ ;;; ## ------------------------------------------------------------ VALUE PLACEHOLDER ------------------------------------------------------------ +(defprotocol ^:private IParseValueForField + (^:private parse-value [this value] + "Parse a value for a given type of `Field`.")) + +(extend-protocol IParseValueForField + Field + (parse-value [this value] + (s/validate Value (map->Value {:field this, :value value}))) + + DateTimeField + (parse-value [this value] + (cond + (u/date-string? value) + (s/validate DateTimeValue (map->DateTimeValue {:field this, :value (u/->Timestamp value)})) + + (instance? RelativeDatetime value) + (do (s/validate RelativeDatetime value) + (s/validate RelativeDateTimeValue (map->RelativeDateTimeValue {:field this, :amount (:amount value), :unit (:unit value)}))) + + (nil? value) + nil + + :else + (throw (Exception. (format "Invalid value '%s': expected a DateTime." value)))))) + (defn- value-ph-resolve-field [{:keys [field-placeholder value]} field-id->fields] (let [resolved-field (resolve-field field-placeholder field-id->fields)] (when-not resolved-field (throw (Exception. (format "Unable to resolve field: %s" field-placeholder)))) - (parse/parse-value resolved-field value))) + (parse-value resolved-field value))) (extend ValuePlaceholder IResolve (merge IResolveDefaults @@ -215,9 +245,14 @@ ;;; # ------------------------------------------------------------ PUBLIC INTERFACE ------------------------------------------------------------ -(defn resolve [expanded-query-dict] +(defn resolve + "Resolve placeholders by fetching `Fields`, `Databases`, and `Tables` that are referred to in EXPANDED-QUERY-DICT." + [expanded-query-dict] (some-> expanded-query-dict record-fk-field-ids resolve-fields resolve-database resolve-tables)) + + +(u/require-dox-in-this-namespace) diff --git a/src/metabase/email/messages.clj b/src/metabase/email/messages.clj index 619794e0ebd7e557439a3dd14a046c9446c0c6ca..c729e2821bf7fb17098da364d08d3b5dadb7bd75 100644 --- a/src/metabase/email/messages.clj +++ b/src/metabase/email/messages.clj @@ -8,8 +8,8 @@ [metabase.models.setting :as setting] [metabase.pulse :as p, :refer [render-pulse-section]] [metabase.util :as u] - [metabase.util.quotation :as q] - [metabase.util.urls :as url])) + (metabase.util [quotation :as quotation] + [urls :as url]))) ;; NOTE: uncomment this in development to disable template caching ;; (loader/set-cache (clojure.core.cache/ttl-cache-factory {} :ttl 0)) @@ -19,7 +19,7 @@ (defn send-new-user-email "Format and Send an welcome email for newly created users." [invited invitor join-url] - (let [data-quote (rand-nth q/quotations) + (let [data-quote (quotation/random-quote) company (or (setting/get :site-name) "Unknown") message-body (stencil/render-file "metabase/email/new_user_invite" {:emailType "new_user_invite" @@ -69,7 +69,7 @@ "Segment" url/segment-url) add-url (fn [{:keys [id model] :as obj}] (assoc obj :url (apply (model->url-fn model) [id]))) - data-quote (rand-nth q/quotations) + data-quote (quotation/random-quote) context (-> context (update :dependencies (fn [deps-by-model] (for [model (sort (set (keys deps-by-model))) @@ -111,7 +111,7 @@ (let [images (atom []) body (apply vector :div (for [result results] (render-pulse-section (partial render-image images) :include-buttons result))) - data-quote (rand-nth q/quotations) + data-quote (quotation/random-quote) message-body (stencil/render-file "metabase/email/pulse" {:emailType "pulse" :pulse (html body) diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj index 5f5380e5e5c2753b2aaf28939f49ce60a0fa7843..3b8fcd083f7dba52994780ccf209de10a096566d 100644 --- a/src/metabase/models/database.clj +++ b/src/metabase/models/database.clj @@ -14,9 +14,9 @@ (defn- post-select [{:keys [engine] :as database}] (if-not engine database - (assoc database :features (if-let [driver ((resolve 'metabase.driver/engine->driver) engine)] - ((resolve 'metabase.driver/features) driver) - [])))) + (assoc database :features (or (when-let [driver ((resolve 'metabase.driver/engine->driver) engine)] + (seq ((resolve 'metabase.driver/features) driver))) + [])))) (defn- pre-cascade-delete [{:keys [id]}] (cascade-delete 'Card :database_id id) diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj index f74b7230818a0e8ea64fc3577f3b458e185d7e40..e2eb3a2d7ac7631bd938630accd6cf067456de05 100644 --- a/src/metabase/models/setting.clj +++ b/src/metabase/models/setting.clj @@ -4,7 +4,7 @@ [environ.core :as env] [korma.core :as k] [metabase.config :as config] - [metabase.db :refer [sel del]] + [metabase.db :refer [exists? sel del]] [metabase.models [common :as common] [interface :as i]] [metabase.setup :as setup] @@ -62,10 +62,11 @@ [k] {:pre [(keyword? k)]} (restore-cache-if-needed) - (or (@cached-setting->value k) - (when-let [v (sel :one :field [Setting :value] :key (name k))] - (swap! cached-setting->value assoc k v) - v))) + (if (contains? @cached-setting->value k) + (@cached-setting->value k) + (let [v (sel :one :field [Setting :value] :key (name k))] + (swap! cached-setting->value assoc k v) + v))) (defn- get-from-env-var "Given a `Setting` like `:mandrill-api-key`, return the value of the corresponding env var, @@ -188,46 +189,39 @@ :value (k settings)))) (sort-by :key)))) -(defn short-timezone-name - "Get a short display name for a TIMEZONE, e.g. `PST`." - [^TimeZone timezone] - (.getDisplayName timezone (.inDaylightTime timezone (new java.util.Date)) TimeZone/SHORT)) - -(defn get-instance-timezone - "Get the `report-timezone`, or fall back to the System default if it's not set." - [] - (let [^String timezone-name (get :report-timezone)] - (or (when (seq timezone-name) - (TimeZone/getTimeZone timezone-name)) - (TimeZone/getDefault)))) +(def ^:private short-timezone-name + "Get a short display name (e.g. `PST`) for `report-timezone`, or fall back to the System default if it's not set." + (memoize (fn [^String timezone-name] + (let [^TimeZone timezone (or (when (seq timezone-name) + (TimeZone/getTimeZone timezone-name)) + (TimeZone/getDefault))] + (.getDisplayName timezone (.inDaylightTime timezone (java.util.Date.)) TimeZone/SHORT))))) (defn public-settings "Return a simple map of key/value pairs which represent the public settings for the front-end application." [] - {:engines (@(resolve 'metabase.driver/available-drivers)) - :ga_code "UA-60817802-1" - :password_complexity (password/active-password-complexity) - :setup_token (setup/token-value) + {:ga_code "UA-60817802-1" + :password_complexity password/active-password-complexity :timezones common/timezones - :version (config/mb-version-info) - ;; all of these values are dynamic settings controlled at runtime + :version config/mb-version-info + :engines ((resolve 'metabase.driver/available-drivers)) + :setup_token (setup/token-value) :anon_tracking_enabled (let [tracking? (get :anon-tracking-enabled)] (or (nil? tracking?) (= "true" tracking?))) :site_name (get :site-name) - :email_configured (@(resolve 'metabase.email/email-configured?)) + :email_configured ((resolve 'metabase.email/email-configured?)) :admin_email (get :admin-email) :report_timezone (get :report-timezone) - :timezone_short (short-timezone-name (get-instance-timezone)) - :has_sample_dataset (some? (sel :one 'Database :is_sample true))}) + :timezone_short (short-timezone-name (get :report-timezone)) + :has_sample_dataset (exists? 'Database, :is_sample true)}) + ;;; # IMPLEMENTATION (defn- restore-cache-if-needed [] (when-not @cached-setting->value - (reset! cached-setting->value (->> (sel :many Setting) - (map (fn [{k :key v :value}] - {(keyword k) v})) - (into {}))))) + (reset! cached-setting->value (into {} (for [{k :key, v :value} (sel :many Setting)] + {(keyword k) v}))))) (def ^:private cached-setting->value "Map of setting name (keyword) -> string value, as they exist in the DB." diff --git a/src/metabase/query.clj b/src/metabase/query.clj index 762444eca11f58cbb94af3f1adddcd29ca387dc2..1c1b36d86f0759eb54528ca73e2737602be9eef5 100644 --- a/src/metabase/query.clj +++ b/src/metabase/query.clj @@ -2,11 +2,10 @@ "Functions for dealing with structured queries." (:require [clojure.core.match :refer [match]])) - (defn- parse-filter-subclause [subclause] (match subclause ["SEGMENT" (segment-id :guard integer?)] segment-id - subclause nil)) + _ nil)) (defn- parse-filter [clause] (match clause @@ -21,8 +20,9 @@ (filter identity) set))) -(defn extract-metric-ids [query] +(defn extract-metric-ids + [query] (when-let [aggregation-clause (:aggregation query)] (match aggregation-clause ["METRIC" (metric-id :guard integer?)] #{metric-id} - other nil))) + _ nil))) diff --git a/src/metabase/util/password.clj b/src/metabase/util/password.clj index c1b52fb7bd6e5ff04218ab9c2498e25d9cbc6c5a..224b6d72ef139d0e5e2a59aebb588182b4f2e9d6 100644 --- a/src/metabase/util/password.clj +++ b/src/metabase/util/password.clj @@ -45,9 +45,8 @@ (when (>= (occurances char-type) min-count) (recur more))))))) -(defn active-password-complexity +(def ^:const active-password-complexity "The currently configured description of the password complexity rules being enforced" - [] (merge (complexity->char-type->min (config/config-kw :mb-password-complexity)) ;; Setting MB_PASSWORD_LENGTH overrides the default :total for a given password complexity class (when-let [min-len (config/config-int :mb-password-length)] @@ -55,7 +54,7 @@ (def ^{:arglists '([password])} is-complex? "Check if a given password meets complexity standards for the application." - (partial password-has-char-counts? (active-password-complexity))) + (partial password-has-char-counts? active-password-complexity)) (defn verify-password diff --git a/src/metabase/util/quotation.clj b/src/metabase/util/quotation.clj index ac8c824770874ca09784fbbc68609bb7e1093ab8..b36d94b1e078f7b4ca0784605822bcd679ab15ef 100644 --- a/src/metabase/util/quotation.clj +++ b/src/metabase/util/quotation.clj @@ -1,7 +1,7 @@ (ns metabase.util.quotation) -(def ^:const quotations +(def ^:const ^:private quotations [{:quote "The world is one big data problem." :author "Andrew McAfee"} {:quote "Data really powers everything that we do." diff --git a/test/metabase/api/database_test.clj b/test/metabase/api/database_test.clj index 9d47c7a66cf98246825a131cf3d1b45ca71f07a9..66a89c2f5ad10aa2127fb664bdc35bd882ac18b6 100644 --- a/test/metabase/api/database_test.clj +++ b/test/metabase/api/database_test.clj @@ -40,7 +40,23 @@ :is_full_sync true :organization_id nil :description nil - :features (mapv name (metabase.driver/features (metabase.driver/engine->driver (:engine db))))}))) + :features (mapv name (driver/features (driver/engine->driver (:engine db))))}))) + +(defn- table-details [table] + (match-$ table + {:description $ + :entity_type $ + :visibility_type $ + :schema $ + :name $ + :display_name $ + :rows $ + :updated_at $ + :entity_name $ + :active $ + :id $ + :db_id $ + :created_at $})) ;; # DB LIFECYCLE ENDPOINTS @@ -71,7 +87,7 @@ :is_full_sync false :organization_id nil :description nil - :features (mapv name (metabase.driver/features (metabase.driver/engine->driver :postgres)))}) + :features (mapv name (driver/features (driver/engine->driver :postgres)))}) (create-db db-name false))) ;; ## DELETE /api/database/:id @@ -113,42 +129,85 @@ ;; Database details *should not* come back for Rasta since she's not a superuser (let [db-name (str "A" (random-name))] ; make sure this name comes before "test-data" (expect-eval-actual-first - (set (filter identity - (conj (for [engine datasets/all-valid-engines] - (datasets/when-testing-engine engine - (match-$ (get-or-create-test-data-db! (driver/engine->driver engine)) - {:created_at $ - :engine (name $engine) - :id $ - :updated_at $ - :name "test-data" - :is_sample false - :is_full_sync true - :organization_id nil - :description nil - :features (mapv name (metabase.driver/features (metabase.driver/engine->driver engine)))}))) - (match-$ (sel :one Database :name db-name) - {:created_at $ - :engine "postgres" - :id $ - :updated_at $ - :name $ - :is_sample false - :is_full_sync true - :organization_id nil - :description nil - :features (mapv name (metabase.driver/features (metabase.driver/engine->driver :postgres)))})))) + (set (filter identity (conj (for [engine datasets/all-valid-engines] + (datasets/when-testing-engine engine + (match-$ (get-or-create-test-data-db! (driver/engine->driver engine)) + {:created_at $ + :engine (name $engine) + :id $ + :updated_at $ + :name "test-data" + :is_sample false + :is_full_sync true + :organization_id nil + :description nil + :features (mapv name (driver/features (driver/engine->driver engine)))}))) + ;; (?) I don't remember why we have to do this for postgres but not any other of the bonus drivers + (match-$ (sel :one Database :name db-name) + {:created_at $ + :engine "postgres" + :id $ + :updated_at $ + :name $ + :is_sample false + :is_full_sync true + :organization_id nil + :description nil + :features (mapv name (driver/features (driver/engine->driver :postgres)))})))) + (do + ;; Delete all the randomly created Databases we've made so far + (cascade-delete Database :id [not-in (set (filter identity + (for [engine datasets/all-valid-engines] + (datasets/when-testing-engine engine + (:id (get-or-create-test-data-db! (driver/engine->driver engine)))))))]) + ;; Add an extra DB so we have something to fetch besides the Test DB + (create-db db-name) + ;; Now hit the endpoint + (set ((user->client :rasta) :get 200 "database"))))) + +;; GET /api/databases (include tables) +(let [db-name (str "A" (random-name))] ; make sure this name comes before "test-data" + (expect-eval-actual-first + (set (filter identity (conj (for [engine datasets/all-valid-engines] + (datasets/when-testing-engine engine + (let [database (get-or-create-test-data-db! (driver/engine->driver engine))] + (match-$ database + {:created_at $ + :engine (name $engine) + :id $ + :updated_at $ + :name "test-data" + :is_sample false + :is_full_sync true + :organization_id nil + :description nil + :tables (->> (sel :many Table :db_id (:id database)) + (mapv table-details) + (sort-by :name)) + :features (mapv name (driver/features (driver/engine->driver engine)))})))) + ;; (?) I don't remember why we have to do this for postgres but not any other of the bonus drivers + (match-$ (sel :one Database :name db-name) + {:created_at $ + :engine "postgres" + :id $ + :updated_at $ + :name $ + :is_sample false + :is_full_sync true + :organization_id nil + :description nil + :tables [] + :features (mapv name (driver/features (driver/engine->driver :postgres)))})))) (do ;; Delete all the randomly created Databases we've made so far (cascade-delete Database :id [not-in (set (filter identity (for [engine datasets/all-valid-engines] (datasets/when-testing-engine engine - (:id (get-or-create-test-data-db! (driver/engine->driver engine)))))))]) + (:id (get-or-create-test-data-db! (driver/engine->driver engine)))))))]) ;; Add an extra DB so we have something to fetch besides the Test DB (create-db db-name) ;; Now hit the endpoint - (set ((user->client :rasta) :get 200 "database"))))) - + (set ((user->client :rasta) :get 200 "database" :include_tables true))))) ;; ## GET /api/meta/table/:id/query_metadata ;; TODO - add in example with Field :values @@ -163,7 +222,7 @@ :is_full_sync true :organization_id nil :description nil - :features (mapv name (metabase.driver/features (metabase.driver/engine->driver :h2))) + :features (mapv name (driver/features (driver/engine->driver :h2))) :tables [(match-$ (Table (id :categories)) {:description nil :entity_type nil diff --git a/test/metabase/api/dataset_test.clj b/test/metabase/api/dataset_test.clj index 48883e8548f19913010726117c9fdef5653b5e2d..b91f6635fcd489fe12d2d270c302c7f73ff68b53 100644 --- a/test/metabase/api/dataset_test.clj +++ b/test/metabase/api/dataset_test.clj @@ -27,22 +27,22 @@ ;; Even if a query fails we still expect a 200 response from the api (expect-eval-actual-first - (match-$ (sel :one QueryExecution (k/order :id :desc)) - {:data {:rows [] - :cols [] - :columns []} - :error "Syntax error in SQL statement \"FOOBAR[*] \"; expected \"FROM, {\"" - :raw_query "" - :row_count 0 - :result_rows 0 - :status "failed" - :version 0 - :json_query $ - :started_at $ - :finished_at $ - :running_time $ - :id $ - :uuid $}) - ((user->client :rasta) :post 200 "dataset" {:database (id) - :type "native" - :native {:query "foobar"}})) + (match-$ (sel :one QueryExecution (k/order :id :desc)) + {:data {:rows [] + :cols [] + :columns []} + :error "Syntax error in SQL statement \"FOOBAR[*] \"; expected \"FROM, {\"" + :raw_query "" + :row_count 0 + :result_rows 0 + :status "failed" + :version 0 + :json_query $ + :started_at $ + :finished_at $ + :running_time $ + :id $ + :uuid $}) + ((user->client :rasta) :post 200 "dataset" {:database (id) + :type "native" + :native {:query "foobar"}})) diff --git a/test/metabase/api/notify_test.clj b/test/metabase/api/notify_test.clj index bf937c6ee31bdf30c9eca6116b4803ed914df630..044c72e3d64266475d2ffd84ed25526c2e8f4e83 100644 --- a/test/metabase/api/notify_test.clj +++ b/test/metabase/api/notify_test.clj @@ -17,8 +17,8 @@ (expect {:status 404 :body "Not found."} - (try (client/post (http/build-url "notify/db/100" {}) {:accept :json - :headers {"X-METABASE-APIKEY" "test-api-key"}}) + (try (client/post ((resolve 'metabase.http-client/build-url) "notify/db/100" {}) {:accept :json + :headers {"X-METABASE-APIKEY" "test-api-key"}}) (catch clojure.lang.ExceptionInfo e (-> (.getData ^clojure.lang.ExceptionInfo e) (:object) diff --git a/test/metabase/driver/event_query_processor_test.clj b/test/metabase/driver/event_query_processor_test.clj index b0ee971d98957cb4cd3568aa04ddeb72df322a38..62044d2b5a6acd558f68468426c06d9dc23d517d 100644 --- a/test/metabase/driver/event_query_processor_test.clj +++ b/test/metabase/driver/event_query_processor_test.clj @@ -1,23 +1,34 @@ (ns metabase.driver.event-query-processor-test "Query processor tests for DBs that are event-based, like Druid. There architecture is different enough that we can't test them along with our 'normal' DBs in `query-procesor-test`." + ;; TODO - renamed this `timeseries-query-processor-test` since Druid is a "timeseries database" according to Wikipedia (:require [expectations :refer :all] [metabase.driver.query-processor.expand :as ql] [metabase.driver.query-processor-test :refer [format-rows-by rows first-row]] [metabase.test.data :as data] - (metabase.test.data [datasets :as datasets] + (metabase.test.data [dataset-definitions :as defs] + [datasets :as datasets] [interface :as i]) [metabase.util :as u])) (def ^:private ^:const event-based-dbs #{:druid}) +;; this is a function rather than a straight delay because clojure complains when they delay gets embedding in the expanded macros below (def ^:private flattened-db-def - (let [def (delay (i/flatten-dbdef metabase.test.data.dataset-definitions/test-data "checkins"))] + (let [def (delay (i/flatten-dbdef defs/test-data "checkins"))] (fn [] @def))) -(defmacro ^:private expect-with-event-based-dbs +;; force loading of the flattened db definitions for the DBs that need it +(defn- load-event-based-db-data! + {:expectations-options :before-run} + [] + (doseq [engine event-based-dbs] + (datasets/with-engine-when-testing engine + (data/do-with-temp-db (flattened-db-def) (fn [& _]))))) + +(defmacro ^:private expect-with-timeseries-dbs {:style/indent 0} [expected actual] `(datasets/expect-with-engines event-based-dbs @@ -36,7 +47,7 @@ ;;; # Tests ;;; "bare rows" query, limit -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["id" "timestamp" "count" @@ -53,7 +64,7 @@ (ql/limit 2)))) ;;; fields clause -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["venue_name" "venue_category_name" "timestamp"], :rows [["Kinaree Thai Bistro" "Thai" "2013-01-03T08:00:00.000Z"] ["Ruen Pair Thai Restaurant" "Thai" "2013-01-10T08:00:00.000Z"]]} @@ -62,33 +73,33 @@ (ql/limit 2)))) ;;; count -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [1000] (first-row (data/run-query checkins (ql/aggregation (ql/count))))) ;;; count(field) -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [1000] (first-row (data/run-query checkins (ql/aggregation (ql/count $user_name))))) ;;; avg -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["avg"] :rows [[1.992]]} (data (data/run-query checkins (ql/aggregation (ql/avg $venue_price))))) ;;; sum -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["sum"] :rows [[1992.0]]} (data (data/run-query checkins (ql/aggregation (ql/sum $venue_price))))) ;;; avg -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["avg"] :rows [[1.992]]} (->> (data/run-query checkins @@ -97,14 +108,14 @@ data)) ;;; distinct count -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [[4]] (->> (data/run-query checkins (ql/aggregation (ql/distinct $venue_price))) rows (format-rows-by [int]))) ;;; 1 breakout (distinct values) -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["user_name"] :rows [["Broen Olujimi"] ["Conchúr Tihomir"] @@ -125,7 +136,7 @@ (ql/breakout $user_name)))) ;;; 2 breakouts -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["user_name" "venue_category_name"] :rows [["Broen Olujimi" "American"] ["Broen Olujimi" "Artisan"] @@ -142,7 +153,7 @@ (ql/limit 10)))) ;;; 1 breakout w/ explicit order by -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["user_name"] :rows [["Szymon Theutrich"] ["Spiros Teofil"] @@ -160,7 +171,7 @@ (ql/limit 10)))) ;;; 2 breakouts w/ explicit order by -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["user_name" "venue_category_name"] :rows [["Broen Olujimi" "American"] ["Conchúr Tihomir" "American"] @@ -178,7 +189,7 @@ (ql/limit 10)))) ;;; count w/ 1 breakout -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["user_name" "count"] :rows [["Broen Olujimi" 62] ["Conchúr Tihomir" 76] @@ -200,7 +211,7 @@ (ql/breakout $user_name)))) ;;; count w/ 2 breakouts -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["user_name" "venue_category_name" "count"] :rows [["Broen Olujimi" "American" 8] ["Broen Olujimi" "Artisan" 1] @@ -218,35 +229,35 @@ (ql/limit 10)))) ;;; filter > -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [49] (first-row (data/run-query checkins (ql/aggregation (ql/count)) (ql/filter (ql/> $venue_price 3))))) ;;; filter < -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [836] (first-row (data/run-query checkins (ql/aggregation (ql/count)) (ql/filter (ql/< $venue_price 3))))) ;;; filter >= -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [164] (first-row (data/run-query checkins (ql/aggregation (ql/count)) (ql/filter (ql/>= $venue_price 3))))) ;;; filter <= -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [951] (first-row (data/run-query checkins (ql/aggregation (ql/count)) (ql/filter (ql/<= $venue_price 3))))) ;;; filter = -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["user_name" "venue_name" "venue_category_name" "timestamp"] :rows [["Plato Yeshua" "Fred 62" "Diner" "2013-03-12T07:00:00.000Z"] ["Plato Yeshua" "Dimples" "Karaoke" "2013-04-11T07:00:00.000Z"] @@ -259,14 +270,14 @@ (ql/limit 5)))) ;;; filter != -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [969] (first-row (data/run-query checkins (ql/aggregation (ql/count)) (ql/filter (ql/!= $user_name "Plato Yeshua"))))) ;;; filter AND -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["user_name" "venue_name" "timestamp"] :rows [["Plato Yeshua" "The Daily Pint" "2013-07-25T07:00:00.000Z"]]} (data (data/run-query checkins @@ -275,7 +286,7 @@ (ql/= $user_name "Plato Yeshua")))))) ;;; filter OR -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [199] (first-row (data/run-query checkins (ql/aggregation (ql/count)) @@ -283,14 +294,14 @@ (ql/= $venue_category_name "American")))))) ;;; filter BETWEEN (inclusive) -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [951] (first-row (data/run-query checkins (ql/aggregation (ql/count)) (ql/filter (ql/between $venue_price 1 3))))) ;;; filter INSIDE -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["venue_name"] :rows [["Red Medicine"]]} (data (data/run-query checkins @@ -298,21 +309,21 @@ (ql/filter (ql/inside $venue_latitude $venue_longitude 10.0649 -165.379 10.0641 -165.371))))) ;;; filter IS_NULL -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [0] (first-row (data/run-query checkins (ql/aggregation (ql/count)) (ql/filter (ql/is-null $venue_category_name))))) ;;; filter NOT_NULL -(expect-with-event-based-dbs +(expect-with-timeseries-dbs [1000] (first-row (data/run-query checkins (ql/aggregation (ql/count)) (ql/filter (ql/not-null $venue_category_name))))) ;;; filter STARTS_WITH -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["venue_category_name"] :rows [["Mediterannian"] ["Mexican"]]} (data (data/run-query checkins @@ -320,7 +331,7 @@ (ql/filter (ql/starts-with $venue_category_name "Me"))))) ;;; filter ENDS_WITH -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["venue_category_name"] :rows [["American"] ["Artisan"] @@ -335,7 +346,7 @@ (ql/filter (ql/ends-with $venue_category_name "an"))))) ;;; filter CONTAINS -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["venue_category_name"] :rows [["American"] ["Bakery"] @@ -350,7 +361,7 @@ (ql/filter (ql/contains $venue_category_name "er"))))) ;;; order by aggregate field (?) -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["user_name" "venue_category_name" "count"] :rows [["Szymon Theutrich" "Bar" 13] ["Dwight Gresham" "Mexican" 12] @@ -369,7 +380,7 @@ (ql/limit 10)))) ;;; date bucketing - default (day) -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["2013-01-03-0800" 1] ["2013-01-10-0800" 1] @@ -382,7 +393,7 @@ (ql/limit 5)))) ;;; date bucketing - minute -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["2013-01-03T00:00:00-0800" 1] ["2013-01-10T00:00:00-0800" 1] ["2013-01-19T00:00:00-0800" 1] ["2013-01-22T00:00:00-0800" 1] ["2013-01-23T00:00:00-0800" 1]]} (data (data/run-query checkins @@ -391,7 +402,7 @@ (ql/limit 5)))) ;;; date bucketing - minute-of-hour -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["00" 1000]]} (data (data/run-query checkins @@ -400,7 +411,7 @@ (ql/limit 5)))) ;;; date bucketing - hour -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["2013-01-03T00:00:00-0800" 1] ["2013-01-10T00:00:00-0800" 1] @@ -413,7 +424,7 @@ (ql/limit 5)))) ;;; date bucketing - hour-of-day -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["00" 1000]]} (data (data/run-query checkins @@ -422,7 +433,7 @@ (ql/limit 5)))) ;;; date bucketing - week -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["2012-12-30" 1] ["2013-01-06" 1] @@ -435,7 +446,7 @@ (ql/limit 5)))) ;;; date bucketing - day -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["2013-01-03-0800" 1] ["2013-01-10-0800" 1] @@ -448,7 +459,7 @@ (ql/limit 5)))) ;;; date bucketing - day-of-week -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["1" 135] ["2" 143] @@ -461,7 +472,7 @@ (ql/limit 5)))) ;;; date bucketing - day-of-month -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["01" 36] ["02" 36] @@ -474,7 +485,7 @@ (ql/limit 5)))) ;;; date bucketing - day-of-year -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["003" 2] ["004" 6] @@ -487,7 +498,7 @@ (ql/limit 5)))) ;;; date bucketing - week-of-year -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["01" 10] ["02" 7] @@ -500,7 +511,7 @@ (ql/limit 5)))) ;;; date bucketing - month -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["2013-01-01" 8] ["2013-02-01" 11] @@ -513,7 +524,7 @@ (ql/limit 5)))) ;;; date bucketing - month-of-year -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["01" 38] ["02" 70] @@ -526,7 +537,7 @@ (ql/limit 5)))) ;;; date bucketing - quarter -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["2013-01-01" 40] ["2013-04-01" 75] @@ -539,7 +550,7 @@ (ql/limit 5)))) ;;; date bucketing - quarter-of-year -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["1" 200] ["2" 284] @@ -551,7 +562,7 @@ (ql/limit 5)))) ;;; date bucketing - year -(expect-with-event-based-dbs +(expect-with-timeseries-dbs {:columns ["timestamp" "count"] :rows [["2013" 235] ["2014" 498] @@ -560,3 +571,100 @@ (ql/aggregation (ql/count)) (ql/breakout (ql/datetime-field $timestamp :year)) (ql/limit 5)))) + + + +;;; `not` filter -- Test that we can negate the various other filter clauses + +;;; = +(expect-with-timeseries-dbs [999] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/= $id 1)))))) + +;;; != +(expect-with-timeseries-dbs [1] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/!= $id 1)))))) +;;; < +(expect-with-timeseries-dbs [961] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/< $id 40)))))) + +;;; > +(expect-with-timeseries-dbs [40] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/> $id 40)))))) + +;;; <= +(expect-with-timeseries-dbs [960] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/<= $id 40)))))) + +;;; >= +(expect-with-timeseries-dbs [39] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/>= $id 40)))))) + +;;; is-null +(expect-with-timeseries-dbs [1000] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/is-null $id)))))) + +;;; between +(expect-with-timeseries-dbs [989] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/between $id 30 40)))))) + +;;; inside +(expect-with-timeseries-dbs [377] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/inside $venue_latitude $venue_longitude 40 -120 30 -110)))))) + +;;; starts-with +(expect-with-timeseries-dbs [795] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/starts-with $venue_name "T")))))) + +;;; contains +(expect-with-timeseries-dbs [971] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/contains $venue_name "BBQ")))))) + +;;; ends-with +(expect-with-timeseries-dbs [884] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/ends-with $venue_name "a")))))) + +;;; and +(expect-with-timeseries-dbs [975] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/and (ql/> $id 32) + (ql/contains $venue_name "BBQ"))))))) +;;; or +(expect-with-timeseries-dbs [28] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/or (ql/> $id 32) + (ql/contains $venue_name "BBQ"))))))) + +;;; nested and/or +(expect-with-timeseries-dbs [969] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/or (ql/and (ql/> $id 32) + (ql/< $id 35)) + (ql/contains $venue_name "BBQ"))))))) + +;;; nested not +(expect-with-timeseries-dbs [29] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/not (ql/contains $venue_name "BBQ"))))))) + +;;; not nested inside and/or +(expect-with-timeseries-dbs [4] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) + (ql/filter (ql/and (ql/not (ql/> $id 32)) + (ql/contains $venue_name "BBQ")))))) + +;;; time-interval +(expect-with-timeseries-dbs [1000] (first-row (data/run-query checkins + (ql/aggregation (ql/count)) ; test data is all in the past so nothing happened today <3 + (ql/filter (ql/not (ql/time-interval $timestamp :current :day)))))) diff --git a/test/metabase/driver/query_processor_test.clj b/test/metabase/driver/query_processor_test.clj index 67f1bda306d63ef44f4641d259e85866cdecf94a..cf8d899518770c34bd20d9fdbd41a788652ebdfb 100644 --- a/test/metabase/driver/query_processor_test.clj +++ b/test/metabase/driver/query_processor_test.clj @@ -25,24 +25,31 @@ ;; ### Helper Fns + Macros ;; Event-Based DBs aren't tested here, but in `event-query-processor-test` instead. -(def ^:private ^:const event-db-engines #{:druid}) -(def ^:private ^:const non-event-db-engines (set/difference datasets/all-valid-engines event-db-engines)) +(def ^:private ^:const timeseries-engines #{:druid}) +(def ^:private ^:const non-timeseries-engines (set/difference datasets/all-valid-engines timeseries-engines)) (defn- engines-that-support [feature] (set (filter (fn [engine] (contains? (driver/features (driver/engine->driver engine)) feature)) - non-event-db-engines))) + non-timeseries-engines))) (defn- engines-that-dont-support [feature] - (set/difference non-event-db-engines (engines-that-support feature))) + (set/difference non-timeseries-engines (engines-that-support feature))) + +(defmacro ^:private expect-with-non-timeseries-dbs + {:style/indent 0} + [expected actual] + `(datasets/expect-with-engines non-timeseries-engines + ~expected + ~actual)) (defmacro ^:private qp-expect-with-all-engines [data q-form & post-process-fns] - `(datasets/expect-with-engines non-event-db-engines - {:status :completed - :row_count ~(count (:rows data)) - :data ~data} - (-> ~q-form - ~@post-process-fns))) + `(expect-with-non-timeseries-dbs + {:status :completed + :row_count ~(count (:rows data)) + :data ~data} + (-> ~q-form + ~@post-process-fns))) (defmacro ^:private qp-expect-with-engines [datasets data q-form] `(datasets/expect-with-engines ~datasets @@ -258,7 +265,8 @@ "Return the result rows from query results, or throw an Exception if they're missing." [results] (vec (or (-> results :data :rows) - (throw (ex-info "Error!" results))))) + (println (u/pprint-to-str 'red results)) + (throw (Exception. "Error!"))))) (defn first-row "Return the first row in the results of a query, or throw an Exception if they're missing." @@ -345,7 +353,7 @@ ;; Test that we can get "pages" of results. ;; ### PAGE - Get the first page -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[1 "African"] [2 "American"] [3 "Artisan"] @@ -357,7 +365,7 @@ rows (format-rows-by [int str]))) ;; ### PAGE - Get the second page -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[ 6 "Bakery"] [ 7 "Bar"] [ 8 "Beer Garden"] @@ -371,7 +379,7 @@ ;; ## "ORDER_BY" CLAUSE ;; Test that we can tell the Query Processor to return results ordered by multiple fields -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[1 12 375] [1 9 139] [1 1 72] @@ -394,7 +402,7 @@ ;; ## "FILTER" CLAUSE ;; ### FILTER -- "AND", ">", ">=" -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[55 "Dal Rae Restaurant" 67 33.983 -118.096 4] [61 "Lawry's The Prime Rib" 67 34.0677 -118.376 4] [77 "Sushi Nakazawa" 40 40.7318 -74.0045 4] @@ -407,7 +415,7 @@ rows formatted-venues-rows)) ;; ### FILTER -- "AND", "<", ">", "!=" -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[21 "PizzaHacker" 58 37.7441 -122.421 2] [23 "Taqueria Los Coyotes" 50 37.765 -122.42 2]] (-> (run-query venues @@ -420,7 +428,7 @@ ;; ### FILTER WITH A FALSE VALUE ;; Check that we're checking for non-nil values, not just logically true ones. ;; There's only one place (out of 3) that I don't like -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[1]] (->> (dataset places-cam-likes (run-query places @@ -436,7 +444,7 @@ x)) ;;; filter = true -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[1 "Tempest" true] [2 "Bullit" true]] (->> (dataset places-cam-likes @@ -446,7 +454,7 @@ rows (format-rows-by [int str ->bool] :format-nil-values))) ;;; filter != false -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[1 "Tempest" true] [2 "Bullit" true]] (->> (dataset places-cam-likes @@ -456,7 +464,7 @@ rows (format-rows-by [int str ->bool] :format-nil-values))) ;;; filter != true -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[3 "The Dentist" false]] (->> (dataset places-cam-likes (run-query places @@ -466,7 +474,7 @@ ;; ### FILTER -- "BETWEEN", single subclause (neither "AND" nor "OR") -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[21 "PizzaHacker" 58 37.7441 -122.421 2] [22 "Gordo Taqueria" 50 37.7822 -122.484 1]] (-> (run-query venues @@ -484,7 +492,7 @@ (ql/filter (ql/between $date "2015-04-01" "2015-05-01"))))) ;; ### FILTER -- "OR", "<=", "=" -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[1 "Red Medicine" 4 10.0646 -165.374 3] [2 "Stout Burgers & Beers" 11 34.0996 -118.329 2] [3 "The Apple Pan" 11 34.0406 -118.428 2] @@ -496,7 +504,7 @@ rows formatted-venues-rows)) ;; ### FILTER -- "INSIDE" -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[1 "Red Medicine" 4 10.0646 -165.374 3]] (-> (run-query venues (ql/filter (ql/inside $latitude $longitude 10.0649 -165.379 10.0641 -165.371))) @@ -744,7 +752,7 @@ ;;; ### order_by aggregate ["avg" field-id] -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs {:columns [(format-name "price") "avg"] :rows [[3 22] @@ -778,25 +786,25 @@ :data (format-rows-by [int (comp int math/round)]))) ;;; ### make sure that rows where preview_display = false are included and properly marked up -(datasets/expect-with-engines non-event-db-engines - [(set (venues-cols)) - #{(venues-col :category_id) - (venues-col :name) - (venues-col :latitude) - (venues-col :id) - (venues-col :longitude) - (-> (venues-col :price) - (assoc :preview_display false))} - (set (venues-cols))] - (let [get-col-names (fn [] (-> (run-query venues - (ql/order-by (ql/asc $id)) - (ql/limit 1)) - :data :cols set))] - [(get-col-names) - (do (upd Field (id :venues :price) :preview_display false) - (get-col-names)) - (do (upd Field (id :venues :price) :preview_display true) - (get-col-names))])) +(expect-with-non-timeseries-dbs + [(set (venues-cols)) + #{(venues-col :category_id) + (venues-col :name) + (venues-col :latitude) + (venues-col :id) + (venues-col :longitude) + (-> (venues-col :price) + (assoc :preview_display false))} + (set (venues-cols))] + (let [get-col-names (fn [] (-> (run-query venues + (ql/order-by (ql/asc $id)) + (ql/limit 1)) + :data :cols set))] + [(get-col-names) + (do (upd Field (id :venues :price) :preview_display false) + (get-col-names)) + (do (upd Field (id :venues :price) :preview_display true) + (get-col-names))])) ;;; ## :sensitive fields @@ -833,7 +841,7 @@ ;; +------------------------------------------------------------------------------------------------------------------------+ ;; There were 9 "sad toucan incidents" on 2015-06-02 -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (if (i/has-questionable-timezone-support? *data-loader*) 10 9) @@ -843,7 +851,7 @@ (ql/< $timestamp "2015-06-03"))) (ql/order-by (ql/asc $timestamp))))))) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (cond (= *engine* :sqlite) [["2015-06-01" 6] @@ -887,7 +895,7 @@ (ql/aggregation (ql/count)) (ql/breakout $timestamp) (ql/limit 10))) - rows (format-rows-by [identity int]))) + rows (format-rows-by [identity int]))) ;; +------------------------------------------------------------------------------------------------------------------------+ @@ -1128,29 +1136,29 @@ ;;; # New Filter Types - CONTAINS, STARTS_WITH, ENDS_WITH ;;; ## STARTS_WITH -(datasets/expect-with-engines non-event-db-engines - [[41 "Cheese Steak Shop" 18 37.7855 -122.44 1] - [74 "Chez Jay" 2 34.0104 -118.493 2]] - (-> (run-query venues - (ql/filter (ql/starts-with $name "Che")) - (ql/order-by (ql/asc $id))) - rows formatted-venues-rows)) +(expect-with-non-timeseries-dbs + [[41 "Cheese Steak Shop" 18 37.7855 -122.44 1] + [74 "Chez Jay" 2 34.0104 -118.493 2]] + (-> (run-query venues + (ql/filter (ql/starts-with $name "Che")) + (ql/order-by (ql/asc $id))) + rows formatted-venues-rows)) ;;; ## ENDS_WITH -(datasets/expect-with-engines non-event-db-engines - [[ 5 "Brite Spot Family Restaurant" 20 34.0778 -118.261 2] - [ 7 "Don Day Korean Restaurant" 44 34.0689 -118.305 2] - [17 "Ruen Pair Thai Restaurant" 71 34.1021 -118.306 2] - [45 "Tu Lan Restaurant" 4 37.7821 -122.41 1] - [55 "Dal Rae Restaurant" 67 33.983 -118.096 4]] - (-> (run-query venues - (ql/filter (ql/ends-with $name "Restaurant")) - (ql/order-by (ql/asc $id))) - rows formatted-venues-rows)) +(expect-with-non-timeseries-dbs + [[ 5 "Brite Spot Family Restaurant" 20 34.0778 -118.261 2] + [ 7 "Don Day Korean Restaurant" 44 34.0689 -118.305 2] + [17 "Ruen Pair Thai Restaurant" 71 34.1021 -118.306 2] + [45 "Tu Lan Restaurant" 4 37.7821 -122.41 1] + [55 "Dal Rae Restaurant" 67 33.983 -118.096 4]] + (-> (run-query venues + (ql/filter (ql/ends-with $name "Restaurant")) + (ql/order-by (ql/asc $id))) + rows formatted-venues-rows)) ;;; ## CONTAINS -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[31 "Bludso's BBQ" 5 33.8894 -118.207 2] [34 "Beachwood BBQ & Brewing" 10 33.7701 -118.191 2] [39 "Baby Blues BBQ" 5 34.0003 -118.465 2]] @@ -1161,7 +1169,7 @@ ;;; ## Nested AND / OR -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[81]] (->> (run-query venues (ql/aggregation (ql/count)) @@ -1173,19 +1181,19 @@ ;;; ## = / != with multiple values -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[81]] (->> (run-query venues (ql/aggregation (ql/count)) (ql/filter (ql/= $price 1 2))) rows (format-rows-by [int]))) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[19]] (->> (run-query venues (ql/aggregation (ql/count)) (ql/filter (ql/!= $price 1 2))) - rows (format-rows-by [int]))) + rows (format-rows-by [int]))) ;; +-------------------------------------------------------------------------------------------------------------+ @@ -1204,7 +1212,7 @@ rows (format-rows-by [(fn [x] (if (number? x) (int x) x)) int]))) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (cond (contains? #{:redshift :sqlserver} *engine*) [[#inst "2015-06-01T17:31" 1] @@ -1243,7 +1251,7 @@ [#inst "2015-06-02T11:11" 1]]) (sad-toucan-incidents-with-bucketing :default)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (cond (= *engine* :sqlite) [["2015-06-01 10:31:00" 1] @@ -1282,7 +1290,7 @@ [#inst "2015-06-02T11:11" 1]]) (sad-toucan-incidents-with-bucketing :minute)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[0 5] [1 4] [2 2] @@ -1295,7 +1303,7 @@ [9 1]] (sad-toucan-incidents-with-bucketing :minute-of-hour)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (cond (= *engine* :sqlite) [["2015-06-01 10:00:00" 1] @@ -1334,13 +1342,13 @@ [#inst "2015-06-02T13" 1]]) (sad-toucan-incidents-with-bucketing :hour)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (if (i/has-questionable-timezone-support? *data-loader*) - [[0 13] [1 8] [2 4] [3 7] [4 5] [5 13] [6 10] [7 8] [8 9] [9 7]] - [[0 8] [1 9] [2 7] [3 10] [4 10] [5 9] [6 6] [7 5] [8 7] [9 7]]) + [[0 13] [1 8] [2 4] [3 7] [4 5] [5 13] [6 10] [7 8] [8 9] [9 7]] + [[0 8] [1 9] [2 7] [3 10] [4 10] [5 9] [6 6] [7 5] [8 7] [9 7]]) (sad-toucan-incidents-with-bucketing :hour-of-day)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (cond (= *engine* :sqlite) [["2015-06-01" 6] @@ -1379,25 +1387,25 @@ [#inst "2015-06-10T07" 10]]) (sad-toucan-incidents-with-bucketing :day)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (if (i/has-questionable-timezone-support? *data-loader*) - [[1 28] [2 38] [3 29] [4 27] [5 24] [6 30] [7 24]] - [[1 29] [2 36] [3 33] [4 29] [5 13] [6 38] [7 22]]) + [[1 28] [2 38] [3 29] [4 27] [5 24] [6 30] [7 24]] + [[1 29] [2 36] [3 33] [4 29] [5 13] [6 38] [7 22]]) (sad-toucan-incidents-with-bucketing :day-of-week)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (if (i/has-questionable-timezone-support? *data-loader*) - [[1 6] [2 10] [3 4] [4 9] [5 9] [6 8] [7 8] [8 9] [9 7] [10 9]] - [[1 8] [2 9] [3 9] [4 4] [5 11] [6 8] [7 6] [8 10] [9 6] [10 10]]) + [[1 6] [2 10] [3 4] [4 9] [5 9] [6 8] [7 8] [8 9] [9 7] [10 9]] + [[1 8] [2 9] [3 9] [4 4] [5 11] [6 8] [7 6] [8 10] [9 6] [10 10]]) (sad-toucan-incidents-with-bucketing :day-of-month)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (if (i/has-questionable-timezone-support? *data-loader*) - [[152 6] [153 10] [154 4] [155 9] [156 9] [157 8] [158 8] [159 9] [160 7] [161 9]] - [[152 8] [153 9] [154 9] [155 4] [156 11] [157 8] [158 6] [159 10] [160 6] [161 10]]) + [[152 6] [153 10] [154 4] [155 9] [156 9] [157 8] [158 8] [159 9] [160 7] [161 9]] + [[152 8] [153 9] [154 9] [155 4] [156 11] [157 8] [158 6] [159 10] [160 6] [161 10]]) (sad-toucan-incidents-with-bucketing :day-of-year)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (cond (= *engine* :sqlite) [["2015-05-31" 46] @@ -1421,7 +1429,7 @@ [#inst "2015-06-28T07" 7]]) (sad-toucan-incidents-with-bucketing :week)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs (cond (contains? #{:sqlserver :sqlite} *engine*) [[23 54] [24 46] [25 39] [26 61]] @@ -1433,23 +1441,23 @@ [[23 49] [24 47] [25 39] [26 58] [27 7]]) (sad-toucan-incidents-with-bucketing :week-of-year)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[(if (= *engine* :sqlite) "2015-06-01", #inst "2015-06-01T07") 200]] (sad-toucan-incidents-with-bucketing :month)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[6 200]] (sad-toucan-incidents-with-bucketing :month-of-year)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[(if (= *engine* :sqlite) "2015-04-01", #inst "2015-04-01T07") 200]] (sad-toucan-incidents-with-bucketing :quarter)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[2 200]] (sad-toucan-incidents-with-bucketing :quarter-of-year)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs [[2015 200]] (sad-toucan-incidents-with-bucketing :year)) @@ -1476,22 +1484,22 @@ (apply ql/relative-datetime relative-datetime-args))))) first-row first int)) -(datasets/expect-with-engines non-event-db-engines 4 (count-of-grouping (checkins:4-per-minute) :minute "current")) -(datasets/expect-with-engines non-event-db-engines 4 (count-of-grouping (checkins:4-per-minute) :minute -1 "minute")) -(datasets/expect-with-engines non-event-db-engines 4 (count-of-grouping (checkins:4-per-minute) :minute 1 "minute")) +(expect-with-non-timeseries-dbs 4 (count-of-grouping (checkins:4-per-minute) :minute "current")) +(expect-with-non-timeseries-dbs 4 (count-of-grouping (checkins:4-per-minute) :minute -1 "minute")) +(expect-with-non-timeseries-dbs 4 (count-of-grouping (checkins:4-per-minute) :minute 1 "minute")) -(datasets/expect-with-engines non-event-db-engines 4 (count-of-grouping (checkins:4-per-hour) :hour "current")) -(datasets/expect-with-engines non-event-db-engines 4 (count-of-grouping (checkins:4-per-hour) :hour -1 "hour")) -(datasets/expect-with-engines non-event-db-engines 4 (count-of-grouping (checkins:4-per-hour) :hour 1 "hour")) +(expect-with-non-timeseries-dbs 4 (count-of-grouping (checkins:4-per-hour) :hour "current")) +(expect-with-non-timeseries-dbs 4 (count-of-grouping (checkins:4-per-hour) :hour -1 "hour")) +(expect-with-non-timeseries-dbs 4 (count-of-grouping (checkins:4-per-hour) :hour 1 "hour")) -(datasets/expect-with-engines non-event-db-engines 1 (count-of-grouping (checkins:1-per-day) :day "current")) -(datasets/expect-with-engines non-event-db-engines 1 (count-of-grouping (checkins:1-per-day) :day -1 "day")) -(datasets/expect-with-engines non-event-db-engines 1 (count-of-grouping (checkins:1-per-day) :day 1 "day")) +(expect-with-non-timeseries-dbs 1 (count-of-grouping (checkins:1-per-day) :day "current")) +(expect-with-non-timeseries-dbs 1 (count-of-grouping (checkins:1-per-day) :day -1 "day")) +(expect-with-non-timeseries-dbs 1 (count-of-grouping (checkins:1-per-day) :day 1 "day")) -(datasets/expect-with-engines non-event-db-engines 7 (count-of-grouping (checkins:1-per-day) :week "current")) +(expect-with-non-timeseries-dbs 7 (count-of-grouping (checkins:1-per-day) :week "current")) ;; SYNTACTIC SUGAR -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs 1 (-> (with-temp-db [_ (checkins:1-per-day)] (run-query checkins @@ -1499,7 +1507,7 @@ (ql/filter (ql/time-interval $timestamp :current :day)))) first-row first int)) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs 7 (-> (with-temp-db [_ (checkins:1-per-day)] (run-query checkins @@ -1520,22 +1528,122 @@ {:rows (-> results :row_count) :unit (-> results :data :cols first :unit)})) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs {:rows 1, :unit :day} (date-bucketing-unit-when-you :breakout-by "day", :filter-by "day")) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs {:rows 7, :unit :day} (date-bucketing-unit-when-you :breakout-by "day", :filter-by "week")) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs {:rows 1, :unit :week} (date-bucketing-unit-when-you :breakout-by "week", :filter-by "day")) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs {:rows 1, :unit :quarter} (date-bucketing-unit-when-you :breakout-by "quarter", :filter-by "day")) -(datasets/expect-with-engines non-event-db-engines +(expect-with-non-timeseries-dbs {:rows 1, :unit :hour} (date-bucketing-unit-when-you :breakout-by "hour", :filter-by "day")) + + +;;; `not` filter -- Test that we can negate the various other filter clauses +;; The majority of these tests aren't necessary since `not` automatically translates them to simpler, logically equivalent expressions +;; but I already wrote them so in this case it doesn't hurt to have a little more test coverage than we need +;; TODO - maybe it makes sense to have a separate namespace to test the Query eXpander so we don't need to run all these extra queries? + +;;; = +(expect-with-non-timeseries-dbs [99] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/= $id 1)))))) + +;;; != +(expect-with-non-timeseries-dbs [1] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/!= $id 1)))))) +;;; < +(expect-with-non-timeseries-dbs [61] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/< $id 40)))))) + +;;; > +(expect-with-non-timeseries-dbs [40] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/> $id 40)))))) + +;;; <= +(expect-with-non-timeseries-dbs [60] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/<= $id 40)))))) + +;;; >= +(expect-with-non-timeseries-dbs [39] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/>= $id 40)))))) + +;;; is-null +(expect-with-non-timeseries-dbs [100] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/is-null $id)))))) + +;;; between +(expect-with-non-timeseries-dbs [89] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/between $id 30 40)))))) + +;;; inside +(expect-with-non-timeseries-dbs [39] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/inside $latitude $longitude 40 -120 30 -110)))))) + +;;; starts-with +(expect-with-non-timeseries-dbs [80] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/starts-with $name "T")))))) + +;;; contains +(expect-with-non-timeseries-dbs [97] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/contains $name "BBQ")))))) + +;;; does-not-contain +;; This should literally be the exact same query as the one above by the time it leaves the Query eXpander, so this is more of a QX test than anything else +(expect-with-non-timeseries-dbs [97] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/does-not-contain $name "BBQ"))))) + +;;; ends-with +(expect-with-non-timeseries-dbs [87] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/ends-with $name "a")))))) + +;;; and +(expect-with-non-timeseries-dbs [98] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/and (ql/> $id 32) + (ql/contains $name "BBQ"))))))) +;;; or +(expect-with-non-timeseries-dbs [31] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/or (ql/> $id 32) + (ql/contains $name "BBQ"))))))) + +;;; nested and/or +(expect-with-non-timeseries-dbs [96] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/or (ql/and (ql/> $id 32) + (ql/< $id 35)) + (ql/contains $name "BBQ"))))))) + +;;; nested not +(expect-with-non-timeseries-dbs [3] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/not (ql/not (ql/contains $name "BBQ"))))))) + +;;; not nested inside and/or +(expect-with-non-timeseries-dbs [1] (first-row (run-query venues + (ql/aggregation (ql/count)) + (ql/filter (ql/and (ql/not (ql/> $id 32)) + (ql/contains $name "BBQ")))))) diff --git a/test/metabase/http_client.clj b/test/metabase/http_client.clj index 8c030735b9cf7e4e796b8e33eaa0ee346adb312a..87b42aa938ebc6158f6cae42ea9df1c97da5c8d2 100644 --- a/test/metabase/http_client.clj +++ b/test/metabase/http_client.clj @@ -111,7 +111,7 @@ (catch Exception e (log/error "Failed to authenticate with email:" email "and password:" password ". Does user exist?")))) -(defn build-url [url url-param-kwargs] +(defn- build-url [url url-param-kwargs] {:pre [(string? url) (or (nil? url-param-kwargs) (map? url-param-kwargs))]} diff --git a/test/metabase/test/data/dataset_definitions/test-data.edn b/test/metabase/test/data/dataset_definitions/test-data.edn index b0f796905a1443b17fd4a756b4f17854be7a440f..32a29284bcc4992f80863ba71a4862f5b379cb4c 100644 --- a/test/metabase/test/data/dataset_definitions/test-data.edn +++ b/test/metabase/test/data/dataset_definitions/test-data.edn @@ -135,106 +135,106 @@ {:field-name "category_id" :base-type :IntegerField :fk :categories}] - [["Red Medicine" 10.0646 -165.374 3 4] - ["Stout Burgers & Beers" 34.0996 -118.329 2 11] - ["The Apple Pan" 34.0406 -118.428 2 11] - ["Wurstküche" 33.9997 -118.465 2 29] - ["Brite Spot Family Restaurant" 34.0778 -118.261 2 20] - ["The 101 Coffee Shop" 34.1054 -118.324 2 20] - ["Don Day Korean Restaurant" 34.0689 -118.305 2 44] - ["25°" 34.1015 -118.342 2 11] - ["Krua Siri" 34.1018 -118.301 1 71] - ["Fred 62" 34.1046 -118.292 2 20] - ["The Gorbals" 34.0474 -118.25 2 2] - ["The Misfit Restaurant + Bar" 34.0154 -118.497 2 2] - ["Pellicola Pizzeria" 34.0451 -118.257 1 58] - ["Jones Hollywood" 34.0908 -118.346 3 7] - ["BCD Tofu House" 34.0619 -118.303 2 44] - ["Pacific Dining Car - Santa Monica" 34.0367 -118.476 4 67] - ["Ruen Pair Thai Restaurant" 34.1021 -118.306 2 71] - ["The Original Pantry" 34.0464 -118.263 2 20] - ["800 Degrees Neapolitan Pizzeria" 34.0597 -118.444 2 58] - ["Greenblatt's Delicatessen & Fine Wine Shop" 34.0981 -118.365 2 3] - ["PizzaHacker" 37.7441 -122.421 2 58] - ["Gordo Taqueria" 37.7822 -122.484 1 50] - ["Taqueria Los Coyotes" 37.765 -122.42 2 50] - ["La Tortilla" 37.7612 -122.435 1 50] - ["Garaje" 37.7818 -122.396 2 50] - ["Taqueria San Francisco" 37.753 -122.408 1 50] - ["Tout Sweet Patisserie" 37.7873 -122.407 2 13] - ["Liguria Bakery" 37.8014 -122.409 1 6] - ["20th Century Cafe" 37.775 -122.423 2 12] - ["Noe Valley Bakery" 37.7513 -122.434 2 6] - ["Bludso's BBQ" 33.8894 -118.207 2 5] - ["Boneyard Bistro" 34.1477 -118.428 3 5] - ["My Brother's Bar-B-Q" 34.167 -118.595 2 5] - ["Beachwood BBQ & Brewing" 33.7701 -118.191 2 10] - ["Smoke City Market" 34.1661 -118.448 1 5] - ["Handy Market" 34.1716 -118.335 2 3] - ["bigmista's barbecue" 34.118 -118.26 2 5] - ["Zeke's Smokehouse" 34.2053 -118.226 2 5] - ["Baby Blues BBQ" 34.0003 -118.465 2 5] - ["Dear Mom" 37.7655 -122.413 2 46] - ["Cheese Steak Shop" 37.7855 -122.44 1 18] - ["Little Star Pizza" 37.7665 -122.422 2 58] - ["Marnee Thai" 37.7634 -122.482 2 71] - ["In-N-Out Burger" 37.8078 -122.418 1 11] - ["Tu Lan Restaurant" 37.7821 -122.41 1 4] - ["Shanghai Dumpling King" 37.7317 -122.451 2 19] - ["Marlowe" 37.7767 -122.396 3 2] - ["The Residence" 37.7677 -122.429 2 7] - ["Hotel Biron" 37.7735 -122.422 3 74] - ["Two Sisters Bar & Books" 37.7765 -122.426 2 48] - ["Empress of China" 37.7949 -122.406 3 15] - ["Cole's" 34.0448 -118.25 2 7] - ["Tam O'Shanter" 34.1254 -118.264 3 18] - ["Yamashiro Hollywood" 34.1057 -118.342 3 2] - ["Dal Rae Restaurant" 33.983 -118.096 4 67] - ["Philippe the Original" 34.0597 -118.237 1 18] - ["Musso & Frank Grill" 34.1018 -118.335 3 2] - ["Taylor's Prime Steak House" 34.0579 -118.302 3 67] - ["Pacific Dining Car" 34.0555 -118.266 3 2] - ["Polo Lounge" 34.0815 -118.414 3 48] - ["Lawry's The Prime Rib" 34.0677 -118.376 4 67] - ["Hot Sauce and Panko" 37.7825 -122.476 1 64] - ["Giordano Bros." 37.765 -122.422 1 18] - ["Festa" 37.7852 -122.432 2 43] - ["Slate" 37.765 -122.418 2 48] - ["Playground" 37.7858 -122.43 2 43] - ["Mint Karaoke Lounge" 37.7702 -122.426 2 43] - ["Dimples" 37.7856 -122.43 1 43] - ["The Virgil" 34.091 -118.287 2 48] - ["The Drawing Room" 34.1037 -118.287 1 7] - ["Frolic Room" 34.1016 -118.326 1 7] - ["The Daily Pint" 34.0211 -118.466 2 7] - ["Pineapple Hill Saloon & Grill" 34.1525 -118.448 2 7] - ["Chez Jay" 34.0104 -118.493 2 2] - ["Geido" 40.6778 -73.9729 2 40] - ["Beyond Sushi" 40.7328 -73.9861 2 40] - ["Sushi Nakazawa" 40.7318 -74.0045 4 40] - ["Soto" 40.7321 -74.0006 2 40] - ["Sushi Yasuda" 40.7514 -73.9736 4 40] - ["Blue Ribbon Sushi" 40.7262 -74.0026 3 40] - ["Tanoshi Sushi & Sake Bar" 40.7677 -73.9533 4 40] - ["Bozu" 40.7129 -73.9576 2 40] - ["Sushi Yasaka" 40.7794 -73.9835 2 40] - ["Spitz Eagle Rock" 34.1411 -118.221 2 49] - ["Cha Cha Chicken" 34.0071 -118.49 1 14] - ["Yuca's Taqueria" 34.1092 -118.287 1 50] - ["The Gumbo Pot" 34.072 -118.361 2 64] - ["Kinaree Thai Bistro" 34.094 -118.344 1 71] - ["Tacos Villa Corona" 34.1177 -118.261 1 50] - ["Señor Fish" 34.0489 -118.238 2 50] - ["Manuel's Original El Tepeyac Cafe" 34.0478 -118.198 2 50] - ["Tito's Tacos" 34.0082 -118.415 1 50] - ["33 Taps" 34.1018 -118.326 2 7] - ["Ye Rustic Inn" 34.1044 -118.288 1 7] - ["Rush Street" 34.023 -118.395 2 2] - ["Busby's West" 34.0372 -118.469 2 48] - ["Barney's Beanery" 34.0908 -118.375 2 46] - ["Lucky Baldwin's Pub" 34.1454 -118.149 2 7] - ["Golden Road Brewing" 34.1505 -118.274 2 10] - ["Mohawk Bend" 34.0777 -118.265 2 46]]] + [["Red Medicine" 10.0646 -165.374 3 4] ; 1 + ["Stout Burgers & Beers" 34.0996 -118.329 2 11] ; 2 + ["The Apple Pan" 34.0406 -118.428 2 11] ; 3 + ["Wurstküche" 33.9997 -118.465 2 29] ; 4 + ["Brite Spot Family Restaurant" 34.0778 -118.261 2 20] ; 5 + ["The 101 Coffee Shop" 34.1054 -118.324 2 20] ; 6 + ["Don Day Korean Restaurant" 34.0689 -118.305 2 44] ; 7 + ["25°" 34.1015 -118.342 2 11] ; 8 + ["Krua Siri" 34.1018 -118.301 1 71] ; 9 + ["Fred 62" 34.1046 -118.292 2 20] ; 10 + ["The Gorbals" 34.0474 -118.25 2 2] ; 11 + ["The Misfit Restaurant + Bar" 34.0154 -118.497 2 2] ; 12 + ["Pellicola Pizzeria" 34.0451 -118.257 1 58] ; 13 + ["Jones Hollywood" 34.0908 -118.346 3 7] ; 14 + ["BCD Tofu House" 34.0619 -118.303 2 44] ; 15 + ["Pacific Dining Car - Santa Monica" 34.0367 -118.476 4 67] ; 16 + ["Ruen Pair Thai Restaurant" 34.1021 -118.306 2 71] ; 17 + ["The Original Pantry" 34.0464 -118.263 2 20] ; 18 + ["800 Degrees Neapolitan Pizzeria" 34.0597 -118.444 2 58] ; 19 + ["Greenblatt's Delicatessen & Fine Wine Shop" 34.0981 -118.365 2 3] ; 20 + ["PizzaHacker" 37.7441 -122.421 2 58] ; 21 + ["Gordo Taqueria" 37.7822 -122.484 1 50] ; 22 + ["Taqueria Los Coyotes" 37.765 -122.42 2 50] ; 23 + ["La Tortilla" 37.7612 -122.435 1 50] ; 24 + ["Garaje" 37.7818 -122.396 2 50] ; 25 + ["Taqueria San Francisco" 37.753 -122.408 1 50] ; 26 + ["Tout Sweet Patisserie" 37.7873 -122.407 2 13] ; 27 + ["Liguria Bakery" 37.8014 -122.409 1 6] ; 28 + ["20th Century Cafe" 37.775 -122.423 2 12] ; 29 + ["Noe Valley Bakery" 37.7513 -122.434 2 6] ; 30 + ["Bludso's BBQ" 33.8894 -118.207 2 5] ; 31 + ["Boneyard Bistro" 34.1477 -118.428 3 5] ; 32 + ["My Brother's Bar-B-Q" 34.167 -118.595 2 5] ; 33 + ["Beachwood BBQ & Brewing" 33.7701 -118.191 2 10] ; 34 + ["Smoke City Market" 34.1661 -118.448 1 5] ; 35 + ["Handy Market" 34.1716 -118.335 2 3] ; 36 + ["bigmista's barbecue" 34.118 -118.26 2 5] ; 37 + ["Zeke's Smokehouse" 34.2053 -118.226 2 5] ; 38 + ["Baby Blues BBQ" 34.0003 -118.465 2 5] ; 39 + ["Dear Mom" 37.7655 -122.413 2 46] ; 40 + ["Cheese Steak Shop" 37.7855 -122.44 1 18] ; 41 + ["Little Star Pizza" 37.7665 -122.422 2 58] ; 42 + ["Marnee Thai" 37.7634 -122.482 2 71] ; 43 + ["In-N-Out Burger" 37.8078 -122.418 1 11] ; 44 + ["Tu Lan Restaurant" 37.7821 -122.41 1 4] ; 45 + ["Shanghai Dumpling King" 37.7317 -122.451 2 19] ; 46 + ["Marlowe" 37.7767 -122.396 3 2] ; 47 + ["The Residence" 37.7677 -122.429 2 7] ; 48 + ["Hotel Biron" 37.7735 -122.422 3 74] ; 49 + ["Two Sisters Bar & Books" 37.7765 -122.426 2 48] ; 50 + ["Empress of China" 37.7949 -122.406 3 15] ; 51 + ["Cole's" 34.0448 -118.25 2 7] ; 52 + ["Tam O'Shanter" 34.1254 -118.264 3 18] ; 53 + ["Yamashiro Hollywood" 34.1057 -118.342 3 2] ; 54 + ["Dal Rae Restaurant" 33.983 -118.096 4 67] ; 55 + ["Philippe the Original" 34.0597 -118.237 1 18] ; 56 + ["Musso & Frank Grill" 34.1018 -118.335 3 2] ; 57 + ["Taylor's Prime Steak House" 34.0579 -118.302 3 67] ; 58 + ["Pacific Dining Car" 34.0555 -118.266 3 2] ; 59 + ["Polo Lounge" 34.0815 -118.414 3 48] ; 60 + ["Lawry's The Prime Rib" 34.0677 -118.376 4 67] ; 61 + ["Hot Sauce and Panko" 37.7825 -122.476 1 64] ; 62 + ["Giordano Bros." 37.765 -122.422 1 18] ; 63 + ["Festa" 37.7852 -122.432 2 43] ; 64 + ["Slate" 37.765 -122.418 2 48] ; 65 + ["Playground" 37.7858 -122.43 2 43] ; 66 + ["Mint Karaoke Lounge" 37.7702 -122.426 2 43] ; 67 + ["Dimples" 37.7856 -122.43 1 43] ; 68 + ["The Virgil" 34.091 -118.287 2 48] ; 69 + ["The Drawing Room" 34.1037 -118.287 1 7] ; 70 + ["Frolic Room" 34.1016 -118.326 1 7] ; 71 + ["The Daily Pint" 34.0211 -118.466 2 7] ; 72 + ["Pineapple Hill Saloon & Grill" 34.1525 -118.448 2 7] ; 73 + ["Chez Jay" 34.0104 -118.493 2 2] ; 74 + ["Geido" 40.6778 -73.9729 2 40] ; 75 + ["Beyond Sushi" 40.7328 -73.9861 2 40] ; 76 + ["Sushi Nakazawa" 40.7318 -74.0045 4 40] ; 77 + ["Soto" 40.7321 -74.0006 2 40] ; 78 + ["Sushi Yasuda" 40.7514 -73.9736 4 40] ; 79 + ["Blue Ribbon Sushi" 40.7262 -74.0026 3 40] ; 80 + ["Tanoshi Sushi & Sake Bar" 40.7677 -73.9533 4 40] ; 81 + ["Bozu" 40.7129 -73.9576 2 40] ; 82 + ["Sushi Yasaka" 40.7794 -73.9835 2 40] ; 83 + ["Spitz Eagle Rock" 34.1411 -118.221 2 49] ; 84 + ["Cha Cha Chicken" 34.0071 -118.49 1 14] ; 85 + ["Yuca's Taqueria" 34.1092 -118.287 1 50] ; 86 + ["The Gumbo Pot" 34.072 -118.361 2 64] ; 87 + ["Kinaree Thai Bistro" 34.094 -118.344 1 71] ; 88 + ["Tacos Villa Corona" 34.1177 -118.261 1 50] ; 89 + ["Señor Fish" 34.0489 -118.238 2 50] ; 90 + ["Manuel's Original El Tepeyac Cafe" 34.0478 -118.198 2 50] ; 91 + ["Tito's Tacos" 34.0082 -118.415 1 50] ; 92 + ["33 Taps" 34.1018 -118.326 2 7] ; 93 + ["Ye Rustic Inn" 34.1044 -118.288 1 7] ; 94 + ["Rush Street" 34.023 -118.395 2 2] ; 95 + ["Busby's West" 34.0372 -118.469 2 48] ; 96 + ["Barney's Beanery" 34.0908 -118.375 2 46] ; 97 + ["Lucky Baldwin's Pub" 34.1454 -118.149 2 7] ; 98 + ["Golden Road Brewing" 34.1505 -118.274 2 10] ; 99 + ["Mohawk Bend" 34.0777 -118.265 2 46]]] ; 100 ["checkins" [{:field-name "user_id" :base-type :IntegerField :fk :users} diff --git a/test/metabase/test/data/datasets.clj b/test/metabase/test/data/datasets.clj index 970b639a21887d64973755b9b92b893873fa766e..adae579926cc640e04c006fd18ffaad61050db9b 100644 --- a/test/metabase/test/data/datasets.clj +++ b/test/metabase/test/data/datasets.clj @@ -103,8 +103,8 @@ [engines expected actual] ;; Make functions to get expected/actual so the code is only compiled one time instead of for every single driver ;; speeds up loading of metabase.driver.query-processor-test significantly - (let [e (gensym "expected-") - a (gensym "actual-")] + (let [e (symbol (str "expected-" (hash expected))) + a (symbol (str "actual-" (hash actual)))] `(let [~e (fn [] ~expected) ~a (fn [] ~actual)] ~@(for [engine (eval engines)] diff --git a/test/metabase/test/data/druid.clj b/test/metabase/test/data/druid.clj index 965279125550f71ec6dd705fc0f55ed9e92ba49e..bb3c4ab201ef3a5769c56ce298f8d4589261756c 100644 --- a/test/metabase/test/data/druid.clj +++ b/test/metabase/test/data/druid.clj @@ -1,19 +1,74 @@ (ns metabase.test.data.druid - (:require [cheshire.core :as json] - [clojure.java.io :as io] + (:require [clojure.java.io :as io] + [cheshire.core :as json] + [environ.core :refer [env]] [metabase.driver.druid :as druid] - (metabase.test.data dataset-definitions + (metabase.test.data [dataset-definitions :as defs] [datasets :as datasets] [interface :as i]) [metabase.test.util :refer [resolve-private-fns]] [metabase.util :as u]) (:import metabase.driver.druid.DruidDriver)) -(def ^:private ^:const temp-dir (System/getProperty "java.io.tmpdir")) -(def ^:private ^:const source-filename "checkins.json") +(defn- database->connection-details [& _] + {:host (or (env :mb-druid-host) + (throw (Exception. "In order to test Druid, you must specify `MB_DRUID_HOST`."))) + :port (Integer/parseInt (or (env :mb-druid-port) + (throw (Exception. "In order to test Druid, you must specify `MB_DRUID_PORT`."))))}) + +(extend DruidDriver + i/IDatasetLoader + (merge i/IDatasetLoaderDefaultsMixin + {:engine (constantly :druid) + :database->connection-details database->connection-details + :create-db! (constantly nil) + :destroy-db! (constantly nil)})) + + + +;;; Setting Up a Server w/ Druid Test Data + +;; Unfortunately the process of loading test data onto an external server for CI purposes is a little involved. Before testing against Druid, you'll need to perform the following steps: +;; For EC2 instances, make sure to expose ports `8082` & `8090` for Druid while loading data. Once data has finished loading, you only need to expose port `8082`. +;; +;; 1. Setup Zookeeper +;; 1A. Download & extract Zookeeper from `http://zookeeper.apache.org/releases.html#download` +;; 1B. Create `zookeeper/conf/zoo.cfg` -- see the Getting Started Guide: `http://zookeeper.apache.org/doc/r3.4.6/zookeeperStarted.html` +;; 1C. `zookeeper/bin/zkServer.sh start` +;; 1D. `zookeeper/bin/zkServer.sh status` (to make sure it started correctly) +;; 2. Setup Druid +;; 2A. Download & extract Druid from `http://druid.io/downloads.html` +;; 2B. `cp druid/run_druid_server.sh druid/run_historical.sh` and bump the `-Xmx` setting to `6g` or so +;; 2C. `cd druid && ./run_druid_server.sh coordinator` +;; 2D. `cd druid && ./run_druid_server.sh broker` +;; 2E. `cd druid && ./run_historical.sh historical` +;; 2E. `cd druid && ./run_druid_server.sh overlord` +;; 3. Generate flattened test data file. Optionally pick a <filename> +;; 3A. From this namespace in the REPL, run `(generate-json-for-batch-ingestion <filename>)` +;; 3B. `scp` or otherwise upload this file to the box running druid (if applicable) +;; 4. Launch Druid Indexing Task +;; 4A. Run the indexing task on the remote instance. +;; +;; (run-indexing-task <remote-host> :base-dir <dir-where-you-uploaded-file>, :filename <file>) +;; e.g. +;; (run-indexing-task "http://ec2-52-90-109-199.compute-1.amazonaws.com", :base-dir "/home/ec2-user", :filename "checkins.json") +;; +;; The task will keep you apprised of its progress until it completes (takes 1-2 minutes) +;; 4B. Keep an eye on `<host>:8082/druid/v2/datasources`. (e.g. "http://ec2-52-90-109-199.compute-1.amazonaws.com:8082/druid/v2/datasources") +;; This endpoint will return an empty array until the broker knows about the newly ingested segments. When it returns an array with the string `"checkins"` you're ready to +;; run the tests. +;; 4C. Kill the `overlord` process once the data has finished loading. +;; 5. Running Tests +;; 5A. You can run tests like `ENGINES=druid MB_DRUID_PORT=8082 MB_DRUID_HOST=http://ec2-52-90-109-199.compute-1.amazonaws.com lein test` + +(def ^:private ^:const default-filename "Default filename for batched ingestion data file." + "checkins.json") + + +;;; Generating Data File (defn- flattened-test-data [] - (let [dbdef (i/flatten-dbdef metabase.test.data.dataset-definitions/test-data "checkins") + (let [dbdef (i/flatten-dbdef defs/test-data "checkins") tabledef (first (:table-definitions dbdef))] (->> (:rows tabledef) (map (partial zipmap (map :field-name (:field-definitions tabledef)))) @@ -29,7 +84,21 @@ (json/generate-stream row writer) (.append writer \newline))))) -(def ^:private ^:const indexing-task +(defn- generate-json-for-batch-ingestion + "Generate the file to be used for a batched data ingestion for Druid." + ([] + (generate-json-for-batch-ingestion default-filename)) + ([filename] + (write-dbdef-to-json (flattened-test-data) filename))) + + +;;; Running Indexing Task + +(defn- indexing-task + "Create a batched ingestion task dictionary." + [{:keys [base-dir filename] + :or {base-dir "/home/ec2-user" + filename default-filename}}] {:type :index :spec {:dataSchema {:dataSource "checkins" :parser {:type :string @@ -53,18 +122,20 @@ :intervals ["2000/2016"]}} :ioConfig {:type :index :firehose {:type :local - :baseDir temp-dir - :filter source-filename}}}}) + :baseDir base-dir + :filter filename}}}}) -(def ^:private ^:const indexer-endpoint "http://localhost:8090/druid/indexer/v1/task") (def ^:private ^:const indexer-timeout-seconds "Maximum number of seconds we should wait for the indexing task to finish before deciding it's failed." - 120) + 180) (resolve-private-fns metabase.driver.druid GET POST) -(defn- run-indexing-task [] - (let [{:keys [task]} (POST indexer-endpoint, :body indexing-task) +(defn- run-indexing-task + "Run a batched ingestion task on HOST." + [host & {:as indexing-task-args}] + (let [indexer-endpoint (str host ":8090/druid/indexer/v1/task") + {:keys [task]} (POST indexer-endpoint, :body (indexing-task indexing-task-args)) status-url (str indexer-endpoint "/" task "/status")] (println "STATUS URL: " (str indexer-endpoint "/" task "/status")) (loop [remaining-seconds indexer-timeout-seconds] @@ -77,43 +148,3 @@ (throw (Exception. (str "Indexing task failed:\n" (u/pprint-to-str status))))) (Thread/sleep 1000) (recur (dec remaining-seconds))))))) - -(defn- setup-druid-test-data* [] - (println (u/format-color 'blue "Loading druid test data...")) - (write-dbdef-to-json (flattened-test-data) (str temp-dir "/" source-filename)) - (run-indexing-task)) - -#_(defn- setup-druid-test-data - {:expectations-options :before-run} - [] - (datasets/when-testing-engine :druid - (setup-druid-test-data*))) - -;; TODO - needs to wait until http://localhost:8082/druid/v2/datasources/checkins?interval=-5000/5000 returns data -#_{:dimensions [:venue_name - :venue_category_name - :user_password - :venue_longitude - :user_name - :id - :venue_latitude - :user_last_login - :venue_price] - :metrics [:count]} - - -(defn- database->connection-details [this context dbdef] - {:host "http://localhost" - :port 8082}) - -(extend DruidDriver - i/IDatasetLoader - (merge i/IDatasetLoaderDefaultsMixin - {:engine (constantly :druid) - :database->connection-details database->connection-details - :create-db! (constantly "nil") - :destroy-db! (constantly nil)})) - - -;; TODO - don't log druid query during sync -;; TODO - make `:paging` a feature? diff --git a/test/metabase/test_setup.clj b/test/metabase/test_setup.clj index 483ad88b58f38bc49e2e16e2a67531f686b280cc..7e7c4e92b18b219f995399b19ed91e3447e9e39c 100644 --- a/test/metabase/test_setup.clj +++ b/test/metabase/test_setup.clj @@ -1,8 +1,10 @@ (ns metabase.test-setup "Functions that run before + after unit tests (setup DB, start web server, load test data)." - (:require [clojure.java.io :as io] + (:require (clojure.java [classpath :as classpath] + [io :as io]) [clojure.set :as set] [clojure.tools.logging :as log] + [clojure.tools.namespace.find :as ns-find] [expectations :refer :all] (metabase [core :as core] [db :as db] @@ -94,3 +96,52 @@ [] (log/info "Shutting down Metabase unit test runner") (core/stop-jetty)) + + + +;; Check that we're on target for every public var in Metabase having a docstring +;; This will abort unit tests if we don't hit our target +(defn- expected-docstr-percentage-for-day-of-year + "Calculate the percentage of public vars we expect to have a docstring by the current date in time. This ranges from 80% for the end of January to 100% half way through the year." + ([] + (expected-docstr-percentage-for-day-of-year (u/date-extract :day-of-year))) + ([doy] + (let [start-day 30 + start-percent 80.0 + target-doy-for-100-percent 180 + remaining-percent (- 100.0 start-percent) + remaining-days (- target-doy-for-100-percent start-day)] + (Math/min (+ start-percent (* (/ remaining-percent remaining-days) + (- doy start-day))) + 100.0)))) + +(defn- does-metabase-need-more-dox? [] + (let [symb->has-doc? (into {} (for [ns (ns-find/find-namespaces (classpath/classpath)) + :let [nm (try (str (ns-name ns)) + (catch Throwable _))] + :when nm + :when (re-find #"^metabase" nm) + :when (not (re-find #"test" nm)) + [symb varr] (ns-publics ns)] + {(symbol (str nm "/" symb)) (boolean (:doc (meta varr)))})) + vs (vals symb->has-doc?) + total (count vs) + num-with-dox (count (filter identity vs)) + percentage (float (* (/ num-with-dox total) + 100.0)) + expected-percentage (expected-docstr-percentage-for-day-of-year) + needs-more-dox? (< percentage expected-percentage)] + (println (u/format-color (if needs-more-dox? 'red 'green) + "%.1f%% of Metabase public vars have docstrings. (%d/%d) Expected for today: %.1f%%" percentage num-with-dox total expected-percentage)) + (println (u/format-color 'cyan "Why don't you go write a docstr for %s?" (first (shuffle (for [[symb has-doc?] symb->has-doc? + :when (not has-doc?)] + symb))))) + needs-more-dox?)) + + +(defn- throw-if-metabase-doesnt-have-enough-docstrings! + {:expectations-options :before-run} + [] + (when (does-metabase-need-more-dox?) + (println (u/format-color 'red "Metabase needs more docstrings! Go write some more (or make some vars ^:private) before proceeding.")) + (System/exit -1)))