Re: [PR] fix(dashboard,explore): wire addWarningToast into download callers [superset]

["sadpandajoe (via GitHub)" <gi...@apache.org>]

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


##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -158,7 +165,15 @@ export const useDownloadMenuItems = (
 
   const onDownloadImage = async (e: SyntheticEvent) => {
     try {
-      downloadAsImage(SCREENSHOT_NODE_SELECTOR, dashboardTitle, true)(e);
+      downloadAsImage(

Review Comment:
   Nothing asserts the new `addWarningToast`/`addInfoToast` arguments on these calls: `DownloadMenuItems.test.tsx` only checks the items' enabled state and its `useToasts` mock has no `addWarningToast`, so dropping or shifting them would still pass CI and bring back the silent failure. Could a test click each item with `downloadAsImage`/`downloadAsPdf` mocked and assert the callbacks arrive in the expected slots (the Explore JPEG items, whose tests only assert call count, have the same gap)?



-- 
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-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org