[Grid] Respect user and workflow permissions for batch edit and batch delete - #3954
Open
xIrusux wants to merge 4 commits into
Open
[Grid] Respect user and workflow permissions for batch edit and batch delete#3954xIrusux wants to merge 4 commits into
xIrusux wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Verdict: Needs changes. The PR captures row permissions and lock state across pagination, then gates data-object batch edit/delete actions.
Changes:
- Persists
permissionsandisLockedfor selected rows. - Requires all selected rows to permit save/delete.
- Disables actions for locked rows.
Review assessment:
- Root cause: Mostly addressed at
with-row-selection.tsx:86-90andbatch-actions.tsx:45-51. - Blocking issue:
selectedRowsDataretains deselected rows, so stale entries incorrectly affect permissions (batch-actions.tsx:45-48). - Call sites/boundary: Object batch edit/delete are covered at the appropriate consumer.
- Compatibility: No public API break.
- Tests: Only manual verification is documented; the stale-selection edge case remains untested.
- Docs/changelog: No update appears necessary for this bug fix.
- Risk: Backend authorization from the counterpart PR remains unverified here.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
with-row-selection.tsx |
Stores permission and lock metadata for selected rows. |
batch-actions.tsx |
Gates object batch edit and delete actions. |
selectedRowsData is an accumulating cache that retains deselected rows; iterate the current selection keys instead so stale entries are ignored and rows with missing metadata fail closed. Co-Authored-By: Claude <noreply@anthropic.com>
selectedRowsData is an accumulating cache that retains deselected rows, so iterating its values let stale entries block batch edit/delete after deselection, while an empty cache briefly allowed actions on freshly selected rows before their metadata was captured. Iterate the current selectedRows ids instead, so stale entries are ignored and rows with missing metadata fail closed. Type the row access to satisfy the no-unsafe-argument lint rule. Co-Authored-By: Claude <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #3413
What
In the grid listing, the batch actions Batch edit and Delete were available regardless of permissions. They are now disabled unless every selected row grants the required permission (
savefor batch edit,deletefor batch delete) and is not locked.batch-actions.tsx: gate menu items via the existingcheckElementPermissionhelper (all-or-nothing over the selection)with-row-selection.tsx: capture each selected row'spermissionsandisLockedintoselectedRowsDataat selection time, so the check stays correct across pagination (row-level fields were previously discarded — only system-column values were kept)The single-delete context menu item already checks
permissions.deleteand needs no change.Workflow permissions
The row
permissionspayload is produced by the backend. A counterpart studio-backend-bundle PR merges workflow place permissions (viaWorkflow\Manager::isDeniedInWorkflow()) into that payload, so these gates automatically respect workflow state as well — no separate frontend logic needed. Fully resolving #3413 requires both PRs.Testing
Verified manually on the demo (Cars in a workflow place denying
modify/delete, plus a non-admin user with a restricted workspace): single delete hidden, batch edit/delete disabled for restricted rows and mixed selections; unrestricted control object keeps all actions.🤖 Generated with Claude Code