vietreport-admin/solution-erp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
cf11faf1ec368f7c7f1dbd82fefe6206012e2665
solution-erp/.claude/workflows/runs/2026-07-12-S112-supplier-excel-import/sub-reviewer-0.md
pqhuy1987 cf11faf1ec wal: flush 20260712T1338
2026-07-12 13:38:40 +07:00

10 KiB
Raw Blame History

sub-reviewer-0

HEADLINE: PASS_WITH_CONCERNS — 3 lane mapping/dedup/approach chính xác (mọi file:line in-repo verify KHỚP, 27/27 field bijective, 0 field bịa, 0 silent data-loss), NHƯNG 1 edge chưa lane nào chốt: dedup Code dùng Ordinal (case-sensitive) trong khi DB unique-index = default CI collation → hard unique-violation cho Approach A; + citation NamGroup không verify được.

Reviewer độc lập — Supplier Excel-Import design (đọc source thật, không tin mù 3 lane)

Method: đọc trực tiếp 14 file in-repo mà 3 lane cite; cross-validate mapping với living seed 4 NCC; grep độc lập "no import endpoint"; grep collation config; verify Mig 62 tồn tại; search NamGroup repo trên disk.

Kết luận nhanh: 3 lane làm việc chất lượng cao — mọi file:line in-repo đều verify KHỚP CHÍNH XÁC (không phịa). 5 điểm kiểm chứng đều đứng vững ở tầng thiết-kế. Tôi bắt được 1 edge MAJOR (case-collation dedup) mà cả 3 lane bỏ sót + vài minor. Không có lỗi nào làm hỏng design; chúng REFINE nó trước khi owner chọn approach.

(1) MAPPING đủ 30 cột, 0 silent data-loss — ✅ PASS

Verify từng cột vs Supplier.cs:5-36:

  • 28/30 cột có nhà rõ ràng (cột 1 STT = ignore-có-lý-do; cột 2–28 = 27 direct-field).
  • Cột 29-30 (NGÀY/NGƯỜI CẬP NHẬT) = KHÔNG bị nuốt câm — được flag tường minh với 3 option owner (ignore / gộp-Note / +2 field Mig 63). Đây là surface-to-owner đúng cách, KHÔNG phải data-loss.
  • Cross-check với seed thật: DbInitializer.cs:2776 TRUONGGIANG có Phone="08 2253 1381" (cột 8) TÁCH BIỆT ContactPhone="0966 801 456" (cột 21), và Address (cột 6) tách MailingAddress (cột 23) — xác nhận 3 address-field + 2 phone-field phân biệt đúng, không đè lên nhau. Mapping là bijective trên 27 field.

(2) DEDUP KEY — ✅ PASS (key choice + null-safe) NHƯNG ⚠️ MAJOR gap ở tầng implementation

Key choice đúng + null-safe (claim literal PASS):

  • Code UNIQUE filtered SupplierConfiguration.cs:44 ✓ verified · TaxCode KHÔNG unique :17 ✓ · Code IsRequired :14 ✓ · TaxCode null DONGDUONG DbInitializer.cs:2781 ✓. Dedup=Code nên TaxCode-null KHÔNG vỡ — null-safe xác nhận.

🔴 MAJOR — edge cả 3 lane bỏ sót (Lane C 12 edge-case MISS case này): Lane C chốt dedup = Code.Trim() Ordinal (case-SENSITIVE) để khớp StringComparer.Ordinal của seed (:2787). NHƯNG:

  • Grep collation trong toàn SolutionErp.Infrastructure = 0 hit (không UseCollation, không explicit column collation) → Code inherit default SQL Server collation = CASE-INSENSITIVE (CI) trên đa số install.
  • Hệ quả cho Approach A (import Excel ngoài): Excel có Code case-variant (vd "truonggiang" vs DB "TRUONGGIANG") → dict Ordinal miss → quyết INSERT → DB unique-index (CI) THROW DbUpdateException unique-violation → hard 500, cả batch all-or-nothing vỡ.
  • Seed hiện tại KHÔNG dính vì nó own 4 code uppercase cố định; nhưng import ăn Excel tùy ý thì dính.
  • Lane A CÓ gợi ý "Trim+UPPER khuyến nghị" cho Code (đúng hướng fix) nhưng Lane C dedup-section lại viết "Ordinal" — 2 lane mâu thuẫn nội bộ, và bản viết cuối (Ordinal) chính là cái tạo bug.
  • Acceptance để đóng: normalize Code (Trim + case nhất-quán, vd UPPER) TRƯỚC cả dedup-check LẪN store, HOẶC dùng StringComparer.OrdinalIgnoreCase cho dict — để khớp collation DB. Preview cũng phải dùng cùng comparer, nếu không preview "sạch" rồi commit mới nổ.

