From 075cba46ee101db55a1b3a2e6e978cd14c110cb8 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 1 Jul 2026 14:53:56 -0400 Subject: [PATCH 01/33] feat(people): redesign TASKS column with per-requirement status rows Replace the collapsed single progress bar + wrapped gray text with a vertical list: fractional requirements (policies, training) show a count + mini bar; binary requirements (HIPAA, device, background) show a Done/Missing badge. Colors use design-system Text/Badge variants (success/warning/muted, accent/secondary). Extracted the rendering into TaskRequirements.tsx to keep MemberRow focused. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../people/all/components/MemberRow.test.tsx | 32 +++---- .../people/all/components/MemberRow.tsx | 57 +++---------- .../MemberRowBackgroundCheck.test.tsx | 6 +- .../all/components/TaskRequirements.test.tsx | 56 +++++++++++++ .../all/components/TaskRequirements.tsx | 83 +++++++++++++++++++ 5 files changed, 169 insertions(+), 65 deletions(-) create mode 100644 apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.test.tsx index 0dd6c16e15..a5924dcb9d 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.test.tsx @@ -86,29 +86,29 @@ describe('MemberRow device status', () => { vi.clearAllMocks(); }); - it('shows "Device 0/1" when deviceStatus is not-installed', () => { + it('shows Device as Missing when deviceStatus is not-installed', () => { renderMemberRow('not-installed'); - expect(screen.getByText('Device 0/1')).toBeInTheDocument(); + expect(screen.getByTestId('requirement-Device')).toHaveTextContent('Missing'); }); - it('shows dash when deviceStatus is omitted (no compliance obligation)', () => { + it('shows no device row when deviceStatus is omitted (no compliance obligation)', () => { renderMemberRow(); - expect(screen.queryByText(/^Device /)).not.toBeInTheDocument(); + expect(screen.queryByTestId('requirement-Device')).not.toBeInTheDocument(); }); - it('shows "Device 1/1" when deviceStatus is compliant', () => { + it('shows Device as Done when deviceStatus is compliant', () => { renderMemberRow('compliant'); - expect(screen.getByText('Device 1/1')).toBeInTheDocument(); + expect(screen.getByTestId('requirement-Device')).toHaveTextContent('Done'); }); - it('shows "Device 0/1" when deviceStatus is non-compliant', () => { + it('shows Device as Missing when deviceStatus is non-compliant', () => { renderMemberRow('non-compliant'); - expect(screen.getByText('Device 0/1')).toBeInTheDocument(); + expect(screen.getByTestId('requirement-Device')).toHaveTextContent('Missing'); }); - it('shows "Device 0/1" when deviceStatus is stale', () => { + it('shows Device as Missing when deviceStatus is stale', () => { renderMemberRow('stale'); - expect(screen.getByText('Device 0/1')).toBeInTheDocument(); + expect(screen.getByTestId('requirement-Device')).toHaveTextContent('Missing'); }); it('does not show device status for platform admin', () => { @@ -134,7 +134,7 @@ describe('MemberRow device status', () => { , ); - expect(screen.queryByText(/^Device /)).not.toBeInTheDocument(); + expect(screen.queryByTestId('requirement-Device')).not.toBeInTheDocument(); }); it('does not show device status for deactivated member', () => { @@ -160,7 +160,7 @@ describe('MemberRow device status', () => { , ); - expect(screen.queryByText(/^Device /)).not.toBeInTheDocument(); + expect(screen.queryByTestId('requirement-Device')).not.toBeInTheDocument(); }); it('does not show device status for member without compliance obligation (e.g. auditor)', () => { @@ -186,7 +186,7 @@ describe('MemberRow device status', () => { , ); - expect(screen.queryByText(/^Device /)).not.toBeInTheDocument(); + expect(screen.queryByTestId('requirement-Device')).not.toBeInTheDocument(); }); it('still shows device status for member with compliance obligation', () => { @@ -212,7 +212,7 @@ describe('MemberRow device status', () => { , ); - expect(screen.getByText('Device 1/1')).toBeInTheDocument(); + expect(screen.getByTestId('requirement-Device')).toHaveTextContent('Done'); }); it('hides the background-check task counter and verified tick when bypassed', () => { @@ -240,7 +240,7 @@ describe('MemberRow device status', () => { , ); - expect(screen.queryByText(/background check/i)).not.toBeInTheDocument(); + expect(screen.queryByTestId('requirement-Background')).not.toBeInTheDocument(); expect( screen.queryByLabelText('Employee has completed a background check'), ).not.toBeInTheDocument(); @@ -273,7 +273,7 @@ describe('MemberRow device status', () => { , ); - expect(screen.queryByText(/background check/i)).not.toBeInTheDocument(); + expect(screen.queryByTestId('requirement-Background')).not.toBeInTheDocument(); expect( screen.queryByLabelText('Employee has completed a background check'), ).not.toBeInTheDocument(); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx index 7a7f067806..b5a617e482 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx @@ -25,7 +25,6 @@ import { DropdownMenuTrigger, HStack, Label, - Skeleton, TableCell, TableRow, Text, @@ -43,6 +42,7 @@ import { toast } from 'sonner'; import { BackgroundCheckVerifiedTick } from '../../components/BackgroundCheckVerifiedTick'; import { MultiRoleCombobox } from './MultiRoleCombobox'; import { RemoveDeviceAlert } from './RemoveDeviceAlert'; +import { TaskRequirements, type TaskRequirementItem } from './TaskRequirements'; import { RemoveMemberAlert } from './RemoveMemberAlert'; import type { CustomRoleOption } from './MultiRoleCombobox'; import type { BackgroundCheckStatus, MemberWithUser, TaskCompletion } from './TeamMembers'; @@ -99,20 +99,6 @@ function isBackgroundCheckComplete(status?: BackgroundCheckStatus): boolean { return status === 'completed' || status === 'completed_with_flags'; } -interface TaskCountItem { - label: string; - completed: number; - total: number; -} - -function TaskCountLabel({ item }: { item: TaskCountItem }) { - return ( - - {item.label} {item.completed}/{item.total} - - ); -} - export function MemberRow({ member, onRemove, @@ -157,13 +143,14 @@ export function MemberRow({ const hasCompletedBackgroundCheck = isBackgroundCheckComplete(backgroundCheckStatus); const memberExempt = member.backgroundCheckExempt === true; const shouldShowTaskRequirements = !isPlatformAdmin && !isDeactivated; - const taskItems: TaskCountItem[] = []; + const taskItems: TaskRequirementItem[] = []; if (taskCompletion) { taskItems.push({ label: 'Policies', completed: taskCompletion.policies.completed, total: taskCompletion.policies.total, + kind: 'count', }); if (taskCompletion.training.total > 0) { @@ -171,6 +158,7 @@ export function MemberRow({ label: 'Training', completed: taskCompletion.training.completed, total: taskCompletion.training.total, + kind: 'count', }); } @@ -179,30 +167,30 @@ export function MemberRow({ label: 'HIPAA', completed: taskCompletion.hipaa.completed, total: taskCompletion.hipaa.total, + kind: 'binary', }); } } - if (shouldShowTaskRequirements && (deviceStatus || isDeviceStatusLoading)) { + if (shouldShowTaskRequirements && deviceStatus) { taskItems.push({ label: 'Device', completed: deviceStatus === 'compliant' ? 1 : 0, total: 1, + kind: 'binary', }); } if (shouldShowTaskRequirements && backgroundCheckStepEnabled && !memberExempt && !isAuditorOnly) { taskItems.push({ - label: 'Background check', + label: 'Background', completed: hasCompletedBackgroundCheck ? 1 : 0, total: 1, + kind: 'binary', }); } - const visibleTaskTotal = taskItems.reduce((sum, item) => sum + item.total, 0); - const visibleTaskCompleted = taskItems.reduce((sum, item) => sum + item.completed, 0); - const taskProgressPercent = - visibleTaskTotal > 0 ? Math.round((visibleTaskCompleted / visibleTaskTotal) * 100) : 0; + const isDeviceLoadingRow = shouldShowTaskRequirements && isDeviceStatusLoading && !deviceStatus; const handleEditRolesClick = () => { setSelectedRoles(parseRoles(member.role)); @@ -353,30 +341,7 @@ export function MemberRow({ {/* TASKS */} - {taskItems.length > 0 ? ( -
-
-
-
-
- {taskItems.map((item) => ( - - ))} - {shouldShowTaskRequirements && isDeviceStatusLoading && ( -
- -
- )} -
-
- ) : ( - - — - - )} + {/* ACTIONS */} diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRowBackgroundCheck.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRowBackgroundCheck.test.tsx index 09d7c528a2..d26916450f 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRowBackgroundCheck.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRowBackgroundCheck.test.tsx @@ -74,12 +74,12 @@ function renderRow({ describe('MemberRow background check status', () => { it('shows background check as incomplete in the tasks column', () => { renderRow({ backgroundCheckStatus: 'invited' }); - expect(screen.getByText('Background check 0/1')).toBeInTheDocument(); + expect(screen.getByTestId('requirement-Background')).toHaveTextContent('Missing'); }); it('shows background check as complete in the tasks column', () => { renderRow({ backgroundCheckStatus: 'completed_with_flags' }); - expect(screen.getByText('Background check 1/1')).toBeInTheDocument(); + expect(screen.getByTestId('requirement-Background')).toHaveTextContent('Done'); expect(screen.getByLabelText('Employee has completed a background check')).toBeInTheDocument(); }); @@ -90,6 +90,6 @@ describe('MemberRow background check status', () => { it('does not show background check tracking for auditor-only members', () => { renderRow({ backgroundCheckStatus: 'invited', role: 'auditor' }); - expect(screen.queryByText(/Background check/)).not.toBeInTheDocument(); + expect(screen.queryByTestId('requirement-Background')).not.toBeInTheDocument(); }); }); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.test.tsx new file mode 100644 index 0000000000..bb3894f554 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.test.tsx @@ -0,0 +1,56 @@ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { TaskRequirements, type TaskRequirementItem } from './TaskRequirements'; + +const count = ( + label: string, + completed: number, + total: number, +): TaskRequirementItem => ({ label, completed, total, kind: 'count' }); + +const binary = (label: string, completed: number): TaskRequirementItem => ({ + label, + completed, + total: 1, + kind: 'binary', +}); + +describe('TaskRequirements', () => { + it('renders a dash when there are no items and nothing loading', () => { + render(); + expect(screen.getByText('—')).toBeInTheDocument(); + }); + + it('renders count requirements as "x/y"', () => { + render(); + const row = screen.getByTestId('requirement-Policies'); + expect(row).toHaveTextContent('Policies'); + expect(row).toHaveTextContent('24/33'); + }); + + it('renders a complete binary requirement as Done', () => { + render(); + expect(screen.getByTestId('requirement-Background')).toHaveTextContent('Done'); + }); + + it('renders an incomplete binary requirement as Missing', () => { + render(); + expect(screen.getByTestId('requirement-Device')).toHaveTextContent('Missing'); + }); + + it('renders each requirement on its own row', () => { + render( + , + ); + expect(screen.getByTestId('requirement-Policies')).toBeInTheDocument(); + expect(screen.getByTestId('requirement-Training')).toBeInTheDocument(); + expect(screen.getByTestId('requirement-Background')).toBeInTheDocument(); + }); + + it('shows a loading placeholder when showLoadingRow is set with no items', () => { + render(); + expect(screen.queryByText('—')).not.toBeInTheDocument(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx new file mode 100644 index 0000000000..41901408a0 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx @@ -0,0 +1,83 @@ +'use client'; + +import { Badge, Skeleton, Text } from '@trycompai/design-system'; + +export interface TaskRequirementItem { + label: string; + completed: number; + total: number; + /** 'count' renders "x/y" + a progress bar; 'binary' renders a Done/Missing badge. */ + kind: 'count' | 'binary'; +} + +function TaskRequirementRow({ item }: { item: TaskRequirementItem }) { + const { label, completed, total, kind } = item; + const isComplete = total > 0 && completed >= total; + + return ( +
+
+ + {label} + +
+ + {kind === 'binary' ? ( + + {isComplete ? 'Done' : 'Missing'} + + ) : ( + <> +
+ 0 ? 'warning' : 'muted'}> + {completed}/{total} + +
+
+
0 ? Math.min(100, Math.round((completed / total) * 100)) : 0}%`, + }} + /> +
+ + )} +
+ ); +} + +/** + * Per-employee requirement rollup shown in the People list TASKS column. + * Each requirement is on its own row: fractional requirements (policies, training) + * show a count + progress bar; binary requirements (HIPAA, device, background) + * show a Done/Missing badge. + */ +export function TaskRequirements({ + items, + showLoadingRow = false, +}: { + items: TaskRequirementItem[]; + showLoadingRow?: boolean; +}) { + if (items.length === 0 && !showLoadingRow) { + return ( + + — + + ); + } + + return ( +
+ {items.map((item) => ( + + ))} + {showLoadingRow && ( +
+ +
+ )} +
+ ); +} From d2ad593d7a8cee61e82e70ca2df17bbe3a44d5e1 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 1 Jul 2026 15:10:49 -0400 Subject: [PATCH 02/33] =?UTF-8?q?feat(people):=202FA=20source=20backend=20?= =?UTF-8?q?=E2=80=94=20schema,=20read=20path,=20endpoints?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Universal per-employee 2FA sourcing from any integration bound to the 2FA evidence task (taskMapping === TASK_TEMPLATES.twoFactorAuth), covering codebase + dynamic integrations via the shared registry. - Organization.twoFactorSource column + migration - CheckRunRepository.findLatestUserResultsByConnectionAndCheck: full per-user results from the latest real (non-inconclusive) run, org-scoped - TwoFactorSourceController: GET/POST two-factor-source, available-2fa-sources (registry filtered by 2FA taskMapping), two-factor-statuses (per-email enabled/missing from the latest run; absence resolves to Not provided client-side) - All endpoints @RequirePermission-gated (integration read/update); 22 tests Co-Authored-By: Claude Opus 4.8 (1M context) --- .../two-factor-source.controller.spec.ts | 235 ++++++++++++++++ .../two-factor-source.controller.ts | 250 ++++++++++++++++++ .../integration-platform.module.ts | 2 + .../repositories/check-run.repository.spec.ts | 69 +++++ .../repositories/check-run.repository.ts | 36 +++ .../migration.sql | 2 + packages/db/prisma/schema/organization.prisma | 4 + 7 files changed, 598 insertions(+) create mode 100644 apps/api/src/integration-platform/controllers/two-factor-source.controller.spec.ts create mode 100644 apps/api/src/integration-platform/controllers/two-factor-source.controller.ts create mode 100644 packages/db/prisma/migrations/20260701120000_add_two_factor_source/migration.sql diff --git a/apps/api/src/integration-platform/controllers/two-factor-source.controller.spec.ts b/apps/api/src/integration-platform/controllers/two-factor-source.controller.spec.ts new file mode 100644 index 0000000000..6a8ca8ad09 --- /dev/null +++ b/apps/api/src/integration-platform/controllers/two-factor-source.controller.spec.ts @@ -0,0 +1,235 @@ +import { HttpException } from '@nestjs/common'; + +jest.mock('@db', () => ({ + db: { + organization: { findUnique: jest.fn(), update: jest.fn() }, + integrationConnection: { findFirst: jest.fn() }, + }, +})); + +jest.mock('@trycompai/integration-platform', () => { + const actual = jest.requireActual< + typeof import('@trycompai/integration-platform') + >('@trycompai/integration-platform'); + return { + ...actual, + registry: { getActiveManifests: jest.fn() }, + }; +}); + +// Break the ESM better-auth import chain pulled in via HybridAuthGuard. +jest.mock('@trycompai/auth', () => ({ + statement: { integration: ['create', 'read', 'update', 'delete'] }, + BUILT_IN_ROLE_PERMISSIONS: {}, +})); +jest.mock('../../auth/auth.server', () => ({ + auth: { api: { getSession: jest.fn() } }, +})); + +import { db } from '@db'; +import { registry, TASK_TEMPLATES } from '@trycompai/integration-platform'; +import { TwoFactorSourceController } from './two-factor-source.controller'; + +const mockGetActiveManifests = ( + registry as unknown as { getActiveManifests: jest.Mock } +).getActiveManifests; +const mockOrgFindUnique = ( + db.organization as unknown as { findUnique: jest.Mock } +).findUnique; +const mockOrgUpdate = (db.organization as unknown as { update: jest.Mock }) + .update; +const mockConnFindFirst = ( + db.integrationConnection as unknown as { findFirst: jest.Mock } +).findFirst; + +// Build a manifest whose check is bound to the 2FA task. +function boundManifest(id: string, name = id) { + return { + id, + name, + logoUrl: null, + checks: [{ id: 'two-factor-auth', taskMapping: TASK_TEMPLATES.twoFactorAuth }], + }; +} +// A manifest with a check NOT bound to the 2FA task. +function unboundManifest(id: string) { + return { + id, + name: id, + logoUrl: null, + checks: [{ id: 'other', taskMapping: TASK_TEMPLATES.codeChanges }], + }; +} + +const mockConnRepo = { findBySlugAndOrg: jest.fn() }; +const mockCheckRunRepo = { findLatestUserResultsByConnectionAndCheck: jest.fn() }; + +function makeController() { + return new TwoFactorSourceController( + mockConnRepo as never, + mockCheckRunRepo as never, + ); +} + +const ORG = 'org_1'; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +describe('TwoFactorSourceController.getTwoFactorSource', () => { + it('returns the configured provider', async () => { + mockOrgFindUnique.mockResolvedValue({ twoFactorSource: 'google-workspace' }); + const result = await makeController().getTwoFactorSource(ORG); + expect(result).toEqual({ provider: 'google-workspace' }); + }); + + it('throws when the org does not exist', async () => { + mockOrgFindUnique.mockResolvedValue(null); + await expect(makeController().getTwoFactorSource(ORG)).rejects.toBeInstanceOf( + HttpException, + ); + }); +}); + +describe('TwoFactorSourceController.setTwoFactorSource', () => { + it('rejects a blank/whitespace provider with 400', async () => { + await expect( + makeController().setTwoFactorSource(ORG, { provider: ' ' }), + ).rejects.toBeInstanceOf(HttpException); + expect(mockOrgUpdate).not.toHaveBeenCalled(); + }); + + it('rejects a provider not bound to the 2FA task', async () => { + mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); + await expect( + makeController().setTwoFactorSource(ORG, { provider: 'slack' }), + ).rejects.toBeInstanceOf(HttpException); + expect(mockOrgUpdate).not.toHaveBeenCalled(); + }); + + it('rejects a bound provider that is not connected', async () => { + mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); + mockConnRepo.findBySlugAndOrg.mockResolvedValue(null); + await expect( + makeController().setTwoFactorSource(ORG, { provider: 'google-workspace' }), + ).rejects.toBeInstanceOf(HttpException); + expect(mockOrgUpdate).not.toHaveBeenCalled(); + }); + + it('sets a valid, connected, bound provider', async () => { + mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); + mockConnRepo.findBySlugAndOrg.mockResolvedValue({ + id: 'conn_1', + status: 'active', + }); + mockOrgUpdate.mockResolvedValue({}); + + const result = await makeController().setTwoFactorSource(ORG, { + provider: 'google-workspace', + }); + + expect(result).toEqual({ success: true, provider: 'google-workspace' }); + expect(mockOrgUpdate).toHaveBeenCalledWith({ + where: { id: ORG }, + data: { twoFactorSource: 'google-workspace' }, + }); + }); + + it('clears the source when provider is null', async () => { + mockOrgUpdate.mockResolvedValue({}); + const result = await makeController().setTwoFactorSource(ORG, { + provider: null, + }); + expect(result).toEqual({ success: true, provider: null }); + expect(mockOrgUpdate).toHaveBeenCalledWith({ + where: { id: ORG }, + data: { twoFactorSource: null }, + }); + }); +}); + +describe('TwoFactorSourceController.getAvailableTwoFactorSources', () => { + it('returns only manifests bound to the 2FA task, with connection state', async () => { + mockGetActiveManifests.mockReturnValue([ + boundManifest('google-workspace', 'Google Workspace'), + unboundManifest('slack'), + boundManifest('github', 'GitHub'), + ]); + mockConnFindFirst.mockImplementation( + (args: { where: { provider: { slug: string } } }) => + args.where.provider.slug === 'google-workspace' + ? Promise.resolve({ id: 'conn_1', lastSyncAt: null, nextSyncAt: null }) + : Promise.resolve(null), + ); + + const { providers } = await makeController().getAvailableTwoFactorSources(ORG); + + expect(providers.map((p) => p.slug)).toEqual(['google-workspace', 'github']); + expect(providers.find((p) => p.slug === 'google-workspace')?.connected).toBe( + true, + ); + expect(providers.find((p) => p.slug === 'github')?.connected).toBe(false); + }); +}); + +describe('TwoFactorSourceController.getTwoFactorStatuses', () => { + it('returns unconfigured when no source is set', async () => { + mockOrgFindUnique.mockResolvedValue({ twoFactorSource: null }); + const result = await makeController().getTwoFactorStatuses(ORG); + expect(result).toEqual({ configured: false, source: null, statuses: [] }); + }); + + it('maps latest-run results to lowercased email + enabled/missing', async () => { + mockOrgFindUnique.mockResolvedValue({ twoFactorSource: 'google-workspace' }); + mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); + mockConnRepo.findBySlugAndOrg.mockResolvedValue({ + id: 'conn_1', + status: 'active', + }); + mockCheckRunRepo.findLatestUserResultsByConnectionAndCheck.mockResolvedValue({ + run: { id: 'run_1' }, + results: [ + { resourceId: 'Alice@X.com', passed: true }, + { resourceId: 'bob@x.com', passed: false }, + ], + }); + + const result = await makeController().getTwoFactorStatuses(ORG); + + expect( + mockCheckRunRepo.findLatestUserResultsByConnectionAndCheck, + ).toHaveBeenCalledWith({ + connectionId: 'conn_1', + checkId: 'two-factor-auth', + organizationId: ORG, + }); + expect(result).toEqual({ + configured: true, + source: 'google-workspace', + statuses: [ + { email: 'alice@x.com', status: 'enabled' }, + { email: 'bob@x.com', status: 'missing' }, + ], + }); + }); + + it('returns empty statuses when the source has no real run', async () => { + mockOrgFindUnique.mockResolvedValue({ twoFactorSource: 'google-workspace' }); + mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); + mockConnRepo.findBySlugAndOrg.mockResolvedValue({ + id: 'conn_1', + status: 'active', + }); + mockCheckRunRepo.findLatestUserResultsByConnectionAndCheck.mockResolvedValue( + null, + ); + + const result = await makeController().getTwoFactorStatuses(ORG); + expect(result).toEqual({ + configured: true, + source: 'google-workspace', + statuses: [], + }); + }); +}); diff --git a/apps/api/src/integration-platform/controllers/two-factor-source.controller.ts b/apps/api/src/integration-platform/controllers/two-factor-source.controller.ts new file mode 100644 index 0000000000..4bc5cb3f15 --- /dev/null +++ b/apps/api/src/integration-platform/controllers/two-factor-source.controller.ts @@ -0,0 +1,250 @@ +import { + Controller, + Get, + Post, + Body, + HttpException, + HttpStatus, + Logger, + UseGuards, +} from '@nestjs/common'; +import { + ApiBody, + ApiOperation, + ApiPropertyOptional, + ApiSecurity, + ApiTags, +} from '@nestjs/swagger'; +import { IsOptional, IsString } from 'class-validator'; +import { db } from '@db'; +import { + registry, + TASK_TEMPLATES, + type IntegrationManifest, +} from '@trycompai/integration-platform'; +import { HybridAuthGuard } from '../../auth/hybrid-auth.guard'; +import { PermissionGuard } from '../../auth/permission.guard'; +import { RequirePermission } from '../../auth/require-permission.decorator'; +import { OrganizationId } from '../../auth/auth-context.decorator'; +import { ConnectionRepository } from '../repositories/connection.repository'; +import { CheckRunRepository } from '../repositories/check-run.repository'; + +/** The check on a manifest that feeds the 2FA evidence task, if any. */ +function twoFactorCheckOf(manifest: IntegrationManifest) { + return ( + manifest.checks?.find( + (check) => check.taskMapping === TASK_TEMPLATES.twoFactorAuth, + ) ?? null + ); +} + +/** + * Every active manifest — codebase OR dynamic — whose check is bound to the 2FA + * task. Dynamic manifests are merged into the same registry, so this is uniformly + * universal: any integration that ships a 2FA check becomes an eligible source + * with zero per-integration wiring. + */ +function manifestsWithTwoFactorCheck(): IntegrationManifest[] { + return registry.getActiveManifests().filter((m) => !!twoFactorCheckOf(m)); +} + +// Body for POST /v1/integrations/sync/two-factor-source. Pass a provider slug to +// set the org's 2FA source, or null/omit to clear it. Class (not inline type) so +// swagger + the ValidationPipe whitelist accept it. +class SetTwoFactorSourceDto { + // UI sends organizationId in the body; ignored by the handler (derived from auth). + @ApiPropertyOptional({ + description: + 'Auto-resolved from your API key / session. You can omit this; it is ignored by the server.', + }) + @IsOptional() + @IsString() + organizationId?: string; + + @ApiPropertyOptional({ + description: + 'Provider slug whose 2FA check feeds the People-tab 2FA column (must have a check bound to the 2FA task — call available-2fa-sources for options). Pass null or omit to clear the source.', + example: 'google-workspace', + nullable: true, + }) + @IsOptional() + @IsString() + provider?: string | null; +} + +@Controller({ path: 'integrations/sync', version: '1' }) +@ApiTags('Integrations') +@UseGuards(HybridAuthGuard, PermissionGuard) +@ApiSecurity('apikey') +export class TwoFactorSourceController { + private readonly logger = new Logger(TwoFactorSourceController.name); + + constructor( + private readonly connectionRepository: ConnectionRepository, + private readonly checkRunRepo: CheckRunRepository, + ) {} + + /** + * Get the currently configured 2FA source provider. + */ + @Get('two-factor-source') + @ApiOperation({ summary: 'Get the configured 2FA source provider' }) + @RequirePermission('integration', 'read') + async getTwoFactorSource(@OrganizationId() organizationId: string) { + const org = await db.organization.findUnique({ + where: { id: organizationId }, + select: { twoFactorSource: true }, + }); + if (!org) { + throw new HttpException('Organization not found', HttpStatus.NOT_FOUND); + } + return { provider: org.twoFactorSource }; + } + + /** + * Set (or clear) the org's 2FA source provider. + */ + @Post('two-factor-source') + @ApiOperation({ summary: 'Set the 2FA source provider' }) + @ApiBody({ type: SetTwoFactorSourceDto }) + @RequirePermission('integration', 'update') + async setTwoFactorSource( + @OrganizationId() organizationId: string, + @Body() body: SetTwoFactorSourceDto, + ) { + // Only an explicit string (set) or null (clear) are valid. Reject anything + // else — non-string, or blank/whitespace — with a 400 rather than silently + // coercing it to null and clearing the org's configured source. + const rawProvider = body.provider; + if ( + rawProvider !== null && + rawProvider !== undefined && + (typeof rawProvider !== 'string' || rawProvider.trim().length === 0) + ) { + throw new HttpException( + 'provider must be a non-empty string or null', + HttpStatus.BAD_REQUEST, + ); + } + const provider = rawProvider ? rawProvider.trim() : null; + + if (provider) { + const validProviders = manifestsWithTwoFactorCheck().map((m) => m.id); + if (!validProviders.includes(provider)) { + throw new HttpException( + `Invalid 2FA source. Must be one of: ${validProviders.join(', ')}`, + HttpStatus.BAD_REQUEST, + ); + } + + const connection = await this.connectionRepository.findBySlugAndOrg( + provider, + organizationId, + ); + if (!connection || connection.status !== 'active') { + throw new HttpException( + `Provider ${provider} is not connected`, + HttpStatus.BAD_REQUEST, + ); + } + } + + await db.organization.update({ + where: { id: organizationId }, + data: { twoFactorSource: provider }, + }); + + this.logger.log( + `Set 2FA source to ${provider ?? 'none'} for org ${organizationId}`, + ); + return { success: true, provider }; + } + + /** + * List integrations that can supply per-user 2FA status (all bound to the 2FA + * task), with each one's connection state for this org. + */ + @Get('available-2fa-sources') + @ApiOperation({ summary: 'List integrations that can supply per-user 2FA status' }) + @RequirePermission('integration', 'read') + async getAvailableTwoFactorSources(@OrganizationId() organizationId: string) { + const sources = manifestsWithTwoFactorCheck(); + const providers = await Promise.all( + sources.map(async (m) => { + const connection = await db.integrationConnection.findFirst({ + where: { organizationId, status: 'active', provider: { slug: m.id } }, + select: { id: true, lastSyncAt: true, nextSyncAt: true }, + }); + return { + slug: m.id, + name: m.name, + logoUrl: m.logoUrl, + connected: !!connection, + connectionId: connection?.id ?? null, + lastSyncAt: connection?.lastSyncAt?.toISOString() ?? null, + nextSyncAt: connection?.nextSyncAt?.toISOString() ?? null, + }; + }), + ); + return { providers }; + } + + /** + * Per-user 2FA status from the configured source's latest real check run. + * + * Returns only emails that had a result row; the People tab resolves any + * member without a matching row to "Not provided". Emails are lowercased to + * match the (lowercased) member emails on the join. + */ + @Get('two-factor-statuses') + @ApiOperation({ summary: 'Per-user 2FA status from the configured source' }) + @RequirePermission('integration', 'read') + async getTwoFactorStatuses(@OrganizationId() organizationId: string) { + const org = await db.organization.findUnique({ + where: { id: organizationId }, + select: { twoFactorSource: true }, + }); + const source = org?.twoFactorSource ?? null; + if (!source) { + return { configured: false, source: null, statuses: [] }; + } + + const manifest = manifestsWithTwoFactorCheck().find((m) => m.id === source); + const check = manifest ? twoFactorCheckOf(manifest) : null; + if (!check) { + // Source no longer offers a 2FA check (removed/disabled) — treat as unset. + return { configured: false, source, statuses: [] }; + } + + const connection = await this.connectionRepository.findBySlugAndOrg( + source, + organizationId, + ); + if (!connection) { + return { configured: true, source, statuses: [] }; + } + + const latest = await this.checkRunRepo.findLatestUserResultsByConnectionAndCheck({ + connectionId: connection.id, + checkId: check.id, + organizationId, + }); + if (!latest) { + return { configured: true, source, statuses: [] }; + } + + // Dedupe by lowercased email (results aren't ordered; last row wins). + const byEmail = new Map(); + for (const result of latest.results) { + const email = result.resourceId.toLowerCase().trim(); + if (!email) continue; + byEmail.set(email, result.passed ? 'enabled' : 'missing'); + } + const statuses = Array.from(byEmail, ([email, status]) => ({ + email, + status, + })); + + return { configured: true, source, statuses }; + } +} diff --git a/apps/api/src/integration-platform/integration-platform.module.ts b/apps/api/src/integration-platform/integration-platform.module.ts index aa9c2a6954..e532c9e3fc 100644 --- a/apps/api/src/integration-platform/integration-platform.module.ts +++ b/apps/api/src/integration-platform/integration-platform.module.ts @@ -14,6 +14,7 @@ import { VariablesController } from './controllers/variables.controller'; import { TaskIntegrationsController } from './controllers/task-integrations.controller'; import { WebhookController } from './controllers/webhook.controller'; import { SyncController } from './controllers/sync.controller'; +import { TwoFactorSourceController } from './controllers/two-factor-source.controller'; import { ServicesController } from './controllers/services.controller'; import { CredentialVaultService } from './services/credential-vault.service'; import { ConnectionService } from './services/connection.service'; @@ -54,6 +55,7 @@ import { GenericDeviceSyncService } from './services/generic-device-sync.service TaskIntegrationsController, WebhookController, SyncController, + TwoFactorSourceController, ServicesController, ], providers: [ diff --git a/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts b/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts index 821db6811d..14f5692219 100644 --- a/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts +++ b/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts @@ -3,9 +3,11 @@ jest.mock('@db', () => ({ integrationCheckRun: { groupBy: jest.fn(), findMany: jest.fn(), + findFirst: jest.fn(), }, integrationCheckResult: { count: jest.fn(), + findMany: jest.fn(), }, }, })); @@ -24,6 +26,12 @@ const mockFindMany = mockedCheckRun.findMany; const mockResultCount = ( db.integrationCheckResult as unknown as { count: jest.Mock } ).count; +const mockFindFirst = ( + db.integrationCheckRun as unknown as { findFirst: jest.Mock } +).findFirst; +const mockResultFindMany = ( + db.integrationCheckResult as unknown as { findMany: jest.Mock } +).findMany; function makeRun(opts: { id: string; @@ -286,3 +294,64 @@ describe('CheckRunRepository.countExceptedFailures', () => { }); }); }); + +describe('CheckRunRepository.findLatestUserResultsByConnectionAndCheck', () => { + const repo = new CheckRunRepository(); + const params = { + connectionId: 'conn_1', + checkId: 'two-factor-auth', + organizationId: 'org_1', + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('returns null and never loads results when there is no real run', async () => { + mockFindFirst.mockResolvedValue(null); + + const result = await repo.findLatestUserResultsByConnectionAndCheck(params); + + expect(result).toBeNull(); + expect(mockResultFindMany).not.toHaveBeenCalled(); + }); + + it('scopes the run to org + connection + check, excluding held runs and disconnected connections', async () => { + mockFindFirst.mockResolvedValue({ id: 'run_1' }); + mockResultFindMany.mockResolvedValue([]); + + await repo.findLatestUserResultsByConnectionAndCheck(params); + + expect(mockFindFirst).toHaveBeenCalledWith({ + where: { + connectionId: 'conn_1', + checkId: 'two-factor-auth', + // Held (inconclusive) runs must never surface to the customer. + status: { not: 'inconclusive' }, + connection: { + organizationId: 'org_1', + status: { not: 'disconnected' }, + }, + }, + orderBy: { createdAt: 'desc' }, + }); + }); + + it('loads the FULL per-user result set for the latest run — no take cap', async () => { + mockFindFirst.mockResolvedValue({ id: 'run_1' }); + const rows = [ + { id: 'icx_1', resourceId: 'a@x.com', passed: true }, + { id: 'icx_2', resourceId: 'b@x.com', passed: false }, + ]; + mockResultFindMany.mockResolvedValue(rows); + + const result = await repo.findLatestUserResultsByConnectionAndCheck(params); + + expect(mockResultFindMany).toHaveBeenCalledWith({ + where: { checkRunId: 'run_1', resourceType: 'user' }, + }); + // Every user must map to a member — the 30-row display cap is NOT reused. + expect(mockResultFindMany.mock.calls[0][0].take).toBeUndefined(); + expect(result).toEqual({ run: { id: 'run_1' }, results: rows }); + }); +}); diff --git a/apps/api/src/integration-platform/repositories/check-run.repository.ts b/apps/api/src/integration-platform/repositories/check-run.repository.ts index 38d07ac76b..4c9ac93e63 100644 --- a/apps/api/src/integration-platform/repositories/check-run.repository.ts +++ b/apps/api/src/integration-platform/repositories/check-run.repository.ts @@ -301,6 +301,42 @@ export class CheckRunRepository { }); } + /** + * Latest per-USER results for a (connection, check), scoped to an org. + * + * Used by the People-tab 2FA column: returns the full, untruncated set of + * `resourceType='user'` results from the connection's most recent REAL run + * (never `inconclusive`/held, never a disconnected connection). Unlike the + * task-run-history read, there is NO `take` cap — every user must map to a + * member, so the 30-row display sample ({@link DISPLAY_RESULTS_PER_RUN}) is + * deliberately not reused here. Returns null when the source has no real run. + */ + async findLatestUserResultsByConnectionAndCheck({ + connectionId, + checkId, + organizationId, + }: { + connectionId: string; + checkId: string; + organizationId: string; + }) { + const run = await db.integrationCheckRun.findFirst({ + where: { + connectionId, + checkId, + status: { not: 'inconclusive' }, + connection: { organizationId, status: { not: 'disconnected' } }, + }, + orderBy: { createdAt: 'desc' }, + }); + if (!run) return null; + + const results = await db.integrationCheckResult.findMany({ + where: { checkRunId: run.id, resourceType: 'user' }, + }); + return { run, results }; + } + /** * Get check runs for a connection */ diff --git a/packages/db/prisma/migrations/20260701120000_add_two_factor_source/migration.sql b/packages/db/prisma/migrations/20260701120000_add_two_factor_source/migration.sql new file mode 100644 index 0000000000..66e00d5518 --- /dev/null +++ b/packages/db/prisma/migrations/20260701120000_add_two_factor_source/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "public"."Organization" ADD COLUMN "twoFactorSource" TEXT; diff --git a/packages/db/prisma/schema/organization.prisma b/packages/db/prisma/schema/organization.prisma index 406dfef709..b1cf46f429 100644 --- a/packages/db/prisma/schema/organization.prisma +++ b/packages/db/prisma/schema/organization.prisma @@ -29,6 +29,10 @@ model Organization { // When set, the scheduled sync will import devices from this provider deviceSyncProvider String? + // 2FA source provider (e.g., 'google-workspace') + // When set, the People tab reads per-user 2FA status from this integration's latest check run + twoFactorSource String? + apiKeys ApiKey[] mcpOrgBindings McpOrgBinding[] auditLog AuditLog[] From aded01fe1b13d02d779d5b39c73bcff4436b9db6 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 1 Jul 2026 16:52:05 -0400 Subject: [PATCH 03/33] refactor(integrations): universal CheckResultsService for reusing check results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract a feature-agnostic, read-only service that returns ANY integration check's per-resource results in a stable envelope. Interpretation stays in the feature — the service never types/interprets evidence (raw JSON). The 2FA controller becomes consumer #1 and gets thinner (delegates all fetching). - CheckResultsService: listSourcesBoundToTask / getLatestResultsByCheck / getLatestResultsForTask; exported from IntegrationPlatformModule - Generalize repo method -> findLatestResultsByConnectionAndCheck (optional resourceType) - TwoFactorSourceController now depends only on the service (+ its own twoFactorSource column) - Agent docs: check-results-service skill + README + apps/api/CLAUDE.md pointer - 29 tests across service/controller/repo Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/skills/check-results-service/SKILL.md | 136 ++++++++++++ apps/api/CLAUDE.md | 4 + .../two-factor-source.controller.spec.ts | 154 +++++--------- .../two-factor-source.controller.ts | 118 ++++------- .../integration-platform.module.ts | 5 + .../repositories/check-run.repository.spec.ts | 28 ++- .../repositories/check-run.repository.ts | 25 ++- .../services/README-check-results.md | 39 ++++ .../services/check-results.service.spec.ts | 193 +++++++++++++++++ .../services/check-results.service.ts | 196 ++++++++++++++++++ 10 files changed, 705 insertions(+), 193 deletions(-) create mode 100644 .claude/skills/check-results-service/SKILL.md create mode 100644 apps/api/src/integration-platform/services/README-check-results.md create mode 100644 apps/api/src/integration-platform/services/check-results.service.spec.ts create mode 100644 apps/api/src/integration-platform/services/check-results.service.ts diff --git a/.claude/skills/check-results-service/SKILL.md b/.claude/skills/check-results-service/SKILL.md new file mode 100644 index 0000000000..f79904ebff --- /dev/null +++ b/.claude/skills/check-results-service/SKILL.md @@ -0,0 +1,136 @@ +--- +name: check-results-service +description: How to reuse ANY integration check's results in a feature via the universal CheckResultsService (apps/api integration-platform). Use whenever a feature needs data produced by an integration check — "show 2FA status on People", "surface AWS S3 findings in X", "reuse a check's results", "per-user/per-resource results from a connected integration", "which integrations feed task T". Read this BEFORE writing your own IntegrationCheckResult / CheckRunRepository query — don't hand-roll it. +--- + +# CheckResultsService — reuse any integration check's results + +## The one idea + +Integration checks (2FA, AWS S3 encryption, device posture, …) all write per-resource +results to one table (`IntegrationCheckResult`). **`CheckResultsService` is the single, +universal, read-only way to get those results into any feature.** It is deliberately +**feature-agnostic**: it fetches results and hands them back in a stable envelope, and it +does **not** know or care what the data means. + +> **Service = fetch the results (generic). Feature = interpret + map + present.** + +If you're about to query `IntegrationCheckResult` or `CheckRunRepository` directly from a +new feature — stop. Use this service instead. + +- Service: `apps/api/src/integration-platform/services/check-results.service.ts` +- Exported from `IntegrationPlatformModule` — inject it into any feature module. +- Reference consumer (copy this): the 2FA feature — `two-factor-source.controller.ts`. + +## When to use it + +Use it whenever a feature needs the output of an integration check: +- "Show each employee's 2FA status on the People tab" (the current consumer) +- "Surface which S3 buckets failed encryption inside feature X" +- "Show device compliance per user from the MDM check" +- "List which connected integrations can supply data for task T" + +## The API (3 methods) + +```ts +// 1. Discover sources: which connected integrations can feed a task, + connection state. +listSourcesBoundToTask(organizationId, taskTemplateId): Promise + +// 2. The primitive: full results of a check's latest REAL run for ONE connection. +getLatestResultsByCheck({ organizationId, connectionId, checkId, resourceType? }): Promise + +// 3. Convenience: results for a task-bound check from a chosen source (provider slug). +// Resolves task -> check and slug -> connection for you. +getLatestResultsForTask({ organizationId, taskTemplateId, sourceSlug, resourceType? }): Promise +``` + +Notes: +- "Latest REAL run" = newest run that isn't `inconclusive`/held and whose connection isn't + disconnected. Held runs (our-side self-heal) never leak to a feature. +- No row cap — you get the full result set (unlike the 30-row task-history display). +- `resourceType` filters rows (e.g. `'user'`, `'bucket'`). Omit to get all. +- Empty array = "no data" (source not bound/connected, or never really ran). Never throws + for "no results". + +## The envelope you get back + +```ts +interface CheckResultRow { + resourceId: string; // provider-native id: email (2FA), bucket ARN (S3), repo, … + resourceType: string; // 'user' | 'bucket' | … + passed: boolean; // did this resource pass the check + title: string; + description: string | null; + evidence: Prisma.JsonValue; // ← check-SPECIFIC payload. The service does NOT interpret it. + collectedAt: Date; + runId: string; + connectionId: string; +} +``` + +**The `evidence` rule (important):** the envelope is universal; the check-specific data lives +in `evidence` as raw JSON. The service never types or interprets it. **Your feature validates +`evidence` at its own edge with a zod schema** and reads only the fields it understands: + +```ts +const TwoFaEvidence = z.object({ isEnrolledIn2Sv: z.boolean() }); +const parsed = TwoFaEvidence.safeParse(row.evidence); +// use parsed.data.isEnrolledIn2Sv — never `as any` +``` + +For a simple pass/fail feature you often don't even need `evidence` — just use `row.passed` +and `row.resourceId` (that's all 2FA needs). + +## How to add a NEW consumer (step by step) + +Say a feature wants "which AWS S3 buckets failed encryption": + +1. Inject the service: add `CheckResultsService` to your feature's constructor (the module + already exports it; import `IntegrationPlatformModule` if your module doesn't already). +2. Get results with the primitive (S3 usually spans many AWS connections, so you may loop + connections from `listSourcesBoundToTask` or your own connection lookup): + ```ts + const rows = await this.checkResults.getLatestResultsByCheck({ + organizationId, + connectionId, + checkId: 'aws-s3-encryption', // the check's manifest id + resourceType: 'bucket', + }); + ``` + …or, if your feature lets the user PICK a source for a task (like 2FA does), use + `getLatestResultsForTask` with the task template id and the chosen `sourceSlug`. +3. Interpret in YOUR feature: map `resourceId`/`passed`, validate `evidence` with zod, shape + the response your UI needs. **Do not** add feature logic to the service. +4. Test with the service mocked (see `two-factor-source.controller.spec.ts`). + +## Worked example — the 2FA feature (reference implementation) + +The People-tab 2FA column is consumer #1. Study it end to end: + +- `two-factor-source.controller.ts` + - `available-2fa-sources` → `listSourcesBoundToTask(org, TASK_TEMPLATES.twoFactorAuth)` + - `two-factor-statuses` → `getLatestResultsForTask({ org, taskTemplateId: twoFactorAuth, + sourceSlug: org.twoFactorSource, resourceType: 'user' })`, then maps `resourceId`→email + (lowercased) and `passed`→`enabled`/`missing`. Emails with no row are resolved to + "Not provided" on the client, never a false "missing". +- The controller owns the 2FA-specific bits (the `Organization.twoFactorSource` column, the + enabled/missing interpretation). The generic fetch is all in the service. + +## Rules / do & don't + +- ✅ DO put every "read a check's results" path through this service. +- ✅ DO validate `evidence` with zod in your feature. No `as any`. +- ✅ DO treat an empty array as "no data / not provided" — never a failure. +- ❌ DON'T query `IntegrationCheckResult` / `CheckRunRepository` directly from a feature. +- ❌ DON'T add feature-specific interpretation (enabled/missing, domain mapping) to the + service — it must stay universal and read-only. +- ❌ DON'T assume a source produces per-`user` (or email-keyed) rows — that's per-check. If a + chosen source's check emits a different `resourceType` or a non-mappable `resourceId`, your + feature should degrade gracefully (e.g. "Not provided"), not crash. + +## Source selection is NOT part of this service + +Which integration an org uses for a purpose (e.g. `Organization.twoFactorSource`) is +feature-owned config, mirroring `employeeSyncProvider` / `deviceSyncProvider`. Add a column +per feature. (If a 3rd/4th feature needs it, consider generalizing selection then — not +before.) diff --git a/apps/api/CLAUDE.md b/apps/api/CLAUDE.md index 3bafe45ef3..f840a709b5 100644 --- a/apps/api/CLAUDE.md +++ b/apps/api/CLAUDE.md @@ -126,6 +126,10 @@ const module = await Test.createTestingModule({ - Use Prisma via `@trycompai/db` - Always scope queries by `organizationId` for multi-tenancy - Use transactions for operations that modify multiple records +- **Reusing an integration check's results in a feature?** Use `CheckResultsService` + (`integration-platform/services/check-results.service.ts`) — the universal, read-only way + to get any check's per-resource results. Do NOT query `IntegrationCheckResult` / + `CheckRunRepository` directly from a feature. See the `check-results-service` skill. ### Gotchas diff --git a/apps/api/src/integration-platform/controllers/two-factor-source.controller.spec.ts b/apps/api/src/integration-platform/controllers/two-factor-source.controller.spec.ts index 6a8ca8ad09..461aabee73 100644 --- a/apps/api/src/integration-platform/controllers/two-factor-source.controller.spec.ts +++ b/apps/api/src/integration-platform/controllers/two-factor-source.controller.spec.ts @@ -3,20 +3,9 @@ import { HttpException } from '@nestjs/common'; jest.mock('@db', () => ({ db: { organization: { findUnique: jest.fn(), update: jest.fn() }, - integrationConnection: { findFirst: jest.fn() }, }, })); -jest.mock('@trycompai/integration-platform', () => { - const actual = jest.requireActual< - typeof import('@trycompai/integration-platform') - >('@trycompai/integration-platform'); - return { - ...actual, - registry: { getActiveManifests: jest.fn() }, - }; -}); - // Break the ESM better-auth import chain pulled in via HybridAuthGuard. jest.mock('@trycompai/auth', () => ({ statement: { integration: ['create', 'read', 'update', 'delete'] }, @@ -27,50 +16,36 @@ jest.mock('../../auth/auth.server', () => ({ })); import { db } from '@db'; -import { registry, TASK_TEMPLATES } from '@trycompai/integration-platform'; import { TwoFactorSourceController } from './two-factor-source.controller'; -const mockGetActiveManifests = ( - registry as unknown as { getActiveManifests: jest.Mock } -).getActiveManifests; const mockOrgFindUnique = ( db.organization as unknown as { findUnique: jest.Mock } ).findUnique; const mockOrgUpdate = (db.organization as unknown as { update: jest.Mock }) .update; -const mockConnFindFirst = ( - db.integrationConnection as unknown as { findFirst: jest.Mock } -).findFirst; -// Build a manifest whose check is bound to the 2FA task. -function boundManifest(id: string, name = id) { - return { - id, - name, - logoUrl: null, - checks: [{ id: 'two-factor-auth', taskMapping: TASK_TEMPLATES.twoFactorAuth }], - }; +const mockCheckResults = { + listSourcesBoundToTask: jest.fn(), + getLatestResultsForTask: jest.fn(), +}; + +function makeController() { + return new TwoFactorSourceController(mockCheckResults as never); } -// A manifest with a check NOT bound to the 2FA task. -function unboundManifest(id: string) { + +function source(slug: string, connected: boolean, name = slug) { return { - id, - name: id, + slug, + name, logoUrl: null, - checks: [{ id: 'other', taskMapping: TASK_TEMPLATES.codeChanges }], + checkId: 'two-factor-auth', + connected, + connectionId: connected ? `conn_${slug}` : null, + lastSyncAt: null, + nextSyncAt: null, }; } -const mockConnRepo = { findBySlugAndOrg: jest.fn() }; -const mockCheckRunRepo = { findLatestUserResultsByConnectionAndCheck: jest.fn() }; - -function makeController() { - return new TwoFactorSourceController( - mockConnRepo as never, - mockCheckRunRepo as never, - ); -} - const ORG = 'org_1'; beforeEach(() => { @@ -80,8 +55,9 @@ beforeEach(() => { describe('TwoFactorSourceController.getTwoFactorSource', () => { it('returns the configured provider', async () => { mockOrgFindUnique.mockResolvedValue({ twoFactorSource: 'google-workspace' }); - const result = await makeController().getTwoFactorSource(ORG); - expect(result).toEqual({ provider: 'google-workspace' }); + expect(await makeController().getTwoFactorSource(ORG)).toEqual({ + provider: 'google-workspace', + }); }); it('throws when the org does not exist', async () => { @@ -101,7 +77,9 @@ describe('TwoFactorSourceController.setTwoFactorSource', () => { }); it('rejects a provider not bound to the 2FA task', async () => { - mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); + mockCheckResults.listSourcesBoundToTask.mockResolvedValue([ + source('google-workspace', true), + ]); await expect( makeController().setTwoFactorSource(ORG, { provider: 'slack' }), ).rejects.toBeInstanceOf(HttpException); @@ -109,8 +87,9 @@ describe('TwoFactorSourceController.setTwoFactorSource', () => { }); it('rejects a bound provider that is not connected', async () => { - mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); - mockConnRepo.findBySlugAndOrg.mockResolvedValue(null); + mockCheckResults.listSourcesBoundToTask.mockResolvedValue([ + source('google-workspace', false), + ]); await expect( makeController().setTwoFactorSource(ORG, { provider: 'google-workspace' }), ).rejects.toBeInstanceOf(HttpException); @@ -118,11 +97,9 @@ describe('TwoFactorSourceController.setTwoFactorSource', () => { }); it('sets a valid, connected, bound provider', async () => { - mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); - mockConnRepo.findBySlugAndOrg.mockResolvedValue({ - id: 'conn_1', - status: 'active', - }); + mockCheckResults.listSourcesBoundToTask.mockResolvedValue([ + source('google-workspace', true), + ]); mockOrgUpdate.mockResolvedValue({}); const result = await makeController().setTwoFactorSource(ORG, { @@ -150,60 +127,47 @@ describe('TwoFactorSourceController.setTwoFactorSource', () => { }); describe('TwoFactorSourceController.getAvailableTwoFactorSources', () => { - it('returns only manifests bound to the 2FA task, with connection state', async () => { - mockGetActiveManifests.mockReturnValue([ - boundManifest('google-workspace', 'Google Workspace'), - unboundManifest('slack'), - boundManifest('github', 'GitHub'), + it('returns bound sources with connection state (without the internal checkId)', async () => { + mockCheckResults.listSourcesBoundToTask.mockResolvedValue([ + source('google-workspace', true, 'Google Workspace'), + source('github', false, 'GitHub'), ]); - mockConnFindFirst.mockImplementation( - (args: { where: { provider: { slug: string } } }) => - args.where.provider.slug === 'google-workspace' - ? Promise.resolve({ id: 'conn_1', lastSyncAt: null, nextSyncAt: null }) - : Promise.resolve(null), - ); const { providers } = await makeController().getAvailableTwoFactorSources(ORG); expect(providers.map((p) => p.slug)).toEqual(['google-workspace', 'github']); - expect(providers.find((p) => p.slug === 'google-workspace')?.connected).toBe( - true, - ); - expect(providers.find((p) => p.slug === 'github')?.connected).toBe(false); + expect(providers[0]).not.toHaveProperty('checkId'); + expect(providers[0].connected).toBe(true); }); }); describe('TwoFactorSourceController.getTwoFactorStatuses', () => { it('returns unconfigured when no source is set', async () => { mockOrgFindUnique.mockResolvedValue({ twoFactorSource: null }); - const result = await makeController().getTwoFactorStatuses(ORG); - expect(result).toEqual({ configured: false, source: null, statuses: [] }); + expect(await makeController().getTwoFactorStatuses(ORG)).toEqual({ + configured: false, + source: null, + statuses: [], + }); + expect(mockCheckResults.getLatestResultsForTask).not.toHaveBeenCalled(); }); - it('maps latest-run results to lowercased email + enabled/missing', async () => { + it('maps the service results to lowercased email + enabled/missing', async () => { mockOrgFindUnique.mockResolvedValue({ twoFactorSource: 'google-workspace' }); - mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); - mockConnRepo.findBySlugAndOrg.mockResolvedValue({ - id: 'conn_1', - status: 'active', - }); - mockCheckRunRepo.findLatestUserResultsByConnectionAndCheck.mockResolvedValue({ - run: { id: 'run_1' }, - results: [ - { resourceId: 'Alice@X.com', passed: true }, - { resourceId: 'bob@x.com', passed: false }, - ], - }); + mockCheckResults.getLatestResultsForTask.mockResolvedValue([ + { resourceId: 'Alice@X.com', passed: true }, + { resourceId: 'bob@x.com', passed: false }, + ]); const result = await makeController().getTwoFactorStatuses(ORG); - expect( - mockCheckRunRepo.findLatestUserResultsByConnectionAndCheck, - ).toHaveBeenCalledWith({ - connectionId: 'conn_1', - checkId: 'two-factor-auth', - organizationId: ORG, - }); + expect(mockCheckResults.getLatestResultsForTask).toHaveBeenCalledWith( + expect.objectContaining({ + organizationId: ORG, + sourceSlug: 'google-workspace', + resourceType: 'user', + }), + ); expect(result).toEqual({ configured: true, source: 'google-workspace', @@ -214,19 +178,11 @@ describe('TwoFactorSourceController.getTwoFactorStatuses', () => { }); }); - it('returns empty statuses when the source has no real run', async () => { + it('returns empty statuses when the source has no results', async () => { mockOrgFindUnique.mockResolvedValue({ twoFactorSource: 'google-workspace' }); - mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); - mockConnRepo.findBySlugAndOrg.mockResolvedValue({ - id: 'conn_1', - status: 'active', - }); - mockCheckRunRepo.findLatestUserResultsByConnectionAndCheck.mockResolvedValue( - null, - ); + mockCheckResults.getLatestResultsForTask.mockResolvedValue([]); - const result = await makeController().getTwoFactorStatuses(ORG); - expect(result).toEqual({ + expect(await makeController().getTwoFactorStatuses(ORG)).toEqual({ configured: true, source: 'google-workspace', statuses: [], diff --git a/apps/api/src/integration-platform/controllers/two-factor-source.controller.ts b/apps/api/src/integration-platform/controllers/two-factor-source.controller.ts index 4bc5cb3f15..ed50946523 100644 --- a/apps/api/src/integration-platform/controllers/two-factor-source.controller.ts +++ b/apps/api/src/integration-platform/controllers/two-factor-source.controller.ts @@ -17,36 +17,12 @@ import { } from '@nestjs/swagger'; import { IsOptional, IsString } from 'class-validator'; import { db } from '@db'; -import { - registry, - TASK_TEMPLATES, - type IntegrationManifest, -} from '@trycompai/integration-platform'; +import { TASK_TEMPLATES } from '@trycompai/integration-platform'; import { HybridAuthGuard } from '../../auth/hybrid-auth.guard'; import { PermissionGuard } from '../../auth/permission.guard'; import { RequirePermission } from '../../auth/require-permission.decorator'; import { OrganizationId } from '../../auth/auth-context.decorator'; -import { ConnectionRepository } from '../repositories/connection.repository'; -import { CheckRunRepository } from '../repositories/check-run.repository'; - -/** The check on a manifest that feeds the 2FA evidence task, if any. */ -function twoFactorCheckOf(manifest: IntegrationManifest) { - return ( - manifest.checks?.find( - (check) => check.taskMapping === TASK_TEMPLATES.twoFactorAuth, - ) ?? null - ); -} - -/** - * Every active manifest — codebase OR dynamic — whose check is bound to the 2FA - * task. Dynamic manifests are merged into the same registry, so this is uniformly - * universal: any integration that ships a 2FA check becomes an eligible source - * with zero per-integration wiring. - */ -function manifestsWithTwoFactorCheck(): IntegrationManifest[] { - return registry.getActiveManifests().filter((m) => !!twoFactorCheckOf(m)); -} +import { CheckResultsService } from '../services/check-results.service'; // Body for POST /v1/integrations/sync/two-factor-source. Pass a provider slug to // set the org's 2FA source, or null/omit to clear it. Class (not inline type) so @@ -63,7 +39,7 @@ class SetTwoFactorSourceDto { @ApiPropertyOptional({ description: - 'Provider slug whose 2FA check feeds the People-tab 2FA column (must have a check bound to the 2FA task — call available-2fa-sources for options). Pass null or omit to clear the source.', + 'Provider slug whose 2FA check feeds the People-tab 2FA column (must be bound to the 2FA task — call available-2fa-sources for options). Pass null or omit to clear the source.', example: 'google-workspace', nullable: true, }) @@ -72,6 +48,14 @@ class SetTwoFactorSourceDto { provider?: string | null; } +/** + * 2FA source configuration + per-employee 2FA status for the People tab. + * + * This controller owns only the 2FA-SPECIFIC concerns: the org's chosen source + * (`Organization.twoFactorSource`) and the enabled/missing interpretation. All + * the generic "read an integration check's results" work is delegated to + * {@link CheckResultsService} — it is consumer #1 of that universal service. + */ @Controller({ path: 'integrations/sync', version: '1' }) @ApiTags('Integrations') @UseGuards(HybridAuthGuard, PermissionGuard) @@ -79,10 +63,7 @@ class SetTwoFactorSourceDto { export class TwoFactorSourceController { private readonly logger = new Logger(TwoFactorSourceController.name); - constructor( - private readonly connectionRepository: ConnectionRepository, - private readonly checkRunRepo: CheckRunRepository, - ) {} + constructor(private readonly checkResults: CheckResultsService) {} /** * Get the currently configured 2FA source provider. @@ -129,19 +110,18 @@ export class TwoFactorSourceController { const provider = rawProvider ? rawProvider.trim() : null; if (provider) { - const validProviders = manifestsWithTwoFactorCheck().map((m) => m.id); - if (!validProviders.includes(provider)) { + const sources = await this.checkResults.listSourcesBoundToTask( + organizationId, + TASK_TEMPLATES.twoFactorAuth, + ); + const source = sources.find((s) => s.slug === provider); + if (!source) { throw new HttpException( - `Invalid 2FA source. Must be one of: ${validProviders.join(', ')}`, + `Invalid 2FA source. Must be one of: ${sources.map((s) => s.slug).join(', ')}`, HttpStatus.BAD_REQUEST, ); } - - const connection = await this.connectionRepository.findBySlugAndOrg( - provider, - organizationId, - ); - if (!connection || connection.status !== 'active') { + if (!source.connected) { throw new HttpException( `Provider ${provider} is not connected`, HttpStatus.BAD_REQUEST, @@ -168,24 +148,20 @@ export class TwoFactorSourceController { @ApiOperation({ summary: 'List integrations that can supply per-user 2FA status' }) @RequirePermission('integration', 'read') async getAvailableTwoFactorSources(@OrganizationId() organizationId: string) { - const sources = manifestsWithTwoFactorCheck(); - const providers = await Promise.all( - sources.map(async (m) => { - const connection = await db.integrationConnection.findFirst({ - where: { organizationId, status: 'active', provider: { slug: m.id } }, - select: { id: true, lastSyncAt: true, nextSyncAt: true }, - }); - return { - slug: m.id, - name: m.name, - logoUrl: m.logoUrl, - connected: !!connection, - connectionId: connection?.id ?? null, - lastSyncAt: connection?.lastSyncAt?.toISOString() ?? null, - nextSyncAt: connection?.nextSyncAt?.toISOString() ?? null, - }; - }), + const sources = await this.checkResults.listSourcesBoundToTask( + organizationId, + TASK_TEMPLATES.twoFactorAuth, ); + // Expose only what the selector needs (drop the internal checkId). + const providers = sources.map((s) => ({ + slug: s.slug, + name: s.name, + logoUrl: s.logoUrl, + connected: s.connected, + connectionId: s.connectionId, + lastSyncAt: s.lastSyncAt, + nextSyncAt: s.nextSyncAt, + })); return { providers }; } @@ -209,33 +185,17 @@ export class TwoFactorSourceController { return { configured: false, source: null, statuses: [] }; } - const manifest = manifestsWithTwoFactorCheck().find((m) => m.id === source); - const check = manifest ? twoFactorCheckOf(manifest) : null; - if (!check) { - // Source no longer offers a 2FA check (removed/disabled) — treat as unset. - return { configured: false, source, statuses: [] }; - } - - const connection = await this.connectionRepository.findBySlugAndOrg( - source, - organizationId, - ); - if (!connection) { - return { configured: true, source, statuses: [] }; - } - - const latest = await this.checkRunRepo.findLatestUserResultsByConnectionAndCheck({ - connectionId: connection.id, - checkId: check.id, + // Delegate the generic fetch; interpret 'user' rows here (2FA's concern). + const results = await this.checkResults.getLatestResultsForTask({ organizationId, + taskTemplateId: TASK_TEMPLATES.twoFactorAuth, + sourceSlug: source, + resourceType: 'user', }); - if (!latest) { - return { configured: true, source, statuses: [] }; - } // Dedupe by lowercased email (results aren't ordered; last row wins). const byEmail = new Map(); - for (const result of latest.results) { + for (const result of results) { const email = result.resourceId.toLowerCase().trim(); if (!email) continue; byEmail.set(email, result.passed ? 'enabled' : 'missing'); diff --git a/apps/api/src/integration-platform/integration-platform.module.ts b/apps/api/src/integration-platform/integration-platform.module.ts index e532c9e3fc..344a45628b 100644 --- a/apps/api/src/integration-platform/integration-platform.module.ts +++ b/apps/api/src/integration-platform/integration-platform.module.ts @@ -38,6 +38,7 @@ import { DynamicCheckRepository } from './repositories/dynamic-check.repository' import { IntegrationSyncLoggerService } from './services/integration-sync-logger.service'; import { GenericEmployeeSyncService } from './services/generic-employee-sync.service'; import { GenericDeviceSyncService } from './services/generic-device-sync.service'; +import { CheckResultsService } from './services/check-results.service'; @Module({ imports: [AuthModule, forwardRef(() => CloudSecurityModule)], @@ -73,6 +74,7 @@ import { GenericDeviceSyncService } from './services/generic-device-sync.service IntegrationSyncLoggerService, GenericEmployeeSyncService, GenericDeviceSyncService, + CheckResultsService, // Repositories ProviderRepository, ConnectionRepository, @@ -90,6 +92,9 @@ import { GenericDeviceSyncService } from './services/generic-device-sync.service OAuthCredentialsService, AutoCheckRunnerService, DynamicManifestLoaderService, + // Universal, feature-agnostic access to integration check results. Any + // feature module that needs to reuse check output injects this. + CheckResultsService, ], }) export class IntegrationPlatformModule {} diff --git a/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts b/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts index 14f5692219..ee4fbba814 100644 --- a/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts +++ b/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts @@ -295,12 +295,13 @@ describe('CheckRunRepository.countExceptedFailures', () => { }); }); -describe('CheckRunRepository.findLatestUserResultsByConnectionAndCheck', () => { +describe('CheckRunRepository.findLatestResultsByConnectionAndCheck', () => { const repo = new CheckRunRepository(); const params = { connectionId: 'conn_1', checkId: 'two-factor-auth', organizationId: 'org_1', + resourceType: 'user', }; beforeEach(() => { @@ -310,7 +311,7 @@ describe('CheckRunRepository.findLatestUserResultsByConnectionAndCheck', () => { it('returns null and never loads results when there is no real run', async () => { mockFindFirst.mockResolvedValue(null); - const result = await repo.findLatestUserResultsByConnectionAndCheck(params); + const result = await repo.findLatestResultsByConnectionAndCheck(params); expect(result).toBeNull(); expect(mockResultFindMany).not.toHaveBeenCalled(); @@ -320,7 +321,7 @@ describe('CheckRunRepository.findLatestUserResultsByConnectionAndCheck', () => { mockFindFirst.mockResolvedValue({ id: 'run_1' }); mockResultFindMany.mockResolvedValue([]); - await repo.findLatestUserResultsByConnectionAndCheck(params); + await repo.findLatestResultsByConnectionAndCheck(params); expect(mockFindFirst).toHaveBeenCalledWith({ where: { @@ -337,7 +338,7 @@ describe('CheckRunRepository.findLatestUserResultsByConnectionAndCheck', () => { }); }); - it('loads the FULL per-user result set for the latest run — no take cap', async () => { + it('loads the FULL result set for the latest run — no take cap — filtered by resourceType', async () => { mockFindFirst.mockResolvedValue({ id: 'run_1' }); const rows = [ { id: 'icx_1', resourceId: 'a@x.com', passed: true }, @@ -345,13 +346,28 @@ describe('CheckRunRepository.findLatestUserResultsByConnectionAndCheck', () => { ]; mockResultFindMany.mockResolvedValue(rows); - const result = await repo.findLatestUserResultsByConnectionAndCheck(params); + const result = await repo.findLatestResultsByConnectionAndCheck(params); expect(mockResultFindMany).toHaveBeenCalledWith({ where: { checkRunId: 'run_1', resourceType: 'user' }, }); - // Every user must map to a member — the 30-row display cap is NOT reused. + // Every resource must map to a domain entity — the 30-row display cap is NOT reused. expect(mockResultFindMany.mock.calls[0][0].take).toBeUndefined(); expect(result).toEqual({ run: { id: 'run_1' }, results: rows }); }); + + it('omits the resourceType filter when not provided (loads all rows)', async () => { + mockFindFirst.mockResolvedValue({ id: 'run_1' }); + mockResultFindMany.mockResolvedValue([]); + + await repo.findLatestResultsByConnectionAndCheck({ + connectionId: 'conn_1', + checkId: 'aws-s3-encryption', + organizationId: 'org_1', + }); + + expect(mockResultFindMany).toHaveBeenCalledWith({ + where: { checkRunId: 'run_1' }, + }); + }); }); diff --git a/apps/api/src/integration-platform/repositories/check-run.repository.ts b/apps/api/src/integration-platform/repositories/check-run.repository.ts index 4c9ac93e63..8d360adbe6 100644 --- a/apps/api/src/integration-platform/repositories/check-run.repository.ts +++ b/apps/api/src/integration-platform/repositories/check-run.repository.ts @@ -302,23 +302,27 @@ export class CheckRunRepository { } /** - * Latest per-USER results for a (connection, check), scoped to an org. + * Full results of a (connection, check)'s most recent REAL run, scoped to an + * org. "Real" = never `inconclusive`/held, never a disconnected connection. * - * Used by the People-tab 2FA column: returns the full, untruncated set of - * `resourceType='user'` results from the connection's most recent REAL run - * (never `inconclusive`/held, never a disconnected connection). Unlike the - * task-run-history read, there is NO `take` cap — every user must map to a - * member, so the 30-row display sample ({@link DISPLAY_RESULTS_PER_RUN}) is - * deliberately not reused here. Returns null when the source has no real run. + * This is the generic read behind {@link CheckResultsService} — any feature + * reusing check results goes through that service, not this method directly. + * Unlike the task-run-history read there is NO `take` cap: a consumer that + * maps every resource (e.g. one member per user row) needs the full set, so + * the 30-row display sample ({@link DISPLAY_RESULTS_PER_RUN}) is deliberately + * not reused here. Pass `resourceType` to load only rows of that kind (e.g. + * `'user'`); omit it to load all. Returns null when there is no real run. */ - async findLatestUserResultsByConnectionAndCheck({ + async findLatestResultsByConnectionAndCheck({ connectionId, checkId, organizationId, + resourceType, }: { connectionId: string; checkId: string; organizationId: string; + resourceType?: string; }) { const run = await db.integrationCheckRun.findFirst({ where: { @@ -332,7 +336,10 @@ export class CheckRunRepository { if (!run) return null; const results = await db.integrationCheckResult.findMany({ - where: { checkRunId: run.id, resourceType: 'user' }, + where: { + checkRunId: run.id, + ...(resourceType ? { resourceType } : {}), + }, }); return { run, results }; } diff --git a/apps/api/src/integration-platform/services/README-check-results.md b/apps/api/src/integration-platform/services/README-check-results.md new file mode 100644 index 0000000000..fe0c05d5ff --- /dev/null +++ b/apps/api/src/integration-platform/services/README-check-results.md @@ -0,0 +1,39 @@ +# CheckResultsService — reuse any integration check's results + +`CheckResultsService` is the **single, universal, read-only** way for any feature to consume +the output of an integration check (2FA, AWS S3, device posture, …). It fetches per-resource +results and returns them in a stable, feature-agnostic envelope. It does **not** interpret +the data — that's the feature's job. + +> Service = fetch results (generic). Feature = interpret + map + present. + +**Full usage map, examples, and rules:** see the `check-results-service` skill +(`.claude/skills/check-results-service/SKILL.md`). + +## 30-second version + +```ts +// inject CheckResultsService (exported by IntegrationPlatformModule), then: + +// which connected integrations can feed a task, + connection state +await checkResults.listSourcesBoundToTask(orgId, TASK_TEMPLATES.twoFactorAuth); + +// full results of a check's latest real run for one connection +await checkResults.getLatestResultsByCheck({ organizationId, connectionId, checkId, resourceType }); + +// results for a task-bound check from a chosen source (resolves task->check, slug->connection) +await checkResults.getLatestResultsForTask({ organizationId, taskTemplateId, sourceSlug, resourceType }); +``` + +Each row is `{ resourceId, resourceType, passed, title, description, evidence, collectedAt, +runId, connectionId }`. The check-specific payload is `evidence` (raw JSON) — **validate it +with zod in your feature**; the service never types it. Empty array = "no data" (never throws). + +## Don't + +- Don't query `IntegrationCheckResult` / `CheckRunRepository` directly from a feature — use this service. +- Don't add feature-specific interpretation to the service — keep it universal and read-only. + +## Reference consumer + +The People-tab 2FA column: `../controllers/two-factor-source.controller.ts`. diff --git a/apps/api/src/integration-platform/services/check-results.service.spec.ts b/apps/api/src/integration-platform/services/check-results.service.spec.ts new file mode 100644 index 0000000000..3fbdc4fc2b --- /dev/null +++ b/apps/api/src/integration-platform/services/check-results.service.spec.ts @@ -0,0 +1,193 @@ +jest.mock('@db', () => ({ + db: { integrationConnection: { findFirst: jest.fn() } }, +})); + +jest.mock('@trycompai/integration-platform', () => { + const actual = jest.requireActual< + typeof import('@trycompai/integration-platform') + >('@trycompai/integration-platform'); + return { + ...actual, + registry: { getActiveManifests: jest.fn() }, + }; +}); + +import { db } from '@db'; +import { registry, TASK_TEMPLATES } from '@trycompai/integration-platform'; +import { CheckResultsService } from './check-results.service'; + +const mockGetActiveManifests = ( + registry as unknown as { getActiveManifests: jest.Mock } +).getActiveManifests; +const mockConnFindFirst = ( + db.integrationConnection as unknown as { findFirst: jest.Mock } +).findFirst; + +const mockCheckRunRepo = { findLatestResultsByConnectionAndCheck: jest.fn() }; +const mockConnRepo = { findBySlugAndOrg: jest.fn() }; + +function makeService() { + return new CheckResultsService( + mockCheckRunRepo as never, + mockConnRepo as never, + ); +} + +function boundManifest(id: string, name = id) { + return { + id, + name, + logoUrl: null, + checks: [{ id: 'two-factor-auth', taskMapping: TASK_TEMPLATES.twoFactorAuth }], + }; +} +function unboundManifest(id: string) { + return { + id, + name: id, + logoUrl: null, + checks: [{ id: 'other', taskMapping: TASK_TEMPLATES.codeChanges }], + }; +} + +const ORG = 'org_1'; +const TWO_FA = TASK_TEMPLATES.twoFactorAuth; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +describe('CheckResultsService.listSourcesBoundToTask', () => { + it('returns only manifests bound to the task, with connection state + checkId', async () => { + mockGetActiveManifests.mockReturnValue([ + boundManifest('google-workspace', 'Google Workspace'), + unboundManifest('slack'), + boundManifest('github', 'GitHub'), + ]); + mockConnFindFirst.mockImplementation( + (args: { where: { provider: { slug: string } } }) => + args.where.provider.slug === 'google-workspace' + ? Promise.resolve({ id: 'c1', lastSyncAt: null, nextSyncAt: null }) + : Promise.resolve(null), + ); + + const sources = await makeService().listSourcesBoundToTask(ORG, TWO_FA); + + expect(sources.map((s) => s.slug)).toEqual(['google-workspace', 'github']); + const gws = sources.find((s) => s.slug === 'google-workspace'); + expect(gws).toMatchObject({ + connected: true, + connectionId: 'c1', + checkId: 'two-factor-auth', + }); + expect(sources.find((s) => s.slug === 'github')?.connected).toBe(false); + }); +}); + +describe('CheckResultsService.getLatestResultsByCheck', () => { + it('maps repo rows into the generic envelope (evidence passed through untouched)', async () => { + const collectedAt = new Date('2026-01-01T00:00:00Z'); + mockCheckRunRepo.findLatestResultsByConnectionAndCheck.mockResolvedValue({ + run: { id: 'run_1' }, + results: [ + { + resourceId: 'a@x.com', + resourceType: 'user', + passed: true, + title: 'Has 2FA', + description: null, + evidence: { isEnrolledIn2Sv: true }, + collectedAt, + }, + ], + }); + + const rows = await makeService().getLatestResultsByCheck({ + organizationId: ORG, + connectionId: 'conn_1', + checkId: 'two-factor-auth', + resourceType: 'user', + }); + + expect(rows).toEqual([ + { + resourceId: 'a@x.com', + resourceType: 'user', + passed: true, + title: 'Has 2FA', + description: null, + evidence: { isEnrolledIn2Sv: true }, + collectedAt, + runId: 'run_1', + connectionId: 'conn_1', + }, + ]); + }); + + it('returns [] when there is no real run', async () => { + mockCheckRunRepo.findLatestResultsByConnectionAndCheck.mockResolvedValue( + null, + ); + const rows = await makeService().getLatestResultsByCheck({ + organizationId: ORG, + connectionId: 'conn_1', + checkId: 'two-factor-auth', + }); + expect(rows).toEqual([]); + }); +}); + +describe('CheckResultsService.getLatestResultsForTask', () => { + it('resolves task->check and slug->connection, then fetches results', async () => { + mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); + mockConnRepo.findBySlugAndOrg.mockResolvedValue({ + id: 'conn_1', + status: 'active', + }); + mockCheckRunRepo.findLatestResultsByConnectionAndCheck.mockResolvedValue({ + run: { id: 'run_1' }, + results: [], + }); + + await makeService().getLatestResultsForTask({ + organizationId: ORG, + taskTemplateId: TWO_FA, + sourceSlug: 'google-workspace', + resourceType: 'user', + }); + + expect( + mockCheckRunRepo.findLatestResultsByConnectionAndCheck, + ).toHaveBeenCalledWith({ + connectionId: 'conn_1', + checkId: 'two-factor-auth', + organizationId: ORG, + resourceType: 'user', + }); + }); + + it('returns [] when the source is not bound to the task', async () => { + mockGetActiveManifests.mockReturnValue([]); + const rows = await makeService().getLatestResultsForTask({ + organizationId: ORG, + taskTemplateId: TWO_FA, + sourceSlug: 'google-workspace', + }); + expect(rows).toEqual([]); + expect(mockConnRepo.findBySlugAndOrg).not.toHaveBeenCalled(); + }); + + it('returns [] when the source has no connection', async () => { + mockGetActiveManifests.mockReturnValue([boundManifest('google-workspace')]); + mockConnRepo.findBySlugAndOrg.mockResolvedValue(null); + const rows = await makeService().getLatestResultsForTask({ + organizationId: ORG, + taskTemplateId: TWO_FA, + sourceSlug: 'google-workspace', + }); + expect(rows).toEqual([]); + expect( + mockCheckRunRepo.findLatestResultsByConnectionAndCheck, + ).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/api/src/integration-platform/services/check-results.service.ts b/apps/api/src/integration-platform/services/check-results.service.ts new file mode 100644 index 0000000000..c24be73543 --- /dev/null +++ b/apps/api/src/integration-platform/services/check-results.service.ts @@ -0,0 +1,196 @@ +import { Injectable } from '@nestjs/common'; +import { db } from '@db'; +import type { Prisma } from '@db'; +import { + registry, + type IntegrationManifest, + type TaskTemplateId, +} from '@trycompai/integration-platform'; +import { CheckRunRepository } from '../repositories/check-run.repository'; +import { ConnectionRepository } from '../repositories/connection.repository'; + +/** + * One row of an integration check's output, in a stable, FEATURE-AGNOSTIC shape. + * + * The envelope (resourceId/passed/etc.) is universal across every check. The + * check-SPECIFIC payload lives in `evidence` (raw JSON) — this service does NOT + * interpret it. Each consuming feature validates `evidence` at its own edge + * (e.g. with a zod schema) and reads only the fields it understands. + */ +export interface CheckResultRow { + /** Provider-native identifier for the resource (email, bucket ARN, repo, …). */ + resourceId: string; + /** Kind of resource the check produced (e.g. 'user', 'bucket'). */ + resourceType: string; + /** Whether this resource passed the check. */ + passed: boolean; + title: string; + description: string | null; + /** Check-specific payload — interpret this in the feature, never here. */ + evidence: Prisma.JsonValue; + collectedAt: Date; + runId: string; + connectionId: string; +} + +/** A connected integration that can supply results for a given task. */ +export interface CheckSourceInfo { + slug: string; + name: string; + logoUrl: string | null; + /** The check on this source bound to the requested task. */ + checkId: string; + connected: boolean; + connectionId: string | null; + lastSyncAt: string | null; + nextSyncAt: string | null; +} + +/** + * Universal, read-only access to integration CHECK RESULTS. + * + * The single reuse point for any feature that wants to consume the output of an + * integration check — 2FA today, background checks / device posture / S3 config + * tomorrow. It is deliberately feature-agnostic: it fetches results and returns + * them in a stable envelope, and it does NOT know or care what the data means. + * Interpretation, domain mapping, and presentation are the feature's job. + * + * See `apps/api/src/integration-platform/services/README-check-results.md` + * (and the `check-results-service` skill) for the usage map + examples. + */ +@Injectable() +export class CheckResultsService { + constructor( + private readonly checkRunRepo: CheckRunRepository, + private readonly connectionRepository: ConnectionRepository, + ) {} + + /** The check on a manifest bound to a task template, if any. */ + private checkBoundToTask( + manifest: IntegrationManifest, + taskTemplateId: TaskTemplateId, + ) { + return ( + manifest.checks?.find((check) => check.taskMapping === taskTemplateId) ?? + null + ); + } + + /** + * Every active manifest — codebase OR dynamic — whose check is bound to a task + * template. Dynamic manifests are merged into the same registry, so this is + * uniformly universal. + */ + private manifestsBoundToTask( + taskTemplateId: TaskTemplateId, + ): IntegrationManifest[] { + return registry + .getActiveManifests() + .filter((m) => !!this.checkBoundToTask(m, taskTemplateId)); + } + + /** + * List the integrations that can supply results for a task in this org, with + * each one's connection state. Use this to build a "source" selector or to + * discover what's available. Returns ALL bound integrations (connected or not) + * so the caller can decide how to present unconnected ones. + */ + async listSourcesBoundToTask( + organizationId: string, + taskTemplateId: TaskTemplateId, + ): Promise { + const manifests = this.manifestsBoundToTask(taskTemplateId); + return Promise.all( + manifests.map(async (m) => { + const check = this.checkBoundToTask(m, taskTemplateId); + const connection = await db.integrationConnection.findFirst({ + where: { organizationId, status: 'active', provider: { slug: m.id } }, + select: { id: true, lastSyncAt: true, nextSyncAt: true }, + }); + return { + slug: m.id, + name: m.name, + logoUrl: m.logoUrl ?? null, + checkId: check?.id ?? '', + connected: !!connection, + connectionId: connection?.id ?? null, + lastSyncAt: connection?.lastSyncAt?.toISOString() ?? null, + nextSyncAt: connection?.nextSyncAt?.toISOString() ?? null, + }; + }), + ); + } + + /** + * The primitive: full results of a specific check's latest real run for a + * specific connection. Everything else composes from this. Pass `resourceType` + * to load only rows of that kind. Returns [] when the check has never really run. + */ + async getLatestResultsByCheck({ + organizationId, + connectionId, + checkId, + resourceType, + }: { + organizationId: string; + connectionId: string; + checkId: string; + resourceType?: string; + }): Promise { + const latest = await this.checkRunRepo.findLatestResultsByConnectionAndCheck( + { connectionId, checkId, organizationId, resourceType }, + ); + if (!latest) return []; + + return latest.results.map((r) => ({ + resourceId: r.resourceId, + resourceType: r.resourceType, + passed: r.passed, + title: r.title, + description: r.description, + evidence: r.evidence, + collectedAt: r.collectedAt, + runId: latest.run.id, + connectionId, + })); + } + + /** + * Convenience: results for a task-bound check from a chosen source (provider + * slug). Resolves task -> check and slug -> connection, then fetches the + * latest real run. Returns [] when the source isn't bound/connected or has no + * real run. This is what a "pick a source, show its results" feature uses. + */ + async getLatestResultsForTask({ + organizationId, + taskTemplateId, + sourceSlug, + resourceType, + }: { + organizationId: string; + taskTemplateId: TaskTemplateId; + sourceSlug: string; + resourceType?: string; + }): Promise { + const manifest = this.manifestsBoundToTask(taskTemplateId).find( + (m) => m.id === sourceSlug, + ); + const check = manifest + ? this.checkBoundToTask(manifest, taskTemplateId) + : null; + if (!check) return []; + + const connection = await this.connectionRepository.findBySlugAndOrg( + sourceSlug, + organizationId, + ); + if (!connection) return []; + + return this.getLatestResultsByCheck({ + organizationId, + connectionId: connection.id, + checkId: check.id, + resourceType, + }); + } +} From 7f75e36f7eb8069f8247ebb53041dd968c4b354d Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 2 Jul 2026 10:30:57 -0400 Subject: [PATCH 04/33] feat(people): per-employee 2FA column from the org's selected 2FA source Frontend for the universal 2FA source: a toolbar selector (clone of the sync-source pattern) picks which connected integration bound to the 2FA evidence task supplies per-user status; the People list joins the latest check-run results to members by email and renders a 2FA row per person as Done / Missing / Not provided (absence never reads as failure). Also: service cleanups (batched connection lookup via ConnectionRepository.findActiveBySlugsAndOrg, typed manifest+check pairs) and hide 0/0 count rows (nothing-to-complete is not-applicable). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../repositories/connection.repository.ts | 44 +++++++ .../services/check-results.service.spec.ts | 27 ++--- .../services/check-results.service.ts | 84 ++++++-------- .../people/all/components/MemberRow.test.tsx | 87 ++++++++++++++ .../people/all/components/MemberRow.tsx | 44 +++++-- .../all/components/TaskRequirements.test.tsx | 20 ++++ .../all/components/TaskRequirements.tsx | 20 +++- .../people/all/components/TeamMembers.tsx | 20 +++- .../all/components/TeamMembersClient.tsx | 12 +- .../TwoFactorSourceSelector.test.tsx | 87 ++++++++++++++ .../components/TwoFactorSourceSelector.tsx | 92 +++++++++++++++ .../components/two-factor-status-map.test.ts | 43 +++++++ .../all/components/two-factor-status-map.ts | 34 ++++++ .../[orgId]/people/all/hooks/use2faSource.ts | 107 ++++++++++++++++++ 14 files changed, 647 insertions(+), 74 deletions(-) create mode 100644 apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/people/all/components/two-factor-status-map.test.ts create mode 100644 apps/app/src/app/(app)/[orgId]/people/all/components/two-factor-status-map.ts create mode 100644 apps/app/src/app/(app)/[orgId]/people/all/hooks/use2faSource.ts diff --git a/apps/api/src/integration-platform/repositories/connection.repository.ts b/apps/api/src/integration-platform/repositories/connection.repository.ts index a4ba3be825..c88241cabd 100644 --- a/apps/api/src/integration-platform/repositories/connection.repository.ts +++ b/apps/api/src/integration-platform/repositories/connection.repository.ts @@ -61,6 +61,50 @@ export class ConnectionRepository { return this.findByProviderAndOrg(provider.id, organizationId); } + /** + * Active connections for a set of provider slugs, keyed by slug, in ONE query. + * When a provider has several active connections (e.g. AWS multi-account) the + * newest wins. Slugs with no active connection are simply absent from the map. + */ + async findActiveBySlugsAndOrg( + providerSlugs: string[], + organizationId: string, + ): Promise< + Map + > { + if (providerSlugs.length === 0) return new Map(); + + const connections = await db.integrationConnection.findMany({ + where: { + organizationId, + status: 'active', + provider: { slug: { in: providerSlugs } }, + }, + select: { + id: true, + lastSyncAt: true, + nextSyncAt: true, + provider: { select: { slug: true } }, + }, + orderBy: { createdAt: 'desc' }, + }); + + const bySlug = new Map< + string, + { id: string; lastSyncAt: Date | null; nextSyncAt: Date | null } + >(); + for (const connection of connections) { + if (!bySlug.has(connection.provider.slug)) { + bySlug.set(connection.provider.slug, { + id: connection.id, + lastSyncAt: connection.lastSyncAt, + nextSyncAt: connection.nextSyncAt, + }); + } + } + return bySlug; + } + async findByOrganization( organizationId: string, ): Promise { diff --git a/apps/api/src/integration-platform/services/check-results.service.spec.ts b/apps/api/src/integration-platform/services/check-results.service.spec.ts index 3fbdc4fc2b..05a89dac1f 100644 --- a/apps/api/src/integration-platform/services/check-results.service.spec.ts +++ b/apps/api/src/integration-platform/services/check-results.service.spec.ts @@ -1,6 +1,4 @@ -jest.mock('@db', () => ({ - db: { integrationConnection: { findFirst: jest.fn() } }, -})); +jest.mock('@db', () => ({ db: {} })); jest.mock('@trycompai/integration-platform', () => { const actual = jest.requireActual< @@ -12,19 +10,18 @@ jest.mock('@trycompai/integration-platform', () => { }; }); -import { db } from '@db'; import { registry, TASK_TEMPLATES } from '@trycompai/integration-platform'; import { CheckResultsService } from './check-results.service'; const mockGetActiveManifests = ( registry as unknown as { getActiveManifests: jest.Mock } ).getActiveManifests; -const mockConnFindFirst = ( - db.integrationConnection as unknown as { findFirst: jest.Mock } -).findFirst; const mockCheckRunRepo = { findLatestResultsByConnectionAndCheck: jest.fn() }; -const mockConnRepo = { findBySlugAndOrg: jest.fn() }; +const mockConnRepo = { + findBySlugAndOrg: jest.fn(), + findActiveBySlugsAndOrg: jest.fn(), +}; function makeService() { return new CheckResultsService( @@ -64,15 +61,19 @@ describe('CheckResultsService.listSourcesBoundToTask', () => { unboundManifest('slack'), boundManifest('github', 'GitHub'), ]); - mockConnFindFirst.mockImplementation( - (args: { where: { provider: { slug: string } } }) => - args.where.provider.slug === 'google-workspace' - ? Promise.resolve({ id: 'c1', lastSyncAt: null, nextSyncAt: null }) - : Promise.resolve(null), + mockConnRepo.findActiveBySlugsAndOrg.mockResolvedValue( + new Map([ + ['google-workspace', { id: 'c1', lastSyncAt: null, nextSyncAt: null }], + ]), ); const sources = await makeService().listSourcesBoundToTask(ORG, TWO_FA); + // One batched lookup for all bound slugs — not one query per manifest. + expect(mockConnRepo.findActiveBySlugsAndOrg).toHaveBeenCalledWith( + ['google-workspace', 'github'], + ORG, + ); expect(sources.map((s) => s.slug)).toEqual(['google-workspace', 'github']); const gws = sources.find((s) => s.slug === 'google-workspace'); expect(gws).toMatchObject({ diff --git a/apps/api/src/integration-platform/services/check-results.service.ts b/apps/api/src/integration-platform/services/check-results.service.ts index c24be73543..313a42d94e 100644 --- a/apps/api/src/integration-platform/services/check-results.service.ts +++ b/apps/api/src/integration-platform/services/check-results.service.ts @@ -1,8 +1,8 @@ import { Injectable } from '@nestjs/common'; -import { db } from '@db'; import type { Prisma } from '@db'; import { registry, + type IntegrationCheck, type IntegrationManifest, type TaskTemplateId, } from '@trycompai/integration-platform'; @@ -65,28 +65,21 @@ export class CheckResultsService { private readonly connectionRepository: ConnectionRepository, ) {} - /** The check on a manifest bound to a task template, if any. */ - private checkBoundToTask( - manifest: IntegrationManifest, - taskTemplateId: TaskTemplateId, - ) { - return ( - manifest.checks?.find((check) => check.taskMapping === taskTemplateId) ?? - null - ); - } - /** - * Every active manifest — codebase OR dynamic — whose check is bound to a task - * template. Dynamic manifests are merged into the same registry, so this is - * uniformly universal. + * Every active manifest — codebase OR dynamic — paired with its check bound to + * the task template. Dynamic manifests are merged into the same registry, so + * this is uniformly universal. Pairing (instead of re-finding the check later) + * lets the type system carry the "bound check exists" guarantee. */ - private manifestsBoundToTask( + private boundPairsForTask( taskTemplateId: TaskTemplateId, - ): IntegrationManifest[] { - return registry - .getActiveManifests() - .filter((m) => !!this.checkBoundToTask(m, taskTemplateId)); + ): Array<{ manifest: IntegrationManifest; check: IntegrationCheck }> { + return registry.getActiveManifests().flatMap((manifest) => { + const check = manifest.checks?.find( + (c) => c.taskMapping === taskTemplateId, + ); + return check ? [{ manifest, check }] : []; + }); } /** @@ -99,26 +92,26 @@ export class CheckResultsService { organizationId: string, taskTemplateId: TaskTemplateId, ): Promise { - const manifests = this.manifestsBoundToTask(taskTemplateId); - return Promise.all( - manifests.map(async (m) => { - const check = this.checkBoundToTask(m, taskTemplateId); - const connection = await db.integrationConnection.findFirst({ - where: { organizationId, status: 'active', provider: { slug: m.id } }, - select: { id: true, lastSyncAt: true, nextSyncAt: true }, - }); - return { - slug: m.id, - name: m.name, - logoUrl: m.logoUrl ?? null, - checkId: check?.id ?? '', - connected: !!connection, - connectionId: connection?.id ?? null, - lastSyncAt: connection?.lastSyncAt?.toISOString() ?? null, - nextSyncAt: connection?.nextSyncAt?.toISOString() ?? null, - }; - }), - ); + const pairs = this.boundPairsForTask(taskTemplateId); + const connectionsBySlug = + await this.connectionRepository.findActiveBySlugsAndOrg( + pairs.map((p) => p.manifest.id), + organizationId, + ); + + return pairs.map(({ manifest, check }) => { + const connection = connectionsBySlug.get(manifest.id); + return { + slug: manifest.id, + name: manifest.name, + logoUrl: manifest.logoUrl ?? null, + checkId: check.id, + connected: !!connection, + connectionId: connection?.id ?? null, + lastSyncAt: connection?.lastSyncAt?.toISOString() ?? null, + nextSyncAt: connection?.nextSyncAt?.toISOString() ?? null, + }; + }); } /** @@ -172,13 +165,10 @@ export class CheckResultsService { sourceSlug: string; resourceType?: string; }): Promise { - const manifest = this.manifestsBoundToTask(taskTemplateId).find( - (m) => m.id === sourceSlug, + const pair = this.boundPairsForTask(taskTemplateId).find( + (p) => p.manifest.id === sourceSlug, ); - const check = manifest - ? this.checkBoundToTask(manifest, taskTemplateId) - : null; - if (!check) return []; + if (!pair) return []; const connection = await this.connectionRepository.findBySlugAndOrg( sourceSlug, @@ -189,7 +179,7 @@ export class CheckResultsService { return this.getLatestResultsByCheck({ organizationId, connectionId: connection.id, - checkId: check.id, + checkId: pair.check.id, resourceType, }); } diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.test.tsx index a5924dcb9d..8682c8c9b4 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.test.tsx @@ -278,4 +278,91 @@ describe('MemberRow device status', () => { screen.queryByLabelText('Employee has completed a background check'), ).not.toBeInTheDocument(); }); + + it('hides the Policies row when there are no required policies (0/0 is not-applicable)', () => { + render( + + + + +
, + ); + + expect(screen.queryByTestId('requirement-Policies')).not.toBeInTheDocument(); + }); +}); + +describe('MemberRow 2FA status', () => { + function render2fa({ + member = baseMember, + twoFactorStatus, + }: { + member?: MemberWithUser; + twoFactorStatus?: 'enabled' | 'missing' | 'not-provided'; + }) { + return render( + + + + +
, + ); + } + + it('shows no 2FA row when no source is configured (status undefined)', () => { + render2fa({}); + expect(screen.queryByTestId('requirement-2FA')).not.toBeInTheDocument(); + }); + + it.each([ + ['enabled', 'Done'], + ['missing', 'Missing'], + ['not-provided', 'Not provided'], + ] as const)('renders %s as "%s"', (status, text) => { + render2fa({ twoFactorStatus: status }); + expect(screen.getByTestId('requirement-2FA')).toHaveTextContent(text); + }); + + it('shows the 2FA row even for platform admins (2FA applies to all members)', () => { + const adminMember = { + ...baseMember, + user: { ...baseMember.user, role: 'admin' as const }, + } as MemberWithUser; + + render2fa({ member: adminMember, twoFactorStatus: 'enabled' }); + expect(screen.getByTestId('requirement-2FA')).toHaveTextContent('Done'); + }); + + it('hides the 2FA row for deactivated members', () => { + const deactivatedMember = { + ...baseMember, + deactivated: true, + } as MemberWithUser; + + render2fa({ member: deactivatedMember, twoFactorStatus: 'missing' }); + expect(screen.queryByTestId('requirement-2FA')).not.toBeInTheDocument(); + }); }); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx index b5a617e482..25d66a98f5 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx @@ -45,7 +45,12 @@ import { RemoveDeviceAlert } from './RemoveDeviceAlert'; import { TaskRequirements, type TaskRequirementItem } from './TaskRequirements'; import { RemoveMemberAlert } from './RemoveMemberAlert'; import type { CustomRoleOption } from './MultiRoleCombobox'; -import type { BackgroundCheckStatus, MemberWithUser, TaskCompletion } from './TeamMembers'; +import type { + BackgroundCheckStatus, + MemberWithUser, + TaskCompletion, + TwoFactorStatus, +} from './TeamMembers'; interface MemberRowProps { member: MemberWithUser; @@ -62,6 +67,8 @@ interface MemberRowProps { isDeviceStatusLoading?: boolean; backgroundCheckStatus?: BackgroundCheckStatus; backgroundCheckStepEnabled?: boolean; + /** From the org's configured 2FA source; absent when no source is configured. */ + twoFactorStatus?: TwoFactorStatus; } function getInitials(name?: string | null, email?: string | null): string { @@ -114,6 +121,7 @@ export function MemberRow({ isDeviceStatusLoading = false, backgroundCheckStatus, backgroundCheckStepEnabled = true, + twoFactorStatus, }: MemberRowProps) { const { orgId } = useParams<{ orgId: string }>(); @@ -146,12 +154,16 @@ export function MemberRow({ const taskItems: TaskRequirementItem[] = []; if (taskCompletion) { - taskItems.push({ - label: 'Policies', - completed: taskCompletion.policies.completed, - total: taskCompletion.policies.total, - kind: 'count', - }); + // total === 0 means "nothing to complete" (e.g. no required policies) — + // that's not-applicable, not incomplete, so no row at all. + if (taskCompletion.policies.total > 0) { + taskItems.push({ + label: 'Policies', + completed: taskCompletion.policies.completed, + total: taskCompletion.policies.total, + kind: 'count', + }); + } if (taskCompletion.training.total > 0) { taskItems.push({ @@ -190,6 +202,24 @@ export function MemberRow({ }); } + // 2FA applies to EVERY non-deactivated member (platform admins included), + // unlike the compliance-scoped rows above. Absent status = no 2FA source + // configured for the org, so no row. + if (!isDeactivated && twoFactorStatus) { + taskItems.push({ + label: '2FA', + completed: twoFactorStatus === 'enabled' ? 1 : 0, + total: 1, + kind: 'binary', + state: + twoFactorStatus === 'enabled' + ? 'done' + : twoFactorStatus === 'missing' + ? 'missing' + : 'not-provided', + }); + } + const isDeviceLoadingRow = shouldShowTaskRequirements && isDeviceStatusLoading && !deviceStatus; const handleEditRolesClick = () => { diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.test.tsx index bb3894f554..c7825a74b2 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.test.tsx @@ -53,4 +53,24 @@ describe('TaskRequirements', () => { render(); expect(screen.queryByText('—')).not.toBeInTheDocument(); }); + + it('renders an explicit not-provided state distinctly from Missing', () => { + render( + , + ); + const row = screen.getByTestId('requirement-2FA'); + expect(row).toHaveTextContent('Not provided'); + expect(row).not.toHaveTextContent('Missing'); + }); + + it('lets an explicit state override the completed/total derivation', () => { + render( + , + ); + expect(screen.getByTestId('requirement-2FA')).toHaveTextContent('Done'); + }); }); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx index 41901408a0..35b3ceb25f 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx @@ -6,13 +6,21 @@ export interface TaskRequirementItem { label: string; completed: number; total: number; - /** 'count' renders "x/y" + a progress bar; 'binary' renders a Done/Missing badge. */ + /** 'count' renders "x/y" + a progress bar; 'binary' renders a status badge. */ kind: 'count' | 'binary'; + /** + * Explicit badge state for binary requirements whose status comes from an + * external source (e.g. 2FA). 'not-provided' = the source had no data for + * this member — distinct from 'missing' (an explicit failure). When omitted, + * the state derives from completed/total. + */ + state?: 'done' | 'missing' | 'not-provided'; } function TaskRequirementRow({ item }: { item: TaskRequirementItem }) { const { label, completed, total, kind } = item; const isComplete = total > 0 && completed >= total; + const state = item.state ?? (isComplete ? 'done' : 'missing'); return (
@@ -23,9 +31,13 @@ function TaskRequirementRow({ item }: { item: TaskRequirementItem }) {
{kind === 'binary' ? ( - - {isComplete ? 'Done' : 'Missing'} - + state === 'not-provided' ? ( + Not provided + ) : ( + + {state === 'done' ? 'Done' : 'Missing'} + + ) ) : ( <>
diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembers.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembers.tsx index ebfb9338ff..94234a3163 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembers.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembers.tsx @@ -7,8 +7,13 @@ import { db } from '@db/server'; import { getEmployeeSyncConnections } from '../data/queries'; import { TeamMembersClient } from './TeamMembersClient'; import type { BackgroundCheckStatus } from '../../[employeeId]/components/backgroundCheckTypes'; +import { + buildTwoFactorStatusMap, + type TwoFactorStatusesResponse, +} from './two-factor-status-map'; export type { BackgroundCheckStatus }; +export type { TwoFactorStatus } from './two-factor-status-map'; export interface MemberWithUser extends Member { user: User; @@ -54,12 +59,15 @@ export async function TeamMembers(props: TeamMembersProps) { return null; } - // Fetch members and invitations from API - const [membersRes, invitationsRes] = await Promise.all([ + // Fetch members, invitations, and 2FA statuses from API + const [membersRes, invitationsRes, twoFactorRes] = await Promise.all([ serverApi.get<{ data: MemberWithUser[]; count: number }>( '/v1/people?includeDeactivated=true', ), serverApi.get<{ data: Invitation[] }>('/v1/auth/invitations'), + serverApi.get( + '/v1/integrations/sync/two-factor-statuses', + ), ]); const members: MemberWithUser[] = Array.isArray(membersRes.data?.data) @@ -71,6 +79,13 @@ export async function TeamMembers(props: TeamMembersProps) { const data: TeamMembersData = { members, pendingInvitations }; + // Empty when no 2FA source is configured (or the fetch failed) — rows then + // show no 2FA entry rather than a wrong one. + const twoFactorStatusMap = buildTwoFactorStatusMap( + members, + twoFactorRes.data ?? undefined, + ); + // Fetch employee sync connections server-side const employeeSyncData = await getEmployeeSyncConnections(organizationId); @@ -165,6 +180,7 @@ export async function TeamMembers(props: TeamMembersProps) { taskCompletionMap={taskCompletionMap} complianceMemberIds={complianceMemberIds} backgroundCheckStepEnabled={backgroundCheckStepEnabled} + twoFactorStatusMap={twoFactorStatusMap} /> ); } 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 c481f278b1..9be69f48d6 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 @@ -48,7 +48,13 @@ import { buildDisplayItems, filterDisplayItems } from './filter-members'; import { computeDeviceStatusMap } from './compute-device-status-map'; import { MemberRow } from './MemberRow'; import { PendingInvitationRow } from './PendingInvitationRow'; -import type { MemberWithUser, TaskCompletion, TeamMembersData } from './TeamMembers'; +import { TwoFactorSourceSelector } from './TwoFactorSourceSelector'; +import type { + MemberWithUser, + TaskCompletion, + TeamMembersData, + TwoFactorStatus, +} from './TeamMembers'; import type { EmployeeSyncConnectionsData } from '../data/queries'; import { useEmployeeSync } from '../hooks/useEmployeeSync'; @@ -63,6 +69,7 @@ interface TeamMembersClientProps { taskCompletionMap: Record; complianceMemberIds: string[]; backgroundCheckStepEnabled: boolean; + twoFactorStatusMap: Record; } export function TeamMembersClient({ @@ -74,6 +81,7 @@ export function TeamMembersClient({ taskCompletionMap, complianceMemberIds, backgroundCheckStepEnabled, + twoFactorStatusMap, }: TeamMembersClientProps) { const { agentDevices, isLoading: isAgentDevicesLoading } = useAgentDevices(); const { fleetHosts, isLoading: isFleetHostsLoading } = useFleetHosts(); @@ -493,6 +501,7 @@ export function TeamMembersClient({
)} +
{/* Table */} @@ -556,6 +565,7 @@ export function TeamMembersClient({ (item as MemberWithUser).backgroundCheckRequests?.[0]?.status } backgroundCheckStepEnabled={backgroundCheckStepEnabled} + twoFactorStatus={twoFactorStatusMap[(item as MemberWithUser).id]} /> ) : ( ({ + mockHasPermission: vi.fn(), + mockUse2faSource: vi.fn(), +})); + +vi.mock('next/navigation', () => ({ + useParams: () => ({ orgId: 'org_1' }), + useRouter: () => ({ refresh: vi.fn() }), +})); + +vi.mock('@/hooks/use-permissions', () => ({ + usePermissions: () => ({ hasPermission: mockHasPermission }), +})); + +vi.mock('../hooks/use2faSource', () => ({ + use2faSource: (opts: { organizationId: string; enabled?: boolean }) => + mockUse2faSource(opts), +})); + +const source: TwoFactorSourceProviderInfo = { + slug: 'google-workspace', + name: 'Google Workspace', + logoUrl: 'https://example.com/gws.png', + connected: true, + connectionId: 'icn_1', + lastSyncAt: null, + nextSyncAt: null, +}; + +beforeEach(() => { + vi.clearAllMocks(); + mockUse2faSource.mockReturnValue({ + selectedSource: 'google-workspace', + isLoading: false, + availableSources: [source], + setSource: vi.fn(), + hasAnyConnection: true, + }); +}); + +describe('TwoFactorSourceSelector — RBAC gating', () => { + it('renders the selector for a user with integration:update', () => { + mockHasPermission.mockImplementation( + (resource: string, action: string) => + resource === 'integration' && action === 'update', + ); + + render(); + + expect(screen.getByText('Google Workspace')).toBeInTheDocument(); + // Hook is enabled (and therefore allowed to hit the 2FA-source APIs). + expect(mockUse2faSource).toHaveBeenCalledWith( + expect.objectContaining({ enabled: true }), + ); + }); + + it('renders nothing for a user without integration:update and disables the hook', () => { + mockHasPermission.mockReturnValue(false); + + const { container } = render(); + + expect(container).toBeEmptyDOMElement(); + // The hook must be disabled so no 2FA-source API is called without permission. + expect(mockUse2faSource).toHaveBeenCalledWith( + expect.objectContaining({ enabled: false }), + ); + }); + + it('renders nothing when no bound integration is connected', () => { + mockHasPermission.mockReturnValue(true); + mockUse2faSource.mockReturnValue({ + selectedSource: null, + isLoading: false, + availableSources: [{ ...source, connected: false, connectionId: null }], + setSource: vi.fn(), + hasAnyConnection: false, + }); + + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx new file mode 100644 index 0000000000..e6052b2914 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx @@ -0,0 +1,92 @@ +'use client'; + +import Image from 'next/image'; +import { useParams, useRouter } from 'next/navigation'; + +import { usePermissions } from '@/hooks/use-permissions'; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@trycompai/design-system'; + +import { use2faSource } from '../hooks/use2faSource'; + +const NONE_VALUE = 'none'; + +/** + * Picks which connected integration (bound to the 2FA evidence task) supplies + * the per-employee 2FA column. Hidden for users without integration:update and + * for orgs with no eligible connected integration. + */ +export function TwoFactorSourceSelector() { + const { orgId } = useParams<{ orgId: string }>(); + const router = useRouter(); + const { hasPermission } = usePermissions(); + // Selecting a source is an integration:update action. Gate the hook itself so + // users without the permission never hit any 2FA-source API. + const canManage = hasPermission('integration', 'update'); + const { selectedSource, availableSources, setSource, hasAnyConnection } = + use2faSource({ organizationId: orgId, enabled: canManage }); + + if (!canManage || !hasAnyConnection) { + return null; + } + + const connectedSources = availableSources.filter((p) => p.connected); + const selected = connectedSources.find((p) => p.slug === selectedSource); + + const handleSourceChange = async (value: string) => { + const provider = value === NONE_VALUE ? null : value; + if (provider === selectedSource) return; + const ok = await setSource(provider); + if (ok) { + // The 2FA column is server-computed; refresh so it reflects the new source. + router.refresh(); + } + }; + + return ( +
+ +
+ ); +} diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/two-factor-status-map.test.ts b/apps/app/src/app/(app)/[orgId]/people/all/components/two-factor-status-map.test.ts new file mode 100644 index 0000000000..5515207bd7 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/two-factor-status-map.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest'; +import { buildTwoFactorStatusMap } from './two-factor-status-map'; + +const members = [ + { id: 'mem_1', user: { email: 'Alice@X.com' } }, + { id: 'mem_2', user: { email: 'bob@x.com' } }, + { id: 'mem_3', user: { email: 'carol@x.com' } }, + { id: 'mem_4', user: { email: null } }, +]; + +describe('buildTwoFactorStatusMap', () => { + it('returns an empty map when no source is configured', () => { + expect( + buildTwoFactorStatusMap(members, { + configured: false, + source: null, + statuses: [], + }), + ).toEqual({}); + }); + + it('returns an empty map when the response is missing (fetch failed)', () => { + expect(buildTwoFactorStatusMap(members, undefined)).toEqual({}); + }); + + it('joins by email case-insensitively and resolves absence to not-provided', () => { + const map = buildTwoFactorStatusMap(members, { + configured: true, + source: 'google-workspace', + statuses: [ + { email: 'alice@x.com', status: 'enabled' }, + { email: 'BOB@x.com', status: 'missing' }, + ], + }); + + expect(map).toEqual({ + mem_1: 'enabled', // matched despite different casing on both sides + mem_2: 'missing', + mem_3: 'not-provided', // no result row for this member + mem_4: 'not-provided', // member without an email can never match + }); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/two-factor-status-map.ts b/apps/app/src/app/(app)/[orgId]/people/all/components/two-factor-status-map.ts new file mode 100644 index 0000000000..4ad34491cc --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/two-factor-status-map.ts @@ -0,0 +1,34 @@ +export type TwoFactorStatus = 'enabled' | 'missing' | 'not-provided'; + +/** Shape of GET /v1/integrations/sync/two-factor-statuses. */ +export interface TwoFactorStatusesResponse { + configured: boolean; + source: string | null; + statuses: Array<{ email: string; status: 'enabled' | 'missing' }>; +} + +/** + * Join the 2FA source's per-email results onto members by lowercased email. + * + * Returns an empty map when no 2FA source is configured (callers render no 2FA + * row at all). A member with no matching result row gets 'not-provided' — + * absence means the source had no data for them, which must never read as an + * explicit 'missing' failure. + */ +export function buildTwoFactorStatusMap( + members: Array<{ id: string; user: { email: string | null } }>, + response: TwoFactorStatusesResponse | undefined, +): Record { + if (!response?.configured) return {}; + + const byEmail = new Map( + response.statuses.map((s) => [s.email.toLowerCase().trim(), s.status]), + ); + + const map: Record = {}; + for (const member of members) { + const email = member.user.email?.toLowerCase().trim(); + map[member.id] = (email && byEmail.get(email)) || 'not-provided'; + } + return map; +} diff --git a/apps/app/src/app/(app)/[orgId]/people/all/hooks/use2faSource.ts b/apps/app/src/app/(app)/[orgId]/people/all/hooks/use2faSource.ts new file mode 100644 index 0000000000..7122dcfa8a --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/people/all/hooks/use2faSource.ts @@ -0,0 +1,107 @@ +'use client'; + +import { apiClient } from '@/lib/api-client'; +import { toast } from 'sonner'; +import useSWR from 'swr'; + +export interface TwoFactorSourceProviderInfo { + slug: string; + name: string; + logoUrl: string | null; + connected: boolean; + connectionId: string | null; + lastSyncAt: string | null; + nextSyncAt: string | null; +} + +interface Use2faSourceOptions { + organizationId: string; + /** + * When false, the hook makes no API calls (used to fully disable it for users + * who lack the integration:update permission, so no 2FA-source API is hit). + */ + enabled?: boolean; +} + +interface Use2faSourceReturn { + selectedSource: string | null; + isLoading: boolean; + availableSources: TwoFactorSourceProviderInfo[]; + setSource: (provider: string | null) => Promise; + hasAnyConnection: boolean; +} + +/** + * The org's 2FA source: which connected integration (bound to the 2FA evidence + * task) supplies the per-employee 2FA column on the People tab. + */ +export function use2faSource({ + organizationId, + enabled = true, +}: Use2faSourceOptions): Use2faSourceReturn { + const { data: sourceData, mutate: mutateSource } = useSWR<{ + provider: string | null; + }>( + enabled + ? `/v1/integrations/sync/two-factor-source?organizationId=${organizationId}` + : null, + async (url: string) => { + const res = await apiClient.get<{ provider: string | null }>(url); + if (res.error) throw new Error(res.error); + return res.data as { provider: string | null }; + }, + ); + + const { data: availableData, isLoading } = useSWR<{ + providers: TwoFactorSourceProviderInfo[]; + }>( + enabled + ? `/v1/integrations/sync/available-2fa-sources?organizationId=${organizationId}` + : null, + async (url: string) => { + const res = await apiClient.get<{ + providers: TwoFactorSourceProviderInfo[]; + }>(url); + if (res.error) throw new Error(res.error); + return res.data as { providers: TwoFactorSourceProviderInfo[] }; + }, + ); + + const selectedSource = sourceData?.provider ?? null; + const availableSources = Array.isArray(availableData?.providers) + ? availableData.providers + : []; + + const setSource = async (provider: string | null): Promise => { + try { + const response = await apiClient.post( + `/v1/integrations/sync/two-factor-source?organizationId=${organizationId}`, + { provider }, + ); + if (response.error) { + toast.error('Failed to set 2FA source'); + return false; + } + + mutateSource({ provider }, false); + + if (provider) { + const name = + availableSources.find((p) => p.slug === provider)?.name ?? provider; + toast.success(`${name} set as your 2FA source`); + } + return true; + } catch { + toast.error('Failed to set 2FA source'); + return false; + } + }; + + return { + selectedSource, + isLoading, + availableSources, + setSource, + hasAnyConnection: availableSources.some((p) => p.connected), + }; +} From 07808ba574adec243671c335ec586ec0ae00eedb Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 2 Jul 2026 10:40:48 -0400 Subject: [PATCH 05/33] feat(ui): responsive-by-default mandate + collapse 2FA selector on mobile New responsive-ui skill: every UI change must work at mobile/tablet/ desktop/large-desktop widths by default, with repo patterns (DS Table overflow-x-auto, toolbar hidden sm:block convention, mobile-first Tailwind) and a done-checklist. Wired into the ui skill and CLAUDE.md so it loads without being asked. Apply it to the 2FA source selector: hidden sm:block, matching the other secondary toolbar controls (the no-wrap toolbar squeezed on phones); the per-member 2FA status stays visible at every width via table scroll. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/skills/responsive-ui/SKILL.md | 73 +++++++++++++++++++ .claude/skills/ui/SKILL.md | 9 +++ CLAUDE.md | 1 + .../TwoFactorSourceSelector.test.tsx | 8 ++ .../components/TwoFactorSourceSelector.tsx | 6 +- 5 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 .claude/skills/responsive-ui/SKILL.md diff --git a/.claude/skills/responsive-ui/SKILL.md b/.claude/skills/responsive-ui/SKILL.md new file mode 100644 index 0000000000..1e7a15d250 --- /dev/null +++ b/.claude/skills/responsive-ui/SKILL.md @@ -0,0 +1,73 @@ +--- +name: responsive-ui +description: MANDATORY for any UI work in apps/app, apps/portal, or packages/design-system — every component, page, or layout change must work on mobile (~375px), tablet (~768px), desktop (~1280px), and large desktop (~1920px) BY DEFAULT, without being asked. Read this before writing or editing any JSX/TSX that renders visible UI. Triggers on: new component, page, layout, table, toolbar, form, modal/sheet, dashboard, "build UI", "add a column", "add a filter", any styling change. +--- + +# Responsive UI — default, not a feature request + +## The rule + +**Every UI change ships working at all four device classes. Nobody has to ask.** + +| Device | Test width | Tailwind context | +|---|---|---| +| Mobile | 375px | base (no prefix) | +| Tablet | 768px | `md:` | +| Desktop | 1280px | `xl:` | +| Large desktop | 1920px | beyond `2xl:` | + +Tailwind is **mobile-first**: unprefixed classes are the mobile layout; prefixes add +behavior at wider screens (`sm:` 640, `md:` 768, `lg:` 1024, `xl:` 1280, `2xl:` 1536). +Write the mobile layout first, then widen — not the other way around. + +A change is NOT done until you have reasoned through (or better, viewed via the +`webapp-testing` skill / browser dev tools) what it looks like at 375, 768, 1280, +and 1920. If you only checked one width, you checked none. + +## Non-negotiables + +1. **The page body never scrolls horizontally.** Wide content (tables, code, diagrams) + scrolls inside its own container, never the page. +2. **No fixed pixel width without a responsive strategy.** A bare `w-[200px]` is only + acceptable if the element hides, shrinks, or wraps on smaller screens + (`hidden sm:block`, `w-full md:w-[200px]`, or a wrapping parent). +3. **Touch targets** on mobile: interactive elements ≥40px tall; don't rely on hover + for anything essential (no hover on touch devices). +4. **Test with real content lengths** — long names, emails, 33-item counts. Use + `truncate` / `min-w-0` on flex children that hold text. +5. **Large desktops:** content must not stretch into unreadable full-width lines — + cap with `max-w-*` containers where the layout doesn't already. + +## Repo patterns (use these, don't invent) + +- **Tables**: the design-system `Table` already wraps rows in `overflow-x-auto` — + wide tables scroll horizontally inside themselves on mobile. That is the accepted + pattern for dense admin tables. Do NOT try to reflow table columns per breakpoint; + do keep cells reasonable (`min-w-* max-w-*` on cell content is fine). +- **Toolbars / filter rows**: primary control (search) is `w-full md:max-w-[300px]`; + secondary controls (filters, source selectors) collapse on phones with + `hidden sm:block`. Example: the People tab toolbar in + `apps/app/.../people/all/components/TeamMembersClient.tsx`. +- **Page shells**: use design-system `PageLayout` / `Stack` / `HStack` — they carry + the responsive spacing; don't rebuild them with raw divs. +- **Grids**: `grid-cols-1 md:grid-cols-2 xl:grid-cols-3` style progressions — never a + fixed column count for all widths. +- **Sheets/Modals**: design-system `Sheet`/`Dialog` are responsive already; don't fix + their widths with arbitrary values. +- **Images/logos**: constrain (`max-w-full`, explicit small sizes); never let an + image force layout width. + +## Checklist before "done" (run mentally or in the browser) + +- [ ] 375px — nothing overflows the viewport; controls usable or intentionally hidden; text truncates instead of breaking layout +- [ ] 768px — secondary controls visible; layout uses available width sensibly +- [ ] 1280px — the intended "design" width; matches mockups +- [ ] 1920px — no absurdly stretched content; whitespace balanced +- [ ] Long content (names, emails, high counts) doesn't break any of the above +- [ ] Tests: if a component hides/reflows per breakpoint in a way that matters, its test asserts the class contract (e.g. `hidden sm:block`) + +## When you can skip a width + +Admin-only dense screens (e.g. internal tooling) may treat mobile as "usable via +horizontal scroll" rather than fully reflowed — but that must be a conscious choice +following the existing pattern of that page, never an accident of not checking. diff --git a/.claude/skills/ui/SKILL.md b/.claude/skills/ui/SKILL.md index 7dc3e4edfd..52e5aebda7 100644 --- a/.claude/skills/ui/SKILL.md +++ b/.claude/skills/ui/SKILL.md @@ -135,3 +135,12 @@ import { Plus, X } from 'lucide-react';
// Hardcoded colors
// Arbitrary values ``` + +## Responsive (MANDATORY — read the `responsive-ui` skill) + +Every UI change must work on mobile (375px), tablet (768px), desktop (1280px), and +large desktop (1920px) **by default — nobody has to ask**. Tailwind is mobile-first: +write the base (mobile) layout, widen with `sm:`/`md:`/`lg:`/`xl:`. No fixed pixel +widths without a responsive strategy (`hidden sm:block`, `w-full md:w-[...]`, or a +wrapping parent). Wide tables scroll inside the DS `Table` (`overflow-x-auto`), never +the page. Full rules + repo patterns + checklist: `.claude/skills/responsive-ui/SKILL.md`. diff --git a/CLAUDE.md b/CLAUDE.md index cfecc4842d..454a1e2b9e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -112,6 +112,7 @@ Every customer-facing API endpoint MUST have: - **DS components that do NOT accept `className`**: `Text`, `Stack`, `HStack`, `Badge`, `Button` — wrap in `
` for custom styling - **Layout**: Use `PageLayout`, `PageHeader`, `Stack`, `HStack`, `Section`, `SettingGroup` - **Patterns**: Sheet (`Sheet > SheetContent > SheetHeader + SheetBody`), Drawer, Collapsible +- **Responsive (MANDATORY)**: every UI change must work on mobile (375px), tablet (768px), desktop (1280px), and large desktop (1920px) by default — no one has to ask. See the `responsive-ui` skill for breakpoints, repo patterns, and the checklist. - **After editing any frontend component**: Run the `audit-design-system` skill to catch `@trycompai/ui` or `lucide-react` imports that should be migrated ## Data Fetching diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx index dc2bf454b9..ffbf8bbfc6 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx @@ -71,6 +71,14 @@ describe('TwoFactorSourceSelector — RBAC gating', () => { ); }); + it('collapses on mobile like the other secondary toolbar controls (hidden sm:block)', () => { + mockHasPermission.mockReturnValue(true); + + const { container } = render(); + + expect(container.firstElementChild).toHaveClass('hidden', 'sm:block'); + }); + it('renders nothing when no bound integration is connected', () => { mockHasPermission.mockReturnValue(true); mockUse2faSource.mockReturnValue({ diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx index e6052b2914..0f506a183a 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx @@ -49,7 +49,11 @@ export function TwoFactorSourceSelector() { }; return ( -
+ // hidden sm:block matches the other secondary toolbar controls (status/role/ + // date filters): config actions collapse on phones, where the toolbar row + // doesn't wrap. The per-member 2FA status stays visible at every width via + // the table's own horizontal scroll. +
{ const provider = String(value); @@ -705,7 +706,7 @@ function DateRangeFilter({
-
+
{label} · diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx index 710a0d2903..4fbd9390da 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx @@ -53,6 +53,8 @@ describe('TwoFactorSourceSelector — RBAC gating', () => { render(); expect(screen.getByText('Google Workspace')).toBeInTheDocument(); + // The label above the select is what tells users what this control is for. + expect(screen.getByText('2FA source')).toBeInTheDocument(); // Hook is enabled (and therefore allowed to hit the 2FA-source APIs). expect(mockUse2faSource).toHaveBeenCalledWith( expect.objectContaining({ enabled: true }), diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx index d2f24cc050..92b07ad605 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx @@ -9,7 +9,6 @@ import { SelectContent, SelectItem, SelectTrigger, - SelectValue, } from '@trycompai/design-system'; import { use2faSource } from '../hooks/use2faSource'; @@ -62,7 +61,9 @@ export function TwoFactorSourceSelector() { // doesn't wrap. The per-member 2FA status stays visible at every width via // the table's own horizontal scroll.
- { if (value) void handleSourceChange(value); @@ -84,7 +85,7 @@ export function TwoFactorSourceSelector() { {selected.name}
) : ( - + None )} @@ -98,7 +99,8 @@ export function TwoFactorSourceSelector() { ))} - + +
); } From 2f18dae7525865251dbbc4f04bc29eaa219bd6fb Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 2 Jul 2026 14:09:42 -0400 Subject: [PATCH 14/33] fix(people): associate toolbar select labels for screen readers The 'Sync source' / '2FA source' labels were visual-only spans; assistive tech announced unnamed selects. Give the spans ids and point the Radix SelectTriggers at them via aria-labelledby. Test asserts the accessible name resolves. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../people/all/components/TeamMembersClient.tsx | 6 ++++-- .../all/components/TwoFactorSourceSelector.test.tsx | 11 +++++++++-- .../people/all/components/TwoFactorSourceSelector.tsx | 6 ++++-- 3 files changed, 17 insertions(+), 6 deletions(-) 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 703f9cbe53..92d3ce1470 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 @@ -386,7 +386,9 @@ export function TeamMembersClient({ {hasAnyConnection && (
- Sync source + + Sync source + { if (value) void handleSourceChange(value); }} > - + {selected ? (
{selected.logoUrl && ( From 1925d819b73fcb631f9706c16503a4ddb47cb591 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 2 Jul 2026 15:47:08 -0400 Subject: [PATCH 15/33] style(people): brand-green counts + inline chip labels for source selects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Complete counts use the primary (brand) green — matching the Done badges — instead of the DS success green, which is a different hue - 'Sync source ·' / '2FA source ·' render inline inside the triggers, matching the Onboarded/Offboarded chip pattern (single-height toolbar); aria-labels name the comboboxes since the role ignores content for accessible names Co-Authored-By: Claude Opus 4.8 (1M context) --- .../all/components/TaskRequirements.tsx | 4 +- .../all/components/TeamMembersClient.tsx | 62 ++++++++++--------- .../TwoFactorSourceSelector.test.tsx | 10 +-- .../components/TwoFactorSourceSelector.tsx | 56 +++++++++-------- 4 files changed, 68 insertions(+), 64 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx index 2b36971f62..ce3b7b86df 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx @@ -43,7 +43,9 @@ function TaskRequirementRow({ item }: { item: TaskRequirementItem }) { // share one vertical line across rows. The colored count alone carries // the state (green done / amber partial / muted none) — no bar needed.
- 0 ? 'warning' : 'muted'}> + {/* 'primary' = the brand green — matches the Done badge (accent), + unlike the DS 'success' variant which is a different green. */} + 0 ? 'warning' : 'muted'}> {completed}/{total}
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 92d3ce1470..9e547da999 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 @@ -308,7 +308,7 @@ export function TeamMembersClient({ return ( {/* Search and Filters */} -
+
@@ -384,11 +384,8 @@ export function TeamMembersClient({ onClear={() => { setOffboardFrom(undefined); setOffboardTo(undefined); setPage(1); }} /> {hasAnyConnection && ( -
-
- - Sync source - +
+
+ -
+
); } From 234ba8ed763b24f6114b2e70e77a51817ab08303 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 2 Jul 2026 15:51:49 -0400 Subject: [PATCH 16/33] style(people): self-explanatory sentence labels for the source selects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit '2FA source' / 'Sync source' read as jargon to end users. The chips now read as sentences — '2FA status from · Google Workspace' and 'Sync people from · Google Workspace' — and the clear-option is 'Don't show 2FA status'. aria-labels and dropdown hint updated to match. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../[orgId]/people/all/components/TeamMembersClient.tsx | 6 +++--- .../all/components/TwoFactorSourceSelector.test.tsx | 4 ++-- .../people/all/components/TwoFactorSourceSelector.tsx | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) 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 9e547da999..59549c0e7e 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 @@ -398,12 +398,12 @@ export function TeamMembersClient({ }} disabled={isSyncing || isDisablingSync || !canManageMembers} > - - {/* Inline "Sync source ·" prefix mirrors the date chips; + + {/* Inline "Sync people from ·" prefix mirrors the date chips; the aria-label names the combobox for screen readers (comboboxes don't take their name from contents). */}
- Sync source + Sync people from · {isSyncing ? ( <> diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx index b1a10abdb4..11d8987674 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx @@ -53,10 +53,10 @@ describe('TwoFactorSourceSelector — RBAC gating', () => { render(); expect(screen.getByText('Google Workspace')).toBeInTheDocument(); - // The inline "2FA source ·" prefix tells users what the control is for and + // The inline "2FA status from ·" prefix tells users what the control is for and // makes it part of the trigger's accessible name for screen readers. expect( - screen.getByRole('combobox', { name: /2FA source/ }), + screen.getByRole('combobox', { name: /2FA status from/ }), ).toBeInTheDocument(); // Hook is enabled (and therefore allowed to hit the 2FA-source APIs). expect(mockUse2faSource).toHaveBeenCalledWith( diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx index 020af83964..95d1ac943a 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx @@ -70,9 +70,9 @@ export function TwoFactorSourceSelector() { > {/* combobox takes its accessible name from author attrs only — the inline prefix text alone can't name it, hence the aria-label. */} - +
- 2FA source + 2FA status from · {selected ? ( <> @@ -95,9 +95,9 @@ export function TwoFactorSourceSelector() {
- Shows each person's 2FA status from this integration + Shows each person's 2FA status from the selected integration's latest check
- No 2FA source + Don't show 2FA status {connectedSources.map((p) => ( {p.name} From 050bb87ccaa48f412b92b783fa2bd876a2bf2a2f Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 2 Jul 2026 16:34:41 -0400 Subject: [PATCH 17/33] style(people): uniform labels above every toolbar control MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inline chip labels read badly in practice — switch the whole toolbar to one pattern: a small muted label above each control. Status, Role, Onboarded, Offboarded, Sync people from, and 2FA status from all get labels; date chips and select triggers show just their value. All comboboxes/popovers are aria-labelledby their label for screen readers. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../all/components/TeamMembersClient.tsx | 87 ++++++++++--------- .../TwoFactorSourceSelector.test.tsx | 2 +- .../components/TwoFactorSourceSelector.tsx | 57 ++++++------ 3 files changed, 74 insertions(+), 72 deletions(-) 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 59549c0e7e..35d039f1df 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 @@ -308,7 +308,7 @@ export function TeamMembersClient({ return ( {/* Search and Filters */} -
+
@@ -322,7 +322,10 @@ export function TeamMembersClient({
{/* Status Filter Select */} -
+
+ + Status +
{/* Role Filter Select */} -
+
+ + Role + { const provider = String(value); @@ -398,36 +407,29 @@ export function TeamMembersClient({ }} disabled={isSyncing || isDisablingSync || !canManageMembers} > - - {/* Inline "Sync people from ·" prefix mirrors the date chips; - the aria-label names the combobox for screen readers - (comboboxes don't take their name from contents). */} -
- Sync people from - · - {isSyncing ? ( - <> - - Syncing... - - ) : selectedProvider ? ( - <> - {getProviderLogo(selectedProvider) && ( - - )} - {getProviderName(selectedProvider)} - - ) : ( - None - )} -
+ + {isSyncing ? ( + <> + + Syncing... + + ) : selectedProvider ? ( +
+ {getProviderLogo(selectedProvider) && ( + + )} + {getProviderName(selectedProvider)} +
+ ) : ( + None + )}
@@ -708,14 +710,17 @@ function DateRangeFilter({ ? `Until ${format(to, 'MMM d, yyyy')}` : 'Any time'; + const labelId = `people-${label.toLowerCase()}-filter-label`; + return ( -
+
+ + {label} + - +
- {label} - · {displayLabel}
diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx index 11d8987674..1f9fc88985 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx @@ -81,7 +81,7 @@ describe('TwoFactorSourceSelector — RBAC gating', () => { const { container } = render(); - expect(container.firstElementChild).toHaveClass('hidden', 'sm:block'); + expect(container.firstElementChild).toHaveClass('hidden', 'sm:flex'); }); it('renders nothing while the source/selection are still loading (no placeholder flash)', () => { diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx index 95d1ac943a..d1831e7ed7 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx @@ -56,42 +56,39 @@ export function TwoFactorSourceSelector() { }; return ( - // hidden sm:block matches the other secondary toolbar controls (status/role/ - // date filters): config actions collapse on phones, where the toolbar row - // doesn't wrap. The per-member 2FA status stays visible at every width via - // the table's own horizontal scroll. The inline "2FA source ·" prefix inside - // the trigger mirrors the Onboarded/Offboarded chips. -
+ // hidden sm:flex matches the other toolbar controls: config actions + // collapse on phones, where the toolbar row doesn't wrap. The per-member + // 2FA status stays visible at every width via the table's own horizontal + // scroll. Label above the input, uniform with the rest of the toolbar; + // aria-labelledby names the combobox (the role ignores content for names). +
+ + 2FA status from + + + + {hasOffboardFilter && !statusFilter + ? 'All People' + : ({ all: 'All People', active: 'Active', pending: 'Pending', deactivated: 'Deactivated' }[ + statusFilter + ] ?? 'Active')} + + + + All People + Active + Pending + Deactivated + + +
+ +
+ + Role + + +
+ + + +
+ + + ); +} diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/RequirementCell.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/RequirementCell.tsx new file mode 100644 index 0000000000..2448cb91c0 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/RequirementCell.tsx @@ -0,0 +1,72 @@ +'use client'; + +import { Badge, Skeleton, Text } from '@trycompai/design-system'; + +/** Muted dash for a requirement that doesn't apply to this member. */ +export function RequirementDash() { + return ( + + — + + ); +} + +/** + * Fractional requirement (policies, training): a colored count. 'primary' is + * the brand green, matching the Done badge; amber = partial; muted = none. + */ +export function RequirementCount({ + label, + completed, + total, +}: { + label: string; + completed: number; + total: number; +}) { + const isComplete = total > 0 && completed >= total; + return ( +
+ 0 ? 'warning' : 'muted'} + > + {completed}/{total} + +
+ ); +} + +/** + * Binary requirement (HIPAA, device, background, 2FA): a status badge. + * 'not-provided' = the source had no data for this member — distinct from an + * explicit 'missing' failure. + */ +export function RequirementBadge({ + label, + state, +}: { + label: string; + state: 'done' | 'missing' | 'not-provided'; +}) { + return ( +
+ {state === 'not-provided' ? ( + Not provided + ) : ( + + {state === 'done' ? 'Done' : 'Missing'} + + )} +
+ ); +} + +/** Loading placeholder while a requirement's status is still resolving. */ +export function RequirementLoading() { + return ( +
+ +
+ ); +} diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.test.tsx deleted file mode 100644 index 933e33223b..0000000000 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.test.tsx +++ /dev/null @@ -1,82 +0,0 @@ -import { render, screen } from '@testing-library/react'; -import { describe, expect, it } from 'vitest'; -import { TaskRequirements, type TaskRequirementItem } from './TaskRequirements'; - -const count = ( - label: string, - completed: number, - total: number, -): TaskRequirementItem => ({ label, completed, total, kind: 'count' }); - -const binary = (label: string, completed: number): TaskRequirementItem => ({ - label, - completed, - total: 1, - kind: 'binary', -}); - -describe('TaskRequirements', () => { - it('renders a dash when there are no items and nothing loading', () => { - render(); - expect(screen.getByText('—')).toBeInTheDocument(); - }); - - it('renders count requirements as "x/y"', () => { - render(); - const row = screen.getByTestId('requirement-Policies'); - expect(row).toHaveTextContent('Policies'); - expect(row).toHaveTextContent('24/33'); - }); - - it('renders a complete binary requirement as Done', () => { - render(); - expect(screen.getByTestId('requirement-Background')).toHaveTextContent('Done'); - }); - - it('renders an incomplete binary requirement as Missing', () => { - render(); - expect(screen.getByTestId('requirement-Device')).toHaveTextContent('Missing'); - }); - - it('renders each requirement on its own row', () => { - render( - , - ); - expect(screen.getByTestId('requirement-Policies')).toBeInTheDocument(); - expect(screen.getByTestId('requirement-Training')).toBeInTheDocument(); - expect(screen.getByTestId('requirement-Background')).toBeInTheDocument(); - }); - - it('shows a loading placeholder when showLoadingRow is set with no items', () => { - render(); - expect(screen.getByTestId('task-requirements-loading')).toBeInTheDocument(); - expect(screen.queryByText('—')).not.toBeInTheDocument(); - }); - - it('shows no loading placeholder when showLoadingRow is not set', () => { - render(); - expect(screen.queryByTestId('task-requirements-loading')).not.toBeInTheDocument(); - }); - - it('renders an explicit not-provided state distinctly from Missing', () => { - render( - , - ); - const row = screen.getByTestId('requirement-2FA'); - expect(row).toHaveTextContent('Not provided'); - expect(row).not.toHaveTextContent('Missing'); - }); - - it('lets an explicit state override the completed/total derivation', () => { - render( - , - ); - expect(screen.getByTestId('requirement-2FA')).toHaveTextContent('Done'); - }); -}); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx deleted file mode 100644 index ce3b7b86df..0000000000 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TaskRequirements.tsx +++ /dev/null @@ -1,92 +0,0 @@ -'use client'; - -import { Badge, Skeleton, Text } from '@trycompai/design-system'; - -export interface TaskRequirementItem { - label: string; - completed: number; - total: number; - /** 'count' renders a colored "x/y"; 'binary' renders a status badge. */ - kind: 'count' | 'binary'; - /** - * Explicit badge state for binary requirements whose status comes from an - * external source (e.g. 2FA). 'not-provided' = the source had no data for - * this member — distinct from 'missing' (an explicit failure). When omitted, - * the state derives from completed/total. - */ - state?: 'done' | 'missing' | 'not-provided'; -} - -function TaskRequirementRow({ item }: { item: TaskRequirementItem }) { - const { label, completed, total, kind } = item; - const isComplete = total > 0 && completed >= total; - const state = item.state ?? (isComplete ? 'done' : 'missing'); - - return ( -
-
- - {label} - -
- - {kind === 'binary' ? ( - state === 'not-provided' ? ( - Not provided - ) : ( - - {state === 'done' ? 'Done' : 'Missing'} - - ) - ) : ( - // pl-1.5 matches the Badge's inner padding so counts and badge text - // share one vertical line across rows. The colored count alone carries - // the state (green done / amber partial / muted none) — no bar needed. -
- {/* 'primary' = the brand green — matches the Done badge (accent), - unlike the DS 'success' variant which is a different green. */} - 0 ? 'warning' : 'muted'}> - {completed}/{total} - -
- )} -
- ); -} - -/** - * Per-employee requirement rollup shown in the People list TASKS column. - * Each requirement is on its own row: fractional requirements (policies, training) - * show a colored count; binary requirements (HIPAA, device, background) - * show a Done/Missing badge. - */ -export function TaskRequirements({ - items, - showLoadingRow = false, -}: { - items: TaskRequirementItem[]; - showLoadingRow?: boolean; -}) { - if (items.length === 0 && !showLoadingRow) { - return ( - - — - - ); - } - - return ( - // Content-sized (w-max): label column + value only — the column stays - // compact regardless of table width. -
- {items.map((item) => ( - - ))} - {showLoadingRow && ( -
- -
- )} -
- ); -} 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 35d039f1df..9e197e58d6 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 @@ -14,7 +14,6 @@ import useSWR from 'swr'; import type { Invitation } from '@db'; import { Button, - Calendar, Empty, EmptyDescription, EmptyHeader, @@ -22,9 +21,6 @@ import { InputGroup, InputGroupAddon, InputGroupInput, - Popover, - PopoverContent, - PopoverTrigger, Select, SelectContent, SelectItem, @@ -38,7 +34,7 @@ import { TableHeader, TableRow, } from '@trycompai/design-system'; -import { Calendar as CalendarIcon, ChevronDown, InProgress, Search } from '@trycompai/design-system/icons'; +import { InProgress, Search } from '@trycompai/design-system/icons'; import { apiClient } from '@/lib/api-client'; import { useMemo } from 'react'; @@ -46,7 +42,8 @@ import { useAgentDevices } from '../../devices/hooks/useAgentDevices'; import { useFleetHosts } from '../../devices/hooks/useFleetHosts'; import { buildDisplayItems, filterDisplayItems } from './filter-members'; import { computeDeviceStatusMap } from './compute-device-status-map'; -import { MemberRow } from './MemberRow'; +import { MemberRow, type RequirementColumnKey } from './MemberRow'; +import { PeopleFilters } from './PeopleFilters'; import { PendingInvitationRow } from './PendingInvitationRow'; import { TwoFactorSourceSelector } from './TwoFactorSourceSelector'; import type { @@ -96,6 +93,27 @@ export function TeamMembersClient({ () => computeDeviceStatusMap({ agentDevices, fleetHosts, complianceMemberIds }), [agentDevices, fleetHosts, complianceMemberIds], ); + + // Which requirement columns the table shows, in order. A column only exists + // when the underlying tracking applies to this org (flag on / framework + // present / 2FA source configured), so orgs never see empty dash columns. + const requirementColumns = useMemo< + Array<{ key: RequirementColumnKey; label: string }> + >(() => { + const completions = Object.values(taskCompletionMap); + const cols: Array<{ key: RequirementColumnKey; label: string }> = []; + if (completions.some((c) => c.policies.total > 0)) + cols.push({ key: 'policies', label: 'POLICIES' }); + if (completions.some((c) => c.training.total > 0)) + cols.push({ key: 'training', label: 'TRAINING' }); + if (completions.some((c) => !!c.hipaa)) cols.push({ key: 'hipaa', label: 'HIPAA' }); + if (complianceMemberIds.length > 0) cols.push({ key: 'device', label: 'DEVICE' }); + if (backgroundCheckStepEnabled) + cols.push({ key: 'background', label: 'BACKGROUND' }); + if (Object.keys(twoFactorStatusMap).length > 0) + cols.push({ key: 'twoFactor', label: '2FA' }); + return cols; + }, [taskCompletionMap, complianceMemberIds, backgroundCheckStepEnabled, twoFactorStatusMap]); const router = useRouter(); const [searchQuery, setSearchQuery] = useState(''); const [roleFilter, setRoleFilter] = useState(''); @@ -321,73 +339,26 @@ export function TeamMembersClient({ />
- {/* Status Filter Select */} -
- - Status - - -
- {/* Role Filter Select */} -
- - Role - - -
- { setOnboardFrom(from); setOnboardTo(to); setPage(1); }} - onClear={() => { setOnboardFrom(undefined); setOnboardTo(undefined); setPage(1); }} - /> - { setOffboardFrom(from); setOffboardTo(to); setPage(1); }} - onClear={() => { setOffboardFrom(undefined); setOffboardTo(undefined); setPage(1); }} + { + setStatusFilter(value ?? ''); + setPage(1); + }} + roleFilter={roleFilter} + onRoleChange={(value) => { + setRoleFilter(value === 'all' ? '' : (value ?? '')); + setPage(1); + }} + onboardFrom={onboardFrom} + onboardTo={onboardTo} + onOnboardApply={(from, to) => { setOnboardFrom(from); setOnboardTo(to); setPage(1); }} + onOnboardClear={() => { setOnboardFrom(undefined); setOnboardTo(undefined); setPage(1); }} + offboardFrom={offboardFrom} + offboardTo={offboardTo} + onOffboardApply={(from, to) => { setOffboardFrom(from); setOffboardTo(to); setPage(1); }} + onOffboardClear={() => { setOffboardFrom(undefined); setOffboardTo(undefined); setPage(1); }} /> {hasAnyConnection && (
@@ -582,7 +553,9 @@ export function TeamMembersClient({ ONBOARDED OFFBOARDED - TASKS + {requirementColumns.map((col) => ( + {col.label} + ))} ACTIONS @@ -608,10 +581,12 @@ export function TeamMembersClient({ } backgroundCheckStepEnabled={backgroundCheckStepEnabled} twoFactorStatus={twoFactorStatusMap[(item as MemberWithUser).id]} + requirementColumns={requirementColumns.map((c) => c.key)} /> ) : ( void; - onClear: () => void; -}) { - const [open, setOpen] = useState(false); - const [draftFrom, setDraftFrom] = useState(from); - const [draftTo, setDraftTo] = useState(to); - const [activePreset, setActivePreset] = useState(null); - const [fromPickerOpen, setFromPickerOpen] = useState(false); - const [toPickerOpen, setToPickerOpen] = useState(false); - - const handleOpenChange = (isOpen: boolean) => { - if (isOpen) { - setDraftFrom(from); - setDraftTo(to); - setActivePreset(getActivePresetLabel(from, to)); - } - setOpen(isOpen); - }; - - const handlePreset = (days: number, presetLabel: string) => { - const range = getPresetRange(days); - setDraftFrom(range.from); - setDraftTo(range.to); - setActivePreset(presetLabel); - }; - - const handleApply = () => { - onApply(draftFrom, draftTo); - setOpen(false); - }; - - const handleClear = () => { - onClear(); - setOpen(false); - }; - - const displayLabel = from && to - ? `${format(from, 'MMM d')} – ${format(to, 'MMM d, yyyy')}` - : from - ? `From ${format(from, 'MMM d, yyyy')}` - : to - ? `Until ${format(to, 'MMM d, yyyy')}` - : 'Any time'; - - const labelId = `people-${label.toLowerCase()}-filter-label`; - - return ( -
- - {label} - - - -
- - {displayLabel} - -
- - -
- - {label} between - - -
- {PRESETS.map((p) => ( - - ))} -
- -
- - -
- - {draftFrom ? format(draftFrom, 'MMM d, yyyy') : Start date} -
- - - { setDraftFrom(d ?? undefined); setActivePreset(null); setFromPickerOpen(false); }} - captionLayout="dropdown" - fromYear={2000} - toYear={new Date().getFullYear() + 1} - /> - - - → - - -
- - {draftTo ? format(draftTo, 'MMM d, yyyy') : End date} -
- - - { setDraftTo(d ?? undefined); setActivePreset(null); setToPickerOpen(false); }} - captionLayout="dropdown" - fromYear={2000} - toYear={new Date().getFullYear() + 1} - /> - - -
- -
-
- -
-
- -
-
-
- - -
- ); -} From dc2be7b92cf7498f5bd9760dfe3b9d0246f1ea1d Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 2 Jul 2026 17:04:27 -0400 Subject: [PATCH 19/33] feat(people): removable active-filter chips + always-visible table scrollbar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Applied filters render as chips beside the Filters button (Status: Deactivated x) — visible and one-click clearable without reopening the popover - DS table scroll containers get a slim always-visible horizontal scrollbar: macOS overlay scrollbars gave no hint that more requirement columns exist to the right Co-Authored-By: Claude Opus 4.8 (1M context) --- .../all/components/PeopleFilters.test.tsx | 60 +++++++++++++++ .../people/all/components/PeopleFilters.tsx | 74 ++++++++++++++++++- apps/app/src/styles/globals.css | 19 +++++ 3 files changed, 150 insertions(+), 3 deletions(-) create mode 100644 apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.test.tsx diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.test.tsx new file mode 100644 index 0000000000..080ef372f0 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.test.tsx @@ -0,0 +1,60 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { PeopleFilters } from './PeopleFilters'; + +const noop = vi.fn(); + +function renderFilters(overrides: Partial[0]> = {}) { + return render( + , + ); +} + +describe('PeopleFilters', () => { + it('shows no count badge or chips when nothing is filtered', () => { + renderFilters(); + expect(screen.getByText('Filters')).toBeInTheDocument(); + expect(screen.queryByText(/Status:/)).not.toBeInTheDocument(); + }); + + it('shows the active count and a removable chip per applied filter', () => { + const onStatusChange = vi.fn(); + renderFilters({ + statusFilter: 'deactivated', + roleFilter: 'admin', + onStatusChange, + }); + + expect(screen.getByText('2')).toBeInTheDocument(); + expect(screen.getByText('Status: Deactivated')).toBeInTheDocument(); + expect(screen.getByText('Role: Admin')).toBeInTheDocument(); + + fireEvent.click(screen.getByLabelText('Remove filter: Status: Deactivated')); + expect(onStatusChange).toHaveBeenCalledWith(undefined); + }); + + it('shows a date chip that clears via its remove button', () => { + const onOnboardClear = vi.fn(); + renderFilters({ onboardFrom: new Date('2026-06-01'), onOnboardClear }); + + const chip = screen.getByText(/Onboarded from/); + expect(chip).toBeInTheDocument(); + fireEvent.click(screen.getByLabelText(/Remove filter: Onboarded/)); + expect(onOnboardClear).toHaveBeenCalled(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx index 22d54e87e7..a67651eb3a 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx @@ -12,10 +12,49 @@ import { SelectTrigger, SelectValue, } from '@trycompai/design-system'; -import { Filter } from '@trycompai/design-system/icons'; +import { Close, Filter } from '@trycompai/design-system/icons'; + +import { format } from 'date-fns'; import { DateRangeFilter } from './DateRangeFilter'; +const STATUS_LABELS: Record = { + all: 'All People', + active: 'Active', + pending: 'Pending', + deactivated: 'Deactivated', +}; +const ROLE_LABELS: Record = { + owner: 'Owner', + admin: 'Admin', + auditor: 'Auditor', + employee: 'Employee', + contractor: 'Contractor', +}; + +function rangeLabel(from: Date | undefined, to: Date | undefined): string { + if (from && to) return `${format(from, 'MMM d')} – ${format(to, 'MMM d')}`; + if (from) return `from ${format(from, 'MMM d')}`; + return `until ${format(to as Date, 'MMM d')}`; +} + +/** Removable chip for one applied filter — clearable without opening the popover. */ +function FilterChip({ label, onRemove }: { label: string; onRemove: () => void }) { + return ( + + {label} + + + ); +} + interface PeopleFiltersProps { statusFilter: string; hasOffboardFilter: boolean; @@ -60,7 +99,8 @@ export function PeopleFilters({ ].filter(Boolean).length; return ( - +
+
- + + + {/* Applied filters as removable chips — visible + one-click clearable + without reopening the popover. */} + {statusFilter && ( + onStatusChange(undefined)} + /> + )} + {roleFilter && ( + onRoleChange('all')} + /> + )} + {(onboardFrom || onboardTo) && ( + + )} + {(offboardFrom || offboardTo) && ( + + )} +
); } diff --git a/apps/app/src/styles/globals.css b/apps/app/src/styles/globals.css index 3fd9e4858f..529a0d1132 100644 --- a/apps/app/src/styles/globals.css +++ b/apps/app/src/styles/globals.css @@ -129,3 +129,22 @@ body { -webkit-text-fill-color: transparent; animation: thinking-shimmer 3.5s ease-in-out infinite; } + +/* Data tables scroll horizontally inside their own container (DS Table wraps + rows in [data-slot='table-scroll']). macOS overlay scrollbars are invisible + until scrolled, so wide tables (e.g. People requirement columns) gave no hint + that more columns exist — force a slim, always-visible horizontal scrollbar. */ +[data-slot='table-scroll'] { + scrollbar-width: thin; + scrollbar-color: hsl(var(--border)) transparent; +} +[data-slot='table-scroll']::-webkit-scrollbar { + height: 8px; +} +[data-slot='table-scroll']::-webkit-scrollbar-thumb { + background: hsl(var(--border)); + border-radius: 9999px; +} +[data-slot='table-scroll']::-webkit-scrollbar-track { + background: transparent; +} From 5ac1099cc0835a1038a967f8ded0361a472e36ac Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 2 Jul 2026 17:05:11 -0400 Subject: [PATCH 20/33] fix(people): match filter-change handlers to the DS Select signature Co-Authored-By: Claude Opus 4.8 (1M context) --- .../[orgId]/people/all/components/PeopleFilters.test.tsx | 2 +- .../(app)/[orgId]/people/all/components/PeopleFilters.tsx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.test.tsx index 080ef372f0..06a83b4744 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.test.tsx @@ -45,7 +45,7 @@ describe('PeopleFilters', () => { expect(screen.getByText('Role: Admin')).toBeInTheDocument(); fireEvent.click(screen.getByLabelText('Remove filter: Status: Deactivated')); - expect(onStatusChange).toHaveBeenCalledWith(undefined); + expect(onStatusChange).toHaveBeenCalledWith(null); }); it('shows a date chip that clears via its remove button', () => { diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx index a67651eb3a..1ce3e3a0e7 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx @@ -58,9 +58,9 @@ function FilterChip({ label, onRemove }: { label: string; onRemove: () => void } interface PeopleFiltersProps { statusFilter: string; hasOffboardFilter: boolean; - onStatusChange: (value: string | undefined) => void; + onStatusChange: (value: string | null) => void; roleFilter: string; - onRoleChange: (value: string | undefined) => void; + onRoleChange: (value: string | null) => void; onboardFrom: Date | undefined; onboardTo: Date | undefined; onOnboardApply: (from: Date | undefined, to: Date | undefined) => void; @@ -178,7 +178,7 @@ export function PeopleFilters({ {statusFilter && ( onStatusChange(undefined)} + onRemove={() => onStatusChange(null)} /> )} {roleFilter && ( From 9e3cab127499fdaa53e8ad49ca80bfdb976e9ddc Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 2 Jul 2026 17:09:09 -0400 Subject: [PATCH 21/33] fix(people): address cubic review on columns + filters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Requirement column visibility now derives from ORG-LEVEL tracking (policies exist / training flag / HIPAA framework), never from member data — columns can't vanish for orgs with no compliance members yet - Filters trigger no longer nests a button inside PopoverTrigger's button (styled div, same pattern as the date chips) - Nested date pickers reset when the parent popover closes - Unbounded ranges map to the exact 'All time' preset label so the selected preset highlights on reopen Co-Authored-By: Claude Opus 4.8 (1M context) --- .../people/all/components/DateRangeFilter.tsx | 6 +++- .../people/all/components/PeopleFilters.tsx | 8 +++-- .../people/all/components/TeamMembers.tsx | 29 +++++++++++++------ .../all/components/TeamMembersClient.tsx | 14 ++++----- 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/DateRangeFilter.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/DateRangeFilter.tsx index 8e59f7ddfb..c97dbf6e8f 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/DateRangeFilter.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/DateRangeFilter.tsx @@ -29,7 +29,8 @@ function getPresetRange(days: number): { from: Date | undefined; to: Date | unde } function getActivePresetLabel(from: Date | undefined, to: Date | undefined): string | null { - if (!from && !to) return 'Any time'; + // Must be the exact PRESETS label so the chip renders as selected. + if (!from && !to) return 'All time'; if (!from || !to) return null; const diffDays = Math.round((to.getTime() - from.getTime()) / (1000 * 60 * 60 * 24)); const now = new Date(); @@ -68,6 +69,9 @@ export function DateRangeFilter({ setDraftTo(to); setActivePreset(getActivePresetLabel(from, to)); } + // Nested pickers must never stay open across parent close/reopen. + setFromPickerOpen(false); + setToPickerOpen(false); setOpen(isOpen); }; diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx index 1ce3e3a0e7..174e7409c1 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx @@ -2,7 +2,6 @@ import { Badge, - Button, Popover, PopoverContent, PopoverTrigger, @@ -101,11 +100,14 @@ export function PeopleFilters({ return (
+ {/* PopoverTrigger renders its own +
diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembers.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembers.tsx index 94234a3163..3bb7d9bcb9 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembers.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembers.tsx @@ -111,16 +111,26 @@ export async function TeamMembers(props: TeamMembersProps) { const backgroundCheckStepEnabled = orgFlags?.backgroundCheckStepEnabled === true; const hasHipaaFramework = !!hipaaInstance; - if (employeeMembers.length > 0) { + const policies = await db.policy.findMany({ + where: { + organizationId, + isRequiredToSign: true, + status: 'published', + isArchived: false, + }, + }); + + // Column visibility must come from ORG-LEVEL tracking, not from member data: + // an org with tracking enabled but no compliance members yet still shows the + // columns (as dashes), never silently hides them. + const requirementTracking = { + policies: policies.length > 0, + training: + orgFlags?.securityTrainingStepEnabled === true && trainingVideosData.length > 0, + hipaa: hasHipaaFramework, + }; - const policies = await db.policy.findMany({ - where: { - organizationId, - isRequiredToSign: true, - status: 'published', - isArchived: false, - }, - }); + if (employeeMembers.length > 0) { const employeeIds = employeeMembers.map((m) => m.id); const trainingCompletions = orgFlags?.securityTrainingStepEnabled @@ -181,6 +191,7 @@ export async function TeamMembers(props: TeamMembersProps) { complianceMemberIds={complianceMemberIds} backgroundCheckStepEnabled={backgroundCheckStepEnabled} twoFactorStatusMap={twoFactorStatusMap} + requirementTracking={requirementTracking} /> ); } 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 9e197e58d6..e03945084b 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 @@ -72,6 +72,8 @@ interface TeamMembersClientProps { complianceMemberIds: string[]; backgroundCheckStepEnabled: boolean; twoFactorStatusMap: Record; + /** Org-level tracking flags — column visibility never depends on member data. */ + requirementTracking: { policies: boolean; training: boolean; hipaa: boolean }; } export function TeamMembersClient({ @@ -84,6 +86,7 @@ export function TeamMembersClient({ complianceMemberIds, backgroundCheckStepEnabled, twoFactorStatusMap, + requirementTracking, }: TeamMembersClientProps) { const { agentDevices, isLoading: isAgentDevicesLoading } = useAgentDevices(); const { fleetHosts, isLoading: isFleetHostsLoading } = useFleetHosts(); @@ -100,20 +103,17 @@ export function TeamMembersClient({ const requirementColumns = useMemo< Array<{ key: RequirementColumnKey; label: string }> >(() => { - const completions = Object.values(taskCompletionMap); const cols: Array<{ key: RequirementColumnKey; label: string }> = []; - if (completions.some((c) => c.policies.total > 0)) - cols.push({ key: 'policies', label: 'POLICIES' }); - if (completions.some((c) => c.training.total > 0)) - cols.push({ key: 'training', label: 'TRAINING' }); - if (completions.some((c) => !!c.hipaa)) cols.push({ key: 'hipaa', label: 'HIPAA' }); + if (requirementTracking.policies) cols.push({ key: 'policies', label: 'POLICIES' }); + if (requirementTracking.training) cols.push({ key: 'training', label: 'TRAINING' }); + if (requirementTracking.hipaa) cols.push({ key: 'hipaa', label: 'HIPAA' }); if (complianceMemberIds.length > 0) cols.push({ key: 'device', label: 'DEVICE' }); if (backgroundCheckStepEnabled) cols.push({ key: 'background', label: 'BACKGROUND' }); if (Object.keys(twoFactorStatusMap).length > 0) cols.push({ key: 'twoFactor', label: '2FA' }); return cols; - }, [taskCompletionMap, complianceMemberIds, backgroundCheckStepEnabled, twoFactorStatusMap]); + }, [requirementTracking, complianceMemberIds, backgroundCheckStepEnabled, twoFactorStatusMap]); const router = useRouter(); const [searchQuery, setSearchQuery] = useState(''); const [roleFilter, setRoleFilter] = useState(''); From 111cced86565bc3cb5a60bb63c4f26cbae0e5680 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 3 Jul 2026 13:28:22 -0400 Subject: [PATCH 22/33] style(people): move source selects into a Sources popover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sync-people-from and 2FA-status-from are settings, not filters — they now live behind a compact Sources button symmetric with Filters, where their labels have room. Toolbar: Search - Filters - Sources. Sources are also reachable on mobile now (they were sm+-only inline). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../all/components/TeamMembersClient.tsx | 23 ++++++++++++++++--- .../TwoFactorSourceSelector.test.tsx | 4 ++-- .../components/TwoFactorSourceSelector.tsx | 10 ++++---- 3 files changed, 26 insertions(+), 11 deletions(-) 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 e03945084b..9f1f3ca77a 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 @@ -21,6 +21,9 @@ import { InputGroup, InputGroupAddon, InputGroupInput, + Popover, + PopoverContent, + PopoverTrigger, Select, SelectContent, SelectItem, @@ -34,7 +37,7 @@ import { TableHeader, TableRow, } from '@trycompai/design-system'; -import { InProgress, Search } from '@trycompai/design-system/icons'; +import { InProgress, Search, SettingsAdjust } from '@trycompai/design-system/icons'; import { apiClient } from '@/lib/api-client'; import { useMemo } from 'react'; @@ -360,9 +363,20 @@ export function TeamMembersClient({ onOffboardApply={(from, to) => { setOffboardFrom(from); setOffboardTo(to); setPage(1); }} onOffboardClear={() => { setOffboardFrom(undefined); setOffboardTo(undefined); setPage(1); }} /> + {/* Source settings (sync / 2FA) — settings, not filters, so they get + their own compact popover, symmetric with the Filters button. */} + + +
+ + Sources +
+ + +
{hasAnyConnection && ( -
-
+
+
Sync people from @@ -515,6 +529,9 @@ export function TeamMembersClient({
)} +
+ +
{/* Table */} diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx index 1f9fc88985..214b34c56a 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx @@ -76,12 +76,12 @@ describe('TwoFactorSourceSelector — RBAC gating', () => { ); }); - it('collapses on mobile like the other secondary toolbar controls (hidden sm:block)', () => { + it('fills the Sources popover width (visible at every breakpoint inside it)', () => { mockHasPermission.mockReturnValue(true); const { container } = render(); - expect(container.firstElementChild).toHaveClass('hidden', 'sm:flex'); + expect(container.firstElementChild).toHaveClass('flex', 'w-full'); }); it('renders nothing while the source/selection are still loading (no placeholder flash)', () => { diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx index d1831e7ed7..f15a1fec10 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx @@ -56,12 +56,10 @@ export function TwoFactorSourceSelector() { }; return ( - // hidden sm:flex matches the other toolbar controls: config actions - // collapse on phones, where the toolbar row doesn't wrap. The per-member - // 2FA status stays visible at every width via the table's own horizontal - // scroll. Label above the input, uniform with the rest of the toolbar; - // aria-labelledby names the combobox (the role ignores content for names). -
+ // Renders inside the toolbar's Sources popover, so it fills the popover + // width and stays reachable at every breakpoint. aria-labelledby names the + // combobox (the role ignores content for accessible names). +
2FA status from From f9de116e3e14687c10d45ba11ed064c5712dc046 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 3 Jul 2026 13:32:00 -0400 Subject: [PATCH 23/33] style(people): rename Sources button to Data sources Co-Authored-By: Claude Opus 4.8 (1M context) --- .../(app)/[orgId]/people/all/components/TeamMembersClient.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9f1f3ca77a..a8a19973ec 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 @@ -369,7 +369,7 @@ export function TeamMembersClient({
- Sources + Data sources
From 87068c5cdaa2e74790fa0d21c93ddd994f0dea7c Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 3 Jul 2026 13:35:53 -0400 Subject: [PATCH 24/33] style(people): empty state with CTA in the Data sources popover A fresh org with no connected integrations saw an empty popover. Teach instead: explain what data sources do and deep-link to the Integrations page. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../all/components/TeamMembersClient.tsx | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 a8a19973ec..202d418d1d 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 @@ -1,6 +1,7 @@ 'use client'; import Image from 'next/image'; +import Link from 'next/link'; import { useRouter } from 'next/navigation'; import { useState } from 'react'; import { toast } from 'sonner'; @@ -374,6 +375,23 @@ export function TeamMembersClient({
+ {/* Fresh orgs have nothing to configure yet — teach instead of + showing an empty popover. */} + {!hasAnyConnection && ( +
+ No integrations connected + + Connect an integration like Google Workspace to sync your + team automatically and show each person's 2FA status. + + + Browse integrations → + +
+ )} {hasAnyConnection && (
From 3fdfeedd77e0c1ddd41da68c9b310832eaaf8a0f Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 3 Jul 2026 13:42:08 -0400 Subject: [PATCH 25/33] style(people): simpler, general empty-state copy in Data sources Co-Authored-By: Claude Opus 4.8 (1M context) --- .../(app)/[orgId]/people/all/components/TeamMembersClient.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 202d418d1d..956e2d8dd5 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 @@ -381,8 +381,7 @@ export function TeamMembersClient({
No integrations connected - Connect an integration like Google Workspace to sync your - team automatically and show each person's 2FA status. + Data sources appear here once you connect an integration. Date: Fri, 3 Jul 2026 13:48:23 -0400 Subject: [PATCH 26/33] style(people): labeled empty slots in Data sources instead of prose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The empty state now renders the REAL structure — 'Sync people from' and '2FA status from' as labeled placeholder slots with a dashed Connect row deep-linking to Integrations — so users see exactly what can appear here instead of reading about it. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../all/components/TeamMembersClient.tsx | 28 ++++++++----------- .../TwoFactorSourceSelector.test.tsx | 16 +++++++++-- .../components/TwoFactorSourceSelector.tsx | 20 ++++++++++++- 3 files changed, 44 insertions(+), 20 deletions(-) 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 956e2d8dd5..6d23732f83 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 @@ -375,22 +375,18 @@ export function TeamMembersClient({
- {/* Fresh orgs have nothing to configure yet — teach instead of - showing an empty popover. */} - {!hasAnyConnection && ( -
- No integrations connected - - Data sources appear here once you connect an integration. - - - Browse integrations → - -
- )} + {!hasAnyConnection && ( +
+ Sync people from + + Connect an integration + → + +
+ )} {hasAnyConnection && (
diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx index 214b34c56a..b4ecce5a74 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx @@ -8,6 +8,12 @@ const { mockHasPermission, mockUse2faSource } = vi.hoisted(() => ({ mockUse2faSource: vi.fn(), })); +vi.mock('next/link', () => ({ + default: ({ children, href, ...rest }: { children: React.ReactNode; href: string }) => ( + {children} + ), +})); + vi.mock('next/navigation', () => ({ useParams: () => ({ orgId: 'org_1' }), useRouter: () => ({ refresh: vi.fn() }), @@ -98,7 +104,7 @@ describe('TwoFactorSourceSelector — RBAC gating', () => { expect(container).toBeEmptyDOMElement(); }); - it('renders nothing when no bound integration is connected', () => { + it('shows a labeled connect prompt when no bound integration is connected', () => { mockHasPermission.mockReturnValue(true); mockUse2faSource.mockReturnValue({ selectedSource: null, @@ -108,7 +114,11 @@ describe('TwoFactorSourceSelector — RBAC gating', () => { hasAnyConnection: false, }); - const { container } = render(); - expect(container).toBeEmptyDOMElement(); + render(); + expect(screen.getByText('2FA status from')).toBeInTheDocument(); + expect(screen.getByText('Connect an integration')).toHaveAttribute( + 'href', + '/org_1/integrations', + ); }); }); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx index f15a1fec10..8a4db91ad7 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx @@ -1,6 +1,7 @@ 'use client'; import Image from 'next/image'; +import Link from 'next/link'; import { useParams, useRouter } from 'next/navigation'; import { usePermissions } from '@/hooks/use-permissions'; @@ -38,10 +39,27 @@ export function TwoFactorSourceSelector() { // Wait for BOTH the available sources and the current selection before // rendering, so the trigger never flashes the placeholder while the saved // selection is still resolving. - if (!canManage || isLoading || !hasAnyConnection) { + if (!canManage || isLoading) { return null; } + // Empty slot instead of nothing: the labeled placeholder shows exactly what + // this setting is and how to unlock it. + if (!hasAnyConnection) { + return ( +
+ 2FA status from + + Connect an integration + → + +
+ ); + } + const connectedSources = availableSources.filter((p) => p.connected); const selected = connectedSources.find((p) => p.slug === selectedSource); From 389c6c6540dc11ce31c2ef0b525492da818e7c67 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 3 Jul 2026 13:52:53 -0400 Subject: [PATCH 27/33] style(people): query controls left, Data sources right MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Search + Filters (query) group left; Data sources (view configuration) sits alone on the right — the Linear/Stripe toolbar convention. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../[orgId]/people/all/components/TeamMembersClient.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 6d23732f83..2e77125988 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 @@ -330,7 +330,10 @@ export function TeamMembersClient({ return ( {/* Search and Filters */} -
+ {/* Left = query (search, filters); right = view configuration (data + sources) — the Linear/Stripe toolbar convention. */} +
+
@@ -364,6 +367,7 @@ export function TeamMembersClient({ onOffboardApply={(from, to) => { setOffboardFrom(from); setOffboardTo(to); setPage(1); }} onOffboardClear={() => { setOffboardFrom(undefined); setOffboardTo(undefined); setPage(1); }} /> +
{/* Source settings (sync / 2FA) — settings, not filters, so they get their own compact popover, symmetric with the Filters button. */} From 1d1246a3540e368e452b2cc2dd6a4b0acf682b58 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 3 Jul 2026 14:08:12 -0400 Subject: [PATCH 28/33] style(people): rename Data sources button to Sync settings Co-Authored-By: Claude Opus 4.8 (1M context) --- .../(app)/[orgId]/people/all/components/TeamMembersClient.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 2e77125988..fb1fa75d75 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 @@ -374,7 +374,7 @@ export function TeamMembersClient({
- Data sources + Sync settings
From aeb707edaca859670bac5504597d63c7c2d7478f Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 3 Jul 2026 14:27:46 -0400 Subject: [PATCH 29/33] fix(people): inline select dropdowns inside toolbar popovers Selects portaled outside their parent popover broke outside-click dismissal: closing an open select left the popover needing two more clicks to dismiss. Render SelectContent with portal={false} inside the Sync settings and Filters popovers so the dismiss layering stays sane. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../app/(app)/[orgId]/people/all/components/PeopleFilters.tsx | 4 ++-- .../(app)/[orgId]/people/all/components/TeamMembersClient.tsx | 2 +- .../people/all/components/TwoFactorSourceSelector.test.tsx | 4 +++- .../[orgId]/people/all/components/TwoFactorSourceSelector.tsx | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx index 174e7409c1..860f88f5c3 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx @@ -125,7 +125,7 @@ export function PeopleFilters({ ] ?? 'Active')} - + All People Active Pending @@ -146,7 +146,7 @@ export function PeopleFilters({ ] ?? 'All Roles'} - + All Roles Owner Admin 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 fb1fa75d75..7df2183c82 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 @@ -433,7 +433,7 @@ export function TeamMembersClient({ None )} - +
{selectedProvider ? ( <> diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx index b4ecce5a74..2abc8e7eaf 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.test.tsx @@ -58,7 +58,9 @@ describe('TwoFactorSourceSelector — RBAC gating', () => { render(); - expect(screen.getByText('Google Workspace')).toBeInTheDocument(); + // portal={false} keeps the inline option list mounted, so the name can + // appear in both the trigger and the (hidden) list. + expect(screen.getAllByText('Google Workspace').length).toBeGreaterThan(0); // The inline "2FA status from ·" prefix tells users what the control is for and // makes it part of the trigger's accessible name for screen readers. expect( diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx index 8a4db91ad7..b458b41422 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx @@ -106,7 +106,7 @@ export function TwoFactorSourceSelector() { None )} - +
Shows each person's 2FA status from the selected integration's latest check
From b1598df49e5380877959d0b0915a1b1667c4049e Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 3 Jul 2026 14:31:13 -0400 Subject: [PATCH 30/33] style(people): logos + tighter copy in the 2FA source dropdown Items now show the integration logo (matching the sync select and the trigger); the clear option is a muted 'None'; the helper line is short. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/TwoFactorSourceSelector.tsx | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx index b458b41422..8a33cb71a8 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx @@ -108,12 +108,26 @@ export function TwoFactorSourceSelector() {
- Shows each person's 2FA status from the selected integration's latest check + Where each person's 2FA status comes from
- Don't show 2FA status + + None + {connectedSources.map((p) => ( - {p.name} +
+ {p.logoUrl && ( + + )} + {p.name} +
))} From 8e568e9bbe0fc843d6cab264503b6b92617c12e5 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 3 Jul 2026 14:36:45 -0400 Subject: [PATCH 31/33] style(people): friendlier labels in Sync settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'Sync people from'/'2FA status from' -> 'People'/'2FA status' (the popover is already called Sync settings — key-value rows, no redundancy). Clear option is 'Don't show' (mirrors 'Don't auto-sync'); unset triggers read 'Not syncing'/'Not shown' instead of the technical 'None'. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../[orgId]/people/all/components/TeamMembersClient.tsx | 6 +++--- .../all/components/TwoFactorSourceSelector.test.tsx | 4 ++-- .../people/all/components/TwoFactorSourceSelector.tsx | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) 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 7df2183c82..08d9aa2d96 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 @@ -381,7 +381,7 @@ export function TeamMembersClient({
{!hasAnyConnection && (
- Sync people from + People
- Sync people from + People {selected.name}
) : ( - None + Not shown )} @@ -111,7 +111,7 @@ export function TwoFactorSourceSelector() { Where each person's 2FA status comes from
- None + Not shown {connectedSources.map((p) => ( From 339f5f45ff48bb9e4c82ddef391e532f9e6a8685 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 3 Jul 2026 14:59:06 -0400 Subject: [PATCH 32/33] revert(people): portal={false} on in-popover selects broke positioning Inline (non-portaled) select dropdowns mis-position inside the popover and stretch the page width on mousedown. Restore the portal; the minor extra-click-to-dismiss quirk returns and needs a proper fix at the design-system layer instead. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../app/(app)/[orgId]/people/all/components/PeopleFilters.tsx | 4 ++-- .../(app)/[orgId]/people/all/components/TeamMembersClient.tsx | 2 +- .../[orgId]/people/all/components/TwoFactorSourceSelector.tsx | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx index 860f88f5c3..174e7409c1 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/PeopleFilters.tsx @@ -125,7 +125,7 @@ export function PeopleFilters({ ] ?? 'Active')} - + All People Active Pending @@ -146,7 +146,7 @@ export function PeopleFilters({ ] ?? 'All Roles'} - + All Roles Owner Admin 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 08d9aa2d96..5c2da67817 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 @@ -433,7 +433,7 @@ export function TeamMembersClient({ Not syncing )} - +
{selectedProvider ? ( <> diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx index 3db610f179..3949a3191b 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx @@ -106,7 +106,7 @@ export function TwoFactorSourceSelector() { Not shown )} - +
Where each person's 2FA status comes from
From 4c4492d7bf30cf24626a13fd67f7cd6bb44df48d Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 3 Jul 2026 15:14:45 -0400 Subject: [PATCH 33/33] fix(people): make the 2FA select uncontrolled like the sync select The people-sync select (uncontrolled) dismisses cleanly inside the popover; the 2FA select (controlled via SWR-backed value) re-rendered mid-dismissal and ate the popover's next outside click. Mirror the working pattern: no value prop, manual 'Active' marker on the selected item, and no focus-revalidation on the source hooks. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../people/all/components/TwoFactorSourceSelector.tsx | 10 ++++++++-- .../app/(app)/[orgId]/people/all/hooks/use2faSource.ts | 2 ++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx index 3949a3191b..f3fc70b7d0 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TwoFactorSourceSelector.tsx @@ -81,10 +81,13 @@ export function TwoFactorSourceSelector() { 2FA status + {/* Uncontrolled on purpose — mirrors the (working) people-sync select. + A controlled value re-renders mid-dismissal when SWR revalidates, + which breaks the parent popover's outside-click tracking. */}