Merge pull request #80 from topcoder-platform/fix/issue-58

#58 Refactor the GET reviews logic and change the review status to an enum

Author: jmgasper

prisma/migrations/20250911064653_add_a_review_status/migration.sql

@@ -0,0 +1,15 @@
+/*
+  Warnings:
+
+  - The `status` column on the `review` table would be dropped and recreated. This will lead to data loss if there is data in the column.
+
+*/
+-- CreateEnum
+CREATE TYPE "ReviewStatus" AS ENUM ('PENDING', 'IN_PROGRESS', 'COMPLETED', 'DRAFT', 'SUBMITTED', 'APPROVED', 'REJECTED');
+
+-- AlterTable
+ALTER TABLE "review" DROP COLUMN "status",
+ADD COLUMN     "status" "ReviewStatus";
+
+-- CreateIndex
+CREATE INDEX "review_status_idx" ON "review"("status");

prisma/schema.prisma

@@ -141,6 +141,16 @@ model scorecardQuestion {
   @@index([sortOrder]) // Index for ordering questions
 }
 
+enum ReviewStatus {
+  PENDING
+  IN_PROGRESS
+  COMPLETED
+  DRAFT
+  SUBMITTED
+  APPROVED
+  REJECTED
+}
+
 model review {
   id                 String    @id @default(dbgenerated("gen_random_uuid()")) @db.VarChar(36)
   legacyId           String?
@@ -154,7 +164,7 @@ model review {
   initialScore       Float?
   typeId             String?
   metadata           Json?
-  status             String?
+  status             ReviewStatus?
   reviewDate         DateTime?
   createdAt          DateTime  @default(now())
   createdBy          String
@@ -170,6 +180,7 @@ model review {
   @@index([resourceId]) // Index for filtering by resource (reviewer)
   @@index([phaseId]) // Index for filtering by phase
   @@index([scorecardId]) // Index for joining with scorecard table
+  @@index([status]) // Index for filtering by review status
 }
 
 model reviewItem {

src/api/review/review.controller.ts

@@ -33,11 +33,11 @@ import {
   ReviewItemRequestDto,
   ReviewItemResponseDto,
   ReviewProgressResponseDto,
+  ReviewStatus,
   mapReviewRequestToDto,
   mapReviewItemRequestToDto,
 } from 'src/dto/review.dto';
 import { PrismaService } from '../../shared/modules/global/prisma.service';
-import { ScorecardStatus } from '../../dto/scorecard.dto';
 import { LoggerService } from '../../shared/modules/global/logger.service';
 import { PaginatedResponse, PaginationDto } from '../../dto/pagination.dto';
 import { PrismaErrorService } from '../../shared/modules/global/prisma-error.service';
@@ -337,16 +337,16 @@ export class ReviewController {
   @Roles(UserRole.Reviewer, UserRole.Copilot, UserRole.Admin, UserRole.User)
   @Scopes(Scope.ReadReview)
   @ApiOperation({
-    summary: 'Search for pending reviews',
+    summary: 'Search for reviews',
     description:
       'Roles: Reviewer, Copilot, Admin, User. For User, only applies to their own review, until challenge completion. | Scopes: read:review',
   })
   @ApiQuery({
     name: 'status',
     description: 'The review status to filter by',
-    example: 'pending',
+    enum: ReviewStatus,
+    example: ReviewStatus.PENDING,
     required: false,
-    enum: ScorecardStatus,
   })
   @ApiQuery({
     name: 'challengeId',
@@ -360,17 +360,17 @@ export class ReviewController {
   })
   @ApiResponse({
     status: 200,
-    description: 'List of pending reviews.',
+    description: 'List of reviews.',
     type: [ReviewResponseDto],
   })
-  async getPendingReviews(
-    @Query('status') status?: ScorecardStatus,
+  async getReviews(
+    @Query('status') status?: ReviewStatus,
     @Query('challengeId') challengeId?: string,
     @Query('submissionId') submissionId?: string,
     @Query() paginationDto?: PaginationDto,
   ): Promise<PaginatedResponse<ReviewResponseDto>> {
     this.logger.log(
-      `Getting pending reviews with filters - status: ${status}, challengeId: ${challengeId}, submissionId: ${submissionId}`,
+      `Getting reviews with filters - status: ${status}, challengeId: ${challengeId}, submissionId: ${submissionId}`,
     );
 
     const { page = 1, perPage = 10 } = paginationDto || {};
@@ -379,115 +379,71 @@ export class ReviewController {
     try {
       // Build the where clause for reviews based on available filter parameters
       const reviewWhereClause: any = {};
+
       if (submissionId) {
         reviewWhereClause.submissionId = submissionId;
       }
 
-      // Get reviews by status if provided
-      let reviews: any[] = [];
-      let totalCount = 0;
-
       if (status) {
-        this.logger.debug(`Fetching reviews by scorecard status: ${status}`);
-        const scorecards = await this.prisma.scorecard.findMany({
-          where: {
-            status: ScorecardStatus[status as keyof typeof ScorecardStatus],
-          },
-          include: {
-            reviews: {
-              where: reviewWhereClause,
-              skip,
-              take: perPage,
-            },
-          },
-        });
-
-        reviews = scorecards.flatMap((d) => d.reviews);
-
-        // Count total reviews matching the filter for pagination metadata
-        const scorecardIds = scorecards.map((s) => s.id);
-        totalCount = await this.prisma.review.count({
-          where: {
-            ...reviewWhereClause,
-            scorecardId: { in: scorecardIds },
-          },
-        });
+        reviewWhereClause.status = status;
       }
 
       // Get reviews by challengeId if provided
       if (challengeId) {
         this.logger.debug(`Fetching reviews by challengeId: ${challengeId}`);
-        const challengeResults = await this.prisma.challengeResult.findMany({
+        // Get submissions for this challenge directly (consistent with POST /reviews)
+        const submissions = await this.prisma.submission.findMany({
           where: { challengeId },
+          select: { id: true },
         });
 
-        const submissionIds = challengeResults.map((c) => c.submissionId);
+        const submissionIds = submissions.map((s) => s.id);
 
         if (submissionIds.length > 0) {
-          const challengeReviews = await this.prisma.review.findMany({
-            where: {
-              submissionId: { in: submissionIds },
-              ...reviewWhereClause,
-            },
-            skip,
-            take: perPage,
-            include: {
-              reviewItems: {
-                include: {
-                  reviewItemComments: true,
-                },
-              },
-            },
-          });
-
-          reviews = [...reviews, ...challengeReviews];
-
-          // Count total for this condition separately
-          const challengeReviewCount = await this.prisma.review.count({
-            where: {
-              submissionId: { in: submissionIds },
-              ...reviewWhereClause,
+          reviewWhereClause.submissionId = { in: submissionIds };
+        } else {
+          // No submissions found for this challenge, return empty result
+          return {
+            data: [],
+            meta: {
+              page,
+              perPage,
+              totalCount: 0,
+              totalPages: 0,
             },
-          });
-
-          totalCount += challengeReviewCount;
+          };
         }
       }
 
-      // If no specific filters, get all reviews with pagination
-      if (!status && !challengeId && !submissionId) {
-        this.logger.debug('Fetching all reviews with pagination');
-        reviews = await this.prisma.review.findMany({
-          skip,
-          take: perPage,
-          include: {
-            reviewItems: {
-              include: {
-                reviewItemComments: true,
-              },
+      // Get reviews with the built where clause
+      this.logger.debug(
+        `Fetching reviews with where clause:`,
+        reviewWhereClause,
+      );
+      const reviews = await this.prisma.review.findMany({
+        where: reviewWhereClause,
+        skip,
+        take: perPage,
+        include: {
+          reviewItems: {
+            include: {
+              reviewItemComments: true,
             },
           },
-        });
-
-        totalCount = await this.prisma.review.count();
-      }
+        },
+      });
 
-      // Deduplicate reviews by ID
-      const uniqueReviews = Object.values(
-        reviews.reduce((acc: Record<string, any>, review) => {
-          if (!acc[review.id]) {
-            acc[review.id] = review;
-          }
-          return acc;
-        }, {}),
-      );
+      // Count total reviews matching the filter for pagination metadata
+      const totalCount = await this.prisma.review.count({
+        where: reviewWhereClause,
+      });
 
       this.logger.log(
-        `Found ${uniqueReviews.length} reviews (page ${page} of ${Math.ceil(totalCount / perPage)})`,
+        `Found ${reviews.length} reviews (page ${page} of ${Math.ceil(totalCount / perPage)})`,
       );
 
       return {
-        data: uniqueReviews as ReviewResponseDto[],
+        data: reviews as ReviewResponseDto[],
         meta: {
           page,
           perPage,

src/dto/review.dto.ts

@@ -24,6 +24,16 @@ export enum ReviewItemCommentType {
   SPECIFICATION_REVIEW_COMMENT = 'SPECIFICATION_REVIEW_COMMENT',
 }
 
+export enum ReviewStatus {
+  PENDING = 'PENDING',
+  IN_PROGRESS = 'IN_PROGRESS',
+  COMPLETED = 'COMPLETED',
+  DRAFT = 'DRAFT',
+  SUBMITTED = 'SUBMITTED',
+  APPROVED = 'APPROVED',
+  REJECTED = 'REJECTED',
+}
+
 export class ReviewItemCommentBaseDto {
   @ApiProperty({
     description: 'Content of the comment',
@@ -222,11 +232,12 @@ export class ReviewBaseDto {
 
   @ApiProperty({
     description: 'Status for the review',
-    example: 'Review',
+    enum: ReviewStatus,
+    example: ReviewStatus.PENDING,
   })
   @IsString()
   @IsNotEmpty()
-  status: string;
+  status: ReviewStatus;
 
   @ApiProperty({
     description: 'Review date for the review',
@@ -316,12 +327,13 @@ export class ReviewPatchRequestDto {
 
   @ApiProperty({
     description: 'Status for the review',
-    example: 'Review',
+    enum: ReviewStatus,
+    example: ReviewStatus.PENDING,
   })
   @IsOptional()
   @IsString()
   @IsNotEmpty()
-  status?: string;
+  status?: ReviewStatus;
 
   @ApiProperty({
     description: 'Review date for the review',