Skip to content

Commit b58c4d3

Browse files
github-actions[bot]Marfuenclaude
authored
fix(framework-editor): don't auto-link new task/policy templates to all framework controls (#2686)
When CX created a new task or policy template from a framework's editor view, the create endpoint queried every control template attached to that framework and connected the new primitive to all of them. CX rarely wants the new template on every control — it forced manual cleanup after every create. Make all primitives consistent: a fresh create produces an unlinked row regardless of which framework page the user happened to be on. Linking to specific controls happens explicitly via the dedicated link endpoints (POST /:id/control-templates/:ctId), which already existed. - task-template.service.ts: drop frameworkId param + the findMany + connect logic - policy-template.service.ts: same - task-template.controller.ts, policy-template.controller.ts: drop the @query('frameworkId') from the POST handler - frontend (useTaskChangeTracking, CreatePolicyDialog): stop appending ?frameworkId=... to the POST URL Control template and requirement creates were already correct (no auto-linking) — this brings task and policy templates in line. New regression tests on TaskTemplateService and PolicyTemplateService mirror the existing CS-271 guard on ControlTemplateService: even if a stray frameworkId is passed, the service must not query for or attach controls. Verified red without the fix, green with it. Co-authored-by: Mariano <marfuen98@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a10ecc3 commit b58c4d3

9 files changed

Lines changed: 190 additions & 43 deletions

File tree

