rusackas commented on code in PR #42142:
URL: https://github.com/apache/superset/pull/42142#discussion_r3685819447
##########
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:
Good catch. Added `cross_filters_enabled=False` to the seed and assertions
so this actually exercises the new preservation logic, not just re-covering the
refresh_frequency guard from #42354.
##########
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:
Good catch. Added a second case where the JSON holds an explicit
`refresh_frequency: 0` while the dropdown sits at a non-zero value, so a `??`
to `||` regression would actually fail it.
--
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]
