[CLAUDE] Docs: S89 closeout — PE visibility/finalize + user-cleanup + Word guide

- STATUS/HANDOFF: prepend S89 (demote S88); state Mig 59 · 422 test · prod 17 user
- session log 2026-06-25-S89 (D1 draft-privacy IDOR · D2 CCM suggested-price !readOnly · D3 finalize-note 3-surface + flow badge + matrix flag · prod user-delete · Word guide)
- docs/Huong-dan-Duyet-NCC.docx (skill docx — logo Solution, 8 sections, khoanh-do 1->2->3; real screenshots pending anh)
- harvest residual workflow-sub agent-memory (cicd / investigator-codebase / reviewer + 2 plan-reviews)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
pqhuy1987
2026-06-25 17:05:49 +07:00
parent 681b1edc7e
commit 946c5b21ae
9 changed files with 107 additions and 5 deletions

View File

@ -8,6 +8,8 @@
- [S62 PE budget soft-warning](project_s62_pe_budget_soft_warning.md) — PASS: hard-block→soft-warning; submit-guard intact + validator giữ `BudgetPeriodAmount>0`, row8 negative-safe (additive-only). Validator class `PurchaseEvaluationFeatures.cs:317`.
- [Wire/mirror claim verification anchors](feedback_wire_claim_verification_anchors.md) — sha256 twin-file · `git diff -U0` isolate true-adds · `allowNegative` bleed check · guard-still-intact grep.
- [S89 PE budget-sectionB review](project_s89_pe_sectionb_review.md) — investigator 3/3 upheld (do-not-touch FROZEN) + CAUGHT MISSED #70 race: Block B PRO row3/row8 share adjustMut + cross-echo `ev` stale + NO `||peFetching` (every other cell gated). authz-lens ≠ concurrency-lens.
- [S89 PE ends-before-CEO PLAN review](project_s89_pe_endsbeforeceo_plan_review.md) — CONCERN: BE 3-field design sound (Mig 58 col committed, no mig) but FE coverage gap — 4 PeUrgentChips render-sites, plan covered only 2 (missed InboxPage:290 despite BE Inbox A2 updated + wrong ListPage dir path).
- [S89 finalize-note PLAN review](project_s89_finalize_note_plan_review.md) — CONCERN: plan ground-truth 100% accurate (all file:line verified incl self-flagged); gaps = note doesn't say intermediate approvers still sign + skipToFinal-last-level corner + Dev DB 9 migs behind.
---

View File

