-
Notifications
You must be signed in to change notification settings - Fork 2
Add bulk page operations (move, copy, delete) with multi-select UI #338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…drives - Add useMultiSelectStore for tracking selection state and mode - Update PageTreeItem with checkbox UI when in multi-select mode - Add context menu options: "Select multiple", "Move to...", "Copy to..." - Create MovePageDialog for moving pages to another drive with folder picker - Create CopyPageDialog for copying pages with optional child pages - Create MultiSelectToolbar showing selected count with bulk action buttons - Add API endpoints: - POST /api/pages/tree - fetch page tree for drive selection - POST /api/pages/bulk-move - move multiple pages across drives - POST /api/pages/bulk-copy - copy pages with recursive children - DELETE /api/pages/bulk-delete - bulk trash pages https://claude.ai/code/session_01CEWo9vztg3kJy4amDGUagP
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f24728b032
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Get the max position in target parent | ||
| const lastPage = await db.query.pages.findFirst({ | ||
| where: and( | ||
| eq(pages.driveId, targetDriveId), | ||
| targetParentId ? eq(pages.parentId, targetParentId) : eq(pages.parentId, ''), | ||
| eq(pages.isTrashed, false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use NULL parent lookup for root-level copy position
The position lookup treats a root-level destination as parentId = '', but pages.parentId is nullable and root pages are queried elsewhere with isNull(pages.parentId) (e.g., create flow). With the empty-string comparison, lastPage will be null even when there are root pages, so bulk-copy to the root starts at position 1 and can collide with existing ordering. This will surface when copying pages into the drive root and result in unstable ordering.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 6c790a0. Changed from eq(pages.parentId, '') to isNull(pages.parentId) for root-level position lookup. This now correctly queries for pages with NULL parentId when copying to the drive root.
| // Get the max position in target parent | ||
| const lastPage = await db.query.pages.findFirst({ | ||
| where: and( | ||
| eq(pages.driveId, targetDriveId), | ||
| targetParentId ? eq(pages.parentId, targetParentId) : eq(pages.parentId, ''), | ||
| eq(pages.isTrashed, false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use NULL parent lookup for root-level move position
The max-position query uses parentId = '' when targetParentId is null. Since root pages are stored with NULL parentId (see other page queries that use isNull(pages.parentId)), this query skips existing root pages and assigns position 1. Moving pages to the root can therefore create duplicate or out-of-order positions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 6c790a0. Changed from eq(pages.parentId, '') to isNull(pages.parentId) for root-level position lookup. This now correctly queries for pages with NULL parentId when moving to the drive root.
| // Track affected drives for cache invalidation | ||
| const affectedDriveIds = new Set<string>(); | ||
| const hasAIChatPages = sourcePages.some(p => p.type === 'AI_CHAT'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalidate agent cache for trashed AI_CHAT children
hasAIChatPages only checks the explicitly selected pages, but when trashChildren is true you also trash descendants. If any child page is AI_CHAT, the agent awareness cache will not be invalidated, leaving stale agents until another invalidation happens. This shows up when bulk deleting a folder containing AI_CHAT pages without selecting those pages directly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 6c790a0. The trashChildrenRecursively function now returns a boolean indicating whether any AI_CHAT pages were trashed among descendants. The cache invalidation now correctly triggers for all affected AI_CHAT pages, including those in child hierarchies.
- Fix NULL parent lookup for root-level operations (use isNull instead of empty string) - Fix role check to use valid MemberRole values (OWNER|ADMIN instead of EDITOR) - Fix agent cache invalidation to check AI_CHAT pages in trashed children - Fix TypeScript errors in dialog components (use fetchWithAuth for Response) - Simplify websocket event payloads to match PageEventPayload interface Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
This PR adds comprehensive bulk page operations to PageSpace, enabling users to efficiently manage multiple pages at once. It includes three new API endpoints for bulk operations and a multi-select UI with a toolbar for executing these operations.
Key Changes
API Endpoints
POST /api/pages/bulk-copy- Copy multiple pages to a target drive/folder with optional recursive child copyingPOST /api/pages/bulk-move- Move multiple pages across drives with circular reference validation and recursive drive ID updates for childrenDELETE /api/pages/bulk-delete- Trash multiple pages with options to recursively trash or orphan childrenPOST /api/pages/tree- Fetch the page tree structure for a drive (used by dialogs for destination selection)UI Components
MultiSelectToolbar- Sticky toolbar that appears when pages are selected, providing quick access to move, copy, and delete actionsMovePageDialog- Modal for selecting destination drive and folder when moving pagesCopyPageDialog- Modal for selecting destination drive and folder when copying pages, with option to include child pagesPageTreecomponent to integrate the multi-select toolbarImplementation Details
Technical Notes
canUserViewPage,canUserEditPage,canUserDeletePage)pageTreeCache,agentAwarenessCache)https://claude.ai/code/session_01CEWo9vztg3kJy4amDGUagP