From eaa34306f7c35b6e7a7c0788a4492578870d7e58 Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Tue, 9 Jul 2024 19:55:48 +1000 Subject: [PATCH 1/8] remove teamId from Patch ideation endpoint and change ideations default path --- .../ability.factory/ability.factory.ts | 3 ++ src/ability/conditions/ideations.ability.ts | 38 +++++++++++++ src/app.module.ts | 2 +- src/ideations/ideations.controller.ts | 18 +++---- src/ideations/ideations.service.ts | 53 ++++++------------- test/ideations.e2e-spec.ts | 17 ++---- 6 files changed, 70 insertions(+), 61 deletions(-) diff --git a/src/ability/ability.factory/ability.factory.ts b/src/ability/ability.factory/ability.factory.ts index beb2c84b..af6fc9bb 100644 --- a/src/ability/ability.factory/ability.factory.ts +++ b/src/ability/ability.factory/ability.factory.ts @@ -32,6 +32,9 @@ export class AbilityFactory { can([Action.Manage], "VoyageTeam", { id: { in: user.voyageTeams.map((vt) => vt.teamId) }, }); + // For Ideation and Tech stack, we make the permission team based here + // as there are times we'll need them to be able to manage other team members ideations/tech + // more specific permission checks can be found in `ideations.ability.ts` etc can([Action.Manage], "Ideation", { voyageTeamMemberId: { in: user.voyageTeams.map((vt) => vt.memberId), diff --git a/src/ability/conditions/ideations.ability.ts b/src/ability/conditions/ideations.ability.ts index e69de29b..ffc1e280 100644 --- a/src/ability/conditions/ideations.ability.ts +++ b/src/ability/conditions/ideations.ability.ts @@ -0,0 +1,38 @@ +import { UserReq } from "../../global/types/CustomRequest"; +import prisma from "../../prisma/client"; +import { ForbiddenException, NotFoundException } from "@nestjs/common"; + +// Note: Use methods in `voyage-teams.ability.ts` to check for team ideation management + +// Check if the user "owns" this ideation +export const manageOwnIdeationById = async ( + user: UserReq, + ideationId: number, +) => { + if (user.roles?.includes("admin")) return; + const ideation = await prisma.projectIdea.findUnique({ + where: { + id: ideationId, + }, + }); + if (!ideation) { + throw new NotFoundException(`Ideation (id:${ideationId} not found`); + } + // ideation is not linked to any members, this should never happen unless the user gets deleted + // or removed from the team? + // in this case we should let the rest of the team manage it + if (!ideation.voyageTeamMemberId) { + throw new ForbiddenException( + `Ideation access control: You do not have sufficient permission to access this resource.`, + ); + } + if ( + !user.voyageTeams + .map((vt) => vt.memberId) + .includes(ideation.voyageTeamMemberId) + ) { + throw new ForbiddenException( + "Ideation access control: You can only manage your own ideations.", + ); + } +}; diff --git a/src/app.module.ts b/src/app.module.ts index 130a2971..92bdd030 100644 --- a/src/app.module.ts +++ b/src/app.module.ts @@ -34,7 +34,7 @@ import { DevelopmentModule } from "./development/development.module"; { path: "/", module: TechsModule }, { path: "/", module: FeaturesModule }, { - path: "teams/:teamId/ideations", + path: "/", module: IdeationsModule, }, { path: "sprints", module: SprintsModule }, diff --git a/src/ideations/ideations.controller.ts b/src/ideations/ideations.controller.ts index 588a171e..a1750adc 100644 --- a/src/ideations/ideations.controller.ts +++ b/src/ideations/ideations.controller.ts @@ -55,7 +55,7 @@ export class IdeationsController { type: IdeationResponse, }) @CheckAbilities({ action: Action.Create, subject: "Ideation" }) - @Post() + @Post("teams/:teamId/ideations") createIdeation( @Request() req: CustomRequest, @Param("teamId", ParseIntPipe) teamId: number, @@ -94,7 +94,7 @@ export class IdeationsController { type: IdeationVoteResponse, }) @CheckAbilities({ action: Action.Create, subject: "Ideation" }) - @Post("/:ideationId/ideation-votes") + @Post("teams/:teamId/ideations/:ideationId/ideation-votes") createIdeationVote( @Request() req: CustomRequest, @Param("teamId", ParseIntPipe) teamId: number, @@ -122,7 +122,7 @@ export class IdeationsController { type: TeamIdeationsResponse, }) @CheckAbilities({ action: Action.Read, subject: "Ideation" }) - @Get() + @Get("teams/:teamId/ideations") getIdeationsByVoyageTeam( @Param("teamId", ParseIntPipe) teamId: number, @Request() req: CustomRequest, @@ -151,17 +151,15 @@ export class IdeationsController { type: IdeationResponse, }) @CheckAbilities({ action: Action.Update, subject: "Ideation" }) - @Patch("/:ideationId") + @Patch("ideations/:ideationId") updateIdeation( @Request() req: CustomRequest, @Param("ideationId", ParseIntPipe) ideationId: number, - @Param("teamId", ParseIntPipe) teamId: number, @Body() updateIdeationDto: UpdateIdeationDto, ) { return this.ideationsService.updateIdeation( req, ideationId, - teamId, updateIdeationDto, ); } @@ -197,7 +195,7 @@ export class IdeationsController { type: ConflictErrorResponse, }) @CheckAbilities({ action: Action.Delete, subject: "Ideation" }) - @Delete("/:ideationId") + @Delete("teams/:teamId/ideations/:ideationId") deleteIdeation( @Request() req: CustomRequest, @Param("teamId", ParseIntPipe) teamId: number, @@ -227,7 +225,7 @@ export class IdeationsController { type: IdeationVoteResponse, }) @CheckAbilities({ action: Action.Delete, subject: "Ideation" }) - @Delete("/:ideationId/ideation-votes") + @Delete("teams/:teamId/ideations/:ideationId/ideation-votes") deleteIdeationVote( @Request() req: CustomRequest, @Param("teamId", ParseIntPipe) teamId: number, @@ -278,7 +276,7 @@ export class IdeationsController { example: 1, }) @CheckAbilities({ action: Action.Manage, subject: "Ideation" }) - @Post("/:ideationId/select") + @Post("teams/:teamId/ideations/:ideationId/select") setIdeationSelection( @Request() req: CustomRequest, @Param("teamId", ParseIntPipe) teamId: number, @@ -318,7 +316,7 @@ export class IdeationsController { example: 1, }) @CheckAbilities({ action: Action.Manage, subject: "all" }) - @Post("/reset-selection") + @Post("teams/:teamId/ideations/reset-selection") resetIdeationSelection( @Request() req: CustomRequest, @Param("teamId", ParseIntPipe) teamId: number, diff --git a/src/ideations/ideations.service.ts b/src/ideations/ideations.service.ts index 131cd8da..fdfa8201 100644 --- a/src/ideations/ideations.service.ts +++ b/src/ideations/ideations.service.ts @@ -1,7 +1,6 @@ import { BadRequestException, ConflictException, - ForbiddenException, Injectable, NotFoundException, } from "@nestjs/common"; @@ -11,6 +10,7 @@ import { UpdateIdeationDto } from "./dto/update-ideation.dto"; import { GlobalService } from "../global/global.service"; import { CustomRequest } from "../global/types/CustomRequest"; import { manageOwnVoyageTeamWithIdParam } from "../ability/conditions/voyage-teams.ability"; +import { manageOwnIdeationById } from "../ability/conditions/ideations.ability"; @Injectable() export class IdeationsService { @@ -160,56 +160,33 @@ export class IdeationsService { async updateIdeation( req: CustomRequest, ideationId: number, - teamId: number, + // teamId: number, updateIdeationDto: UpdateIdeationDto, ) { const { title, description, vision } = updateIdeationDto; - const voyageTeamMemberId = this.globalService.getVoyageTeamMemberId( - req, - teamId, - ); - - const ideationExistsCheck = await this.prisma.projectIdea.findUnique({ - where: { - id: ideationId, - }, - select: { - voyageTeamMemberId: true, - }, - }); - if (!ideationExistsCheck) { - throw new NotFoundException( - `Ideation (id: ${ideationId}) does not exist.`, - ); - } + await manageOwnIdeationById(req.user, ideationId); try { //only allow the user that created the idea to edit it - if (voyageTeamMemberId === ideationExistsCheck.voyageTeamMemberId) { - const updatedIdeation = await this.prisma.projectIdea.update({ - where: { - id: ideationId, - }, - data: { - title, - description, - vision, - }, - }); - return updatedIdeation; - } else { - throw new ForbiddenException( - "[Ideation Service]: You can only update your own project ideas.", - ); - } + const updatedIdeation = await this.prisma.projectIdea.update({ + where: { + id: ideationId, + }, + data: { + title, + description, + vision, + }, + }); + return updatedIdeation; } catch (e) { throw e; } } async removeIdeation(ideationId: number) { - return await this.prisma.projectIdea.delete({ + return this.prisma.projectIdea.delete({ where: { id: ideationId, }, diff --git a/test/ideations.e2e-spec.ts b/test/ideations.e2e-spec.ts index 59b1c38d..ccc02fef 100644 --- a/test/ideations.e2e-spec.ts +++ b/test/ideations.e2e-spec.ts @@ -287,9 +287,9 @@ describe("IdeationsController (e2e)", () => { }); }); - describe("/PATCH /teams/:teamId/ideations/:ideationId", () => { + describe("/PATCH /ideations/:ideationId", () => { const ideationId = 2; - const updateIdeationUrl = `/voyages/teams/${userVoyageTeamId}/ideations/${ideationId}`; + const updateIdeationUrl = `/voyages/ideations/${ideationId}`; it("should return 200 if update is successful", async () => { const ideationToUpdate = await prisma.projectIdea.findUnique({ where: { id: ideationId }, @@ -314,29 +314,22 @@ describe("IdeationsController (e2e)", () => { .expect(200); }); - it("should return 400 when ideation Id is not in the specified team", async () => { - await request(app.getHttpServer()) - .patch(`/voyages/teams/2/ideations/1`) - .set("Cookie", accessToken) - .expect(400); - }); - it("should return 401 when user is not logged in", async () => { await request(app.getHttpServer()) .patch(updateIdeationUrl) .expect(401); }); - it("should return 403 when user tries to delete someone else's ideation in the same team", async () => { + it("should return 403 when user tries to update someone else's ideation in the same team", async () => { await request(app.getHttpServer()) - .patch(`/voyages/teams/${userVoyageTeamId}/ideations/8`) + .patch(`/voyages/ideations/8`) .set("Cookie", accessToken) .expect(403); }); it("should return 404 when ideation Id does not exist", async () => { await request(app.getHttpServer()) - .patch(`/voyages/teams/1/ideations/100`) + .patch(`/voyages/ideations/100`) .set("Cookie", accessToken) .expect(404); }); From b2cfb3047a39df4432c340e2021157a3372a904e Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Tue, 9 Jul 2024 21:33:12 +1000 Subject: [PATCH 2/8] remove teamId dependency for removeVote --- src/ideations/ideations.service.ts | 60 +++++++++++++----------------- 1 file changed, 26 insertions(+), 34 deletions(-) diff --git a/src/ideations/ideations.service.ts b/src/ideations/ideations.service.ts index fdfa8201..3131b1f9 100644 --- a/src/ideations/ideations.service.ts +++ b/src/ideations/ideations.service.ts @@ -167,9 +167,11 @@ export class IdeationsService { await manageOwnIdeationById(req.user, ideationId); + // TODO: might need to check if user can update it + // (they should only be able to update it when they have the only vote) try { //only allow the user that created the idea to edit it - const updatedIdeation = await this.prisma.projectIdea.update({ + return this.prisma.projectIdea.update({ where: { id: ideationId, }, @@ -179,7 +181,6 @@ export class IdeationsService { vision, }, }); - return updatedIdeation; } catch (e) { throw e; } @@ -194,16 +195,23 @@ export class IdeationsService { } async removeVote(req: CustomRequest, teamId: number, ideationId: number) { - let ideationVote: any; try { - const voyageTeamMemberId = this.globalService.getVoyageTeamMemberId( - req, - teamId, - ); - ideationVote = await this.getIdeationVote( - ideationId, - voyageTeamMemberId, - ); + const ideationVote = await this.prisma.projectIdeaVote.findFirst({ + where: { + projectIdeaId: ideationId, + voyageTeamMemberId: { + in: req.user.voyageTeams.map((vt) => vt.teamId), + }, + }, + select: { + id: true, + }, + }); + if (!ideationVote) + throw new BadRequestException( + `Invalid Ideation Id or Team Member Id. The user does not have a vote for ideation id: ${ideationId}`, + ); + return await this.prisma.projectIdeaVote.delete({ where: { id: ideationVote.id, @@ -212,14 +220,18 @@ export class IdeationsService { } catch (e) { if (e.code === "P2002") { throw new NotFoundException( - `IdeationVote (id: ${ideationVote.id}) does not exist.`, + `Vote for ideation (id=${ideationId}) does not exist for the user.`, ); } throw e; } } - async deleteIdeation(req: CustomRequest, teamId, ideationId: number) { + async deleteIdeation( + req: CustomRequest, + teamId: number, + ideationId: number, + ) { const checkVotes = await this.getIdeationVoteCount(ideationId); if (checkVotes > 1) { throw new ConflictException( @@ -275,7 +287,7 @@ export class IdeationsService { //extract ids into array const idArray = teamMemberIds.map((member) => member.id); //search all team member project ideas for isSelected === true - return await this.prisma.projectIdea.findFirst({ + return this.prisma.projectIdea.findFirst({ where: { voyageTeamMemberId: { in: idArray, @@ -367,24 +379,4 @@ export class IdeationsService { ); return votesForIdeation.length; } - - private async getIdeationVote( - ideationId: number, - voyageTeamMemberId: number, - ) { - const oneIdeationVote = await this.prisma.projectIdeaVote.findFirst({ - where: { - projectIdeaId: ideationId, - voyageTeamMemberId: voyageTeamMemberId, - }, - select: { - id: true, - }, - }); - if (!oneIdeationVote) - throw new BadRequestException( - `Invalid Ideation Id or Team Member Id. The user (teamMemberId:${voyageTeamMemberId}) does not have a vote for ideation id: ${ideationId}`, - ); - return oneIdeationVote; - } } From 1abb3b7781f5043729c247ade012b11cf58c6766 Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Tue, 9 Jul 2024 21:50:38 +1000 Subject: [PATCH 3/8] update all delete ideation endpoint and tests --- src/ideations/ideations.controller.ts | 11 +++-------- src/ideations/ideations.service.ts | 27 +++++---------------------- test/ideations.e2e-spec.ts | 14 +++++++------- 3 files changed, 15 insertions(+), 37 deletions(-) diff --git a/src/ideations/ideations.controller.ts b/src/ideations/ideations.controller.ts index a1750adc..66594a7c 100644 --- a/src/ideations/ideations.controller.ts +++ b/src/ideations/ideations.controller.ts @@ -195,13 +195,12 @@ export class IdeationsController { type: ConflictErrorResponse, }) @CheckAbilities({ action: Action.Delete, subject: "Ideation" }) - @Delete("teams/:teamId/ideations/:ideationId") + @Delete("ideations/:ideationId") deleteIdeation( @Request() req: CustomRequest, - @Param("teamId", ParseIntPipe) teamId: number, @Param("ideationId", ParseIntPipe) ideationId: number, ) { - return this.ideationsService.deleteIdeation(req, teamId, ideationId); + return this.ideationsService.deleteIdeation(req, ideationId); } @ApiOperation({ @@ -231,11 +230,7 @@ export class IdeationsController { @Param("teamId", ParseIntPipe) teamId: number, @Param("ideationId", ParseIntPipe) ideationId: number, ) { - return this.ideationsService.deleteIdeationVote( - req, - teamId, - ideationId, - ); + return this.ideationsService.deleteIdeationVote(req, ideationId); } @ApiOperation({ diff --git a/src/ideations/ideations.service.ts b/src/ideations/ideations.service.ts index 3131b1f9..efb28ad0 100644 --- a/src/ideations/ideations.service.ts +++ b/src/ideations/ideations.service.ts @@ -194,7 +194,7 @@ export class IdeationsService { }); } - async removeVote(req: CustomRequest, teamId: number, ideationId: number) { + async removeVote(req: CustomRequest, ideationId: number) { try { const ideationVote = await this.prisma.projectIdeaVote.findFirst({ where: { @@ -227,11 +227,7 @@ export class IdeationsService { } } - async deleteIdeation( - req: CustomRequest, - teamId: number, - ideationId: number, - ) { + async deleteIdeation(req: CustomRequest, ideationId: number) { const checkVotes = await this.getIdeationVoteCount(ideationId); if (checkVotes > 1) { throw new ConflictException( @@ -239,24 +235,16 @@ export class IdeationsService { ); } try { - await this.removeVote(req, teamId, ideationId); + await this.removeVote(req, ideationId); return await this.removeIdeation(ideationId); } catch (e) { throw e; } } - async deleteIdeationVote( - req: CustomRequest, - teamId: number, - ideationId: number, - ) { + async deleteIdeationVote(req: CustomRequest, ideationId: number) { try { - const deleteIdeationVote = await this.removeVote( - req, - teamId, - ideationId, - ); + const deleteIdeationVote = await this.removeVote(req, ideationId); //delete ideation if no remaining votes const voteCount = await this.getIdeationVoteCount(ideationId); if (voteCount === 0) { @@ -265,11 +253,6 @@ export class IdeationsService { return deleteIdeationVote; } catch (e) { - // if (e.code === "P2002") { - // throw new NotFoundException( - // `IdeationVote (id: ${ideationVote.id}) does not exist.`, - // ); - // } throw e; } } diff --git a/test/ideations.e2e-spec.ts b/test/ideations.e2e-spec.ts index ccc02fef..ede85656 100644 --- a/test/ideations.e2e-spec.ts +++ b/test/ideations.e2e-spec.ts @@ -335,9 +335,9 @@ describe("IdeationsController (e2e)", () => { }); }); - describe("/DELETE /teams/:teamId/ideations/:ideationId", () => { + describe("/DELETE /ideations/:ideationId", () => { const ideationId = 1; - const deleteIdeationUrl = `/voyages/teams/${userVoyageTeamId}/ideations/${ideationId}`; + const deleteIdeationUrl = `/voyages/ideations/${ideationId}`; // user can only delete their own ideation when there is no votes except their own, // this will also delete their own vote if exist @@ -347,7 +347,7 @@ describe("IdeationsController (e2e)", () => { await prisma.projectIdeaVote.count(); await request(app.getHttpServer()) - .delete(`/voyages/teams/${userVoyageTeamId}/ideations/3`) + .delete(`/voyages/ideations/3`) .set("Cookie", accessToken) .expect(200); @@ -359,14 +359,14 @@ describe("IdeationsController (e2e)", () => { }); // user cannot delete someone else's ideation - // Note: should be 403 + // Add a 403 test it("should return 400 if the user delete their own ideation with other votes", async () => { const ideationCountBefore = await prisma.projectIdea.count(); const ideationVoteCountBefore = await prisma.projectIdeaVote.count(); await request(app.getHttpServer()) - .delete(`/voyages/teams/${userVoyageTeamId}/ideations/7`) + .delete(`/voyages/ideations/7`) .set("Cookie", accessToken) .expect(400); @@ -386,7 +386,7 @@ describe("IdeationsController (e2e)", () => { // Should be 404 it("should return 400 if ideation Id does not exist", async () => { await request(app.getHttpServer()) - .delete(`/voyages/teams/${userVoyageTeamId}/ideations/200`) + .delete(`/voyages/ideations/200`) .set("Cookie", accessToken) .expect(400); }); @@ -410,7 +410,7 @@ describe("IdeationsController (e2e)", () => { await prisma.projectIdeaVote.count(); await request(app.getHttpServer()) - .delete(`/voyages/teams/${userVoyageTeamId}/ideations/2`) + .delete(`/voyages/ideations/2`) .set("Cookie", accessToken) .expect(409); From fc30309e8035c6f02a058c4cf8f125d2d82e9ed6 Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Wed, 10 Jul 2024 15:23:19 +1000 Subject: [PATCH 4/8] fix DELETE ideation e2e tests --- src/ideations/ideations.service.ts | 102 +++++++++++++++++------------ test/ideations.e2e-spec.ts | 4 +- 2 files changed, 61 insertions(+), 45 deletions(-) diff --git a/src/ideations/ideations.service.ts b/src/ideations/ideations.service.ts index efb28ad0..1eeadbe4 100644 --- a/src/ideations/ideations.service.ts +++ b/src/ideations/ideations.service.ts @@ -186,50 +186,9 @@ export class IdeationsService { } } - async removeIdeation(ideationId: number) { - return this.prisma.projectIdea.delete({ - where: { - id: ideationId, - }, - }); - } - - async removeVote(req: CustomRequest, ideationId: number) { - try { - const ideationVote = await this.prisma.projectIdeaVote.findFirst({ - where: { - projectIdeaId: ideationId, - voyageTeamMemberId: { - in: req.user.voyageTeams.map((vt) => vt.teamId), - }, - }, - select: { - id: true, - }, - }); - if (!ideationVote) - throw new BadRequestException( - `Invalid Ideation Id or Team Member Id. The user does not have a vote for ideation id: ${ideationId}`, - ); - - return await this.prisma.projectIdeaVote.delete({ - where: { - id: ideationVote.id, - }, - }); - } catch (e) { - if (e.code === "P2002") { - throw new NotFoundException( - `Vote for ideation (id=${ideationId}) does not exist for the user.`, - ); - } - throw e; - } - } - async deleteIdeation(req: CustomRequest, ideationId: number) { - const checkVotes = await this.getIdeationVoteCount(ideationId); - if (checkVotes > 1) { + const voteCount = await this.checkIdeationAndVotes(ideationId); + if (voteCount > 1) { throw new ConflictException( `Ideation cannot be deleted when others have voted for it.`, ); @@ -362,4 +321,61 @@ export class IdeationsService { ); return votesForIdeation.length; } + + private async checkIdeationAndVotes(ideationId: number) { + const ideation = await this.prisma.projectIdea.findUnique({ + where: { + id: ideationId, + }, + }); + if (!ideation) { + throw new NotFoundException(); + } + return this.prisma.projectIdeaVote.count({ + where: { + projectIdeaId: ideationId, + }, + }); + } + + private async removeIdeation(ideationId: number) { + return this.prisma.projectIdea.delete({ + where: { + id: ideationId, + }, + }); + } + + private async removeVote(req: CustomRequest, ideationId: number) { + try { + const ideationVote = await this.prisma.projectIdeaVote.findFirst({ + where: { + projectIdeaId: ideationId, + voyageTeamMemberId: { + in: req.user.voyageTeams.map((vt) => vt.teamId), + }, + }, + select: { + id: true, + }, + }); + if (!ideationVote) + throw new BadRequestException( + `Invalid Ideation Id or Team Member Id. The user does not have a vote for ideation id: ${ideationId}`, + ); + + return await this.prisma.projectIdeaVote.delete({ + where: { + id: ideationVote.id, + }, + }); + } catch (e) { + if (e.code === "P2002") { + throw new NotFoundException( + `Vote for ideation (id=${ideationId}) does not exist for the user.`, + ); + } + throw e; + } + } } diff --git a/test/ideations.e2e-spec.ts b/test/ideations.e2e-spec.ts index ede85656..8909140e 100644 --- a/test/ideations.e2e-spec.ts +++ b/test/ideations.e2e-spec.ts @@ -384,11 +384,11 @@ describe("IdeationsController (e2e)", () => { }); // Should be 404 - it("should return 400 if ideation Id does not exist", async () => { + it("should return 404 if ideation Id does not exist", async () => { await request(app.getHttpServer()) .delete(`/voyages/ideations/200`) .set("Cookie", accessToken) - .expect(400); + .expect(404); }); // user cannot delete ideation with votes in it From 80c2f5dba5472ebfccae739ee2511677728e167a Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Wed, 10 Jul 2024 20:30:43 +1000 Subject: [PATCH 5/8] remove teamId from POST ideations/votes endpoint --- src/ideations/ideations.controller.ts | 9 +- src/ideations/ideations.service.ts | 133 +++++++++++++------------- test/ideations.e2e-spec.ts | 17 ++-- 3 files changed, 78 insertions(+), 81 deletions(-) diff --git a/src/ideations/ideations.controller.ts b/src/ideations/ideations.controller.ts index 66594a7c..db38229d 100644 --- a/src/ideations/ideations.controller.ts +++ b/src/ideations/ideations.controller.ts @@ -94,17 +94,12 @@ export class IdeationsController { type: IdeationVoteResponse, }) @CheckAbilities({ action: Action.Create, subject: "Ideation" }) - @Post("teams/:teamId/ideations/:ideationId/ideation-votes") + @Post("ideations/:ideationId/ideation-votes") createIdeationVote( @Request() req: CustomRequest, - @Param("teamId", ParseIntPipe) teamId: number, @Param("ideationId", ParseIntPipe) ideationId: number, ) { - return this.ideationsService.createIdeationVote( - req, - teamId, - ideationId, - ); + return this.ideationsService.createIdeationVote(req, ideationId); } @ApiOperation({ diff --git a/src/ideations/ideations.service.ts b/src/ideations/ideations.service.ts index 1eeadbe4..9d151d13 100644 --- a/src/ideations/ideations.service.ts +++ b/src/ideations/ideations.service.ts @@ -1,6 +1,7 @@ import { BadRequestException, ConflictException, + ForbiddenException, Injectable, NotFoundException, } from "@nestjs/common"; @@ -39,55 +40,13 @@ export class IdeationsService { }, }); - await this.createIdeationVote(req, teamId, createdIdeation.id); + await this.createIdeationVote(req, createdIdeation.id); return createdIdeation; } catch (e) { throw e; } } - async createIdeationVote(req, teamId: number, ideationId: number) { - // user can create Ideation votes for their own team(s) - manageOwnVoyageTeamWithIdParam(req.user, teamId); - const ideationExistsCheck = await this.prisma.projectIdea.findUnique({ - where: { - id: ideationId, - }, - }); - if (!ideationExistsCheck) { - throw new NotFoundException( - `Ideation (id: ${ideationId}) does not exist.`, - ); - } - - try { - const userHasVoted = await this.hasIdeationVote( - this.globalService.getVoyageTeamMemberId(req, teamId), - ideationId, - ); - //if user has not voted then a vote can be created - if (!userHasVoted) { - const createVote = await this.prisma.projectIdeaVote.create({ - data: { - voyageTeamMemberId: - this.globalService.getVoyageTeamMemberId( - req, - teamId, - ), - projectIdeaId: ideationId, - }, - }); - return createVote; - } else { - throw new ConflictException( - `User has already voted for ideationId: ${ideationId}`, - ); - } - } catch (e) { - throw e; - } - } - async getIdeationsByVoyageTeam(req: CustomRequest, id: number) { // user can read Ideation votes for their own team(s) manageOwnVoyageTeamWithIdParam(req.user, id); @@ -201,15 +160,70 @@ export class IdeationsService { } } + async createIdeationVote(req: CustomRequest, ideationId: number) { + // user can create Ideation votes for their own team(s) + const ideationToVote = await this.prisma.projectIdea.findUnique({ + where: { + id: ideationId, + }, + select: { + contributedBy: { + select: { + voyageTeamId: true, + }, + }, + voyageTeamMemberId: true, + }, + }); + + // ideation belongs to ideationToVote.contributedBy.voyageTeamId (and their voyageTeamMember id is ideationToVote.voyageTeamMemberId) + // (The contributor, not necessarily the voter but we can derive the voters voyageTeamMemberId from this + if (!ideationToVote) { + throw new NotFoundException( + `Ideation (id: ${ideationId}) does not exist.`, + ); + } + + // get voyageTeamMemberId from ideationId + const voyageTeamMemberId = req.user.voyageTeams.find( + (vt) => vt.teamId === ideationToVote.contributedBy?.voyageTeamId, + )?.memberId; + + // if voyageTeamMember is not found that means user is not in the team which owns the ideation + if (!voyageTeamMemberId) + throw new ForbiddenException( + "Forbidden: user is not authorized to access this resource.", + ); + + try { + const userHasVoted = await this.hasIdeationVote(req, ideationId); + //if user has not voted then a vote can be created + if (!userHasVoted) { + const createVote = await this.prisma.projectIdeaVote.create({ + data: { + voyageTeamMemberId, + projectIdeaId: ideationId, + }, + }); + return createVote; + } else { + throw new ConflictException( + `User has already voted for ideationId: ${ideationId}`, + ); + } + } catch (e) { + throw e; + } + } + async deleteIdeationVote(req: CustomRequest, ideationId: number) { try { const deleteIdeationVote = await this.removeVote(req, ideationId); //delete ideation if no remaining votes - const voteCount = await this.getIdeationVoteCount(ideationId); + const voteCount = await this.checkIdeationAndVotes(ideationId); if (voteCount === 0) { await this.removeIdeation(ideationId); } - return deleteIdeationVote; } catch (e) { throw e; @@ -289,39 +303,22 @@ export class IdeationsService { throw new NotFoundException(`no ideation found for team ${teamId}`); } - private async hasIdeationVote(teamMemberId: number, ideationId: number) { + private async hasIdeationVote(req: CustomRequest, ideationId: number) { const checkVoteStatus = await this.prisma.projectIdeaVote.findMany({ where: { - voyageTeamMemberId: teamMemberId, + voyageTeamMemberId: { + in: req.user.voyageTeams.map((vt) => vt.memberId), + }, projectIdeaId: ideationId, }, select: { id: true, }, }); - if (!checkVoteStatus) - throw new BadRequestException( - `Invalid Ideation Id or Team Member Id. The user (teamMemberId:${teamMemberId}) does not have a vote for ideation id: ${ideationId}`, - ); return checkVoteStatus.length > 0; } - private async getIdeationVoteCount(ideationId: number) { - const votesForIdeation = await this.prisma.projectIdeaVote.findMany({ - where: { - projectIdeaId: ideationId, - }, - select: { - id: true, - }, - }); - if (!votesForIdeation) - throw new NotFoundException( - `Ideation (id: ${ideationId}) does not exist`, - ); - return votesForIdeation.length; - } - + // check ideation Id exists and return the vote count private async checkIdeationAndVotes(ideationId: number) { const ideation = await this.prisma.projectIdea.findUnique({ where: { @@ -352,7 +349,7 @@ export class IdeationsService { where: { projectIdeaId: ideationId, voyageTeamMemberId: { - in: req.user.voyageTeams.map((vt) => vt.teamId), + in: req.user.voyageTeams.map((vt) => vt.memberId), }, }, select: { diff --git a/test/ideations.e2e-spec.ts b/test/ideations.e2e-spec.ts index 8909140e..c1cbbe4c 100644 --- a/test/ideations.e2e-spec.ts +++ b/test/ideations.e2e-spec.ts @@ -134,9 +134,9 @@ describe("IdeationsController (e2e)", () => { }); }); - describe("/POST voyages/teams/:teamId/ideations/:ideationId/ideation-votes", () => { + describe("/POST voyages/ideations/:ideationId/ideation-votes", () => { const ideationId = 1; - const ideationVoteUrl = `/voyages/teams/${userVoyageTeamId}/ideations/${ideationId}/ideation-votes`; + const ideationVoteUrl = `/voyages/ideations/${ideationId}/ideation-votes`; it("should return 201 if an ideation vote is successfully created", async () => { // login to another user in the same team to vote @@ -173,15 +173,20 @@ describe("IdeationsController (e2e)", () => { }); it("should return 403 when user is voting for another ideation which belongs to another team", async () => { + const { access_token } = await loginAndGetTokens( + "JosoMadar@dayrep.com", + "password", + app, + ); await request(app.getHttpServer()) - .post(`/voyages/teams/2/ideations/4/ideation-votes`) - .set("Cookie", accessToken) + .post(`/voyages/ideations/4/ideation-votes`) + .set("Cookie", access_token) .expect(403); }); it("should return 404 when the ideation ID doesn't exist", async () => { await request(app.getHttpServer()) - .post(`/voyages/teams/1/ideations/20/ideation-votes`) + .post(`/voyages/ideations/20/ideation-votes`) .set("Cookie", accessToken) .expect(404); }); @@ -401,7 +406,7 @@ describe("IdeationsController (e2e)", () => { ); await request(app.getHttpServer()) - .post(`/voyages/teams/1/ideations/2/ideation-votes`) + .post(`/voyages/ideations/2/ideation-votes`) .set("Cookie", access_token) .expect(201); From d2b9093283a3fb98543ebe0c742e385234e14279 Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Wed, 10 Jul 2024 20:59:20 +1000 Subject: [PATCH 6/8] update delete ideation votes e2e tests --- src/ideations/ideations.controller.ts | 3 +-- src/ideations/ideations.service.ts | 3 +-- test/ideations.e2e-spec.ts | 12 ++++-------- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/ideations/ideations.controller.ts b/src/ideations/ideations.controller.ts index db38229d..6d9d3d5a 100644 --- a/src/ideations/ideations.controller.ts +++ b/src/ideations/ideations.controller.ts @@ -219,10 +219,9 @@ export class IdeationsController { type: IdeationVoteResponse, }) @CheckAbilities({ action: Action.Delete, subject: "Ideation" }) - @Delete("teams/:teamId/ideations/:ideationId/ideation-votes") + @Delete("ideations/:ideationId/ideation-votes") deleteIdeationVote( @Request() req: CustomRequest, - @Param("teamId", ParseIntPipe) teamId: number, @Param("ideationId", ParseIntPipe) ideationId: number, ) { return this.ideationsService.deleteIdeationVote(req, ideationId); diff --git a/src/ideations/ideations.service.ts b/src/ideations/ideations.service.ts index 9d151d13..8925915e 100644 --- a/src/ideations/ideations.service.ts +++ b/src/ideations/ideations.service.ts @@ -199,13 +199,12 @@ export class IdeationsService { const userHasVoted = await this.hasIdeationVote(req, ideationId); //if user has not voted then a vote can be created if (!userHasVoted) { - const createVote = await this.prisma.projectIdeaVote.create({ + return this.prisma.projectIdeaVote.create({ data: { voyageTeamMemberId, projectIdeaId: ideationId, }, }); - return createVote; } else { throw new ConflictException( `User has already voted for ideationId: ${ideationId}`, diff --git a/test/ideations.e2e-spec.ts b/test/ideations.e2e-spec.ts index c1cbbe4c..0798225f 100644 --- a/test/ideations.e2e-spec.ts +++ b/test/ideations.e2e-spec.ts @@ -199,9 +199,9 @@ describe("IdeationsController (e2e)", () => { }); }); - describe("/DELETE voyages/teams/:teamId/ideations/:ideationId/ideation-votes", () => { + describe("/DELETE voyages/ideations/:ideationId/ideation-votes", () => { const ideationId = 1; - const ideationVoteUrl = `/voyages/teams/${userVoyageTeamId}/ideations/${ideationId}/ideation-votes`; + const ideationVoteUrl = `/voyages/ideations/${ideationId}/ideation-votes`; it("should return 200 when ideation vote is deleted", async () => { const ideationCountBefore = await prisma.projectIdea.count(); @@ -248,9 +248,7 @@ describe("IdeationsController (e2e)", () => { const ideationVoteCountBefore = await prisma.projectIdeaVote.count(); await request(app.getHttpServer()) - .delete( - `/voyages/teams/${userVoyageTeamId}/ideations/8/ideation-votes`, - ) + .delete(`/voyages/ideations/8/ideation-votes`) .set("Cookie", accessToken) .expect(400); @@ -264,9 +262,7 @@ describe("IdeationsController (e2e)", () => { // Note: this should be 404 it("should return 400 when ideation id does not exist", async () => { await request(app.getHttpServer()) - .delete( - `/voyages/teams/${userVoyageTeamId}/ideations/100/ideation-votes`, - ) + .delete(`/voyages/ideations/100/ideation-votes`) .set("Cookie", accessToken) .expect(400); }); From eaa09981f9a0b185fde83110fe3b15a8ccc8a7e8 Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Wed, 10 Jul 2024 21:14:22 +1000 Subject: [PATCH 7/8] update changelog --- CHANGELOG.md | 2 +- test/ideations.e2e-spec.ts | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54422e7e..bf568b2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,7 +75,7 @@ Another example [here](https://co-pilot.dev/changelog) - Updated DELETE ideation-vote service to also delete ideation when no votes remain [#161](https://github.com/chingu-x/chingu-dashboard-be/pull/161) - Refactored the prisma models to be grouped by domain type [#172](https://github.com/chingu-x/chingu-dashboard-be/pull/172) - Updated response for GET teams/:teamId/techs to include isSelected value for techs [#173](https://github.com/chingu-x/chingu-dashboard-be/pull/173) - +- Refactor ideation endpoints to remove redundant teamId params [#175](https://github.com/chingu-x/chingu-dashboard-be/pull/175) ### Fixed diff --git a/test/ideations.e2e-spec.ts b/test/ideations.e2e-spec.ts index 0798225f..939613ca 100644 --- a/test/ideations.e2e-spec.ts +++ b/test/ideations.e2e-spec.ts @@ -359,8 +359,6 @@ describe("IdeationsController (e2e)", () => { expect(ideationVoteCountAfter).toEqual(ideationVoteCountBefore - 1); }); - // user cannot delete someone else's ideation - // Add a 403 test it("should return 400 if the user delete their own ideation with other votes", async () => { const ideationCountBefore = await prisma.projectIdea.count(); const ideationVoteCountBefore = @@ -384,7 +382,6 @@ describe("IdeationsController (e2e)", () => { .expect(401); }); - // Should be 404 it("should return 404 if ideation Id does not exist", async () => { await request(app.getHttpServer()) .delete(`/voyages/ideations/200`) From 69c07d160db71d0149dcebc5ee34dca02a4f3dbc Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Wed, 10 Jul 2024 22:37:45 +1000 Subject: [PATCH 8/8] update ideations unit tests --- src/ability/conditions/ideations.ability.ts | 2 +- src/ideations/ideations.controller.spec.ts | 13 +-------- src/ideations/ideations.service.spec.ts | 31 +++++++++++---------- 3 files changed, 19 insertions(+), 27 deletions(-) diff --git a/src/ability/conditions/ideations.ability.ts b/src/ability/conditions/ideations.ability.ts index ffc1e280..c384e267 100644 --- a/src/ability/conditions/ideations.ability.ts +++ b/src/ability/conditions/ideations.ability.ts @@ -16,7 +16,7 @@ export const manageOwnIdeationById = async ( }, }); if (!ideation) { - throw new NotFoundException(`Ideation (id:${ideationId} not found`); + throw new NotFoundException(`Ideation (id:${ideationId}) not found`); } // ideation is not linked to any members, this should never happen unless the user gets deleted // or removed from the team? diff --git a/src/ideations/ideations.controller.spec.ts b/src/ideations/ideations.controller.spec.ts index 9716e395..e496ff02 100644 --- a/src/ideations/ideations.controller.spec.ts +++ b/src/ideations/ideations.controller.spec.ts @@ -74,7 +74,6 @@ describe("IdeationsController", () => { it("should create an ideation vote", async () => { const userId = "cc1b7a12-72f6-11ee-b962-0242ac120002"; - const teamId = 1; const ideationId = 1; const req = { user: { @@ -83,7 +82,6 @@ describe("IdeationsController", () => { } as CustomRequest; const ideationVote = await controller.createIdeationVote( req, - teamId, ideationId, ); @@ -112,7 +110,6 @@ describe("IdeationsController", () => { it("should update an ideation", async () => { const userId = "cc1b7a12-72f6-11ee-b962-0242ac120002"; const ideationId = 1; - const teamId = 1; const req = { user: { userId: userId, @@ -126,7 +123,6 @@ describe("IdeationsController", () => { const ideation = await controller.updateIdeation( req, ideationId, - teamId, updateIdeationDto, ); expect(service.updateIdeation).toHaveBeenCalled(); @@ -136,17 +132,12 @@ describe("IdeationsController", () => { it("should delete an ideation", async () => { const userId = "cc1b7a12-72f6-11ee-b962-0242ac120002"; const ideationId = 1; - const teamId = 1; const req = { user: { userId: userId, }, } as CustomRequest; - const ideation = await controller.deleteIdeation( - req, - teamId, - ideationId, - ); + const ideation = await controller.deleteIdeation(req, ideationId); expect(service.deleteIdeation).toHaveBeenCalled(); expect(ideation).toBe(true); @@ -154,7 +145,6 @@ describe("IdeationsController", () => { it("should delete an ideation vote", async () => { const userId = "cc1b7a12-72f6-11ee-b962-0242ac120002"; - const teamId = 1; const ideationId = 1; const req = { user: { @@ -163,7 +153,6 @@ describe("IdeationsController", () => { } as CustomRequest; const ideationVote = await controller.deleteIdeationVote( req, - teamId, ideationId, ); diff --git a/src/ideations/ideations.service.spec.ts b/src/ideations/ideations.service.spec.ts index 0591de1c..0e5a9a49 100644 --- a/src/ideations/ideations.service.spec.ts +++ b/src/ideations/ideations.service.spec.ts @@ -3,7 +3,9 @@ import { IdeationsService } from "./ideations.service"; import { PrismaService } from "../prisma/prisma.service"; import { GlobalService } from "../global/global.service"; import { CustomRequest } from "../global/types/CustomRequest"; +import * as IdeationAbility from "../ability/conditions/ideations.ability"; +// TODO: these tests probably need to be updated, it shouldn't use prisma, should only use prismaMock describe("IdeationsService", () => { let service: IdeationsService; @@ -14,6 +16,9 @@ describe("IdeationsService", () => { description: "Ideation 1 description", vision: "Ideation 1 vision", voyageTeamMemberId: 1, + contributedBy: { + voyageTeamId: 1, + }, }, ]; const ideationOne = ideationArr[0]; @@ -114,6 +119,8 @@ describe("IdeationsService", () => { }).compile(); service = module.get(IdeationsService); + + jest.spyOn(service, "checkIdeationAndVotes").mockResolvedValue(1); }); it("should be defined", () => { @@ -134,7 +141,7 @@ describe("IdeationsService", () => { ], roles: ["voyager"], }, - } as CustomRequest; + } as unknown as CustomRequest; const createIdeationDto = { req, title: "Ideation 1", @@ -152,7 +159,6 @@ describe("IdeationsService", () => { it("should create an ideation vote", async () => { const userId = "00a10ade-7308-11ee-a962-0242ac120002"; - const teamId = 1; const ideationId = 1; const req = { user: { @@ -168,8 +174,7 @@ describe("IdeationsService", () => { }; const result = await service.createIdeationVote( - req, - teamId, + req as CustomRequest, ideationId, ); expect(result).toEqual(ideationVoteOne); @@ -198,7 +203,11 @@ describe("IdeationsService", () => { it("should update an ideation", async () => { const userId = "00a10ade-7308-11ee-b962-0242ac120002"; const ideationId = 1; - const teamId = 1; + + const manageOwnIdeationByIdMock = jest + .spyOn(IdeationAbility, "manageOwnIdeationById") + .mockResolvedValue(); + const req = { user: { userId: userId, @@ -221,15 +230,14 @@ describe("IdeationsService", () => { const result = await service.updateIdeation( req, ideationId, - teamId, updateIdeationDto, ); expect(result).toEqual(ideationOne); + expect(manageOwnIdeationByIdMock).toHaveBeenCalled(); }); it("should delete an ideation vote", async () => { const userId = "00a10ade-7308-11ee-a962-0242ac120002"; - const teamId = 1; const ideationId = 1; const req = { user: { @@ -244,18 +252,13 @@ describe("IdeationsService", () => { }, } as CustomRequest; - const result = await service.deleteIdeationVote( - req, - teamId, - ideationId, - ); + const result = await service.deleteIdeationVote(req, ideationId); expect(result).toEqual(ideationVoteOne); }); it("should delete an ideation", async () => { const userId = "00a10ade-7308-11ee-b962-0242ac120002"; const ideationId = 1; - const teamId = 1; const req = { user: { userId: userId, @@ -268,7 +271,7 @@ describe("IdeationsService", () => { roles: ["voyager"], }, } as CustomRequest; - const result = await service.deleteIdeation(req, teamId, ideationId); + const result = await service.deleteIdeation(req, ideationId); expect(result).toEqual(ideationOne); }); });