(Soft-delete slot: Excel Code trùng row IsDeleted=1 → dict [HasQueryFilter !IsDeleted] không thấy → INSERT → index filter [IsDeleted]=0 cho phép → tạo row active mới. Đây là intended "slot reusable" :44, KHÔNG phải bug.)

(3) APPROACH cho solo-dev + 4-row — ✅ SOUND (không over-engineer, không mất data) + 2 MINOR

  • Correction của Lane B verified ĐÚNG: spec framing "seed fill PARTIAL — không đụng 18 expansion field" là SAI. DbInitializer.cs:2776,2778,2780,2782 set đủ expansion field (PackageCategory/BankAccount/LegalRepresentative/LinkGuq/Gpkd/Hsnl/ContactTitle/ContactPhone/MailingAddress/MailRecipient/ReferralSource/OwnerPmh/Status/Fax) cho 4 NCC. Approach B THỰC ĐÃ SHIP 30-field. Rec "file này đã xong, khỏi build" là anti-over-engineer đúng cho solo-dev.
  • Fill-nulls :2796-2813 verified: mỗi dòng if (e.X is null && s.X is not null) — KHÔNG bao giờ đè non-null. Idempotent an toàn.
  • ⚠️ MINOR-1 (external citation không verify được): find toàn D:/Dropbox/CONG_VIEC cho HrEmployeeImportService.cs = 0 hit; repo ERP_MINI_FINAL KHÔNG có mặt (chỉ FORM/QUY_TRINH/SOLUTION_ERP). Lane B dựng luận điểm "hạ effort A đáng kể nhờ port NamGroup" trên file:line (:12/:15/:18/:24-44/:256/:317/:325) không thể verify từ env này — có thể ở checkout khác, nhưng effort-estimate A đang tựa artifact chưa xác nhận. Design vẫn sound (preview→confirm là pattern chuẩn), chỉ con-số-effort là unbacked.
  • ⚠️ MINOR-2 (Approach B là ONE-TIME): fill-nulls không đè non-null → nếu owner SỬA Excel (đổi phone TRUONGGIANG) rồi re-seed, thay đổi KHÔNG propagate. Approach B = load-một-lần, KHÔNG phải cơ chế update. Lane B ngụ ý qua "thêm NCC = thêm row+redeploy" nhưng không nói sắc điểm "corrections không sync".

(4) FIDELITY 27-field — ✅ PASS (0 bịa, 0 sót)

  • Đếm Supplier.cs:7-35: đúng 27 business field (9 core + 18 expansion). Tất cả 27 target mà 3 lane map ĐỀU tồn tại — 0 field ngoài Supplier.cs.
  • 27/27 field được map đúng 1 lần (audit đầy đủ ở đầu review). 0 field khớp bị sót.
  • Field "đề xuất mới" (SourceUpdatedAt/SourceUpdatedBy trong Lane A option-c) được label rõ là PROPOSED — không claim đã tồn tại → không vi phạm fidelity.
  • Audit-field xác nhận không map được: BaseEntity.cs:9 UpdatedBy là Guid? (Excel = tên text → type mismatch) + AuditingInterceptor.cs:48-49 ghi đè UpdatedAt/UpdatedBy mỗi Modified → map cột 29-30 vào audit = futile. Cả 3 lane claim verified ĐÚNG.

