From 79e2cad5b90f02dff4926e982f5d01fcd62c9d49 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Fri, 21 Jan 2022 18:11:50 -0800 Subject: [PATCH] feat: Add reordering to starred documents (#2953) * draft * reordering * JIT Index stars on first load * test * Remove unused code on client * small unrefactor --- .../Sidebar/components/Collections.tsx | 2 +- .../Sidebar/components/DropCursor.tsx | 11 +- app/components/Sidebar/components/Starred.tsx | 59 +++++--- .../Sidebar/components/StarredLink.tsx | 52 ++++++- app/components/SocketProvider.tsx | 13 +- app/models/Document.ts | 6 +- app/models/Star.ts | 28 ++++ app/stores/DocumentsStore.ts | 54 +------ app/stores/RootStore.ts | 4 + app/stores/StarsStore.ts | 44 ++++++ server/commands/starCreator.test.ts | 58 ++++++++ server/commands/starCreator.ts | 89 ++++++++++++ server/commands/starDestroyer.test.ts | 39 +++++ server/commands/starDestroyer.ts | 54 +++++++ server/commands/starUpdater.test.ts | 40 ++++++ server/commands/starUpdater.ts | 53 +++++++ .../20220117012250-add-starred-sorting.js | 13 ++ server/models/Star.ts | 5 + server/policies/index.ts | 1 + server/policies/star.ts | 9 ++ server/presenters/index.ts | 2 + server/presenters/star.ts | 11 ++ server/queues/processors/websockets.ts | 20 ++- server/routes/api/collections.ts | 8 +- server/routes/api/documents.ts | 3 + server/routes/api/index.ts | 2 + server/routes/api/stars.test.ts | 86 +++++++++++ server/routes/api/stars.ts | 134 ++++++++++++++++++ server/test/factories.ts | 25 ++++ server/types.ts | 12 ++ server/utils/collectionIndexing.ts | 44 ------ server/utils/indexing.ts | 82 +++++++++++ 32 files changed, 931 insertions(+), 132 deletions(-) create mode 100644 app/models/Star.ts create mode 100644 app/stores/StarsStore.ts create mode 100644 server/commands/starCreator.test.ts create mode 100644 server/commands/starCreator.ts create mode 100644 server/commands/starDestroyer.test.ts create mode 100644 server/commands/starDestroyer.ts create mode 100644 server/commands/starUpdater.test.ts create mode 100644 server/commands/starUpdater.ts create mode 100644 server/migrations/20220117012250-add-starred-sorting.js create mode 100644 server/policies/star.ts create mode 100644 server/presenters/star.ts create mode 100644 server/routes/api/stars.test.ts create mode 100644 server/routes/api/stars.ts delete mode 100644 server/utils/collectionIndexing.ts create mode 100644 server/utils/indexing.ts diff --git a/app/components/Sidebar/components/Collections.tsx b/app/components/Sidebar/components/Collections.tsx index 3e25b793d..b9cc9e937 100644 --- a/app/components/Sidebar/components/Collections.tsx +++ b/app/components/Sidebar/components/Collections.tsx @@ -73,7 +73,7 @@ function Collections() { {orderedCollections.map((collection: Collection, index: number) => ( ; - from?: string; + position?: "top"; }) { - return ; + return ; } // transparent hover zone with a thin visible band vertically centered -const Cursor = styled.div<{ isOver?: boolean; from?: string }>` +const Cursor = styled.div<{ isOver?: boolean; position?: "top" }>` opacity: ${(props) => (props.isOver ? 1 : 0)}; transition: opacity 150ms; - position: absolute; z-index: 1; width: 100%; height: 14px; - ${(props) => (props.from === "collections" ? "top: 25px;" : "bottom: -7px;")} background: transparent; + ${(props) => (props.position === "top" ? "top: 25px;" : "bottom: -7px;")} ::after { background: ${(props) => props.theme.slateDark}; diff --git a/app/components/Sidebar/components/Starred.tsx b/app/components/Sidebar/components/Starred.tsx index d9dd3aa2f..96b66a41a 100644 --- a/app/components/Sidebar/components/Starred.tsx +++ b/app/components/Sidebar/components/Starred.tsx @@ -1,12 +1,15 @@ +import fractionalIndex from "fractional-index"; import { observer } from "mobx-react"; import { CollapsedIcon } from "outline-icons"; import * as React from "react"; -import { useEffect } from "react"; +import { useDrop } from "react-dnd"; import { useTranslation } from "react-i18next"; import styled from "styled-components"; +import Star from "~/models/Star"; import Flex from "~/components/Flex"; import useStores from "~/hooks/useStores"; import useToasts from "~/hooks/useToasts"; +import DropCursor from "./DropCursor"; import PlaceholderCollections from "./PlaceholderCollections"; import Section from "./Section"; import SidebarLink from "./SidebarLink"; @@ -23,14 +26,13 @@ function Starred() { const [offset, setOffset] = React.useState(0); const [upperBound, setUpperBound] = React.useState(STARRED_PAGINATION_LIMIT); const { showToast } = useToasts(); - const { documents } = useStores(); + const { stars, documents } = useStores(); const { t } = useTranslation(); - const { fetchStarred, starred } = documents; const fetchResults = React.useCallback(async () => { try { setIsFetching(true); - await fetchStarred({ + await stars.fetchPage({ limit: STARRED_PAGINATION_LIMIT, offset, }); @@ -42,9 +44,9 @@ function Starred() { } finally { setIsFetching(false); } - }, [fetchStarred, offset, showToast, t]); + }, [stars, offset, showToast, t]); - useEffect(() => { + React.useEffect(() => { let stateInLocal; try { @@ -60,19 +62,19 @@ function Starred() { } }, [expanded]); - useEffect(() => { - setOffset(starred.length); + React.useEffect(() => { + setOffset(stars.orderedData.length); - if (starred.length <= STARRED_PAGINATION_LIMIT) { + if (stars.orderedData.length <= STARRED_PAGINATION_LIMIT) { setShow("Nothing"); - } else if (starred.length >= upperBound) { + } else if (stars.orderedData.length >= upperBound) { setShow("More"); - } else if (starred.length < upperBound) { + } else if (stars.orderedData.length < upperBound) { setShow("Less"); } - }, [starred, upperBound]); + }, [stars.orderedData, upperBound]); - useEffect(() => { + React.useEffect(() => { if (offset === 0) { fetchResults(); } @@ -106,20 +108,34 @@ function Starred() { [expanded] ); - const content = starred.slice(0, upperBound).map((document) => { - return ( + // Drop to reorder document + const [{ isOverReorder }, dropToReorder] = useDrop({ + accept: "star", + drop: async (item: Star) => { + item?.save({ index: fractionalIndex(null, stars.orderedData[0].index) }); + }, + collect: (monitor) => ({ + isOverReorder: !!monitor.isOver(), + }), + }); + + const content = stars.orderedData.slice(0, upperBound).map((star) => { + const document = documents.get(star.documentId); + + return document ? ( - ); + ) : null; }); - if (!starred.length) { + if (!stars.orderedData.length) { return null; } @@ -133,6 +149,11 @@ function Starred() { /> {expanded && ( <> + {content} {show === "More" && !isFetching && ( )} - {(isFetching || fetchError) && !starred.length && ( + {(isFetching || fetchError) && !stars.orderedData.length && ( diff --git a/app/components/Sidebar/components/StarredLink.tsx b/app/components/Sidebar/components/StarredLink.tsx index 807365f08..d4d8c8979 100644 --- a/app/components/Sidebar/components/StarredLink.tsx +++ b/app/components/Sidebar/components/StarredLink.tsx @@ -1,18 +1,23 @@ +import fractionalIndex from "fractional-index"; import { observer } from "mobx-react"; import * as React from "react"; import { useEffect, useState } from "react"; +import { useDrag, useDrop } from "react-dnd"; import { useTranslation } from "react-i18next"; import styled from "styled-components"; import { MAX_TITLE_LENGTH } from "@shared/constants"; +import Star from "~/models/Star"; import Fade from "~/components/Fade"; import useBoolean from "~/hooks/useBoolean"; import useStores from "~/hooks/useStores"; import DocumentMenu from "~/menus/DocumentMenu"; import Disclosure from "./Disclosure"; +import DropCursor from "./DropCursor"; import EditableTitle from "./EditableTitle"; import SidebarLink from "./SidebarLink"; type Props = { + star?: Star; depth: number; title: string; to: string; @@ -20,7 +25,14 @@ type Props = { collectionId: string; }; -function StarredLink({ depth, title, to, documentId, collectionId }: Props) { +function StarredLink({ + depth, + title, + to, + documentId, + collectionId, + star, +}: Props) { const { t } = useTranslation(); const { collections, documents, policies } = useStores(); const collection = collections.get(collectionId); @@ -74,9 +86,37 @@ function StarredLink({ depth, title, to, documentId, collectionId }: Props) { setIsEditing(isEditing); }, []); + // Draggable + const [{ isDragging }, drag] = useDrag({ + type: "star", + item: () => star, + collect: (monitor) => ({ + isDragging: !!monitor.isDragging(), + }), + canDrag: () => { + return depth === 2; + }, + }); + + // Drop to reorder + const [{ isOverReorder, isDraggingAny }, dropToReorder] = useDrop({ + accept: "star", + drop: (item: Star) => { + const next = star?.next(); + + item?.save({ + index: fractionalIndex(star?.index || null, next?.index || null), + }); + }, + collect: (monitor) => ({ + isOverReorder: !!monitor.isOver(), + isDraggingAny: !!monitor.canDrop(), + }), + }); + return ( <> - + - + {isDraggingAny && ( + + )} + {expanded && childDocuments.map((childDocument) => ( ` position: relative; + opacity: ${(props) => (props.$isDragging ? 0.5 : 1)}; `; const ObserveredStarredLink = observer(StarredLink); diff --git a/app/components/SocketProvider.tsx b/app/components/SocketProvider.tsx index 1f305e40c..7e5b43982 100644 --- a/app/components/SocketProvider.tsx +++ b/app/components/SocketProvider.tsx @@ -72,6 +72,7 @@ class SocketProvider extends React.Component { collections, groups, pins, + stars, memberships, policies, presence, @@ -273,12 +274,16 @@ class SocketProvider extends React.Component { pins.remove(event.modelId); }); - this.socket.on("documents.star", (event: any) => { - documents.starredIds.set(event.documentId, true); + this.socket.on("stars.create", (event: any) => { + stars.add(event); }); - this.socket.on("documents.unstar", (event: any) => { - documents.starredIds.set(event.documentId, false); + this.socket.on("stars.update", (event: any) => { + stars.add(event); + }); + + this.socket.on("stars.delete", (event: any) => { + stars.remove(event.modelId); }); this.socket.on("documents.permanent_delete", (event: any) => { diff --git a/app/models/Document.ts b/app/models/Document.ts index 5a9e03f79..9e08fe95c 100644 --- a/app/models/Document.ts +++ b/app/models/Document.ts @@ -147,7 +147,9 @@ export default class Document extends BaseModel { @computed get isStarred(): boolean { - return !!this.store.starredIds.get(this.id); + return !!this.store.rootStore.stars.orderedData.find( + (star) => star.documentId === this.id + ); } @computed @@ -258,7 +260,7 @@ export default class Document extends BaseModel { }; @action - star = () => { + star = async () => { return this.store.star(this); }; diff --git a/app/models/Star.ts b/app/models/Star.ts new file mode 100644 index 000000000..e8555f60a --- /dev/null +++ b/app/models/Star.ts @@ -0,0 +1,28 @@ +import { observable } from "mobx"; +import BaseModel from "./BaseModel"; +import Field from "./decorators/Field"; + +class Star extends BaseModel { + id: string; + + @Field + @observable + index: string; + + documentId: string; + + createdAt: string; + updatedAt: string; + + next(): Star | undefined { + const index = this.store.orderedData.indexOf(this); + return this.store.orderedData[index + 1]; + } + + previous(): Star | undefined { + const index = this.store.orderedData.indexOf(this); + return this.store.orderedData[index + 1]; + } +} + +export default Star; diff --git a/app/stores/DocumentsStore.ts b/app/stores/DocumentsStore.ts index 39deefc9b..7d1de02e9 100644 --- a/app/stores/DocumentsStore.ts +++ b/app/stores/DocumentsStore.ts @@ -43,9 +43,6 @@ export default class DocumentsStore extends BaseStore { @observable searchCache: Map = new Map(); - @observable - starredIds: Map = new Map(); - @observable backlinks: Map = new Map(); @@ -172,14 +169,6 @@ export default class DocumentsStore extends BaseStore { return this.searchCache.get(query) || []; } - get starred(): Document[] { - return orderBy( - this.all.filter((d) => d.isStarred), - "updatedAt", - "desc" - ); - } - @computed get archived(): Document[] { return orderBy(this.orderedData, "archivedAt", "desc").filter( @@ -194,11 +183,6 @@ export default class DocumentsStore extends BaseStore { ); } - @computed - get starredAlphabetical(): Document[] { - return naturalSort(this.starred, "title"); - } - @computed get templatesAlphabetical(): Document[] { return naturalSort(this.templates, "title"); @@ -623,19 +607,6 @@ export default class DocumentsStore extends BaseStore { return this.add(res.data); }; - _add = this.add; - - @action - add = (item: Record): Document => { - const document = this._add(item); - - if (item.starred !== undefined) { - this.starredIds.set(document.id, item.starred); - } - - return document; - }; - @action removeCollectionDocuments(collectionId: string) { const documents = this.inCollection(collectionId); @@ -739,27 +710,16 @@ export default class DocumentsStore extends BaseStore { }; star = async (document: Document) => { - this.starredIds.set(document.id, true); - - try { - return await client.post("/documents.star", { - id: document.id, - }); - } catch (err) { - this.starredIds.set(document.id, false); - } + await this.rootStore.stars.create({ + documentId: document.id, + }); }; unstar = async (document: Document) => { - this.starredIds.set(document.id, false); - - try { - return await client.post("/documents.unstar", { - id: document.id, - }); - } catch (err) { - this.starredIds.set(document.id, true); - } + const star = this.rootStore.stars.orderedData.find( + (star) => star.documentId === document.id + ); + await star?.delete(); }; getByUrl = (url = ""): Document | null | undefined => { diff --git a/app/stores/RootStore.ts b/app/stores/RootStore.ts index d5a9d9721..8693358eb 100644 --- a/app/stores/RootStore.ts +++ b/app/stores/RootStore.ts @@ -17,6 +17,7 @@ import PoliciesStore from "./PoliciesStore"; import RevisionsStore from "./RevisionsStore"; import SearchesStore from "./SearchesStore"; import SharesStore from "./SharesStore"; +import StarsStore from "./StarsStore"; import ToastsStore from "./ToastsStore"; import UiStore from "./UiStore"; import UsersStore from "./UsersStore"; @@ -42,6 +43,7 @@ export default class RootStore { searches: SearchesStore; shares: SharesStore; ui: UiStore; + stars: StarsStore; users: UsersStore; views: ViewsStore; toasts: ToastsStore; @@ -67,6 +69,7 @@ export default class RootStore { this.revisions = new RevisionsStore(this); this.searches = new SearchesStore(this); this.shares = new SharesStore(this); + this.stars = new StarsStore(this); this.ui = new UiStore(); this.users = new UsersStore(this); this.views = new ViewsStore(this); @@ -92,6 +95,7 @@ export default class RootStore { this.revisions.clear(); this.searches.clear(); this.shares.clear(); + this.stars.clear(); this.fileOperations.clear(); // this.ui omitted to keep ui settings between sessions this.users.clear(); diff --git a/app/stores/StarsStore.ts b/app/stores/StarsStore.ts new file mode 100644 index 000000000..d3bfe613e --- /dev/null +++ b/app/stores/StarsStore.ts @@ -0,0 +1,44 @@ +import invariant from "invariant"; +import { action, runInAction, computed } from "mobx"; +import Star from "~/models/Star"; +import { PaginationParams } from "~/types"; +import { client } from "~/utils/ApiClient"; +import BaseStore from "./BaseStore"; +import RootStore from "./RootStore"; + +export default class StarsStore extends BaseStore { + constructor(rootStore: RootStore) { + super(rootStore, Star); + } + + @action + fetchPage = async (params?: PaginationParams | undefined): Promise => { + this.isFetching = true; + + try { + const res = await client.post(`/stars.list`, params); + invariant(res && res.data, "Data not available"); + runInAction(`StarsStore#fetchPage`, () => { + res.data.documents.forEach(this.rootStore.documents.add); + res.data.stars.forEach(this.add); + this.addPolicies(res.policies); + this.isLoaded = true; + }); + } finally { + this.isFetching = false; + } + }; + + @computed + get orderedData(): Star[] { + const stars = Array.from(this.data.values()); + + return stars.sort((a, b) => { + if (a.index === b.index) { + return a.updatedAt > b.updatedAt ? -1 : 1; + } + + return a.index < b.index ? -1 : 1; + }); + } +} diff --git a/server/commands/starCreator.test.ts b/server/commands/starCreator.test.ts new file mode 100644 index 000000000..09f563671 --- /dev/null +++ b/server/commands/starCreator.test.ts @@ -0,0 +1,58 @@ +import { Star, Event } from "@server/models"; +import { buildDocument, buildUser } from "@server/test/factories"; +import { flushdb } from "@server/test/support"; +import starCreator from "./starCreator"; + +beforeEach(() => flushdb()); +describe("starCreator", () => { + const ip = "127.0.0.1"; + + it("should create star", async () => { + const user = await buildUser(); + const document = await buildDocument({ + userId: user.id, + teamId: user.teamId, + }); + + const star = await starCreator({ + documentId: document.id, + user, + ip, + }); + + const event = await Event.findOne(); + expect(star.documentId).toEqual(document.id); + expect(star.userId).toEqual(user.id); + expect(star.index).toEqual("P"); + expect(event!.name).toEqual("stars.create"); + expect(event!.modelId).toEqual(star.id); + }); + + it("should not record event if star is existing", async () => { + const user = await buildUser(); + const document = await buildDocument({ + userId: user.id, + teamId: user.teamId, + }); + + await Star.create({ + teamId: document.teamId, + documentId: document.id, + userId: user.id, + createdById: user.id, + index: "P", + }); + + const star = await starCreator({ + documentId: document.id, + user, + ip, + }); + + const events = await Event.count(); + expect(star.documentId).toEqual(document.id); + expect(star.userId).toEqual(user.id); + expect(star.index).toEqual("P"); + expect(events).toEqual(0); + }); +}); diff --git a/server/commands/starCreator.ts b/server/commands/starCreator.ts new file mode 100644 index 000000000..f809cc184 --- /dev/null +++ b/server/commands/starCreator.ts @@ -0,0 +1,89 @@ +import fractionalIndex from "fractional-index"; +import { Sequelize, WhereOptions } from "sequelize"; +import { sequelize } from "@server/database/sequelize"; +import { Star, User, Event } from "@server/models"; + +type Props = { + /** The user creating the star */ + user: User; + /** The document to star */ + documentId: string; + /** The sorted index for the star in the sidebar If no index is provided then it will be at the end */ + index?: string; + /** The IP address of the user creating the star */ + ip: string; +}; + +/** + * This command creates a "starred" document via the star relation. Stars are + * only visible to the user that created them. + * + * @param Props The properties of the star to create + * @returns Star The star that was created + */ +export default async function starCreator({ + user, + documentId, + ip, + ...rest +}: Props): Promise { + let { index } = rest; + const where: WhereOptions = { + userId: user.id, + }; + + if (!index) { + const stars = await Star.findAll({ + where, + attributes: ["id", "index", "updatedAt"], + limit: 1, + order: [ + // using LC_COLLATE:"C" because we need byte order to drive the sorting + // find only the first star so we can create an index before it + Sequelize.literal('"star"."index" collate "C"'), + ["updatedAt", "DESC"], + ], + }); + + // create a star at the beginning of the list + index = fractionalIndex(null, stars.length ? stars[0].index : null); + } + + const transaction = await sequelize.transaction(); + let star; + + try { + const response = await Star.findOrCreate({ + where: { + userId: user.id, + documentId, + }, + defaults: { + index, + }, + transaction, + }); + star = response[0]; + + if (response[1]) { + await Event.create( + { + name: "stars.create", + modelId: star.id, + userId: user.id, + actorId: user.id, + documentId, + ip, + }, + { transaction } + ); + } + + await transaction.commit(); + } catch (err) { + await transaction.rollback(); + throw err; + } + + return star; +} diff --git a/server/commands/starDestroyer.test.ts b/server/commands/starDestroyer.test.ts new file mode 100644 index 000000000..234bf98b8 --- /dev/null +++ b/server/commands/starDestroyer.test.ts @@ -0,0 +1,39 @@ +import { Star, Event } from "@server/models"; +import { buildDocument, buildUser } from "@server/test/factories"; +import { flushdb } from "@server/test/support"; +import starDestroyer from "./starDestroyer"; + +beforeEach(() => flushdb()); + +describe("starDestroyer", () => { + const ip = "127.0.0.1"; + + it("should destroy existing star", async () => { + const user = await buildUser(); + const document = await buildDocument({ + userId: user.id, + teamId: user.teamId, + }); + + const star = await Star.create({ + teamId: document.teamId, + documentId: document.id, + userId: user.id, + createdById: user.id, + index: "P", + }); + + await starDestroyer({ + star, + user, + ip, + }); + + const count = await Star.count(); + expect(count).toEqual(0); + + const event = await Event.findOne(); + expect(event!.name).toEqual("stars.delete"); + expect(event!.modelId).toEqual(star.id); + }); +}); diff --git a/server/commands/starDestroyer.ts b/server/commands/starDestroyer.ts new file mode 100644 index 000000000..b8ff557d9 --- /dev/null +++ b/server/commands/starDestroyer.ts @@ -0,0 +1,54 @@ +import { Transaction } from "sequelize"; +import { sequelize } from "@server/database/sequelize"; +import { Event, Star, User } from "@server/models"; + +type Props = { + /** The user destroying the star */ + user: User; + /** The star to destroy */ + star: Star; + /** The IP address of the user creating the star */ + ip: string; + /** Optional existing transaction */ + transaction?: Transaction; +}; + +/** + * This command destroys a document star. This just removes the star itself and + * does not touch the document + * + * @param Props The properties of the star to destroy + * @returns void + */ +export default async function starDestroyer({ + user, + star, + ip, + transaction: t, +}: Props): Promise { + const transaction = t || (await sequelize.transaction()); + + try { + await star.destroy({ transaction }); + + await Event.create( + { + name: "stars.delete", + modelId: star.id, + teamId: user.teamId, + actorId: user.id, + userId: star.userId, + documentId: star.documentId, + ip, + }, + { transaction } + ); + + await transaction.commit(); + } catch (err) { + await transaction.rollback(); + throw err; + } + + return star; +} diff --git a/server/commands/starUpdater.test.ts b/server/commands/starUpdater.test.ts new file mode 100644 index 000000000..74f69d291 --- /dev/null +++ b/server/commands/starUpdater.test.ts @@ -0,0 +1,40 @@ +import { Star, Event } from "@server/models"; +import { buildDocument, buildUser } from "@server/test/factories"; +import { flushdb } from "@server/test/support"; +import starUpdater from "./starUpdater"; + +beforeEach(() => flushdb()); + +describe("starUpdater", () => { + const ip = "127.0.0.1"; + + it("should update (move) existing star", async () => { + const user = await buildUser(); + const document = await buildDocument({ + userId: user.id, + teamId: user.teamId, + }); + + let star = await Star.create({ + teamId: document.teamId, + documentId: document.id, + userId: user.id, + createdById: user.id, + index: "P", + }); + + star = await starUpdater({ + star, + index: "h", + user, + ip, + }); + + const event = await Event.findOne(); + expect(star.documentId).toEqual(document.id); + expect(star.userId).toEqual(user.id); + expect(star.index).toEqual("h"); + expect(event!.name).toEqual("stars.update"); + expect(event!.modelId).toEqual(star.id); + }); +}); diff --git a/server/commands/starUpdater.ts b/server/commands/starUpdater.ts new file mode 100644 index 000000000..0b5d48846 --- /dev/null +++ b/server/commands/starUpdater.ts @@ -0,0 +1,53 @@ +import { sequelize } from "@server/database/sequelize"; +import { Event, Star, User } from "@server/models"; + +type Props = { + /** The user updating the star */ + user: User; + /** The existing star */ + star: Star; + /** The index to star the document at */ + index: string; + /** The IP address of the user creating the star */ + ip: string; +}; + +/** + * This command updates a "starred" document. A star can only be moved to a new + * index (reordered) once created. + * + * @param Props The properties of the star to update + * @returns Star The updated star + */ +export default async function starUpdater({ + user, + star, + index, + ip, +}: Props): Promise { + const transaction = await sequelize.transaction(); + + try { + star.index = index; + await star.save({ transaction }); + + await Event.create( + { + name: "stars.update", + modelId: star.id, + userId: star.userId, + teamId: user.teamId, + actorId: user.id, + documentId: star.documentId, + ip, + }, + { transaction } + ); + await transaction.commit(); + } catch (err) { + await transaction.rollback(); + throw err; + } + + return star; +} diff --git a/server/migrations/20220117012250-add-starred-sorting.js b/server/migrations/20220117012250-add-starred-sorting.js new file mode 100644 index 000000000..51f365903 --- /dev/null +++ b/server/migrations/20220117012250-add-starred-sorting.js @@ -0,0 +1,13 @@ +"use strict"; + +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.addColumn("stars", "index", { + type: Sequelize.STRING, + allowNull: true, + }); + }, + down: async (queryInterface) => { + await queryInterface.removeColumn("stars", "index"); + }, +}; diff --git a/server/models/Star.ts b/server/models/Star.ts index c0e00ac01..0ede31dbc 100644 --- a/server/models/Star.ts +++ b/server/models/Star.ts @@ -13,6 +13,11 @@ import Fix from "./decorators/Fix"; @Table({ tableName: "stars", modelName: "star" }) @Fix class Star extends BaseModel { + @Column + index: string | null; + + // associations + @BelongsTo(() => User, "userId") user: User; diff --git a/server/policies/index.ts b/server/policies/index.ts index 6398ab81a..c2240889f 100644 --- a/server/policies/index.ts +++ b/server/policies/index.ts @@ -17,6 +17,7 @@ import "./notificationSetting"; import "./pins"; import "./searchQuery"; import "./share"; +import "./star"; import "./user"; import "./team"; import "./group"; diff --git a/server/policies/star.ts b/server/policies/star.ts new file mode 100644 index 000000000..faa4bde2b --- /dev/null +++ b/server/policies/star.ts @@ -0,0 +1,9 @@ +import { User, Star } from "@server/models"; +import { allow } from "./cancan"; + +allow( + User, + ["update", "delete"], + Star, + (user, star) => user.id === star?.userId +); diff --git a/server/presenters/index.ts b/server/presenters/index.ts index 483163d73..24cc1bd63 100644 --- a/server/presenters/index.ts +++ b/server/presenters/index.ts @@ -16,6 +16,7 @@ import presentRevision from "./revision"; import presentSearchQuery from "./searchQuery"; import presentShare from "./share"; import presentSlackAttachment from "./slackAttachment"; +import presentStar from "./star"; import presentTeam from "./team"; import presentUser from "./user"; import presentView from "./view"; @@ -32,6 +33,7 @@ export { presentCollection, presentShare, presentSearchQuery, + presentStar, presentTeam, presentGroup, presentIntegration, diff --git a/server/presenters/star.ts b/server/presenters/star.ts new file mode 100644 index 000000000..0795d26c5 --- /dev/null +++ b/server/presenters/star.ts @@ -0,0 +1,11 @@ +import { Star } from "@server/models"; + +export default function present(star: Star) { + return { + id: star.id, + documentId: star.documentId, + index: star.index, + createdAt: star.createdAt, + updatedAt: star.updatedAt, + }; +} diff --git a/server/queues/processors/websockets.ts b/server/queues/processors/websockets.ts index 304ecf759..25dcd6a1a 100644 --- a/server/queues/processors/websockets.ts +++ b/server/queues/processors/websockets.ts @@ -7,8 +7,9 @@ import { CollectionGroup, GroupUser, Pin, + Star, } from "@server/models"; -import { presentPin } from "@server/presenters"; +import { presentPin, presentStar } from "@server/presenters"; import { Event } from "../../types"; export default class WebsocketsProcessor { @@ -386,6 +387,23 @@ export default class WebsocketsProcessor { }); } + case "stars.create": + case "stars.update": { + const star = await Star.findByPk(event.modelId); + if (!star) { + return; + } + return socketio + .to(`user-${event.userId}`) + .emit(event.name, presentStar(star)); + } + + case "stars.delete": { + return socketio.to(`user-${event.userId}`).emit(event.name, { + modelId: event.modelId, + }); + } + case "groups.create": case "groups.update": { const group = await Group.findByPk(event.modelId, { diff --git a/server/routes/api/collections.ts b/server/routes/api/collections.ts index 70a476df1..19a22a81f 100644 --- a/server/routes/api/collections.ts +++ b/server/routes/api/collections.ts @@ -25,7 +25,7 @@ import { presentCollectionGroupMembership, presentFileOperation, } from "@server/presenters"; -import collectionIndexing from "@server/utils/collectionIndexing"; +import { collectionIndexing } from "@server/utils/indexing"; import removeIndexCollision from "@server/utils/removeIndexCollision"; import { assertUuid, @@ -623,12 +623,12 @@ router.post("collections.list", auth(), pagination(), async (ctx) => { offset: ctx.state.pagination.offset, limit: ctx.state.pagination.limit, }); - const nullIndexCollection = collections.findIndex( + const nullIndex = collections.findIndex( (collection) => collection.index === null ); - if (nullIndexCollection !== -1) { - const indexedCollections = await collectionIndexing(ctx.state.user.teamId); + if (nullIndex !== -1) { + const indexedCollections = await collectionIndexing(user.teamId); collections.forEach((collection) => { collection.index = indexedCollections[collection.id]; }); diff --git a/server/routes/api/documents.ts b/server/routes/api/documents.ts index 06582b582..1a29414d2 100644 --- a/server/routes/api/documents.ts +++ b/server/routes/api/documents.ts @@ -312,6 +312,7 @@ router.post("documents.viewed", auth(), pagination(), async (ctx) => { }; }); +// Deprecated – use stars.list instead router.post("documents.starred", auth(), pagination(), async (ctx) => { let { direction } = ctx.body; const { sort = "updatedAt" } = ctx.body; @@ -864,6 +865,7 @@ router.post("documents.search", auth(), pagination(), async (ctx) => { }; }); +// Deprecated – use stars.create instead router.post("documents.star", auth(), async (ctx) => { const { id } = ctx.body; assertPresent(id, "id is required"); @@ -898,6 +900,7 @@ router.post("documents.star", auth(), async (ctx) => { }; }); +// Deprecated – use stars.delete instead router.post("documents.unstar", auth(), async (ctx) => { const { id } = ctx.body; assertPresent(id, "id is required"); diff --git a/server/routes/api/index.ts b/server/routes/api/index.ts index 1456fee25..f386013bb 100644 --- a/server/routes/api/index.ts +++ b/server/routes/api/index.ts @@ -22,6 +22,7 @@ import pins from "./pins"; import revisions from "./revisions"; import searches from "./searches"; import shares from "./shares"; +import stars from "./stars"; import team from "./team"; import users from "./users"; import utils from "./utils"; @@ -58,6 +59,7 @@ router.use("/", hooks.routes()); router.use("/", apiKeys.routes()); router.use("/", searches.routes()); router.use("/", shares.routes()); +router.use("/", stars.routes()); router.use("/", team.routes()); router.use("/", integrations.routes()); router.use("/", notificationSettings.routes()); diff --git a/server/routes/api/stars.test.ts b/server/routes/api/stars.test.ts new file mode 100644 index 000000000..fd9197536 --- /dev/null +++ b/server/routes/api/stars.test.ts @@ -0,0 +1,86 @@ +import TestServer from "fetch-test-server"; +import webService from "@server/services/web"; +import { buildUser, buildStar, buildDocument } from "@server/test/factories"; +import { flushdb } from "@server/test/support"; + +const app = webService(); +const server = new TestServer(app.callback()); +beforeEach(() => flushdb()); +afterAll(() => server.close()); + +describe("#stars.create", () => { + it("should create a star", async () => { + const user = await buildUser(); + const document = await buildDocument({ + userId: user.id, + teamId: user.teamId, + }); + + const res = await server.post("/api/stars.create", { + body: { + token: user.getJwtToken(), + documentId: document.id, + }, + }); + + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data.documentId).toEqual(document.id); + }); + + it("should require authentication", async () => { + const res = await server.post("/api/stars.create"); + expect(res.status).toEqual(401); + }); +}); + +describe("#stars.list", () => { + it("should list users stars", async () => { + const user = await buildUser(); + + await buildStar(); + + const star = await buildStar({ + userId: user.id, + }); + + const res = await server.post("/api/stars.list", { + body: { + token: user.getJwtToken(), + }, + }); + + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data.stars.length).toEqual(1); + expect(body.data.stars[0].id).toEqual(star.id); + }); + + it("should require authentication", async () => { + const res = await server.post("/api/stars.list"); + expect(res.status).toEqual(401); + }); +}); + +describe("#stars.delete", () => { + it("should delete users star", async () => { + const user = await buildUser(); + const star = await buildStar({ + userId: user.id, + }); + + const res = await server.post("/api/stars.delete", { + body: { + id: star.id, + token: user.getJwtToken(), + }, + }); + + expect(res.status).toEqual(200); + }); + + it("should require authentication", async () => { + const res = await server.post("/api/stars.delete"); + expect(res.status).toEqual(401); + }); +}); diff --git a/server/routes/api/stars.ts b/server/routes/api/stars.ts new file mode 100644 index 000000000..f8159ee33 --- /dev/null +++ b/server/routes/api/stars.ts @@ -0,0 +1,134 @@ +import Router from "koa-router"; +import { Sequelize } from "sequelize"; +import starCreator from "@server/commands/starCreator"; +import starDestroyer from "@server/commands/starDestroyer"; +import starUpdater from "@server/commands/starUpdater"; +import auth from "@server/middlewares/authentication"; +import { Document, Star } from "@server/models"; +import { authorize } from "@server/policies"; +import { + presentStar, + presentDocument, + presentPolicies, +} from "@server/presenters"; +import { starIndexing } from "@server/utils/indexing"; +import { assertUuid, assertIndexCharacters } from "@server/validation"; +import pagination from "./middlewares/pagination"; + +const router = new Router(); + +router.post("stars.create", auth(), async (ctx) => { + const { documentId } = ctx.body; + const { index } = ctx.body; + assertUuid(documentId, "documentId is required"); + + const { user } = ctx.state; + const document = await Document.findByPk(documentId, { + userId: user.id, + }); + authorize(user, "star", document); + + if (index) { + assertIndexCharacters(index); + } + + const star = await starCreator({ + user, + documentId, + ip: ctx.request.ip, + index, + }); + + ctx.body = { + data: presentStar(star), + policies: presentPolicies(user, [star]), + }; +}); + +router.post("stars.list", auth(), pagination(), async (ctx) => { + const { user } = ctx.state; + + const [stars, collectionIds] = await Promise.all([ + Star.findAll({ + where: { + userId: user.id, + }, + order: [ + Sequelize.literal('"star"."index" collate "C"'), + ["updatedAt", "DESC"], + ], + offset: ctx.state.pagination.offset, + limit: ctx.state.pagination.limit, + }), + user.collectionIds(), + ]); + + const nullIndex = stars.findIndex((star) => star.index === null); + + if (nullIndex !== -1) { + const indexedStars = await starIndexing(user.id); + stars.forEach((star) => { + star.index = indexedStars[star.id]; + }); + } + + const documents = await Document.defaultScopeWithUser(user.id).findAll({ + where: { + id: stars.map((star) => star.documentId), + collectionId: collectionIds, + }, + }); + + const policies = presentPolicies(user, [...documents, ...stars]); + + ctx.body = { + pagination: ctx.state.pagination, + data: { + stars: stars.map(presentStar), + documents: await Promise.all( + documents.map((document: Document) => presentDocument(document)) + ), + }, + policies, + }; +}); + +router.post("stars.update", auth(), async (ctx) => { + const { id, index } = ctx.body; + assertUuid(id, "id is required"); + + assertIndexCharacters(index); + + const { user } = ctx.state; + let star = await Star.findByPk(id); + authorize(user, "update", star); + + star = await starUpdater({ + user, + star, + ip: ctx.request.ip, + index, + }); + + ctx.body = { + data: presentStar(star), + policies: presentPolicies(user, [star]), + }; +}); + +router.post("stars.delete", auth(), async (ctx) => { + const { id } = ctx.body; + assertUuid(id, "id is required"); + + const { user } = ctx.state; + const star = await Star.findByPk(id); + authorize(user, "delete", star); + + await starDestroyer({ user, star, ip: ctx.request.ip }); + + ctx.body = { + success: true, + }; +}); + +export default router; diff --git a/server/test/factories.ts b/server/test/factories.ts index 27579f1ab..3415abb49 100644 --- a/server/test/factories.ts +++ b/server/test/factories.ts @@ -5,6 +5,7 @@ import { User, Event, Document, + Star, Collection, Group, GroupUser, @@ -44,6 +45,30 @@ export async function buildShare(overrides: Partial = {}) { }); } +export async function buildStar(overrides: Partial = {}) { + let user; + + if (overrides.userId) { + user = await User.findByPk(overrides.userId); + } else { + user = await buildUser(); + overrides.userId = user.id; + } + + if (!overrides.documentId) { + const document = await buildDocument({ + createdById: overrides.userId, + teamId: user?.teamId, + }); + overrides.documentId = document.id; + } + + return Star.create({ + index: "h", + ...overrides, + }); +} + export function buildTeam(overrides: Record = {}) { count++; return Team.create( diff --git a/server/types.ts b/server/types.ts index 44e7ae57e..07932b45c 100644 --- a/server/types.ts +++ b/server/types.ts @@ -241,15 +241,27 @@ export type PinEvent = { name: "pins.create" | "pins.update" | "pins.delete"; teamId: string; modelId: string; + documentId: string; collectionId?: string; actorId: string; ip: string; }; +export type StarEvent = { + name: "stars.create" | "stars.update" | "stars.delete"; + teamId: string; + modelId: string; + documentId: string; + userId: string; + actorId: string; + ip: string; +}; + export type Event = | UserEvent | DocumentEvent | PinEvent + | StarEvent | CollectionEvent | CollectionImportEvent | CollectionExportAllEvent diff --git a/server/utils/collectionIndexing.ts b/server/utils/collectionIndexing.ts deleted file mode 100644 index 9239f8b5b..000000000 --- a/server/utils/collectionIndexing.ts +++ /dev/null @@ -1,44 +0,0 @@ -import fractionalIndex from "fractional-index"; -import naturalSort from "@shared/utils/naturalSort"; -import { Collection } from "@server/models"; - -export default async function collectionIndexing(teamId: string) { - const collections = await Collection.findAll({ - where: { - teamId, - deletedAt: null, - }, - //no point in maintaining index of deleted collections. - attributes: ["id", "index", "name"], - }); - - let sortableCollections: [Collection, string | null][] = collections.map( - (collection) => { - return [collection, collection.index]; - } - ); - - sortableCollections = naturalSort( - sortableCollections, - (collection) => collection[0].name - ); - - //for each collection with null index, use previous collection index to create new index - let previousCollectionIndex = null; - - for (const collection of sortableCollections) { - if (collection[1] === null) { - const index = fractionalIndex(previousCollectionIndex, collection[1]); - collection[0].index = index; - await collection[0].save(); - } - - previousCollectionIndex = collection[0].index; - } - - const indexedCollections = {}; - sortableCollections.forEach((collection) => { - indexedCollections[collection[0].id] = collection[0].index; - }); - return indexedCollections; -} diff --git a/server/utils/indexing.ts b/server/utils/indexing.ts new file mode 100644 index 000000000..6bbe8bbb8 --- /dev/null +++ b/server/utils/indexing.ts @@ -0,0 +1,82 @@ +import fractionalIndex from "fractional-index"; +import naturalSort from "@shared/utils/naturalSort"; +import { Collection, Document, Star } from "@server/models"; + +export async function collectionIndexing( + teamId: string +): Promise<{ [id: string]: string }> { + const collections = await Collection.findAll({ + where: { + teamId, + // no point in maintaining index of deleted collections. + deletedAt: null, + }, + attributes: ["id", "index", "name"], + }); + + const sortable = naturalSort(collections, (collection) => collection.name); + + // for each collection with null index, use previous collection index to create new index + let previousIndex = null; + const promises = []; + + for (const collection of sortable) { + if (collection.index === null) { + collection.index = fractionalIndex(previousIndex, null); + promises.push(collection.save()); + } + + previousIndex = collection.index; + } + + await Promise.all(promises); + + const indexedCollections = {}; + sortable.forEach((collection) => { + indexedCollections[collection.id] = collection.index; + }); + return indexedCollections; +} + +export async function starIndexing( + userId: string +): Promise<{ [id: string]: string }> { + const stars = await Star.findAll({ + where: { userId }, + }); + + const documents = await Document.findAll({ + attributes: ["id", "updatedAt"], + where: { + id: stars.map((star) => star.documentId), + }, + order: [["updatedAt", "DESC"]], + }); + + const sortable = stars.sort(function (a, b) { + return ( + documents.findIndex((d) => d.id === a.documentId) - + documents.findIndex((d) => d.id === b.documentId) + ); + }); + + let previousIndex = null; + const promises = []; + + for (const star of sortable) { + if (star.index === null) { + star.index = fractionalIndex(previousIndex, null); + promises.push(star.save()); + } + + previousIndex = star.index; + } + + await Promise.all(promises); + + const indexedStars = {}; + sortable.forEach((star) => { + indexedStars[star.id] = star.index; + }); + return indexedStars; +}