wal: flush 20260712T1454
This commit is contained in:
pqhuy1987
@ -0,0 +1,95 @@
|
||||
**HEADLINE:** GO-WITH-ADJUSTMENTS — A1 quy-tắc `drafterSlots.Min` là đúng về state-machine (không kẹt phiếu, không vỡ 5-state, creator ký tay chạy đúng qua ApproveV2Async có sẵn), nhưng cần 3 điều-chỉnh code chính-xác + viết lại 4 test bypass + 2 điểm chờ owner chốt (cấp không liền nhau + cơ-chế history) trước khi deploy prod live.
|
||||
|
||||
## Lăng-kính STATE-MACHINE — Kết luận từng trục
|
||||
|
||||
### 1. Validate A1 (decision 1): sửa bypass chỉ-subordinate — ĐÚNG, không kẹt phiếu
|
||||
|
||||
Đọc chi tiết `PurchaseEvaluationWorkflowService.cs:571-681`. Code HIỆN TẠI dùng `k = drafterSlots.Max(l => l.Order)` (`:592`) rồi bypass + **auto-ký hộ** mọi cấp `1..k` (kể cả cấp của chính creator, `:617-642` UPSERT opinion canned comment). A1 muốn: chỉ bypass NV DƯỚI creator, creator KÝ TAY cấp mình.
|
||||
|
||||
**Chuyển sang quy-tắc `Min` là chính xác và an-toàn state-machine.** Cụ thể:
|
||||
|
||||
- `:592` đổi `k = drafterSlots.Max(...)` → `var minOwn = drafterSlots.Min(l => l.Order);`
|
||||
- `:595` đổi `Where(o => o <= k)` → `Where(o => o < minOwn)` (chỉ cấp NV strictly dưới cấp thấp nhất của creator).
|
||||
- `:597-643` — vòng `foreach`: vì mọi `order < minOwn` KHÔNG thể là slot của creator (minOwn là cấp thấp nhất của creator), nên nhánh `ownSlot is not null` (`:610-611`, `:617-642`) trở thành **dead code** → chỉ giữ Approval row "(bỏ qua — phiếu do người duyệt cấp cao hơn cùng phòng soạn)", XÓA hẳn khối UPSERT opinion. Không ghi hộ chữ ký ai.
|
||||
- `:645-680` — thay TOÀN BỘ khối advance bằng: `if (minOwn > 1) { evaluation.CurrentApprovalLevelOrder = minOwn; await LogTransitionAsync(...ChoDuyet→ChoDuyet... "Bỏ qua Cấp 1..{minOwn-1}, chờ người soạn ký Cấp {minOwn}"); }`. **XÓA cả nhánh advance-sang-Bước-2 (`:657-666`) LẪN nhánh terminal-DaDuyet-khi-submit (`:667-680`)** — vì creator không còn được bypass qua chính cấp mình; pointer DỪNG tại `minOwn` để creator ký tay.
|
||||
|
||||
**Không kẹt phiếu — chứng minh:** `minOwn` luôn là Order của một `drafterSlot` (level có thật trong firstStep, có ≥1 approver là creator). Pointer `(0, minOwn)` luôn hợp-lệ và luôn có approver ký được. Không có nhánh nào advance QUA creator, nên phiếu luôn dừng ở cấp creator ký được → không kẹt.
|
||||
|
||||
**Creator ký tay chạy đúng qua `ApproveV2Async` có sẵn:** gate `:728-739` match `actorUserId ∈ pendingLevelGroup.ApproverUserId`. Creator LÀ approver tại `minOwn` → pass. Decision 2 (không guard cứng self-approve) → không có guard nào chặn creator duyệt cấp mình. FE cũng đã hỗ-trợ: `PeWorkflowPanel.tsx:98-103` `actorInV2Level = currentUser.id ∈ currentApproval.approvers` → creator THẤY nút "Duyệt" ngay tại cấp mình. **Không kẹt cả BE lẫn UX.** Không cần sửa FE cho luồng lõi A1.
|
||||
|
||||
**Không vỡ 5-state:** submit vẫn set `Phase=ChoDuyet` (`:243`); bypass chỉ ghi Approval/Changelog + dịch `CurrentApprovalLevelOrder`, KHÔNG đổi Phase. Case terminal (1 bước, creator là cấp cuối) NAY tiến DaDuyet qua bước duyệt-tay bình-thường thay vì auto-terminal lúc submit → vẫn về đúng DaDuyet, chỉ khác thời-điểm (creator bấm Duyệt).
|
||||
|
||||
### 2. Case creator chiếm cấp KHÔNG liền nhau (Cấp1 + Cấp3) — MURKY, cần chốt
|
||||
|
||||
Với quy-tắc `Min`: `drafterSlots={1,3}` → `minOwn=1` → `bypassedOrders = {o < 1} = {}` (rỗng) → KHÔNG bypass gì, pointer đứng `(0,1)`. Diễn-tiến: creator ký tay Cấp 1 → advance Cấp 2 (người KHÁC ký) → advance Cấp 3 (creator ký tay lần 2) → advance tiếp.
|
||||
|
||||
- **Không kẹt** (pointer luôn advance), **không vỡ 5-state**. Đây là hành-vi AN-TOÀN NHẤT và khớp ĐÚNG câu chữ decision ("bypass order < drafterSlots.Min").
|
||||
- **Nhưng murky về nghiệp-vụ:** creator ký 2 lần (Cấp 1 + Cấp 3), và người Cấp 2 (kẹp giữa 2 slot của creator) KHÔNG được bypass dù creator cũng ngồi Cấp 3 cao hơn. Rationale gốc S60 ("sếp tạo thì NV dưới khỏi duyệt lại") không phủ rõ tình-huống creator trải nhiều cấp không liền.
|
||||
- **Đề xuất:** GIỮ quy-tắc `Min` (conservative, đúng câu chữ, không tự-ý bypass Cấp 2 mà creator chưa "vượt" ở slot thấp nhất). Ghi 1 comment trong code nêu rõ hành-vi non-adjacent. **Cần anh Kiệt xác-nhận** chấp-nhận "creator ký 2 lần + Cấp 2 vẫn duyệt". (Config này hiếm — Designer chỉ chặn duplicate cùng-cấp, cho phép 1 người ở Cấp 1 và Cấp 3.)
|
||||
|
||||
### 3. Reject-return SAU A1 — CÒN ĐÚNG
|
||||
|
||||
`EnsureCanRejectV2Async:316-341` + `ApplyReturnModeAsync:348-548` thao-tác theo POINTER hiện-tại, không phụ-thuộc cách bypass tính. Sau A1 pointer dừng tại `minOwn` (một cấp có thật) → mọi mode OneLevel/OneStep/Assignee/Drafter resolve bình-thường.
|
||||
|
||||
- Lưu ý (pre-existing, KHÔNG do A1): nếu approver "Trả lại 1 Cấp" từ `(0, minOwn)` xuống cấp `< minOwn` (đã bypass lúc submit), phiếu re-pend tại cấp NV từng bị bỏ qua. Đây là hành-vi ĐÚNG của Return (approver chủ-động yêu-cầu cấp dưới xem lại) — bypass chỉ là tiện-ích lúc submit, không phải skip vĩnh-viễn. Hành-vi này y-hệt code hiện-tại, A1 không đổi → out-of-scope, chỉ note.
|
||||
|
||||
### 4. Decision 3 (HARD-LOCK EDGE-5) — BẮT BUỘC thêm guard, đã verify đóng đúng lỗ
|
||||
|
||||
**Xác nhận lỗ EDGE-5 CÓ THẬT:** hiện `EnsureCanRejectV2Async:321` `if (Phase != ChoDuyet) return;` → phiếu **DaDuyet + Reject** bỏ qua guard → vào `ApplyReturnModeAsync`, pointer DaDuyet đã null → mode Drafter default (`:418-424`) set `Phase=TraLai`. **Phiếu ĐÃ DUYỆT bị hồi về Trả-lại** — đúng bug decision 3 muốn khoá. Tương-tự TuChoi + Reject → hồi TraLai; và Admin override `:290-298` chuyển DaDuyet đi bất-kỳ đâu.
|
||||
|
||||
**Fix chính-xác:** chèn ngay SAU `:55` (sau khi tính `isSystem`), TRƯỚC block `:57`:
|
||||
```
|
||||
if (fromPhase == PurchaseEvaluationPhase.DaDuyet || fromPhase == PurchaseEvaluationPhase.TuChoi)
|
||||
throw new ConflictException("Phiếu đã ở trạng thái cuối (Đã duyệt / Từ chối) — không thể Trả lại / Từ chối / chuyển trạng thái.");
|
||||
```
|
||||
Đặt ở ĐẦU chặn CẢ reject-branch (`:92`), approve-step (`:268`) LẪN admin-override (`:290`) → "chặn MỌI transition" như decision. `EnsureCanRejectV2Async:321` giữ nguyên (redundant cho terminal nhưng vô-hại + còn phòng-thủ phase non-terminal khác).
|
||||
|
||||
**Không regression:** grep tests chỉ thấy 2 nơi set `Phase=DaDuyet` (`PeFinalizeProjectionTests.cs:84`, `PeListWinnerNamesProjectionTests.cs:56`) — đều là test LIST/projection, KHÔNG gọi `TransitionAsync` trên phiếu DaDuyet. Contract-from-evaluation là command riêng, không qua `TransitionAsync`. Delete guard (`PurchaseEvaluationFeatures.cs:1347`) độc-lập, không đụng. An-toàn.
|
||||
|
||||
### 5. Decision 4+5 (1-nút + history) tương-tác state-machine
|
||||
|
||||
- **1-nút (decision 4):** `ApproveV2Async` vốn đã là 1 hành-động save=advance → giữ nguyên, không đụng state-machine. Sau A1 creator chỉ thêm 1 cú bấm Duyệt (trước auto-ký) — đúng ý decision 1.
|
||||
- **Opinion history (decision 5) — cảnh-báo schema + state-neutral:** `PurchaseEvaluationLevelOpinion` UPSERT theo key `(PE, ApprovalWorkflowLevelId)` (`ApproveV2Async:763-790`), entity có UNIQUE composite `(PEId, LevelId)`. Muốn giữ history:
|
||||
- **Nếu chọn append-only cùng bảng** → PHẢI drop/sửa UNIQUE index → cần migration 3-file (gotcha #17). Flag Category 2.
|
||||
- **Đề-xuất MVP không migration:** trước khi overwrite opinion, ghi 1 `PurchaseEvaluationChangelog` snapshot comment cũ (bảng audit-log đã sẵn) → giữ history mà không đụng schema.
|
||||
- **State-machine guardrail BẮT BUỘC:** cơ-chế sửa/cập-nhật opinion PHẢI đi endpoint/command RIÊNG, chỉ ghi opinion + history + changelog, **TUYỆT ĐỐI không route qua `TransitionAsync`** (tránh vô-tình advance pointer / đổi Phase). Đây là rủi-ro state-machine lớn nhất của decision 5.
|
||||
|
||||
### 6. BONUS CATCH — thay-đổi hành-vi giá-chốt (cần biết trước UAT)
|
||||
|
||||
Nhánh terminal-on-submit hiện-tại (`:667-680`) set DaDuyet mà **KHÔNG gọi `ApplyApprovedPriceOnFinalize`** → phiếu solo-drafter 1-bước hiện về DaDuyet với `ApprovedPriceAmount = null` (lỗ hổng thiếu giá chốt). Sau A1, creator ký tay cấp cuối đi qua `ApproveV2Async:942` → **BẮT BUỘC chọn giá chốt** (`:970-972` throw nếu thiếu, non-system). Đây là **đóng gap tốt** (creator = người duyệt cuối thật thì phải chọn giá) + FE đã lo (`PeWorkflowPanel.tsx:115,174` `currentIsFinalApprover` hiện bộ chọn giá). Nhưng là behavior-change: cần test + heads-up UAT "duyệt cấp cuối của phiếu mình phải chọn giá chốt".
|
||||
|
||||
---
|
||||
|
||||
## Test cần cập-nhật (`PeSubmitGuardAndBypassTests.cs`)
|
||||
|
||||
Baseline hiện encode hành-vi CŨ (auto-ký cấp creator). Sau A1:
|
||||
|
||||
| Test | Hiện | Sau A1 | Hành-động |
|
||||
|---|---|---|---|
|
||||
| (9) `Submit_DrafterIsTopLevelOfFirstStep_...MovesToStep2` (`:464`) | k=2 max → StepIdx=1, 2 AutoApprove, 1 opinion | minOwn=2, bypass Cấp1, pointer `(0,2)`, 1 AutoApprove, **0 opinion** (creator chưa ký) | **Viết lại** |
|
||||
| (10) `Submit_DrafterIsLevel1...MovesToLevel2SameStep` (`:509`) | k=1, pointer Cấp2, 1 AutoApprove, 1 opinion | minOwn=1, **0 bypass**, pointer đứng `(0,1)`, 0 AutoApprove, 0 opinion | **Viết lại** (case "Creator Cấp-1 → đứng (0,1)") |
|
||||
| (11) `Submit_DrafterNotInFirstStep_NoBypass` (`:544`) | pointer `(0,1)`, 0 bypass | KHÔNG đổi | **Giữ** |
|
||||
| (12) `Submit_OneStepWorkflow_DrafterIsLastLevel_TerminalDaDuyet` (`:579`) | terminal DaDuyet lúc submit, pointers null, 2 AutoApprove | minOwn=2, bypass Cấp1, pointer `(0,2)` Phase=**ChoDuyet** (KHÔNG DaDuyet), 1 AutoApprove | **Viết lại** |
|
||||
| (14) `Resubmit_FromTraLai_ReAppliesBypass` (`:642`) | mỗi submit terminal DaDuyet | submit→ChoDuyet `(0,1)`, KHÔNG terminal | **Viết lại** (resubmit vẫn 0-bypass vì solo Cấp1) |
|
||||
|
||||
**Test MỚI cần thêm:**
|
||||
1. `Submit_DrafterAtLevel2_SignsOwnManually_ThenApproveReachesNext` — sau submit pointer `(0,minOwn)`; gọi `ApproveV2Async` bằng chính creator → advance đúng (creator ký tay chạy được).
|
||||
2. `Submit_DrafterNonAdjacentLevels_Cap1AndCap3_NoBypass_SignsTwice` — chốt hành-vi murky mục 2.
|
||||
3. `Transition_FromDaDuyet_AnyReject_ThrowsHardLock` + `Transition_FromTuChoi_ThrowsHardLock` + `Transition_FromDaDuyet_AdminOverride_AlsoBlocked` — regression EDGE-5 (test-before-fix theo §7 vì là bug fix).
|
||||
4. `SoloDrafterFinalApprove_RequiresApprovedPrice` — behavior-change mục 6.
|
||||
|
||||
## Risk trước deploy prod live
|
||||
- **Cao:** decision 5 nếu route qua TransitionAsync → vỡ pointer. Enforce endpoint riêng.
|
||||
- **Trung:** 4 test cũ FAIL nếu không sửa cùng commit → CI đỏ (đúng, vì spec-change → update test cùng commit §7). Đừng để lọt.
|
||||
- **Thấp:** non-adjacent chờ chốt; nếu deploy trước khi chốt, quy-tắc Min vẫn KHÔNG kẹt phiếu (an-toàn để ship, chỉnh nghiệp-vụ sau nếu cần).
|
||||
|
||||
## Open Questions
|
||||
Còn 4 quyết-định chờ owner (anh Kiệt) chốt trước/ngay khi fan-out:
|
||||
|
||||
1. **[anh Kiệt] Cấp không liền nhau (Cấp1+Cấp3):** xác-nhận quy-tắc `Min` — creator ký TAY cả Cấp 1 lẫn Cấp 3, người Cấp 2 (kẹp giữa) VẪN phải duyệt. Nếu không muốn vậy thì phải định-nghĩa rule khác (nhưng mọi rule "bypass tới Max" đều mâu-thuẫn decision 1 "ký tay"). Khuyến-nghị: giữ Min.
|
||||
|
||||
2. **[anh Kiệt + AI_INFRA/schema] Cơ-chế opinion history:** chọn (a) reuse `PurchaseEvaluationChangelog` snapshot — KHÔNG migration (đề-xuất MVP), hay (b) bảng history mới / append-only cùng bảng — CẦN migration 3-file + drop UNIQUE `(PEId, LevelId)`. Quyết định này chặn thiết-kế endpoint sửa opinion.
|
||||
|
||||
3. **[anh Kiệt] Opinion có được sửa SAU khi phiếu DaDuyet không?** Căng với decision 3 ("đã duyệt là cứng"). Hard-lock chặn TRANSITION, nhưng sửa ý-kiến (không đổi phase) là vùng xám — cho phép hay khoá luôn?
|
||||
|
||||
4. **[anh Kiệt] Xác-nhận behavior-change giá-chốt:** phiếu solo-drafter / creator-là-cấp-cuối NAY bắt creator chọn giá chốt khi bấm Duyệt lần cuối (trước đây auto-DaDuyet bỏ qua). Đúng ý "người duyệt cuối phải chọn 1 giá" — chỉ cần xác-nhận không phá UAT.
|
||||
@ -0,0 +1,111 @@
|
||||
**HEADLINE:** GO-WITH-ADJUSTMENTS — hướng đúng, nhưng guard EDGE-5 theo spec BỎ SÓT đường admin-override (SVC:290) nên chưa "cứng" thật, và fix A1 MỞ một đường CEO-skip MỚI cần anh Kiệt chốt trước khi fan-out + deploy prod.
|
||||
|
||||
## Lăng-kính SECURITY / GOVERNANCE — kết quả kiểm
|
||||
|
||||
Tôi đã đọc source thật (không tin framing của spec/investigation). Hai vấn đề trọng tâm được xác minh dưới đây, kèm 4 mục phụ.
|
||||
|
||||
---
|
||||
|
||||
### 1. EDGE-5 hard-lock (Decision 3) — CẦN ĐIỀU CHỈNH vị trí guard (must-fix trước implement)
|
||||
|
||||
**Lỗ hổng hiện tại đã xác nhận (đúng EDGE-5):** một phiếu `DaDuyet` bị lật ngược được ngay hôm nay. Truy vết `TransitionAsync`: decision=Reject vào reject-branch (SVC:92) → `EnsureCanRejectV2Async` **early-return im lặng tại SVC:321** vì `Phase != ChoDuyet` (bỏ qua actor-scope) → `ApplyReturnModeAsync` mode mặc định Drafter → `evaluation.Phase = TraLai` (SVC:420). Phiếu đã duyệt quay lại Trả lại. Xác nhận.
|
||||
|
||||
**Spec đề xuất đặt guard ở: reject-branch (SVC:92) + `EnsureCanRejectV2Async` :321 + handler FEAT:510-539 + controller. Đây là INCOMPLETE.** Nó phủ được đường reject/return, nhưng **bỏ sót đường admin-override tại SVC:290**:
|
||||
|
||||
```
|
||||
289 // Admin manual override (vd test cứng phase)
|
||||
290 if (isAdmin) { evaluation.Phase = targetPhase; ... return; }
|
||||
```
|
||||
|
||||
Một Admin gửi `decision=Approve`, `targetPhase=<bất kỳ>` trên phiếu `DaDuyet`: KHÔNG vào reject-branch (SVC:92 đòi decision==Reject), KHÔNG vào approve-branch (SVC:268 đòi fromPhase==ChoDuyet) → rơi thẳng xuống SVC:290 → **un-terminal phiếu**. Decision 3 nói "chặn MỌI reject/trả-lại/transition" + "Đã duyệt rồi là cứng" ⇒ phải chặn CẢ Admin. Guard chỉ đặt ở reject-branch KHÔNG đạt yêu cầu này.
|
||||
|
||||
**ĐỀ XUẤT CHÍNH XÁC (1 guard duy nhất, đầu method):** chèn ngay sau `var fromPhase = evaluation.Phase;` tại **SVC:53** (trước guard TraLai/TuChoi hiện có ở SVC:57):
|
||||
|
||||
```
|
||||
if (fromPhase is PurchaseEvaluationPhase.DaDuyet or PurchaseEvaluationPhase.TuChoi)
|
||||
throw new ConflictException(
|
||||
$"Phiếu đã ở trạng thái kết thúc ({fromPhase}) — không thể trả lại, từ chối hay chuyển trạng thái. Đã duyệt/từ chối là cố định.");
|
||||
```
|
||||
|
||||
**Chứng minh ĐỦ KÍN — 5 đường ra của `TransitionAsync` đều nằm SAU SVC:53:**
|
||||
|
||||
| Đường | Vị trí | Guard đầu method chặn? |
|
||||
|---|---|---|
|
||||
| Reject / 4 return-mode (qua `ApplyReturnModeAsync`, private, caller duy nhất = SVC:118) | SVC:92 | ✅ |
|
||||
| Drafter submit/resubmit | SVC:150 | ✅ |
|
||||
| Approve step (ApproveV2/V1) | SVC:268 | ✅ |
|
||||
| **Admin override** | SVC:290 | ✅ (đây là chỗ spec bỏ sót) |
|
||||
| Fall-through throw | SVC:300 | ✅ |
|
||||
|
||||
**Bài học pattern (ghi diary):** terminal-lock phải gác **FROM-state ở ĐỈNH method**, KHÔNG rải guard per-branch — rải per-branch luôn bỏ sót nhánh admin/fall-through.
|
||||
|
||||
**Lưu ý thêm:**
|
||||
- **KHÔNG sửa semantics `EnsureCanRejectV2Async` :321.** Các early-return của nó phục vụ ca hợp lệ (system/cron reject phase khác). Khi đã có guard đỉnh-method, không cần đụng :321; đụng vào có rủi ro vỡ đường system.
|
||||
- Controller PurchaseEvaluationsController :130-143 chỉ có `[Authorize]` class-level (đúng pattern gotcha #44), không kiểm phase; handler FEAT:510-539 nạp entity nhưng cũng không kiểm phase ⇒ **service là cổng duy nhất**, nên đặt ở đầu service là single-source đúng. Thêm guard ở handler chỉ là defense-in-depth thừa, KHÔNG bắt buộc.
|
||||
- **Hệ quả cần anh Kiệt biết:** guard này gỡ luôn "escape hatch" của Admin (SVC:290) để lật một phiếu bị finalize nhầm — sau này chỉ sửa được bằng DB. Đây đúng là ý "cứng", nhưng cần xác nhận chấp nhận.
|
||||
|
||||
---
|
||||
|
||||
### 2. Decision 2 (không hard-guard self-approve) — RỦI RO CEO-SKIP, phải escalate anh Kiệt
|
||||
|
||||
**Xác minh chuỗi:** `DrafterUserId = currentUser.UserId` (FEAT:147) ⇒ creator chính là drafter. Sau A1, pointer đứng tại ô Cấp thấp nhất của creator; creator duyệt qua `ApproveV2Async`, match-approver SVC:728-739 CHO QUA vì creator ∈ `ApproverUserId`.
|
||||
|
||||
**Đường CEO-skip MỚI do chính A1 tạo ra:** nếu ô của creator có `AllowApproverFinalize=true`, `ApproveV2Async` **SVC:867** (`if (matchingLevel.AllowApproverFinalize && applyLevelFinalize)`) set `Phase=DaDuyet` → **bỏ qua mọi Bước/Cấp sau, kể cả CEO**. FE `PeWorkflowPanel.tsx:203` gửi `applyLevelFinalize` = giá trị ô-tick khi `approverFinalizeEligible`; creator VẪN thấy nút "✓ Duyệt" (nút Trả lại/Từ chối bị ẩn tại :446, nhưng Duyệt thì không). Mặc định ô-tick = false (opt-in S96/S97) ⇒ cần creator **chủ động tích**, nhưng hệ thống CHO PHÉP.
|
||||
|
||||
**Đính chính framing của investigation (giá trị độc lập):** đường này **KHÔNG tồn tại trước A1** với workflow nhiều-bước-có-CEO. Trước A1, `ApplyDrafterBypassOnSubmitAsync` tự advance VƯỢT QUA ô creator (SVC:645-666) mà KHÔNG hề gọi `AllowApproverFinalize` ⇒ CEO không bị bỏ. Ca self-finalize duy nhất tồn-tại-nay là 1-bước-drafter-là-cấp-cuối (SVC:667-680) — ca đó **không có bước CEO nào cả**. Vậy A1 **thực sự MỞ RỘNG bề mặt phơi nhiễm**; câu invest-C "đã tồn tại hôm nay" là IMPRECISE (chỉ đúng cho 1-step-no-CEO).
|
||||
|
||||
**`skipToFinal` KHÔNG phải CEO-skip:** nó chỉ nhảy pointer tới Cấp cuối, Phase giữ `ChoDuyet`, người cuối (CEO) VẪN phải ký (SVC:805-846). Chỉ nguy nếu creator ĐỒNG THỜI là approver Cấp cuối — cấu hình bệnh lý, rủi ro thấp.
|
||||
|
||||
**Escalate:** lý do Decision 2 ("cấp chỉ 1 người → tạo và duyệt luôn được") nói về việc DUYỆT một cấp một-người, KHÔNG nói về việc trao cho creator quyền tự KẾT THÚC phiếu bỏ CEO. Đây nhiều khả năng NGOÀI Ý. Xem openQuestion 1.
|
||||
|
||||
**Nếu anh Kiệt muốn bịt (đề xuất chính xác, tôn trọng Decision 2):** guard hẹp chỉ chặn self-**FINALIZE** (không phải self-approve), đặt tại **SVC:867** và **SVC:805**:
|
||||
```
|
||||
// tại SVC:867, thêm điều kiện trước khi finalize:
|
||||
if (matchingLevel.AllowApproverFinalize && applyLevelFinalize
|
||||
&& actorUserId == evaluation.DrafterUserId && !isAdmin && !isSystem)
|
||||
→ soft-warn hoặc throw (tùy anh Kiệt chốt a/b/c)
|
||||
```
|
||||
Cách này khác guard B2 mà Decision 2 đã bác (B2 = Forbidden cứng lên self-approve); guard hẹp này chỉ chạm hành vi self-finalize.
|
||||
|
||||
---
|
||||
|
||||
### 3. Ghi chú triển khai A1 (liên quan governance)
|
||||
|
||||
- **Phải sửa ĐỒNG THỜI cả logic advance (SVC:645-680) lẫn `bypassedOrders` (SVC:595) + `k` (SVC:592).** Nếu chỉ đổi `bypassedOrders` mà advance vẫn nhảy vượt `k`, ô của creator sẽ bị bỏ qua mà chưa ai ký → phiếu kẹt hoặc mất chữ ký cấp creator. Yêu cầu: chỉ auto-sign `order < drafterSlots.Min`, set pointer = `drafterSlots.Min` (không advance qua).
|
||||
- Điểm tích cực governance: chuyển từ auto-bypass ký-hộ (comment đóng hộp) sang creator ký tay comment thật (`SignedByUserId=creator`) là **nâng cấp** minh bạch audit, không phải nới lỏng.
|
||||
|
||||
---
|
||||
|
||||
### 4. Mục phụ (anh Kiệt nắm — không chặn deploy)
|
||||
|
||||
- **Admin ghi trên phiếu terminal:** `AdjustPurchaseEvaluationBudgetCommandHandler` bỏ TOÀN BỘ kiểm phase cho Admin (FEAT:367 `if (!isAdmin)`) ⇒ Admin sửa ngân sách được trên phiếu `DaDuyet`. Các endpoint suggested-price / ccm-period cũng theo pattern role-gate tương tự. Nếu "không thao tác gì được" (spec:8) hiểu chặt thì đây là đường ghi-terminal còn lại của Admin. Giảm nhẹ: `ApprovedPriceAmount` là snapshot (SVC:978) nên sửa giá sau finalize không đổi số đã chốt. Xem openQuestion 2.
|
||||
- **Pre-existing minor (không thuộc thay đổi này):** reject từ `DangSoanThao`/`TraLai` — `EnsureCanRejectV2Async` :321 bỏ actor-scope cho phase ≠ ChoDuyet ⇒ user bất kỳ có thể đẩy phiếu Nháp của người khác sang TraLai (griefing). Guard đỉnh-method ở mục 1 KHÔNG che ca này (chỉ chặn DaDuyet/TuChoi). Ghi nhận, không đề xuất sửa trong scope này.
|
||||
|
||||
---
|
||||
|
||||
### 5. Test cần thêm/cập nhật (rule §7 — bug-fix + security = test-before)
|
||||
|
||||
- **MỚI (bắt buộc, EDGE-5):** `Transition_FromDaDuyet_RejectToTraLai_Throws`, **`Transition_FromDaDuyet_AdminApprove_Throws`** (đúng đường SVC:290 mà spec bỏ sót), `Transition_FromTuChoi_AnyDecision_Throws`. Grep xác nhận HIỆN CHƯA có test terminal-lock reject nào.
|
||||
- **CẬP NHẬT (spec-change A1):** `PeSubmitGuardAndBypassTests.cs` — `Submit_OneStepWorkflow_DrafterIsLastLevel_TerminalDaDuyet` (:580, nay phải giữ `ChoDuyet` chờ creator ký), assertion `HaveCount(2)`/`HaveCount(1)` bypass-count (:505/:540), test resubmit-terminal (:645-675).
|
||||
- Nếu chọn guard finalize (2c): thêm test creator-finalize-slot bị chặn/cảnh báo.
|
||||
|
||||
---
|
||||
|
||||
### 6. Rủi ro DEPLOY (prod ERP live)
|
||||
|
||||
- **A1 + EDGE-5 = code-only, không migration** ⇒ rủi ro thấp. Decision 6 (không migrate phiếu cũ) an toàn: phiếu cũ giữ comment auto-bypass, không đụng schema.
|
||||
- **Decision 5 (opinion history) MỚI cần migration** — đây là mảnh rủi ro cao hơn, thuộc lăng-kính schema/history (đã có lane riêng: xem Area-memory `project_pe_signoff_history_schema_review.md`). Không thuộc phán quyết của tôi.
|
||||
- **Cảnh báo:** nếu deploy mà đặt guard EDGE-5 theo spec (chỉ reject-branch), Decision 3 KHÔNG thực sự đạt (còn hở Admin) → sẽ là một "done" giả. Phải dùng guard đỉnh-method mục 1.
|
||||
|
||||
**Kết luận:** thiết kế đúng hướng và các adjustment đều nhỏ, khả thi ⇒ **GO-WITH-ADJUSTMENTS** (không NO-GO): (a) đổi vị trí guard EDGE-5 sang đỉnh-method SVC:53, (b) anh Kiệt chốt openQuestion 1 (CEO-skip) trước khi fan-out.
|
||||
|
||||
## Open Questions
|
||||
Các quyết định còn lại thuộc owner (anh Kiệt) — cần chốt TRƯỚC khi fan-out implement + deploy:
|
||||
|
||||
1. **[PRIMARY — chặn implement câu 2] CEO-skip qua ô finalize của creator:** Decision 2 "tạo và duyệt luôn được" có bao gồm việc creator TỰ KẾT THÚC phiếu (bỏ CEO) khi ô Cấp của họ bật `AllowApproverFinalize` không, hay chỉ áp cho cấp KHÔNG-terminal? A1 mở đường này MỚI (trước không có với workflow đa-bước-có-CEO). Chọn: (a) chấp nhận như hiện tại; (b) cảnh báo mềm trên UI khi creator sắp tự-finalize; (c) guard hẹp chặn self-finalize (actor==Drafter) tại SVC:867 + SVC:805 — khác guard B2 mà anh đã bác vì chỉ chạm self-FINALIZE, không chạm self-approve.
|
||||
|
||||
2. **[Phạm vi Decision 3]** "Đã duyệt rồi là cứng không thao tác gì được" có mở rộng tới việc khóa Admin sửa ngân sách / giá trên phiếu DaDuyet (FEAT:367 admin bỏ kiểm phase) không, hay Decision 3 chỉ giới hạn ở transition/reject/trả-lại? (mục phụ 4)
|
||||
|
||||
3. **[Xác nhận hệ quả]** Guard hard-lock đỉnh-method gỡ luôn khả năng Admin-override lật phiếu terminal (SVC:290) — phiếu finalize nhầm sẽ chỉ sửa được bằng DB. Anh Kiệt xác nhận chấp nhận mất escape-hatch này? (đúng ý "cứng" nhưng cần biết rõ).
|
||||
|
||||
Ghi chú: Decision 5 (cơ chế lưu history opinion + migration) đã có lane schema/history phụ trách — không nằm trong phán quyết security/governance của tôi.
|
||||
@ -16,5 +16,13 @@
|
||||
## deliverable
|
||||
Validated plan + điều-chỉnh + file:line + test-list + DEPLOY-risk + GO/NO-GO → em synthesize → fan-out implement → reviewer → deploy.
|
||||
|
||||
## synthesis
|
||||
_(pending)_
|
||||
## synthesis — GO-WITH-ADJUSTMENTS (wf_7df56b89-7bb, 2/3 lane; L3 #53-garble, L1 phủ scope history)
|
||||
|
||||
- **A1 (L1+L2 confirm correct+safe):** đổi `:592` k→`minOwn=drafterSlots.Min` · `:595` `Where o<minOwn` · dead-code ownSlot-UPSERT · `:645-680` replace advance → **pointer DỪNG tại minOwn** (xóa advance-Bước-2 + terminal-DaDuyet-on-submit). Không kẹt, không vỡ 5-state. Non-contiguous RESOLVED (anh Kiệt: follow-workflow). ⚠️ phải sửa ĐỒNG THỜI advance+bypassedOrders+k (không thì kẹt/mất chữ ký).
|
||||
- **EDGE-5 (L2 REFINE spec):** guard đặt ĐỈNH-method `SVC:53` (sau `fromPhase=`, trước `:57`): `if fromPhase∈{DaDuyet,TuChoi} throw ConflictException`. Chặn CẢ 5 đường ra incl **admin-override :290** (spec per-branch BỎ SÓT). KHÔNG đụng `:321` (phục vụ system/cron).
|
||||
- **🔴 A1 MỞ CEO-skip MỚI (L2 catch):** creator-level + `AllowApproverFinalize` + tick finalize (FE:203) → `ApproveV2Async:867` Phase=DaDuyet **bỏ CEO**. KHÔNG pre-existing cho multi-step-CEO (invest framing imprecise). Vs Decision-2 (chỉ nói duyệt-cấp, không nói finalize-bỏ-CEO) → **likely NGOÀI-Ý → owner oQ1**. Fix hẹp: guard self-FINALIZE `SVC:867+:805` (actor==Drafter→warn/throw), ≠ B2-đã-bác.
|
||||
- **History (L1; L3 failed):** MVP = log comment-cũ→`PurchaseEvaluationChangelog` TRƯỚC overwrite (0 migration); MUST endpoint/command RIÊNG, **KHÔNG route TransitionAsync** (rủi-ro advance-pointer).
|
||||
- **Bonus (L1):** A1 đóng latent price-gap (solo-drafter final → ép chọn ApprovedPrice).
|
||||
- **Test:** rewrite `PeSubmitGuardAndBypassTests` + thêm terminal-lock (incl `Transition_FromDaDuyet_AdminApprove_Throws`).
|
||||
|
||||
**3 owner-decision CHẶN fan-out:** oQ1 CEO-skip · oQ2 admin-edit-on-DaDuyet scope · oQ3 confirm mất admin-escape-hatch. Harvest pending @closeout.
|
||||
|
||||
@ -0,0 +1,20 @@
|
||||
# SPEC — Supplier Excel-import CLOSE-REVIEW (đóng spec trước deploy) — 12-07-2026
|
||||
|
||||
> `/fable-clone review` đóng-spec (anh-directed): soi toàn feature BE+FE + chốt adjust + GO/NO-GO deploy. Opus 4.8 MAX. Feature = Supplier Phase B Approach A (upload Excel NCC).
|
||||
|
||||
## Trạng thái feature (code-complete)
|
||||
- **BE** (implementer-backend, build EXIT-0): entity +SourceUpdatedAt/By · Mig 63 `AddSupplierImportSourceFields` 3-file · `SupplierExcelImportService` (PreviewAsync/ConfirmAsync) · 2 CQRS command · 2 endpoint `import/preview`+`import/confirm` [Authorize Admin,CatalogManager] · DTO · DI.
|
||||
- **Test** (test-specialist): `SupplierExcelImportServiceTests` 10 test → `dotnet test` **450 PASS** (440→450), 0 prod-bug. Case-collation test có RĂNG (SQLite BINARY → dedup ở service OrdinalIgnoreCase).
|
||||
- **FE** (implementer-frontend, npm build ×2 PASS): `SupplierImportDialog.tsx` 2-app byte-identical SHA-verified + `SuppliersPage` nút "Import Excel NCC". upload→preview(layoutValid/counts/table)→confirm→toast+invalidate.
|
||||
- **5 decision** đã áp: Code=col4 · +2 field · fill-nulls-safe · Type-lạ→NhaCungCap · A-scoped-layout-file-này.
|
||||
|
||||
## 🔴 ADJUST đã biết (bake trong pha adjust sau review)
|
||||
`ExpectedHeaderTokens` trong `SupplierExcelImportService.cs:35-67` = BEST-GUESS (file thật chưa ở repo lúc scaffold) → hiện **reject file thật**. Bake 30 token THẬT (đã extract từ Excel, `NormalizeHeader` upper+collapse nên tolerant):
|
||||
```
|
||||
STT · GÓI THẦU · PHÂN LOẠI (NTP/NCC/Cả hai) · TÊN VIẾT TẮT (Dùng trong HĐ) · TÊN CÔNG TY (Đầy đủ, đúng pháp lý) · ĐỊA CHỈ XUẤT HÓA ĐƠN (Địa chỉ đăng ký kinh doanh) · ĐỊA CHỈ VĂN PHÒNG (nếu có) · SỐ ĐIỆN THOẠI CÔNG TY · FAX · SỐ TÀI KHOẢN+ TÊN+CN. NGÂN HÀNG (Đầy đủ, đúng pháp lý) · SỐ TK PHỤ (nếu có) · MÃ SỐ THUẾ · NGƯỜI ĐẠI DIỆN PHÁP LUẬT · CHỨC VỤ ĐẠI DIỆN · GIẤY ỦY QUYỀN (số, ngày, người ủy quyền) · Link GUQ · Link GPKD · Link HSNL · NGƯỜI LIÊN HỆ CHÍNH · CHỨC VỤ NGƯỜI LH · SĐT CHÍNH · EMAIL · ĐỊA CHỈ GỬI THƯ · NGƯỜI NHẬN THƯ/ SDT · NGUỒN GIỚI THIỆU · NGƯỜI PHỤ TRÁCH (PMH) · TÌNH TRẠNG HIỆN TẠI · GHI CHÚ / LÝ DO BLACKLIST · NGÀY CẬP NHẬT CUỐI · NGƯỜI CẬP NHẬT
|
||||
```
|
||||
Test Case-9 dùng reflection → tự lật `LayoutValid=true` khi bake, 0-test-edit.
|
||||
|
||||
## Checklist review (2 lane đối kháng)
|
||||
- **L1 (BE + adjust):** verify case-fix (OrdinalIgnoreCase preview+confirm) · fill-nulls không đè non-null · all-or-nothing · parser abs-index+#REF!+backslash-raw · Type/Status map · Mig 63 reversible. Confirm bake ExpectedHeaderTokens (30 real-token) đủ + đúng thứ tự cột→field. Bất kỳ BE gap trước deploy.
|
||||
- **L2 (FE + e2e + deploy):** FE flow upload/preview/confirm khớp contract (enum-int, camelCase, rows round-trip verbatim) · 2-app mirror · error-handling (layoutValid=false, errorCount>0 disable) · e2e (upload file thật → preview → confirm sau khi bake header) · deploy-risk trên prod ERP live (4 NCC file này ĐÃ ở prod qua seed → confirm no-op an-toàn, không nhân đôi) · GO/NO-GO.
|
||||
Reference in New Issue
Block a user