Skip to content

Commit 7f79351

Browse files
github-actions[bot]tofikwestclaude
authored
fix: policy version API content bug + published version protection (#2130)
* fix(api): fix policy version content stored as empty arrays via API class-transformer with enableImplicitConversion was converting TipTap node objects to empty arrays when processing content: unknown[] DTO fields. Added @Transform decorator to preserve raw values. Also: - Block content updates on published policies via PATCH /policies/:id - Align updateVersionContent guard with UI (only block current version when published) - Sync content to current version when updating via PATCH /policies/:id - Add GET /policies/:id/versions/:versionId endpoint - Add Swagger docs for new endpoint Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(app): allow PDF upload/delete on draft policy versions and fix false success toast The upload and delete PDF guards blocked all operations on the current version regardless of policy status. Now only blocks when policy is actually published (matching the pattern used everywhere else). Also fixed PdfViewer onSuccess handlers to check result.data.success before showing the success toast — previously showed "PDF uploaded successfully" even when the server action returned { success: false }. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(api,app): protect current version during needs_review status and fix stale pointer Change version mutation guards from `status === 'published'` to `status !== 'draft'` so that the current version is also protected when the policy is in needs_review state. Fix stale currentVersionId in updateById by reading it inside the transaction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(api): move status guard inside transaction to prevent concurrent publish bypass The draft-only content guard was reading policy status before the transaction, allowing a concurrent publish to bypass the check. Now the existence check and status guard both run inside the transaction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Tofik Hasanov <annexcies@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent eb2957f commit 7f79351

12 files changed

Lines changed: 310 additions & 59 deletions

File tree

‎apps/api/src/policies/dto/ai-suggest-policy.dto.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { ApiProperty } from '@nestjs/swagger';
2+
import { Transform } from 'class-transformer';
23
import { IsString, IsOptional, IsArray } from 'class-validator';
34

45
export class AISuggestPolicyRequestDto {
@@ -29,5 +30,6 @@ export class AISuggestPolicyRequestDto {
2930
})
3031
@IsOptional()
3132
@IsArray()
33+
@Transform(({ value }) => value)
3234
chatHistory?: Array<{ role: 'user' | 'assistant'; content: string }>;
3335
}

‎apps/api/src/policies/dto/create-policy.dto.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { ApiProperty } from '@nestjs/swagger';
2+
import { Transform } from 'class-transformer';
23
import {
34
IsString,
45
IsOptional,
@@ -81,6 +82,7 @@ export class CreatePolicyDto {
8182
items: { type: 'object', additionalProperties: true },
8283
})
8384
@IsArray()
85+
@Transform(({ value }) => value)
8486
content: unknown[];
8587

