Implement product lifecycle and supplier reorder planning#156
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 594af9368b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Four findings from review of the product lifecycle + supplier reorder implementation PR (P8.1 + P8.2 from PR #155's plan): - onetwo3d-ims-27w [P2] getReorderReport uses SupplierProduct[0] as primary supplier with preferredSupplier as fallback only — inverts the auto-update intent; 'latest placed PO wins' is invisible at the read path - onetwo3d-ims-ywu [P2] OPEN_WMS_ASN_STATUSES counts CREATE_PENDING / CREATE_IN_FLIGHT as firm incoming stock — stuck ASNs indefinitely defer EOL → ARCHIVED transition - onetwo3d-ims-3i3 [P2] updatePreferredSuppliersForPlacedPurchaseOrder writes no activity log — silent supplier reassignment, operator has no audit trail when 'why did the preferred supplier change?' is asked - onetwo3d-ims-zqe [P3] Third instance of \$executeRaw used for SELECT FOR UPDATE — same anti-pattern as PR #127. Worth a codebase sweep. Closed PR #155 P8.1 / P8.2 spec issues addressed in this PR: - onetwo3d-ims-ljw (migration path) → migration SQL maps NOT_FOR_SALE → EOL - onetwo3d-ims-dn2 (incoming stock spec) → structured getProductIncomingStock - onetwo3d-ims-tep (SupplierProduct overlap) → preferredSupplierId field, SupplierProduct kept - onetwo3d-ims-2uu (timing/rollback) → PO_SENT trigger, skipPreferred, preferredSupplierLocked, no rollback - onetwo3d-ims-f91 (forecast evidence) → reorderEvidence JSONB on PO line P2 weighting on the new findings because each leaves a load-bearing feature with a silent failure mode. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
OneTwo3D
left a comment
There was a problem hiding this comment.
PR #156 — Product lifecycle + supplier reorder planning (P8.1 + P8.2)
Implementation of Group 21 from PR #155. 39 files / +976/-133. Migrates ProductLifecycleStatus enum, adds preferredSupplierId + preferredSupplierLocked to Product, skipPreferredSupplierUpdate to PurchaseOrder, reorderEvidence JSONB to PO lines. New lib/products/lifecycle.ts state machine helpers, lib/domain/purchasing/preferred-supplier.ts auto-update on PO_SENT, lib/domain/inventory/product-lifecycle-archive.ts cron-driven archive job, route at /api/cron/product-lifecycle-archive.
PR #155 spec findings — all addressed
| PR #155 ID | Issue | Status |
|---|---|---|
ljw |
Migration path from NOT_FOR_SALE |
✓ — SQL maps NOT_FOR_SALE → EOL |
dn2 |
Incoming stock spec | ✓ — getProductIncomingStock returns structured breakdown across PO/transfer/production/WMS ASN |
tep |
SupplierProduct overlap |
✓ — field named preferredSupplierId; SupplierProduct retained for catalog metadata |
2uu |
Timing / rollback semantics | ✓ — PO_SENT trigger, skipPreferredSupplierUpdate per-PO opt-out, preferredSupplierLocked per-product opt-out, explicit no-rollback policy |
f91 |
Forecast evidence schema | ✓ — reorderEvidence JSONB field on PO line |
Good response to spec feedback. The implementation is clean: backfill query in migration uses DISTINCT ON + COALESCE(poSentAt, updatedAt, createdAt) DESC for ordering, row-level FOR UPDATE locks ordered by id, EOL → ARCHIVED transition is per-candidate transactional.
4 new findings filed in bd on PR #156's implementation (commit 33fbd7ce).
Findings filed in bd
| Priority | bd ID | Title |
|---|---|---|
| P2 | onetwo3d-ims-27w |
getReorderReport uses supplierProducts[0] as primary, preferredSupplier as fallback — inverts auto-update intent |
| P2 | onetwo3d-ims-ywu |
OPEN_WMS_ASN_STATUSES counts CREATE_PENDING / CREATE_IN_FLIGHT as firm incoming stock |
| P2 | onetwo3d-ims-3i3 |
updatePreferredSuppliersForPlacedPurchaseOrder writes no activity log — silent supplier reassignment |
| P3 | onetwo3d-ims-zqe |
Third instance of $executeRaw for SELECT FOR UPDATE — same anti-pattern as PR #127 |
Why P2 on 27w
This one closes the loop on the P8.2 implementation. The auto-update writes the latest-PO-supplier to product.preferredSupplierId. But getReorderReport's displaySupplier logic:
const supplier = product.supplierProducts[0] ?? null
const displaySupplier = supplier ? { ...A from catalog } : product.preferredSupplier ? { ...B } : null…uses supplierProducts[0] (the SupplierProduct catalog row) as the PRIMARY source. preferredSupplier only fills in when no SupplierProduct row exists. So a product that had Supplier A in its catalog AND a recent PO with Supplier B (which set preferredSupplierId=B) shows A in the reorder report. The "latest placed PO wins" signal is silently dropped at the read path.
Cleanest fix: look up the SupplierProduct row matching preferredSupplierId first; fall back to supplierProducts[0]. Gets the catalog metadata for the most-recently-used supplier when both signals exist.
Why P2 on ywu
OPEN_WMS_ASN_STATUSES = ['CREATE_PENDING', 'CREATE_IN_FLIGHT', 'OPEN', 'PARTIALLY_BOOKED_IN'] includes pre-confirmation states. A stuck CREATE_PENDING ASN (queue dead-letter, WMS reject loop) indefinitely counts as "incoming stock", silently deferring archive forever. EOL products with stuck ASNs never auto-archive. Either exclude the pre-confirmation states or time-bound them.
Why P2 on 3i3
updatePreferredSuppliersForPlacedPurchaseOrder performs updateMany and returns the affected count, but writes no activity log. A 200-line PO can flip 200 products' preferred supplier without audit trail. Operators asking "why did the preferred supplier change?" have no answer. Same audit gap pattern as PRs #128/#129/#138 silent dedup branches.
P3 — $executeRaw for SELECT FOR UPDATE
Third instance in the codebase (after PR #127's po-cancellation.ts finding). Both preferred-supplier.ts and product-lifecycle-archive.ts use this anti-pattern in the new code. Worth a codebase sweep + ESLint rule.
Notes
- The migration SQL backfill uses
COALESCE(po.poSentAt, po.updatedAt, po.createdAt)for ordering — robust to historical data wherepoSentAtmay be missing. Good defensive choice. - The auto-archive job's
archiveExhaustedEolProductsis per-candidate transactional (good) but processes sequentially up tolimit=500. For tenants with thousands of EOL products, archive lag is multiple cron ticks. Acceptable but worth documenting. isPurchasableProductStatusincludesDRAFT— operators can place POs for products that aren't yet sellable. Worth a UX warning on PO create for DRAFT products (would prevent the "the goods landed but I can't sell them" surprise).- All 4 bd issues tagged
pr-156for filtering:bd list --labels pr-156
Audited the onboarding wizard (components/onboarding/ + app/(dashboard)/onboarding/) against changes shipped in PRs #143–#156. The wizard hasn't been refreshed to reflect recent infrastructure additions; 4 deviations filed: - onetwo3d-ims-mzs [P2] Onboarding skips PR #152 connection test gate for WC/Xero/Shopify — operators leave wizard with un-tested credentials; sync fails to enable later with a confusing 'must test first' error - onetwo3d-ims-a08 [P2] Products step doesn't surface PR #156 lifecycle (DRAFT/ACTIVE/EOL/ARCHIVED) or preferredSupplier fields; default ACTIVE assumption misses the new DRAFT workflow for review-before-publish - onetwo3d-ims-8o5 [P3] No backup/cron configuration step — operators leave wizard without verifying CRON_SECRET, scheduled backups, or remote upload targets (PR #148 / #149 infrastructure) - onetwo3d-ims-tan [P3] WC webhook secret dual purpose (incoming webhook verification + outgoing PDF request signing) added in PR #154 not explained — operators forget to mirror the value in the WP helper plugin Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three new docs (greenfield): - help-docs/glossary.md — plain-English definitions of FIFO, COGS, RFQ, EOL, landed cost, preferred supplier, realised/unrealised FX, sub-ledger A1/A2/B, connection test gate, etc. The canonical source for term definitions; other articles link to it rather than re-defining. - help-docs/onboarding-walkthrough.md — step-by-step walkthrough of the setup wizard (Company → Currency → Integrations → Products). Covers what each step does, beginner tips, and a "recommended next steps" checklist for backup/cron/security/users that the wizard doesn't itself cover. - help-docs/troubleshooting.md — first-stop reference for common errors: login, inventory, sales/refund, purchasing/FX validation, integrations (connection test gate, tax fallback, FX pending queue), backups, cron rate limits, etc. Refreshes to existing docs (concentrating on recent-PR gaps): - help-docs/inventory.md — lifecycle status (DRAFT/ACTIVE/EOL/ARCHIVED) + preferred supplier auto-update + reorder evidence + EOL auto-archive (PR #156) - help-docs/sales.md — multi-currency FX, realised FX on settlement, tax inclusive/exclusive validation, refund warehouse-scoped idempotency, refunds without shipped source (PRs #135, #136, #142) - help-docs/purchasing.md — FX rate validation (10%/2% bands), PO cancellation cost-layer reversal, preferred supplier auto-update, supplier-scoped reorder drafts, landed-cost freightPoId attribution (PRs #127, #139, #143, #156) - help-docs/settings.md — backup hardening (manifest sidecar, 50MB cap, S3/SFTP remote, restore token), password policy, activity-log redaction, cron rate limits, invariant check (PRs #146, #148, #149, #152, #154, #155) - help-docs/woocommerce.md — connection test gate, tax fallback policy (block non-zero / warn zero-rated), FX pending retry queue, customer PDF download via WP helper plugin handoff (PRs #144, #152, #154) - help-docs/xero-sync.md — connection test gate, realised + unrealised FX, daily batch caps with deterministic split-batch refs (PRs #142, #149, #152) - help-docs/getting-started.md — link to new beginner docs; lifecycle intro; "where to find help" table - help-docs/README.md — articles index with sections (getting started, day-to-day, administration, integrations) Entry points and deployment: - README.md — link to new beginner docs; production env-var note (CRON_SECRET, RATE_LIMIT_BACKEND, backup cron) - docs/installation.md — added env vars RATE_LIMIT_BACKEND, DATABASE_RESTORE_MAX_FILE_BYTES, XERO_DAILY_BATCH_LIMIT, WC_PENDING_FX_ORDER_NOTIFY_THRESHOLD; new "Production Readiness Checklist" section covering env, crons, backup, integrations, security, monitoring The onboarding wizard itself is unchanged (audited but flagged for separate work in bd-mzs/a08/8o5/tan); these docs explain what the wizard does set up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Phase 2 doc refresh covering docs not reached in the previous pass: - user-management.md: fix stale password policy (was 8 chars, now 12 + uppercase/lowercase/number/symbol + deny-list per PRs #146/#154) - analytics.md: full rewrite - adds 50K source-row cap (PR #150), VAT inclusive/exclusive handling, AR/AP aging, dead stock report, preferred-supplier scoping in reorder forecast (PR #156), cash bridge, margin/COGS reports, troubleshooting matrix - documents-email.md: invoice PDF token binding (PR #146/#154) and the WC customer-facing PDF download flow via wc-invoice-handoff plugin - activity-log.md: redaction coverage, cost-layer snapshot revaluation entries (PR #144), connection-test fingerprint events (PR #152), cron rate-limit hits (PR #149) - manufacturing.md: lifecycle states (DRAFT/ACTIVE/EOL/ARCHIVED) effect on assembly/disassembly orders and components (PR #156) - stock-control.md: explicit FIFO invariants (PR #126/#139) - strict oldest-first consumption, landed-cost retro updates, transfers preserve per-layer cost - dashboard.md: System Health card (integrations, cron, backup, invariants, pending FX queue) - docs/backup-restore.md: manifest sidecar + checksum (PR #148), DATABASE_RESTORE_MAX_FILE_BYTES cap (50MB), disk-space preflight, restore token session+IP binding - docs/architecture.md: lifecycle status table, password policy + rate limiter section, full cron table (Xero daily batch / payment poll / WC pending FX retry / auto-archive EOL), cron rate limits, connection test gate, new models (WcPendingFxOrder, ConnectionTestRecord, BackupManifest), multi-currency fxRateToBase
Three new docs (greenfield): - help-docs/glossary.md — plain-English definitions of FIFO, COGS, RFQ, EOL, landed cost, preferred supplier, realised/unrealised FX, sub-ledger A1/A2/B, connection test gate, etc. The canonical source for term definitions; other articles link to it rather than re-defining. - help-docs/onboarding-walkthrough.md — step-by-step walkthrough of the setup wizard (Company → Currency → Integrations → Products). Covers what each step does, beginner tips, and a "recommended next steps" checklist for backup/cron/security/users that the wizard doesn't itself cover. - help-docs/troubleshooting.md — first-stop reference for common errors: login, inventory, sales/refund, purchasing/FX validation, integrations (connection test gate, tax fallback, FX pending queue), backups, cron rate limits, etc. Refreshes to existing docs (concentrating on recent-PR gaps): - help-docs/inventory.md — lifecycle status (DRAFT/ACTIVE/EOL/ARCHIVED) + preferred supplier auto-update + reorder evidence + EOL auto-archive (PR #156) - help-docs/sales.md — multi-currency FX, realised FX on settlement, tax inclusive/exclusive validation, refund warehouse-scoped idempotency, refunds without shipped source (PRs #135, #136, #142) - help-docs/purchasing.md — FX rate validation (10%/2% bands), PO cancellation cost-layer reversal, preferred supplier auto-update, supplier-scoped reorder drafts, landed-cost freightPoId attribution (PRs #127, #139, #143, #156) - help-docs/settings.md — backup hardening (manifest sidecar, 50MB cap, S3/SFTP remote, restore token), password policy, activity-log redaction, cron rate limits, invariant check (PRs #146, #148, #149, #152, #154, #155) - help-docs/woocommerce.md — connection test gate, tax fallback policy (block non-zero / warn zero-rated), FX pending retry queue, customer PDF download via WP helper plugin handoff (PRs #144, #152, #154) - help-docs/xero-sync.md — connection test gate, realised + unrealised FX, daily batch caps with deterministic split-batch refs (PRs #142, #149, #152) - help-docs/getting-started.md — link to new beginner docs; lifecycle intro; "where to find help" table - help-docs/README.md — articles index with sections (getting started, day-to-day, administration, integrations) Entry points and deployment: - README.md — link to new beginner docs; production env-var note (CRON_SECRET, RATE_LIMIT_BACKEND, backup cron) - docs/installation.md — added env vars RATE_LIMIT_BACKEND, DATABASE_RESTORE_MAX_FILE_BYTES, XERO_DAILY_BATCH_LIMIT, WC_PENDING_FX_ORDER_NOTIFY_THRESHOLD; new "Production Readiness Checklist" section covering env, crons, backup, integrations, security, monitoring The onboarding wizard itself is unchanged (audited but flagged for separate work in bd-mzs/a08/8o5/tan); these docs explain what the wizard does set up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Phase 2 doc refresh covering docs not reached in the previous pass: - user-management.md: fix stale password policy (was 8 chars, now 12 + uppercase/lowercase/number/symbol + deny-list per PRs #146/#154) - analytics.md: full rewrite - adds 50K source-row cap (PR #150), VAT inclusive/exclusive handling, AR/AP aging, dead stock report, preferred-supplier scoping in reorder forecast (PR #156), cash bridge, margin/COGS reports, troubleshooting matrix - documents-email.md: invoice PDF token binding (PR #146/#154) and the WC customer-facing PDF download flow via wc-invoice-handoff plugin - activity-log.md: redaction coverage, cost-layer snapshot revaluation entries (PR #144), connection-test fingerprint events (PR #152), cron rate-limit hits (PR #149) - manufacturing.md: lifecycle states (DRAFT/ACTIVE/EOL/ARCHIVED) effect on assembly/disassembly orders and components (PR #156) - stock-control.md: explicit FIFO invariants (PR #126/#139) - strict oldest-first consumption, landed-cost retro updates, transfers preserve per-layer cost - dashboard.md: System Health card (integrations, cron, backup, invariants, pending FX queue) - docs/backup-restore.md: manifest sidecar + checksum (PR #148), DATABASE_RESTORE_MAX_FILE_BYTES cap (50MB), disk-space preflight, restore token session+IP binding - docs/architecture.md: lifecycle status table, password policy + rate limiter section, full cron table (Xero daily batch / payment poll / WC pending FX retry / auto-archive EOL), cron rate limits, connection test gate, new models (WcPendingFxOrder, ConnectionTestRecord, BackupManifest), multi-currency fxRateToBase
Summary
Validation