fix(people): address fourth round of cubic review findings

apps/api/src/offboarding-checklist/access-revocation.service.ts

@@ -1,13 +1,16 @@
 import {
   BadRequestException,
   Injectable,
+  Logger,
   NotFoundException,
 } from '@nestjs/common';
 import { AttachmentEntityType, db } from '@db';
 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({
@@ -127,11 +130,15 @@ export class AccessRevocationService {
       }
     }
 
-    await this.syncAccessRevocationCompletion(
-      organizationId,
-      memberId,
-      revokedById,
-    );
+    try {
+      await this.syncAccessRevocationCompletion(
+        organizationId,
+        memberId,
+        revokedById,
+      );
+    } catch (err) {
+      this.logger.warn(`Failed to sync access revocation completion for member ${memberId}`, err);
+    }
 
     return revocation;
   }

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,
@@ -513,6 +514,7 @@ describe('OffboardingChecklistService', () => {
         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({

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}`,
         });
       }
     }

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<PendingOffboardingResponse>('/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({
         </div>
       </CardHeader>
       <CardContent className="flex flex-col gap-4">
-        <Tabs
-          defaultValue={
-            pendingOffboardings.length > 0
-              ? 'offboarding'
-              : unpublishedPolicies.length === 0
-                ? 'tasks'
-                : 'policies'
-          }
-          className="w-full"
-        >
+        <Tabs value={activeTab} onValueChange={setActiveTab} className="w-full">
           <TabsList className="grid w-full grid-cols-3">
             <TabsTrigger
               value="policies"

apps/app/src/app/(app)/[orgId]/people/[employeeId]/components/OffboardingChecklistItem.tsx

@@ -223,7 +223,7 @@ export function OffboardingChecklistItem({
     item.isAccessRevocation || item.evidenceRequired || canEdit;
 
   return (
-    <Collapsible open={isOpen} onOpenChange={setIsOpen}>
+    <Collapsible open={isExpandable ? isOpen : false} onOpenChange={isExpandable ? setIsOpen : undefined}>
       <div className="overflow-hidden rounded-lg border bg-background">
         <CollapsibleTrigger className="flex w-full items-center gap-2 px-3.5 py-3 text-left transition-colors hover:bg-muted/50">
           <ChecklistStatusCircle item={item} memberId={memberId} />

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 };
 }
 

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;
     }