chore: Remove shares.info apiVersion 1 (#3758)

* chore: Remove shares.info apiVersion 1

* fix: Sporadic test failure

app/stores/SharesStore.ts

@@ -59,7 +59,6 @@ export default class SharesStore extends BaseStore<Share> {
     try {
       const res = await client.post(`/${this.modelName}s.info`, {
         documentId,
-        apiVersion: 2,
       });
 
       if (isUndefined(res)) {

server/routes/api/__snapshots__/shares.test.ts.snap

@@ -9,15 +9,6 @@ Object {
 }
 `;
 
-exports[`#shares.info should require authentication 1`] = `
-Object {
-  "error": "authentication_required",
-  "message": "Authentication required",
-  "ok": false,
-  "status": 401,
-}
-`;
-
 exports[`#shares.list should require authentication 1`] = `
 Object {
   "error": "authentication_required",
@@ -44,3 +35,12 @@ Object {
   "status": 401,
 }
 `;
+
+exports[`should require authentication 1`] = `
+Object {
+  "error": "authentication_required",
+  "message": "Authentication required",
+  "ok": false,
+  "status": 401,
+}
+`;

server/routes/api/shares.test.ts

@@ -1,7 +1,13 @@
 import TestServer from "fetch-test-server";
 import { CollectionUser } from "@server/models";
 import webService from "@server/services/web";
-import { buildUser, buildDocument, buildShare } from "@server/test/factories";
+import {
+  buildUser,
+  buildDocument,
+  buildShare,
+  buildAdmin,
+  buildCollection,
+} from "@server/test/factories";
 import { flushdb, seed } from "@server/test/support";
 
 const app = webService();
@@ -287,48 +293,6 @@ describe("#shares.create", () => {
 });
 
 describe("#shares.info", () => {
-  it("should allow reading share by id", async () => {
-    const { user, document } = await seed();
-    const share = await buildShare({
-      documentId: document.id,
-      teamId: user.teamId,
-      userId: user.id,
-    });
-    const res = await server.post("/api/shares.info", {
-      body: {
-        token: user.getJwtToken(),
-        id: share.id,
-      },
-    });
-    const body = await res.json();
-    expect(res.status).toEqual(200);
-    expect(body.data.id).toBe(share.id);
-    expect(body.data.createdBy.id).toBe(user.id);
-  });
-
-  it("should allow reading share created by deleted user", async () => {
-    const { user, document } = await seed();
-    const author = await buildUser({
-      teamId: user.teamId,
-    });
-    const share = await buildShare({
-      documentId: document.id,
-      teamId: author.teamId,
-      userId: author.id,
-    });
-    await author.destroy();
-    const res = await server.post("/api/shares.info", {
-      body: {
-        token: user.getJwtToken(),
-        id: share.id,
-      },
-    });
-    const body = await res.json();
-    expect(res.status).toEqual(200);
-    expect(body.data.id).toBe(share.id);
-    expect(body.data.createdBy.id).toBe(author.id);
-  });
-
   it("should allow reading share by documentId", async () => {
     const { user, document } = await seed();
     const share = await buildShare({
@@ -344,214 +308,162 @@ describe("#shares.info", () => {
     });
     const body = await res.json();
     expect(res.status).toEqual(200);
-    expect(body.data.id).toBe(share.id);
-    expect(body.data.published).toBe(true);
+    expect(body.data.shares.length).toBe(1);
+    expect(body.data.shares[0].id).toBe(share.id);
+    expect(body.data.shares[0].published).toBe(true);
   });
-
-  it("should find share created by another user", async () => {
-    const { admin, document } = await seed();
-    const user = await buildUser({
-      teamId: admin.teamId,
+  it("should return share for parent document with includeChildDocuments=true", async () => {
+    const { user, document, collection } = await seed();
+    const childDocument = await buildDocument({
+      teamId: document.teamId,
+      parentDocumentId: document.id,
+      collectionId: collection.id,
     });
     const share = await buildShare({
       documentId: document.id,
-      teamId: admin.teamId,
-      userId: admin.id,
+      teamId: document.teamId,
+      userId: user.id,
+      includeChildDocuments: true,
     });
+    await collection.addDocumentToStructure(childDocument, 0);
     const res = await server.post("/api/shares.info", {
       body: {
         token: user.getJwtToken(),
-        documentId: document.id,
+        documentId: childDocument.id,
       },
     });
     const body = await res.json();
     expect(res.status).toEqual(200);
-    expect(body.data.id).toBe(share.id);
-    expect(body.data.published).toBe(true);
+    expect(body.data.shares.length).toBe(1);
+    expect(body.data.shares[0].id).toBe(share.id);
+    expect(body.data.shares[0].documentId).toBe(document.id);
+    expect(body.data.shares[0].published).toBe(true);
+    expect(body.data.shares[0].includeChildDocuments).toBe(true);
+    expect(body.policies.length).toBe(1);
+    expect(body.policies[0].abilities.update).toBe(true);
   });
-
-  it("should not find revoked share", async () => {
-    const { user, document } = await seed();
-    const share = await buildShare({
-      documentId: document.id,
-      teamId: user.teamId,
-      userId: user.id,
+  it("should not return share for parent document with includeChildDocuments=false", async () => {
+    const { user, document, collection } = await seed();
+    const childDocument = await buildDocument({
+      teamId: document.teamId,
+      parentDocumentId: document.id,
+      collectionId: collection.id,
     });
-    await share.revoke(user.id);
-    const res = await server.post("/api/shares.info", {
-      body: {
-        token: user.getJwtToken(),
-        documentId: document.id,
-      },
-    });
-    expect(res.status).toEqual(204);
-  });
-
-  it("should not find share for deleted document", async () => {
-    const { user, document } = await seed();
     await buildShare({
       documentId: document.id,
-      teamId: user.teamId,
+      teamId: document.teamId,
       userId: user.id,
+      includeChildDocuments: false,
     });
-    await document.delete(user.id);
+    await collection.addDocumentToStructure(childDocument, 0);
     const res = await server.post("/api/shares.info", {
       body: {
         token: user.getJwtToken(),
-        documentId: document.id,
+        documentId: childDocument.id,
       },
     });
     expect(res.status).toEqual(204);
   });
-  describe("apiVersion=2", () => {
-    it("should allow reading share by documentId", async () => {
-      const { user, document } = await seed();
-      const share = await buildShare({
-        documentId: document.id,
-        teamId: user.teamId,
-        userId: user.id,
-      });
-      const res = await server.post("/api/shares.info", {
-        body: {
-          token: user.getJwtToken(),
-          documentId: document.id,
-          apiVersion: 2,
-        },
-      });
-      const body = await res.json();
-      expect(res.status).toEqual(200);
-      expect(body.data.shares.length).toBe(1);
-      expect(body.data.shares[0].id).toBe(share.id);
-      expect(body.data.shares[0].published).toBe(true);
+  it("should return shares for parent document and current document", async () => {
+    const { user, document, collection } = await seed();
+    const childDocument = await buildDocument({
+      teamId: document.teamId,
+      parentDocumentId: document.id,
+      collectionId: collection.id,
     });
-    it("should return share for parent document with includeChildDocuments=true", async () => {
-      const { user, document, collection } = await seed();
-      const childDocument = await buildDocument({
-        teamId: document.teamId,
-        parentDocumentId: document.id,
-        collectionId: collection.id,
-      });
-      const share = await buildShare({
-        documentId: document.id,
-        teamId: document.teamId,
-        userId: user.id,
-        includeChildDocuments: true,
-      });
-      await collection.addDocumentToStructure(childDocument, 0);
-      const res = await server.post("/api/shares.info", {
-        body: {
-          token: user.getJwtToken(),
-          documentId: childDocument.id,
-          apiVersion: 2,
-        },
-      });
-      const body = await res.json();
-      expect(res.status).toEqual(200);
-      expect(body.data.shares.length).toBe(1);
-      expect(body.data.shares[0].id).toBe(share.id);
-      expect(body.data.shares[0].documentId).toBe(document.id);
-      expect(body.data.shares[0].published).toBe(true);
-      expect(body.data.shares[0].includeChildDocuments).toBe(true);
-      expect(body.policies.length).toBe(1);
-      expect(body.policies[0].abilities.update).toBe(true);
-    });
-    it("should not return share for parent document with includeChildDocuments=false", async () => {
-      const { user, document, collection } = await seed();
-      const childDocument = await buildDocument({
-        teamId: document.teamId,
-        parentDocumentId: document.id,
-        collectionId: collection.id,
-      });
-      await buildShare({
-        documentId: document.id,
-        teamId: document.teamId,
-        userId: user.id,
-        includeChildDocuments: false,
-      });
-      await collection.addDocumentToStructure(childDocument, 0);
-      const res = await server.post("/api/shares.info", {
-        body: {
-          token: user.getJwtToken(),
-          documentId: childDocument.id,
-          apiVersion: 2,
-        },
-      });
-      expect(res.status).toEqual(204);
-    });
-    it("should return shares for parent document and current document", async () => {
-      const { user, document, collection } = await seed();
-      const childDocument = await buildDocument({
-        teamId: document.teamId,
-        parentDocumentId: document.id,
-        collectionId: collection.id,
-      });
-      const share = await buildShare({
-        documentId: childDocument.id,
-        teamId: user.teamId,
-        userId: user.id,
-        includeChildDocuments: false,
-      });
-      const share2 = await buildShare({
-        documentId: document.id,
-        teamId: document.teamId,
-        userId: user.id,
-        includeChildDocuments: true,
-      });
-      await collection.addDocumentToStructure(childDocument, 0);
-      const res = await server.post("/api/shares.info", {
-        body: {
-          token: user.getJwtToken(),
-          documentId: childDocument.id,
-          apiVersion: 2,
-        },
-      });
-      const body = await res.json();
-      expect(res.status).toEqual(200);
-      expect(body.data.shares.length).toBe(2);
-      expect(body.data.shares[0].id).toBe(share.id);
-      expect(body.data.shares[0].includeChildDocuments).toBe(false);
-      expect(body.data.shares[0].documentId).toBe(childDocument.id);
-      expect(body.data.shares[0].published).toBe(true);
-      expect(body.data.shares[1].id).toBe(share2.id);
-      expect(body.data.shares[1].documentId).toBe(document.id);
-      expect(body.data.shares[1].published).toBe(true);
-      expect(body.data.shares[1].includeChildDocuments).toBe(true);
-    });
-  });
-
-  it("should require authentication", async () => {
-    const { user, document } = await seed();
     const share = await buildShare({
-      documentId: document.id,
+      documentId: childDocument.id,
       teamId: user.teamId,
       userId: user.id,
+      includeChildDocuments: false,
     });
+    const share2 = await buildShare({
+      documentId: document.id,
+      teamId: document.teamId,
+      userId: user.id,
+      includeChildDocuments: true,
+    });
+    await collection.addDocumentToStructure(childDocument, 0);
     const res = await server.post("/api/shares.info", {
       body: {
-        id: share.id,
+        token: user.getJwtToken(),
+        documentId: childDocument.id,
       },
     });
     const body = await res.json();
-    expect(res.status).toEqual(401);
-    expect(body).toMatchSnapshot();
+    expect(res.status).toEqual(200);
+    expect(body.data.shares.length).toBe(2);
+    expect(body.data.shares[0].id).toBe(share.id);
+    expect(body.data.shares[0].includeChildDocuments).toBe(false);
+    expect(body.data.shares[0].documentId).toBe(childDocument.id);
+    expect(body.data.shares[0].published).toBe(true);
+    expect(body.data.shares[1].id).toBe(share2.id);
+    expect(body.data.shares[1].documentId).toBe(document.id);
+    expect(body.data.shares[1].published).toBe(true);
+    expect(body.data.shares[1].includeChildDocuments).toBe(true);
   });
+});
 
-  it("should require authorization", async () => {
-    const { admin, document } = await seed();
-    const user = await buildUser();
-    const share = await buildShare({
-      documentId: document.id,
-      teamId: admin.teamId,
-      userId: admin.id,
-    });
-    const res = await server.post("/api/shares.info", {
-      body: {
-        token: user.getJwtToken(),
-        id: share.id,
-      },
-    });
-    expect(res.status).toEqual(403);
+it("should not find share by documentId in private collection", async () => {
+  const admin = await buildAdmin();
+  const collection = await buildCollection({
+    permission: null,
+    teamId: admin.teamId,
   });
+  const document = await buildDocument({
+    collectionId: collection.id,
+    userId: admin.id,
+    teamId: admin.teamId,
+  });
+  const user = await buildUser({
+    teamId: admin.teamId,
+  });
+  await buildShare({
+    documentId: document.id,
+    teamId: admin.teamId,
+    userId: admin.id,
+  });
+  const res = await server.post("/api/shares.info", {
+    body: {
+      token: user.getJwtToken(),
+      documentId: document.id,
+    },
+  });
+  expect(res.status).toEqual(403);
+});
+
+it("should require authentication", async () => {
+  const { user, document } = await seed();
+  const share = await buildShare({
+    documentId: document.id,
+    teamId: user.teamId,
+    userId: user.id,
+  });
+  const res = await server.post("/api/shares.info", {
+    body: {
+      id: share.id,
+    },
+  });
+  const body = await res.json();
+  expect(res.status).toEqual(401);
+  expect(body).toMatchSnapshot();
+});
+
+it("should require authorization", async () => {
+  const { admin, document } = await seed();
+  const user = await buildUser();
+  const share = await buildShare({
+    documentId: document.id,
+    teamId: admin.teamId,
+    userId: admin.id,
+  });
+  const res = await server.post("/api/shares.info", {
+    body: {
+      token: user.getJwtToken(),
+      id: share.id,
+    },
+  });
+  expect(res.status).toEqual(403);
 });
 
 describe("#shares.update", () => {

server/routes/api/shares.ts

@@ -11,8 +11,14 @@ import pagination from "./middlewares/pagination";
 const router = new Router();
 
 router.post("shares.info", auth(), async (ctx) => {
-  const { id, documentId, apiVersion } = ctx.body;
-  assertUuid(id || documentId, "id or documentId is required");
+  const { id, documentId } = ctx.body;
+  assertPresent(id || documentId, "id or documentId is required");
+  if (id) {
+    assertUuid(id, "id is must be a uuid");
+  }
+  if (documentId) {
+    assertUuid(documentId, "documentId is must be a uuid");
+  }
 
   const { user } = ctx.state;
   const shares = [];
@@ -35,37 +41,21 @@ router.post("shares.info", auth(), async (ctx) => {
         },
   });
 
-  // Deprecated API response returns just the share for the current documentId
-  if (apiVersion !== 2) {
-    if (!share || !share.document) {
-      ctx.response.status = 204;
-      return;
-    }
-
-    authorize(user, "read", share);
-    ctx.body = {
-      data: presentShare(share, user.isAdmin),
-      policies: presentPolicies(user, [share]),
-    };
-    return;
-  }
-
-  // API version 2 returns the response for the current documentId and any
-  // parent documents that are publicly shared and accessible to the user
+  // We return the response for the current documentId and any parent documents
+  // that are publicly shared and accessible to the user
   if (share && share.document) {
     authorize(user, "read", share);
     shares.push(share);
   }
 
   if (documentId) {
-    const document = await Document.unscoped()
-      .scope("withCollection")
-      .findOne({
-        where: {
-          id: documentId,
-        },
-      });
-    const parentIds = document?.collection?.getDocumentParents(documentId);
+    const document = await Document.findByPk(documentId, {
+      userId: user.id,
+    });
+    authorize(user, "read", document);
+
+    const collection = await document.$get("collection");
+    const parentIds = collection?.getDocumentParents(documentId);
     const parentShare = parentIds
       ? await Share.scope({
           method: ["withCollectionPermissions", user.id],

server/routes/api/team.test.ts

@@ -95,9 +95,9 @@ describe("#team.update", () => {
     });
     const body = await res.json();
     expect(res.status).toEqual(200);
-    expect(body.data.allowedDomains).toEqual([
-      "example-company.org",
+    expect(body.data.allowedDomains.sort()).toEqual([
       "example-company.net",
+      "example-company.org",
     ]);
 
     const teamDomains: TeamDomain[] = await TeamDomain.findAll({