codeant-ai-for-open-source[bot] commented on code in PR #32995:
URL: https://github.com/apache/superset/pull/32995#discussion_r3526443727
##########
superset-frontend/src/pages/DashboardList/index.tsx:
##########
@@ -469,15 +496,21 @@ function DashboardList(props: DashboardListProps) {
{canEdit && (
<Tooltip
id="edit-action-tooltip"
- title={t('Edit')}
+ title={
+ allowEdit
+ ? t('Edit')
+ : t(
+ 'You must be a dashboard owner in order to edit.
Please reach out to a dashboard owner to request modifications or edit access.',
+ )
+ }
placement="bottom"
>
<span
data-test="dashboard-row-edit"
role="button"
tabIndex={0}
- className="action-button"
- onClick={handleEdit}
+ className={`action-button ${allowEdit ? '' : 'disabled'}`}
+ onClick={allowEdit ? handleEdit : undefined}
Review Comment:
**Suggestion:** This permission gating is applied only to the table row
actions, but card view actions are still rendered via
`renderCard`/`DashboardCard` without the owner/admin guard, so users can still
attempt forbidden edits/deletes in card mode. Apply the same allow-check to the
card action path to avoid inconsistent behavior. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Card view allows edit/delete for non-owner writers.
- ⚠️ Inconsistent permissions between table and card Dashboards.
- ⚠️ Users see Forbidden errors when using card actions.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In src/pages/DashboardList/index.tsx:359-377 and 479-487, observe the
table view
Actions column, where `allowEdit` is computed using ownership and
isUserAdmin, and
`onClick={allowEdit ? handleEdit : undefined}` plus `.disabled` styling
prevent
non-owner/non-admin users from triggering edit/delete.
2. In the same file at lines 141-159, see `renderCard` defined to return a
<DashboardCard
/> with props including `dashboard`, `hasPerm`, `bulkSelectEnabled`,
`userId`,
`openDashboardEditModal`, `handleBulkDashboardExport`, and
`onDelete={dashboard =>
setDashboardToDelete(dashboard)}`; note that `allowEdit` is not passed or
used here.
3. In src/features/dashboards/DashboardCard.tsx:49-65 and 71-88, observe
that `canEdit`,
`canDelete`, and `canExport` are derived solely from `hasPerm('can_write')` /
`hasPerm('can_export')`. When `canEdit` is true, the Kebab menu adds an Edit
item whose
onClick calls `openDashboardEditModal(dashboard)`, and when `canDelete` is
true, a Delete
item whose onClick calls `onDelete(dashboard)`, with no owner/admin gating.
4. Run the app, log in as a user who has dashboard `can_write` (so
`hasPerm('can_write')`
is true) but is not listed as an owner for a given dashboard
(DashboardList’s `owners`
column data in index.tsx:95-103). Switch the Dashboards list to card view
(ListView
defaultViewMode uses 'card' when FeatureFlag.ListviewsDefaultCardView is
enabled at
index.tsx:98-101, or use the view toggle buttons). In this card view, open
the Kebab menu
on a non-owned dashboard and click Edit or Delete: DashboardCard still
allows the action
via its Kebab menu, leading to backend 403/Forbidden responses, whereas the
table view
Actions column correctly disables the same actions. This demonstrates the
fix only applies
to table-row actions and not to the card view path.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c055079c2add449280c3abbb7a8f2efa&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=c055079c2add449280c3abbb7a8f2efa&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/pages/DashboardList/index.tsx
**Line:** 513:513
**Comment:**
*Incomplete Implementation: This permission gating is applied only to
the table row actions, but card view actions are still rendered via
`renderCard`/`DashboardCard` without the owner/admin guard, so users can still
attempt forbidden edits/deletes in card mode. Apply the same allow-check to the
card action path to avoid inconsistent behavior.
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=adb5513210604979194cbda124dddc85ff6c8f90646f1f81ea6c09e5049f1b6b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32995&comment_hash=adb5513210604979194cbda124dddc85ff6c8f90646f1f81ea6c09e5049f1b6b&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]
