fix(policy): capture and display policy signature timestamps

apps/portal/src/app/api/portal/accept-policies/route.test.ts

@@ -0,0 +1,193 @@
+import { NextRequest } from 'next/server';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+// CS-790 regression guard: accepting a policy only pushed the member id onto
+// `Policy.signedBy`, which carries no timestamp — so nothing recorded WHEN an
+// employee signed and auditors could not be shown an acknowledgment date. The
+// acceptance must also write an audit log (the timestamped record the policy
+// Activity tab and /v1/audit-logs read), atomically with the signature, and
+// exactly once per member even when accepts for the same policy overlap.
+
+const mocks = vi.hoisted(() => ({
+  getSession: vi.fn(),
+  memberFindFirst: vi.fn(),
+  policyFindFirst: vi.fn(),
+  policyUpdateMany: vi.fn(),
+  auditLogCreate: vi.fn(),
+  transaction: vi.fn(),
+}));
+
+vi.mock('@/app/lib/auth', () => ({
+  auth: { api: { getSession: mocks.getSession } },
+}));
+
+vi.mock('@db/server', () => ({
+  db: {
+    member: { findFirst: mocks.memberFindFirst },
+    policy: { findFirst: mocks.policyFindFirst, updateMany: mocks.policyUpdateMany },
+    auditLog: { create: mocks.auditLogCreate },
+    $transaction: mocks.transaction,
+  },
+}));
+
+import { POST } from './route';
+
+const MEMBER = {
+  id: 'mem_1',
+  userId: 'user_1',
+  organizationId: 'org_1',
+  deactivated: false,
+};
+
+const POLICY = {
+  id: 'pol_1',
+  name: 'Code of Conduct',
+  currentVersionId: 'pv_1',
+};
+
+/** The transaction client exposes the same models as `db`, like Prisma's does. */
+const TX = {
+  policy: { updateMany: mocks.policyUpdateMany },
+  auditLog: { create: mocks.auditLogCreate },
+};
+
+function makeRequest(body: unknown): NextRequest {
+  return new NextRequest('http://localhost/api/portal/accept-policies', {
+    method: 'POST',
+    headers: { 'content-type': 'application/json' },
+    body: JSON.stringify(body),
+  });
+}
+
+describe('POST /api/portal/accept-policies', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mocks.transaction.mockImplementation((arg) => arg(TX));
+    // Models the row lock behind the conditional claim: whichever accept gets
+    // to the row first matches it, every later one matches zero rows.
+    let claimed = false;
+    mocks.policyUpdateMany.mockImplementation(async () => {
+      if (claimed) {
+        return { count: 0 };
+      }
+      claimed = true;
+      return { count: 1 };
+    });
+  });
+
+  it('returns 401 when there is no session', async () => {
+    mocks.getSession.mockResolvedValue(null);
+
+    const res = await POST(makeRequest({ policyIds: ['pol_1'], memberId: 'mem_1' }));
+
+    expect(res.status).toBe(401);
+    expect(mocks.auditLogCreate).not.toHaveBeenCalled();
+  });
+
+  it('returns 403 when the member does not belong to the authenticated user', async () => {
+    mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } });
+    mocks.memberFindFirst.mockResolvedValue(null);
+
+    const res = await POST(makeRequest({ policyIds: ['pol_1'], memberId: 'mem_other' }));
+
+    expect(res.status).toBe(403);
+    expect(mocks.policyUpdateMany).not.toHaveBeenCalled();
+    expect(mocks.auditLogCreate).not.toHaveBeenCalled();
+  });
+
+  it('records the signature and its timestamp in one transaction', async () => {
+    mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } });
+    mocks.memberFindFirst.mockResolvedValue(MEMBER);
+    mocks.policyFindFirst.mockResolvedValue(POLICY);
+
+    const res = await POST(makeRequest({ policyIds: ['pol_1'], memberId: 'mem_1' }));
+
+    expect(res.status).toBe(200);
+    // Conditional write, so an already-signed member cannot be appended twice.
+    expect(mocks.policyUpdateMany).toHaveBeenCalledWith({
+      where: {
+        id: 'pol_1',
+        organizationId: 'org_1',
+        NOT: { signedBy: { has: 'mem_1' } },
+      },
+      data: { signedBy: { push: 'mem_1' } },
+    });
+    // The timestamped acceptance record: AuditLog.timestamp defaults to now(),
+    // so this row is what answers "when did this member sign this policy".
+    expect(mocks.auditLogCreate).toHaveBeenCalledWith({
+      data: expect.objectContaining({
+        organizationId: 'org_1',
+        userId: 'user_1',
+        memberId: 'mem_1',
+        entityType: 'policy',
+        entityId: 'pol_1',
+        description: 'accepted this policy',
+        data: expect.objectContaining({ action: 'accept', policyVersionId: 'pv_1' }),
+      }),
+    });
+    // Claim and log are one atomic unit — a signature without its timestamp is
+    // the bug, so the check must happen inside the transaction, not before it.
+    expect(mocks.transaction).toHaveBeenCalledTimes(1);
+    expect(mocks.transaction.mock.calls[0][0]).toBeTypeOf('function');
+  });
+
+  it('does not re-log an acceptance the member already made', async () => {
+    mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } });
+    mocks.memberFindFirst.mockResolvedValue(MEMBER);
+    mocks.policyFindFirst.mockResolvedValue(POLICY);
+    // The member is already in signedBy, so the conditional claim matches nothing.
+    mocks.policyUpdateMany.mockResolvedValue({ count: 0 });
+
+    const res = await POST(makeRequest({ policyIds: ['pol_1'], memberId: 'mem_1' }));
+
+    expect(res.status).toBe(200);
+    // Re-accepting must keep the original signing time, not stamp a new one.
+    expect(mocks.auditLogCreate).not.toHaveBeenCalled();
+  });
+
+  it('logs one acceptance when two accepts of the same policy overlap', async () => {
+    mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } });
+    mocks.memberFindFirst.mockResolvedValue(MEMBER);
+    mocks.policyFindFirst.mockResolvedValue(POLICY);
+
+    const [first, second] = await Promise.all([
+      POST(makeRequest({ policyIds: ['pol_1'], memberId: 'mem_1' })),
+      POST(makeRequest({ policyIds: ['pol_1'], memberId: 'mem_1' })),
+    ]);
+
+    expect(first.status).toBe(200);
+    expect(second.status).toBe(200);
+    // Both requests read the policy before either wrote, so only the atomic
+    // claim keeps this from becoming two signatures with two timestamps.
+    expect(mocks.policyUpdateMany).toHaveBeenCalledTimes(2);
+    expect(mocks.auditLogCreate).toHaveBeenCalledTimes(1);
+  });
+
+  it('accepts a duplicated policy id once', async () => {
+    mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } });
+    mocks.memberFindFirst.mockResolvedValue(MEMBER);
+    mocks.policyFindFirst.mockResolvedValue(POLICY);
+
+    const res = await POST(makeRequest({ policyIds: ['pol_1', 'pol_1'], memberId: 'mem_1' }));
+
+    expect(res.status).toBe(200);
+    expect(mocks.policyFindFirst).toHaveBeenCalledTimes(1);
+    expect(mocks.auditLogCreate).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not sign a policy outside the member organization', async () => {
+    mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } });
+    mocks.memberFindFirst.mockResolvedValue(MEMBER);
+    // Scoped lookup finds nothing for another org's policy id.
+    mocks.policyFindFirst.mockResolvedValue(null);
+
+    const res = await POST(makeRequest({ policyIds: ['pol_other_org'], memberId: 'mem_1' }));
+
+    expect(res.status).toBe(200);
+    expect(mocks.policyFindFirst).toHaveBeenCalledWith(
+      expect.objectContaining({ where: { id: 'pol_other_org', organizationId: 'org_1' } }),
+    );
+    expect(mocks.policyUpdateMany).not.toHaveBeenCalled();
+    expect(mocks.auditLogCreate).not.toHaveBeenCalled();
+  });
+});

