From 976f96bbcbfb1ca52b4b8ea771abc76d15f583ad Mon Sep 17 00:00:00 2001
From: Cal Herries <39073188+calherries@users.noreply.github.com>
Date: Thu, 5 Oct 2023 10:15:50 +0300
Subject: [PATCH] Fix corrupted dashboard parameters to have "unnamed" names
and slugs (#34311)
---
...ully-deal-with-corrupted-filter.cy.spec.js | 24 ++++++++++++-------
src/metabase/models/dashboard.clj | 15 ++++++++++++
test/metabase/models/dashboard_test.clj | 11 +++++++++
3 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/e2e/test/scenarios/dashboard-filters/reproductions/24500-gracefully-deal-with-corrupted-filter.cy.spec.js b/e2e/test/scenarios/dashboard-filters/reproductions/24500-gracefully-deal-with-corrupted-filter.cy.spec.js
index 99eb88c6906..d2c4ead1fc2 100644
--- a/e2e/test/scenarios/dashboard-filters/reproductions/24500-gracefully-deal-with-corrupted-filter.cy.spec.js
+++ b/e2e/test/scenarios/dashboard-filters/reproductions/24500-gracefully-deal-with-corrupted-filter.cy.spec.js
@@ -44,7 +44,7 @@ const questionDetails = {
const dashboardDetails = { parameters };
-describe.skip("issues 15279 and 24500", () => {
+describe("issues 15279 and 24500", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
@@ -110,17 +110,23 @@ describe.skip("issues 15279 and 24500", () => {
// The corrupted filter is now present in the UI, but it doesn't work (as expected)
// People can now easily remove it
- // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
- cy.findByText("Select…");
-
editDashboard();
- filterWidget().last().find(".Icon-gear").click();
- // Uncomment the next line if we end up disabling fields for the corrupted filter
- // cy.findByText("No valid fields")
- // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
- cy.findByText("Remove").click();
+ cy.findByTestId("edit-dashboard-parameters-widget-container")
+ .findByText("unnamed")
+ .icon("gear")
+ .click();
+ cy.findByRole("button", { name: "Remove" }).click();
saveDashboard();
+ // Check the list filter again
+ filterWidget().contains("List").click();
+ cy.wait("@values");
+
+ // Check that the search filter works
+ filterWidget().contains("Search").click();
+ cy.findByPlaceholderText("Search by Name").type("Lora Cronin");
+ cy.button("Add filter").click();
+
cy.get(".DashCard")
.should("contain", "Lora Cronin")
.and("not.contain", "Dagmar Fay");
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index 05a55426fbd..3c736704248 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -127,9 +127,24 @@
[dashboard]
(update-dashboard-subscription-pulses! dashboard))
+(defn- migrate-parameters-list
+ "Update the `:parameters` list of a dashboard from legacy formats."
+ [dashboard]
+ (cond-> dashboard
+ (:parameters dashboard)
+ (update :parameters (fn [params]
+ (for [p params]
+ (cond-> p
+ ;; It was previously possible for parameters to have empty strings for :name and
+ ;; :slug, but these are now required to be non-blank strings.
+ (or (= (:name p) "")
+ (= (:slug p) ""))
+ (assoc :name "unnamed" :slug "unnamed")))))))
+
(t2/define-after-select :model/Dashboard
[dashboard]
(-> dashboard
+ migrate-parameters-list
public-settings/remove-public-uuid-if-public-sharing-is-disabled))
(defmethod serdes/hash-fields :model/Dashboard
diff --git a/test/metabase/models/dashboard_test.clj b/test/metabase/models/dashboard_test.clj
index 478764bfaf8..ba8b9a7d3bd 100644
--- a/test/metabase/models/dashboard_test.clj
+++ b/test/metabase/models/dashboard_test.clj
@@ -648,6 +648,17 @@
(is (= nil
(:public_uuid dashboard))))))))
+(deftest migrate-parameters-list-test
+ (testing "test that a Dashboard's :parameters is selected with a non-nil name and slug"
+ (doseq [[name slug] [["" ""] ["" "slug"] ["name" ""]]]
+ (mt/with-temp [:model/Dashboard dashboard {:parameters [{:id "_CATEGORY_NAME_"
+ :type "category"
+ :name name
+ :slug slug}]}]
+ (is (=? {:name "unnamed"
+ :slug "unnamed"}
+ (first (:parameters dashboard))))))))
+
(deftest post-update-test
(t2.with-temp/with-temp [Collection {collection-id-1 :id} {}
Collection {collection-id-2 :id} {}
--
GitLab