fix(workflow): ADR-021 code review fixes (8 bugs)

- fix(transmittal): guard duplicate workflow instance on submit()
- fix(workflow-guard): add organizationId to context so Level-2 RBAC works
- fix(circulation): organizationId context passed relation object not INT FK
- fix(transmittal): require Idempotency-Key header on POST submit endpoint
- fix(workflow): userId non-optional in processTransition controller
- fix(circulation): auto-close counts PENDING and IN_PROGRESS tasks
- fix(transmittal): status badge uses workflowState/DRAFT not purpose field
- fix(workflow): log cache invalidation failures instead of swallowing
- fix(workflow): implement getAvailableActions endpoint stub
- fix(i18n): add removeFile key to EN/TH locales
This commit is contained in:
2026-04-17 16:25:51 +07:00
parent 3a5fc8d4af
commit 5977e48e38
10 changed files with 71 additions and 33 deletions
@@ -51,7 +51,7 @@ export class CirculationWorkflowService {
// Context — Circulation เป็น internal document ระดับ Organization (ไม่ผูก contract) // Context — Circulation เป็น internal document ระดับ Organization (ไม่ผูก contract)
// Guard Level 2 ตรวจ organizationId; Level 2.5 (contract check) จะ skip เมื่อ contractId = null // Guard Level 2 ตรวจ organizationId; Level 2.5 (contract check) จะ skip เมื่อ contractId = null
const context: Record<string, unknown> = { const context: Record<string, unknown> = {
organizationId: circulation.organization, organizationId: circulation.organizationId,
creatorId: userId, creatorId: userId,
}; };
@@ -325,12 +325,14 @@ export class CirculationService {
await this.routingRepo.save(routing); await this.routingRepo.save(routing);
// Check: ถ้าทุกคนทำเสร็จแล้ว ให้ปิดใบเวียน (Master) // Check: ถ้าทุกคนทำเสร็จแล้ว ให้ปิดใบเวียน (Master)
const pendingCount = await this.routingRepo.count({ // Bug 5 fix: นับทั้ง PENDING และ IN_PROGRESS — forceClose() ปิดทั้งสองสถานะ
where: { const pendingCount = await this.routingRepo
circulationId: routing.circulationId, .createQueryBuilder('r')
status: 'PENDING', // หรือ status ที่ยังไม่เสร็จ .where('r.circulationId = :cid', { cid: routing.circulationId })
}, .andWhere('r.status IN (:...statuses)', {
}); statuses: ['PENDING', 'IN_PROGRESS'],
})
.getCount();
if (pendingCount === 0) { if (pendingCount === 0) {
await this.circulationRepo.update(routing.circulationId, { await this.circulationRepo.update(routing.circulationId, {
@@ -1,6 +1,8 @@
import { import {
BadRequestException,
Controller, Controller,
Get, Get,
Headers,
Post, Post,
Body, Body,
Param, Param,
@@ -85,8 +87,12 @@ export class TransmittalController {
@Audit('transmittal.submit', 'transmittal') @Audit('transmittal.submit', 'transmittal')
submit( submit(
@Param('uuid', ParseUuidPipe) uuid: string, @Param('uuid', ParseUuidPipe) uuid: string,
@CurrentUser() user: User @CurrentUser() user: User,
@Headers('Idempotency-Key') idempotencyKey: string
) { ) {
if (!idempotencyKey) {
throw new BadRequestException('Idempotency-Key header is required');
}
return this.transmittalService.submit(uuid, user); return this.transmittalService.submit(uuid, user);
} }
} }
@@ -252,7 +252,7 @@ export class TransmittalService {
Correspondence, Correspondence,
{ {
where: { publicId: uuid }, where: { publicId: uuid },
select: ['id', 'correspondenceNumber', 'disciplineId'], select: ['id', 'correspondenceNumber', 'disciplineId', 'originatorId'],
} }
); );
if (!correspondence) if (!correspondence)
@@ -285,10 +285,17 @@ export class TransmittalService {
} }
} }
// เริ่ม Workflow Instance สำหรับ Transmittal // Bug 1 fix: ป้องกัน duplicate instance — ถ้ามี Active Instance อยู่แล้ว ให้หยุด
const statusDraft = await this.statusRepo.findOne({ const existingInstance = await this.workflowEngine.getInstanceByEntity(
where: { statusCode: 'DRAFT' }, 'transmittal',
}); correspondence.id.toString()
);
if (existingInstance) {
throw new ValidationException(
`Transmittal นี้ถูก Submit ไปแล้ว (Workflow Instance: ${existingInstance.id})`
);
}
// [C3] Resolve contractId from discipline for contract-scoped workflow // [C3] Resolve contractId from discipline for contract-scoped workflow
let contractId: number | null = null; let contractId: number | null = null;
if (correspondence.disciplineId) { if (correspondence.disciplineId) {
@@ -298,11 +305,17 @@ export class TransmittalService {
); );
contractId = rows[0]?.contract_id ?? null; contractId = rows[0]?.contract_id ?? null;
} }
// Bug 2 fix: ใส่ organizationId ใน context เพื่อให้ WorkflowTransitionGuard Level 2 (Org Admin) ทำงานได้
const instance = await this.workflowEngine.createInstance( const instance = await this.workflowEngine.createInstance(
'TRANSMITTAL_FLOW_V1', 'TRANSMITTAL_FLOW_V1',
'transmittal', 'transmittal',
correspondence.id.toString(), correspondence.id.toString(),
{ ownerId: user.user_id, contractId } {
ownerId: user.user_id,
contractId,
organizationId: correspondence.originatorId ?? null,
}
); );
const result = await this.workflowEngine.processTransition( const result = await this.workflowEngine.processTransition(
@@ -313,18 +326,16 @@ export class TransmittalService {
); );
// Sync สถานะกลับที่ Correspondence Revision // Sync สถานะกลับที่ Correspondence Revision
if (statusDraft) { const revision = await this.revisionRepo.findOne({
const revision = await this.revisionRepo.findOne({ where: { correspondenceId: correspondence.id, isCurrent: true },
where: { correspondenceId: correspondence.id, isCurrent: true }, });
if (revision) {
const submittedStatus = await this.statusRepo.findOne({
where: { statusCode: 'SUBMITTED' },
}); });
if (revision) { if (submittedStatus) {
const submittedStatus = await this.statusRepo.findOne({ revision.statusId = submittedStatus.id;
where: { statusCode: 'SUBMITTED' }, await this.revisionRepo.save(revision);
});
if (submittedStatus) {
revision.statusId = submittedStatus.id;
await this.revisionRepo.save(revision);
}
} }
} }
@@ -116,7 +116,7 @@ export class WorkflowEngineController {
throw new BadRequestException('Idempotency-Key header is required'); throw new BadRequestException('Idempotency-Key header is required');
} }
const userId = req.user?.user_id; const userId = req.user.user_id;
// ตรวจ Redis ว่า Request นี้ถูกส่งมาแล้วหรือไม่ (key ผูกกับ userId ป้องกัน cross-user replay) // ตรวจ Redis ว่า Request นี้ถูกส่งมาแล้วหรือไม่ (key ผูกกับ userId ป้องกัน cross-user replay)
const cacheKey = `idempotency:transition:${idempotencyKey}:${userId}`; const cacheKey = `idempotency:transition:${idempotencyKey}:${userId}`;
@@ -154,9 +154,18 @@ export class WorkflowEngineController {
@ApiOperation({ @ApiOperation({
summary: 'ดึงรายการปุ่ม Action ที่สามารถกดได้ ณ สถานะปัจจุบัน', summary: 'ดึงรายการปุ่ม Action ที่สามารถกดได้ ณ สถานะปัจจุบัน',
}) })
@RequirePermission('document.view') // ผู้ที่มีสิทธิ์ดูเอกสาร ควรดู Action ได้ @ApiParam({ name: 'id', description: 'Workflow Instance ID (UUID)' })
getAvailableActions(@Param('id') _instanceId: string) { @RequirePermission('document.view')
// Note: Logic การดึง Action ตาม Instance ID จะถูก Implement ใน Task ถัดไป async getAvailableActions(@Param('id') instanceId: string) {
return { message: 'Pending implementation in Service layer' }; const instance = await this.workflowService.getInstanceById(instanceId);
const actions = await this.workflowService.getAvailableActions(
instance.definition.workflow_code,
instance.currentState
);
return {
instanceId,
currentState: instance.currentState,
availableActions: actions,
};
} }
} }
@@ -387,7 +387,13 @@ export class WorkflowEngineService {
await queryRunner.commitTransaction(); await queryRunner.commitTransaction();
// ADR-021 T043: Invalidate Workflow History cache หลัง transition สำเร็จ // ADR-021 T043: Invalidate Workflow History cache หลัง transition สำเร็จ
void this.cacheManager.del(`wf:history:${instanceId}`); this.cacheManager
.del(`wf:history:${instanceId}`)
.catch((e: unknown) =>
this.logger.warn(
`Cache invalidation failed for wf:history:${instanceId} — stale data may be served. Error: ${e instanceof Error ? e.message : String(e)}`
)
);
// [NEW] เก็บค่าไว้ Dispatch หลัง Commit // [NEW] เก็บค่าไว้ Dispatch หลัง Commit
eventsToDispatch = evaluation.events; eventsToDispatch = evaluation.events;
@@ -60,7 +60,9 @@ export default function TransmittalDetailPage() {
const transmittalDocNo = transmittal.correspondence?.correspondenceNumber ?? transmittal.transmittalNo ?? ''; const transmittalDocNo = transmittal.correspondence?.correspondenceNumber ?? transmittal.transmittalNo ?? '';
const transmittalSubject = transmittal.subject ?? ''; const transmittalSubject = transmittal.subject ?? '';
const transmittalStatus = transmittal.purpose ?? ''; const transmittalStatus = transmittal.workflowInstanceId
? (transmittal.workflowState ?? 'SUBMITTED')
: 'DRAFT';
return ( return (
<section className="space-y-4"> <section className="space-y-4">
@@ -294,7 +294,7 @@ export function WorkflowLifecycle({
type="button" type="button"
className="ml-0.5 hover:text-destructive" className="ml-0.5 hover:text-destructive"
onClick={(e) => { e.stopPropagation(); removeUploadedFile(f.publicId); }} onClick={(e) => { e.stopPropagation(); removeUploadedFile(f.publicId); }}
aria-label={t('workflow.timeline.uploadError')} aria-label={t('workflow.timeline.removeFile')}
> >
<XIcon className="h-3 w-3" /> <XIcon className="h-3 w-3" />
</button> </button>
+1
View File
@@ -29,6 +29,7 @@
"workflow.timeline.uploading": "Uploading...", "workflow.timeline.uploading": "Uploading...",
"workflow.timeline.uploadTypes": "PDF, DOCX, XLSX, DWG, ZIP · Max 50 MB", "workflow.timeline.uploadTypes": "PDF, DOCX, XLSX, DWG, ZIP · Max 50 MB",
"workflow.timeline.uploadError": "Unable to upload", "workflow.timeline.uploadError": "Unable to upload",
"workflow.timeline.removeFile": "Remove file",
"filePreview.fallbackTitle": "File", "filePreview.fallbackTitle": "File",
"filePreview.fileUnavailable": "File has been removed from storage.", "filePreview.fileUnavailable": "File has been removed from storage.",
+1
View File
@@ -29,6 +29,7 @@
"workflow.timeline.uploading": "กำลังอัปโหลด...", "workflow.timeline.uploading": "กำลังอัปโหลด...",
"workflow.timeline.uploadTypes": "PDF, DOCX, XLSX, DWG, ZIP · สูงสุด 50 MB", "workflow.timeline.uploadTypes": "PDF, DOCX, XLSX, DWG, ZIP · สูงสุด 50 MB",
"workflow.timeline.uploadError": "ไม่สามารถอัปโหลด", "workflow.timeline.uploadError": "ไม่สามารถอัปโหลด",
"workflow.timeline.removeFile": "ลบไฟล์",
"filePreview.fallbackTitle": "ไฟล์", "filePreview.fallbackTitle": "ไฟล์",
"filePreview.fileUnavailable": "ไฟล์ถูกลบออกจาก Storage แล้ว", "filePreview.fileUnavailable": "ไฟล์ถูกลบออกจาก Storage แล้ว",