codeant-ai-for-open-source[bot] commented on code in PR #32995:
URL: https://github.com/apache/superset/pull/32995#discussion_r3558053339
##########
superset-frontend/src/features/charts/ChartCard.tsx:
##########
@@ -67,35 +69,49 @@ export default function ChartCard({
saveFavoriteStatus,
favoriteStatus,
chartFilter,
- userId,
+ user,
handleBulkChartExport,
getData,
}: ChartCardProps) {
+ const userId = user?.userId;
+
const history = useHistory();
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canExport = hasPerm('can_export');
+ const allowEdit = isUserEditorOrAdmin(user, chart.editors);
Review Comment:
**Suggestion:** The new permission check passes a user object that only
contains `userId`, but `isUserEditorOrAdmin` determines admin status from
`username`/`roles`; this causes admin users who are not explicit editors to be
treated as unauthorized and incorrectly disables edit/delete actions. Pass a
full `UserWithPermissionsAndRoles` shape (or read the current bootstrap user
inside the permission helper) so admin fallback works. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
❌ Admins blocked from editing non-editor charts in ChartList.
❌ Admins blocked from deleting charts from home card view.
⚠️ Confusing UX: admin sees forbidden editor-only message.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Log in as a user with Admin role; Redux user state is populated with
`roles` including
`Admin` (see `UserWithPermissionsAndRoles` in
`superset-frontend/src/types/bootstrapTypes.ts:40-62` and usage in
`superset-frontend/src/pages/ChartList/index.tsx:37-39` where `state.user`
is selected).
2. Navigate to the Charts list page (`/chart/list`), which renders the
`ChartList`
component; `ChartListProps` in
`superset-frontend/src/pages/ChartList/index.tsx:3-10`
defines `user` as `{ userId: string | number; firstName; lastName; }`
without `roles` or
`permissions`, and this `user` prop is passed down to `<ChartCard
user={user} ...>` in
`renderCard` at `superset-frontend/src/pages/ChartList/index.tsx:79-99`.
3. Observe that `ChartCard` receives `user?: { userId?: string | number }`
as defined in
its props at `superset-frontend/src/features/charts/ChartCard.tsx:12-27`
(line 24 in the
file, line 53 in the diff) and uses `const allowEdit =
isUserEditorOrAdmin(user,
chart.editors);` in
`superset-frontend/src/features/charts/ChartCard.tsx:47-54` (line 82
in the diff).
4. The permission helper `isUserEditorOrAdmin` in
`superset-frontend/src/dashboard/util/permissionUtils.ts:10-13` returns
`isUserInEditors(editors) || isUserAdmin(user)`; `isUserAdmin` at
`superset-frontend/src/dashboard/util/permissionUtils.ts:2-8` first checks
`isUserWithPermissionsAndRoles(user)` (requiring `permissions` and `roles`
on the object).
Because the `user` passed from `ChartList`/`ChartCard` only has
`userId`/names and no
`roles`, `isUserWithPermissionsAndRoles(user)` is false, `isUserAdmin(user)`
is always
false, and `allowEdit` is false for admins who are not listed in
`chart.editors`. As a
result, in `ChartCard` the edit/delete menu items (lines 56-87 and 110-163)
set `disabled:
!allowEdit` and their `onClick` handlers are not wired when `allowEdit` is
false, so an
Admin user with `can_write` permissions but not in `chart.editors` sees
disabled
edit/delete actions and cannot perform operations the backend would allow.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9b56f6cb2f6345298b02bdf2c2158b52&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9b56f6cb2f6345298b02bdf2c2158b52&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/features/charts/ChartCard.tsx
**Line:** 53:82
**Comment:**
*Api Mismatch: The new permission check passes a user object that only
contains `userId`, but `isUserEditorOrAdmin` determines admin status from
`username`/`roles`; this causes admin users who are not explicit editors to be
treated as unauthorized and incorrectly disables edit/delete actions. Pass a
full `UserWithPermissionsAndRoles` shape (or read the current bootstrap user
inside the permission helper) so admin fallback works.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32995&comment_hash=62546e8eb1ae7830afcbbb1d1d2129737a4ab4aac66ba2a78d525f27f542aa8f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32995&comment_hash=62546e8eb1ae7830afcbbb1d1d2129737a4ab4aac66ba2a78d525f27f542aa8f&reaction=dislike'>👎</a>
--
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]
