68 lines
3.0 KiB
Markdown
68 lines
3.0 KiB
Markdown
# FirstPrinciples Review — council-phase2-assessment.md
|
|
|
|
**Reviewer**: council/FirstPrinciples
|
|
**Date**: 2026-04-21
|
|
**Document reviewed**: `reviews/council-phase2-assessment.md` (BackendArchitect + FrontendDev joint output)
|
|
**Verdict**: `[APPROVE]`
|
|
|
|
---
|
|
|
|
## Review Summary
|
|
|
|
The merged assessment accurately captures the root causes and recommended fixes. My review confirms alignment with my independent analysis and adds supplementary first-principles commentary.
|
|
|
|
---
|
|
|
|
## What the Assessment Gets Right
|
|
|
|
### P0 — submit() GET vs POST
|
|
|
|
Correctly identified that `location.href` produces a GET request that never calls `BuyDataStorage()`. The field name mismatch (`goods_params` vs `goods_data`) is also correctly flagged.
|
|
|
|
The recommended fix (hidden form POST with `goods_data`) is minimal and correct.
|
|
|
|
### P1 — Zoom container
|
|
|
|
The `.vr-zoom-container` wrapper solution is the right approach. Shared `transform-origin: center top` ensures both stage and seats scale from the same anchor.
|
|
|
|
### P1 — spec loading
|
|
|
|
Correctly identifies that ShopXO's spec matching is `type:value` string-based, not ID-based. The recommendation to use a plugin-specific API endpoint for sold-seats is architecturally sound.
|
|
|
|
---
|
|
|
|
## FirstPrinciples Supplementary Commentary
|
|
|
|
### A note on spec_base_id_map complexity
|
|
|
|
The assessment recommends keeping the map as-is. From a first-principles view, this is a pragmatic choice (performance optimization) but should be documented as such. Future maintainers should know:
|
|
- `spec_base_id_map` is a **performance cache**, not a business invariant
|
|
- If `BatchGenerate` reruns and clears the map, the frontend will have stale data
|
|
- The authoritative source of truth is `GoodsSpecBase` + `GoodsSpecValue`, not the JSON map
|
|
|
|
### Shopping cart is not the problem
|
|
|
|
The assessment frames Issue 1 as a "cart format" issue. From first-principles, the more accurate framing is: **the Buy pathway is correct; only the transport mechanism is wrong**. The code already bypasses cart (goes directly to `Buy::Index()`). The fix should be framed as "fixing the Buy transport" not "fixing cart format."
|
|
|
|
### The real P0 no one mentioned
|
|
|
|
`onOrderPaid()` is the seat-uniqueness authority. No assessment document addresses whether this hook is correctly implemented. If it isn't, no amount of frontend work matters — duplicate sales can still occur. This should be verified before any release.
|
|
|
|
### New finding: GetGoodsViewData() returns only first session
|
|
|
|
Confirmed from `SeatSkuService.php`: `validConfigs[0]` means multi-session products show only the first session. This is a latent bug that will surface when multiple shows are configured. Not in the original 4-issue list — flagging as **P2 (latent)**.
|
|
|
|
---
|
|
|
|
## Recommended Additions to council-phase2-assessment.md
|
|
|
|
1. **Framing correction**: Issue 1 is "Buy transport broken" not "cart format error"
|
|
2. **onOrderPaid audit**: Add as prerequisite before P0 fix deployment
|
|
3. **GetGoodsViewData multi-session bug**: Add to issue list as P2 latent
|
|
|
|
---
|
|
|
|
## Vote
|
|
|
|
`[APPROVE]` — Assessment quality meets standard. Ready for implementation phase.
|