aminghadersohi commented on code in PR #42142:
URL: https://github.com/apache/superset/pull/42142#discussion_r3681328113
##########
tests/integration_tests/dashboards/dao_tests.py:
##########
@@ -73,6 +73,46 @@ def test_get_dashboard_changed_on(self, mock_sm_g, mock_g):
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"
Review Comment:
nit: this block passes on the pre-PR backend too — the -when-absent guard
already landed in #42354 (), so reverting this PR's change wouldn't fail this
test. The behavior genuinely new here is preserving /// when absent. Consider
seeding e.g. , then asserting it survives a payload that omits it, so the test
guards this diff rather than #42354's.
##########
superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx:
##########
@@ -259,6 +284,56 @@ 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);
Review Comment:
nit: this proves the JSON editor wins over a stale dropdown, but with a
truthy it wouldn't catch a regression from back to on — the falsy case is
the whole point of the nullish coalescing. Consider a second assertion where
the JSON holds while the dropdown state is non-zero, asserting survives the
save.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
