59 lines
4.3 KiB
Markdown
59 lines
4.3 KiB
Markdown
# Session 18 — 2026-06-17 (RFA Service Code Review & Refactor)
|
||
|
||
## Summary
|
||
|
||
Implement 8 improvement suggestions from code review ของ `rfa.service.ts` ตาม AGENTS.md — magic strings → constants, DRY helper extraction, DTO narrowing, method extraction, repository injection fix, workflow cleanup on cancel, และ `terminateInstance()` ใน WorkflowEngineService
|
||
|
||
## ปัญหาที่พบ (Root Cause)
|
||
|
||
| # | ปัญหา | ระดับ |
|
||
|---|-------|-------|
|
||
| 1 | Magic strings (`'DDW'`, `'SDW'`, `'ADW'`, `'DFT'`, `'CC'`, `'DRAFT'`, `'TO'` ฯลฯ) กระจาย ~20 ตำแหน่ง | 🟡 |
|
||
| 2 | `getCurrentRevision()` pattern ซ้ำ 4 ครั้ง (submit, processAction, update, cancel) | 🟡 |
|
||
| 3 | `disciplineId ?? 0` — dead code (DTO require `@Min(1)`) | 🟡 |
|
||
| 4 | `UpdateRfaDto` contract กว้างเกิน — รับ 10+ fields แต่ใช้แค่ 6 | 🟡 |
|
||
| 5 | `create()` ยาว 388 บรรทัด — validation, entity creation, post-commit รวมกัน | 🟢 |
|
||
| 6 | `notifyRecipients()` ใช้ `this.dataSource.manager.find()` แทน injected repo | 🟢 |
|
||
| 7 | `cancel()` ไม่ cleanup workflow instance — orphan instance ค้าง | 🟡 |
|
||
| 8 | Fire-and-forget (.catch log-only) — already acceptable per ADR-007 | 🟢 (deferred) |
|
||
|
||
## การแก้ไข (Fix)
|
||
|
||
| ไฟล์ | การเปลี่ยนแปลง |
|
||
|------|---------------|
|
||
| `backend/src/modules/rfa/constants/rfa.constants.ts` | **NEW** — Constants: type codes, status codes, workflow states, error codes, search types |
|
||
| `backend/src/modules/rfa/dto/update-rfa.dto.ts` | **Narrow** — จาก 10+ fields (extends PartialType CreateRfaRevisionDto) → 6 fields เฉพาะที่ใช้ |
|
||
| `backend/src/modules/rfa/rfa.service.ts` | **Imports** — `import * as RFA from './constants/rfa.constants'` + `CorrespondenceRecipient` repo injection |
|
||
| | **Static constants** — `WORKFLOW_CODE`, `STATE_TO_STATUS`, `DEFAULT_APPROVED_CODE` → อ่านจาก constants |
|
||
| | **`getCurrentRevision()` helper** — DRY 4× blocks → 1 private method |
|
||
| | **`validateRfaTypeDrawingConstraints()`** — extracted from `create()` |
|
||
| | **`disciplineId ?? 0`** → `disciplineId` (DTO require `@Min(1)`) |
|
||
| | **Magic strings** → constants (~20 ตำแหน่ง) |
|
||
| | **`notifyRecipients()`** → `this.corrRecipientRepo.find()` แทน `this.dataSource.manager.find()` |
|
||
| | **`cancel()`** — terminate workflow instance via `workflowEngine.terminateInstance()` |
|
||
| `backend/src/modules/workflow-engine/workflow-engine.service.ts` | **NEW `terminateInstance()`** — set status=CANCELLED + currentState=CANCELLED สำหรับ cancel workflow |
|
||
|
||
## กฎที่ Lock แล้ว
|
||
|
||
- 🔒 **Magic strings → Constants** — type codes, status codes, error codes, workflow states ต้องใช้จาก `rfa.constants.ts` เสมอ ห้าม hardcode
|
||
- 🔒 **`getCurrentRevision()`** — ทุก method ที่ต้องการ revision ปัจจุบัน ต้องใช้ helper นี้ ไม่ duplicate logic
|
||
- 🔒 **`cancel()` ต้อง terminate workflow** — เมื่อยกเลิก DRAFT RFA ให้ cleanup workflow instance ด้วย
|
||
- 🔒 **`notifyRecipients()`** — use injected repo (ไม่ใช่ `dataSource.manager.find()`)
|
||
|
||
## Verification
|
||
|
||
- [x] TypeScript `tsc --noEmit` — **0 errors** ใน rfa.service.ts, workflow-engine.service.ts, rfa.constants.ts, update-rfa.dto.ts
|
||
- [x] magic strings grep — ไม่เหลือ `'DFT'`, `'FRE'`, `'FAP'`, `'FCO'`, `'CC'`, `'OBS'`, `'DRAFT'`, `'DDW'`, `'SDW'`, `'ADW'`, `'TO'` ใน rfa.service.ts
|
||
- [x] Build integrity — unchanged files not affected
|
||
|
||
## Files Changed
|
||
|
||
```
|
||
backend/src/modules/rfa/constants/rfa.constants.ts (NEW — 70 lines)
|
||
backend/src/modules/rfa/dto/update-rfa.dto.ts (OVERWRITTEN — 22 lines)
|
||
backend/src/modules/rfa/rfa.service.ts (MODIFIED — 1056 lines, +45/-55 net)
|
||
backend/src/modules/workflow-engine/workflow-engine.service.ts (MODIFIED — +20 lines)
|
||
specs/88-logs/session-2026-06-17-rfa-service-refactor.md (THIS FILE)
|
||
specs/88-logs/rollouts.md (UPDATED)
|
||
```
|