sboulahtit/orion
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

docs(audit): lessons learned from loyalty migration
All checks were successful
CI / ruff (push) Successful in 18s
Details
CI / pytest (push) Successful in 2h51m39s
Details
CI / validate (push) Successful in 40s
Details
CI / dependency-scanning (push) Successful in 36s
Details
CI / docs (push) Successful in 54s
Details
CI / deploy (push) Successful in 1m46s
Details

Browse Source 
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) <noreply@anthropic.com>
This commit is contained in:
Samir Boulahtit
2026-05-24 23:59:05 +02:00
parent eaf180c64f
commit f04cbb8ca2
1 changed files with 39 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
docs/proposals/persona-template-consolidation-audit.md
View File

@@ -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.