sadpandajoe commented on code in PR #43365:
URL: https://github.com/apache/superset/pull/43365#discussion_r3826440364


##########
superset-frontend/src/features/reports/ReportModal/reducer.ts:
##########
@@ -52,14 +52,10 @@ export default function reportsReducer(
     [SET_REPORT]() {
       const { report, resourceId, creationMethod, filterField } =
         action as SetReportAction;
-      // Map filterField ('dashboard_id' or 'chart_id') to the corresponding
-      // ReportObject property ('dashboard' or 'chart')
-      const propertyName =
-        filterField === 'dashboard_id' ? 'dashboard' : 'chart';
       // For now report count should only be one, but we are checking in case
       // functionality changes.
       const reportObject = report.result?.find(
-        (r: ReportObject) => r[propertyName] === resourceId,
+        (r: ReportObject) => r[filterField] === resourceId,

Review Comment:
   This also restores the chart menu for an Explore view opened from a 
dashboard, but `ReportModal` selects `reports.dashboards[dashboardId]` whenever 
`dashboardId` is present. The chart schedule is stored under 
`reports.charts[chartId]`, so Edit opens as create and submits both IDs, which 
the API rejects. Could the modal select the schedule using its `creationMethod` 
(and add this end-to-end case)?



##########
superset-frontend/src/features/reports/ReportModal/reducer.ts:
##########
@@ -52,14 +52,10 @@ export default function reportsReducer(
     [SET_REPORT]() {
       const { report, resourceId, creationMethod, filterField } =
         action as SetReportAction;
-      // Map filterField ('dashboard_id' or 'chart_id') to the corresponding
-      // ReportObject property ('dashboard' or 'chart')
-      const propertyName =
-        filterField === 'dashboard_id' ? 'dashboard' : 'chart';
       // For now report count should only be one, but we are checking in case
       // functionality changes.
       const reportObject = report.result?.find(
-        (r: ReportObject) => r[propertyName] === resourceId,
+        (r: ReportObject) => r[filterField] === resourceId,

Review Comment:
   The list response does not include `report_format`, but this now makes that 
partial object drive the restored edit flow. Editing an existing XLSX report 
after a reload falls back to the visualization default and overwrites its 
delivery format. Could the menu fetch the full schedule before editing, or 
preserve the existing format instead?



-- 
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]

Reply via email to