[comp] Production Deploy

apps/api/src/tasks/tasks.service.spec.ts

@@ -0,0 +1,220 @@
+// Mock the DB layer before importing the service. We spread the real Prisma
+// client so generated enums (TaskStatus, TaskFrequency) keep their values.
+jest.mock('@db', () => ({
+  ...jest.requireActual('@prisma/client'),
+  db: {
+    task: {
+      findFirst: jest.fn(),
+      update: jest.fn(),
+      updateMany: jest.fn(),
+    },
+    organization: {
+      findUnique: jest.fn(),
+    },
+    member: {
+      findFirst: jest.fn(),
+      findUnique: jest.fn(),
+    },
+    auditLog: {
+      create: jest.fn(),
+    },
+  },
+}));
+
+// checkAutoCompletePhases reaches into the DB; stub it out for unit tests.
+jest.mock('../frameworks/frameworks-timeline.helper', () => ({
+  checkAutoCompletePhases: jest.fn().mockResolvedValue(undefined),
+}));
+
+// The service's transitive imports pull in @trycompai/auth (ESM better-auth)
+// which jest can't transform; stub it as the controller spec does.
+jest.mock('@trycompai/auth', () => ({
+  statement: {
+    task: ['create', 'read', 'update', 'delete'],
+    evidence: ['create', 'read', 'delete'],
+  },
+  BUILT_IN_ROLE_PERMISSIONS: {},
+}));
+
+jest.mock('../auth/auth.server', () => ({
+  auth: { api: { getSession: jest.fn() } },
+}));
+
+import { Test } from '@nestjs/testing';
+import { db, TaskStatus } from '@db';
+import { TaskNotifierService } from './task-notifier.service';
+import { TimelinesService } from '../timelines/timelines.service';
+import { TasksService } from './tasks.service';
+
+const taskFindFirst = db.task.findFirst as jest.Mock;
+const taskUpdate = db.task.update as jest.Mock;
+const taskUpdateMany = db.task.updateMany as jest.Mock;
+const orgFindUnique = db.organization.findUnique as jest.Mock;
+const memberFindFirst = db.member.findFirst as jest.Mock;
+const auditLogCreate = db.auditLog.create as jest.Mock;
+
+const ORG_ID = 'org_1';
+const TASK_ID = 'tsk_1';
+const USER_ID = 'usr_1';
+
+// Builds the existingTask row updateTask() reads. evidenceApprovalEnabled is
+// folded into the task's organization relation (a single query); pass null to
+// simulate a missing organization row.
+const existing = (
+  approvalEnabled: boolean | null,
+  overrides: Record<string, unknown> = {},
+) => ({
+  id: TASK_ID,
+  title: 'Test task',
+  status: TaskStatus.todo,
+  assigneeId: null,
+  approverId: null,
+  frequency: null,
+  organization:
+    approvalEnabled === null
+      ? null
+      : { evidenceApprovalEnabled: approvalEnabled },
+  ...overrides,
+});
+
+describe('TasksService approval gating', () => {
+  let service: TasksService;
+
+  const notifier = {
+    notifyStatusChange: jest.fn().mockResolvedValue(undefined),
+    notifyAssigneeChange: jest.fn().mockResolvedValue(undefined),
+    notifyBulkStatusChange: jest.fn().mockResolvedValue(undefined),
+  };
+
+  beforeEach(async () => {
+    jest.clearAllMocks();
+    const moduleRef = await Test.createTestingModule({
+      providers: [
+        TasksService,
+        { provide: TaskNotifierService, useValue: notifier },
+        { provide: TimelinesService, useValue: {} },
+      ],
+    }).compile();
+    service = moduleRef.get(TasksService);
+    memberFindFirst.mockResolvedValue({ id: 'mem_actor', user: { role: 'owner' } });
+    taskUpdate.mockResolvedValue({ id: TASK_ID });
+    auditLogCreate.mockResolvedValue({});
+  });
+
+  const lastUpdateData = () => taskUpdate.mock.calls[0][0].data;
+
+  describe('updateTask', () => {
+    it('REGRESSION: approval OFF + stale approver allows todo→done and clears the approver', async () => {
+      taskFindFirst.mockResolvedValue(existing(false, { approverId: 'mem_appr' }));
+
+      await service.updateTask(ORG_ID, TASK_ID, { status: TaskStatus.done }, USER_ID);
+
+      expect(taskUpdate).toHaveBeenCalledTimes(1);
+      expect(lastUpdateData().status).toBe(TaskStatus.done);
+      expect(lastUpdateData().approverId).toBeNull();
+    });
+
+    it('PRESERVE: approval ON + approver assigned blocks a direct todo→done', async () => {
+      taskFindFirst.mockResolvedValue(existing(true, { approverId: 'mem_appr' }));
+
+      await expect(
+        service.updateTask(ORG_ID, TASK_ID, { status: TaskStatus.done }, USER_ID),
+      ).rejects.toThrow(
+        'Cannot mark task as done directly when an approver is assigned. Submit for review instead.',
+      );
+      expect(taskUpdate).not.toHaveBeenCalled();
+    });
+
+    it('ALLOWED: approval ON + no approver allows todo→done', async () => {
+      taskFindFirst.mockResolvedValue(existing(true, { approverId: null }));
+
+      await service.updateTask(ORG_ID, TASK_ID, { status: TaskStatus.done }, USER_ID);
+
+      expect(taskUpdate).toHaveBeenCalledTimes(1);
+      expect(lastUpdateData().status).toBe(TaskStatus.done);
+    });
+
+    it('SYMMETRIC: approval OFF lets an in_review task move to done and clears the approver', async () => {
+      taskFindFirst.mockResolvedValue(
+        existing(false, { status: TaskStatus.in_review, approverId: 'mem_appr' }),
+      );
+
+      await service.updateTask(ORG_ID, TASK_ID, { status: TaskStatus.done }, USER_ID);
+
+      expect(lastUpdateData().status).toBe(TaskStatus.done);
+      expect(lastUpdateData().approverId).toBeNull();
+    });
+
+    it('IN_REVIEW LOCK: approval ON blocks moving an in_review task directly', async () => {
+      taskFindFirst.mockResolvedValue(existing(true, { status: TaskStatus.in_review }));
+
+      await expect(
+        service.updateTask(ORG_ID, TASK_ID, { status: TaskStatus.todo }, USER_ID),
+      ).rejects.toThrow('Cannot change status directly while task is in review');
+      expect(taskUpdate).not.toHaveBeenCalled();
+    });
+
+    it('SELF-HEAL GUARD: an explicit approverId from the caller is not force-cleared', async () => {
+      taskFindFirst.mockResolvedValue(existing(false, { approverId: 'mem_old' }));
+
+      await service.updateTask(ORG_ID, TASK_ID, { approverId: 'mem_new' }, USER_ID);
+
+      expect(lastUpdateData().approverId).toBe('mem_new');
+    });
+
+    it('NO-OP: approval OFF + no approver leaves approverId untouched on todo→done', async () => {
+      taskFindFirst.mockResolvedValue(existing(false, { approverId: null }));
+
+      await service.updateTask(ORG_ID, TASK_ID, { status: TaskStatus.done }, USER_ID);
+
+      expect(lastUpdateData().status).toBe(TaskStatus.done);
+      expect(lastUpdateData().approverId).toBeUndefined();
+    });
+
+    it('DEFENSIVE: a missing organization row is treated as approval disabled', async () => {
+      taskFindFirst.mockResolvedValue(existing(null, { approverId: 'mem_appr' }));
+
+      await service.updateTask(ORG_ID, TASK_ID, { status: TaskStatus.done }, USER_ID);
+
+      expect(lastUpdateData().status).toBe(TaskStatus.done);
+      expect(lastUpdateData().approverId).toBeNull();
+    });
+  });
+
+  describe('updateTasksStatus (bulk)', () => {
+    it('approval OFF: bulk done does not exclude approver-assigned / in_review tasks', async () => {
+      orgFindUnique.mockResolvedValue({ evidenceApprovalEnabled: false });
+      taskUpdateMany.mockResolvedValue({ count: 1 });
+
+      await service.updateTasksStatus(
+        ORG_ID,
+        [TASK_ID],
+        TaskStatus.done,
+        undefined,
+        USER_ID,
+      );
+
+      const where = taskUpdateMany.mock.calls[0][0].where;
+      expect(where).toEqual({ id: { in: [TASK_ID] }, organizationId: ORG_ID });
+      expect(where.approverId).toBeUndefined();
+      expect(where.status).toBeUndefined();
+    });
+
+    it('approval ON: bulk done excludes approver-assigned and in_review tasks', async () => {
+      orgFindUnique.mockResolvedValue({ evidenceApprovalEnabled: true });
+      taskUpdateMany.mockResolvedValue({ count: 1 });
+
+      await service.updateTasksStatus(
+        ORG_ID,
+        [TASK_ID],
+        TaskStatus.done,
+        undefined,
+        USER_ID,
+      );
+
+      const where = taskUpdateMany.mock.calls[0][0].where;
+      expect(where.approverId).toBeNull();
+      expect(where.status).toEqual({ not: 'in_review' });
+    });
+  });
+});