apps/portal/src/app/api/portal/accept-policies/route.ts

@@ -1,4 +1,5 @@
 import { auth } from '@/app/lib/auth';
+import { acceptPolicyForMember } from '@/lib/policy-acceptance';
 import { db } from '@db/server';
 import { type NextRequest, NextResponse } from 'next/server';
 import { z } from 'zod';
@@ -44,25 +45,12 @@ export async function POST(req: NextRequest) {
   }
 
   try {
-    const updatePromises = policyIds.map(async (policyId) => {
-      const policy = await db.policy.findUnique({
-        where: { id: policyId },
-      });
-
-      if (policy && !policy.signedBy.includes(memberId)) {
-        return db.policy.update({
-          where: { id: policyId },
-          data: {
-            signedBy: {
-              push: memberId,
-            },
-          },
-        });
-      }
-      return null;
-    });
-
-    await Promise.all(updatePromises);
+    await Promise.all(
+      // Deduplicated so a repeated id does not race itself into two acceptances.
+      [...new Set(policyIds)].map((policyId) =>
+        acceptPolicyForMember({ policyId, member, userId: session.user.id }),
+      ),
+    );
 
     return NextResponse.json({ success: true });
   } catch (error) {

apps/portal/src/app/api/portal/mark-policy-completed/route.ts

@@ -1,4 +1,5 @@
 import { auth } from '@/app/lib/auth';
+import { acceptPolicyForMember } from '@/lib/policy-acceptance';
 import { db } from '@db/server';
 import { type NextRequest, NextResponse } from 'next/server';
 import { z } from 'zod';
@@ -40,25 +41,19 @@ export async function POST(req: NextRequest) {
     return NextResponse.json({ error: 'Member not found' }, { status: 404 });
   }
 
-  const policy = await db.policy.findUnique({
-    where: { id: policyId },
+  const result = await acceptPolicyForMember({
+    policyId,
+    member,
+    userId: session.user.id,
   });
 
-  if (!policy) {
+  if (result === 'not-found') {
     return NextResponse.json({ error: 'Policy not found' }, { status: 404 });
   }
 
-  // Check if user has already signed this policy
-  if (policy.signedBy.includes(member.id)) {
+  if (result === 'already-signed') {
     return NextResponse.json({ success: true, alreadySigned: true });
   }
 
-  await db.policy.update({
-    where: { id: policyId },
-    data: {
-      signedBy: [...policy.signedBy, member.id],
-    },
-  });
-
   return NextResponse.json({ success: true });
 }

apps/portal/src/lib/policy-acceptance.ts

@@ -0,0 +1,77 @@
+import type { Member } from '@db';
+import { db } from '@db/server';
+
+/** Outcome of a single acceptance attempt, so routes can map it to a response. */
+export type PolicyAcceptanceResult = 'accepted' | 'already-signed' | 'not-found';
+
+/**
+ * Accepts a policy for `member` and records WHEN it happened.
+ *
+ * `Policy.signedBy` is a flat array of member ids with no timestamp, so the
+ * acceptance audit log is the only record of the signing time. It is what the
+ * policy Activity tab and `GET /v1/audit-logs?entityType=policy&entityId=...`
+ * read to answer "when did X sign policy Y" for auditors. Both writes go in one
+ * transaction so a signature can never exist without its timestamp.
+ *
+ * Re-accepting is a no-op, keeping the logged time the first acceptance of the
+ * current version (publishing a new version clears `signedBy`).
+ */
+export async function acceptPolicyForMember({
+  policyId,
+  member,
+  userId,
+}: {
+  policyId: string;
+  member: Pick<Member, 'id' | 'organizationId'>;
+  userId: string;
+}): Promise<PolicyAcceptanceResult> {
+  const policy = await db.policy.findFirst({
+    where: { id: policyId, organizationId: member.organizationId },
+    select: { id: true, name: true, currentVersionId: true },
+  });
+
+  if (!policy) {
+    return 'not-found';
+  }
+
+  const accepted = await db.$transaction(async (tx) => {
+    // The signature is claimed with a conditional update instead of a
+    // read-then-write check: Postgres re-evaluates the `NOT has` filter after
+    // taking the row lock, so a concurrent accept of the same policy matches
+    // zero rows rather than appending a second signature and a second log.
+    const { count } = await tx.policy.updateMany({
+      where: {
+        id: policy.id,
+        organizationId: member.organizationId,
+        NOT: { signedBy: { has: member.id } },
+      },
+      data: { signedBy: { push: member.id } },
+    });
+
+    if (count === 0) {
+      return false;
+    }
+
+    await tx.auditLog.create({
+      data: {
+        organizationId: member.organizationId,
+        userId,
+        memberId: member.id,
+        entityType: 'policy',
+        entityId: policy.id,
+        description: 'accepted this policy',
+        data: {
+          action: 'accept',
+          policyName: policy.name,
+          // Which version was accepted — a republish clears signedBy, so an
+          // acceptance always belongs to the version current at that moment.
+          policyVersionId: policy.currentVersionId,
+        },
+      },
+    });
+
+    return true;
+  });
+
+  return accepted ? 'accepted' : 'already-signed';
+}