Re: [PR] fix(frontend): fix edit/delete permissions for charts, dashboards, and datasets (#18870) (#32981) [superset]

via GitHub Wed, 15 Jul 2026 23:51:37 -0700


codeant-ai-for-open-source[bot] commented on code in PR #32995:
URL: https://github.com/apache/superset/pull/32995#discussion_r3593234161



##########
superset-frontend/src/features/dashboards/DashboardCard.tsx:
##########
@@ -55,18 +57,21 @@ function DashboardCard({
   dashboard,
   hasPerm,
   bulkSelectEnabled,
-  userId,
+  user,
   openDashboardEditModal,
   favoriteStatus,
   saveFavoriteStatus,
   showThumbnails,
   handleBulkDashboardExport,
   onDelete,
 }: DashboardCardProps) {
+  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, dashboard.editors);

Review Comment:
   **Suggestion:** The new edit/delete gate relies on `isUserEditorOrAdmin`, 
but this component now accepts a `user` shape that may only contain `userId`; 
in the Home page call path the user object does not include roles, so admin 
detection always fails and admins who are not explicitly in editors get 
edit/delete incorrectly disabled. Use the full bootstrap user type (with roles) 
for this prop and pass that through so admin checks are evaluated correctly. 
[api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   ❌ Admins on dashboards list page see edit/delete disabled.
   ❌ Dashboard list kebab actions misrepresent backend permissions.
   ⚠️ Inconsistent behavior between Home dashboard cards and list cards.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Render the dashboards list UI using the `DashboardList` component in
   `superset-frontend/src/pages/DashboardList/index.tsx` (function 
`DashboardList` starting
   around line 178), which passes its `user` prop into `DashboardCard` in the 
`renderCard`
   callback at lines 22–40 (around line 763 in the file): `<DashboardCard ... 
user={user} ...
   />`.
   
   2. Observe that `DashboardListProps` (defined in
   `superset-frontend/src/pages/DashboardList/index.tsx:5–13`) declares `user` 
as a simple
   object with only `userId`, `firstName`, and `lastName` and does not include 
`roles`,
   `permissions`, or `username`, so the `user` passed to `DashboardCard` on the 
dashboards
   list page does not carry role information.
   
   3. In `DashboardCard` 
(`superset-frontend/src/features/dashboards/DashboardCard.tsx`), see
   that `DashboardCardProps` defines `user?: { userId?: string | number };` at 
line 50 and
   that `allowEdit` is computed as `const allowEdit = isUserEditorOrAdmin(user,
   dashboard.editors);` at line 74, passing this minimal `user` object into
   `isUserEditorOrAdmin`.
   
   4. In `superset-frontend/src/dashboard/util/permissionUtils.ts`, 
`isUserEditorOrAdmin` is
   defined at lines 49–52 to call `isUserAdmin(user)`, and `isUserAdmin` (lines 
41–47) relies
   on `isUserWithPermissionsAndRoles(user)` and then checks 
`Object.keys(user.roles || {})`.
   The type guard `isUserWithPermissionsAndRoles` in
   `superset-frontend/src/types/bootstrapTypes.ts:204–207` returns true only 
when `user` has
   `username`, `permissions`, and `roles`. Because the `user` coming from 
`DashboardList`
   lacks these fields, `isUserAdmin(user)` always returns false, so 
`isUserEditorOrAdmin`
   only returns true when `isUserInEditors(editors)` matches.
   
   5. Log in as an admin user who is not listed among a dashboard’s `editors` 
(the `editors`
   data is fetched in `DashboardList` via the `DASHBOARD_COLUMNS_TO_FETCH` list 
including
   `editors.*` at lines 31–57) and view that dashboard in the dashboards list 
UI. The backend
   still considers the admin allowed to edit/delete, but on the list page 
`allowEdit` is
   false, causing the menu items in `DashboardCard` (edit/delete at lines 
83–117 and 140–170)
   to render with `disabled: !allowEdit` and click handlers omitted, so the 
admin sees
   disabled edit/delete actions on the list page despite having permission.
   
   6. Note that on the Home page path
   (`superset-frontend/src/features/home/DashboardTable.tsx`), 
`DashboardTableProps.user` is
   typed as `BootstrapUser` in 
`superset-frontend/src/views/CRUD/types.ts:9–18`, so there the
   `user` object does contain roles. This means the bug specifically affects 
the dashboards
   list path where a non-bootstrap `user` object is passed into 
`DashboardCard`, confirming a
   real API mismatch between `DashboardCard`’s `user` prop and what 
`isUserEditorOrAdmin`
   expects.
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9ddc00d4652c4482aebafde6543fc445&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9ddc00d4652c4482aebafde6543fc445&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/dashboards/DashboardCard.tsx
   **Line:** 50:74
   **Comment:**
        *Api Mismatch: The new edit/delete gate relies on 
`isUserEditorOrAdmin`, but this component now accepts a `user` shape that may 
only contain `userId`; in the Home page call path the user object does not 
include roles, so admin detection always fails and admins who are not 
explicitly in editors get edit/delete incorrectly disabled. Use the full 
bootstrap user type (with roles) for this prop and pass that through so admin 
checks are evaluated correctly.
   
   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=0320093913ed5e7d275c2a33a2a1eda78b2e35c3dac1e760f000f78e670efea6&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32995&comment_hash=0320093913ed5e7d275c2a33a2a1eda78b2e35c3dac1e760f000f78e670efea6&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]

Re: [PR] fix(frontend): fix edit/delete permissions for charts, dashboards, and datasets (#18870) (#32981) [superset]

Reply via email to