fix/281: allows creating comments non-strapi users (#282)

* fix/281: allows creating comments non-strapi users * fix/281: allows creating comments non-strapi users - tests * chore: version bump, dependencies upgrade --------- Co-authored-by: Mateusz Ziarko <[email protected]>
VirtusLab-Open-Source · Jan 11, 2025 · 500ae48 · 500ae48
1 parent d865f96
commit 500ae48
Show file tree

Hide file tree

Showing 9 changed files with 665 additions and 493 deletions.
diff --git a/README.md b/README.md
@@ -104,7 +104,7 @@ In our minimum support we're following [official Node.js releases timelines](htt
 
 **Supported Strapi versions**:
 
-- Strapi v5.6.0 (recently tested)
+- Strapi v5.7.0 (recently tested)
 - Strapi v5.x
 
 > This plugin is designed for **Strapi v5**. To get support for other Strapi versions, please follow the [versions](#-versions) section.

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "strapi-plugin-comments",
-  "version": "3.0.0",
+  "version": "3.0.1",
   "description": "Strapi - Comments plugin",
   "strapi": {
     "name": "comments",
@@ -67,12 +67,12 @@
   "devDependencies": {
     "@jest/types": "29.5.x",
     "@sensinum/strapi-utils": "^1.0.4",
-    "@strapi/plugin-cloud": "^5.6.0",
-    "@strapi/plugin-graphql": "5.6.0",
-    "@strapi/plugin-users-permissions": "^5.6.0",
+    "@strapi/plugin-cloud": "^5.7.0",
+    "@strapi/plugin-graphql": "5.7.0",
+    "@strapi/plugin-users-permissions": "^5.7.0",
     "@strapi/sdk-plugin": "^5.2.8",
-    "@strapi/strapi": "^5.6.0",
-    "@strapi/types": "^5.6.0",
+    "@strapi/strapi": "^5.7.0",
+    "@strapi/types": "^5.7.0",
     "@types/jest": "^29.5.12",
     "@types/koa": "^2.15.0",
     "@types/react": "^18.3.8",
@@ -90,7 +90,7 @@
   },
   "peerDependencies": {
     "@strapi/sdk-plugin": "^5.2.8",
-    "@strapi/strapi": "^5.6.0",
+    "@strapi/strapi": "^5.7.0",
     "react": "^18.3.1",
     "react-dom": "^18.3.1",
     "react-router-dom": "^6.26.2",

diff --git a/server/src/controllers/__tests__/client.controller.test.ts b/server/src/controllers/__tests__/client.controller.test.ts
@@ -254,7 +254,7 @@ describe('Client controller', () => {
         query: {},
         state: { user: { id: 1 } },
         request: { body: { content: 'Updated content' } }
-      } as RequestContext;
+      } as RequestContext<{ content: string; author: unknown }>;
       const config = { enabledCollections: ['test'] };
       const validatedData = { id: '1', content: 'Updated content' };
       const expectedResult = { id: 1, content: 'Updated content' };
@@ -268,6 +268,81 @@ describe('Client controller', () => {
       expect(result).toEqual(expectedResult);
       expect(mockClientService.update).toHaveBeenCalledWith(validatedData, { id: 1 });
     });
+
+    it('should throw error when store config validation fails', async () => {
+      const ctx = {
+        params: { id: '1' },
+        query: {},
+        state: { user: { id: 1 } },
+        request: { body: { content: 'Updated content' } }
+      } as RequestContext<{ content: string; author: unknown }>;
+      const error = new Error('Config validation failed');
+
+      mockStoreRepository.get.mockResolvedValue({ left: error });
+
+      await expect(getController(getStrapi()).put(ctx)).rejects.toThrow();
+    });
+
+    it('should throw error when comment validation fails', async () => {
+      const ctx = {
+        params: { id: '1' },
+        query: {},
+        state: { user: { id: 1 } },
+        request: { body: { content: '' } }
+      } as RequestContext<{ content: string; author: unknown }>;
+      const config = { enabledCollections: ['test'] };
+      const validationError = new Error('Content cannot be empty');
+
+      mockStoreRepository.get.mockResolvedValue({ right: config });
+      caster<jest.Mock>(clientValidator.updateCommentValidator).mockReturnValue({ left: validationError });
+
+      await expect(getController(getStrapi()).put(ctx)).rejects.toThrow();
+    });
+
+    it('should update comment with custom author when provided', async () => {
+      const ctx = {
+        params: { id: '1' },
+        query: {},
+        state: { user: { id: 1 } },
+        request: { body: { content: 'Updated content', author: { name: 'Custom Author' } } }
+      } as RequestContext<{ content: string; author: unknown }>;
+      const config = { enabledCollections: ['test'] };
+      const validatedData = { 
+        id: '1', 
+        content: 'Updated content',
+        author: { name: 'Custom Author' }
+      };
+      const expectedResult = { 
+        id: 1, 
+        content: 'Updated content',
+        author: { name: 'Custom Author' }
+      };
+
+      mockStoreRepository.get.mockResolvedValue({ right: config });
+      caster<jest.Mock>(clientValidator.updateCommentValidator).mockReturnValue({ right: validatedData });
+      mockClientService.update.mockResolvedValue(expectedResult);
+
+      const result = await getController(getStrapi()).put(ctx);
+
+      expect(result).toEqual(expectedResult);
+      expect(mockClientService.update).toHaveBeenCalledWith(validatedData, { id: 1 });
+    });
+
+    it('should handle update with empty request body', async () => {
+      const ctx = {
+        params: { id: '1' },
+        query: {},
+        state: { user: { id: 1 } },
+        request: { body: {} }
+      } as RequestContext<{ content: string; author: unknown }>;
+      const config = { enabledCollections: ['test'] };
+      const validationError = new Error('Content is required');
+
+      mockStoreRepository.get.mockResolvedValue({ right: config });
+      caster<jest.Mock>(clientValidator.updateCommentValidator).mockReturnValue({ left: validationError });
+
+      await expect(getController(getStrapi()).put(ctx)).rejects.toThrow();
+    });
   });
 
   describe('reportAbuse', () => {

diff --git a/server/src/controllers/client.controller.ts b/server/src/controllers/client.controller.ts
@@ -27,15 +27,15 @@ const controllers = ({ strapi }: StrapiContext) => ({
     }
     throw throwError(ctx, unwrapEither(configResult));
   },
-  
+
   async findAllFlat(ctx: RequestContext<object, Pick<clientValidator.FindAllFlatSchema, 'relation'>>) {
     const configResult = await this.getStoreRepository().get(true);
     if (isRight(configResult)) {
       const config = unwrapEither(configResult);
       const result = clientValidator.findAllFlatValidator(config.enabledCollections, ctx.params.relation, ctx.query);
       if (isRight(result)) {
         return this.getService('common').findAllFlat(
-          flatInput<clientValidator.FindAllFlatSchema>(result.right)
+          flatInput<clientValidator.FindAllFlatSchema>(result.right),
         );
       }
       throw throwError(ctx, unwrapEither(result));
@@ -50,7 +50,7 @@ const controllers = ({ strapi }: StrapiContext) => ({
       const result = clientValidator.findAllInHierarchyValidator(config.enabledCollections, ctx.params.relation, ctx.query);
       if (isRight(result)) {
         return this.getService('common').findAllInHierarchy(
-          flatInput<clientValidator.FindAllInHierarchyValidatorSchema>(result.right)
+          flatInput<clientValidator.FindAllInHierarchyValidatorSchema>(result.right),
         );
       }
       throw throwError(ctx, unwrapEither(result));
@@ -69,14 +69,15 @@ const controllers = ({ strapi }: StrapiContext) => ({
     throw throwError(ctx, unwrapEither(result));
   },
 
-  async put(ctx: RequestContext) {
+  async put(ctx: RequestContext<{ content: string, author: unknown }>) {
     const { user } = ctx.state;
     const configResult = await this.getStoreRepository().get(true);
     if (isRight(configResult)) {
       const config = unwrapEither(configResult);
       const result = clientValidator.updateCommentValidator(config.enabledCollections, {
-        ...ctx.query,
         ...ctx.params,
+        content: ctx.request.body.content,
+        author: ctx.request.body.author,
       });
       if (isRight(result)) {
         return await this.getService('client').update(

diff --git a/server/src/services/__tests__/client.service.test.ts b/server/src/services/__tests__/client.service.test.ts
@@ -142,16 +142,22 @@ describe('client.service', () => {
     const mockPayload = {
       commentId: 1,
       content: 'Updated comment',
+      relation: 'api::test.test:1' as const,
+      author: {
+        id: 1,
+      }
     };
 
-    it('should update a comment when all validations pass', async () => {
+    it('should update a comment when all validations pass with user context', async () => {
       const strapi = getStrapi();
       const service = getService(strapi);
       const mockEntity = { id: 1, content: 'Updated comment' };
       const mockSanitizedEntity = { id: 1, content: 'Clean updated comment' };
 
       mockCommonService.isValidUserContext.mockReturnValue(true);
       mockCommonService.checkBadWords.mockResolvedValue(true);
+      mockCommonService.getConfig.mockResolvedValue([]);
+      mockCommonService.findOne.mockResolvedValue({ id: 1, author: { id: 1 } });
       mockCommentRepository.update.mockResolvedValue(mockEntity);
       mockCommonService.sanitizeCommentEntity.mockReturnValue(mockSanitizedEntity);
 
@@ -165,13 +171,68 @@ describe('client.service', () => {
       });
     });
 
-    it('should throw error when user context is invalid', async () => {
+    it('should update a comment when all validations pass with author context', async () => {
+      const strapi = getStrapi();
+      const service = getService(strapi);
+      const mockEntity = { id: 1, content: 'Updated comment' };
+      const mockSanitizedEntity = { id: 1, content: 'Clean updated comment' };
+
+      mockCommonService.isValidUserContext.mockReturnValue(true);
+      mockCommonService.checkBadWords.mockResolvedValue(true);
+      mockCommonService.getConfig.mockResolvedValue([]);
+      mockCommonService.findOne.mockResolvedValue({ id: 1, author: { id: 1 } });
+      mockCommentRepository.update.mockResolvedValue(mockEntity);
+      mockCommonService.sanitizeCommentEntity.mockReturnValue(mockSanitizedEntity);
+
+      const result = await service.update(mockPayload, undefined);
+
+      expect(result).toEqual(mockSanitizedEntity);
+      expect(mockCommentRepository.update).toHaveBeenCalledWith({
+        where: { id: 1 },
+        data: { content: 'Updated comment' },
+        populate: { threadOf: true, authorUser: true },
+      });
+    });
+
+    it('should throw error when user context is invalid and no author provided', async () => {
       const strapi = getStrapi();
       const service = getService(strapi);
 
       mockCommonService.isValidUserContext.mockReturnValue(false);
 
-      await expect(service.update(mockPayload, mockUser)).rejects.toThrow(PluginError);
+      await expect(service.update({ ...mockPayload, author: null }, undefined)).rejects.toThrow(PluginError);
+    });
+
+    it('should throw error when comment does not exist', async () => {
+      const strapi = getStrapi();
+      const service = getService(strapi);
+
+      mockCommonService.isValidUserContext.mockReturnValue(true);
+      mockCommonService.checkBadWords.mockResolvedValue(true);
+      mockCommonService.findOne.mockResolvedValue(null);
+
+      await expect(service.update(mockPayload, mockUser)).resolves.toBeUndefined();
+    });
+
+    it('should throw error when author id does not match comment author', async () => {
+      const strapi = getStrapi();
+      const service = getService(strapi);
+
+      mockCommonService.isValidUserContext.mockReturnValue(true);
+      mockCommonService.checkBadWords.mockResolvedValue(true);
+      mockCommonService.findOne.mockResolvedValue({ id: 1, author: { id: 2 } });
+
+      await expect(service.update(mockPayload, mockUser)).resolves.toBeUndefined();
+    });
+
+    it('should throw error when bad words check fails', async () => {
+      const strapi = getStrapi();
+      const service = getService(strapi);
+
+      mockCommonService.isValidUserContext.mockReturnValue(true);
+      mockCommonService.checkBadWords.mockResolvedValue(false);
+
+      await expect(service.update(mockPayload, mockUser)).resolves.toBeUndefined();
     });
   });
 
@@ -267,4 +328,4 @@ describe('client.service', () => {
       await expect(service.markAsRemoved(mockPayload, mockUser)).rejects.toThrow(PluginError);
     });
   });
-});
+});
diff --git a/server/src/services/client.service.ts b/server/src/services/client.service.ts
@@ -18,7 +18,7 @@ export const clientService = ({ strapi }: StrapiContext) => {
   const createAuthor = (author: client.NewCommentValidatorSchema['author'], user?: AdminUser) => {
     if (user) {
       return {
-        authorUser: user.id, 
+        authorUser: user.id,
       };
     } else if (author) {
       return {
@@ -61,8 +61,7 @@ export const clientService = ({ strapi }: StrapiContext) => {
         throw unwrapEither(threadData);
       }
       const linkToThread = unwrapEither(threadData);
-      const isValidContext = this.getCommonService().isValidUserContext(user);
-      if (!isValidContext) {
+      if (!author && !this.getCommonService().isValidUserContext(user)) {
         throw resolveUserContextError(user);
       }
 
@@ -103,21 +102,23 @@ export const clientService = ({ strapi }: StrapiContext) => {
     },
 
     // Update a comment
-    async update({ commentId, content }: client.UpdateCommentValidatorSchema, user?: AdminUser) {
-      const validContext = this.getCommonService().isValidUserContext(user);
-
-      if (!validContext) {
+    async update({ commentId, content, author, relation }: client.UpdateCommentValidatorSchema, user?: AdminUser) {
+      if (!author && !this.getCommonService().isValidUserContext(user)) {
         throw resolveUserContextError(user);
       }
-      // TODO: check if user is allowed to update this comment
+      const authorId = user?.id || author?.id;
       if (await this.getCommonService().checkBadWords(content)) {
         const blockedAuthorProps = await this.getCommonService().getConfig(CONFIG_PARAMS.AUTHOR_BLOCKED_PROPS, []);
-        const entity = await getCommentRepository(strapi).update({
-          where: { id: commentId },
-          data: { content },
-          populate: { threadOf: true, authorUser: true },
-        });
-        return this.getCommonService().sanitizeCommentEntity(entity, blockedAuthorProps);
+        const existingComment = await this.getCommonService().findOne({ id: commentId, related: relation });
+
+        if (existingComment && existingComment.author?.id?.toString() === authorId?.toString()) {
+          const entity = await getCommentRepository(strapi).update({
+            where: { id: commentId },
+            data: { content },
+            populate: { threadOf: true, authorUser: true },
+          });
+          return this.getCommonService().sanitizeCommentEntity(entity, blockedAuthorProps);
+        }
       }
     },
 
@@ -174,7 +175,7 @@ export const clientService = ({ strapi }: StrapiContext) => {
     },
 
     async markAsRemoved({ commentId, relation, authorId }: client.RemoveCommentValidatorSchema, user: AdminUser) {
-      if (!this.getCommonService().isValidUserContext(user)) {
+      if (!authorId && !this.getCommonService().isValidUserContext(user)) {
         throw resolveUserContextError(user);
       }
 

diff --git a/server/src/services/common.service.ts b/server/src/services/common.service.ts
@@ -185,12 +185,10 @@ const commonService = ({ strapi }: StrapiContext) => ({
         authorUser: true,
       },
     });
-    console.log('findOne::entity', entity);
     if (!entity) {
       throw new PluginError(400, 'Comment does not exist. Check your payload please.');
     }
     const doNotPopulateAuthor: Array<string> = await this.getConfig(CONFIG_PARAMS.AUTHOR_BLOCKED_PROPS, []);
-    console.log('doNotPopulateAuthor', doNotPopulateAuthor);
     const item = this.sanitizeCommentEntity(entity, doNotPopulateAuthor);
     return filterOurResolvedReports(item);
   },

diff --git a/server/src/validators/api/controllers/client.controller.validator.ts b/server/src/validators/api/controllers/client.controller.validator.ts
@@ -34,7 +34,7 @@ export const updateCommentValidator = (
   payload: object,
 ) => {
   return validate(
-    getCommentSchema(enabledCollections).pick({ content: true, relation: true })
+    getCommentSchema(enabledCollections).pick({ content: true, relation: true, author: true })
                                         .merge(getStringToNumberValidator({ commentId: AVAILABLE_OPERATORS.single }))
                                         .safeParse(payload),
   );