98 lines
7.1 KiB
Markdown
98 lines
7.1 KiB
Markdown
REVIEW-F1: (đang soi — dòng verdict ghi lại ở §7 khi xong)
|
||
|
||
# Reviewer F-1 — ô-tích opt-out `applyLevelFinalize` panel duyệt KHKK
|
||
|
||
Ghi-đĩa TRONG LÚC SOI (chống #53). Đo từ ĐĨA, HEAD `24ea71e`, diff CHƯA commit.
|
||
Phạm vi: `git diff -- src fe-user fe-admin` = **5 file M, +137 / −14**.
|
||
|
||
## §0 Mirror 2 app — chứng bằng blob-SHA của git (KHÔNG tin lời khai)
|
||
|
||
`git diff` in ra index-line của cả 2 app:
|
||
|
||
| Cặp file | blob TRƯỚC | blob SAU |
|
||
|---|---|---|
|
||
| `fe-{admin,user}/src/pages/khkk/KhkkWorkflowPanel.tsx` | `e2fa3bc` (cả hai) | `6913182` (cả hai) |
|
||
| `fe-{admin,user}/src/types/khkk.ts` | `a99658c` (cả hai) | `1ffd79b` (cả hai) |
|
||
|
||
⇒ 2 cặp **byte-identical cả trước lẫn sau** (git blob = sha1 nội dung) — mirror §3.9 ĐẠT, và
|
||
không có nhánh "sửa 1 app quên app kia". ✅
|
||
|
||
## §1 Ranh giới BE — cờ có tới FE bằng ĐÚNG một đường
|
||
|
||
- `ContractSigningPlanWorkflowLevelDto` (`ContractSigningPlanFeatures.cs:116`) +1 positional
|
||
`bool AllowApproverFinalize`; **construction site = 1** (`:640`, grep toàn `src/Backend` cho
|
||
đúng 3 hit: `:116` def · `:132` kiểu List · `:640` new) ⇒ không có site thứ 2 lỡ truyền `false` cứng.
|
||
- `l.AllowApproverFinalize` đọc thẳng từ `ApprovalWorkflowLevel` của **đúng workflow đã pin**
|
||
(cùng bảng, cùng row mà `ResolveActingLevel` sẽ chấm lúc duyệt) ⇒ không có nguồn thứ hai.
|
||
- 0 đổi logic BE: `git diff` trên `src` chỉ 2 hunk, cả 2 nằm trong DTO + projection. ✅
|
||
|
||
## §2 (đề bài 1) `actingLevel` FE có mirror ĐÚNG `ResolveActingLevel` không
|
||
|
||
BE (`ContractSigningPlanWorkflowService.cs:463-468`):
|
||
`own = pendingLevelGroup.FirstOrDefault(l => l.ApproverUserId == actorId)` → `if (own is not null) return own;`
|
||
→ `if (isAdmin) return pendingLevelGroup.First();` → còn lại **ném Forbidden**.
|
||
FE (`KhkkWorkflowPanel.tsx:135-137`):
|
||
`currentLevels.find(l => l.approverUserId === user?.id) ?? (isAdmin ? currentLevels[0] : undefined)`.
|
||
|
||
| Ca | BE chấm level nào | FE chấm level nào | Khớp? |
|
||
|---|---|---|---|
|
||
| Actor ∈ Cấp, cấp CÓ cờ | own (có cờ) | own (có cờ) → hiện ô-tích | ✅ |
|
||
| Actor ∈ Cấp OR-of-N, **cờ ở NGƯỜI KHÁC cùng Cấp** | own (KHÔNG cờ) ⇒ không finalize | own (KHÔNG cờ) ⇒ ẩn ô-tích, gửi `true` = no-op | ✅ (ca đề bài hỏi kỹ — không rò) |
|
||
| Admin **có** slot riêng trong Cấp | own | own (vì `find` chạy TRƯỚC nhánh isAdmin) | ✅ |
|
||
| Admin **không** có slot | `pendingLevelGroup.First()` | `currentLevels[0]` | ⚠️ xem §6 (thứ tự 2 truy vấn) |
|
||
| Không phải approver, không Admin | **Forbidden 403** | `undefined` ⇒ ẩn ô-tích | ✅ (và nút Duyệt đã `disabled` bởi `blockedByLevel` `:130`) |
|
||
| `currentLevels` **RỖNG** (phase ≠ ChoDuyet, hoặc con-trỏ null, hoặc idx ngoài biên) | không tới `ApproveV2Async` (409 `:250-251`) | `find`→undefined, `currentLevels[0]`→`undefined` ⇒ eligible=false | ✅ **không nổ** (index-out-of-range trên mảng rỗng trong JS = `undefined`, không throw) |
|
||
| `user` chưa nạp (`user?.id` undefined) | — | `find` không khớp (`approverUserId` là `string` non-null) ⇒ undefined; `isAdmin`=false ⇒ nút Duyệt disabled | ✅ |
|
||
|
||
Vế **so-khớp GUID dạng chuỗi**: `user.id` sinh từ `res.data.user` (`AuthContext.tsx:53-56`, DTO login
|
||
serialize `Guid` → chữ thường có gạch), `approverUserId` cũng là `Guid` serialize cùng kiểu ⇒ `===`
|
||
hợp lệ; và đây là **cùng phép so đã sống từ W3** (`:127` `actorIsCurrentApprover`), không phải phép mới.
|
||
|
||
⇒ **Mirror ĐÚNG 6/7 ca**; ca thứ 7 (Admin duyệt-thay trên Cấp **trộn cờ**) là finding §6/G-4.
|
||
|
||
## §3 (đề bài 2) Body — rò field ở action khác? khớp hợp-đồng `bool?`?
|
||
|
||
`KhkkWorkflowPanel.tsx:151-157`:
|
||
`{ action, comment, ...(a === Approve ? { applyLevelFinalize: eligible ? state : true } : {}) }`
|
||
|
||
| Action | Key có trong JSON? | Giá trị | BE nhận | Ảnh hưởng |
|
||
|---|---|---|---|---|
|
||
| `submit` | **KHÔNG** | — | `ApplyLevelFinalize = null` → `?? true` (`Controller:213`) | `SubmitAsync` **không nhận tham số** (`Service:122`) ⇒ vô hại |
|
||
| `return` / `reject` | **KHÔNG** | — | như trên | `ReturnOrRejectAsync` **không nhận tham số** (`:130-136`) ⇒ vô hại |
|
||
| `approve`, cấp thường | CÓ | `true` | `true` | `actingLevel.AllowApproverFinalize && true` = **false** ⇒ no-op (§4) |
|
||
| `approve`, cấp có cờ, giữ tick | CÓ | `true` | `true` | finalize (đúng ý người bấm) |
|
||
| `approve`, cấp có cờ, bỏ tick | CÓ | `false` | `false` | rơi xuống advance thường ⇒ trình tiếp (đúng mục tiêu F-1) |
|
||
|
||
⇒ **0 rò field** sang action khác; và ngay cả khi rò cũng không có chỗ đọc ở BE (3 nhánh kia không có tham số).
|
||
|
||
**absent vs null vs true** — hợp-đồng `ContractSigningPlanTransitionBody(string Action, string? Comment = null, bool? ApplyLevelFinalize = null)`:
|
||
key vắng ⇒ System.Text.Json để `bool?` = `null` (bằng ĐÚNG giá trị mặc định khai báo, nên **không phụ thuộc**
|
||
vào chuyện STJ có tôn trọng default-parameter-value của positional record hay không — cả hai đường đều ra `null`)
|
||
⇒ `?? true`. FE **không bao giờ** gửi `null` tường minh (spread bỏ hẳn key, không set `undefined`).
|
||
⇒ 3 trạng thái absent/null/true hội tụ về CÙNG một hành vi. ✅ Khớp hợp-đồng.
|
||
|
||
## §4 (đề bài 3) Regression cấp thường + rò ô-tích ở phase khác
|
||
|
||
**Cấp thường (không cờ), trước ⟂ sau vá:**
|
||
- trước: body `{action, comment}` ⇒ BE `?? true` ⇒ `applyLevelFinalize=true`
|
||
- sau: body `{action, comment, applyLevelFinalize: true}` ⇒ BE `true`
|
||
⇒ **cùng một giá trị đi vào cùng một biểu thức** `if (actingLevel.AllowApproverFinalize && applyLevelFinalize)`
|
||
(`Service:276`), mà vế trái = `false` ⇒ nhánh không vào ở CẢ HAI thế giới. Hành vi **y hệt**, không phải
|
||
"giống về mặt cảm tính". ✅
|
||
(Điểm yếu còn lại là của thiết kế BE, không phải của diff: no-op này chỉ đúng vì BE **AND** với cờ cấp —
|
||
FE gửi `true` cho người không có quyền finalize là dữ liệu thừa, không phải quyền thừa.)
|
||
|
||
**Rò ô-tích ở phase khác — 3 lớp chặn ĐỘC LẬP, hỏng 1 lớp vẫn không rò:**
|
||
1. `currentLevels` chỉ khác `[]` khi `isWaiting` (`:118-121`) ⇒ DaDuyet/TraLai/TuChoi/Nháp ⇒ `actingLevel=undefined` ⇒ `approverFinalizeEligible=false`.
|
||
2. Nút "Duyệt" (đường DUY NHẤT đặt `action=Approve`) chỉ render trong `{isWaiting && (…)}` (`:312-335`).
|
||
3. Chính JSX ô-tích đòi `action === Approve` (`:443`).
|
||
⇒ **0 rò**. Kiểm chéo: `KhkkTransitionAction.Approve` được gán ở đúng 1 site (`:323`), grep 2 app cho thấy
|
||
không có site thứ hai. ✅
|
||
|
||
**Reset trạng thái:** `setApplyLevelFinalize(true)` đặt ở `onClick` mở dialog (`:322`) ⇒ bỏ tick rồi **Huỷ**
|
||
rồi mở lại vẫn về mặc định. (PE chỉ reset trong `onSuccess` `:288` ⇒ PE **giữ** tick sau khi Huỷ — chỗ này
|
||
KHKK **chặt hơn** khuôn.) ✅
|
||
|
||
|
||
|