fix: Prevent double pinning of documents (#8503)

* fix: Prevent double pinning of documents

* tests

* review

* policy

* schema

app/hooks/useRequest.ts

@@ -8,6 +8,8 @@ type RequestResponse<T> = {
   error: unknown;
   /** Whether the request is currently in progress. */
   loading: boolean;
+  /** Whether the request has completed - useful to check if the request has completed at least once. */
+  loaded: boolean;
   /** Function to start the request. */
   request: () => Promise<T | undefined>;
 };
@@ -26,6 +28,7 @@ export default function useRequest<T = unknown>(
   const isMounted = useIsMounted();
   const [data, setData] = React.useState<T>();
   const [loading, setLoading] = React.useState<boolean>(false);
+  const [loaded, setLoaded] = React.useState<boolean>(false);
   const [error, setError] = React.useState();
 
   const request = React.useCallback(async () => {
@@ -36,6 +39,7 @@ export default function useRequest<T = unknown>(
       if (isMounted()) {
         setData(response);
         setError(undefined);
+        setLoaded(true);
       }
       return response;
     } catch (err) {
@@ -57,5 +61,5 @@ export default function useRequest<T = unknown>(
     }
   }, [request, makeRequestOnMount]);
 
-  return { data, loading, error, request };
+  return { data, loading, loaded, error, request };
 }

app/menus/DocumentMenu.tsx

@@ -1,6 +1,5 @@
 import capitalize from "lodash/capitalize";
 import isEmpty from "lodash/isEmpty";
-import isUndefined from "lodash/isUndefined";
 import { observer } from "mobx-react";
 import { EditIcon, InputIcon, RestoreIcon, SearchIcon } from "outline-icons";
 import * as React from "react";
@@ -92,22 +91,32 @@ type MenuTriggerProps = {
 const MenuTrigger: React.FC<MenuTriggerProps> = ({ label, onTrigger }) => {
   const { t } = useTranslation();
 
-  const { subscriptions } = useStores();
+  const { subscriptions, pins } = useStores();
   const { model: document, menuState } = useMenuContext<Document>();
 
-  const { data, loading, error, request } = useRequest(() =>
-    subscriptions.fetchOne({
-      documentId: document.id,
-      event: "documents.update",
-    })
+  const {
+    loading: auxDataLoading,
+    loaded: auxDataLoaded,
+    request: auxDataRequest,
+  } = useRequest(() =>
+    Promise.all([
+      subscriptions.fetchOne({
+        documentId: document.id,
+        event: "documents.update",
+      }),
+      pins.fetchOne({
+        documentId: document.id,
+        collectionId: document.collectionId ?? null,
+      }),
+    ])
   );
 
   const handlePointerEnter = React.useCallback(() => {
-    if (isUndefined(data ?? error) && !loading) {
-      void request();
+    if (!auxDataLoading && !auxDataLoaded) {
+      void auxDataRequest();
       void document.loadRelations();
     }
-  }, [data, error, loading, request, document]);
+  }, [auxDataLoading, auxDataLoaded, auxDataRequest, document]);
 
   return label ? (
     <MenuButton

app/models/Pin.ts

@@ -13,7 +13,7 @@ class Pin extends Model {
   static modelName = "Pin";
 
   /** The collection ID that the document is pinned to. If empty the document is pinned to home. */
-  collectionId: string;
+  collectionId: string | null;
 
   /** The collection that the document is pinned to. If empty the document is pinned to home. */
   @Relation(() => Collection, { onDelete: "cascade" })

app/stores/PinsStore.ts

@@ -3,6 +3,7 @@ import { action, runInAction, computed } from "mobx";
 import Pin from "~/models/Pin";
 import { PaginationParams } from "~/types";
 import { client } from "~/utils/ApiClient";
+import { AuthorizationError, NotFoundError } from "~/utils/errors";
 import RootStore from "./RootStore";
 import Store from "./base/Store";
 
@@ -13,6 +14,41 @@ export default class PinsStore extends Store<Pin> {
     super(rootStore, Pin);
   }
 
+  @action
+  async fetchOne({
+    documentId,
+    collectionId,
+  }: {
+    documentId: string;
+    collectionId: string | null;
+  }) {
+    const pin = this.orderedData.find(
+      (p) => p.documentId === documentId && p.collectionId === collectionId
+    );
+
+    if (pin) {
+      return pin;
+    }
+
+    this.isFetching = true;
+
+    try {
+      const res = await client.post(`/${this.apiEndpoint}.info`, {
+        documentId,
+        collectionId,
+      });
+      invariant(res?.data, "Data should be available");
+      return this.add(res.data);
+    } catch (err) {
+      if (err instanceof AuthorizationError || err instanceof NotFoundError) {
+        return;
+      }
+      throw err;
+    } finally {
+      this.isFetching = false;
+    }
+  }
+
   @action
   fetchPage = async (params?: FetchParams | undefined): Promise<Pin[]> => {
     this.isFetching = true;

server/commands/pinCreator.ts

@@ -62,12 +62,13 @@ export default async function pinCreator({
     index = fractionalIndex(pins.length ? pins[0].index : null, null);
   }
 
-  const pin = await Pin.createWithCtx(ctx, {
-    createdById: user.id,
-    teamId: user.teamId,
-    collectionId,
-    documentId,
-    index,
+  const [pin] = await Pin.findOrCreateWithCtx(ctx, {
+    where: {
+      collectionId: collectionId ?? null,
+      documentId,
+      teamId: user.teamId,
+    },
+    defaults: { index, createdById: user.id },
   });
 
   return pin;

server/routes/api/pins/pins.test.ts

@@ -168,6 +168,84 @@ describe("#pins.create", () => {
   });
 });
 
+describe("#pins.info", () => {
+  it("should provide info about a home pin", async () => {
+    const admin = await buildAdmin();
+    const document = await buildDocument({
+      userId: admin.id,
+      teamId: admin.teamId,
+    });
+
+    await server.post("/api/pins.create", {
+      body: {
+        token: admin.getJwtToken(),
+        documentId: document.id,
+      },
+    });
+
+    const res = await server.post("/api/pins.info", {
+      body: {
+        token: admin.getJwtToken(),
+        documentId: document.id,
+      },
+    });
+    const pin = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(pin.data.id).toBeDefined();
+    expect(pin.data.documentId).toEqual(document.id);
+    expect(pin.data.collectionId).toBeFalsy();
+  });
+
+  it("should provide info about a collection pin", async () => {
+    const user = await buildUser();
+    const document = await buildDocument({
+      userId: user.id,
+      teamId: user.teamId,
+    });
+
+    await server.post("/api/pins.create", {
+      body: {
+        token: user.getJwtToken(),
+        documentId: document.id,
+        collectionId: document.collectionId,
+      },
+    });
+
+    const res = await server.post("/api/pins.info", {
+      body: {
+        token: user.getJwtToken(),
+        documentId: document.id,
+        collectionId: document.collectionId,
+      },
+    });
+    const pin = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(pin.data.id).toBeDefined();
+    expect(pin.data.documentId).toEqual(document.id);
+    expect(pin.data.collectionId).toEqual(document.collectionId);
+  });
+
+  it("should throw 404 if no pin found", async () => {
+    const user = await buildUser();
+    const document = await buildDocument({
+      userId: user.id,
+      teamId: user.teamId,
+    });
+
+    const res = await server.post("/api/pins.info", {
+      body: {
+        token: user.getJwtToken(),
+        documentId: document.id,
+        collectionId: null,
+      },
+    });
+
+    expect(res.status).toEqual(404);
+  });
+});
+
 describe("#pins.list", () => {
   let user: User;
   let pins: Pin[];

server/routes/api/pins/pins.ts

@@ -57,12 +57,41 @@ router.post(
   }
 );
 
+router.post(
+  "pins.info",
+  auth(),
+  validate(T.PinsInfoSchema),
+  async (ctx: APIContext<T.PinsInfoReq>) => {
+    const { user } = ctx.state.auth;
+    const { documentId, collectionId } = ctx.input.body;
+
+    const document = await Document.findByPk(documentId, { userId: user.id });
+    authorize(user, "read", document);
+
+    // There can be only one pin with these props.
+    const pin = await Pin.findOne({
+      where: {
+        documentId,
+        collectionId: collectionId ?? null,
+        createdById: user.id,
+        teamId: user.teamId,
+      },
+      rejectOnEmpty: true,
+    });
+
+    ctx.body = {
+      data: presentPin(pin),
+      policies: presentPolicies(user, [pin]),
+    };
+  }
+);
+
 router.post(
   "pins.list",
   auth(),
   validate(T.PinsListSchema),
   pagination(),
-  async (ctx: APIContext<T.PinsCreateReq>) => {
+  async (ctx: APIContext<T.PinsListReq>) => {
     const { collectionId } = ctx.input.body;
     const { user } = ctx.state.auth;
 

server/routes/api/pins/schema.ts

@@ -1,6 +1,7 @@
 import isUUID from "validator/lib/isUUID";
 import { z } from "zod";
 import { UrlHelper } from "@shared/utils/UrlHelper";
+import { zodIdType } from "@server/utils/zod";
 import { BaseSchema } from "../schema";
 
 export const PinsCreateSchema = BaseSchema.extend({
@@ -24,13 +25,24 @@ export const PinsCreateSchema = BaseSchema.extend({
 
 export type PinsCreateReq = z.infer<typeof PinsCreateSchema>;
 
+export const PinsInfoSchema = BaseSchema.extend({
+  body: z.object({
+    /** Document to get the pin info for. */
+    documentId: zodIdType(),
+    /** Collection to which the pin belongs to. If not set, it's considered as "Home" pin. */
+    collectionId: z.string().uuid().nullish(),
+  }),
+});
+
+export type PinsInfoReq = z.infer<typeof PinsInfoSchema>;
+
 export const PinsListSchema = BaseSchema.extend({
   body: z.object({
     collectionId: z.string().uuid().nullish(),
   }),
 });
 
-export type PinsListReq = z.infer<typeof PinsCreateSchema>;
+export type PinsListReq = z.infer<typeof PinsListSchema>;
 
 export const PinsUpdateSchema = BaseSchema.extend({
   body: z.object({