Refactor ideation endpoints to remove redundant teamId params

CHANGELOG.md

@@ -77,7 +77,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

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),

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.",
+        );
+    }
+};

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 },

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,25 +132,19 @@ 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);
     });
 
     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,
         );
 

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,17 +94,12 @@ export class IdeationsController {
         type: IdeationVoteResponse,
     })
     @CheckAbilities({ action: Action.Create, subject: "Ideation" })
-    @Post("/: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({
@@ -122,7 +117,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 +146,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,13 +190,12 @@ export class IdeationsController {
         type: ConflictErrorResponse,
     })
     @CheckAbilities({ action: Action.Delete, subject: "Ideation" })
-    @Delete("/: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({
@@ -227,17 +219,12 @@ export class IdeationsController {
         type: IdeationVoteResponse,
     })
     @CheckAbilities({ action: Action.Delete, subject: "Ideation" })
-    @Delete("/: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,
-            teamId,
-            ideationId,
-        );
+        return this.ideationsService.deleteIdeationVote(req, ideationId);
     }
 
     @ApiOperation({
@@ -278,7 +265,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 +305,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,

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>(IdeationsService);
+
+        jest.spyOn(service, <any>"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);
     });
 });