fix(people): address cubic review findings for offboarding feature

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

@@ -132,8 +132,8 @@ export class AccessRevocationService {
     memberId: string;
     vendorId: string;
   }) {
-    const revocation = await db.offboardingAccessRevocation.findUnique({
-      where: { memberId_vendorId: { memberId, vendorId } },
+    const revocation = await db.offboardingAccessRevocation.findFirst({
+      where: { memberId, vendorId, organizationId },
     });
 
     if (!revocation) {

apps/api/src/offboarding-checklist/dto/update-template-item.dto.ts

@@ -1,5 +1,5 @@
 import { ApiProperty } from '@nestjs/swagger';
-import { IsString, IsOptional, IsBoolean, IsNumber } from 'class-validator';
+import { IsString, IsOptional, IsBoolean, IsInt } from 'class-validator';
 
 export class UpdateTemplateItemDto {
   @ApiProperty({ required: false })
@@ -19,7 +19,7 @@ export class UpdateTemplateItemDto {
 
   @ApiProperty({ required: false })
   @IsOptional()
-  @IsNumber()
+  @IsInt()
   sortOrder?: number;
 
   @ApiProperty({ required: false })

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

@@ -195,17 +195,24 @@ export class OffboardingChecklistService {
     });
 
     if (dto.fileName && dto.fileData && dto.fileType) {
-      await this.attachmentsService.uploadAttachment(
-        organizationId,
-        completion.id,
-        AttachmentEntityType.offboarding_checklist,
-        {
-          fileName: dto.fileName,
-          fileData: dto.fileData,
-          fileType: dto.fileType,
-        },
-        completedById,
-      );
+      try {
+        await this.attachmentsService.uploadAttachment(
+          organizationId,
+          completion.id,
+          AttachmentEntityType.offboarding_checklist,
+          {
+            fileName: dto.fileName,
+            fileData: dto.fileData,
+            fileType: dto.fileType,
+          },
+          completedById,
+        );
+      } catch (err) {
+        await db.offboardingChecklistCompletion.delete({
+          where: { id: completion.id },
+        });
+        throw err;
+      }
     }
 
     return completion;
@@ -327,7 +334,12 @@ export class OffboardingChecklistService {
       },
       include: {
         user: { select: { id: true, name: true, email: true, image: true } },
-        offboardingChecklistCompletions: { select: { id: true } },
+        offboardingChecklistCompletions: {
+          where: {
+            templateItem: { organizationId, isEnabled: true },
+          },
+          select: { id: true },
+        },
       },
       orderBy: { offboardDate: 'desc' },
     });

apps/api/src/offboarding-checklist/offboarding-export.service.ts

@@ -111,8 +111,9 @@ export class OffboardingExportService {
       for (const file of vendor.evidence) {
         const buffer = await this.getAttachmentBuffer(organizationId, file.id);
         if (!buffer) continue;
+        const safeName = sanitizeFileName(file.name);
         archive.append(buffer, {
-          name: `${prefix}vendor-access-revocations/evidence/${file.name}`,
+          name: `${prefix}vendor-access-revocations/evidence/${safeName}`,
         });
       }
     }
@@ -135,8 +136,9 @@ export class OffboardingExportService {
       for (const file of item.evidence) {
         const buffer = await this.getAttachmentBuffer(organizationId, file.id);
         if (!buffer) continue;
+        const safeName = sanitizeFileName(file.name);
         archive.append(buffer, {
-          name: `${prefix}checklist-items/${folderNum}-${folderName}/${file.name}`,
+          name: `${prefix}checklist-items/${folderNum}-${folderName}/${safeName}`,
         });
       }
     }
@@ -163,7 +165,7 @@ export class OffboardingExportService {
         .replace(/[^a-zA-Z0-9 ]/g, '')
         .replace(/\s+/g, '-')
         .toLowerCase();
-      const prefix = `offboarded-employees/${safeName}/`;
+      const prefix = `offboarded-employees/${safeName}-${member.id}/`;
 
       const checklist = await this.offboardingChecklistService.getMemberChecklist(
         organizationId,
@@ -199,6 +201,14 @@ export class OffboardingExportService {
   }
 }
 
+function sanitizeFileName(name: string): string {
+  return name.replace(/.*[/\\]/, '').replace(/[/\\]/g, '_') || 'file';
+}
+
 function escapeCsvField(value: string): string {
-  return value.replace(/"/g, '""');
+  const escaped = value.replace(/"/g, '""');
+  if (/^[=+\-@\t\r]/.test(escaped)) {
+    return `'${escaped}`;
+  }
+  return escaped;
 }

apps/api/src/people/people.controller.ts

@@ -117,11 +117,20 @@ export class PeopleController {
     @Query('offboardAfter') offboardAfter?: string,
     @Query('offboardBefore') offboardBefore?: string,
   ) {
+    const parseDateParam = (param: string | undefined, name: string): Date | undefined => {
+      if (!param) return undefined;
+      const date = new Date(param);
+      if (isNaN(date.getTime())) {
+        throw new BadRequestException(`Invalid date value for "${name}": ${param}`);
+      }
+      return date;
+    };
+
     const filters = {
-      ...(onboardAfter ? { onboardAfter: new Date(onboardAfter) } : {}),
-      ...(onboardBefore ? { onboardBefore: new Date(onboardBefore) } : {}),
-      ...(offboardAfter ? { offboardAfter: new Date(offboardAfter) } : {}),
-      ...(offboardBefore ? { offboardBefore: new Date(offboardBefore) } : {}),
+      ...(onboardAfter ? { onboardAfter: parseDateParam(onboardAfter, 'onboardAfter') } : {}),
+      ...(onboardBefore ? { onboardBefore: parseDateParam(onboardBefore, 'onboardBefore') } : {}),
+      ...(offboardAfter ? { offboardAfter: parseDateParam(offboardAfter, 'offboardAfter') } : {}),
+      ...(offboardBefore ? { offboardBefore: parseDateParam(offboardBefore, 'offboardBefore') } : {}),
     };
 
     const hasFilters = Object.keys(filters).length > 0;
@@ -610,8 +619,16 @@ export class PeopleController {
     @Param('attachmentId') attachmentId: string,
     @OrganizationId() organizationId: string,
   ) {
-    this.resolveEventType(eventType);
+    const entityType = this.resolveEventType(eventType);
     await this.peopleService.findById(memberId, organizationId);
+    const attachments = await this.attachmentsService.getAttachments(
+      organizationId,
+      memberId,
+      entityType,
+    );
+    if (!attachments.some((a) => a.id === attachmentId)) {
+      throw new BadRequestException('Attachment not found for this member and event type');
+    }
     await this.attachmentsService.deleteAttachment(organizationId, attachmentId);
     return { success: true };
   }

apps/app/src/app/(app)/[orgId]/overview/components/TodosOverview.tsx

@@ -23,7 +23,7 @@ interface PendingResponse {
 export function TodosOverview() {
   const params = useParams<{ orgId: string }>();
   const organizationId = params.orgId;
-  const { data, isLoading } = useApiSWR<PendingResponse>(
+  const { data, isLoading, error } = useApiSWR<PendingResponse>(
     '/v1/offboarding-checklist/pending',
   );
   const members = data?.data?.members ?? [];
@@ -44,6 +44,12 @@ export function TodosOverview() {
           <div className="flex items-center justify-center py-8">
             <Text variant="muted">Loading...</Text>
           </div>
+        ) : error ? (
+          <div className="flex items-center justify-center py-8">
+            <Text size="sm" variant="muted">
+              Failed to load todos
+            </Text>
+          </div>
         ) : members.length === 0 ? (
           <div className="flex flex-col items-center justify-center gap-2 py-8">
             <div className="flex h-8 w-8 items-center justify-center rounded-full bg-primary/10">

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

@@ -74,7 +74,15 @@ export function Employee({
   const pathname = usePathname();
   const router = useRouter();
 
-  const VALID_TABS: EmployeeTab[] = ['details', 'policies', 'training', 'hipaa', 'device', 'offboarding', 'background-check'];
+  const availableTabs: EmployeeTab[] = [
+    'details',
+    'policies',
+    'training',
+    ...(hasHipaaFramework ? (['hipaa'] as EmployeeTab[]) : []),
+    'device',
+    ...(backgroundCheckStepEnabled ? (['background-check'] as EmployeeTab[]) : []),
+    ...(employee.offboardDate ? (['offboarding'] as EmployeeTab[]) : []),
+  ];
 
   const resolveTab = (): EmployeeTab => {
     if (
@@ -84,7 +92,7 @@ export function Employee({
       return 'background-check';
     }
     const tabParam = searchParams.get('tab');
-    if (tabParam && VALID_TABS.includes(tabParam as EmployeeTab)) {
+    if (tabParam && availableTabs.includes(tabParam as EmployeeTab)) {
       return tabParam as EmployeeTab;
     }
     return 'details';

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

@@ -198,8 +198,11 @@ export function OffboardingChecklistItem({
   const handleFileSelect = async (e: React.ChangeEvent<HTMLInputElement>) => {
     const file = e.target.files?.[0];
     if (!file) return;
-    await handleFileUpload(file);
-    if (dropzoneInputRef.current) dropzoneInputRef.current.value = '';
+    try {
+      await handleFileUpload(file);
+    } finally {
+      if (dropzoneInputRef.current) dropzoneInputRef.current.value = '';
+    }
   };
 
   const handleFileUpload = async (file: File) => {

apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx

@@ -646,7 +646,9 @@ function DateRangeFilter({
     ? `${format(from, 'MMM d')} – ${format(to, 'MMM d, yyyy')}`
     : from
       ? `From ${format(from, 'MMM d, yyyy')}`
-      : 'Any time';
+      : to
+        ? `Until ${format(to, 'MMM d, yyyy')}`
+        : 'Any time';
 
   return (
     <div className="hidden sm:block">