diff --git a/e2e/test/scenarios/filters-reproductions/dashboard-filters-reproductions.cy.spec.js b/e2e/test/scenarios/filters-reproductions/dashboard-filters-reproductions.cy.spec.js index b9053b7948dafb342e557ae643cc042c8caf2f88..916a7a6ed231696aa7d35209afb6ecb9c9b1a319 100644 --- a/e2e/test/scenarios/filters-reproductions/dashboard-filters-reproductions.cy.spec.js +++ b/e2e/test/scenarios/filters-reproductions/dashboard-filters-reproductions.cy.spec.js @@ -34,6 +34,7 @@ import { setModelMetadata, sidebar, tableHeaderClick, + tableInteractive, undoToast, updateDashboardCards, visitDashboard, @@ -240,7 +241,7 @@ describe("issue 8030 + 32444", () => { }); }); -describe("issue 12720", () => { +describe("issue 12720, issue 47172", () => { function clickThrough(title) { visitDashboard(ORDERS_DASHBOARD_ID); cy.findAllByTestId("dashcard-container").contains(title).click(); @@ -291,7 +292,7 @@ describe("issue 12720", () => { { card_id: SQL_ID, row: 0, - col: 6, // making sure it doesn't overlap the existing card + col: 8, // making sure it doesn't overlap the existing card size_x: 7, size_y: 5, parameter_mappings: [ @@ -330,6 +331,33 @@ describe("issue 12720", () => { clickThrough("12720_SQL"); clickThrough("Orders"); }); + + it("should apply the specific (before|after) filter on a native question with field filter (metabase#47172)", () => { + visitDashboard(ORDERS_DASHBOARD_ID); + + getDashboardCard(1).within(() => { + cy.findByTestId("TableFooter").should("exist"); + cy.findByText("There was a problem displaying this chart.").should( + "not.exist", + ); + + cy.log("Drill down to the question"); + cy.intercept("POST", "/api/card/*/query").as("cardQuery"); + cy.findByText(questionDetails.name).click(); + }); + + cy.location("search").should("eq", `?filter=${dashboardFilter.default}`); + cy.wait("@cardQuery"); + tableInteractive().should("be.visible").and("contain", "97.44"); + cy.findByTestId("question-row-count").should( + "not.have.text", + "Showing 0 rows", + ); + cy.findByTestId("question-row-count").should( + "have.text", + "Showing 1,980 rows", + ); + }); }); describe("issue 12985 > dashboard filter dropdown/search", () => { diff --git a/src/metabase/driver/common/parameters/dates.clj b/src/metabase/driver/common/parameters/dates.clj index 9a2f0087189225834d0267325e29c64fc2f3dd0f..01e2fe90fd6ff7e4ea15d6a0a219def91d636aa5 100644 --- a/src/metabase/driver/common/parameters/dates.clj +++ b/src/metabase/driver/common/parameters/dates.clj @@ -506,6 +506,13 @@ {:start date-str :end date-str})) +(defn- maybe-adjust-open-range + [{:keys [start end] :as range} unit-fn] + (assert (some some? [start end])) + (cond (and start end) range + start (update range :start t/+ (unit-fn 1)) + end (update range :end t/- (unit-fn 1)))) + (mu/defn date-str->datetime-range :- DateStringRange "Generate range from `date-range-str`. @@ -525,7 +532,8 @@ (catch Throwable _ (fallback-raw-range raw-date-str)))] (-> (update-vals range-raw date-str->qp-aware-offset-dt) - (update :end exclusive-datetime-range-end (date-str->unit-fn (:end range-raw))) + (m/update-existing :end exclusive-datetime-range-end (date-str->unit-fn (:end range-raw))) + (maybe-adjust-open-range (date-str->unit-fn ((some-fn :start :end) range-raw))) format-date-range))) (mu/defn date-string->filter :- mbql.s/Filter diff --git a/src/metabase/driver/sql/parameters/substitution.clj b/src/metabase/driver/sql/parameters/substitution.clj index 4f35c1bfffb0bb87cc872a8d5eebc95e3fc8e651..5b4185fe01532bff36294b770af18ba2175ae33d 100644 --- a/src/metabase/driver/sql/parameters/substitution.clj +++ b/src/metabase/driver/sql/parameters/substitution.clj @@ -215,12 +215,19 @@ (defmethod ->replacement-snippet-info [:sql DateTimeRange] [driver {:keys [start end]} & [field-identifier]] - (let [[start end] (map (fn [s] - (->prepared-substitution driver (maybe-parse-temporal-literal s))) - [start end])] - {:replacement-snippet (format "%s >= %s AND %s < %s" - field-identifier (:sql-string start) field-identifier (:sql-string end)) - :prepared-statement-args (concat (:param-values start) (:param-values end))})) + (let [[start end] (map (fn [s] + (when s + (->prepared-substitution driver (maybe-parse-temporal-literal s)))) + [start end]) + start-expr-native (when start + (format "%s >= %s" field-identifier (:sql-string start))) + end-expr-native (when end + (format "%s < %s" field-identifier (:sql-string end)))] + {:replacement-snippet (str/join " AND " (remove nil? [start-expr-native end-expr-native])) + :prepared-statement-args (into [] + (comp (keep :param-values) + cat) + [start end])})) ;;; ------------------------------------- Field Filter replacement snippet info -------------------------------------- diff --git a/test/metabase/driver/sql/parameters/substitute_test.clj b/test/metabase/driver/sql/parameters/substitute_test.clj index 0744de87d8184a90b3c26a557a5a4a592d060cb6..65a3e70b962e53a4906b3164af7c078f22d53bb5 100644 --- a/test/metabase/driver/sql/parameters/substitute_test.clj +++ b/test/metabase/driver/sql/parameters/substitute_test.clj @@ -552,7 +552,7 @@ expand* (dissoc :template-tags))))) -(deftest expand-field-filters-test +(deftest expand-field-filters-for-date-field-test (mt/with-temporary-setting-values [start-of-week :sunday] (testing "dimension (date/single)" (is (= {:query "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" = ?;" @@ -645,6 +645,146 @@ "SELECT * FROM ORDERS WHERE TOTAL > 100 [[AND {{created}} #]] AND CREATED_AT < now()" nil)))))) +(defn- expand-with-field-filter-param-on-datetime-field + ([field-filter-param] + (expand-with-field-filter-param-on-datetime-field "SELECT * FROM orders WHERE {{date}};" field-filter-param)) + + ([sql field-filter-param] + ;; TIMEZONE FIXME + (mt/with-clock (t/mock-clock #t "2016-06-07T12:00-00:00" (t/zone-id "UTC")) + (-> {:native {:query + sql + :template-tags {"date" {:name "date" + :display-name "Created At" + :type :dimension + :widget-type :date/all-options + :dimension [:field (meta/id :orders :created-at) nil]}}} + :parameters (when field-filter-param + [(merge {:target [:dimension [:template-tag "date"]]} + field-filter-param)])} + expand* + (dissoc :template-tags))))) + +;;;; The following test is [[expand-field-filters-for-date-field-test]] adjusted to datetime field. +(deftest expand-field-filters-for-datetime-field-test + (mt/with-temporary-setting-values [start-of-week :sunday] + (testing "dimension (date/all-options) (`on` filter with with time)" + (is (= {:query "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2024-08-20T10:20Z[UTC]" + #t "2024-08-20T10:21Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/all-options, :value "2024-08-20T10:20:00"})))) + (testing "dimension (date/all-options) (`before` filter with with time)" + (is (= {:query "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2024-08-20T10:20Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/all-options, :value "~2024-08-20T10:20:00"})))) + (testing "dimension (date/all-options) (`after` filter with with time)" + (is (= {:query "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ?;", + :params [#t "2024-08-20T10:21Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/all-options, :value "2024-08-20T10:20:00~"})))) + (testing "dimension (date/single)" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-07-01T00:00Z[UTC]" + #t "2016-07-02T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/single, :value "2016-07-01"})))) + (testing "dimension (date/range)" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-07-01T00:00Z[UTC]" + #t "2016-08-02T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/range, :value "2016-07-01~2016-08-01"})))) + (testing "dimension (date/month-year)" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-07-01T00:00Z[UTC]" + #t "2016-08-01T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/month-year, :value "2016-07"})))) + (testing "dimension (date/quarter-year)" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-01-01T00:00Z[UTC]" + #t "2016-04-01T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/quarter-year, :value "Q1-2016"})))) + (testing "dimension (date/all-options, before)" + (is (= {:query "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", :params [#t "2016-07-01T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/all-options, :value "~2016-07-01"})))) + (testing "dimension (date/all-options, after)" + (is (= {:query "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ?;", :params [#t "2016-07-02T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/all-options, :value "2016-07-01~"})))) + (testing "relative date — 'yesterday'" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-06-06T00:00Z[UTC]" + #t "2016-06-07T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/range, :value "yesterday"})))) + (testing "relative date — 'past7days'" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-05-31T00:00Z[UTC]" + #t "2016-06-07T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/range, :value "past7days"})))) + (testing "relative date — 'past30days'" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-05-08T00:00Z[UTC]" + #t "2016-06-07T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/range, :value "past30days"})))) + (testing "relative date — 'thisweek'" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-06-05T00:00Z[UTC]" + #t "2016-06-12T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/range, :value "thisweek"})))) + (testing "relative date — 'thismonth'" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-06-01T00:00Z[UTC]" + #t "2016-07-01T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/range, :value "thismonth"})))) + (testing "relative date — 'thisyear'" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-01-01T00:00Z[UTC]" + #t "2017-01-01T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/range, :value "thisyear"})))) + (testing "relative date — 'lastweek'" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-05-29T00:00Z[UTC]" + #t "2016-06-05T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/range, :value "lastweek"})))) + (testing "relative date — 'lastmonth'" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2016-05-01T00:00Z[UTC]" + #t "2016-06-01T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/range, :value "lastmonth"})))) + (testing "relative date — 'lastyear'" + (is (= {:query + "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ? AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?;", + :params [#t "2015-01-01T00:00Z[UTC]" + #t "2016-01-01T00:00Z[UTC]"]} + (expand-with-field-filter-param-on-datetime-field {:type :date/range, :value "lastyear"})))) + (testing "dimension with no value — just replace with an always true clause (e.g. 'WHERE 1 = 1')" + (is (= {:query "SELECT * FROM orders WHERE 1 = 1;" + :params []} + (expand-with-field-filter-param-on-datetime-field nil)))) + (testing "dimension — number — should get parsed to Number" + (is (= {:query "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" = 100;" + :params []} + (expand-with-field-filter-param-on-datetime-field {:type :number, :value "100"})))) + (testing "dimension — text" + (is (= {:query "SELECT * FROM orders WHERE \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" = ?;" + :params ["100"]} + (expand-with-field-filter-param-on-datetime-field {:type :text, :value "100"})))) + (testing (str "*OPTIONAL* Field Filter params should not get replaced with 1 = 1 if the param is not present " + "(#5541, #9489). *Optional params should be emitted entirely.") + (is (= {:query "SELECT * FROM ORDERS WHERE TOTAL > 100 AND CREATED_AT < now()" + :params []} + (expand-with-field-filter-param-on-datetime-field + "SELECT * FROM ORDERS WHERE TOTAL > 100 [[AND {{created}} #]] AND CREATED_AT < now()" + nil)))))) + (deftest ^:parallel expand-exclude-field-filter-test (mt/with-driver :h2 (testing "exclude date parts"