You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/07/17 17:37:01 UTC
(superset) branch fix/dashboard-refresh-frequency-json-metadata created (now 23cfddfc527)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a change to branch fix/dashboard-refresh-frequency-json-metadata
in repository https://gitbox.apache.org/repos/asf/superset.git
at 23cfddfc527 fix(dashboard): keep refresh_frequency set via the Advanced JSON editor
This branch includes the following new commits:
new 23cfddfc527 fix(dashboard): keep refresh_frequency set via the Advanced JSON editor
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(dashboard): keep refresh_frequency set via the Advanced JSON editor
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix/dashboard-refresh-frequency-json-metadata
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 23cfddfc527d5abcad1a6597d53c2b742ef9ca22
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Fri Jul 17 10:36:29 2026 -0700
fix(dashboard): keep refresh_frequency set via the Advanced JSON editor
The Dashboard Properties modal seeds a separate refreshFrequency state from the
Refresh dropdown and, on save, unconditionally overwrote
json_metadata.refresh_frequency with it, so a value typed directly into the
Advanced JSON editor was clobbered by the (stale) dropdown value and reset to 0.
This mirrors how color_scheme is already handled: the JSON editor value now
takes precedence on save, and a dropdown change syncs back into the JSON so the
two sources can't diverge.
Also hardens DashboardDAO.set_dash_metadata so metadata fields absent from the
incoming payload are preserved instead of reset to their defaults (the pattern
dosu flagged on the issue), preventing partial updates from wiping unrelated
fields.
Fixes #42116
Co-Authored-By: Claude Opus 4.8 (1M context) <no...@anthropic.com>
---
.../PropertiesModal/PropertiesModal.test.tsx | 61 ++++++++++++++++++++++
.../dashboard/components/PropertiesModal/index.tsx | 15 +++++-
superset/daos/dashboard.py | 30 ++++++++---
tests/integration_tests/dashboards/dao_tests.py | 40 ++++++++++++++
4 files changed, 136 insertions(+), 10 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
index 0dc7b269a23..524dc078b8f 100644
--- a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
+++ b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
@@ -17,6 +17,7 @@
* under the License.
*/
import {
+ fireEvent,
render,
screen,
userEvent,
@@ -41,6 +42,29 @@ 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
+ 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 +283,43 @@ describe('PropertiesModal', () => {
});
});
+ test('preserves a refresh_frequency edited in the JSON editor on save (#42116)', async () => {
+ 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('should close modal', async () => {
mockedIsFeatureEnabled.mockImplementation((flag: any) => {
if (flag === FeatureFlag.TaggingSystem) return true;
diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
index adbeb75aeee..541618feff5 100644
--- a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
+++ b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
@@ -343,7 +343,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 +542,14 @@ 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).
+ const jsonMetadataObj = getJsonMetadata();
+ jsonMetadataObj.refresh_frequency = value;
+ setJsonMetadata(jsonStringify(jsonMetadataObj));
+ };
// Helper function for styling section
const hasCustomLabelsColor = !!Object.keys(
diff --git a/superset/daos/dashboard.py b/superset/daos/dashboard.py
index 4afa24edac8..88d2241dbdd 100644
--- a/superset/daos/dashboard.py
+++ b/superset/daos/dashboard.py
@@ -396,14 +396,28 @@ class DashboardDAO(BaseDAO[Dashboard]):
else:
md["color_namespace"] = data.get("color_namespace")
- md["expanded_slices"] = data.get("expanded_slices", {})
- md["refresh_frequency"] = data.get("refresh_frequency", 0)
- md["color_scheme"] = data.get("color_scheme", "")
- md["label_colors"] = data.get("label_colors", {})
- 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)
+ # 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": {},
+ "shared_label_colors": [],
+ "map_label_colors": {},
+ "color_scheme_domain": [],
+ "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)
dashboard.json_metadata = json.dumps(md)
@staticmethod
diff --git a/tests/integration_tests/dashboards/dao_tests.py b/tests/integration_tests/dashboards/dao_tests.py
index 986fe253381..74667698c30 100644
--- a/tests/integration_tests/dashboards/dao_tests.py
+++ b/tests/integration_tests/dashboards/dao_tests.py
@@ -73,6 +73,46 @@ 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 an existing refresh_frequency in the stored metadata.
+ metadata = json.loads(dashboard.json_metadata or "{}")
+ metadata["refresh_frequency"] = 60
+ dashboard.json_metadata = json.dumps(metadata)
+ db.session.commit()
+
+ # Payload omits refresh_frequency: it must be preserved, not
+ # reset to 0.
+ DashboardDAO.set_dash_metadata(
+ dashboard, {"color_scheme": "d3Category10"}
+ )
+ db.session.commit()
+ saved = json.loads(dashboard.json_metadata)
+ assert saved["refresh_frequency"] == 60
+ assert saved["color_scheme"] == "d3Category10"
+
+ # An explicitly-sent value still overrides.
+ DashboardDAO.set_dash_metadata(dashboard, {"refresh_frequency": 30})
+ db.session.commit()
+ assert json.loads(dashboard.json_metadata)["refresh_frequency"] == 30
+ 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")