8688
@ApiProperty({

‎apps/api/src/policies/dto/version.dto.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { ApiProperty } from '@nestjs/swagger';
2+
import { Transform } from 'class-transformer';
23
import { IsArray, IsBoolean, IsOptional, IsString } from 'class-validator';
34

45
export class CreateVersionDto {
@@ -36,6 +37,7 @@ export class UpdateVersionContentDto {
3637
items: { type: 'object', additionalProperties: true },
3738
})
3839
@IsArray()
40+
@Transform(({ value }) => value)
3941
content: unknown[];
4042
}
4143

‎apps/api/src/policies/policies.controller.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import { VERSION_BODIES } from './schemas/version-bodies';
5252
import {
5353
CREATE_POLICY_VERSION_RESPONSES,
5454
DELETE_VERSION_RESPONSES,
55+
GET_POLICY_VERSION_BY_ID_RESPONSES,
5556
GET_POLICY_VERSIONS_RESPONSES,
5657
PUBLISH_VERSION_RESPONSES,
5758
SET_ACTIVE_VERSION_RESPONSES,
@@ -265,6 +266,37 @@ export class PoliciesController {
265266
};
266267
}
267268

269+
@Get(':id/versions/:versionId')
270+
@ApiOperation(VERSION_OPERATIONS.getPolicyVersionById)
271+
@ApiParam(VERSION_PARAMS.policyId)
272+
@ApiParam(VERSION_PARAMS.versionId)
273+
@ApiResponse(GET_POLICY_VERSION_BY_ID_RESPONSES[200])
274+
@ApiResponse(GET_POLICY_VERSION_BY_ID_RESPONSES[401])
275+
@ApiResponse(GET_POLICY_VERSION_BY_ID_RESPONSES[404])
276+
async getPolicyVersionById(
277+
@Param('id') id: string,
278+
@Param('versionId') versionId: string,
279+
@OrganizationId() organizationId: string,
280+
@AuthContext() authContext: AuthContextType,
281+
) {
282+
const data = await this.policiesService.getVersionById(
283+
id,
284+
versionId,
285+
organizationId,
286+
);
287+
288+
return {
289+
data,
290+
authType: authContext.authType,
291+
...(authContext.userId && {
292+
authenticatedUser: {
293+
id: authContext.userId,
294+
email: authContext.userEmail,
295+
},
296+
}),
297+
};
298+
}
299+
268300
@Post(':id/versions')
269301
@ApiOperation(VERSION_OPERATIONS.createPolicyVersion)
270302
@ApiParam(VERSION_PARAMS.policyId)

‎apps/api/src/policies/policies.service.ts‎

Lines changed: 111 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -222,19 +222,6 @@ export class PoliciesService {
222222
updateData: UpdatePolicyDto,
223223
) {
224224
try {
225-
// First check if the policy exists and belongs to the organization
226-
const existingPolicy = await db.policy.findFirst({
227-
where: {
228-
id,
229-
organizationId,
230-
},
231-
select: { id: true, name: true },
232-
});
233-
234-
if (!existingPolicy) {
235-
throw new NotFoundException(`Policy with ID ${id} not found`);
236-
}
237-
238225
// Prepare update data with special handling for status changes
239226
const updatePayload: Record<string, unknown> = { ...updateData };
240227

@@ -249,35 +236,70 @@ export class PoliciesService {
249236
}
250237

251238
// Coerce content to Prisma JSON[] input if provided
252-
if (Array.isArray(updateData.content)) {
253-
updatePayload.content = updateData.content as Prisma.InputJsonValue[];
239+
const contentValue = Array.isArray(updateData.content)
240+
? (updateData.content as Prisma.InputJsonValue[])
241+
: null;
242+
243+
if (contentValue) {
244+
updatePayload.content = contentValue;
254245
}
255246

256-
// Update the policy
257-
const updatedPolicy = await db.policy.update({
258-
where: { id },
259-
data: updatePayload,
260-
select: {
261-
id: true,
262-
name: true,
263-
description: true,
264-
status: true,
265-
content: true,
266-
frequency: true,
267-
department: true,
268-
isRequiredToSign: true,
269-
signedBy: true,
270-
reviewDate: true,
271-
isArchived: true,
272-
createdAt: true,
273-
updatedAt: true,
274-
lastArchivedAt: true,
275-
lastPublishedAt: true,
276-
organizationId: true,
277-
assigneeId: true,
278-
approverId: true,
279-
policyTemplateId: true,
280-
},
247+
// All reads and writes in one transaction to prevent concurrent publish bypass
248+
const updatedPolicy = await db.$transaction(async (tx) => {
249+
// Check existence and status inside the transaction
250+
const existingPolicy = await tx.policy.findFirst({
251+
where: { id, organizationId },
252+
select: { id: true, status: true },
253+
});
254+
255+
if (!existingPolicy) {
256+
throw new NotFoundException(`Policy with ID ${id} not found`);
257+
}
258+
259+
// Cannot update content unless policy is in draft status
260+
// This covers both 'published' and 'needs_review' states
261+
if (contentValue && existingPolicy.status !== 'draft') {
262+
throw new BadRequestException(
263+
'Cannot update content of a published policy. Create a new version to make changes.',
264+
);
265+
}
266+
267+
const policy = await tx.policy.update({
268+
where: { id },
269+
data: updatePayload,
270+
select: {
271+
id: true,
272+
name: true,
273+
description: true,
274+
status: true,
275+
content: true,
276+
frequency: true,
277+
department: true,
278+
isRequiredToSign: true,
279+
signedBy: true,
280+
reviewDate: true,
281+
isArchived: true,
282+
createdAt: true,
283+
updatedAt: true,
284+
lastArchivedAt: true,
285+
lastPublishedAt: true,
286+
organizationId: true,
287+
assigneeId: true,
288+
approverId: true,
289+
policyTemplateId: true,
290+
currentVersionId: true,
291+
},
292+
});
293+
294+
// Keep current version content in sync with policy content
295+
if (contentValue && policy.currentVersionId) {
296+
await tx.policyVersion.update({
297+
where: { id: policy.currentVersionId },
298+
data: { content: contentValue },
299+
});
300+
}
301+
302+
return policy;
281303
});
282304

283305
this.logger.log(`Updated policy: ${updatedPolicy.name} (${id})`);
@@ -361,6 +383,48 @@ export class PoliciesService {
361383
}
362384
}
363385

386+
async getVersionById(
387+
policyId: string,
388+
versionId: string,
389+
organizationId: string,
390+
) {
391+
const policy = await db.policy.findFirst({
392+
where: { id: policyId, organizationId },
393+
select: { id: true, currentVersionId: true, pendingVersionId: true },
394+
});
395+
396+
if (!policy) {
397+
throw new NotFoundException(`Policy with ID ${policyId} not found`);
398+
}
399+
400+
const version = await db.policyVersion.findUnique({
401+
where: { id: versionId },
402+
include: {
403+
publishedBy: {
404+
include: {
405+
user: {
406+
select: {
407+
id: true,
408+
name: true,
409+
image: true,
410+
},
411+
},
412+
},
413+
},
414+
},
415+
});
416+
417+
if (!version || version.policyId !== policyId) {
418+
throw new NotFoundException('Version not found');
419+
}
420+
421+
return {
422+
version,
423+
currentVersionId: policy.currentVersionId,
424+
pendingVersionId: policy.pendingVersionId,
425+
};
426+
}
427+
364428
async getVersions(policyId: string, organizationId: string) {
365429
const policy = await db.policy.findFirst({
366430
where: { id: policyId, organizationId },
@@ -530,6 +594,7 @@ export class PoliciesService {
530594
select: {
531595
id: true,
532596
organizationId: true,
597+
status: true,
533598
currentVersionId: true,
534599
pendingVersionId: true,
535600
},
@@ -545,7 +610,12 @@ export class PoliciesService {
545610
throw new NotFoundException('Version not found');
546611
}
547612

548-
if (version.id === version.policy.currentVersionId) {
613+
// Cannot edit the current version unless the policy is in draft status
614+
// This covers both 'published' and 'needs_review' states
615+
if (
616+
version.id === version.policy.currentVersionId &&
617+
version.policy.status !== 'draft'
618+
) {
549619
throw new BadRequestException(
550620
'Cannot edit the published version. Create a new version to make changes.',
551621
);

‎apps/api/src/policies/schemas/version-operations.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ export const VERSION_OPERATIONS: Record<string, ApiOperationOptions> = {
66
description:
77
'Returns all versions for a policy in descending order. Supports both API key authentication and session authentication.',
88
},
9+
getPolicyVersionById: {
10+
summary: 'Get policy version by ID',
11+
description:
12+
'Returns a single policy version by its ID, including content and metadata.',
13+
},
914
createPolicyVersion: {
1015
summary: 'Create policy version',
1116
description:

‎apps/api/src/policies/schemas/version-responses.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,30 @@ const BAD_REQUEST_RESPONSE: ApiResponseOptions = {
4343
},
4444
};
4545

46+
export const GET_POLICY_VERSION_BY_ID_RESPONSES: Record<
47+
string,
48+
ApiResponseOptions
49+
> = {
50+
200: {
51+
status: 200,
52+
description: 'Policy version retrieved successfully',
53+
content: {
54+
'application/json': {
55+
schema: {
56+
type: 'object',
57+
properties: {
58+
version: { type: 'object' },
59+
currentVersionId: { type: 'string', nullable: true },
60+
pendingVersionId: { type: 'string', nullable: true },
61+
},
62+
},
63+
},
64+
},
65+
},
66+
401: UNAUTHORIZED_RESPONSE,
67+
404: NOT_FOUND_RESPONSE,
68+
};
69+
4670
export const GET_POLICY_VERSIONS_RESPONSES: Record<string, ApiResponseOptions> =
4771
{
4872
200: {

‎apps/app/src/actions/policies/update-version-content.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,9 @@ export const updateVersionContentAction = authActionClient
105105
return { success: false, error: 'Version does not belong to this policy' };
106106
}
107107

108-
// Cannot edit published version (only if the policy is actually published)
109-
if (version.id === version.policy.currentVersionId && version.policy.status === PolicyStatus.published) {
108+
// Cannot edit the current version unless the policy is in draft status
109+
// This covers both 'published' and 'needs_review' states
110+
if (version.id === version.policy.currentVersionId && version.policy.status !== PolicyStatus.draft) {
110111
return {
111112
success: false,
112113
error: 'Cannot edit the published version. Create a new version to make changes.',

‎apps/app/src/app/(app)/[orgId]/policies/[policyId]/actions/delete-policy-pdf.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ export const deletePolicyPdfAction = authActionClient
3434
// Verify policy belongs to organization
3535
const policy = await db.policy.findUnique({
3636
where: { id: policyId, organizationId },
37-
select: {
38-
id: true,
37+
select: {
38+
id: true,
39+
status: true,
3940
pdfUrl: true,
4041
currentVersionId: true,
4142
pendingVersionId: true,
@@ -59,8 +60,9 @@ export const deletePolicyPdfAction = authActionClient
5960
return { success: false, error: 'Version not found' };
6061
}
6162

62-
// Don't allow deleting PDF from published or pending versions
63-
if (version.id === policy.currentVersionId) {
63+
// Don't allow deleting PDF from the current version unless policy is in draft
64+
// This covers both 'published' and 'needs_review' states
65+
if (version.id === policy.currentVersionId && policy.status !== 'draft') {
6466
return { success: false, error: 'Cannot delete PDF from the published version' };
6567
}
6668
if (version.id === policy.pendingVersionId) {

‎apps/app/src/app/(app)/[orgId]/policies/[policyId]/actions/upload-policy-pdf.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ export const uploadPolicyPdfAction = authActionClient
4141
// Verify policy belongs to organization
4242
const policy = await db.policy.findUnique({
4343
where: { id: policyId, organizationId },
44-
select: {
45-
id: true,
44+
select: {
45+
id: true,
46+
status: true,
4647
pdfUrl: true,
4748
currentVersionId: true,
4849
pendingVersionId: true,
@@ -66,8 +67,9 @@ export const uploadPolicyPdfAction = authActionClient
6667
return { success: false, error: 'Version not found' };
6768
}
6869

69-
// Don't allow uploading PDF to published or pending versions
70-
if (version.id === policy.currentVersionId) {
70+
// Don't allow uploading PDF to the current version unless policy is in draft
71+
// This covers both 'published' and 'needs_review' states
72+
if (version.id === policy.currentVersionId && policy.status !== 'draft') {
7173
return { success: false, error: 'Cannot upload PDF to the published version' };
7274
}
7375
if (version.id === policy.pendingVersionId) {

0 commit comments

Comments
 (0)