From f04cbb8ca26367da3ef534e9c7ec24c3a3ded81d Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Sun, 24 May 2026 23:59:05 +0200 Subject: [PATCH] docs(audit): lessons learned from loyalty migration Adds a post-audit section to the persona-template consolidation audit capturing what came out of the in-prod card-detail test on rewardflow.lu vs fashionhub.rewardflow.lu: - Template alignment != data alignment: shared partial guarantees the markup is the same per persona, NOT that the API response is. Loyalty's category column rendered empty on merchant + admin because only the store route enriched category_names. Future migrations should diff API response shapes per persona, not just templates. Fixed in d32c1fd5. - Locale-aware formatters are infrastructure, not per-feature. The hardcoded 'en-US' bug spanned 27 callsites across 20+ files. Now swept (dd1f9af8 + 06e59f73 + bb4c4004) and locked down by the JS-016 architecture rule at error severity (eaf180c6). - Sweep + rule, not just sweep. Each cleanup should land with a matching arch rule so the work doesn't decay. Table of the three rules currently guarding this surface (TPL-016, FE-024, JS-016). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../persona-template-consolidation-audit.md | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/docs/proposals/persona-template-consolidation-audit.md b/docs/proposals/persona-template-consolidation-audit.md index 8be3e5f5..d86f3810 100644 --- a/docs/proposals/persona-template-consolidation-audit.md +++ b/docs/proposals/persona-template-consolidation-audit.md @@ -207,3 +207,42 @@ These look like duplicates but should stay separate. The cost of forcing them in - Shared macros under `app/templates/shared/macros/` — those are already shared infrastructure. - The 5 already-suppressed loyalty exceptions — already documented inline. - Storefront / customer-facing templates — different audience, not in scope. + +--- + +## Lessons learned from the loyalty migration (post-audit notes) + +These came out of in-prod testing on `rewardflow.lu/merchants/loyalty/cards/{id}` vs `/store/.../loyalty/cards/{id}` after the loyalty consolidation shipped. They sharpen the migration recipe for the rest of the backlog. + +### 1. Template alignment ≠ data alignment + +The shared partial guarantees the **markup** is the same across personas. It doesn't guarantee the **API response** is. Loyalty's `card-detail-view.html` had a `show_category_column` flag, but the column rendered empty on merchant + admin because only the **store** route enriched `tx.category_names` from `category_ids` via `category_service.validate_category_for_store`. Merchant + admin returned raw rows. + +**Recipe for every migration**: after wiring up the shared partial, hit each persona's endpoint in a browser (or `curl` to the JSON) and **diff the response shapes**. Any optional/enriched field used by the shared template must be populated by every persona's route, or the shared template must gracefully render `-` for missing data. Fixed in commit `d32c1fd5`. + +### 2. Locale-aware formatters are infrastructure, not per-feature + +The same bug — hardcoded `'en-US'` in `toLocaleDateString` / `Intl.NumberFormat` — turned up in **27 places** across **20+ JS files** (5 loyalty shared factories, 8 loyalty persona files, 13 non-loyalty files + the shared `Utils` helper). All of them were silently rendering dates and numbers in English even when the dashboard language was French. + +The fix landed as three swept commits + a new architecture rule: + +| Commit | Scope | +| --- | --- | +| `dd1f9af8` | 5 loyalty shared factories + new `I18n.locale` getter on `static/shared/js/i18n.js` | +| `06e59f73` | 13 non-loyalty files (catalog, marketplace, orders, tenancy, inventory, monitoring, cms, storefront layout, shared Utils) | +| `bb4c4004` | 8 remaining loyalty persona files (admin/merchant/store/storefront) | +| `eaf180c6` | New `JS-016` architecture rule at **error severity** — CI rejects any future hardcoded locale tag | + +**Recipe**: don't write `'en-US'` ever; use `I18n.locale`. The rule will reject the PR otherwise. Suppressible per-line with `// noqa: JS-016` for the genuine US-only formatter case. + +### 3. Sweep + rule, not just sweep + +Sweeping the codebase clean is necessary but not sufficient — without a rule, the next contributor reintroduces the pattern. Every consolidation-style cleanup in this audit should land with a matching architecture rule (warning or error) so the work doesn't decay. Current rules guarding this surface area: + +| Rule | Severity | What it blocks | +| --- | --- | --- | +| `TPL-016` | warning | Persona templates >75 LOC that don't include a `*/shared/*` partial | +| `FE-024` | warning | Raw `url_for()` on JS/CSS instead of `static_v()` | +| `JS-016` | **error** | Hardcoded `'en-US'` in `toLocale*` / `Intl.*` calls | + +When a Wave 1-3 migration lands, consider whether it deserves a new rule (e.g., "messaging shared factories must accept a `config` arg") — small, focused rules that prevent regression are cheap and high-value.