test: compare parsed json_metadata instead of raw string in dashboard update tests

set_dash_metadata now merges incoming fields against the dashboard's stored
params_dict rather than the just-written payload, so the serialized key
order of the merged json_metadata is no longer guaranteed to match the
order of the incoming payload. test_update_dashboard and
test_update_partial_dashboard asserted exact string equality, which broke
on the (semantically identical) reordered output; the resulting
AssertionError also skipped test_update_dashboard's cleanup, leaving a
stray "slug1_changed" dashboard that made test_update_partial_dashboard
fail slug-uniqueness validation with a 422.

Compare parsed JSON instead, since key order was never part of the
contract.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx

@@ -17,6 +17,7 @@
  * under the License.
  */
 import {
+  fireEvent,
   render,
   screen,
   userEvent,
@@ -41,6 +42,30 @@ jest.mock('@superset-ui/core', () => ({
   })),
 }));
 
+// Mock the Advanced JSON editor (Ace-based, not drivable in jsdom) with a plain
+// textarea so a direct JSON edit can be simulated. Keeps the "JSON Metadata"
+// label so the existing "should open advance" test still passes.
+jest.mock('./sections/AdvancedSection', () => ({
+  __esModule: true,
+  default: ({
+    jsonMetadata,
+    onJsonMetadataChange,
+  }: {
+    jsonMetadata: string;
+    onJsonMetadataChange: (value: string) => void;
+  }) => (
+    <div>
+      <span>JSON Metadata</span>
+      <textarea
+        aria-label="JSON metadata editor"
+        data-test="mock-json-editor"
+        value={jsonMetadata}
+        onChange={e => onJsonMetadataChange(e.target.value)}
+      />
+    </div>
+  ),
+}));
+
 const mockedIsFeatureEnabled = isFeatureEnabled as jest.Mock;
 
 const spyColorSchemeSelect = jest.spyOn(ColorSchemeSelect, 'default');
@@ -259,6 +284,175 @@ describe('PropertiesModal', () => {
     });
   });
 
