diff --git a/apps/framework-editor/app/(pages)/controls/ControlsClientPage.tsx b/apps/framework-editor/app/(pages)/controls/ControlsClientPage.tsx index f4f5d17172..326df586d5 100644 --- a/apps/framework-editor/app/(pages)/controls/ControlsClientPage.tsx +++ b/apps/framework-editor/app/(pages)/controls/ControlsClientPage.tsx @@ -41,21 +41,37 @@ async function fetchAllPolicyTemplates(): Promise { return apiClient('/policy-template'); } +function toRequirementItem(r: RequirementApiItem): RelationalItem { + let displayName = r.identifier; + if (r.identifier && r.name) { + displayName = `${r.identifier} - ${r.name}`; + } else if (r.name) { + displayName = r.name; + } + return { + id: r.id, + name: displayName || 'Unnamed Requirement', + sublabel: r.framework?.name, + }; +} + async function fetchAllRequirements(): Promise { const reqs = await apiClient('/requirement'); - return reqs.map((r) => { - let displayName = r.identifier; - if (r.identifier && r.name) { - displayName = `${r.identifier} - ${r.name}`; - } else if (r.name) { - displayName = r.name; - } - return { - id: r.id, - name: displayName || 'Unnamed Requirement', - sublabel: r.framework?.name, - }; - }); + return reqs.map(toRequirementItem); +} + +// On a framework's Controls tab only this framework's requirements are +// linkable — links to them are what makes a control show up on the tab. +async function fetchRequirementsForFramework( + frameworkId: string, +): Promise { + const framework = await apiClient<{ + name: string; + requirements: Array<{ id: string; name: string; identifier: string }>; + }>(`/framework/${frameworkId}`); + return framework.requirements.map((r) => + toRequirementItem({ ...r, framework: { name: framework.name } }), + ); } async function fetchAllTaskTemplates(): Promise { @@ -115,6 +131,17 @@ export function ControlsClientPage({ initialControls, emptyMessage, frameworkId apiClient(`/control-template/${id}`, { method: 'DELETE', }), + linkRequirement: (controlId: string, requirementId: string) => + linkControlRelation(controlId, 'requirements', requirementId), + // Policy/task links are framework-scoped; only offered on a framework tab. + ...(frameworkId + ? { + linkPolicyTemplate: (controlId: string, policyTemplateId: string) => + linkControlRelation(controlId, 'policy-templates', policyTemplateId, frameworkId), + linkTaskTemplate: (controlId: string, taskTemplateId: string) => + linkControlRelation(controlId, 'task-templates', taskTemplateId, frameworkId), + } + : {}), }), [frameworkId], ); @@ -157,7 +184,9 @@ export function ControlsClientPage({ initialControls, emptyMessage, frameworkId isDirty, createdIds, changesSummary, - } = useChangeTracking(initialGridData, mutations); + } = useChangeTracking(initialGridData, mutations, { + requireRequirementLink: !!frameworkId, + }); const { families, @@ -175,6 +204,12 @@ export function ControlsClientPage({ initialControls, emptyMessage, frameworkId [updateCell], ); + const getRequirementItems = useCallback( + () => + frameworkId ? fetchRequirementsForFramework(frameworkId) : fetchAllRequirements(), + [frameworkId], + ); + const columns = useMemo( () => [ columnHelper.accessor('name', { @@ -226,6 +261,7 @@ export function ControlsClientPage({ initialControls, emptyMessage, frameworkId items={getValue()} rowId={row.original.id} isNewRow={createdIds.has(row.original.id)} + allowSelectOnNewRows={!!frameworkId} getAllItems={fetchAllPolicyTemplates} onLink={(controlId: string, ptId: string) => linkControlRelation(controlId, 'policy-templates', ptId, frameworkId) @@ -252,7 +288,8 @@ export function ControlsClientPage({ initialControls, emptyMessage, frameworkId items={getValue()} rowId={row.original.id} isNewRow={createdIds.has(row.original.id)} - getAllItems={fetchAllRequirements} + allowSelectOnNewRows + getAllItems={getRequirementItems} onLink={(controlId: string, reqId: string) => linkControlRelation(controlId, 'requirements', reqId) } @@ -278,6 +315,7 @@ export function ControlsClientPage({ initialControls, emptyMessage, frameworkId items={getValue()} rowId={row.original.id} isNewRow={createdIds.has(row.original.id)} + allowSelectOnNewRows={!!frameworkId} getAllItems={fetchAllTaskTemplates} onLink={(controlId: string, ttId: string) => linkControlRelation(controlId, 'task-templates', ttId, frameworkId) @@ -346,7 +384,7 @@ export function ControlsClientPage({ initialControls, emptyMessage, frameworkId ), }), ], - [uniqueFamilies, updateCell, updateRelational, deleteRow, createdIds, handleDocumentTypesUpdate, frameworkId], + [uniqueFamilies, updateCell, updateRelational, deleteRow, createdIds, handleDocumentTypesUpdate, frameworkId, getRequirementItems], ); const [sorting, setSorting] = useState([]); diff --git a/apps/framework-editor/app/(pages)/controls/hooks/useChangeTracking.test.tsx b/apps/framework-editor/app/(pages)/controls/hooks/useChangeTracking.test.tsx new file mode 100644 index 0000000000..c981130b3a --- /dev/null +++ b/apps/framework-editor/app/(pages)/controls/hooks/useChangeTracking.test.tsx @@ -0,0 +1,187 @@ +import { act, renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { ControlsPageGridData } from '../types'; +import { useChangeTracking, type ControlMutations } from './useChangeTracking'; + +const { refreshMock, toastMock } = vi.hoisted(() => ({ + refreshMock: vi.fn(), + toastMock: { success: vi.fn(), error: vi.fn() }, +})); + +vi.mock('next/navigation', () => ({ + useRouter: () => ({ refresh: refreshMock }), +})); +vi.mock('sonner', () => ({ toast: toastMock })); + +// Stable reference: the hook resets its state whenever the initialData +// identity changes, so a fresh [] per render would loop forever. +const NO_ROWS: ControlsPageGridData[] = []; + +function makeRow(overrides: Partial = {}): ControlsPageGridData { + return { + id: 'temp-1', + name: 'New Control', + description: '', + controlFamily: 'Access Control', + policyTemplates: [], + requirements: [], + taskTemplates: [], + documentTypes: [], + policyTemplatesLength: 0, + requirementsLength: 0, + taskTemplatesLength: 0, + documentTypesLength: 0, + createdAt: new Date(), + updatedAt: new Date(), + ...overrides, + }; +} + +function makeMutations(): ControlMutations { + return { + createControl: vi.fn(async () => ({ id: 'frk_ct_new' })), + updateControl: vi.fn(async () => ({})), + deleteControl: vi.fn(async () => ({})), + linkRequirement: vi.fn(async () => ({})), + linkPolicyTemplate: vi.fn(async () => ({})), + linkTaskTemplate: vi.fn(async () => ({})), + }; +} + +describe('useChangeTracking handleCommit', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('blocks creating an unlinked control when requireRequirementLink is set', async () => { + const mutations = makeMutations(); + const { result } = renderHook(() => + useChangeTracking(NO_ROWS, mutations, { requireRequirementLink: true }), + ); + + act(() => { + result.current.addRow(makeRow()); + }); + await act(async () => { + await result.current.handleCommit(); + }); + + expect(mutations.createControl).not.toHaveBeenCalled(); + expect(toastMock.error).toHaveBeenCalledWith( + 'Some operations failed', + expect.objectContaining({ + description: expect.stringContaining('link at least one requirement'), + }), + ); + // The row stays in the grid so the user can fix it instead of losing it. + expect(result.current.isDirty).toBe(true); + expect(result.current.data).toHaveLength(1); + expect(refreshMock).not.toHaveBeenCalled(); + }); + + it('creates and links requirements, policies and tasks picked on the new row', async () => { + const mutations = makeMutations(); + const { result } = renderHook(() => + useChangeTracking(NO_ROWS, mutations, { requireRequirementLink: true }), + ); + + act(() => { + result.current.addRow( + makeRow({ + requirements: [{ id: 'req_1', name: '10.3 - Access Review' }], + requirementsLength: 1, + policyTemplates: [{ id: 'pol_1', name: 'Access Policy' }], + policyTemplatesLength: 1, + taskTemplates: [{ id: 'task_1', name: 'Review accounts' }], + taskTemplatesLength: 1, + }), + ); + }); + await act(async () => { + await result.current.handleCommit(); + }); + + expect(mutations.createControl).toHaveBeenCalledWith({ + name: 'New Control', + description: '', + controlFamily: 'Access Control', + documentTypes: [], + }); + expect(mutations.linkRequirement).toHaveBeenCalledWith('frk_ct_new', 'req_1'); + expect(mutations.linkPolicyTemplate).toHaveBeenCalledWith('frk_ct_new', 'pol_1'); + expect(mutations.linkTaskTemplate).toHaveBeenCalledWith('frk_ct_new', 'task_1'); + // Links picked before commit stay visible after the id swap. + expect(result.current.data[0].id).toBe('frk_ct_new'); + expect(result.current.data[0].requirements).toHaveLength(1); + expect(result.current.isDirty).toBe(false); + expect(refreshMock).toHaveBeenCalled(); + }); + + it('allows unlinked controls when requireRequirementLink is not set (global page)', async () => { + const mutations = makeMutations(); + const { result } = renderHook(() => useChangeTracking(NO_ROWS, mutations)); + + act(() => { + result.current.addRow(makeRow()); + }); + await act(async () => { + await result.current.handleCommit(); + }); + + expect(mutations.createControl).toHaveBeenCalled(); + expect(result.current.isDirty).toBe(false); + expect(refreshMock).toHaveBeenCalled(); + }); + + it('reports an error instead of silently skipping rows without a name', async () => { + const mutations = makeMutations(); + const { result } = renderHook(() => useChangeTracking(NO_ROWS, mutations)); + + act(() => { + result.current.addRow(makeRow({ name: '' })); + }); + await act(async () => { + await result.current.handleCommit(); + }); + + expect(mutations.createControl).not.toHaveBeenCalled(); + expect(toastMock.error).toHaveBeenCalledWith( + 'Some operations failed', + expect.objectContaining({ + description: expect.stringContaining('name is required'), + }), + ); + expect(result.current.isDirty).toBe(true); + }); + + it('surfaces link failures after a successful create', async () => { + const mutations = makeMutations(); + mutations.linkRequirement = vi.fn(async () => { + throw new Error('link failed'); + }); + const { result } = renderHook(() => + useChangeTracking(NO_ROWS, mutations, { requireRequirementLink: true }), + ); + + act(() => { + result.current.addRow( + makeRow({ + requirements: [{ id: 'req_1', name: '10.3 - Access Review' }], + requirementsLength: 1, + }), + ); + }); + await act(async () => { + await result.current.handleCommit(); + }); + + expect(result.current.data[0].id).toBe('frk_ct_new'); + expect(toastMock.error).toHaveBeenCalledWith( + 'Some operations failed', + expect.objectContaining({ + description: expect.stringContaining('failed to link'), + }), + ); + expect(refreshMock).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/framework-editor/app/(pages)/controls/hooks/useChangeTracking.ts b/apps/framework-editor/app/(pages)/controls/hooks/useChangeTracking.ts index 3b987eb1a7..0cf0d3dacd 100644 --- a/apps/framework-editor/app/(pages)/controls/hooks/useChangeTracking.ts +++ b/apps/framework-editor/app/(pages)/controls/hooks/useChangeTracking.ts @@ -1,3 +1,5 @@ +import { useUnsavedChangesGuard } from '@/app/lib/unsaved-changes'; +import { useRouter } from 'next/navigation'; import { useCallback, useEffect, useMemo, useState } from 'react'; import { toast } from 'sonner'; import type { ControlsPageGridData } from '../types'; @@ -16,12 +18,29 @@ export interface ControlMutations { data: { name: string; description: string; controlFamily: string | null; documentTypes: string[] }, ) => Promise; deleteControl: (id: string) => Promise; + /** Persist links picked on uncommitted rows. Optional per page context. */ + linkRequirement?: (controlId: string, requirementId: string) => Promise; + linkPolicyTemplate?: (controlId: string, policyTemplateId: string) => Promise; + linkTaskTemplate?: (controlId: string, taskTemplateId: string) => Promise; +} + +export interface ChangeTrackingOptions { + /** + * On a framework's Controls tab the listing only shows controls linked to + * one of the framework's requirements — committing an unlinked control + * would make it silently vanish from the tab. When set, new rows must + * have at least one requirement link before they can be committed. + */ + requireRequirementLink?: boolean; } export const useChangeTracking = ( initialData: ControlsPageGridData[], mutations: ControlMutations, + options: ChangeTrackingOptions = {}, ) => { + const router = useRouter(); + const { requireRequirementLink = false } = options; const [data, setData] = useState(() => initialData); const [prevData, setPrevData] = useState(() => initialData); @@ -179,7 +198,17 @@ export const useChangeTracking = ( for (const tempId of createdIds) { const row = currentData.find((r) => r.id === tempId); - if (!row?.name) continue; + if (!row) continue; + if (!row.name?.trim()) { + results.errors.push('New control: name is required'); + continue; + } + if (requireRequirementLink && row.requirements.length === 0) { + results.errors.push( + `"${row.name}": link at least one requirement (Linked Requirements column) — controls only appear on this framework's page through their requirement links`, + ); + continue; + } try { const newControl = await mutations.createControl({ @@ -188,21 +217,39 @@ export const useChangeTracking = ( controlFamily: row.controlFamily, documentTypes: row.documentTypes, }); - results.successes.push(`Created: ${row.name}`); + + // Persist links the user picked on the uncommitted row. + const failedLinks: string[] = []; + const linkSelections = [ + { items: row.requirements, link: mutations.linkRequirement, noun: 'requirement' }, + { items: row.policyTemplates, link: mutations.linkPolicyTemplate, noun: 'policy' }, + { items: row.taskTemplates, link: mutations.linkTaskTemplate, noun: 'task' }, + ]; + for (const { items, link, noun } of linkSelections) { + for (const item of items) { + if (!link) { + failedLinks.push(`${noun} "${item.name}"`); + continue; + } + try { + await link(newControl.id, item.id); + } catch { + failedLinks.push(`${noun} "${item.name}"`); + } + } + } + + if (failedLinks.length > 0) { + results.errors.push( + `Created "${row.name}" but failed to link: ${failedLinks.join(', ')}`, + ); + } else { + results.successes.push(`Created: ${row.name}`); + } successfullyCreatedIds.add(tempId); setData((prev) => - prev.map((r) => - r.id === tempId - ? { - ...r, - id: newControl.id, - policyTemplates: [], - requirements: [], - taskTemplates: [], - } - : r, - ), + prev.map((r) => (r.id === tempId ? { ...r, id: newControl.id } : r)), ); } catch (error) { results.errors.push( @@ -275,11 +322,22 @@ export const useChangeTracking = ( toast.success('Changes saved', { description: `${results.successes.length} operation(s) completed`, }); + // Re-sync the grid with server truth (ids, timestamps, links). + router.refresh(); } } finally { setIsCommitting(false); } - }, [data, createdIds, updatedIds, deletedIds, mutations, isCommitting]); + }, [ + data, + createdIds, + updatedIds, + deletedIds, + mutations, + isCommitting, + requireRequirementLink, + router, + ]); const handleCancel = useCallback(() => { if (isCommitting) return; @@ -293,6 +351,10 @@ export const useChangeTracking = ( return createdIds.size > 0 || updatedIds.size > 0 || deletedIds.size > 0; }, [createdIds, updatedIds, deletedIds]); + // Uncommitted rows only live in this grid's state — warn before they are + // discarded by a reload or a tab switch. + useUnsavedChangesGuard('controls-grid', isDirty); + const changesSummary = useMemo(() => { const total = createdIds.size + updatedIds.size + deletedIds.size; if (total === 0) return ''; diff --git a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.tsx b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.tsx index a20650d1e1..d842b1b353 100644 --- a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.tsx +++ b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.tsx @@ -174,6 +174,7 @@ export function FrameworkRequirementsClientPage({ items={getValue()} rowId={row.original.id} isNewRow={createdIds.has(row.original.id)} + allowSelectOnNewRows getAllItems={fetchAllControlTemplates} onLink={linkControlToRequirement} onUnlink={unlinkControlFromRequirement} diff --git a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkTabs.tsx b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkTabs.tsx index 253b83a37c..5a6d27c0da 100644 --- a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkTabs.tsx +++ b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkTabs.tsx @@ -1,5 +1,6 @@ 'use client'; +import { confirmDiscardUnsavedChanges } from '@/app/lib/unsaved-changes'; import { Tabs, TabsList, TabsTrigger } from '@trycompai/ui'; import Link from 'next/link'; import { useParams, useSelectedLayoutSegment } from 'next/navigation'; @@ -29,7 +30,14 @@ export function FrameworkTabs() { className="flex-1" asChild > - {tab.name} + { + if (!confirmDiscardUnsavedChanges()) event.preventDefault(); + }} + > + {tab.name} + ))} diff --git a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/hooks/useRequirementChangeTracking.test.tsx b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/hooks/useRequirementChangeTracking.test.tsx new file mode 100644 index 0000000000..9f91bc66ff --- /dev/null +++ b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/hooks/useRequirementChangeTracking.test.tsx @@ -0,0 +1,166 @@ +import { act, renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + useRequirementChangeTracking, + type RequirementGridRow, +} from './useRequirementChangeTracking'; + +const { apiClientMock, refreshMock, toastMock } = vi.hoisted(() => ({ + apiClientMock: vi.fn(), + refreshMock: vi.fn(), + toastMock: { success: vi.fn(), error: vi.fn() }, +})); + +vi.mock('@/app/lib/api-client', () => ({ apiClient: apiClientMock })); +vi.mock('next/navigation', () => ({ + useRouter: () => ({ refresh: refreshMock }), +})); +vi.mock('sonner', () => ({ toast: toastMock })); + +// Stable reference: the hook resets its state whenever the initialData +// identity changes, so a fresh [] per render would loop forever. +const NO_ROWS: RequirementGridRow[] = []; + +function makeRow(overrides: Partial = {}): RequirementGridRow { + return { + id: 'temp-1', + name: 'New Requirement', + identifier: '10.3', + description: '', + requirementFamily: 'Access Control', + controlTemplates: [], + controlTemplatesLength: 0, + createdAt: new Date(), + updatedAt: new Date(), + ...overrides, + }; +} + +describe('useRequirementChangeTracking handleCommit', () => { + beforeEach(() => { + vi.clearAllMocks(); + apiClientMock.mockImplementation(async (path: string) => { + if (path === '/requirement') return { id: 'frk_rq_new' }; + return {}; + }); + }); + + it('creates a new requirement and clears dirty state', async () => { + const { result } = renderHook(() => useRequirementChangeTracking(NO_ROWS, 'frk_1')); + + act(() => { + result.current.addRow(makeRow()); + }); + expect(result.current.isDirty).toBe(true); + + await act(async () => { + await result.current.handleCommit(); + }); + + expect(apiClientMock).toHaveBeenCalledWith( + '/requirement', + expect.objectContaining({ method: 'POST' }), + ); + const createCall = apiClientMock.mock.calls.find((call) => call[0] === '/requirement'); + expect(JSON.parse(createCall![1].body)).toMatchObject({ + frameworkId: 'frk_1', + name: 'New Requirement', + identifier: '10.3', + requirementFamily: 'Access Control', + }); + expect(result.current.data[0].id).toBe('frk_rq_new'); + expect(result.current.isDirty).toBe(false); + expect(toastMock.success).toHaveBeenCalled(); + expect(refreshMock).toHaveBeenCalled(); + }); + + it('persists control links picked on the uncommitted row', async () => { + const { result } = renderHook(() => useRequirementChangeTracking(NO_ROWS, 'frk_1')); + + act(() => { + result.current.addRow( + makeRow({ + controlTemplates: [{ id: 'ct_1', name: 'Control 1' }], + controlTemplatesLength: 1, + }), + ); + }); + await act(async () => { + await result.current.handleCommit(); + }); + + expect(apiClientMock).toHaveBeenCalledWith( + '/control-template/ct_1/requirements/frk_rq_new', + { method: 'POST' }, + ); + expect(toastMock.error).not.toHaveBeenCalled(); + expect(refreshMock).toHaveBeenCalled(); + }); + + it('reports an error instead of silently skipping rows without a name', async () => { + const { result } = renderHook(() => useRequirementChangeTracking(NO_ROWS, 'frk_1')); + + act(() => { + result.current.addRow(makeRow({ name: '' })); + }); + await act(async () => { + await result.current.handleCommit(); + }); + + expect(apiClientMock).not.toHaveBeenCalled(); + expect(toastMock.error).toHaveBeenCalledWith( + 'Some operations failed', + expect.objectContaining({ + description: expect.stringContaining('name is required'), + }), + ); + expect(result.current.isDirty).toBe(true); + expect(refreshMock).not.toHaveBeenCalled(); + }); + + it('keeps failed rows dirty and does not refresh', async () => { + apiClientMock.mockRejectedValue(new Error('boom')); + const { result } = renderHook(() => useRequirementChangeTracking(NO_ROWS, 'frk_1')); + + act(() => { + result.current.addRow(makeRow()); + }); + await act(async () => { + await result.current.handleCommit(); + }); + + expect(toastMock.error).toHaveBeenCalled(); + expect(result.current.isDirty).toBe(true); + expect(refreshMock).not.toHaveBeenCalled(); + }); + + it('surfaces link failures after a successful create', async () => { + apiClientMock.mockImplementation(async (path: string) => { + if (path === '/requirement') return { id: 'frk_rq_new' }; + throw new Error('link failed'); + }); + const { result } = renderHook(() => useRequirementChangeTracking(NO_ROWS, 'frk_1')); + + act(() => { + result.current.addRow( + makeRow({ + controlTemplates: [{ id: 'ct_1', name: 'Control 1' }], + controlTemplatesLength: 1, + }), + ); + }); + await act(async () => { + await result.current.handleCommit(); + }); + + // The requirement exists (id swapped in) but the user is told links failed. + expect(result.current.data[0].id).toBe('frk_rq_new'); + expect(toastMock.error).toHaveBeenCalledWith( + 'Some operations failed', + expect.objectContaining({ + description: expect.stringContaining('failed to link'), + }), + ); + expect(refreshMock).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/hooks/useRequirementChangeTracking.ts b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/hooks/useRequirementChangeTracking.ts index 6bc3aa5cf9..3d699e390d 100644 --- a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/hooks/useRequirementChangeTracking.ts +++ b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/hooks/useRequirementChangeTracking.ts @@ -1,4 +1,6 @@ import { apiClient } from '@/app/lib/api-client'; +import { useUnsavedChangesGuard } from '@/app/lib/unsaved-changes'; +import { useRouter } from 'next/navigation'; import { useCallback, useEffect, useMemo, useState } from 'react'; import { toast } from 'sonner'; @@ -20,6 +22,7 @@ export function useRequirementChangeTracking( initialData: RequirementGridRow[], frameworkId: string, ) { + const router = useRouter(); const [data, setData] = useState(() => initialData); const [prevData, setPrevData] = useState(() => initialData); @@ -123,7 +126,13 @@ export function useRequirementChangeTracking( for (const tempId of createdIds) { const row = currentData.find((r) => r.id === tempId); - if (!row?.name) continue; + if (!row) continue; + if (!row.name?.trim()) { + results.errors.push( + `New requirement${row.identifier ? ` "${row.identifier}"` : ''}: name is required`, + ); + continue; + } try { const created = await apiClient<{ id: string }>('/requirement', { method: 'POST', @@ -135,7 +144,25 @@ export function useRequirementChangeTracking( requirementFamily: row.requirementFamily ?? undefined, }), }); - results.successes.push(`Created: ${row.name}`); + // Persist control links the user picked on the uncommitted row. + const failedLinks: string[] = []; + for (const controlTemplate of row.controlTemplates) { + try { + await apiClient( + `/control-template/${controlTemplate.id}/requirements/${created.id}`, + { method: 'POST' }, + ); + } catch { + failedLinks.push(controlTemplate.name); + } + } + if (failedLinks.length > 0) { + results.errors.push( + `Created "${row.name}" but failed to link control(s): ${failedLinks.join(', ')}`, + ); + } else { + results.successes.push(`Created: ${row.name}`); + } okCreated.add(tempId); setData((prev) => prev.map((r) => (r.id === tempId ? { ...r, id: created.id } : r)), @@ -226,8 +253,10 @@ export function useRequirementChangeTracking( toast.success('Changes saved', { description: `${results.successes.length} operation(s) completed`, }); + // Re-sync the grid with server truth (ids, timestamps, links). + router.refresh(); } - }, [data, createdIds, updatedIds, deletedIds, frameworkId]); + }, [data, createdIds, updatedIds, deletedIds, frameworkId, router]); const handleCancel = useCallback(() => { setData(prevData); @@ -241,6 +270,10 @@ export function useRequirementChangeTracking( [createdIds, updatedIds, deletedIds], ); + // Uncommitted rows only live in this grid's state — warn before they are + // discarded by a reload or a tab switch. + useUnsavedChangesGuard('framework-requirements-grid', isDirty); + const changesSummary = useMemo(() => { const total = createdIds.size + updatedIds.size + deletedIds.size; if (total === 0) return ''; diff --git a/apps/framework-editor/app/components/AddExistingItemDialog.tsx b/apps/framework-editor/app/components/AddExistingItemDialog.tsx index 66a9b0bd7c..786f56da4e 100644 --- a/apps/framework-editor/app/components/AddExistingItemDialog.tsx +++ b/apps/framework-editor/app/components/AddExistingItemDialog.tsx @@ -58,6 +58,23 @@ interface AddExistingItemDialogProps { fetchAllItems: () => Promise; } +// apiClient throws Error(); pull the NestJS `message` +// field out so the user sees e.g. "Framework has no requirements to link +// the control to" instead of a generic failure. +function extractApiErrorMessage(error: unknown): string | null { + if (!(error instanceof Error) || !error.message) return null; + try { + const parsed: unknown = JSON.parse(error.message); + if (parsed && typeof parsed === 'object' && 'message' in parsed) { + const message = (parsed as { message?: unknown }).message; + if (typeof message === 'string') return message; + } + } catch { + // Not JSON — fall through to the raw message. + } + return error.message; +} + function extractFrameworkNames(item: ExistingItemRaw): string[] { const names = new Set(); @@ -142,8 +159,11 @@ export function AddExistingItemDialog({ setLinkedIds((prev) => new Set(prev).add(item.id)); toast.success(`${config.label} "${item.name}" linked successfully`); router.refresh(); - } catch { - toast.error(`Failed to link ${config.label.toLowerCase()}`); + } catch (error) { + toast.error( + extractApiErrorMessage(error) ?? + `Failed to link ${config.label.toLowerCase()}`, + ); } finally { setLinkingId(null); } diff --git a/apps/framework-editor/app/components/table/RelationalCell.test.tsx b/apps/framework-editor/app/components/table/RelationalCell.test.tsx new file mode 100644 index 0000000000..7efbf39b59 --- /dev/null +++ b/apps/framework-editor/app/components/table/RelationalCell.test.tsx @@ -0,0 +1,88 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { RelationalCell } from './RelationalCell'; + +const { toastMock } = vi.hoisted(() => ({ + toastMock: { success: vi.fn(), error: vi.fn() }, +})); +vi.mock('sonner', () => ({ toast: toastMock })); + +// The ui package ships untranspiled JSX in dist; the cell only needs Button. +vi.mock('@trycompai/ui', () => ({ + Button: ({ + children, + variant: _variant, + size: _size, + ...props + }: { variant?: string; size?: string } & React.ComponentProps<'button'>) => ( + + ), +})); + +function renderCell(props: Partial[0]> = {}) { + const onLink = vi.fn(async () => {}); + const onUnlink = vi.fn(async () => {}); + const onLocalUpdate = vi.fn(); + const getAllItems = vi.fn(async () => [{ id: 'item_1', name: 'Item One' }]); + render( + , + ); + return { onLink, onUnlink, onLocalUpdate, getAllItems }; +} + +describe('RelationalCell on uncommitted rows', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('blocks link selection by default', () => { + renderCell(); + fireEvent.click(screen.getByText('None')); + expect(screen.getByText('Save row first to link items')).toBeDefined(); + expect(screen.queryByRole('button', { name: /add control/i })).toBeNull(); + }); + + it('allows local link selection with allowSelectOnNewRows', async () => { + const { onLink, onLocalUpdate, getAllItems } = renderCell({ + allowSelectOnNewRows: true, + }); + + fireEvent.click(screen.getByText('None')); + expect(screen.queryByText('Save row first to link items')).toBeNull(); + + fireEvent.click(screen.getByRole('button', { name: /add control/i })); + expect(getAllItems).toHaveBeenCalled(); + + fireEvent.click(await screen.findByText('Item One')); + + // New rows collect links locally; nothing is persisted until commit. + expect(onLink).not.toHaveBeenCalled(); + expect(onLocalUpdate).toHaveBeenCalledWith([{ id: 'item_1', name: 'Item One' }]); + expect(toastMock.success).toHaveBeenCalledWith('Control will be linked when you commit'); + }); + + it('keeps immediate linking for already-saved rows', async () => { + const { onLink, onLocalUpdate } = renderCell({ isNewRow: false }); + + fireEvent.click(screen.getByText('None')); + fireEvent.click(screen.getByRole('button', { name: /add control/i })); + fireEvent.click(await screen.findByText('Item One')); + + expect(onLink).toHaveBeenCalledWith('row-1', 'item_1'); + // onLocalUpdate runs after the awaited onLink resolves. + await waitFor(() => + expect(onLocalUpdate).toHaveBeenCalledWith([{ id: 'item_1', name: 'Item One' }]), + ); + }); +}); diff --git a/apps/framework-editor/app/components/table/RelationalCell.tsx b/apps/framework-editor/app/components/table/RelationalCell.tsx index c89e3d4098..b29d5fc33d 100644 --- a/apps/framework-editor/app/components/table/RelationalCell.tsx +++ b/apps/framework-editor/app/components/table/RelationalCell.tsx @@ -15,6 +15,8 @@ interface RelationalCellProps { items: RelationalItem[]; rowId: string; isNewRow: boolean; + /** Let uncommitted rows pick links locally; they are persisted on commit. */ + allowSelectOnNewRows?: boolean; getAllItems: () => Promise; onLink: (parentId: string, itemId: string) => Promise; onUnlink: (parentId: string, itemId: string) => Promise; @@ -27,6 +29,7 @@ export function RelationalCell({ items, rowId, isNewRow, + allowSelectOnNewRows = false, getAllItems, onLink, onUnlink, @@ -183,7 +186,7 @@ export function RelationalCell({ {/* Add section */} - {!isNewRow && ( + {(!isNewRow || allowSelectOnNewRows) && (
{isSearching ? ( <> @@ -235,7 +238,7 @@ export function RelationalCell({
)} - {isNewRow && ( + {isNewRow && !allowSelectOnNewRows && (
Save row first to link items
diff --git a/apps/framework-editor/app/lib/unsaved-changes.test.tsx b/apps/framework-editor/app/lib/unsaved-changes.test.tsx new file mode 100644 index 0000000000..2270832412 --- /dev/null +++ b/apps/framework-editor/app/lib/unsaved-changes.test.tsx @@ -0,0 +1,45 @@ +import { renderHook } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + confirmDiscardUnsavedChanges, + hasUnsavedChanges, + useUnsavedChangesGuard, +} from './unsaved-changes'; + +describe('unsaved-changes guard', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('tracks dirty state while mounted and releases on cleanup', () => { + const { rerender, unmount } = renderHook( + ({ dirty }: { dirty: boolean }) => useUnsavedChangesGuard('grid-a', dirty), + { initialProps: { dirty: false } }, + ); + expect(hasUnsavedChanges()).toBe(false); + + rerender({ dirty: true }); + expect(hasUnsavedChanges()).toBe(true); + + rerender({ dirty: false }); + expect(hasUnsavedChanges()).toBe(false); + + rerender({ dirty: true }); + unmount(); + expect(hasUnsavedChanges()).toBe(false); + }); + + it('confirmDiscardUnsavedChanges passes through when clean and prompts when dirty', () => { + const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(false); + expect(confirmDiscardUnsavedChanges()).toBe(true); + expect(confirmSpy).not.toHaveBeenCalled(); + + const { unmount } = renderHook(() => useUnsavedChangesGuard('grid-b', true)); + expect(confirmDiscardUnsavedChanges()).toBe(false); + expect(confirmSpy).toHaveBeenCalledOnce(); + + confirmSpy.mockReturnValue(true); + expect(confirmDiscardUnsavedChanges()).toBe(true); + unmount(); + }); +}); diff --git a/apps/framework-editor/app/lib/unsaved-changes.ts b/apps/framework-editor/app/lib/unsaved-changes.ts new file mode 100644 index 0000000000..28caf85df2 --- /dev/null +++ b/apps/framework-editor/app/lib/unsaved-changes.ts @@ -0,0 +1,42 @@ +'use client'; + +import { useEffect } from 'react'; + +// Module-level registry of grids that currently hold uncommitted changes. +// Navigation chrome (e.g. FrameworkTabs) consults it before letting the user +// leave, since leaving a page discards all uncommitted grid rows. +const dirtyKeys = new Set(); + +export const UNSAVED_CHANGES_MESSAGE = + 'You have uncommitted changes that will be lost if you leave. Click "Commit Changes" first, or leave anyway?'; + +export function hasUnsavedChanges(): boolean { + return dirtyKeys.size > 0; +} + +export function confirmDiscardUnsavedChanges(): boolean { + if (!hasUnsavedChanges()) return true; + return window.confirm(UNSAVED_CHANGES_MESSAGE); +} + +/** + * Registers a grid's dirty state while mounted and warns on hard + * reload/close via beforeunload. In-app navigation is guarded by callers of + * confirmDiscardUnsavedChanges() (e.g. FrameworkTabs). + */ +export function useUnsavedChangesGuard(key: string, isDirty: boolean) { + useEffect(() => { + if (!isDirty) return; + dirtyKeys.add(key); + const handleBeforeUnload = (event: BeforeUnloadEvent) => { + event.preventDefault(); + // Chrome still requires returnValue to be set to show the prompt. + event.returnValue = ''; + }; + window.addEventListener('beforeunload', handleBeforeUnload); + return () => { + dirtyKeys.delete(key); + window.removeEventListener('beforeunload', handleBeforeUnload); + }; + }, [key, isDirty]); +} diff --git a/apps/framework-editor/package.json b/apps/framework-editor/package.json index 72457f1fd6..9d1736d060 100644 --- a/apps/framework-editor/package.json +++ b/apps/framework-editor/package.json @@ -30,14 +30,17 @@ "zod": "^3.24.2" }, "devDependencies": { + "@testing-library/react": "^16.3.0", "@types/node": "^22.15.0", "@types/react": "^19.1.6", "@types/react-dom": "^19.1.1", "autoprefixer": "^10.4.21", + "jsdom": "^26.1.0", "postcss": "^8.5.4", "prisma": "7.6.0", "tailwindcss": "^4.1.8", - "typescript": "^5.8.3" + "typescript": "^5.8.3", + "vitest": "^3.2.4" }, "private": true, "scripts": { @@ -47,6 +50,7 @@ "lint": "echo 'no lint configured'", "prebuild": "bun run db:generate", "start": "next start", + "test": "vitest run", "typecheck": "tsc --noEmit" }, "type": "module" diff --git a/apps/framework-editor/vitest.config.ts b/apps/framework-editor/vitest.config.ts new file mode 100644 index 0000000000..dca56bdfde --- /dev/null +++ b/apps/framework-editor/vitest.config.ts @@ -0,0 +1,19 @@ +import { resolve } from 'path'; +import { defineConfig } from 'vitest/config'; + +// No @vitejs/plugin-react on purpose: esbuild already transforms TSX with the +// automatic JSX runtime, and the plugin's vite Plugin type clashes with +// vitest's bundled vite during Next's build-time typecheck on Vercel. +export default defineConfig({ + test: { + environment: 'jsdom', + setupFiles: ['./vitest.setup.ts'], + include: ['app/**/*.{test,spec}.{ts,tsx}'], + exclude: ['node_modules', '.next'], + }, + resolve: { + alias: { + '@': resolve(__dirname, './'), + }, + }, +}); diff --git a/apps/framework-editor/vitest.setup.ts b/apps/framework-editor/vitest.setup.ts new file mode 100644 index 0000000000..878063b678 --- /dev/null +++ b/apps/framework-editor/vitest.setup.ts @@ -0,0 +1,7 @@ +import { cleanup } from '@testing-library/react'; +import { afterEach } from 'vitest'; + +// Without test globals RTL cannot register its own afterEach cleanup. +afterEach(() => { + cleanup(); +}); diff --git a/bun.lock b/bun.lock index f2f0be905f..3d06bfdff4 100644 --- a/bun.lock +++ b/bun.lock @@ -430,14 +430,17 @@ "zod": "^3.24.2", }, "devDependencies": { + "@testing-library/react": "^16.3.0", "@types/node": "^22.15.0", "@types/react": "^19.1.6", "@types/react-dom": "^19.1.1", "autoprefixer": "^10.4.21", + "jsdom": "^26.1.0", "postcss": "^8.5.4", "prisma": "7.6.0", "tailwindcss": "^4.1.8", "typescript": "^5.8.3", + "vitest": "^3.2.4", }, }, "apps/portal": {