Fix corrupted dashboard parameters to have "unnamed" names and slugs (#34311)

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");

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

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} {}