+  test('preserves a refresh_frequency edited in the JSON editor on save (#42116)', async () => {
+    // Save (onlyApply: false) PUTs to the API before calling onSubmit, so the
+    // request must be mocked or onSubmit is never reached.
+    const put = jest.spyOn(SupersetCore.SupersetClient, 'put');
+    put.mockResolvedValue({
+      json: {
+        result: {
+          dashboard_title: 'dashboard_title',
+          slug: 'slug',
+          json_metadata: 'json_metadata',
+          editors: 'editors',
+        },
+      },
+    } as any);
+    mockedIsFeatureEnabled.mockReturnValue(false);
+    const props = createProps();
+    const propsWithDashboardInfo = {
+      ...props,
+      dashboardInfo: {
+        ...dashboardInfo,
+        json_metadata: mockedJsonMetadata,
+      },
+    };
+    render(<PropertiesModal {...propsWithDashboardInfo} />, {
+      useRedux: true,
+    });
+    await screen.findByTestId('dashboard-edit-properties-form');
+
+    // Expand the Advanced settings panel so the (mocked) JSON editor mounts.
+    const advancedHeader = screen
+      .getByText('Advanced settings')
+      .closest('.ant-collapse-header');
+    await userEvent.click(advancedHeader!);
+
+    // Edit refresh_frequency directly in the JSON editor without touching the
+    // Refresh dropdown (the reproduction of #42116).
+    const editor = await screen.findByTestId('mock-json-editor');
+    fireEvent.change(editor, {
+      target: { value: JSON.stringify({ refresh_frequency: 30 }) },
+    });
+
+    await userEvent.click(screen.getByRole('button', { name: 'Save' }));
+
+    await waitFor(() => {
+      expect(props.onSubmit).toHaveBeenCalledTimes(1);
+    });
+    const submitted = JSON.parse(props.onSubmit.mock.calls[0][0].jsonMetadata);
+    expect(submitted.refresh_frequency).toBe(30);
+  });
+
+  test('preserves an explicit 0 from the JSON editor over a non-zero dropdown value (#42116)', async () => {
+    // A truthy value like 30 can't tell `??` and `||` apart. Only a falsy-but-
+    // explicit `refresh_frequency: 0` in the JSON, combined with a non-zero
+    // Refresh dropdown, catches a regression from `??` back to `||` on
+    // index.tsx, which would let the dropdown's non-zero value win over an
+    // explicit "Don't refresh".
+    const put = jest.spyOn(SupersetCore.SupersetClient, 'put');
+    put.mockResolvedValue({
+      json: {
+        result: {
+          dashboard_title: 'dashboard_title',
+          slug: 'slug',
+          json_metadata: 'json_metadata',
+          editors: 'editors',
+        },
+      },
+    } as any);
+    mockedIsFeatureEnabled.mockReturnValue(false);
+    const props = createProps();
+    // A non-zero refresh_frequency in dashboardInfo so the Refresh dropdown
+    // initializes to a truthy value (dashboardInfo, when passed, is used
+    // directly instead of triggering a fetch -- see the `!currentDashboardInfo`
+    // check in the data-loading effect). handleDashboardData reads the parsed
+    // `metadata` object, not the `json_metadata` string.
+    const nonZeroMetadata = mockedJsonMetadata.replace(
+      '"refresh_frequency": 0',
+      '"refresh_frequency": 30',
+    );
+    const propsWithDashboardInfo = {
+      ...props,
+      dashboardInfo: {
+        ...dashboardInfo,
+        json_metadata: nonZeroMetadata,
+        metadata: JSON.parse(nonZeroMetadata),
+      },
+    };
+    render(<PropertiesModal {...propsWithDashboardInfo} />, {
+      useRedux: true,
+    });
+    await screen.findByTestId('dashboard-edit-properties-form');
+
+    // Confirm the Refresh dropdown actually picked up the non-zero value.
+    const refreshHeader = screen
+      .getByText('Refresh settings')
+      .closest('.ant-collapse-header');
+    await userEvent.click(refreshHeader!);
+    expect(
+      await screen.findByRole('radio', { name: '30 seconds' }),
+    ).toBeChecked();
+
+    // Edit the JSON editor to explicitly set refresh_frequency to 0 ("Don't
+    // refresh") without touching the Refresh dropdown.
+    const advancedHeader = screen
+      .getByText('Advanced settings')
+      .closest('.ant-collapse-header');
+    await userEvent.click(advancedHeader!);
+    const editor = await screen.findByTestId('mock-json-editor');
+    fireEvent.change(editor, {
+      target: { value: JSON.stringify({ refresh_frequency: 0 }) },
+    });
+
+    await userEvent.click(screen.getByRole('button', { name: 'Save' }));
+
+    await waitFor(() => {
+      expect(props.onSubmit).toHaveBeenCalledTimes(1);
+    });
+    const submitted = JSON.parse(props.onSubmit.mock.calls[0][0].jsonMetadata);
+    expect(submitted.refresh_frequency).toBe(0);
+  });
+
+  test('propagates a Refresh dropdown-only change into the submitted JSON metadata', async () => {
+    // Selecting a value from the Refresh dropdown without touching the
+    // Advanced JSON editor must still make it into the submitted payload --
+    // handleRefreshFrequencyChange writes the new value into the JSON
+    // metadata object on change (#42116, requested in review).
+    const put = jest.spyOn(SupersetCore.SupersetClient, 'put');
+    put.mockResolvedValue({
+      json: {
+        result: {
+          dashboard_title: 'dashboard_title',
+          slug: 'slug',
+          json_metadata: 'json_metadata',
+          editors: 'editors',
+        },
+      },
+    } as any);
+    mockedIsFeatureEnabled.mockReturnValue(false);
+    const props = createProps();
+    const propsWithDashboardInfo = {
+      ...props,
+      dashboardInfo: {
+        ...dashboardInfo,
+        json_metadata: mockedJsonMetadata,
+      },
+    };
+    render(<PropertiesModal {...propsWithDashboardInfo} />, {
+      useRedux: true,
+    });
+    await screen.findByTestId('dashboard-edit-properties-form');
+
+    // mockedJsonMetadata starts at refresh_frequency: 0, so selecting
+    // "1 minute" (60) is a real change coming only from the dropdown.
+    const refreshHeader = screen
+      .getByText('Refresh settings')
+      .closest('.ant-collapse-header');
+    await userEvent.click(refreshHeader!);
+    await userEvent.click(
+      await screen.findByRole('radio', { name: '1 minute' }),
+    );
+
+    await userEvent.click(screen.getByRole('button', { name: 'Save' }));
+
+    await waitFor(() => {
+      expect(props.onSubmit).toHaveBeenCalledTimes(1);
+    });
+    const submitted = JSON.parse(props.onSubmit.mock.calls[0][0].jsonMetadata);
+    expect(submitted.refresh_frequency).toBe(60);
+  });
+
   test('should close modal', async () => {
     mockedIsFeatureEnabled.mockImplementation((flag: any) => {
       if (flag === FeatureFlag.TaggingSystem) return true;

superset-frontend/src/dashboard/components/PropertiesModal/index.tsx

@@ -238,17 +238,19 @@ const PropertiesModal = ({
     }, handleErrorResponse);
   }, [dashboardId, handleDashboardData]);
 
-  const getJsonMetadata = () => {
+  // Returns undefined (rather than {}) when the editor holds invalid JSON,
+  // so callers can distinguish "empty/no metadata yet" from "user is
+  // mid-edit with unparsable text" and avoid clobbering the latter.
+  const parseJsonMetadata = () => {
     try {
-      const jsonMetadataObj = jsonMetadata?.length
-        ? JSON.parse(jsonMetadata)
-        : {};
-      return jsonMetadataObj;
+      return jsonMetadata?.length ? JSON.parse(jsonMetadata) : {};
     } catch (_) {
-      return {};
+      return undefined;
     }
   };
 
+  const getJsonMetadata = () => parseJsonMetadata() ?? {};
+
   const handleOnChangeEditors = (values: SubjectPickerValue[]) => {
     setEditors(mapPickerValuesToSubjects(values));
   };
@@ -343,7 +345,12 @@ const PropertiesModal = ({
         ? resettableCustomLabels
         : false;
     const jsonMetadataObj = getJsonMetadata();
-    jsonMetadataObj.refresh_frequency = refreshFrequency;
+    // A refresh_frequency edited directly in the Advanced JSON editor takes
+    // precedence over the Refresh dropdown state, mirroring how color_scheme is
+    // handled above. Nullish coalescing preserves an explicit 0 ("Don't
+    // refresh") rather than falling through to the dropdown value (#42116).
+    jsonMetadataObj.refresh_frequency =
+      jsonMetadataObj.refresh_frequency ?? refreshFrequency;
     jsonMetadataObj.show_chart_timestamps = Boolean(showChartTimestamps);
     const customLabelColors = jsonMetadataObj.label_colors || {};
     const updatedDashboardMetadata = {
@@ -537,8 +544,19 @@ const PropertiesModal = ({
 
   // Section handlers for extracted components
   const handleThemeChange = (value: any) => setSelectedThemeId(value || null);
-  const handleRefreshFrequencyChange = (value: number) =>
+  const handleRefreshFrequencyChange = (value: number) => {
     setRefreshFrequency(value);
+    // Keep the Advanced JSON editor in sync with the dropdown so the two
+    // sources can't diverge, mirroring onColorSchemeChange (#42116). Skip
+    // this while the editor holds invalid/in-progress JSON so we don't
+    // clobber the user's unfinished edit with a one-field object.
+    const jsonMetadataObj = parseJsonMetadata();
+    if (!jsonMetadataObj) {
+      return;
+    }
+    jsonMetadataObj.refresh_frequency = value;
+    setJsonMetadata(jsonStringify(jsonMetadataObj));
+  };
 
   // Helper function for styling section
   const hasCustomLabelsColor = !!Object.keys(
@@ -580,7 +598,15 @@ const PropertiesModal = ({
           const refreshLimit =
             dashboardInfo?.common?.conf
               ?.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT;
-          return validateRefreshFrequency(refreshFrequency, refreshLimit);
+          // A refresh_frequency edited directly in the Advanced JSON editor
+          // takes precedence over the dropdown when saving (see onFinish),
+          // so validate that effective value rather than the dropdown state.
+          const effectiveRefreshFrequency =
+            getJsonMetadata()?.refresh_frequency ?? refreshFrequency;
+          return validateRefreshFrequency(
+            effectiveRefreshFrequency,
+            refreshLimit,
+          );
         },
       },
       {

superset/commands/dashboard/update.py

@@ -83,11 +83,23 @@ class UpdateDashboardCommand(UpdateMixin, BaseCommand):
                     json.loads(position_json)
                 )
 
-            dashboard = DashboardDAO.update(self._model, self._properties)
-            if self._properties.get("json_metadata"):
+            # ``set_dash_metadata`` merges the incoming metadata against
+            # ``dashboard.params_dict`` (the *stored* ``json_metadata``) to
+            # preserve fields the caller omitted. Routing ``json_metadata``
+            # through the generic attribute update below would overwrite
+            # that stored value before the merge ever sees it, silently
+            # collapsing the merge into a no-op and resetting any omitted
+            # field to its default -- so it is excluded here and applied
+            # exclusively via ``set_dash_metadata``.
+            json_metadata = self._properties.get("json_metadata")
+            dashboard = DashboardDAO.update(
+                self._model,
+                {k: v for k, v in self._properties.items() if k != "json_metadata"},
+            )
+            if json_metadata:
                 DashboardDAO.set_dash_metadata(
                     dashboard,
-                    data=json.loads(self._properties.get("json_metadata", "{}")),
+                    data=json.loads(json_metadata),
                 )
         return dashboard
 

superset/daos/dashboard.py

@@ -396,15 +396,36 @@ class DashboardDAO(BaseDAO[Dashboard]):
         else:
             md["color_namespace"] = data.get("color_namespace")
 
-        md["expanded_slices"] = data.get("expanded_slices", {})
-        if "refresh_frequency" in data:
-            md["refresh_frequency"] = data["refresh_frequency"]
-        md["color_scheme"] = data.get("color_scheme", "")
-        md["label_colors"] = data.get("label_colors", {})
+        # Only overwrite these metadata fields when the caller explicitly sends
+        # them. Previously each used ``data.get(key, default)``, which reset a
+        # value to its default whenever it was absent from the payload -- e.g. a
+        # ``refresh_frequency`` set directly in the Advanced JSON editor got
+        # wiped on save. ``setdefault`` still seeds a default for brand-new
+        # dashboards that have never had the key, keeping the shape stable
+        # without clobbering existing values (#42116).
+        metadata_defaults: dict[str, Any] = {
+            "expanded_slices": {},
+            "refresh_frequency": 0,
+            "color_scheme": "",
+            "label_colors": {},
+            "cross_filters_enabled": True,
+        }
+        for key, default_value in metadata_defaults.items():
+            if key in data:
+                md[key] = data[key]
+            else:
+                md.setdefault(key, default_value)
+
+        # These are derived from ``color_scheme``/``label_colors`` on the
+        # frontend (see ``applyDashboardLabelsColorOnLoad``) and are always
+        # omitted from the Properties modal's payload, so they must keep
+        # resetting to their default when absent rather than being preserved
+        # -- otherwise a color scheme change made through that modal leaves
+        # stale label-to-color mappings in place instead of triggering
+        # recomputation on next load.
         md["shared_label_colors"] = data.get("shared_label_colors", [])
         md["map_label_colors"] = data.get("map_label_colors", {})
         md["color_scheme_domain"] = data.get("color_scheme_domain", [])
-        md["cross_filters_enabled"] = data.get("cross_filters_enabled", True)
         dashboard.json_metadata = json.dumps(md)
 
     @staticmethod

tests/integration_tests/dashboards/api_tests.py

@@ -1998,7 +1998,13 @@ class TestDashboardApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCa
         assert model.slug == self.dashboard_data["slug"]
         assert model.position_json == self.dashboard_data["position_json"]
         assert model.css == self.dashboard_data["css"]
-        assert model.json_metadata == self.dashboard_data["json_metadata"]
+        # Compare parsed JSON rather than raw strings: ``set_dash_metadata``
+        # merges against the dashboard's previously stored metadata, so key
+        # insertion order (and therefore serialized key order) is an
+        # implementation detail, not part of the contract being tested.
+        assert json.loads(model.json_metadata) == json.loads(
+            self.dashboard_data["json_metadata"]
+        )
         assert model.published == self.dashboard_data["published"]
         admin_subject = (
             db.session.query(Subject)
@@ -2010,6 +2016,44 @@ class TestDashboardApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCa
         db.session.delete(model)
         db.session.commit()
 
+    def test_update_dashboard_preserves_unsent_json_metadata_fields(self):
+        """
+        Dashboard API: a PUT whose ``json_metadata`` omits a field must not
+        reset that field to its default. ``UpdateDashboardCommand`` routes
+        the incoming ``json_metadata`` through ``DashboardDAO.update``'s
+        generic attribute assignment before calling
+        ``DashboardDAO.set_dash_metadata`` -- if that assignment overwrites
+        ``dashboard.json_metadata`` first, the merge in
+        ``set_dash_metadata`` (which reads ``dashboard.params_dict``) has
+        nothing but the new, partial payload to merge against, silently
+        collapsing the merge into a no-op (see #42142).
+        """
+        admin = self.get_user("admin")
+        dashboard_id = self.insert_dashboard(
+            "title1",
+            "slug1",
+            [admin.id],
+            json_metadata=json.dumps(
+                {"refresh_frequency": 60, "cross_filters_enabled": False}
+            ),
+        ).id
+        self.login(ADMIN_USERNAME)
+        uri = f"api/v1/dashboard/{dashboard_id}"
+        rv = self.put_assert_metric(
+            uri,
+            {"json_metadata": json.dumps({"color_scheme": "d3Category10"})},
+            "put",
+        )
+        assert rv.status_code == 200
+        model = db.session.query(Dashboard).get(dashboard_id)
+        saved_metadata = json.loads(model.json_metadata)
+        assert saved_metadata["refresh_frequency"] == 60
+        assert saved_metadata["cross_filters_enabled"] is False
+        assert saved_metadata["color_scheme"] == "d3Category10"
+
+        db.session.delete(model)
+        db.session.commit()
+
     def test_add_dashboard_filters(self):
         """
         Dashboard API: Test that a filter was added
@@ -2603,7 +2647,11 @@ class TestDashboardApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCa
         assert rv.status_code == 200
 
         model = db.session.query(Dashboard).get(dashboard_id)
-        assert model.json_metadata == self.dashboard_data["json_metadata"]
+        # Compare parsed JSON rather than raw strings; see the equivalent
+        # comment in ``test_update_dashboard``.
+        assert json.loads(model.json_metadata) == json.loads(
+            self.dashboard_data["json_metadata"]
+        )
         assert model.dashboard_title == self.dashboard_data["dashboard_title"]
         assert model.slug == self.dashboard_data["slug"]
 

tests/integration_tests/dashboards/dao_tests.py

@@ -73,6 +73,57 @@ class TestDashboardDAO(SupersetTestCase):
             DashboardDAO.set_dash_metadata(dashboard, original_data)
             db.session.commit()
 
+    @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+    @patch("superset.utils.core.g")
+    @patch("superset.security.manager.g")
+    def test_set_dash_metadata_preserves_unsent_fields(self, mock_sm_g, mock_g):
+        """
+        set_dash_metadata must not reset metadata fields that are absent from the
+        incoming payload, such as a ``refresh_frequency`` edited directly in the
+        Advanced JSON editor (#42116). Fields that are present still override.
+        """
+        mock_g.user = mock_sm_g.user = security_manager.find_user("admin")
+        with self.client.application.test_request_context():
+            dashboard = (
+                db.session.query(Dashboard).filter_by(slug="world_health").first()
+            )
+            original_json_metadata = dashboard.json_metadata
+            try:
+                # Seed existing values in the stored metadata, including
+                # cross_filters_enabled=False -- its default is True, so this
+                # is the field that actually exercises this PR's change (the
+                # refresh_frequency preservation alone already landed in
+                # #42354).
+                metadata = json.loads(dashboard.json_metadata or "{}")
+                metadata["refresh_frequency"] = 60
+                metadata["cross_filters_enabled"] = False
+                dashboard.json_metadata = json.dumps(metadata)
+                db.session.commit()
+
+                # Payload omits refresh_frequency and cross_filters_enabled:
+                # both must be preserved, not reset to their defaults.
+                DashboardDAO.set_dash_metadata(
+                    dashboard, {"color_scheme": "d3Category10"}
+                )
+                db.session.commit()
+                saved = json.loads(dashboard.json_metadata)
+                assert saved["refresh_frequency"] == 60
+                assert saved["cross_filters_enabled"] is False
+                assert saved["color_scheme"] == "d3Category10"
+
+                # An explicitly-sent value still overrides.
+                DashboardDAO.set_dash_metadata(
+                    dashboard,
+                    {"refresh_frequency": 30, "cross_filters_enabled": True},
+                )
+                db.session.commit()
+                saved = json.loads(dashboard.json_metadata)
+                assert saved["refresh_frequency"] == 30
+                assert saved["cross_filters_enabled"] is True
+            finally:
+                dashboard.json_metadata = original_json_metadata
+                db.session.commit()
+
     @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
     @patch("superset.daos.dashboard.g")
     @patch("superset.security.manager.g")