Repository navigation
fix: allow creating and deleting pages in Shared with me - #629
Conversation
Reviewer's GuideThis PR enables writable shared documents and their descendants to expose correctly authorized subpage actions, removes the client-side shared-parent block while preserving server and guest restrictions, refreshes shared navigation after renames, and adds unit plus live browser coverage for nested creation and privacy isolation. Sequence diagram for creating a subpage under a writable shared documentsequenceDiagram
actor Member
participant SharedRow as SharedViewItem
participant Permissions as useViewActionPermissions
participant Actions as ViewActionsPopover
participant PageOps as usePageOperations
participant Server as Page API
Member->>SharedRow: hover shared document or descendant
SharedRow->>Permissions: resolve permissions for row view
Permissions-->>SharedRow: canCreateViewActions
SharedRow-->>Member: show inline-add-page
Member->>Actions: click Add
Actions->>PageOps: createPage(parentViewId)
PageOps->>Server: POST /v2/page-view/document/default
Server-->>PageOps: created view receipt
PageOps-->>Member: open created subpage
Sequence diagram for renaming a shared descendantsequenceDiagram
actor Member
participant PageOps as usePageOperations
participant Server as Page API
participant Cache as ViewService
participant Outline as Shared navigation
Member->>PageOps: updatePageName(viewId, name)
PageOps->>Server: update-name
Server-->>PageOps: rename succeeded
PageOps->>Cache: invalidateCache(workspaceId, viewId)
PageOps->>Outline: loadOutline(workspaceId, false)
Outline-->>PageOps: refresh result
PageOps-->>Member: saved rename remains successful
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/components/app/share-with-me/ShareViewItem.tsx" line_range="114" />
<code_context>
+ const { canCreateViewActions, hasLoadedViewActionPermissions, isLoadingViewActionPermissions } =
+ useViewActionPermissions(view, canCreateChildren && (visible || addOpen));
+ const showAdd =
+ canCreateChildren && hasLoadedViewActionPermissions && !isLoadingViewActionPermissions && canCreateViewActions;
const canEdit = view.access_level && view.access_level > AccessLevel.ReadAndComment;
</code_context>
<issue_to_address>
**Permission lookup failure hides Add**
When the permission service or network fails during the lookup while the member is hovering the row, `useViewActionPermissions` catches the failed permission request, records the view as loaded without permission data, and `showAdd` stays false. The member cannot see Add again until they leave and re-enter the row to trigger another lookup.
Retry the permission lookup after failure or expose a user-visible retry action.
</issue_to_address>| const { canCreateViewActions, hasLoadedViewActionPermissions, isLoadingViewActionPermissions } = | ||
| useViewActionPermissions(view, canCreateChildren && (visible || addOpen)); | ||
| const showAdd = | ||
| canCreateChildren && hasLoadedViewActionPermissions && !isLoadingViewActionPermissions && canCreateViewActions; |
There was a problem hiding this comment.
🟡 Medium · Permission lookup failure hides Add
When the permission service or network fails during the lookup while the member is hovering the row, useViewActionPermissions catches the failed permission request, records the view as loaded without permission data, and showAdd stays false. The member cannot see Add again until they leave and re-enter the row to trigger another lookup.
Retry the permission lookup after failure or expose a user-visible retry action.
Prompt for AI agents
In `src/components/app/share-with-me/ShareViewItem.tsx` at line 114:
**Permission lookup failure hides Add**
When the permission service or network fails during the lookup while the member is hovering the row, `useViewActionPermissions` catches the failed permission request, records the view as loaded without permission data, and `showAdd` stays false. The member cannot see Add again until they leave and re-enter the row to trigger another lookup.
Retry the permission lookup after failure or expose a user-visible retry action.…ions-implementation Brings in main 05604b3 (#618, #619, #620, #623, #626, #628, #629, #630) on top of the permissions branch 23e604e. Conflicted files and how each was resolved: - src/@types/translations/en.json: key union. The branch's spaceCreation, workspacePermissions, workspaceCreation, groupSpaceAccess and changeFailure objects plus main's atomicCreation, userError and userErrorDetail objects. No key was changed on both sides; none was lost (checked by script). - src/application/db/index.ts: main's Dexie version 13 (atomic_creation) is kept exactly as released; the branch's sync_permission_blocks version and pre-scope cache cleanup move to versions 14 and 15 and also declare atomic_creation, so databases at main's 13 or the branch's 13/14 upgrade without dropping a table. Collab IndexedDB names compose both fences: restore generation outside (serverRestore.cacheKey), server/account/ workspace scope inside; listing strips the generation, then the scope. Scoped provider caches stay; the RID prefix carries the generation. Sign-out cleanup only lists and removes names of the current restore generation. closeCollabDB keeps the branch's form. - src/application/session/token.ts (+ token-storage.ts): main's login-lifetime witness (captureTokenSession, isTokenSessionCurrent, generation bumps on unowned storage changes) moves into token-storage.ts, which now owns getTokenParsed/removeStoredToken; token.ts re-exports it and keeps invalidTokenForSession and saveGoTrueAuth(token, refreshingSession). - src/application/services/js-services/http/gotrue.ts: branch refresh logic (server-keyed dedup, sibling-tab adoption, AuthSessionChangedError, definitive-failure test) plus main's witness: refreshed tokens are saved only for the captured login, and a refresh token that is no longer the stored one is rejected as AuthSessionChangedError (never signs the newer login out). Main's error-identity imports are kept. - src/application/services/js-services/http/core.ts: both imports; refresh on 401 still keeps the session on transient/session-changed failures (branch) and invalidates only the login that sent the request (main). - src/application/services/js-services/http/page-api.ts: createSpaceWithInitialPage takes main's PageCreationActivation; its assertCurrent combines the branch's captured API context (server, account, transport) with the caller's owner check. The initial page goes through addAppPage(activation), so default pages use the atomic endpoints; pending atomic outcomes keep the space; the branch's no-compensation rules for ambiguous outcomes stay. - .../http/__tests__/page-api.test.ts: branch expectations kept; their composed flows now send authored page_data (as main did) so they stay on the legacy page-view path, and the workspace check is passed as an activation. - src/components/app/hooks/usePageOperations.ts: guests are refused with the branch's coded denial and no toast; Shared with me parents are no longer refused locally (#629, the server decides). Add Page uses main's activation; Create space wraps main's page activation with the branch's workspace-scope check, and only non-atomic spaces reload the outline. - src/components/app/view-actions/ViewActionsPopover.tsx: main's shared viewActionPermissions plus the branch's single permission read, duplicate destination check and Manage space authority (never taken from a shared row). - src/components/app/view-actions/ManageSpace.tsx: branch's named change toasts; the last-owner rule is recognized from getErrorDiagnostic (main), since getErrorMessage no longer returns server text. - src/components/app/view-actions/__tests__/ManageSpace.test.tsx: branch expectation (named change toast with the client error's own copy). - src/components/app/view-actions/__tests__/CreateSpaceModal.test.tsx: branch test flows with main's user-facing messages (getErrorMessage never shows raw text: generic copy, "Error 422: ...", catalog copy for code 1008). - src/components/app/settings/billing/SettingsPanelShell.tsx: both imports (branch scroll fade, main error catalog revision). - src/components/app/settings/__tests__/Settings.billing.test.tsx: branch capability mock plus main's act import. - src/components/app/workspaces/Workspaces.tsx: both imports (branch workspace creation hooks, main's paid-plan upgrade entry). - src/components/ws/useWorkspaceRealtimeTransport.ts: leader lock and broadcast channel use the branch's server/account/workspace key inside the restore generation (both fences). Semantic decisions: 1. #626 atomic creation vs permission gating: page creation uses main's atomic endpoints and journal; the branch's destination checks, space page policy and capability hints still run before addPage, and guests get the coded denial callers turn into plain copy. 2. #618 public messages vs plain-language helpers: getErrorMessage keeps main's contract (public server/catalog copy, never raw text). Branch helpers keep their own copy for permission, gone and billing cases and read diagnostics only to recognize rules. 3. #630 restore vs storage scoping: every IndexedDB name, lock, channel and localStorage key applies the restore generation outside the branch's scope. A restore switches to fresh storage; the previous generation's copies and unsent edits are neither uploaded nor removed by the new one. 4. #629 Shared with me: members may create and delete there (server decides); guests stay refused; P applies only to pages added directly to a space. 5. #623 billing: the upgrade entry hides for paid plans inside the branch's redesigned workspace menu; settings panels keep the branch layout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- ShareViewItem (main): the shared Add menu now renders the branch's permission hook and duplicate-destination check, so the suite mocks the app event emitter and keeps the duplicate check (a More-menu action that is mocked out) out of the permission calls it inspects. - usePageOperations.publish (main): an authored initial page still uses the composed refresh path, but Create space now always passes the branch's workspace check as the activation. A new test stops an authored space once its workspace is left while the request is pending. - usePageOperations.addPageRefusal (branch): members creating under Shared with me are no longer refused locally (main #629, the server decides); guests still get the coded denial without a toast, also there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Description
A workspace member with Full Access to a document inside someone else's private space could read and edit it, but Web blocked creating and deleting its subpages under Shared with me. The sidebar omitted those actions, and the page-operation hooks rejected shared pages before contacting the server.
Shared documents now expose Add when the server confirms write access. Shared pages expose Delete when the server confirms Full Access. Each action targets the clicked page, including nested descendants, and uses the existing creation or Move to Trash flow. Guest restrictions and server authorization remain enforced.
Reproduction and history
Create an owner-only private space, invite a regular workspace member, and share only one document with Full Access. Before this fix, the member's shared sidebar has no Add/Delete actions, and the hooks reject shared targets even when the server permits the operation.
The shared-page rejection originated in 3e9655502ca085d7b10a4db1aa8170ed734bbe48 (
chore: support request access (#84)), authored by Kilu.He on 2025-10-13. This identifies the blocked path, not when C12 first observed it.Validation
pnpm run lintpassed, including error-catalog validation, Web TypeScript and ESLint.git diff --checkpassed.The browser test uses the existing
grantTestWorkspaceProSubscriptionfixture and CI'sAPPFLOWY_TEST_POSTGRES_CONTAINER; optionalAPPFLOWY_TEST_POSTGRES_DATABASEselects the disposable local database. It creates fresh test accounts/workspaces and is not a production diagnostic. Live verification used Cloud source5c2e155e298e38d3686fe0080d212f9bc2e61185.Checklist
Feature Preview
Playwright captures shared-menu, descendant-creation and post-deletion screenshots as test artifacts. This PR changes Web only.
Summary by Sourcery
Enable permission-aware creation, editing, and deletion of shared-page descendants under Shared with me while preserving access isolation and existing server authorization.
New Features:
Bug Fixes:
Enhancements:
Tests: