[dev] [tofikwest] tofik/finding-improvements (#2084)

* feat(api): add revision note handling for findings update

* feat(app): update onStatusChange to support async handling in FindingItem

---------

Co-authored-by: Tofik Hasanov <annexcies@gmail.com>

Author: github-actions[bot]

apps/api/src/findings/dto/update-finding.dto.ts

@@ -39,4 +39,16 @@ export class UpdateFindingDto {
   @IsNotEmpty({ message: 'Content cannot be empty if provided' })
   @MaxLength(5000)
   content?: string;
+
+  @ApiProperty({
+    description: 'Auditor note when requesting revision (only for needs_revision status)',
+    example: 'Please provide clearer screenshots showing the timestamp.',
+    maxLength: 2000,
+    required: false,
+    nullable: true,
+  })
+  @IsString()
+  @IsOptional()
+  @MaxLength(2000)
+  revisionNote?: string | null;
 }

apps/api/src/findings/findings.service.ts

@@ -71,9 +71,7 @@ export class FindingsService {
       ],
     });
 
-    this.logger.log(
-      `Retrieved ${findings.length} findings for task ${taskId}`,
-    );
+    this.logger.log(`Retrieved ${findings.length} findings for task ${taskId}`);
     return findings;
   }
 
@@ -311,19 +309,38 @@ export class FindingsService {
       }
 
       // ready_for_review can only be set by non-auditor admins/owners (client signals to auditor)
-      if (updateDto.status === FindingStatus.ready_for_review && isAuditor && !isPlatformAdmin) {
+      if (
+        updateDto.status === FindingStatus.ready_for_review &&
+        isAuditor &&
+        !isPlatformAdmin
+      ) {
         throw new ForbiddenException(
           `Auditors cannot set status to 'ready_for_review'. This status is for clients to signal readiness.`,
         );
       }
     }
 
+    // Handle revisionNote logic:
+    // - Set revisionNote when status is needs_revision and a note is provided
+    // - Clear revisionNote when status changes to anything other than needs_revision
+    let revisionNoteUpdate: { revisionNote?: string | null } = {};
+    if (updateDto.status === FindingStatus.needs_revision) {
+      // Set revision note if provided (can be null to clear)
+      if (updateDto.revisionNote !== undefined) {
+        revisionNoteUpdate = { revisionNote: updateDto.revisionNote || null };
+      }
+    } else if (updateDto.status !== undefined) {
+      // Clear revision note when moving to a different status
+      revisionNoteUpdate = { revisionNote: null };
+    }
+
     const updatedFinding = await db.finding.update({
       where: { id: findingId },
       data: {
         ...(updateDto.status !== undefined && { status: updateDto.status }),
         ...(updateDto.type !== undefined && { type: updateDto.type }),
         ...(updateDto.content !== undefined && { content: updateDto.content }),
+        ...revisionNoteUpdate,
       },
       include: {
         createdBy: {
@@ -411,22 +428,34 @@ export class FindingsService {
 
       switch (updateDto.status) {
         case FindingStatus.ready_for_review:
-          this.logger.log(`Triggering 'ready_for_review' notification for finding ${findingId}`);
+          this.logger.log(
+            `Triggering 'ready_for_review' notification for finding ${findingId}`,
+          );
           void this.findingNotifierService.notifyReadyForReview({
             ...notificationParams,
             findingCreatorMemberId: finding.createdById,
           });
           break;
         case FindingStatus.needs_revision:
-          this.logger.log(`Triggering 'needs_revision' notification for finding ${findingId}`);
-          void this.findingNotifierService.notifyNeedsRevision(notificationParams);
+          this.logger.log(
+            `Triggering 'needs_revision' notification for finding ${findingId}`,
+          );
+          void this.findingNotifierService.notifyNeedsRevision(
+            notificationParams,
+          );
           break;
         case FindingStatus.closed:
-          this.logger.log(`Triggering 'closed' notification for finding ${findingId}`);
-          void this.findingNotifierService.notifyFindingClosed(notificationParams);
+          this.logger.log(
+            `Triggering 'closed' notification for finding ${findingId}`,
+          );
+          void this.findingNotifierService.notifyFindingClosed(
+            notificationParams,
+          );
           break;
         case FindingStatus.open:
-          this.logger.log(`Status changed to 'open' for finding ${findingId}. No notification sent.`);
+          this.logger.log(
+            `Status changed to 'open' for finding ${findingId}. No notification sent.`,
+          );
           break;
         default:
           this.logger.warn(
@@ -489,6 +518,9 @@ export class FindingsService {
     // Verify finding exists
     await this.findById(organizationId, findingId);
 
-    return this.findingAuditService.getFindingActivity(findingId, organizationId);
+    return this.findingAuditService.getFindingActivity(
+      findingId,
+      organizationId,
+    );
   }
 }

apps/app/src/app/(app)/[orgId]/frameworks/components/FindingsOverview.tsx

@@ -18,7 +18,7 @@ const STATUS_COLORS: Record<FindingStatus, string> = {
 
 const STATUS_LABELS: Record<FindingStatus, string> = {
   open: 'Open',
-  ready_for_review: 'Review',
+  ready_for_review: 'Auditor Review',
   needs_revision: 'Revision',
   closed: 'Closed',
 };
@@ -215,7 +215,7 @@ export function FindingsOverview({
                         </div>
                       </div>
                       <Button asChild size="icon" variant="outline" className="shrink-0 ml-2">
-                        <Link href={`/${organizationId}/tasks/${finding.task.id}`}>
+                        <Link href={`/${organizationId}/tasks/${finding.task.id}#finding-${finding.id}`}>
                           <ArrowRight className="h-3 w-3" />
                         </Link>
                       </Button>