refactor(resources): converge Files onto the shared drag hook and batch bulk authorization - #6748
Conversation
…ch bulk authorization Files kept a 280-line copy of the foldered-list drag logic because it also accepts OS file drops. The copies had already drifted, so the external drop becomes an option on the shared hook and the copy goes away. - Add `externalDrop` to `useFolderRowDragDrop`: folder rows highlight and spring open for an OS file drag exactly as for a move, while the body and breadcrumb decline so the page-level upload overlay owns those regions - Collapse the three drop-active booleans into one `ActiveDropTarget` union, so exactly one affordance is armed by construction rather than by hand-clearing - Keep drop-target writes identity-stable so `dragover` does not re-render the list on every event - Give each list its own drag MIME again, restoring the cross-surface isolation `drag-payload.ts` documents - Let a folder spring open more than once per drag, so a drag can walk back out through the breadcrumb and descend again; the guard against re-entering the folder already on screen moves to `useSpringNavigation`, the only layer that can state it - Resolve each bulk item against the workspace context the batch already holds, and memoize the effective-permission lookup for the batch, replacing two invariant queries per item - Fill the drop target at `--surface-active`: `--surface-4` is the button-base token and is lighter than hover in light mode, so the strongest row state read the faintest
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Drop affordance state is a single Bulk move/delete for tables and knowledge bases resolves each item with UI: Reviewed by Cursor Bugbot for commit 6ab3c6f. Configure here. |
Greptile SummaryThe PR consolidates Files drag-and-drop behavior into the shared folder-list hook, improves spring navigation and drop-target state, restores per-surface drag payload isolation, and optimizes bulk resource context resolution while retaining per-item authorization checks.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/components/folders/use-folder-row-drag-drop.ts | Extends the shared drag hook for surface-specific MIME payloads, external file drops, and mutually exclusive drop-target state. |
| apps/sim/app/workspace/[workspaceId]/components/folders/use-spring-loaded-folder.ts | Allows repeated spring-opening during one drag while retaining first-navigation history semantics. |
| apps/sim/app/workspace/[workspaceId]/components/folders/use-spring-navigation.ts | Prevents navigation to the folder already displayed and preserves drag-origin restoration behavior. |
| apps/sim/app/workspace/[workspaceId]/files/files.tsx | Replaces the Files-specific drag implementation with the shared hook and integrates OS-file upload handling. |
| apps/sim/lib/knowledge/application/bulk.ts | Reuses canonical workspace context across the batch while retaining a fresh authorization check for each knowledge item. |
| apps/sim/lib/table/application/bulk.ts | Reuses canonical workspace context across the batch while retaining a fresh authorization check for each table item. |
| apps/sim/lib/knowledge/application/contexts.ts | Adds resource resolution against an already-established workspace context without carrying an authorization verdict. |
| apps/sim/lib/table/application/context.ts | Adds table resolution against an already-established workspace context without carrying an authorization verdict. |
| packages/emcn/src/components/chip/chip-chrome.ts | Updates the drop-target surface styling to use the stronger active-surface token. |
Sequence Diagram
sequenceDiagram
participant Request as Bulk request
participant Bulk as Bulk operation
participant Context as Canonical workspace context
participant Auth as Workspace authorization
participant DB as Database
Request->>Bulk: Move or delete multiple items
Bulk->>Context: Resolve workspace context once
loop Each item
Bulk->>DB: Resolve active resource in workspace
Bulk->>Auth: Authorize current item
Auth->>DB: Read current effective permission
Auth-->>Bulk: Allow or reject
Bulk->>DB: Commit authorized item
end
Reviews (2): Last reviewed commit: "fix(resources): dismiss the upload overl..." | Re-trigger Greptile
…eck permission per item The drag hook stops propagation on a drop it handles, so the page-level handler that cleared the upload overlay never ran and the chrome stayed up over the finished upload. Both consuming paths now share one dismissal. Drop the batch permission memo: each item in a bulk move or delete commits independently, so reusing one allow verdict let a revocation part-way through a batch go unseen by the remaining items. The workspace context is still resolved once per batch, which was the larger saving.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6ab3c6f. Configure here.
Summary
externalDropoption on the shared hook and the copy is gone — folder rows highlight and spring open for an OS file drag exactly as for a move, while the body and breadcrumb decline so the page-level upload overlay owns those regionsActiveDropTargetunion, so exactly one affordance is armed by construction instead of by hand-clearing the other twodragoverno longer re-renders the list on every eventdrag-payload.tsdocuments is true againuseSpringNavigation, the only layer that can express it--surface-active;--surface-4is the button-base token and is lighter than hover in light mode, so the strongest row state painted the faintest fillType of Change
Testing
bun run type-check,bun run check:audits(27/27), and 4013 tests pass.New coverage for the drag lifecycle: re-entering a folder through the breadcrumb, refusing to re-open the folder already on screen, cancelling a pending open when the drag moves onto the current folder, one-hop return to origin after a multi-level round trip, staying put when the round trip ends in a drop, and the next drag taking a fresh origin. Plus unit tests for the permission memo, including key separation and re-query after a rejection.
Not browser-tested — the convergence changes which code path Files runs, so the drag interactions are worth a manual pass.
Checklist