refactor: remove usage of throwOnForbiddenPermission for item service get (#1721)

* refactor: remove usage of throwOnForbidden for item service get

* refactor: fix tests

* refactor: fix tests

Author: pyphilia

src/services/item/service.test.ts

@@ -1,9 +1,17 @@
+import { v4 } from 'uuid';
+
 import { FastifyInstance } from 'fastify';
 
+import { FolderItemFactory } from '@graasp/sdk';
+
 import build, { clearDatabase } from '../../../test/app';
 import { BaseLogger } from '../../logger';
 import { buildRepositories } from '../../utils/repositories';
+import * as authorization from '../authorization';
+import { Actor } from '../member/entities/member';
+import { saveMember } from '../member/test/fixtures/members';
 import { ThumbnailService } from '../thumbnail/service';
+import { Item } from './entities/Item';
 import { ItemThumbnailService } from './plugins/thumbnail/service';
 import { ItemService } from './service';
 import { ItemTestUtils } from './test/fixtures/items';
@@ -20,32 +28,73 @@ const service = new ItemService(
 
 describe('Item Service', () => {
   let app: FastifyInstance;
-  let actor;
 
-  beforeEach(async () => {
-    ({ app, actor } = await build());
+  beforeAll(async () => {
+    ({ app } = await build({ member: null }));
   });
 
-  afterEach(async () => {
-    jest.clearAllMocks();
+  afterAll(async () => {
     await clearDatabase(app.db);
-    actor = null;
     app.close();
   });
+  afterEach(async () => {
+    jest.clearAllMocks();
+    jest.resetAllMocks();
+  });
+
+  describe('get', () => {
+    it('return item if exists and pass validation', async () => {
+      const actor = { id: v4() } as Actor;
+      const item = FolderItemFactory() as unknown as Item;
+      const repositories = buildRepositories();
+      jest.spyOn(repositories.itemRepository, 'getOneOrThrow').mockResolvedValue(item);
+      jest
+        .spyOn(authorization, 'validatePermission')
+        .mockResolvedValue({ itemMembership: null, visibilities: [] });
+
+      const result = await service.get(actor, repositories, item.id);
+      expect(result).toEqual(item);
+    });
+    it('throw if item does not exists', async () => {
+      const actor = { id: v4() } as Actor;
+      const item = FolderItemFactory() as unknown as Item;
+      const repositories = buildRepositories();
+      jest.spyOn(repositories.itemRepository, 'getOneOrThrow').mockRejectedValue(new Error());
+
+      await expect(() => service.get(actor, repositories, item.id)).rejects.toThrow();
+    });
+    it('throw if validation does not pass', async () => {
+      const actor = { id: v4() } as Actor;
+      const item = FolderItemFactory() as unknown as Item;
+      const repositories = buildRepositories();
+      jest.spyOn(repositories.itemRepository, 'getOneOrThrow').mockResolvedValue(item);
+      jest.spyOn(authorization, 'validatePermission').mockRejectedValue(new Error());
+
+      await expect(() => service.get(actor, repositories, item.id)).rejects.toThrow();
+    });
+  });
   describe('Copy', () => {
     it('Should copy thumbnails on item copy if original has thumbnails', async () => {
+      const actor = await saveMember();
       const { item } = await testUtils.saveItemAndMembership({
         member: actor,
         item: { settings: { hasThumbnail: true } },
       });
+      jest
+        .spyOn(authorization, 'validatePermission')
+        .mockResolvedValue({ itemMembership: null, visibilities: [] });
       await service.copy(actor, buildRepositories(), item.id);
       expect(mockedThumbnailService.copyFolder).toHaveBeenCalled();
     });
     it('Should not copy thumbnails on item copy if original has no thumbnails', async () => {
+      const actor = await saveMember();
       const { item } = await testUtils.saveItemAndMembership({
         member: actor,
         item: { settings: { hasThumbnail: false } },
       });
+      jest
+        .spyOn(authorization, 'validatePermission')
+        .mockResolvedValue({ itemMembership: null, visibilities: [] });
       await service.copy(actor, buildRepositories(), item.id);
       expect(mockedThumbnailService.copyFolder).not.toHaveBeenCalled();
     });

src/services/item/service.ts

@@ -215,21 +215,16 @@ export class ItemService {
     repositories: Repositories,
     id: string,
     permission: PermissionLevel = PermissionLevel.Read,
-    throwOnForbiddenPermission: boolean = true,
   ) {
     const item = await repositories.itemRepository.getOneOrThrow(id);
 
-    if (throwOnForbiddenPermission) {
-      const { itemMembership, visibilities } = await validatePermission(
-        repositories,
-        permission,
-        actor,
-        item,
-      );
-      return { item, itemMembership, visibilities };
-    }
-
-    return { item, itemMembership: null, visibilities: [] };
+    const { itemMembership, visibilities } = await validatePermission(
+      repositories,
+      permission,
+      actor,
+      item,
+    );
+    return { item, itemMembership, visibilities };
   }
 
   /**
@@ -245,15 +240,8 @@ export class ItemService {
     repositories: Repositories,
     id: string,
     permission: PermissionLevel = PermissionLevel.Read,
-    throwOnForbiddenPermission?: boolean,
   ) {
-    const { item } = await this._get(
-      actor,
-      repositories,
-      id,
-      permission,
-      throwOnForbiddenPermission,
-    );
+    const { item } = await this._get(actor, repositories, id, permission);
 
     return item;
   }

src/services/itemLogin/index.ts

@@ -43,13 +43,7 @@ const plugin: FastifyPluginAsyncTypebox = async (fastify) => {
         const repositories = buildRepositories(manager);
 
         // Get item to have the path
-        const item = await itemService.get(
-          user?.account,
-          repositories,
-          itemId,
-          PermissionLevel.Read,
-          false,
-        );
+        const item = await repositories.itemRepository.getOneOrThrow(itemId);
 
         // If item is not visible, throw NOT_FOUND
         const isVisible = await isItemVisible(

src/services/itemMembership/plugins/MembershipRequest/index.ts

@@ -88,13 +88,7 @@ const plugin: FastifyPluginAsyncTypebox = async (fastify) => {
           return reply.send({ status: MembershipRequestStatus.Approved });
         }
 
-        const item = await itemService.get(
-          member,
-          repositories,
-          itemId,
-          PermissionLevel.Read,
-          false,
-        );
+        const item = await repositories.itemRepository.getOneOrThrow(itemId);
         if (item) {
           return reply.send({ status: MembershipRequestStatus.NotSubmittedOrDeleted });
         }
@@ -127,13 +121,7 @@ const plugin: FastifyPluginAsyncTypebox = async (fastify) => {
           throw new MembershipRequestAlreadyExists();
         }
 
-        const item = await itemService.get(
-          member,
-          repositories,
-          itemId,
-          PermissionLevel.Read,
-          false,
-        );
+        const item = await repositories.itemRepository.getOneOrThrow(itemId);
 
         const itemLoginSchema = await itemLoginService.getByItemPath(repositories, item.path);
         if (itemLoginSchema) {