apps/api/src/tasks/tasks.service.ts

@@ -375,17 +375,29 @@ export class TasksService {
     notRelevantJustification?: string,
   ): Promise<{ updatedCount: number }> {
     try {
-      // Enforce approval workflow: exclude tasks that can't be bulk-updated
+      // The approval-workflow constraints only apply when evidence approval is
+      // enabled for the org. With it disabled, applying them would silently drop
+      // tasks carrying a stale approverId / in_review status from the bulk
+      // update (and throw "No tasks were updated" if all targets are dropped).
+      const organization = await db.organization.findUnique({
+        where: { id: organizationId },
+        select: { evidenceApprovalEnabled: true },
+      });
+      const evidenceApprovalEnabled =
+        organization?.evidenceApprovalEnabled ?? false;
+
       const where: Record<string, unknown> = {
         id: { in: taskIds },
         organizationId,
-        // Cannot change status of tasks currently in review
-        status: { not: 'in_review' as TaskStatus },
       };
 
-      // Cannot mark tasks as done if they have an approver assigned
-      if (status === TaskStatus.done) {
-        where.approverId = null;
+      if (evidenceApprovalEnabled) {
+        // Cannot change status of tasks currently in review
+        where.status = { not: 'in_review' as TaskStatus };
+        // Cannot mark tasks as done if they have an approver assigned
+        if (status === TaskStatus.done) {
+          where.approverId = null;
+        }
       }
 
       const justificationData =
@@ -586,13 +598,22 @@ export class TasksService {
           status: true,
           assigneeId: true,
           approverId: true,
+          // The submit-for-review → approve workflow only applies when the org
+          // has evidence approval enabled. When it's disabled, the approver
+          // dropdown and review flow are hidden in the UI, so a leftover
+          // approverId / in_review status must never block a direct status
+          // change (otherwise the task is wedged with no way to clear it).
+          organization: { select: { evidenceApprovalEnabled: true } },
         },
       });
 
       if (!existingTask) {
         throw new BadRequestException('Task not found or access denied');
       }
 
+      const evidenceApprovalEnabled =
+        existingTask.organization?.evidenceApprovalEnabled ?? false;
+
       // Prepare update data - Prisma handles updatedAt automatically
       const dataToUpdate: {
         title?: string;
@@ -614,25 +635,30 @@ export class TasksService {
         dataToUpdate.description = updateData.description;
       }
       if (updateData.status !== undefined) {
-        // Prevent bypassing the approval workflow via direct status change
-        if (
-          existingTask.status === 'in_review' &&
-          updateData.status !== 'in_review'
-        ) {
-          throw new BadRequestException(
-            'Cannot change status directly while task is in review. Use the approve or reject actions instead.',
-          );
-        }
-        // Prevent directly setting status to 'done' when an approver is assigned
-        // (must go through submitForReview → approveTask workflow)
-        if (
-          updateData.status === 'done' &&
-          existingTask.status !== 'done' &&
-          existingTask.approverId
-        ) {
-          throw new BadRequestException(
-            'Cannot mark task as done directly when an approver is assigned. Submit for review instead.',
-          );
+        // Only enforce the approval-workflow locks when evidence approval is
+        // enabled for the org. With it disabled, these would wedge a task that
+        // carries a stale approverId / in_review status (see comment above).
+        if (evidenceApprovalEnabled) {
+          // Prevent bypassing the approval workflow via direct status change
+          if (
+            existingTask.status === 'in_review' &&
+            updateData.status !== 'in_review'
+          ) {
+            throw new BadRequestException(
+              'Cannot change status directly while task is in review. Use the approve or reject actions instead.',
+            );
+          }
+          // Prevent directly setting status to 'done' when an approver is assigned
+          // (must go through submitForReview → approveTask workflow)
+          if (
+            updateData.status === 'done' &&
+            existingTask.status !== 'done' &&
+            existingTask.approverId
+          ) {
+            throw new BadRequestException(
+              'Cannot mark task as done directly when an approver is assigned. Submit for review instead.',
+            );
+          }
         }
         dataToUpdate.status = updateData.status;
 
@@ -661,6 +687,11 @@ export class TasksService {
       if (updateData.approverId !== undefined) {
         dataToUpdate.approverId =
           updateData.approverId === null ? null : updateData.approverId;
+      } else if (!evidenceApprovalEnabled && existingTask.approverId) {
+        // Self-heal: with approval disabled, a leftover approverId is invisible
+        // in the UI and non-functional — clear it on any update so it can't
+        // silently re-block future status changes.
+        dataToUpdate.approverId = null;
       }
       if (updateData.frequency !== undefined) {
         dataToUpdate.frequency = updateData.frequency;