690414:1113 Update README.md /.agents/skills, /.windsurf/workflows
This commit is contained in:
@@ -0,0 +1,216 @@
|
||||
# Research: ADR-021 Integrated Workflow Context & Step-specific Attachments
|
||||
|
||||
**Phase 0 Output** | Generated: 2026-04-12
|
||||
|
||||
---
|
||||
|
||||
## Research Question 1: File Attachment Strategy During Workflow Transition
|
||||
|
||||
**Question:** Should files be uploaded atomically with the transition (multipart), or pre-uploaded and referenced by ID?
|
||||
|
||||
### Option A: Upload-Then-Reference (Selected ✅)
|
||||
|
||||
Client uploads files first via existing Two-Phase upload endpoint → receives `publicId`s → sends `publicId`s in `WorkflowTransitionDto.attachmentPublicIds`.
|
||||
|
||||
**Pros:**
|
||||
- ClamAV scan and file validation run **independently** before the transition DB transaction
|
||||
- Simpler transaction boundary (no file I/O inside Redlock + DB transaction)
|
||||
- Compatible with existing `file-storage.service.ts` Two-Phase pattern (ADR-016)
|
||||
- Incremental upload UX (user can upload as they review, then click Approve)
|
||||
- No multipart complexity in transition controller
|
||||
|
||||
**Cons:**
|
||||
- Two API round-trips from client
|
||||
- Orphaned temp files possible if user abandons without transitioning (mitigated by existing `FileCleanupService` cron job)
|
||||
|
||||
### Option B: Multipart Transition Request (Rejected ❌)
|
||||
|
||||
Single multipart request combining DTO JSON + files.
|
||||
|
||||
**Cons:**
|
||||
- ClamAV scan must happen inside the Redlock-protected transaction — high latency risk
|
||||
- Breaks ADR-016 Two-Phase Upload contract
|
||||
- Controller complexity (Multer + DTO parsing + ClamAV + Redlock + TypeORM transaction)
|
||||
|
||||
**Decision:** Option A — Upload-Then-Reference.
|
||||
**Rationale:** Consistent with ADR-016 Two-Phase and existing `file-storage.controller.ts` patterns. Zero change to ClamAV pipeline.
|
||||
|
||||
---
|
||||
|
||||
## Research Question 2: FK Structure for Step-Specific Attachments
|
||||
|
||||
**Question:** Add `workflow_history_id` directly to `attachments` table, or create a junction table `workflow_history_attachments`?
|
||||
|
||||
### Option A: Direct FK on `attachments` (Selected ✅)
|
||||
|
||||
```sql
|
||||
ALTER TABLE attachments
|
||||
ADD COLUMN workflow_history_id CHAR(36) NULL;
|
||||
```
|
||||
|
||||
**Pros:**
|
||||
- Specified explicitly in ADR-021 §5
|
||||
- Single JOIN to get step attachments
|
||||
- Backward-compatible: existing records have `NULL` (treated as main-doc attachments)
|
||||
- Simpler query: `WHERE workflow_history_id = :historyId`
|
||||
|
||||
**Cons:**
|
||||
- `attachments` table grows slightly (one nullable column)
|
||||
|
||||
### Option B: Junction Table `workflow_history_attachments` (Rejected ❌)
|
||||
|
||||
```sql
|
||||
CREATE TABLE workflow_history_attachments (
|
||||
history_id CHAR(36) NOT NULL,
|
||||
attachment_id INT NOT NULL,
|
||||
PRIMARY KEY (history_id, attachment_id)
|
||||
);
|
||||
```
|
||||
|
||||
**Cons:**
|
||||
- Extra table + extra JOIN
|
||||
- ADR-021 explicitly mandates Approach A
|
||||
- More complex service code
|
||||
|
||||
**Decision:** Option A — Direct FK on `attachments` table.
|
||||
**Rationale:** Explicitly mandated by ADR-021 §5 §6. Nullable FK provides backward compatibility.
|
||||
|
||||
---
|
||||
|
||||
## Research Question 3: `workflow_histories.id` UUID Type
|
||||
|
||||
**Finding from codebase:**
|
||||
|
||||
`@/e:/np-dms/lcbp3/backend/src/modules/workflow-engine/entities/workflow-history.entity.ts:22-23`
|
||||
```typescript
|
||||
@PrimaryGeneratedColumn('uuid')
|
||||
id!: string;
|
||||
```
|
||||
|
||||
**Conclusion:** `workflow_histories.id` is `CHAR(36)` UUID generated by TypeORM `@PrimaryGeneratedColumn('uuid')`. This is **NOT** `UuidBaseEntity` pattern (which uses INT PK + `uuid` column separately).
|
||||
|
||||
**Impact on FK:** `attachments.workflow_history_id` must be `CHAR(36)` to match.
|
||||
|
||||
**ADR-019 compliance:** `WorkflowHistory` does NOT have the standard `publicId`/`@Exclude() id` pattern — it uses UUID as its primary key directly. When exposing history via API, use `id` (which is already a UUID string). No conversion needed.
|
||||
|
||||
---
|
||||
|
||||
## Research Question 4: Existing Workflow Visualizer — Reuse vs. New
|
||||
|
||||
**Finding:**
|
||||
|
||||
`@/e:/np-dms/lcbp3/frontend/components/custom/workflow-visualizer.tsx:26-28`
|
||||
```typescript
|
||||
// WorkflowVisualizer Component
|
||||
// แสดงเส้นเวลา (Timeline) ของกระบวนการอนุมัติแบบแนวนอน
|
||||
export function WorkflowVisualizer({ steps, className }: WorkflowVisualizerProps)
|
||||
```
|
||||
|
||||
The existing component is **horizontal** layout. ADR-021 §3.2 requires **Vertical Timeline** with Indigo active highlighting.
|
||||
|
||||
**Decision:** Create NEW `frontend/components/workflow/workflow-lifecycle.tsx` (vertical). Keep existing `workflow-visualizer.tsx` (horizontal) for backward compat on other pages.
|
||||
|
||||
**Interface Difference:**
|
||||
- Existing: `WorkflowStep { id, label, subLabel, status, date }`
|
||||
- New: `WorkflowHistoryStep { id, fromState, toState, action, actor, comment, date, attachments[], isCurrent }`
|
||||
|
||||
---
|
||||
|
||||
## Research Question 5: Idempotency-Key Implementation
|
||||
|
||||
**Existing pattern:** No existing idempotency implementation found in workflow engine.
|
||||
|
||||
**Design:**
|
||||
- Key format: `idempotency:transition:{idempotencyKey}:{userId}`
|
||||
- Storage: Redis `SET` with `EX 86400` (24h TTL)
|
||||
- Value: Serialized transition response `{ success, nextState, historyId, isCompleted }`
|
||||
- On hit: Return cached value, HTTP 200 (or original status code if stored)
|
||||
- On miss: Execute transition, store result, return
|
||||
|
||||
**Where to implement:** `WorkflowEngineController.processTransition()` — pre-execution check using `RedisService`. If key exists → return cached. If not → run `workflowService.processTransition()` → store result.
|
||||
|
||||
**Frontend:** `use-workflow-action` hook generates a UUIDv7 idempotency key once per "action intent" (when user clicks Approve/Reject). Key is regenerated if the user cancels and re-opens the action dialog.
|
||||
|
||||
---
|
||||
|
||||
## Research Question 6: Cache Strategy for Workflow History
|
||||
|
||||
**ADR-021 §9:** Redis Cache TTL 1 hour for workflow history data. Invalidate immediately on state change.
|
||||
|
||||
**Cache key pattern:**
|
||||
- `wf:history:{instanceId}` → JSON of history items with attachments
|
||||
- Invalidated in `WorkflowEngineService.processTransition()` after `commitTransaction()`
|
||||
|
||||
**Implementation:** Use existing `CacheService` or `@nestjs/cache-manager` with Redis store. Set `ttl: 3600` on history queries.
|
||||
|
||||
---
|
||||
|
||||
## Research Question 7: CASL 4-Level RBAC for Transition Guard
|
||||
|
||||
**ADR-021 §6:** 4-Level hierarchy: Superadmin > Org Admin > Assigned Handler > Read-only
|
||||
|
||||
**Existing patterns:**
|
||||
- `RbacGuard` + `@RequirePermission()` in `workflow-engine.controller.ts`
|
||||
- CASL ability checked via `CaslAbilityFactory`
|
||||
|
||||
**New `WorkflowTransitionGuard` logic:**
|
||||
|
||||
```typescript
|
||||
// Level 1: system.manage_all (Superadmin) → always allowed
|
||||
// Level 2: organization.manage_users AND same org as document → allowed
|
||||
// Level 3: Assigned handler (workflow instance context.userId === req.user.user_id) → allowed
|
||||
// Level 4: Read-only → FORBIDDEN (403)
|
||||
```
|
||||
|
||||
**Impersonation (Upload on behalf):** Superadmin and Org Admin may supply `actingAsUserId` in payload to impersonate the handler for upload. Guard validates this permission level.
|
||||
|
||||
---
|
||||
|
||||
## Research Question 8: Priority Enum Visual Indicators
|
||||
|
||||
**ADR-021 Clarification:** `URGENT`, `HIGH`, `MEDIUM`, `LOW` — 4-tier system with visual indicators.
|
||||
|
||||
**Proposed Tailwind classes:**
|
||||
|
||||
| Priority | Badge Color | Dot/Icon |
|
||||
|----------|------------|---------|
|
||||
| `URGENT` | `bg-red-100 text-red-800 border-red-300` | 🔴 Pulse animation |
|
||||
| `HIGH` | `bg-orange-100 text-orange-800 border-orange-300` | 🟠 |
|
||||
| `MEDIUM` | `bg-yellow-100 text-yellow-800 border-yellow-300` | 🟡 |
|
||||
| `LOW` | `bg-green-100 text-green-800 border-green-300` | 🟢 |
|
||||
|
||||
**Note:** Priority field source — `workflow_instances.context` JSON (existing `metadata`/`payload` field) or document master table. Needs clarification on which document modules expose `priority`. **Interim:** Render from `WorkflowInstance.context.priority` if present.
|
||||
|
||||
---
|
||||
|
||||
## Research Question 9: File Preview — Security Considerations
|
||||
|
||||
**Requirements:** PDF and Image preview via Modal (inline, no tab change).
|
||||
|
||||
**Backend:** Need a `GET /files/preview/:publicId` or `GET /attachments/:publicId/preview` endpoint that:
|
||||
1. Validates user has `document.view` permission on the parent document
|
||||
2. Streams file with `Content-Disposition: inline` (not `attachment`)
|
||||
3. Sets `Content-Security-Policy: frame-src 'self'` for PDFs in `<iframe>`
|
||||
|
||||
**Frontend:**
|
||||
- PDF: `<iframe src="/api/files/preview/:publicId" className="w-full h-full" />`
|
||||
- Image: `<img src="/api/files/preview/:publicId" alt={filename} />`
|
||||
- Detection: by `mimeType` field from attachment metadata
|
||||
|
||||
**Finding:** `file-storage.controller.ts` exists — check if preview endpoint already implemented or needs addition.
|
||||
|
||||
---
|
||||
|
||||
## Summary of All Decisions
|
||||
|
||||
| # | Decision | Chosen | Rejected |
|
||||
|---|---------|--------|---------|
|
||||
| 1 | File attachment strategy | Upload-Then-Reference | Multipart atomic |
|
||||
| 2 | FK structure | Direct nullable FK on `attachments` | Junction table |
|
||||
| 3 | `workflow_histories.id` type | `CHAR(36)` UUID direct PK | N/A |
|
||||
| 4 | Visualizer reuse | New vertical `workflow-lifecycle.tsx` | Reuse horizontal |
|
||||
| 5 | Idempotency storage | Redis key `idempotency:transition:{key}:{userId}` TTL 24h | DB table |
|
||||
| 6 | History cache | Redis `wf:history:{instanceId}` TTL 1h, invalidate on transition | No cache |
|
||||
| 7 | Transition RBAC | New `WorkflowTransitionGuard` 4-Level | Extend existing `RbacGuard` |
|
||||
| 8 | Priority display | Tailwind badges from `instance.context.priority` | Hardcoded status |
|
||||
| 9 | File preview security | `Content-Disposition: inline` + permission check | Direct storage URL |
|
||||
Reference in New Issue
Block a user