(5) PARSER-STRATEGY — ✅ PASS (header-4-dòng + #REF! + backslash) + 1 MINOR

  • Read primitive verified: XlsxRenderer.cs:18 new XLWorkbook + :22 CellsUsed() + :25 cell.GetString() (auto-decode shared-string). ClosedXML 0.105.0 csproj:8 ✓ — 0 dependency mới.
  • Anchor r4 + bỏ r1/r2/r3 + data từ r5 + header-fingerprint reject-file-nếu-lệch: xử đúng header-gộp + cột-đổi-vị-trí.
  • #REF! → null-literal skip; NAS backslash → raw verbatim (khớp seed :2776 @"01. GPKD_HSNL_GUQ\02.Trường giang\...pdf"); composite → raw (BankAccount MaxLen 500 SupplierConfiguration.cs:29 đủ). Tất cả đúng.
  • ⚠️ MINOR-3 (implementation caveat): CellsUsed() (idiom được cite) SKIP ô trống. Cho POSITIONAL 30-cột, implementer PHẢI đọc header VÀ data theo cùng absolute column index (row.Cell(i)), KHÔNG iterate CellsUsed — nếu không, 1 ô rỗng (vd FAX trống) làm lệch toàn bộ cột sau. Fingerprint bắt lệch-gross nhưng đọc-bằng-CellsUsed có thể mismap câm. Intent (positional) của Lane C đúng; chỉ cần chốt primitive đọc-theo-index.

Bảng severity

# Severity Điểm Gap Đóng bằng
1 MAJOR (2) Dedup Code Ordinal vs DB default CI collation → hard unique-violation Approach A Normalize Code case (hoặc OrdinalIgnoreCase dict) trước dedup+store, cả preview
2 MINOR (3) NamGroup HrEmployeeImportService cite không verify được (repo vắng) Xác nhận artifact tồn tại trước khi tin effort-A-thấp
3 MINOR (3) Approach B = one-time fill-nulls, corrections không re-sync Ghi rõ giới hạn; nếu cần update → Approach A
4 MINOR (5) CellsUsed() skip ô trống → lệch positional Đọc header+data theo absolute cell index

Điểm resisted scrutiny (positive validation)

  • Mapping bijective 27/27 + cross-validate living seed = solid, không phịa file:line.
  • Dedup KEY = Code (không TaxCode) đúng + 5 chứng cứ đều verify khớp.
  • "No import endpoint" premise verify độc lập: grep Suppliers Application + controller = 0 hit import/IFormFile/upload.
  • Mig 62 ExpandSupplierFields (2026-07-06) tồn tại → 18 expansion column có backing migration (schema integrity OK).
  • Controller auth = [Authorize(Roles=...)] role-based (KHÔNG Policy) → import rec Admin,CatalogManager khớp convention, không dính gotcha #44 (read mở, write role-gated).

Smart-Friend guard: không hạ bar. Design THẬT tốt → PASS phần lớn; nhưng MAJOR case-collation là bug hard-fail thật sự cho Approach A mà không lane nào chốt → nêu rõ, không rubber-stamp.

Token cost ~34K. RETURN-ONLY honored (không ghi MEMORY/file — em-main harvest).

Open Questions

Owner (anh) quyết:

  1. Cột 29-30 (NGÀY/NGƯỜI CẬP NHẬT): ignore (audit tự stamp, mất provenance) / gộp-Note / +2 field SourceUpdatedAt+SourceUpdatedBy (Mig 63)? — 3 lane đồng thuận đây là owner-call.
  2. Code source = cột "TÊN VIẾT TẮT"? Code dùng trong mã HĐ (RG-001 SOL&{SupplierCode}) → cần anh xác nhận lấy từ col4 hay gán riêng (ảnh hưởng upsert-key Approach A).
  3. Có cần tool reusable? Chỉ file 4-NCC → Approach B đã xong, khỏi build. NCC phát sinh liên tục → Approach A.
  4. Re-import Code đã tồn tại: fill-nulls (an toàn, không sync data đổi) hay overwrite-non-empty (cập nhật được nhưng destructive)? — Lane C đề xuất overwrite cho import (khác seed), cần anh chốt.
  5. PHÂN LOẠI (Type non-nullable) gặp giá-trị-lạ: reject-row hay default NhaCungCap?

Implementer PHẢI fix trước khi build Approach A (không phải owner-call, là kỹ thuật): 6. Chuẩn hóa Code case để khớp DB CI collation (MAJOR #1) — nếu bỏ qua, import case-variant Code sẽ nổ unique-violation hard-fail. Đây là điều kiện tiên quyết cho robustness của A, không được để dạng "Ordinal" như Lane C viết.