diff --git a/e2e/test/scenarios/visualizations-charts/line_chart.cy.spec.js b/e2e/test/scenarios/visualizations-charts/line_chart.cy.spec.js index 8cfc2a57da192be6fd48707c2b86ffd68c8f7614..73512e2538054ea1ea7a0f01bfdcb3fbbf6b6cbb 100644 --- a/e2e/test/scenarios/visualizations-charts/line_chart.cy.spec.js +++ b/e2e/test/scenarios/visualizations-charts/line_chart.cy.spec.js @@ -578,35 +578,7 @@ describe("scenarios > visualizations > line chart", () => { }); }); - it( - "should apply filters to the series selecting area range", - { tags: "@flaky" }, - () => { - cy.viewport(1280, 800); - - visitQuestionAdhoc({ - dataset_query: testQuery, - display: "line", - }); - - cy.get(".Visualization") - .trigger("mousedown", 100, 200) - .trigger("mousemove", 230, 200) - .trigger("mouseup", 230, 200); - - cy.wait("@dataset"); - - cy.findByTestId("filter-pill").should("contain.text", "Created At is"); - - const X_AXIS_VALUE = "June 2022"; - cy.get(".CardVisualization").within(() => { - cy.get(".x-axis-label").should("have.text", "Created At"); - cy.findByText(X_AXIS_VALUE); - }); - }, - ); - - it("should apply filters to the series selecting area range when axis is a number", () => { + it("should apply brush filters to the series selecting area range when axis is a number", () => { const testQuery = { type: "query", query: { diff --git a/e2e/test/scenarios/visualizations-charts/maps.cy.spec.js b/e2e/test/scenarios/visualizations-charts/maps.cy.spec.js index 1fb54d72dd6d59f622bec22537c2f4d2f984c86d..4231a63739d6ceda63d8cf507fea5329867a5c0f 100644 --- a/e2e/test/scenarios/visualizations-charts/maps.cy.spec.js +++ b/e2e/test/scenarios/visualizations-charts/maps.cy.spec.js @@ -257,6 +257,7 @@ describe("scenarios > visualizations > maps", () => { cy.wait("@dataset"); + cy.get(".CardVisualization").should("exist"); // selecting area at the map provides different filter values, so the simplified assertion is used cy.findByTestId("filter-pill").should("have.length", 1); }); diff --git a/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js index 00e2ecdf9c604bf2203e76db33a5a72ca5bee38b..2b78b30928c1f56b6608a64b65b74ba9c90f3e66 100644 --- a/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js +++ b/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js @@ -29,7 +29,7 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => { it("should allow brush date filter", { tags: "@flaky" }, () => { cy.createQuestion( { - name: "Brush Date Filter", + name: "Brush Date Temporal Filter", query: { "source-table": ORDERS_ID, aggregation: [["count"]], @@ -80,9 +80,6 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => { ["month", "month-of-year"].forEach(granularity => { it(`brush filter should work post-aggregation for ${granularity} granularity (metabase#18011)`, () => { - // TODO: Remove this line when the issue is fixed! - cy.skipOn(granularity === "month-of-year"); - cy.intercept("POST", "/api/dataset").as("dataset"); const questionDetails = { diff --git a/frontend/src/metabase-lib/queries/utils/actions.js b/frontend/src/metabase-lib/queries/utils/actions.js deleted file mode 100644 index 2376c5ff7d8037c7d5a67029cc4a8b5a6365457c..0000000000000000000000000000000000000000 --- a/frontend/src/metabase-lib/queries/utils/actions.js +++ /dev/null @@ -1,127 +0,0 @@ -// eslint-disable-next-line no-restricted-imports -- deprecated usage -import moment from "moment-timezone"; - -import { fieldRefForColumn } from "metabase-lib/queries/utils/dataset"; -import { FieldDimension } from "metabase-lib/Dimension"; - -// STRUCTURED QUERY UTILITIES - -const fieldRefWithTemporalUnit = (mbqlClause, unit) => { - const dimension = FieldDimension.parseMBQLOrWarn(mbqlClause); - if (dimension) { - return dimension.withTemporalUnit(unit).mbql(); - } - return mbqlClause; -}; - -function addOrUpdateFilter(query, newFilter) { - // replace existing filter, if it exists - for (const filter of query.filters()) { - const dimension = filter.dimension(); - if (dimension && dimension.isSameBaseDimension(newFilter[1])) { - return filter.replace(newFilter); - } - } - // otherwise add a new filter - return query.filter(newFilter); -} - -// min number of points when switching units -const MIN_INTERVALS = 4; - -const UNITS = ["minute", "hour", "day", "week", "month", "quarter", "year"]; -const getNextUnit = unit => { - return UNITS[Math.max(0, UNITS.indexOf(unit) - 1)]; -}; - -function addOrUpdateBreakout(query, newBreakout) { - // replace existing breakout, if it exists - for (const breakout of query.breakouts()) { - if (breakout.dimension().isSameBaseDimension(newBreakout)) { - return breakout.replace(newBreakout); - } - } - // otherwise add a new breakout - return query.breakout(newBreakout); -} - -export function updateDateTimeFilter(query, column, start, end) { - const fieldRef = fieldRefForColumn(column); - start = moment(start); - end = moment(end); - if (column.unit) { - // start with the existing breakout unit - let unit = column.unit; - - // clamp range to unit to ensure we select exactly what's represented by the dots/bars - start = start.add(1, unit).startOf(unit); - end = end.endOf(unit); - - // find the largest unit with at least MIN_INTERVALS - while ( - unit !== getNextUnit(unit) && - end.diff(start, unit) < MIN_INTERVALS - ) { - unit = getNextUnit(unit); - } - - // update the breakout - query = addOrUpdateBreakout( - query, - fieldRefWithTemporalUnit(fieldRef, unit), - ); - - // round to start of the original unit - start = start.startOf(column.unit); - end = end.startOf(column.unit); - - if (start.isAfter(end)) { - return query; - } - if (start.isSame(end, column.unit)) { - // is the start and end are the same (in whatever the original unit was) then just do an "=" - return addOrUpdateFilter(query, [ - "=", - fieldRefWithTemporalUnit(fieldRef, column.unit), - start.format(), - ]); - } else { - // otherwise do a between - return addOrUpdateFilter(query, [ - "between", - fieldRefWithTemporalUnit(fieldRef, column.unit), - start.format(), - end.format(), - ]); - } - } else { - return addOrUpdateFilter(query, [ - "between", - fieldRef, - start.format(), - end.format(), - ]); - } -} - -export function updateLatLonFilter( - query, - latitudeColumn, - longitudeColumn, - bounds, -) { - return addOrUpdateFilter(query, [ - "inside", - fieldRefForColumn(latitudeColumn), - fieldRefForColumn(longitudeColumn), - bounds.getNorth(), - bounds.getWest(), - bounds.getSouth(), - bounds.getEast(), - ]); -} - -export function updateNumericFilter(query, column, start, end) { - const fieldRef = fieldRefForColumn(column); - return addOrUpdateFilter(query, ["between", fieldRef, start, end]); -} diff --git a/frontend/src/metabase/visualizations/components/LeafletMap.jsx b/frontend/src/metabase/visualizations/components/LeafletMap.jsx index 959b582fe739324cbcf5c5c5b7bc50e5fdd39939..40c65bdb52e5ced0fb6d826a0c983fac439a7b3b 100644 --- a/frontend/src/metabase/visualizations/components/LeafletMap.jsx +++ b/frontend/src/metabase/visualizations/components/LeafletMap.jsx @@ -10,7 +10,7 @@ import "leaflet-draw"; import _ from "underscore"; import MetabaseSettings from "metabase/lib/settings"; -import { updateLatLonFilter } from "metabase-lib/queries/utils/actions"; +import * as Lib from "metabase-lib"; import Question from "metabase-lib/Question"; export default class LeafletMap extends Component { @@ -147,6 +147,7 @@ export default class LeafletMap extends Component { ], settings, onChangeCardAndRun, + metadata, } = this.props; const latitudeColumn = _.findWhere(cols, { @@ -156,16 +157,26 @@ export default class LeafletMap extends Component { name: settings["map.longitude_column"], }); - const question = new Question(card); + const question = new Question(card, metadata); if (question.isStructured()) { - const nextCard = updateLatLonFilter( - question.legacyQuery({ useStructuredQuery: true }), + const query = question.query(); + const stageIndex = -1; + const filterBounds = { + north: bounds.getNorth(), + south: bounds.getSouth(), + west: bounds.getWest(), + east: bounds.getEast(), + }; + const updatedQuery = Lib.updateLatLonFilter( + query, + stageIndex, latitudeColumn, longitudeColumn, - bounds, - ) - .question() - .card(); + filterBounds, + ); + const updatedQuestion = question.setQuery(updatedQuery); + const nextCard = updatedQuestion.card(); + onChangeCardAndRun({ nextCard }); } diff --git a/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx b/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx index 73577e2e62e66857b25676f21556879dff682d3b..3cc4f8af0503b90c4f01dbea58a3f62b9a6b7680 100644 --- a/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx +++ b/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx @@ -89,19 +89,6 @@ export default class LineAreaBarChart extends Component { } } - static propTypes = { - card: PropTypes.object.isRequired, - series: PropTypes.array.isRequired, - settings: PropTypes.object.isRequired, - actionButtons: PropTypes.node, - showTitle: PropTypes.bool, - isDashboard: PropTypes.bool, - headerIcon: PropTypes.object, - width: PropTypes.number, - }; - - static defaultProps = {}; - getHoverClasses() { const { hovered } = this.props; if (hovered && hovered.index != null) { @@ -321,6 +308,17 @@ export default class LineAreaBarChart extends Component { } } +LineAreaBarChart.propTypes = { + card: PropTypes.object.isRequired, + series: PropTypes.array.isRequired, + settings: PropTypes.object.isRequired, + actionButtons: PropTypes.node, + showTitle: PropTypes.bool, + isDashboard: PropTypes.bool, + headerIcon: PropTypes.object, + width: PropTypes.number, +}; + function transformSingleSeries(s, series, seriesIndex) { const { card, data } = s; diff --git a/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js b/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js index 9cea000f4d90b994219986a7d3bcc9beef3e106d..712016402dd20090028ea1187c58aab718de24b8 100644 --- a/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js +++ b/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js @@ -7,10 +7,7 @@ import { t } from "ttag"; import { lighten } from "metabase/lib/colors"; import { keyForSingleSeries } from "metabase/visualizations/lib/settings/series"; -import { - updateDateTimeFilter, - updateNumericFilter, -} from "metabase-lib/queries/utils/actions"; +import * as Lib from "metabase-lib"; import { isStructured } from "metabase-lib/queries/utils/card"; import Question from "metabase-lib/Question"; @@ -351,7 +348,7 @@ function getYAxisProps(props, yExtents, datas) { /// make the `onBrushChange()` and `onBrushEnd()` functions we'll use later, as well as an `isBrushing()` function to check /// current status. -function makeBrushChangeFunctions({ series, onChangeCardAndRun }) { +function makeBrushChangeFunctions({ series, onChangeCardAndRun, metadata }) { let _isBrushing = false; const isBrushing = () => _isBrushing; @@ -362,25 +359,44 @@ function makeBrushChangeFunctions({ series, onChangeCardAndRun }) { function onBrushEnd(range) { _isBrushing = false; + if (range) { const column = series[0].data.cols[0]; const card = series[0].card; - const query = new Question(card).legacyQuery({ - useStructuredQuery: true, - }); + const question = new Question(card, metadata); + const query = question.query(); + const stageIndex = -1; + const [start, end] = range; + if (isDimensionTimeseries(series)) { + const nextQuery = Lib.updateTemporalFilter( + query, + stageIndex, + column, + new Date(start).toISOString(), + new Date(end).toISOString(), + ); + const updatedQuestion = question.setQuery(nextQuery); + const nextCard = updatedQuestion.card(); + onChangeCardAndRun({ - nextCard: updateDateTimeFilter(query, column, start, end) - .question() - .card(), + nextCard, previousCard: card, }); } else { + const nextQuery = Lib.updateNumericFilter( + query, + stageIndex, + column, + start, + end, + ); + const updatedQuestion = question.setQuery(nextQuery); + const nextCard = updatedQuestion.card(); + onChangeCardAndRun({ - nextCard: updateNumericFilter(query, column, start, end) - .question() - .card(), + nextCard, previousCard: card, }); } diff --git a/src/metabase/lib/js.cljs b/src/metabase/lib/js.cljs index 53ae9a5dd5f3056d5d307c0083953df1b5d821f3..51f0c8129b8b8ed6cbb2cc83fef043dcaf697df7 100644 --- a/src/metabase/lib/js.cljs +++ b/src/metabase/lib/js.cljs @@ -1234,15 +1234,15 @@ (defn ^:export update-numeric-filter "Add or update a filter against `numeric-column`." - [a-query numeric-column stage-number start end] + [a-query stage-number numeric-column start end] (let [numeric-column (legacy-column->metadata a-query stage-number numeric-column)] - (lib.core/update-numeric-filter a-query numeric-column stage-number start end))) + (lib.core/update-numeric-filter a-query stage-number numeric-column start end))) (defn ^:export update-temporal-filter "Add or update a filter against `temporal-column`. Modify the temporal unit for any breakouts." - [a-query temporal-column stage-number start end] + [a-query stage-number temporal-column start end] (let [temporal-column (legacy-column->metadata a-query stage-number temporal-column)] - (lib.core/update-temporal-filter a-query temporal-column stage-number start end))) + (lib.core/update-temporal-filter a-query stage-number temporal-column start end))) (defn ^:export valid-filter-for? "Given two CLJS `:metadata/columns` returns true if `src-column` is a valid source to use for filtering `dst-column`."