@ -0,0 +1,21 @@
---
name: project-s89-finalize-note-plan-review
description: S89 adversarial PLAN review — PE "phiếu kết thúc trước CEO" NOTE 3-surface (dashboard+sidebar+detail); verdict CONCERN (plan sound, 4 UX gaps)
metadata:
type: project
---
# S89 PLAN review — PE AllowApproverFinalize NOTE (3 surface)
**Verdict: CONCERN** (plan technically sound + ground-truth accurate, but UX-completeness gaps vs anh Kiệt's "MỌI NGƯỜI NAM").
**Why:** Plan is a feature-add (no migration — `AllowApproverFinalize` = col từ Mig 58). Every file:line claim VERIFIED true incl self-flagged uncertainties (`PurchaseEvaluationsListPage:489` urgent-chip site EXACT; inbox proj `:713`; DTO ends `WinnerSupplierNames:45`/`Workflow:300`).
**How to apply (future PE-finalize-note review):**
- BE "first-reachable" compute (lowest Step.Order→Level.Order) MATCHES runtime `PurchaseEvaluationWorkflowService.cs:861` — finalize fires when sequential pointer lands on flagged level, later flagged levels unreachable. CORRECT.
- **CORNER missed by plan:** `skipToFinal` (runtime approver opt-in, :803) jumps pointer to LAST level keeping Phase=ChoDuyet; if last level ALSO has AllowApproverFinalize, finalize lands at LAST not first-reachable → note "kết thúc tại {first}" misleads. Config-pathological, MINOR.
- **UX gap (the real CONCERN):** plan's note says WHERE flow ends ("kết thúc tại Bước X") but NOT that intermediate approvers still sign first. "Đang chạy" copy `⚑ Quy trình này kết thúc tại {Step·Level} — KHÔNG trình CEO` reads to a Cấp-1 approver as if THEY finalize. Recommend add "(các cấp trước vẫn duyệt như thường)".
- 2-app mirror: plan EXPLICIT "làm CẢ fe-user+fe-admin" + correctly flags dashboard NOT-mirror (UserDashboardPage vs DashboardPage = genuinely diff files, legit exception). PeWorkflowPanel/PeListPanel SHA-identical confirmed `e477c939`.
- **type-file NOT SHA-mirror** (pre-existing PHASE_BADGE color drift fe-user text-800 vs fe-admin text-700) → plan's type line-claims (147/497) valid for fe-user only; plan acknowledges "edit từng cái" B0. OK.
- **Dev DB 9 migs behind** (history stuck Mig 49 `AddWorkItemToPE`, Mig 50-58 ALL unapplied) — plan flagged "Mig 58 chưa apply" but UNDERSTATED scope. Dev smoke 500 until `ef database update`. Prod unverified.
- E2 dashboard open-question (badge-on-list vs list-band vs KPI) legitimately deferred to anh — NOT a plan defect.

View File

@ -0,0 +1,28 @@
---
name: s89-pe-endsbeforeceo-plan-review
description: S89 adversarial PLAN review (data-sufficiency) for PE "ends-before-CEO" finalize note across 3 surfaces — CONCERN verdict, render-site coverage gap caught
metadata:
type: project
---
# S89 PE "Phiếu kết thúc trước CEO" NOTE — PLAN data-sufficiency review (CONCERN)
Adversarial PLAN review (no code yet). Feature: surface `ApprovalWorkflowLevel.AllowApproverFinalize` (Mig 58) as a "ends at level X, no CEO" note on 3 surfaces (detail / list-sidebar / dashboard).
**Verdict: CONCERN** — BE data-exposure design is SOUND (3 computed DTO fields, no migration; verified `AllowApproverFinalize` column committed in Mig 58 `20260624071509` + snapshot, 59 mig total). But FE render-site coverage is INCOMPLETE.
## Caught (plan missed)
1. **Render-site gap — 4 `PeUrgentChips` sites exist, plan covers only 2.** Plan names `PeListPanel.tsx:160` + `PurchaseEvaluationsListPage.tsx` (real path `pages/pe/`, plan said `pages/purchase-evaluations/` = WRONG dir, file exists at corrected path :489). MISSES `InboxPage.tsx:290` (sidebar inbox — and BE Inbox projection IS updated in plan A2, so data lands but chip never rendered) + `PeDetailTabs.tsx` header (urgent badge inline :280, not via chip but same list-card surface family).
2. **Semantic-overlap nuance.** `PeDetailTabs.tsx:113-116` already derives `approverFinalizeEligible = levelOptions?.allowApproverFinalize` (current-level-only, fires for current approver). Plan correctly rejects reusing this for the new note (needs workflow-wide visibility) + correctly adds new `endsBeforeCeo` — but puts new note in DIFFERENT component (`PeWorkflowPanel.tsx:252`) than existing finalize UI (`PeDetailTabs`). Both coexist OK but plan should acknowledge.
## Upheld (plan correct)
- `approvalFlow` DTO genuinely LACKS finalize flag (BE build :1045-1075 + FE types `PeApprovalFlowLevel` :263 both omit) → new field justified, can't reuse flow.
- List/Inbox projections (`:616-631` + `:713-728`) do NOT join workflow → subquery-per-PE needed (mirror winner-names). `l.Step!.ApprovalWorkflowId` translates server-side (PROVEN existing :582 `.Select` + :808 `.AnyAsync`).
- DTO constructor insertion points (`:1135` detail tail, `:45` list-item tail) structurally accurate.
- Edge-case table (multi-finalize first-reachable, V1 null-workflow, DaDuyet past-tense, no-finalize) all correct vs `PurchaseEvaluationWorkflowService.cs:861` return-at-first-match.
## Must-fix before code
- Add InboxPage.tsx render site (+ decide PeDetailTabs header). 4 sites total, not 2.
- Correct ListPage path to `fe-user/src/pages/pe/PurchaseEvaluationsListPage.tsx`.
- EF risk: ordered `.FirstOrDefault()` selecting STRING name across join (FinalizeStepName) — bool `.Any()` is safe but verify name-subquery translates (plan's 3-separate-subquery option-a is the safer pick, keep it).
- Dev DB drift: applied only to `20260611044424_AddWorkItem` — Mig 50-59 unapplied → smoke-test 500 until `dotnet ef database update`. Plan flag correct, severity understated (DB further behind than "Mig 58 only").