Copilot commented on code in PR #33392:
URL: https://github.com/apache/superset/pull/33392#discussion_r2263820632
##########
superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx:
##########
@@ -44,62 +44,125 @@ const createProps = () =>
addSuccessToast: jest.fn(),
}) as PropertiesModalProps;
-fetchMock.get('glob:*/api/v1/chart/318', {
- body: {
- description_columns: {},
- id: 318,
- label_columns: {
- cache_timeout: 'Cache Timeout',
- 'dashboards.dashboard_title': 'Dashboards Dashboard Title',
- 'dashboards.id': 'Dashboards Id',
- description: 'Description',
- 'owners.first_name': 'Owners First Name',
- 'owners.id': 'Owners Id',
- 'owners.last_name': 'Owners Last Name',
- 'owners.username': 'Owners Username',
- params: 'Params',
- slice_name: 'Slice Name',
- viz_type: 'Viz Type',
- },
- result: {
- cache_timeout: null,
- certified_by: 'John Doe',
- certification_details: 'Sample certification',
- dashboards: [
- {
- dashboard_title: 'FCC New Coder Survey 2018',
- id: 23,
+let callCount = 0;
Review Comment:
Using a module-level counter variable can lead to test interdependence and
flaky tests. Consider using a more robust approach like
jest.fn().mockReturnValueOnce() or resetting the counter in a beforeEach block.
##########
superset-frontend/src/explore/components/PropertiesModal/index.tsx:
##########
@@ -179,20 +179,15 @@ function PropertiesModal({
}
try {
- const res = await SupersetClient.put({
+ let res = await SupersetClient.put({
endpoint: `/api/v1/chart/${slice.slice_id}`,
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(payload),
});
- // update the redux state
- const updatedChart = {
- ...payload,
- ...res.json.result,
- tags,
- id: slice.slice_id,
- owners: selectedOwners,
- };
- onSave(updatedChart);
+ res = await SupersetClient.get({
+ endpoint: `/api/v1/chart/${slice.slice_id}`,
+ });
Review Comment:
Making two sequential API calls (PUT then GET) could impact performance.
Consider if the PUT response can be enhanced to return complete owner data,
eliminating the need for the additional GET request.
--
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: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
