EnxDev commented on code in PR #43376:
URL: https://github.com/apache/superset/pull/43376#discussion_r4270544424


##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -837,8 +882,6 @@ test('Dataset drill info API call is made when user can 
only view chart as table
     ...createProps(),
     supersetCanExplore: false,
   };
-  // "View as table" has its own permission, so label resolution must not be
-  // gated behind Drill to detail.

Review Comment:
   These are still being removed at the current head, along with the ones at 
old lines 886 and 920. Looks like the restore from 97a8e62 got dropped again 
when master was merged back in, since `master` still has all three.
   
   Mind putting them back? Taking master's side of this file for those hunks 
should be enough.



##########
superset-frontend/src/features/semanticViews/AddSemanticViewModal.tsx:
##########
@@ -538,6 +538,19 @@ export default function AddSemanticViewModal({
   // Modal open / close
   // =========================================================================
 
+  // A parent can unmount the modal without first setting show to false.

Review Comment:
   This unmount cleanup, the rewritten retry test, and the OpenAPI 409 blocks 
aren't related to the query inspector. The OpenAPI part already landed on 
master in #45262, so a rebase drops it on its own.
   
   The modal fix is a real behavior change in a different feature, though. 
Could it go in its own `fix(semantic)` PR so it gets reviewed by people who 
know that modal and doesn't ride in under `feat(dashboard)`?



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