From 4cf2142cf67172d7aa457a58e6761b50adb7e0c1 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 16 Jun 2026 13:02:02 -0400 Subject: [PATCH 1/2] fix(framework-editor): allow deleting a framework that has versions/instances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a framework that had published versions and/or org instances failed with a 409 ("referenced by existing framework instances"). The delete only removed requirements + the framework, so the FrameworkVersion / FrameworkInstance FK references remained — and cascading versions + instances together trips the Restrict FK on FrameworkInstance.currentVersionId (P2003). Delete the dependency graph in explicit order inside the transaction: instances (cascades their org controls/maps/links/sync-operations and frees the currentVersion FK) -> versions -> requirements -> the framework (cascades the editor-side control/policy/task/document links + ISMS docs). Also makes the editor delete confirmation honest about the blast radius (it now removes published versions and any org instances too). The controller is PlatformAdminGuard-only, so this stays an internal-admin operation. Closes FRAME-13 Co-Authored-By: Claude Opus 4.8 --- .../framework/framework.service.spec.ts | 67 ++++++++++++++++++- .../framework/framework.service.ts | 16 ++++- .../components/DeleteFrameworkDialog.tsx | 5 +- 3 files changed, 84 insertions(+), 4 deletions(-) diff --git a/apps/api/src/framework-editor/framework/framework.service.spec.ts b/apps/api/src/framework-editor/framework/framework.service.spec.ts index 706c7916b6..8f5f173916 100644 --- a/apps/api/src/framework-editor/framework/framework.service.spec.ts +++ b/apps/api/src/framework-editor/framework/framework.service.spec.ts @@ -2,19 +2,28 @@ jest.mock('@db', () => { const dbMock = { frameworkEditorFramework: { findUnique: jest.fn(), + delete: jest.fn(), }, frameworkEditorRequirement: { findMany: jest.fn(), + deleteMany: jest.fn(), }, frameworkEditorControlTemplate: { update: jest.fn(), }, + frameworkInstance: { + deleteMany: jest.fn(), + }, + frameworkVersion: { + deleteMany: jest.fn(), + }, + $transaction: jest.fn((ops: unknown[]) => Promise.all(ops)), }; return { db: dbMock, Prisma: { PrismaClientKnownRequestError: class {} } }; }); import { BadRequestException, ConflictException } from '@nestjs/common'; -import { db } from '@db'; +import { db, Prisma } from '@db'; import { FrameworkEditorFrameworkService } from './framework.service'; const mockDb = db as jest.Mocked; @@ -87,3 +96,59 @@ describe('FrameworkEditorFrameworkService.linkControl', () => { expect(mockDb.frameworkEditorControlTemplate.update).not.toHaveBeenCalled(); }); }); + +describe('FrameworkEditorFrameworkService.delete (FRAME-13)', () => { + let service: FrameworkEditorFrameworkService; + + beforeEach(() => { + service = new FrameworkEditorFrameworkService(); + jest.clearAllMocks(); + (mockDb.frameworkEditorFramework.findUnique as jest.Mock).mockResolvedValue({ + id: 'frk_1', + }); + }); + + it('deletes instances, versions, requirements, then the framework — in that order', async () => { + const result = await service.delete('frk_1'); + + expect(result).toEqual({ message: 'Framework deleted successfully' }); + expect(mockDb.frameworkInstance.deleteMany).toHaveBeenCalledWith({ + where: { frameworkId: 'frk_1' }, + }); + expect(mockDb.frameworkVersion.deleteMany).toHaveBeenCalledWith({ + where: { frameworkId: 'frk_1' }, + }); + expect(mockDb.frameworkEditorRequirement.deleteMany).toHaveBeenCalledWith({ + where: { frameworkId: 'frk_1' }, + }); + expect(mockDb.frameworkEditorFramework.delete).toHaveBeenCalledWith({ + where: { id: 'frk_1' }, + }); + + // Order matters: instances free the currentVersion Restrict FK before + // versions are removed, and requirements before the framework itself. + const order = [ + (mockDb.frameworkInstance.deleteMany as jest.Mock).mock.invocationCallOrder[0], + (mockDb.frameworkVersion.deleteMany as jest.Mock).mock.invocationCallOrder[0], + (mockDb.frameworkEditorRequirement.deleteMany as jest.Mock).mock + .invocationCallOrder[0], + (mockDb.frameworkEditorFramework.delete as jest.Mock).mock.invocationCallOrder[0], + ]; + expect(order).toEqual([...order].sort((a, b) => a - b)); + }); + + it('maps a residual FK conflict (P2003) to a ConflictException', async () => { + // Runtime uses the mocked class (ignores ctor args); the args satisfy the + // real Prisma type, and Object.assign sets the code the catch checks. + const fkError = Object.assign( + new Prisma.PrismaClientKnownRequestError('FK constraint', { + code: 'P2003', + clientVersion: '0', + }), + { code: 'P2003' }, + ); + (mockDb.$transaction as jest.Mock).mockRejectedValueOnce(fkError); + + await expect(service.delete('frk_1')).rejects.toBeInstanceOf(ConflictException); + }); +}); diff --git a/apps/api/src/framework-editor/framework/framework.service.ts b/apps/api/src/framework-editor/framework/framework.service.ts index 82754c574c..434747d2c2 100644 --- a/apps/api/src/framework-editor/framework/framework.service.ts +++ b/apps/api/src/framework-editor/framework/framework.service.ts @@ -102,8 +102,22 @@ export class FrameworkEditorFrameworkService { async delete(id: string) { await this.findById(id); + // A framework may have published versions and org-level instances that + // reference it. Delete the dependency graph in order inside one transaction: + // 1. instances — cascades their org controls/maps/links/sync-operations + // AND frees the Restrict FK on + // FrameworkInstance.currentVersionId -> FrameworkVersion + // 2. versions — now unreferenced by instances or sync-operations + // 3. requirements — Restrict FK back to the framework + // 4. the framework — cascades the editor-side control/policy/task/document + // links + ISMS docs + // Deleting the framework directly would cascade versions and instances + // together, which trips the currentVersionId Restrict (P2003) depending on + // cascade order; the explicit ordering avoids that. try { await db.$transaction([ + db.frameworkInstance.deleteMany({ where: { frameworkId: id } }), + db.frameworkVersion.deleteMany({ where: { frameworkId: id } }), db.frameworkEditorRequirement.deleteMany({ where: { frameworkId: id }, }), @@ -115,7 +129,7 @@ export class FrameworkEditorFrameworkService { error.code === 'P2003' ) { throw new ConflictException( - 'Cannot delete framework: it is referenced by existing framework instances', + 'Could not delete framework: it still has references that could not be removed automatically', ); } throw error; diff --git a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/components/DeleteFrameworkDialog.tsx b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/components/DeleteFrameworkDialog.tsx index 99288902aa..7e0b89a5d5 100644 --- a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/components/DeleteFrameworkDialog.tsx +++ b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/components/DeleteFrameworkDialog.tsx @@ -65,8 +65,9 @@ export function DeleteFrameworkDialog({ Are you sure you want to delete {`"${frameworkName}"`}? - This action cannot be undone. This will permanently delete the framework and all of its - associated requirements. + This action cannot be undone. This will permanently delete the framework, all of its + requirements and published versions, and remove it from any organizations currently + tracking it. {error &&

Error: {error}

} From e06d44647362e33730ff36992038d207a6049343 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 16 Jun 2026 13:02:44 -0400 Subject: [PATCH 2/2] fix(framework-editor): commit changes independently of publishing a version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts the FRAME-4 "Save and Commit" flow: committing changes immediately prompted to publish a new version. Per Joe (FRAME-14), publishing is a separate, deliberate activity — editors commit in batches of 5-10 and do a full pass before publishing a meaningful version. This restores the single "Commit Changes" button (save edits to the live templates, no publish prompt). Publishing remains available as its own action on the framework's Versions tab. Reverts commit bb7064b8f. Closes FRAME-14 Co-Authored-By: Claude Opus 4.8 --- ...RequirementsClientPage.expandable.test.tsx | 6 -- ...orkRequirementsClientPage.toolbar.test.tsx | 101 ------------------ .../FrameworkRequirementsClientPage.tsx | 33 +----- .../hooks/useRequirementChangeTracking.ts | 4 - 4 files changed, 2 insertions(+), 142 deletions(-) delete mode 100644 apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.toolbar.test.tsx diff --git a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.expandable.test.tsx b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.expandable.test.tsx index ea4910bf86..51836f7cf1 100644 --- a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.expandable.test.tsx +++ b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.expandable.test.tsx @@ -22,12 +22,6 @@ vi.mock('../../../components/table', () => ({ vi.mock('./components/EditFrameworkDialog', () => ({ EditFrameworkDialog: () => null })); vi.mock('./components/DeleteFrameworkDialog', () => ({ DeleteFrameworkDialog: () => null })); -vi.mock('./versions/components/PublishVersionDialog', () => ({ - PublishVersionDialog: () => null, -})); -vi.mock('./versions/hooks/useFrameworkVersions', () => ({ - useFrameworkVersions: () => ({ data: [], refetch: vi.fn() }), -})); vi.mock('@/app/lib/api-client', () => ({ apiClient: vi.fn() })); vi.mock('next/navigation', () => ({ useRouter: () => ({ refresh: vi.fn(), push: vi.fn() }) })); vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); diff --git a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.toolbar.test.tsx b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.toolbar.test.tsx deleted file mode 100644 index 48b8bf20fc..0000000000 --- a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.toolbar.test.tsx +++ /dev/null @@ -1,101 +0,0 @@ -import { fireEvent, render, screen, waitFor } from '@testing-library/react'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; - -// Shared, hoisted handles so the mock factory and the assertions see the same -// references. publishProps records each render's `open` value. -const { handleCommit, handleCancel, publishProps } = vi.hoisted(() => ({ - handleCommit: vi.fn(async () => true), - handleCancel: vi.fn(), - publishProps: [] as Array<{ open: boolean }>, -})); - -vi.mock('../../../components/table', () => ({ - ComboboxCell: () => null, - DateCell: () => null, - RelationalCell: () => null, - EditableCell: () => null, -})); -vi.mock('./components/EditFrameworkDialog', () => ({ EditFrameworkDialog: () => null })); -vi.mock('./components/DeleteFrameworkDialog', () => ({ DeleteFrameworkDialog: () => null })); -vi.mock('./versions/components/PublishVersionDialog', () => ({ - PublishVersionDialog: (props: { open: boolean }) => { - publishProps.push({ open: props.open }); - return null; - }, -})); -vi.mock('./versions/hooks/useFrameworkVersions', () => ({ - useFrameworkVersions: () => ({ data: [{ version: '1.0.0' }], refetch: vi.fn() }), -})); -vi.mock('@/app/lib/api-client', () => ({ apiClient: vi.fn() })); -vi.mock('next/navigation', () => ({ useRouter: () => ({ refresh: vi.fn(), push: vi.fn() }) })); -vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); -vi.mock('@trycompai/ui', () => ({ - Button: ({ children, variant: _v, size: _s, ...props }: any) => ( - - ), -})); - -vi.mock('./hooks/useRequirementChangeTracking', () => ({ - simpleUUID: () => 'temp-id', - useRequirementChangeTracking: () => ({ - data: [], - updateCell: vi.fn(), - updateRelational: vi.fn(), - addRow: vi.fn(), - deleteRow: vi.fn(), - getRowClassName: () => '', - handleCommit, - handleCancel, - isDirty: true, - createdIds: new Set(), - changesSummary: '(2 changes)', - }), -})); - -import { FrameworkRequirementsClientPage } from './FrameworkRequirementsClientPage'; - -function renderPage() { - render( - , - ); -} - -describe('FrameworkRequirementsClientPage — Save as Draft / Save and Commit (FRAME-4)', () => { - beforeEach(() => { - vi.clearAllMocks(); - handleCommit.mockImplementation(async () => true); - publishProps.length = 0; - }); - - it('shows all three buttons when there are uncommitted changes', () => { - renderPage(); - expect(screen.getByRole('button', { name: 'Cancel' })).toBeTruthy(); - expect(screen.getByRole('button', { name: 'Save as Draft' })).toBeTruthy(); - expect(screen.getByRole('button', { name: 'Save and Commit' })).toBeTruthy(); - }); - - it('Save as Draft commits without opening the publish dialog', () => { - renderPage(); - fireEvent.click(screen.getByRole('button', { name: 'Save as Draft' })); - expect(handleCommit).toHaveBeenCalledTimes(1); - expect(publishProps.every((p) => p.open === false)).toBe(true); - }); - - it('Save and Commit saves then opens the publish dialog', async () => { - renderPage(); - fireEvent.click(screen.getByRole('button', { name: 'Save and Commit' })); - expect(handleCommit).toHaveBeenCalledTimes(1); - await waitFor(() => expect(publishProps.some((p) => p.open === true)).toBe(true)); - }); - - it('does not open the publish dialog when the save fails', async () => { - handleCommit.mockImplementation(async () => false); - renderPage(); - fireEvent.click(screen.getByRole('button', { name: 'Save and Commit' })); - await waitFor(() => expect(handleCommit).toHaveBeenCalled()); - expect(publishProps.every((p) => p.open === false)).toBe(true); - }); -}); diff --git a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.tsx b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.tsx index 43e752d9b0..da517c096c 100644 --- a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.tsx +++ b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/FrameworkRequirementsClientPage.tsx @@ -22,8 +22,6 @@ import { useRequirementChangeTracking, type RequirementGridRow, } from './hooks/useRequirementChangeTracking'; -import { PublishVersionDialog } from './versions/components/PublishVersionDialog'; -import { useFrameworkVersions } from './versions/hooks/useFrameworkVersions'; interface FrameworkDetails { id: string; @@ -78,12 +76,6 @@ export function FrameworkRequirementsClientPage({ // Row whose large description editor is currently open — highlighted so the // edited row is obvious behind the (semi-transparent) editor dialog. const [expandedRowId, setExpandedRowId] = useState(null); - // "Save and Commit" saves the edits then opens the publish flow (FRAME-4). - const [isPublishOpen, setIsPublishOpen] = useState(false); - const { data: publishedVersions, refetch: refetchVersions } = useFrameworkVersions( - frameworkDetails.id, - ); - const latestPublishedVersion = publishedVersions?.[0]?.version; const initialGridData: RequirementGridRow[] = useMemo( () => @@ -115,13 +107,6 @@ export function FrameworkRequirementsClientPage({ changesSummary, } = useRequirementChangeTracking(initialGridData, frameworkDetails.id); - // Save edits, then (only if they all persisted) open the publish dialog so - // the accumulated changes can be committed as a new version. - const handleSaveAndCommit = useCallback(async () => { - const ok = await handleCommit(); - if (ok) setIsPublishOpen(true); - }, [handleCommit]); - const uniqueFamilies = useMemo(() => { const families = new Set(); for (const row of data) { @@ -319,11 +304,8 @@ export function FrameworkRequirementsClientPage({ - - )} @@ -448,17 +430,6 @@ export function FrameworkRequirementsClientPage({ frameworkName={frameworkDetails.name} /> )} - setIsPublishOpen(false)} - latestVersion={latestPublishedVersion} - onPublished={() => { - setIsPublishOpen(false); - void refetchVersions(); - router.refresh(); - }} - /> ); } 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 ebf5d1213b..3d699e390d 100644 --- a/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/hooks/useRequirementChangeTracking.ts +++ b/apps/framework-editor/app/(pages)/frameworks/[frameworkId]/hooks/useRequirementChangeTracking.ts @@ -256,10 +256,6 @@ export function useRequirementChangeTracking( // Re-sync the grid with server truth (ids, timestamps, links). router.refresh(); } - - // Report success so callers (e.g. "Save and Commit") can chain a publish - // only when every edit persisted cleanly. - return results.errors.length === 0; }, [data, createdIds, updatedIds, deletedIds, frameworkId, router]); const handleCancel = useCallback(() => {