‎apps/api/src/framework-editor/policy-template/policy-template.controller.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,8 @@ export class PolicyTemplateController {
4545
@Post()
4646
@ApiOperation({ summary: 'Create a policy template' })
4747
@UsePipes(new ValidationPipe({ whitelist: true, transform: true }))
48-
async create(
49-
@Body() dto: CreatePolicyTemplateDto,
50-
@Query('frameworkId') frameworkId?: string,
51-
) {
52-
return this.service.create(dto, frameworkId);
48+
async create(@Body() dto: CreatePolicyTemplateDto) {
49+
return this.service.create(dto);
5350
}
5451

5552
@Patch(':id')

‎apps/api/src/framework-editor/policy-template/policy-template.service.spec.ts‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
jest.mock('@db', () => ({
2+
db: {
3+
frameworkEditorPolicyTemplate: {
4+
create: jest.fn(),
5+
findUnique: jest.fn(),
6+
findMany: jest.fn(),
7+
update: jest.fn(),
8+
delete: jest.fn(),
9+
},
10+
frameworkEditorControlTemplate: {
11+
findMany: jest.fn(),
12+
},
13+
},
14+
Prisma: { PrismaClientKnownRequestError: class {} },
15+
}));
16+
17+
import { db } from '@db';
18+
import { PolicyTemplateService } from './policy-template.service';
19+
20+
const mockDb = db as jest.Mocked<typeof db>;
21+
22+
describe('PolicyTemplateService', () => {
23+
let service: PolicyTemplateService;
24+
25+
beforeEach(() => {
26+
service = new PolicyTemplateService();
27+
jest.clearAllMocks();
28+
(mockDb.frameworkEditorPolicyTemplate.create as jest.Mock).mockResolvedValue({
29+
id: 'frk_pt_new',
30+
name: 'New Policy',
31+
});
32+
});
33+
34+
describe('create', () => {
35+
const baseDto = {
36+
name: 'New Policy',
37+
description: 'desc',
38+
frequency: 'monthly',
39+
department: 'none',
40+
} as never;
41+
42+
// Regression test: previously, passing `frameworkId` caused the create
43+
// path to query every control template in the framework and auto-connect
44+
// the new policy to all of them. CX rarely wants that — the new policy
45+
// should start unlinked and be attached to specific controls explicitly
46+
// via the dedicated link endpoints. The `frameworkId` parameter is gone,
47+
// but a legacy caller passing one anyway must still produce an unlinked
48+
// row.
49+
it('never queries or auto-links framework controls on create, even when a stray frameworkId is passed', async () => {
50+
(mockDb.frameworkEditorControlTemplate.findMany as jest.Mock).mockResolvedValue([
51+
{ id: 'frk_ct_1' },
52+
{ id: 'frk_ct_2' },
53+
]);
54+
55+
// Bypass TypeScript so we can simulate a stray legacy caller still
56+
// passing frameworkId — the service must ignore it.
57+
await (service.create as (dto: unknown, frameworkId?: string) => Promise<unknown>)(
58+
baseDto,
59+
'frk_soc2',
60+
);
61+
62+
expect(mockDb.frameworkEditorControlTemplate.findMany).not.toHaveBeenCalled();
63+
const createArgs = (mockDb.frameworkEditorPolicyTemplate.create as jest.Mock).mock
64+
.calls[0][0];
65+
expect(createArgs.data).not.toHaveProperty('controlTemplates');
66+
});
67+
68+
it('persists name, description, frequency, department and an empty content blob', async () => {
69+
await service.create(baseDto);
70+
const createArgs = (mockDb.frameworkEditorPolicyTemplate.create as jest.Mock).mock
71+
.calls[0][0];
72+
expect(createArgs.data).toMatchObject({
73+
name: 'New Policy',
74+
description: 'desc',
75+
frequency: 'monthly',
76+
department: 'none',
77+
content: {},
78+
});
79+
});
80+
});
81+
});

‎apps/api/src/framework-editor/policy-template/policy-template.service.ts‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,26 +48,18 @@ export class PolicyTemplateService {
4848
return pt;
4949
}
5050

51-
async create(dto: CreatePolicyTemplateDto, frameworkId?: string) {
52-
const controlIds = frameworkId
53-
? await db.frameworkEditorControlTemplate
54-
.findMany({
55-
where: { requirements: { some: { frameworkId } } },
56-
select: { id: true },
57-
})
58-
.then((cts) => cts.map((ct) => ({ id: ct.id })))
59-
: [];
60-
51+
// New primitives are created unlinked. CX explicitly attaches them to
52+
// controls via the dedicated link endpoints — auto-linking to every
53+
// control in a framework was wrong (CX rarely wants the new policy on
54+
// every control) and forced manual cleanup after each create.
55+
async create(dto: CreatePolicyTemplateDto) {
6156
const pt = await db.frameworkEditorPolicyTemplate.create({
6257
data: {
6358
name: dto.name,
6459
description: dto.description ?? '',
6560
frequency: dto.frequency,
6661
department: dto.department,
6762
content: {},
68-
...(controlIds.length > 0 && {
69-
controlTemplates: { connect: controlIds },
70-
}),
7163
},
7264
});
7365
this.logger.log(`Created policy template: ${pt.name} (${pt.id})`);

‎apps/api/src/framework-editor/task-template/task-template.controller.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,8 @@ export class TaskTemplateController {
4747
transform: true,
4848
}),
4949
)
50-
async createTaskTemplate(
51-
@Body() dto: CreateTaskTemplateDto,
52-
@Query('frameworkId') frameworkId?: string,
53-
) {
54-
return this.taskTemplateService.create(dto, frameworkId);
50+
async createTaskTemplate(@Body() dto: CreateTaskTemplateDto) {
51+
return this.taskTemplateService.create(dto);
5552
}
5653

5754
@Get()

‎apps/api/src/framework-editor/task-template/task-template.service.spec.ts‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
jest.mock('@db', () => ({
2+
db: {
3+
frameworkEditorTaskTemplate: {
4+
create: jest.fn(),
5+
findUnique: jest.fn(),
6+
findMany: jest.fn(),
7+
update: jest.fn(),
8+
delete: jest.fn(),
9+
},
10+
frameworkEditorControlTemplate: {
11+
findMany: jest.fn(),
12+
},
13+
},
14+
Frequency: { monthly: 'monthly', yearly: 'yearly', daily: 'daily', weekly: 'weekly' },
15+
Departments: { none: 'none', admin: 'admin', it: 'it' },
16+
}));
17+
18+
import { db } from '@db';
19+
import { TaskTemplateService } from './task-template.service';
20+
21+
const mockDb = db as jest.Mocked<typeof db>;
22+
23+
describe('TaskTemplateService', () => {
24+
let service: TaskTemplateService;
25+
26+
beforeEach(() => {
27+
service = new TaskTemplateService();
28+
jest.clearAllMocks();
29+
(mockDb.frameworkEditorTaskTemplate.create as jest.Mock).mockResolvedValue({
30+
id: 'frk_tt_new',
31+
name: 'New Task',
32+
});
33+
});
34+
35+
describe('create', () => {
36+
const baseDto = {
37+
name: 'New Task',
38+
description: 'desc',
39+
};
40+
41+
// Regression test: previously, passing `frameworkId` caused the create
42+
// path to query every control template in the framework and auto-connect
43+
// the new task to all of them. CX rarely wants that — the new task should
44+
// start unlinked and be attached to specific controls explicitly via the
45+
// dedicated link endpoints. The `frameworkId` parameter is gone, but a
46+
// legacy caller passing one anyway must still produce an unlinked row.
47+
it('never queries or auto-links framework controls on create, even when a stray frameworkId is passed', async () => {
48+
(mockDb.frameworkEditorControlTemplate.findMany as jest.Mock).mockResolvedValue([
49+
{ id: 'frk_ct_1' },
50+
{ id: 'frk_ct_2' },
51+
]);
52+
53+
// Bypass TypeScript so we can simulate a stray legacy caller still
54+
// passing frameworkId — the service must ignore it.
55+
await (service.create as (dto: unknown, frameworkId?: string) => Promise<unknown>)(
56+
baseDto,
57+
'frk_soc2',
58+
);
59+
60+
expect(mockDb.frameworkEditorControlTemplate.findMany).not.toHaveBeenCalled();
61+
const createArgs = (mockDb.frameworkEditorTaskTemplate.create as jest.Mock).mock
62+
.calls[0][0];
63+
expect(createArgs.data).not.toHaveProperty('controlTemplates');
64+
});
65+
66+
it('persists name and description', async () => {
67+
await service.create(baseDto);
68+
const createArgs = (mockDb.frameworkEditorTaskTemplate.create as jest.Mock).mock
69+
.calls[0][0];
70+
expect(createArgs.data).toMatchObject({
71+
name: 'New Task',
72+
description: 'desc',
73+
});
74+
});
75+
76+
it('applies default frequency and department when not supplied', async () => {
77+
await service.create({ name: 'X' } as never);
78+
const createArgs = (mockDb.frameworkEditorTaskTemplate.create as jest.Mock).mock
79+
.calls[0][0];
80+
expect(createArgs.data.frequency).toBe('monthly');
81+
expect(createArgs.data.department).toBe('none');
82+
});
83+
});
84+
});

‎apps/api/src/framework-editor/task-template/task-template.service.ts‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,25 +7,17 @@ import { UpdateTaskTemplateDto } from './dto/update-task-template.dto';
77
export class TaskTemplateService {
88
private readonly logger = new Logger(TaskTemplateService.name);
99

10-
async create(dto: CreateTaskTemplateDto, frameworkId?: string) {
11-
const controlIds = frameworkId
12-
? await db.frameworkEditorControlTemplate
13-
.findMany({
14-
where: { requirements: { some: { frameworkId } } },
15-
select: { id: true },
16-
})
17-
.then((cts) => cts.map((ct) => ({ id: ct.id })))
18-
: [];
19-
10+
// New primitives are created unlinked. CX explicitly attaches them to
11+
// controls via the dedicated link endpoints — auto-linking to every
12+
// control in a framework was wrong (CX rarely wants the new task on
13+
// every control) and forced manual cleanup after each create.
14+
async create(dto: CreateTaskTemplateDto) {
2015
const taskTemplate = await db.frameworkEditorTaskTemplate.create({
2116
data: {
2217
name: dto.name,
2318
description: dto.description ?? '',
2419
frequency: dto.frequency ?? Frequency.monthly,
2520
department: dto.department ?? Departments.none,
26-
...(controlIds.length > 0 && {
27-
controlTemplates: { connect: controlIds },
28-
}),
2921
},
3022
});
3123

‎apps/framework-editor/app/(pages)/policies/PoliciesClientPage.tsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ export function PoliciesClientPage({ initialPolicies, emptyMessage, frameworkId
6464
<CreatePolicyDialog
6565
isOpen={isCreatePolicyDialogOpen}
6666
onOpenChange={setIsCreatePolicyDialogOpen}
67-
frameworkId={frameworkId}
6867
/>
6968
{frameworkId && (
7069
<AddExistingItemDialog

‎apps/framework-editor/app/(pages)/policies/components/CreatePolicyDialog.tsx‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,9 @@ import { CreatePolicySchema, type CreatePolicySchemaType } from '../schemas';
3535
interface CreatePolicyDialogProps {
3636
isOpen: boolean;
3737
onOpenChange: (open: boolean) => void;
38-
frameworkId?: string;
3938
}
4039

41-
export function CreatePolicyDialog({ isOpen, onOpenChange, frameworkId }: CreatePolicyDialogProps) {
40+
export function CreatePolicyDialog({ isOpen, onOpenChange }: CreatePolicyDialogProps) {
4241
const [isPending, startTransition] = useTransition();
4342
const router = useRouter();
4443

@@ -56,8 +55,11 @@ export function CreatePolicyDialog({ isOpen, onOpenChange, frameworkId }: Create
5655
const onSubmit = (values: CreatePolicySchemaType) => {
5756
startTransition(async () => {
5857
try {
59-
const queryParam = frameworkId ? `?frameworkId=${frameworkId}` : '';
60-
const result = await apiClient<{ id: string }>(`/policy-template${queryParam}`, {
58+
// Don't pass frameworkId — new policy templates are created unlinked.
59+
// CX explicitly attaches them to specific controls afterward via the
60+
// dedicated link endpoints. Auto-linking to every control in the
61+
// framework forced manual cleanup after every create.
62+
const result = await apiClient<{ id: string }>(`/policy-template`, {
6163
method: 'POST',
6264
body: JSON.stringify({
6365
name: values.name,

‎apps/framework-editor/app/(pages)/tasks/hooks/useTaskChangeTracking.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,11 @@ export const useTaskChangeTracking = (initialData: TasksPageGridData[], framewor
155155
}
156156

157157
try {
158-
const queryParam = frameworkId ? `?frameworkId=${frameworkId}` : '';
159-
const newTask = await apiClient<{ id: string }>(`/task-template${queryParam}`, {
158+
// Don't pass frameworkId — new task templates are created unlinked.
159+
// CX explicitly attaches them to specific controls afterward via the
160+
// dedicated link endpoints. Auto-linking to every control in the
161+
// framework forced manual cleanup after every create.
162+
const newTask = await apiClient<{ id: string }>(`/task-template`, {
160163
method: 'POST',
161164
body: JSON.stringify({
162165
name: row.name,

0 commit comments

Comments
 (0)