From acc0f3b0536512be04b1ea13e5078b809ae08c92 Mon Sep 17 00:00:00 2001 From: Mariano Fuentes Date: Thu, 21 May 2026 11:01:46 -0400 Subject: [PATCH 1/2] fix(people): address fourth round of cubic review findings - add findFirst mock to test spec for undoVendorRevocation - prefix checklist evidence filenames with ID to prevent ZIP collisions - wrap sync call in try/catch so revocation stands on sync failure - prevent non-expandable checklist rows from toggling open - normalize preset date ranges to start of day - hide pending invitations when date filters are active - use controlled tab value in ToDoOverview for async data - revalidate from server on error instead of restoring stale snapshot Co-Authored-By: Claude Opus 4.6 (1M context) --- .../access-revocation.service.ts | 12 +++++----- .../offboarding-checklist.service.spec.ts | 8 ++++--- .../offboarding-export.service.ts | 2 +- .../overview/components/ToDoOverview.tsx | 22 +++++++++---------- .../components/OffboardingChecklistItem.tsx | 2 +- .../all/components/TeamMembersClient.tsx | 5 ++++- .../OffboardingChecklistSettings.tsx | 12 +--------- 7 files changed, 30 insertions(+), 33 deletions(-) diff --git a/apps/api/src/offboarding-checklist/access-revocation.service.ts b/apps/api/src/offboarding-checklist/access-revocation.service.ts index e515e283e..14d9eed36 100644 --- a/apps/api/src/offboarding-checklist/access-revocation.service.ts +++ b/apps/api/src/offboarding-checklist/access-revocation.service.ts @@ -119,11 +119,13 @@ export class AccessRevocationService { } } - await this.syncAccessRevocationCompletion( - organizationId, - memberId, - revokedById, - ); + try { + await this.syncAccessRevocationCompletion( + organizationId, + memberId, + revokedById, + ); + } catch {} return revocation; } diff --git a/apps/api/src/offboarding-checklist/offboarding-checklist.service.spec.ts b/apps/api/src/offboarding-checklist/offboarding-checklist.service.spec.ts index b277a5f58..c0fa2a699 100644 --- a/apps/api/src/offboarding-checklist/offboarding-checklist.service.spec.ts +++ b/apps/api/src/offboarding-checklist/offboarding-checklist.service.spec.ts @@ -19,6 +19,7 @@ const mockDb = { }, offboardingAccessRevocation: { findMany: jest.fn(), + findFirst: jest.fn(), findUnique: jest.fn(), create: jest.fn(), delete: jest.fn(), @@ -52,7 +53,7 @@ describe('OffboardingChecklistService', () => { beforeEach(() => { jest.clearAllMocks(); - accessRevocationService = new AccessRevocationService(); + accessRevocationService = new AccessRevocationService(mockAttachmentsService as never); service = new OffboardingChecklistService( mockAttachmentsService as never, accessRevocationService, @@ -509,10 +510,11 @@ describe('OffboardingChecklistService', () => { describe('undoVendorRevocation', () => { it('deletes revocation record', async () => { - mockDb.offboardingAccessRevocation.findUnique.mockResolvedValue({ + mockDb.offboardingAccessRevocation.findFirst.mockResolvedValue({ id: 'oar_1', }); mockDb.offboardingAccessRevocation.delete.mockResolvedValue({}); + mockAttachmentsService.getAttachments.mockResolvedValue([]); // syncAccessRevocationCompletion mocks mockDb.offboardingChecklistTemplate.findFirst.mockResolvedValue(null); @@ -529,7 +531,7 @@ describe('OffboardingChecklistService', () => { }); it('throws if revocation not found', async () => { - mockDb.offboardingAccessRevocation.findUnique.mockResolvedValue(null); + mockDb.offboardingAccessRevocation.findFirst.mockResolvedValue(null); await expect( service.undoVendorRevocation({ diff --git a/apps/api/src/offboarding-checklist/offboarding-export.service.ts b/apps/api/src/offboarding-checklist/offboarding-export.service.ts index 6130996c7..0a3047c63 100644 --- a/apps/api/src/offboarding-checklist/offboarding-export.service.ts +++ b/apps/api/src/offboarding-checklist/offboarding-export.service.ts @@ -139,7 +139,7 @@ export class OffboardingExportService { if (!buffer) continue; const safeName = sanitizeFileName(file.name); archive.append(buffer, { - name: `${prefix}checklist-items/${folderNum}-${folderName}/${safeName}`, + name: `${prefix}checklist-items/${folderNum}-${folderName}/${file.id}-${safeName}`, }); } } diff --git a/apps/app/src/app/(app)/[orgId]/overview/components/ToDoOverview.tsx b/apps/app/src/app/(app)/[orgId]/overview/components/ToDoOverview.tsx index 2c5db70cc..ccb662118 100644 --- a/apps/app/src/app/(app)/[orgId]/overview/components/ToDoOverview.tsx +++ b/apps/app/src/app/(app)/[orgId]/overview/components/ToDoOverview.tsx @@ -19,7 +19,7 @@ import { import { usePermissions } from '@/hooks/use-permissions'; import Link from 'next/link'; import { useRouter } from 'next/navigation'; -import { useMemo, useState } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import { toast } from 'sonner'; import { ConfirmActionDialog } from './ConfirmActionDialog'; @@ -62,6 +62,9 @@ export function ToDoOverview({ const { hasPermission } = usePermissions(); const [isConfirmDialogOpen, setIsConfirmDialogOpen] = useState(false); const [isLoading, setIsLoading] = useState(false); + const [activeTab, setActiveTab] = useState( + unpublishedPolicies.length === 0 ? 'tasks' : 'policies', + ); const { data: pendingData, @@ -70,6 +73,12 @@ export function ToDoOverview({ } = useApiSWR('/v1/offboarding-checklist/pending'); const pendingOffboardings = pendingData?.data?.members ?? []; + useEffect(() => { + if (!isPendingLoading && pendingOffboardings.length > 0) { + setActiveTab('offboarding'); + } + }, [isPendingLoading, pendingOffboardings.length]); + const isOnboardingInProgress = !!onboardingTriggerJobId; const formatStatus = (status: string) => { @@ -135,16 +144,7 @@ export function ToDoOverview({ - 0 - ? 'offboarding' - : unpublishedPolicies.length === 0 - ? 'tasks' - : 'policies' - } - className="w-full" - > + +
diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx index 887e270b3..f3936b718 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx @@ -147,8 +147,10 @@ export function TeamMembersClient({ statusFilter: effectiveStatusFilter, }); + const hasAnyDateFilter = !!(onboardFrom || onboardTo || offboardFrom || offboardTo); + const dateFilteredItems = filteredItems.filter((item) => { - if (item.type !== 'member') return true; + if (item.type !== 'member') return !hasAnyDateFilter; const member = item as MemberWithUser; if (onboardFrom || onboardTo) { @@ -581,6 +583,7 @@ function getPresetRange(days: number): { from: Date | undefined; to: Date | unde const to = new Date(); const from = new Date(); from.setDate(from.getDate() - days); + from.setHours(0, 0, 0, 0); return { from, to }; } diff --git a/apps/app/src/app/(app)/[orgId]/people/settings/components/OffboardingChecklistSettings.tsx b/apps/app/src/app/(app)/[orgId]/people/settings/components/OffboardingChecklistSettings.tsx index 085072297..3377ddd70 100644 --- a/apps/app/src/app/(app)/[orgId]/people/settings/components/OffboardingChecklistSettings.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/settings/components/OffboardingChecklistSettings.tsx @@ -54,7 +54,6 @@ export function OffboardingChecklistSettings() { item: TemplateItem; next: boolean; }) => { - const previous = items; mutate( (current) => { if (!current) return current; @@ -76,16 +75,7 @@ export function OffboardingChecklistSettings() { ); if (res.error) { - mutate( - (current) => { - if (!current) return current; - return { - ...current, - data: previous, - }; - }, - { revalidate: false }, - ); + mutate(); toast.error('Failed to update checklist item'); return; } From e4c8388d7dd83d34902f1dca0d4eba2276e75529 Mon Sep 17 00:00:00 2001 From: Mariano Fuentes Date: Thu, 21 May 2026 11:06:33 -0400 Subject: [PATCH 2/2] fix(people): log sync failures instead of swallowing silently Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/offboarding-checklist/access-revocation.service.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/api/src/offboarding-checklist/access-revocation.service.ts b/apps/api/src/offboarding-checklist/access-revocation.service.ts index 87c77eda0..41a59a792 100644 --- a/apps/api/src/offboarding-checklist/access-revocation.service.ts +++ b/apps/api/src/offboarding-checklist/access-revocation.service.ts @@ -1,6 +1,7 @@ import { BadRequestException, Injectable, + Logger, NotFoundException, } from '@nestjs/common'; import { AttachmentEntityType, db } from '@db'; @@ -8,6 +9,8 @@ import { AttachmentsService } from '../attachments/attachments.service'; @Injectable() export class AccessRevocationService { + private readonly logger = new Logger(AccessRevocationService.name); + constructor(private readonly attachmentsService: AttachmentsService) {} async getAccessRevocations(organizationId: string, memberId: string) { const vendors = await db.vendor.findMany({ @@ -133,7 +136,9 @@ export class AccessRevocationService { memberId, revokedById, ); - } catch {} + } catch (err) { + this.logger.warn(`Failed to sync access revocation completion for member ${memberId}`, err); + } return revocation; }