From d8b4fef554b7ccd2893064fa763d6ed66b20c137 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sun, 30 Apr 2023 09:38:47 -0400 Subject: [PATCH] feat: Collection admins (#5273 * Split permissions for reading documents from updating collection * fix: Admins should have collection read permission, tests * tsc * Add admin option to permission selector * Combine publish and create permissions, update -> createDocuments where appropriate * Plural -> singular * wip * Quick version of collection structure loading, will revisit * Remove documentIds method * stash * fixing tests to account for admin creation * Add self-hosted migration * fix: Allow groups to have admin permission * Prefetch collection documents * fix: Document explorer (move/publish) not working with async documents * fix: Cannot re-parent document to collection by drag and drop * fix: Cannot drag to import into collection item without admin permission * Remove unused isEditor getter --- app/components/AuthenticatedLayout.tsx | 2 +- app/components/DocumentExplorer.tsx | 51 ++++-- app/components/DocumentsLoader.tsx | 21 +++ .../Sidebar/components/CollectionLink.tsx | 11 +- .../components/CollectionLinkChildren.tsx | 47 +++-- .../Sidebar/components/DropToImport.tsx | 16 +- .../components/PlaceholderCollections.tsx | 4 +- .../components/useCollectionDocuments.ts | 4 +- app/components/WebsocketProvider.tsx | 6 +- app/hooks/useCollectionTrees.ts | 4 +- app/menus/CollectionMenu.tsx | 5 +- app/menus/DocumentMenu.tsx | 2 +- app/models/BaseModel.ts | 1 - app/models/Collection.ts | 71 ++++++-- app/models/CollectionGroupMembership.ts | 8 +- app/models/Membership.ts | 8 +- app/scenes/Collection.tsx | 4 +- app/scenes/Collection/Empty.tsx | 6 +- .../CollectionGroupMemberListItem.tsx | 79 +++------ .../InputMemberPermissionSelect.tsx | 48 +++++ .../components/MemberListItem.tsx | 32 +--- app/scenes/CollectionPermissions/index.tsx | 2 +- app/scenes/DocumentMove.tsx | 14 +- app/stores/CollectionsStore.ts | 23 ++- app/stores/DocumentsStore.ts | 2 +- .../20230429005039-collection-admins.js | 56 ++++++ server/models/Collection.ts | 37 ++-- server/models/Document.ts | 19 ++ server/models/User.ts | 5 +- server/policies/collection.test.ts | 91 ++++++++-- server/policies/collection.ts | 150 ++++++++-------- server/policies/document.ts | 164 ++++++------------ server/presenters/collection.ts | 4 +- .../queues/processors/WebsocketsProcessor.ts | 30 ++-- .../CollectionCreatedNotificationsTask.ts | 2 +- server/queues/tasks/ExportJSONTask.ts | 2 +- server/routes/api/collections.test.ts | 129 +++++++------- server/routes/api/collections.ts | 19 +- server/routes/api/documents/documents.test.ts | 106 +++++++---- server/routes/api/documents/documents.ts | 18 +- server/routes/api/documents/schema.ts | 4 +- server/routes/api/revisions.test.ts | 8 +- server/routes/api/shares.test.ts | 18 +- shared/types.ts | 1 + 44 files changed, 799 insertions(+), 535 deletions(-) create mode 100644 app/components/DocumentsLoader.tsx create mode 100644 app/scenes/CollectionPermissions/components/InputMemberPermissionSelect.tsx create mode 100644 server/migrations/20230429005039-collection-admins.js diff --git a/app/components/AuthenticatedLayout.tsx b/app/components/AuthenticatedLayout.tsx index fc4c29fff..92b422a3c 100644 --- a/app/components/AuthenticatedLayout.tsx +++ b/app/components/AuthenticatedLayout.tsx @@ -61,7 +61,7 @@ const AuthenticatedLayout: React.FC = ({ children }) => { return; } const { activeCollectionId } = ui; - if (!activeCollectionId || !can.update) { + if (!activeCollectionId || !can.createDocument) { return; } history.push(newDocumentPath(activeCollectionId)); diff --git a/app/components/DocumentExplorer.tsx b/app/components/DocumentExplorer.tsx index 4a0ccdab7..9790b494f 100644 --- a/app/components/DocumentExplorer.tsx +++ b/app/components/DocumentExplorer.tsx @@ -48,7 +48,6 @@ function DocumentExplorer({ onSubmit, onSelect, items }: Props) { const [initialScrollOffset, setInitialScrollOffset] = React.useState( 0 ); - const [nodes, setNodes] = React.useState([]); const [activeNode, setActiveNode] = React.useState(0); const [expandedNodes, setExpandedNodes] = React.useState([]); const [itemRefs, setItemRefs] = React.useState< @@ -79,19 +78,6 @@ function DocumentExplorer({ onSubmit, onSelect, items }: Props) { setActiveNode(0); }, [searchTerm]); - React.useEffect(() => { - let results; - - if (searchTerm) { - results = searchIndex.search(searchTerm); - } else { - results = items.filter((item) => item.type === "collection"); - } - - setInitialScrollOffset(0); - setNodes(results); - }, [searchTerm, items, searchIndex]); - React.useEffect(() => { setItemRefs((itemRefs) => map( @@ -105,6 +91,22 @@ function DocumentExplorer({ onSubmit, onSelect, items }: Props) { onSelect(selectedNode); }, [selectedNode, onSelect]); + function getNodes() { + function includeDescendants(item: NavigationNode): NavigationNode[] { + return expandedNodes.includes(item.id) + ? [item, ...descendants(item, 1).flatMap(includeDescendants)] + : [item]; + } + + return searchTerm + ? searchIndex.search(searchTerm) + : items + .filter((item) => item.type === "collection") + .flatMap(includeDescendants); + } + + const nodes = getNodes(); + const scrollNodeIntoView = React.useCallback( (node: number) => { if (itemRefs[node] && itemRefs[node].current) { @@ -130,7 +132,7 @@ function DocumentExplorer({ onSubmit, onSelect, items }: Props) { scrollOffset: number; }; const itemsHeight = itemCount * itemSize; - return itemsHeight < height ? 0 : scrollOffset; + return itemsHeight < Number(height) ? 0 : scrollOffset; } return 0; }; @@ -145,7 +147,6 @@ function DocumentExplorer({ onSubmit, onSelect, items }: Props) { const newNodes = filter(nodes, (node) => !includes(descendantIds, node.id)); const scrollOffset = calculateInitialScrollOffset(newNodes.length); setInitialScrollOffset(scrollOffset); - setNodes(newNodes); }; const expand = (node: number) => { @@ -156,9 +157,18 @@ function DocumentExplorer({ onSubmit, onSelect, items }: Props) { newNodes.splice(node + 1, 0, ...descendants(nodes[node], 1)); const scrollOffset = calculateInitialScrollOffset(newNodes.length); setInitialScrollOffset(scrollOffset); - setNodes(newNodes); }; + React.useEffect(() => { + collections.orderedData + .filter( + (collection) => expandedNodes.includes(collection.id) || searchTerm + ) + .forEach((collection) => { + collection.fetchDocuments(); + }); + }, [collections, expandedNodes, searchTerm]); + const isSelected = (node: number) => { if (!selectedNode) { return false; @@ -169,7 +179,8 @@ function DocumentExplorer({ onSubmit, onSelect, items }: Props) { return selectedNodeId === nodeId; }; - const hasChildren = (node: number) => nodes[node].children.length > 0; + const hasChildren = (node: number) => + nodes[node].children.length > 0 || nodes[node].type === "collection"; const toggleCollapse = (node: number) => { if (!hasChildren(node)) { @@ -219,7 +230,7 @@ function DocumentExplorer({ onSubmit, onSelect, items }: Props) { } else if (doc?.isStarred) { icon = ; } else { - icon = ; + icon = ; } path = ancestors(node) @@ -281,12 +292,14 @@ function DocumentExplorer({ onSubmit, onSelect, items }: Props) { switch (ev.key) { case "ArrowDown": { ev.preventDefault(); + ev.stopPropagation(); setActiveNode(next()); scrollNodeIntoView(next()); break; } case "ArrowUp": { ev.preventDefault(); + ev.stopPropagation(); if (activeNode === 0) { focusSearchInput(); } else { diff --git a/app/components/DocumentsLoader.tsx b/app/components/DocumentsLoader.tsx new file mode 100644 index 000000000..7e37837e0 --- /dev/null +++ b/app/components/DocumentsLoader.tsx @@ -0,0 +1,21 @@ +import { observer } from "mobx-react"; +import * as React from "react"; +import Collection from "~/models/Collection"; + +type Props = { + enabled: boolean; + collection: Collection; + children: React.ReactNode; +}; + +function DocumentsLoader({ collection, enabled, children }: Props) { + React.useEffect(() => { + if (enabled) { + void collection.fetchDocuments(); + } + }, [collection, enabled]); + + return <>{children}; +} + +export default observer(DocumentsLoader); diff --git a/app/components/Sidebar/components/CollectionLink.tsx b/app/components/Sidebar/components/CollectionLink.tsx index fa78d3d8f..7890351db 100644 --- a/app/components/Sidebar/components/CollectionLink.tsx +++ b/app/components/Sidebar/components/CollectionLink.tsx @@ -44,7 +44,7 @@ const CollectionLink: React.FC = ({ const { dialogs, documents, collections } = useStores(); const [menuOpen, handleMenuOpen, handleMenuClose] = useBoolean(); const [isEditing, setIsEditing] = React.useState(false); - const canUpdate = usePolicy(collection).update; + const can = usePolicy(collection); const { t } = useTranslation(); const history = useHistory(); const inStarredSection = useStarredContext(); @@ -105,7 +105,7 @@ const CollectionLink: React.FC = ({ } } }, - canDrop: () => canUpdate, + canDrop: () => can.createDocument, collect: (monitor) => ({ isOver: !!monitor.isOver({ shallow: true, @@ -118,6 +118,10 @@ const CollectionLink: React.FC = ({ setIsEditing(isEditing); }, []); + const handleMouseEnter = React.useCallback(() => { + void collection.fetchDocuments(); + }, [collection]); + const context = useActionContext({ activeCollectionId: collection.id, inStarredSection, @@ -134,6 +138,7 @@ const CollectionLink: React.FC = ({ }} expanded={expanded} onDisclosureClick={onDisclosureClick} + onMouseEnter={handleMouseEnter} icon={ } @@ -147,7 +152,7 @@ const CollectionLink: React.FC = ({ title={collection.name} onSubmit={handleTitleChange} onEditing={handleTitleEditing} - canUpdate={canUpdate} + canUpdate={can.update} /> } exact={false} diff --git a/app/components/Sidebar/components/CollectionLinkChildren.tsx b/app/components/Sidebar/components/CollectionLinkChildren.tsx index c1f3d7854..c303c405d 100644 --- a/app/components/Sidebar/components/CollectionLinkChildren.tsx +++ b/app/components/Sidebar/components/CollectionLinkChildren.tsx @@ -2,8 +2,12 @@ import { observer } from "mobx-react"; import * as React from "react"; import { useDrop } from "react-dnd"; import { useTranslation } from "react-i18next"; +import styled from "styled-components"; import Collection from "~/models/Collection"; import Document from "~/models/Document"; +import DelayedMount from "~/components/DelayedMount"; +import DocumentsLoader from "~/components/DocumentsLoader"; +import { ResizingHeightContainer } from "~/components/ResizingHeightContainer"; import usePolicy from "~/hooks/usePolicy"; import useStores from "~/hooks/useStores"; import useToasts from "~/hooks/useToasts"; @@ -11,6 +15,7 @@ import DocumentLink from "./DocumentLink"; import DropCursor from "./DropCursor"; import EmptyCollectionPlaceholder from "./EmptyCollectionPlaceholder"; import Folder from "./Folder"; +import PlaceholderCollections from "./PlaceholderCollections"; import { DragObject } from "./SidebarLink"; import useCollectionDocuments from "./useCollectionDocuments"; @@ -63,28 +68,42 @@ function CollectionLinkChildren({ return ( - {isDraggingAnyDocument && can.update && manualSort && ( + {isDraggingAnyDocument && can.createDocument && manualSort && ( )} - {childDocuments.map((node, index) => ( - - ))} - {childDocuments.length === 0 && } + + {!childDocuments && ( + + + + + + )} + {childDocuments?.map((node, index) => ( + + ))} + {childDocuments?.length === 0 && } + ); } +const Loading = styled(PlaceholderCollections)` + margin-left: 44px; + min-height: 90px; +`; + export default observer(CollectionLinkChildren); diff --git a/app/components/Sidebar/components/DropToImport.tsx b/app/components/Sidebar/components/DropToImport.tsx index 66e574b8e..55bbc6960 100644 --- a/app/components/Sidebar/components/DropToImport.tsx +++ b/app/components/Sidebar/components/DropToImport.tsx @@ -26,10 +26,14 @@ function DropToImport({ disabled, children, collectionId, documentId }: Props) { collectionId, documentId ); - const targetId = collectionId || documentId; - invariant(targetId, "Must provide either collectionId or documentId"); + invariant( + collectionId || documentId, + "Must provide either collectionId or documentId" + ); + + const canCollection = usePolicy(collectionId); + const canDocument = usePolicy(documentId); - const can = usePolicy(targetId); const handleRejection = React.useCallback(() => { showToast( t("Document not supported – try Markdown, Plain text, HTML, or Word"), @@ -39,7 +43,11 @@ function DropToImport({ disabled, children, collectionId, documentId }: Props) { ); }, [t, showToast]); - if (disabled || !can.update) { + if ( + disabled || + (collectionId && !canCollection.createDocument) || + (documentId && !canDocument.createChildDocument) + ) { return children; } diff --git a/app/components/Sidebar/components/PlaceholderCollections.tsx b/app/components/Sidebar/components/PlaceholderCollections.tsx index 8fe5c46dd..49c732f59 100644 --- a/app/components/Sidebar/components/PlaceholderCollections.tsx +++ b/app/components/Sidebar/components/PlaceholderCollections.tsx @@ -2,9 +2,9 @@ import * as React from "react"; import styled from "styled-components"; import PlaceholderText from "~/components/PlaceholderText"; -function PlaceholderCollections() { +function PlaceholderCollections(props: React.HTMLAttributes) { return ( - + diff --git a/app/components/Sidebar/components/useCollectionDocuments.ts b/app/components/Sidebar/components/useCollectionDocuments.ts index 42af1eedf..62734ce11 100644 --- a/app/components/Sidebar/components/useCollectionDocuments.ts +++ b/app/components/Sidebar/components/useCollectionDocuments.ts @@ -8,8 +8,8 @@ export default function useCollectionDocuments( activeDocument: Document | undefined ) { return React.useMemo(() => { - if (!collection) { - return []; + if (!collection?.sortedDocuments) { + return undefined; } const insertDraftDocument = diff --git a/app/components/WebsocketProvider.tsx b/app/components/WebsocketProvider.tsx index 00057e587..32446cb40 100644 --- a/app/components/WebsocketProvider.tsx +++ b/app/components/WebsocketProvider.tsx @@ -196,7 +196,7 @@ class WebsocketProvider extends React.Component { } try { - await collections.fetch(collectionId, { + await collection?.fetchDocuments({ force: true, }); } catch (err) { @@ -293,6 +293,10 @@ class WebsocketProvider extends React.Component { collections.add(event); }); + this.socket.on("collections.update", (event: PartialWithId) => { + collections.add(event); + }); + this.socket.on( "collections.delete", action((event: WebsocketEntityDeletedEvent) => { diff --git a/app/hooks/useCollectionTrees.ts b/app/hooks/useCollectionTrees.ts index fe7ea419f..7d8ed5784 100644 --- a/app/hooks/useCollectionTrees.ts +++ b/app/hooks/useCollectionTrees.ts @@ -73,9 +73,11 @@ export default function useCollectionTrees(): NavigationNode[] { return addParent(addCollectionId(addDepth(addType(collectionNode)))); }; + const key = collections.orderedData.map((o) => o.documents?.length).join("-"); const collectionTrees = React.useMemo( () => collections.orderedData.map(getCollectionTree), - [collections.orderedData] + // eslint-disable-next-line react-hooks/exhaustive-deps + [collections.orderedData, key] ); return collectionTrees; diff --git a/app/menus/CollectionMenu.tsx b/app/menus/CollectionMenu.tsx index 75a7a6bf7..18ff7be33 100644 --- a/app/menus/CollectionMenu.tsx +++ b/app/menus/CollectionMenu.tsx @@ -200,14 +200,14 @@ function CollectionMenu({ { type: "button", title: t("New document"), - visible: can.update, + visible: can.createDocument, onClick: handleNewDocument, icon: , }, { type: "button", title: t("Import document"), - visible: can.update, + visible: can.createDocument, onClick: handleImportDocument, icon: , }, @@ -261,6 +261,7 @@ function CollectionMenu({ collection, can.unstar, can.star, + can.createDocument, can.update, can.delete, handleStar, diff --git a/app/menus/DocumentMenu.tsx b/app/menus/DocumentMenu.tsx index 67d8d86da..2c6117706 100644 --- a/app/menus/DocumentMenu.tsx +++ b/app/menus/DocumentMenu.tsx @@ -132,7 +132,7 @@ function DocumentMenu({ ...collections.orderedData.reduce((filtered, collection) => { const can = policies.abilities(collection.id); - if (can.update) { + if (can.createDocument) { filtered.push({ type: "button", onClick: (ev) => diff --git a/app/models/BaseModel.ts b/app/models/BaseModel.ts index 6811399a2..265644c8d 100644 --- a/app/models/BaseModel.ts +++ b/app/models/BaseModel.ts @@ -60,7 +60,6 @@ export default abstract class BaseModel { }; updateFromJson = (data: any) => { - // const isNew = !data.id && !this.id && this.isNew; set(this, { ...data, isNew: false }); this.persistedAttributes = this.toAPI(); }; diff --git a/app/models/Collection.ts b/app/models/Collection.ts index 932c9b278..f65474413 100644 --- a/app/models/Collection.ts +++ b/app/models/Collection.ts @@ -1,5 +1,5 @@ import { trim } from "lodash"; -import { action, computed, observable } from "mobx"; +import { action, computed, observable, runInAction } from "mobx"; import { CollectionPermission, FileOperationFormat, @@ -18,6 +18,8 @@ export default class Collection extends ParanoidModel { @observable isSaving: boolean; + isFetching = false; + @Field @observable id: string; @@ -57,32 +59,34 @@ export default class Collection extends ParanoidModel { direction: "asc" | "desc"; }; - documents: NavigationNode[]; + @observable + documents?: NavigationNode[]; url: string; urlId: string; @computed - get isEmpty(): boolean { + get isEmpty(): boolean | undefined { + if (!this.documents) { + return undefined; + } + return ( this.documents.length === 0 && this.store.rootStore.documents.inCollection(this.id).length === 0 ); } - @computed - get documentIds(): string[] { - const results: string[] = []; - - const travelNodes = (nodes: NavigationNode[]) => - nodes.forEach((node) => { - results.push(node.id); - travelNodes(node.children); - }); - - travelNodes(this.documents); - return results; + /** + * Convenience method to return if a collection is considered private. + * This means that a membership is required to view it rather than just being + * a workspace member. + * + * @returns boolean + */ + get isPrivate(): boolean { + return !this.permission; } @computed @@ -98,10 +102,35 @@ export default class Collection extends ParanoidModel { } @computed - get sortedDocuments() { + get sortedDocuments(): NavigationNode[] | undefined { + if (!this.documents) { + return undefined; + } return sortNavigationNodes(this.documents, this.sort); } + fetchDocuments = async (options?: { force: boolean }) => { + if (this.isFetching) { + return; + } + if (this.documents && options?.force !== true) { + return; + } + + try { + this.isFetching = true; + const { data } = await client.post("/collections.documents", { + id: this.id, + }); + + runInAction("Collection#fetchDocuments", () => { + this.documents = data; + }); + } finally { + this.isFetching = false; + } + }; + /** * Updates the document identified by the given id in the collection in memory. * Does not update the document in the database. @@ -110,6 +139,10 @@ export default class Collection extends ParanoidModel { */ @action updateDocument(document: Pick) { + if (!this.documents) { + throw new Error("Collection documents not loaded"); + } + const travelNodes = (nodes: NavigationNode[]) => nodes.forEach((node) => { if (node.id === document.id) { @@ -131,6 +164,10 @@ export default class Collection extends ParanoidModel { */ @action removeDocument(documentId: string) { + if (!this.documents) { + throw new Error("Collection documents not loaded"); + } + this.documents = this.documents.filter(function f(node): boolean { if (node.id === documentId) { return false; @@ -163,7 +200,7 @@ export default class Collection extends ParanoidModel { }); }; - if (this.documents) { + if (this.sortedDocuments) { travelNodes(this.sortedDocuments); } diff --git a/app/models/CollectionGroupMembership.ts b/app/models/CollectionGroupMembership.ts index 4563eefd4..af533ed8d 100644 --- a/app/models/CollectionGroupMembership.ts +++ b/app/models/CollectionGroupMembership.ts @@ -1,4 +1,4 @@ -import { computed } from "mobx"; +import { observable } from "mobx"; import { CollectionPermission } from "@shared/types"; import BaseModel from "./BaseModel"; @@ -9,12 +9,8 @@ class CollectionGroupMembership extends BaseModel { collectionId: string; + @observable permission: CollectionPermission; - - @computed - get isEditor(): boolean { - return this.permission === CollectionPermission.ReadWrite; - } } export default CollectionGroupMembership; diff --git a/app/models/Membership.ts b/app/models/Membership.ts index 17ea36bdc..fdd4abbc7 100644 --- a/app/models/Membership.ts +++ b/app/models/Membership.ts @@ -1,4 +1,4 @@ -import { computed } from "mobx"; +import { observable } from "mobx"; import { CollectionPermission } from "@shared/types"; import BaseModel from "./BaseModel"; @@ -9,12 +9,8 @@ class Membership extends BaseModel { collectionId: string; + @observable permission: CollectionPermission; - - @computed - get isEditor(): boolean { - return this.permission === CollectionPermission.ReadWrite; - } } export default Membership; diff --git a/app/scenes/Collection.tsx b/app/scenes/Collection.tsx index f795eb4d3..4cc315e0a 100644 --- a/app/scenes/Collection.tsx +++ b/app/scenes/Collection.tsx @@ -148,7 +148,7 @@ function CollectionScene() { > @@ -159,7 +159,7 @@ function CollectionScene() { {collection.name} - {!collection.permission && ( + {collection.isPrivate && (
- {can.update && Get started by creating a new one!} + {can.createDocument && ( + Get started by creating a new one! + )} - {can.update && ( + {can.createDocument && (