From 7473d5b43777fbbbcbbdb1932ff598414da89131 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 27 May 2026 21:33:33 -0400 Subject: [PATCH] fix: Allow reordering subdocuments with document-only access (#12493) * fix: Allow reordering subdocuments with document-only access When a user has "Manage" (or any move-eligible) permission on a parent document but no access to its collection, the sidebar drop cursors were hidden because they gated on collection.isManualSort, and the move handler bailed out because it built the payload from collection.id. Fall back to the document's own collectionId and the move policy so the reorder UX works for sourced document memberships. * fix: Structure not refetched parentDocumentId not provided --- .../Sidebar/components/DocumentLink.tsx | 26 +++++++---- .../Sidebar/components/SharedWithMeLink.tsx | 1 + app/stores/DocumentsStore.ts | 10 +++++ server/routes/api/documents/documents.test.ts | 44 +++++++++++++++++++ 4 files changed, 73 insertions(+), 8 deletions(-) diff --git a/app/components/Sidebar/components/DocumentLink.tsx b/app/components/Sidebar/components/DocumentLink.tsx index 1d69007b25..1f09adeea6 100644 --- a/app/components/Sidebar/components/DocumentLink.tsx +++ b/app/components/Sidebar/components/DocumentLink.tsx @@ -6,7 +6,7 @@ import { useHistory } from "react-router-dom"; import scrollIntoView from "scroll-into-view-if-needed"; import Icon from "@shared/components/Icon"; import type { NavigationNode } from "@shared/types"; -import { UserPreference } from "@shared/types"; +import { DocumentPermission, UserPreference } from "@shared/types"; import { ProsemirrorHelper } from "@shared/utils/ProsemirrorHelper"; import { sortNavigationNodes } from "@shared/utils/collections"; import type Collection from "~/models/Collection"; @@ -304,14 +304,17 @@ const DocumentLinkInner = observer(function DocumentLinkInner({ const [{ isOverReparent, canDropToReparent }, dropToReparent] = useDropToReparentDocument(node, handleExpand, parentRef); + // Fall back so document-only access (e.g. "Manage" on a parent) can reorder. + const moveCollectionId = collection?.id ?? document?.collectionId; + const [{ isOverReorder: isOverReorderAbove }, dropToReorderAbove] = useDropToReorderDocument(node, collection, (item) => { - if (!collection) { + if (!moveCollectionId) { return; } return { documentId: item.id, - collectionId: collection.id, + collectionId: moveCollectionId, parentDocumentId: parentId, index, }; @@ -319,20 +322,20 @@ const DocumentLinkInner = observer(function DocumentLinkInner({ const [{ isOverReorder, isDraggingAnyDocument }, dropToReorder] = useDropToReorderDocument(node, collection, (item) => { - if (!collection) { + if (!moveCollectionId) { return; } if (expansion.isExpanded(node.id)) { return { documentId: item.id, - collectionId: collection.id, + collectionId: moveCollectionId, parentDocumentId: node.id, index: 0, }; } return { documentId: item.id, - collectionId: collection.id, + collectionId: moveCollectionId, parentDocumentId: parentId, index: index + 1, }; @@ -389,8 +392,15 @@ const DocumentLinkInner = observer(function DocumentLinkInner({ /> ) : undefined; + // Without a collection we can't read isManualSort; fall back to the shared + // membership's permission, which is the same for every descendant. + const canReorderHere = collection + ? collection.isManualSort + : membership?.permission === DocumentPermission.Admin || + membership?.permission === DocumentPermission.ReadWrite; + const cursorBefore = - isDraggingAnyDocument && collection?.isManualSort && index === 0 ? ( + isDraggingAnyDocument && canReorderHere && index === 0 ? ( ) : undefined; diff --git a/app/components/Sidebar/components/SharedWithMeLink.tsx b/app/components/Sidebar/components/SharedWithMeLink.tsx index f1a53bce5c..276afdf5da 100644 --- a/app/components/Sidebar/components/SharedWithMeLink.tsx +++ b/app/components/Sidebar/components/SharedWithMeLink.tsx @@ -210,6 +210,7 @@ function SharedWithMeLink({ membership, depth = 0 }: Props) { isDraft={childNode.isDraft} depth={2} index={index} + parentId={document.id} /> ))} diff --git a/app/stores/DocumentsStore.ts b/app/stores/DocumentsStore.ts index 9c150ddee2..8c5e2d6821 100644 --- a/app/stores/DocumentsStore.ts +++ b/app/stores/DocumentsStore.ts @@ -503,6 +503,16 @@ export default class DocumentsStore extends Store { invariant(res?.data, "Data not available"); res.data.documents.forEach(this.add); this.addPolicies(res.policies); + + // The websocket "documents.move" event is only broadcast to the + // collection channel, so users with document-only access never receive + // it. Refresh the affected membership tree locally so the sidebar + // reflects the new structure. + const membership = + this.rootStore.userMemberships.getByDocumentId(documentId); + if (membership) { + await membership.fetchDocuments({ force: true }); + } } finally { this.movingDocumentId = undefined; } diff --git a/server/routes/api/documents/documents.test.ts b/server/routes/api/documents/documents.test.ts index c5ba1b0937..fa98dce801 100644 --- a/server/routes/api/documents/documents.test.ts +++ b/server/routes/api/documents/documents.test.ts @@ -2672,6 +2672,50 @@ describe("#documents.move", () => { }); expect(res.status).toEqual(403); }); + + it("should allow reordering subdocuments with document-only admin access", async () => { + const owner = await buildUser(); + const collection = await buildCollection({ + teamId: owner.teamId, + userId: owner.id, + permission: null, + }); + const parent = await buildDocument({ + teamId: owner.teamId, + userId: owner.id, + collectionId: collection.id, + }); + const childA = await buildDocument({ + teamId: owner.teamId, + userId: owner.id, + collectionId: collection.id, + parentDocumentId: parent.id, + }); + const childB = await buildDocument({ + teamId: owner.teamId, + userId: owner.id, + collectionId: collection.id, + parentDocumentId: parent.id, + }); + + const user = await buildUser({ teamId: owner.teamId }); + await UserMembership.create({ + documentId: parent.id, + userId: user.id, + createdById: owner.id, + permission: DocumentPermission.Admin, + }); + + const res = await server.post("/api/documents.move", user, { + body: { + id: childB.id, + parentDocumentId: parent.id, + index: 0, + }, + }); + expect(res.status).toEqual(200); + expect(childA).toBeTruthy(); + }); }); describe("#documents.restore", () => {