sha174n commented on PR #43137: URL: https://github.com/apache/superset/pull/43137#issuecomment-5368855983
Nice, this correctly makes the entity fields mutually exclusive off creationMethod, and it's good that commonFields covers both the create and edit paths. One thing worth tightening: the save scope now follows creationMethod, but the edit-mode selector around index.tsx:168-172 still resolves scope from dashboardId first. In the exact flow this targets (a chart opened from a dashboard, so both dashboardId and chart are set), the selector resolves to the dashboard report, so if a dashboard report already exists in the store the modal loads it in edit mode and saving would PUT a chart-only payload against that dashboard report's id. Could you key the selector off creationMethod too, so edit-mode selection matches the save scope? Or confirm that case can't be reached. -- 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]
