From 17f14e1724b7b6b6acddd09bf2e1523e9ed703ab Mon Sep 17 00:00:00 2001 From: BradyMitch Date: Thu, 7 Dec 2023 14:04:57 -0800 Subject: [PATCH 1/4] Added createReply method for POST /comments/reply --- .../modules/comments/comments.controller.ts | 34 +++++++- .../src/modules/comments/comments.service.ts | 79 +++++++++++++++++++ .../modules/comments/dto/create-reply.dto.ts | 20 +++++ .../src/modules/comments/ro/get-comment.ro.ts | 11 +++ 4 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 src/backend/src/modules/comments/dto/create-reply.dto.ts diff --git a/src/backend/src/modules/comments/comments.controller.ts b/src/backend/src/modules/comments/comments.controller.ts index 83550477d..8e51d4f79 100644 --- a/src/backend/src/modules/comments/comments.controller.ts +++ b/src/backend/src/modules/comments/comments.controller.ts @@ -26,7 +26,8 @@ import { CreateCommentDto } from './dto/create-comment.dto'; import { FindCommentsCountDto } from './dto/find-comments-count.dto'; import { FindCommentsDto } from './dto/find-comments.dto'; import { CommentsCountRO } from './ro/comments-count-ro'; -import { CommentRO } from './ro/get-comment.ro'; +import { CommentRO, ReplyRO } from './ro/get-comment.ro'; +import { CreateReplyDto } from './dto/create-reply.dto'; @Controller('comments') @ApiTags('Comments') @@ -63,6 +64,37 @@ export class CommentsController { ); } + // Create Reply + @Post('reply') + @HttpCode(201) + @ApiOperation({ + description: 'Create a new comment reply', + }) + @ApiCreatedResponse({ + description: 'Successfully created a new comment reply', + }) + @ApiForbiddenResponse({ + description: + 'Failed to create comment reply: User lacks permission to create comment reply to this PIA', + }) + @ApiNotFoundResponse({ + description: + 'Failed to create comment reply: Comment for the id provided not found', + }) + @ApiGoneResponse({ + description: 'Failed to create comment reply: The PIA is not active', + }) + createReply( + @Body() createReplyDto: CreateReplyDto, + @Req() req: IRequest, + ): Promise { + return this.commentsService.createReply( + createReplyDto, + req.user, + req.userRoles, + ); + } + @Get() @HttpCode(200) @ApiOperation({ diff --git a/src/backend/src/modules/comments/comments.service.ts b/src/backend/src/modules/comments/comments.service.ts index 6e3f4c4d8..046b4ace0 100644 --- a/src/backend/src/modules/comments/comments.service.ts +++ b/src/backend/src/modules/comments/comments.service.ts @@ -20,15 +20,20 @@ import { } from './ro/comments-count-ro'; import { CommentRO, + ReplyRO, getFormattedComment, getFormattedComments, + getFormattedReply, } from './ro/get-comment.ro'; +import { CreateReplyDto } from './dto/create-reply.dto'; +import { ReplyEntity } from './entities/reply.entity'; @Injectable() export class CommentsService { constructor( @InjectRepository(CommentEntity) private commentRepository: Repository, + private replyRepository: Repository, private readonly piaService: PiaIntakeService, ) {} @@ -101,6 +106,68 @@ export class CommentsService { return getFormattedComment(comment); } + async createReply( + createReplyDto: CreateReplyDto, + user: KeycloakUser, + userRoles: Array, + ): Promise { + // extract user input dto + const { commentId, text } = createReplyDto; + const parentComment = await this.findById(commentId); + + // validate comment exists + if (!parentComment) + throw new NotFoundException({ + commentId, + message: 'No comment found by the id provided.', + }); + + // validate access to PIA. Throw error if not + const pia = await this.piaService.validatePiaAccess( + parentComment.piaId, + user, + userRoles, + ); + + // validate blank text + if ((text || '').trim() === '') { + throw new ForbiddenException({ + piaId: pia.id, + message: + 'Forbidden: Failed to add comment reply to the PIA. Text cannot be blank.', + }); + } + + // check if adding comments to this PIA allowed + const isActionAllowed = checkUpdatePermissions({ + status: pia?.status, + entityType: 'comment', + entityAction: 'add', + }); + + if (!isActionAllowed) { + throw new ForbiddenException({ + piaId: pia.id, + message: 'Forbidden: Failed to add comment reply to the PIA', + }); + } + + // create resource + const reply: ReplyEntity = await this.replyRepository.save({ + comment: parentComment, + text, + isResolved: false, + createdByGuid: user.idir_user_guid, + createdByUsername: user.idir_username, + updatedByGuid: user.idir_user_guid, + updatedByUsername: user.idir_username, + createdByDisplayName: user.display_name, + }); + + // return formatted object + return getFormattedReply(reply); + } + async findByPiaAndPath( piaId: number, path: AllowedCommentPaths, @@ -125,6 +192,18 @@ export class CommentsService { return getFormattedComments(comments); } + async findById(id: number): Promise { + const comment = await this.commentRepository.findOne({ + where: { id }, + }); + + if (!comment) { + throw new NotFoundException(`Comment with ID ${id} not found.`); + } + + return getFormattedComment(comment); + } + async findCountByPia( piaId: number, user: KeycloakUser, diff --git a/src/backend/src/modules/comments/dto/create-reply.dto.ts b/src/backend/src/modules/comments/dto/create-reply.dto.ts new file mode 100644 index 000000000..59a68dd16 --- /dev/null +++ b/src/backend/src/modules/comments/dto/create-reply.dto.ts @@ -0,0 +1,20 @@ +import { IsNumber, IsString } from '@nestjs/class-validator'; +import { ApiProperty } from '@nestjs/swagger'; + +export class CreateReplyDto { + @IsNumber() + @ApiProperty({ + type: Number, + required: true, + example: 1, + }) + commentId: number; + + @IsString() + @ApiProperty({ + type: String, + required: true, + example: 'This is a sample comment', + }) + text: string; +} diff --git a/src/backend/src/modules/comments/ro/get-comment.ro.ts b/src/backend/src/modules/comments/ro/get-comment.ro.ts index ff308caee..50797d181 100644 --- a/src/backend/src/modules/comments/ro/get-comment.ro.ts +++ b/src/backend/src/modules/comments/ro/get-comment.ro.ts @@ -3,18 +3,29 @@ import { omitBaseKeys, } from 'src/common/helpers/base-helper'; import { CommentEntity } from '../entities/comment.entity'; +import { ReplyEntity } from '../entities/reply.entity'; export const excludeCommentKeys = { pia: true }; +export const excludeReplyKeys = { comment: true }; export type CommentRO = Omit< CommentEntity, keyof ExcludeBaseSelection | keyof typeof excludeCommentKeys >; +export type ReplyRO = Omit< + ReplyEntity, + keyof ExcludeBaseSelection | keyof typeof excludeReplyKeys +>; + export const getFormattedComment = (comment: CommentEntity) => { return omitBaseKeys(comment, Object.keys(excludeCommentKeys)); }; +export const getFormattedReply = (reply: ReplyEntity) => { + return omitBaseKeys(reply, Object.keys(excludeReplyKeys)); +}; + export const getFormattedComments = (comments: CommentEntity[]) => { return comments.map((comment) => getFormattedComment(comment)); }; From 5f7c90f5e42cccf420466d2415f19164d2b4e228 Mon Sep 17 00:00:00 2001 From: BradyMitch Date: Thu, 7 Dec 2023 14:36:44 -0800 Subject: [PATCH 2/4] Added removeReply for DELETE /comments/reply/:id --- .../modules/comments/comments.controller.ts | 25 ++++++ .../src/modules/comments/comments.service.ts | 82 +++++++++++++++---- 2 files changed, 93 insertions(+), 14 deletions(-) diff --git a/src/backend/src/modules/comments/comments.controller.ts b/src/backend/src/modules/comments/comments.controller.ts index 8e51d4f79..df688f0d7 100644 --- a/src/backend/src/modules/comments/comments.controller.ts +++ b/src/backend/src/modules/comments/comments.controller.ts @@ -176,6 +176,31 @@ export class CommentsController { return this.commentsService.remove(+id, req.user, req.userRoles); } + // Delete Reply + @Delete('reply/:id') + @ApiOperation({ + description: 'Deletes a comment reply', + }) + @ApiOkResponse({ + description: 'Successfully deleted a comment reply', + }) + @ApiBadRequestResponse({ + description: 'Failed to delete comment reply: Invalid request', + }) + @ApiForbiddenResponse({ + description: + 'Failed to delete comment: User lacks permission to delete comment reply of this PIA', + }) + @ApiNotFoundResponse({ + description: 'Failed to delete comment reply: Comment not found', + }) + @ApiGoneResponse({ + description: 'Failed to delete comment reply: The PIA is not active', + }) + removeReply(@Param('id') id: string, @Req() req: IRequest): Promise { + return this.commentsService.removeReply(+id, req.user, req.userRoles); + } + @Post(':id/resolve') resolve(@Param('id') id: string) { return this.commentsService.resolve(+id); diff --git a/src/backend/src/modules/comments/comments.service.ts b/src/backend/src/modules/comments/comments.service.ts index 046b4ace0..b701943f6 100644 --- a/src/backend/src/modules/comments/comments.service.ts +++ b/src/backend/src/modules/comments/comments.service.ts @@ -51,6 +51,19 @@ export class CommentsService { return comment; } + async findOneReplyBy( + where: FindOptionsWhere, + ): Promise { + const reply: ReplyEntity = await this.replyRepository.findOneBy(where); + + // If the record is not found, throw an exception + if (!reply) { + throw new NotFoundException(); + } + + return reply; + } + async create( createCommentDto: CreateCommentDto, user: KeycloakUser, @@ -113,7 +126,7 @@ export class CommentsService { ): Promise { // extract user input dto const { commentId, text } = createReplyDto; - const parentComment = await this.findById(commentId); + const parentComment = await this.findOneBy({ id: commentId }); // validate comment exists if (!parentComment) @@ -192,18 +205,6 @@ export class CommentsService { return getFormattedComments(comments); } - async findById(id: number): Promise { - const comment = await this.commentRepository.findOne({ - where: { id }, - }); - - if (!comment) { - throw new NotFoundException(`Comment with ID ${id} not found.`); - } - - return getFormattedComment(comment); - } - async findCountByPia( piaId: number, user: KeycloakUser, @@ -236,7 +237,7 @@ export class CommentsService { // if the comment person who created the comment is not the one deleting, throw error if (user.idir_user_guid !== comment.createdByGuid) { throw new ForbiddenException({ - message: "Forbidden: You're are not authorized to remoe this comment", + message: "Forbidden: You're are not authorized to remove this comment", }); } @@ -276,6 +277,59 @@ export class CommentsService { return getFormattedComment(updatedComment); } + // Remove Reply + async removeReply( + id: number, + user: KeycloakUser, + userRoles: Array, + ): Promise { + // fetch reply + const reply = await this.findOneReplyBy({ id }); + const parentComment = await this.findOneBy({ id: reply.commentId }); + + // if the comment person who created the comment is not the one deleting, throw error + if (user.idir_user_guid !== reply.createdByGuid) { + throw new ForbiddenException({ + message: "Forbidden: You're are not authorized to remove this reply", + }); + } + + // validate access to PIA. Throw error if not + const pia = await this.piaService.validatePiaAccess( + parentComment.piaId, + user, + userRoles, + ); + + // check if deleting comments to this PIA allowed + const isActionAllowed = checkUpdatePermissions({ + status: pia?.status, + entityType: 'comment', + entityAction: 'remove', + }); + + if (!isActionAllowed) { + throw new ForbiddenException({ + piaId: pia.id, + message: 'Forbidden: Failed to remove comment reply of the PIA', + }); + } + + // throw error if comment already deleted + if (reply.isActive === false) { + throw new BadRequestException('Reply already deleted'); + } + + // soft delete + const updatedReply = await this.replyRepository.save({ + ...reply, + isActive: false, + text: null, + }); + + return getFormattedReply(updatedReply); + } + // TODO async resolve(id: number) { return `This is a resolve method yet to be developed for comment ${id}`; From 34e26cacb7ef522ce4442c52fa6185a16dfdb430 Mon Sep 17 00:00:00 2001 From: BradyMitch Date: Tue, 12 Dec 2023 09:15:14 -0800 Subject: [PATCH 3/4] Updates tests --- .../src/modules/comments/comments.service.ts | 1 + .../unit/comments/comments.controller.spec.ts | 82 +++++++++++++ .../unit/comments/comments.service.spec.ts | 115 +++++++++++++++++- .../test/util/mocks/data/comments.mock.ts | 20 ++- 4 files changed, 216 insertions(+), 2 deletions(-) diff --git a/src/backend/src/modules/comments/comments.service.ts b/src/backend/src/modules/comments/comments.service.ts index b701943f6..d26085b57 100644 --- a/src/backend/src/modules/comments/comments.service.ts +++ b/src/backend/src/modules/comments/comments.service.ts @@ -33,6 +33,7 @@ export class CommentsService { constructor( @InjectRepository(CommentEntity) private commentRepository: Repository, + @InjectRepository(ReplyEntity) private replyRepository: Repository, private readonly piaService: PiaIntakeService, ) {} diff --git a/src/backend/test/unit/comments/comments.controller.spec.ts b/src/backend/test/unit/comments/comments.controller.spec.ts index f007f9537..82fa129fa 100644 --- a/src/backend/test/unit/comments/comments.controller.spec.ts +++ b/src/backend/test/unit/comments/comments.controller.spec.ts @@ -14,11 +14,15 @@ import { commentROMock, commentsCountROMock, createCommentDtoMock, + createReplyDtoMock, findCommentsROMock, + replyROMock, } from 'test/util/mocks/data/comments.mock'; import { FindCommentsDto } from 'src/modules/comments/dto/find-comments.dto'; import { AllowedCommentPaths } from 'src/modules/comments/enums/allowed-comment-paths.enum'; import { FindCommentsCountDto } from 'src/modules/comments/dto/find-comments-count.dto'; +import { CreateReplyDto } from 'src/modules/comments/dto/create-reply.dto'; +import { ReplyRO } from 'src/modules/comments/ro/get-comment.ro'; /** * @Description @@ -102,6 +106,42 @@ describe('CommentsController', () => { }); }); + /** + * @method createReply + * + * @description + * This test suite validates that the method passes the correct values to the service, + * mock the service result and return correct result to the user + */ + describe('`createReply` method', () => { + it('should be defined', () => { + expect(controller.createReply).toBeDefined(); + }); + + it('succeeds with correct data : Happy flow', async () => { + const createReplyDto: CreateReplyDto = { ...createReplyDtoMock }; + const mockReq: any = { + user: { ...keycloakUserMock }, + userRoles: [], + }; + + service.createReply = jest.fn(async () => { + delay(10); + return replyROMock; + }); + + const result = await controller.createReply(createReplyDto, mockReq); + + expect(service.createReply).toHaveBeenCalledWith( + createReplyDto, + mockReq.user, + mockReq.userRoles, + ); + + expect(result).toStrictEqual(replyROMock); + }); + }); + /** * @Description * This test suite validates that the method passes the correct values to the service, @@ -245,6 +285,48 @@ describe('CommentsController', () => { }); }); + /** + * @method removeReply + * + * @description + * This test suite validates that the method passes the correct values to the service, + * mock the service result and return correct result to the user + */ + describe('`removeReply` method', () => { + it('should be defined', () => { + expect(controller.removeReply).toBeDefined(); + }); + + it('succeeds with correct data', async () => { + const replyId = '101'; // Example ID + const mockReq: any = { + user: { ...keycloakUserMock }, + userRoles: [], + }; + + const expectedResponse: ReplyRO = { + ...replyROMock, + isActive: false, + text: null, + }; + + service.removeReply = jest.fn(async () => { + delay(10); + return expectedResponse; + }); + + const result = await controller.removeReply(replyId, mockReq); + + expect(service.removeReply).toHaveBeenCalledWith( + +replyId, + mockReq.user, + mockReq.userRoles, + ); + + expect(result).toStrictEqual(expectedResponse); + }); + }); + /** * @method resolve * diff --git a/src/backend/test/unit/comments/comments.service.spec.ts b/src/backend/test/unit/comments/comments.service.spec.ts index 5a714e770..b6fe211e6 100644 --- a/src/backend/test/unit/comments/comments.service.spec.ts +++ b/src/backend/test/unit/comments/comments.service.spec.ts @@ -32,6 +32,7 @@ import { CreateCommentDto } from 'src/modules/comments/dto/create-comment.dto'; import { AllowedCommentPaths } from 'src/modules/comments/enums/allowed-comment-paths.enum'; import { piaIntakeServiceMock } from 'test/util/mocks/services/pia-intake.service.mock'; import * as checkUpdatePermissions from 'src/modules/pia-intake/helper/check-update-permissions'; +import { ReplyEntity } from 'src/modules/comments/entities/reply.entity'; /** * @Description @@ -40,7 +41,7 @@ import * as checkUpdatePermissions from 'src/modules/pia-intake/helper/check-upd describe('CommentsService', () => { let commentsService: CommentsService; let piaService: PiaIntakeService; - let commentRepository; + let commentRepository, replyRepository; const checkUpdatePermissionsSpy = jest .spyOn(checkUpdatePermissions, 'checkUpdatePermissions') @@ -60,6 +61,10 @@ describe('CommentsService', () => { provide: getRepositoryToken(CommentEntity), useValue: { ...repositoryMock }, }, + { + provide: getRepositoryToken(ReplyEntity), + useValue: { ...repositoryMock }, + }, { provide: getRepositoryToken(PiaIntakeEntity), useValue: { ...repositoryMock }, @@ -70,6 +75,7 @@ describe('CommentsService', () => { commentsService = module.get(CommentsService); piaService = module.get(PiaIntakeService); commentRepository = module.get(getRepositoryToken(CommentEntity)); + replyRepository = module.get(getRepositoryToken(ReplyEntity)); checkUpdatePermissionsSpy.mockClear(); }); @@ -195,6 +201,71 @@ describe('CommentsService', () => { }); }); + /** + * @Description + * These set of tests validates that the method passes the correct values to the repository, + * mocking the database save operation. + * + * @method createReply + */ + describe('`createReply` method', () => { + it('should be defined', () => { + expect(commentsService.createReply).toBeDefined(); + }); + + it('succeeds creating a reply to a comment', async () => { + const createReplyDto = { + commentId: commentEntityMock.id, + text: 'This is a test reply', + }; + const user = { ...keycloakUserMock }; + const userRoles = []; + + commentsService.findOneBy = jest + .fn() + .mockResolvedValue(commentEntityMock); + + piaService.validatePiaAccess = jest.fn(async () => ({ + ...piaIntakeEntityMock, + })); + + checkUpdatePermissionsSpy.mockReturnValue(true); // Allow adding a reply + + const mockReply = { + id: 101, + commentId: commentEntityMock.id, + text: 'This is a test reply', + createdByDisplayName: user.display_name, + createdAt: new Date(), + updatedAt: new Date(), + }; + + replyRepository.save = jest.fn().mockResolvedValue(mockReply); + + const result = await commentsService.createReply( + createReplyDto, + user, + userRoles, + ); + + expect(commentsService.findOneBy).toHaveBeenCalledWith({ + id: createReplyDto.commentId, + }); + expect(piaService.validatePiaAccess).toHaveBeenCalledWith( + commentEntityMock.piaId, + user, + userRoles, + ); + expect(checkUpdatePermissionsSpy).toHaveBeenCalledWith({ + status: piaIntakeEntityMock.status, + entityType: 'comment', + entityAction: 'add', + }); + expect(replyRepository.save).toHaveBeenCalledWith(expect.any(Object)); + expect(result).toEqual(mockReply); + }); + }); + /** * @Description * These set of tests validates that the method passes the correct values to the repository, @@ -491,6 +562,48 @@ describe('CommentsService', () => { }); }); + /** + * @Description + * These set of tests validates that the method passes the correct values to the repository, + * removes the comment, mocking the database delete operation. + * + * @method removeReply + */ + describe('`removeReply` method', () => { + it('should be defined', () => { + expect(commentsService.removeReply).toBeDefined(); + }); + + it('succeeds in removing a reply', async () => { + const id = 101; // example reply ID + const user = { ...keycloakUserMock }; + const userRoles = []; + + const mockReply = { + id, + commentId: commentEntityMock.id, + createdByGuid: user.idir_user_guid, + }; + + commentsService.findOneReplyBy = jest.fn().mockResolvedValue(mockReply); + commentsService.findOneBy = jest + .fn() + .mockResolvedValue(commentEntityMock); + + const updatedMockReply = { ...mockReply, isActive: false, text: null }; + replyRepository.save = jest.fn().mockResolvedValue(updatedMockReply); + + const result = await commentsService.removeReply(id, user, userRoles); + + expect(commentsService.findOneReplyBy).toHaveBeenCalledWith({ id }); + expect(commentsService.findOneBy).toHaveBeenCalledWith({ + id: mockReply.commentId, + }); + expect(replyRepository.save).toHaveBeenCalledWith(expect.any(Object)); + expect(result).toEqual(updatedMockReply); + }); + }); + /** * @Description * These set of tests validates that the method passes the correct values to the repository, diff --git a/src/backend/test/util/mocks/data/comments.mock.ts b/src/backend/test/util/mocks/data/comments.mock.ts index 33507776f..9199e5d7b 100644 --- a/src/backend/test/util/mocks/data/comments.mock.ts +++ b/src/backend/test/util/mocks/data/comments.mock.ts @@ -5,9 +5,10 @@ import { CommentsCountDbRO, CommentsCountRO, } from 'src/modules/comments/ro/comments-count-ro'; -import { CommentRO } from 'src/modules/comments/ro/get-comment.ro'; +import { CommentRO, ReplyRO } from 'src/modules/comments/ro/get-comment.ro'; import { keycloakUserMock } from './auth.mock'; import { piaIntakeEntityMock } from './pia-intake.mock'; +import { CreateReplyDto } from 'src/modules/comments/dto/create-reply.dto'; export const createCommentDtoMock: CreateCommentDto = { path: AllowedCommentPaths['accuracyCorrectionAndRetention.accuracy'], @@ -68,3 +69,20 @@ export const commentsCountDbRO: CommentsCountDbRO = { export const commentsCountROMock: Partial = { [commentsCountDbRO.path]: +commentsCountDbRO.count, }; + +export const createReplyDtoMock: CreateReplyDto = { + commentId: 1, + text: 'This is a test reply', +}; + +export const replyROMock: ReplyRO = { + id: 101, + commentId: 1, + text: 'Test Reply 1', + createdByDisplayName: 'Test User', + createdAt: new Date(), + updatedAt: new Date(), + isResolved: false, + isActive: true, + createdByGuid: keycloakUserMock.idir_user_guid, +}; From 5ee3ea05461772838e76723055c06b91d88a8d27 Mon Sep 17 00:00:00 2001 From: BradyMitch Date: Tue, 12 Dec 2023 10:25:55 -0800 Subject: [PATCH 4/4] Added replies to get comments --- .../src/modules/comments/comments.service.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/backend/src/modules/comments/comments.service.ts b/src/backend/src/modules/comments/comments.service.ts index d26085b57..ac5757f25 100644 --- a/src/backend/src/modules/comments/comments.service.ts +++ b/src/backend/src/modules/comments/comments.service.ts @@ -202,8 +202,21 @@ export class CommentsService { order: { createdAt: 1 }, }); + // Add replies + const commentsWithReplies = []; + for (const comment of comments) { + // Fetch reply + const replies = await this.replyRepository.find({ + where: { + commentId: comment.id, + }, + order: { createdAt: 1 }, + }); + commentsWithReplies.push({ ...comment, replies }); + } + // return formatted objects - return getFormattedComments(comments); + return getFormattedComments(commentsWithReplies); } async findCountByPia(