fnardin-maystreet commented on PR #32995: URL: https://github.com/apache/superset/pull/32995#issuecomment-4989772751
@rusackas The issue you're raising is real but a bit overstated as it's the type of the `user` prop that's narrower than what's actually passed in at runtime. The review bot flagged this earlier and I deemed it out of scope for this PR ([here](https://github.com/apache/superset/pull/32995#discussion_r3557990646)). This is actually a pre-existing issue: [DatasetList on master](https://github.com/apache/superset/blob/master/superset-frontend/src/pages/DatasetList/index.tsx#L909) already uses the user prop to check admin privileges with the same narrow type. The bug is only in [the declared type of user](https://github.com/apache/superset/blob/master/superset-frontend/src/pages/ChartList/index.tsx#L163). The actual value passed in from [App.tsx](https://github.com/apache/superset/blob/master/superset-frontend/src/views/App.tsx#L102) is the full user object with roles. I think the better fix is widening those types (`ChartListProps`/`DashboardListProps`/`DatasetListProps` and the card prop types) to match what's actually passed, rather than reworking the logic. I'll rebase and do that. I also see your latest commit conflicts with my changes, so I'll fix that too 😅. On the DatasetList spec: the test would behave the same with mockAdminUser, since that fixture lacks username/permissions and therefore fails isUserWithPermissionsAndRoles in permissionUtils.ts. isUserAdmin never actually resolves true for it. I swapped to mockEditorUser to mainly to keep the bot quiet. Good catch though. I'll add a proper fixture (with username/permissions/roles.Admin) that isn't listed as an editor, and a test asserting Edit/Delete stay enabled for it, to actually lock in the admin-bypass behavior. -- 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]
