Skip to content

feat(localization): request localization, localized errors and user locale (1/4) - #1360

Open
marcelo-maciel wants to merge 1 commit into
fullstackhero:mainfrom
marcelo-maciel:feat/i18n-framework
Open

feat(localization): request localization, localized errors and user locale (1/4)#1360
marcelo-maciel wants to merge 1 commit into
fullstackhero:mainfrom
marcelo-maciel:feat/i18n-framework

Conversation

@marcelo-maciel

@marcelo-maciel marcelo-maciel commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Framework slice of the i18n work, split out of #1344 as you asked. This is the one that needs real scrutiny; 56 files.

The split is four PRs rather than three. Your three were framework / clients/admin / clients/dashboard, but the ~220 files of module-level localization fit none of them, and folding them into the framework PR would put it back at ~285 files and defeat the point. So the module catalogs and their handler/validator wiring live in their own PR.

Slice Files PR
Framework 56 this one
Module catalogs and wiring 229 #1361 — depends on this one
clients/admin 111 #1362 — independent
clients/dashboard 134 #1363 — independent

The union of the four is byte-identical to #1344's tree, with empty pairwise intersection apart from the one shared src/Directory.Packages.props hunk. That is asserted by a script, not by eye: for each slice, git diff --quiet feat/i18n <slice> -- <its paths> and git diff --name-only main <slice> equal to its declared path set.

The two front-end PRs depend on nothing here and can be reviewed in parallel. The module PR does not compile without this one — verified, not assumed: src/Modules/** applied alone on main fails with 862 compile errors, all rooted in FSH.Framework.Core.Localization not existing.

⚠️ Touches protected src/BuildingBlocks (Golden Rule #4)

Seventeen files here, needing maintainer sign-off:

CoreCore.csproj; Exceptions/ (CustomException, ForbiddenException, UnauthorizedException, and the new ILocalizableMessage, LocalizedKeyNotFoundException, LocalizedUnauthorizedAccessException); Localization/ (new SharedResources marker + SupportedCultures + the two shared catalogs).

WebExtensions.cs (registers and orders the localization middleware, +6 lines); Exceptions/GlobalExceptionHandler.cs; new Localization/ (LocalizationExtensions, UserLocaleRequestCultureProvider).

JobsExtensions.cs, one exception message. StorageQuotaMeteredStorageService.cs, one exception message.

One eighteenth BuildingBlocks file is in the module PR instead, and I want to be upfront about it: Web/Validation/PagedQueryValidator.cs. Its constructor now takes IStringLocalizer<SharedResources>, and all three subclasses (GetAuditsQueryValidator, GetTenantsQueryValidator, SearchUsersQueryValidator) live in modules. Keeping the base class here would either break this PR's build or drag the Auditing catalog and handler in with it; sending it with its three callers keeps both PRs compiling on their own and puts the change in front of the code it affects. It is declared under Golden Rule #4 there too.

No existing behaviour of other building blocks is altered. src/Directory.Packages.props carries one addition, the SSH.NET pin discussed at the end.

UseRequestLocalization sets the UI culture only

You asked whether UI-culture-only was considered. It is what ships.

main has no request localization at all, so pinning the formatting culture is less change than negotiating it: CultureInfo.CurrentCulture behaves exactly as it does on main today, and only resource lookup follows the request. For an API whose output is JSON that is the safer default, and it makes the CA1305 question moot rather than merely bounded.

It is not one switch. RequestLocalizationMiddleware.SetCurrentThreadCulture assigns both cultures unconditionally, so the culture half has to be pinned:

  • DefaultRequestCulture carries (InvariantCulture, configured default). The middleware resolves the culture half as cultureInfo ??= DefaultRequestCulture.Culture, making invariant the only reachable value.
  • SupportedCultures is null, so the middleware skips culture filtering entirely. A one-element [InvariantCulture] list behaves identically but logs UnsupportedCultures on every request — the middleware's parent-culture walk bails at the empty culture name, so invariant is unmatchable by design.

With formatting out of the negotiation, the neutral pt/en entries in RequestMatch bought nothing and are gone; SupportedCultures.Tags is the single, specific-only list. A request asking for a bare pt or an unsupported variant resolves to the configured default. Both React apps canonicalise variants onto supported tags before calling the API, so app traffic is unaffected; a hand-rolled client sending bare pt gets the default.

Message arguments are culture-insensitive too. The localizer formats with string.Format under CurrentCulture, so a double or DateTime in a message would render with an invariant separator. Every MessageArgs site and every localizer["…", …] call site was enumerated: all int, long, string or enum, except MaxWindow.TotalDays in the two audit-window validators, which is now an int at the source (that change travels with the module PR).

Catalogs are named for specific cultures

SharedResources.pt-BR.resx here, and the same convention for the ten module catalogs in the module PR. Renames only, no string changed.

The asymmetry with the front-end is gone, and so is the trap behind it: a future pt-PT is no longer served Brazilian strings by parent fallback. The documented consequence is that a bare pt or an unsupported variant lands on the neutral English catalog rather than on Portuguese. Adding a language is: add the specific tag to SupportedCultures.Tags, add a *.{tag}.resx per catalog, add the JSON catalogs to both apps, and drop it from the front-end CANON map if it was being folded into another tag. .agents/rules/localization.md records all of this.

The Locale column

The original summary was wrong: there is no DB default. The column is nullable with en-US as a code-level fallback, and it is character varying(10) rather than unbounded text — 10 covers language-script-region (zh-Hant-TW). AddUserLocale was edited in place rather than stacked with an ALTER, since it has never shipped in a release.

Confirmed as you asked: Validation.UnsupportedLocale is wired at the write boundary. UpdateUserCommandValidator restricts Locale to SupportedCultures.Tags, on PUT /identity/profile, via the Mediator ValidationBehavior. The column constraint is the storage-level backstop, not the validation.

Because whole files cannot be split across PRs, three Identity files carry both the Locale plumbing and their IdentityResources wiring in the same diff (IdentityService, UserProfileService, StartImpersonationCommandHandler). They ship here, which is why IdentityResources and its two catalogs ride along in this PR rather than in the module one.

Known behaviour (documented, not bugs)

  • The locale claim lags a language switch by one token. The provider reads the JWT claim, so a switch reaches the API at the next token issue. The front-end persists to the profile and re-mints, so it converges; in between, the shell can be in the new language while an API error is still in the old one. The alternative is a per-request DB read on every authenticated call.
  • SignalR does not carry the app locale. The hub client builds its own requests instead of going through apiFetch, so Accept-Language on the negotiate is the browser's. Applies to every session, not just impersonation. Named explicitly in the front-end handoff-locale.spec.ts so any other channel that stops carrying the locale fails the test.
  • Middleware registered before localization renders its errors in the default language. UseExceptionHandler() sits ahead of UseHeroLocalization(), which in turn has to sit after UseAuthentication() because the culture provider reads the locale claim off HttpContext.User. An exception thrown by anything in between — HTTPS redirection, CORS, static files, routing — is therefore rendered in the configured default culture rather than the caller's. Endpoint handlers, where every localized exception in this codebase is actually thrown, are unaffected. Moving the exception handler below localization would leave those middlewares with no ProblemDetails at all, which is the worse trade, so this stays as documented behaviour rather than being papered over.

Also in this slice, from the last review round

  • LogContext.PushProperty is scoped in using blocks. Pre-existing AsyncLocal leak that contaminated every subsequent log entry in the request; unrelated to i18n, fixed here because the same lines were being touched.
  • Unmapped status codes are no longer titled "an unexpected error occurred". TitleKeyFor sent everything outside four statuses to Error.Unexpected; the type-name fallback beside it only fires on ResourceNotFound, and that key resolves, so it never fired. #1344 regressed this — before it, Title was the exception type name. Unmapped statuses fall back to the type name again, and Conflict gets a real localized title. The 41 Conflict throw sites this affected are in Billing and Catalog, so the visible half of that fix lands with the module PR.
  • ExceptionSeverityClassifier is now exercised with the Localized* subclasses. They subclass the BCL types precisely so audit severity classification keeps working; changing a base type would have silently reclassified every unauthorized access with the suite green.

The SSH.NET pin is carried from #1333

NU1903 / GHSA-q939-rpr3-3284 on SSH.NET 2025.1.0, pulled transitively by Testcontainers, fails restore for the whole solution under TreatWarningsAsErrors — on main too: dotnet restore src/FSH.Starter.slnx at 3f2959e6 fails identically, re-verified today. It is not introduced here, and the fix properly belongs to #1333.

Rather than leave this red on someone else's advisory, the pin is carried byte-identical to #1333's version of the file, comment included. That keeps both mergeable in either order: git merge-tree on both orderings yields a clean tree holding a single SSH.NET entry, and the merged file is identical to what #1333 alone produces. The byte identity is what buys that — the same pin under a reworded comment conflicts. If #1333's pin changes during review this copy should be matched rather than allowed to drift, and once #1333 lands first it can simply be dropped. All four slices carry it, including the two front-end ones: template-smoke.yml runs on clients/** and builds the scaffolded solution, which includes src/Tests/**.

Testing

Every number below is this slice on its own, at main plus these 56 files.

  • dotnet restore src/FSH.Starter.slnx with the audit on: exit 0, no NU1903.
  • dotnet build -warnaserror: exit 0.
  • Full suite: 14 test assemblies, 1868 passed / 0 failed / 1 skipped, including Integration (Testcontainers/Postgres, Docker) at 746 passed / 1 skipped. #1344 reported 15 assemblies and 1891 passed; the missing assembly is Tickets.Tests, which the module PR adds to the solution, and the 23-test difference is that project plus the module-catalog tests. Nothing was dropped — the per-slice sums add back up.

The verdict above is aggregated per assembly rather than taken from the process exit code: dotnet test on this solution has been observed exiting 0 while reporting failures, and zero assemblies reporting is itself treated as red.

Docs (Golden Rule #10)

fullstackhero/docs#238, kept as a single PR covering all four slices — internationalization.mdx is one page whose sections map across the split, so cutting it into four would put four PRs on the same file and leave three describing half a feature. From this slice it documents the culture resolution chain, per-user language, the new config section and code on ProblemDetails, all of which are public contract.

It should merge after the last of the four, not with this one: landing it here alone would publish the module-catalog and front-end sections before that code is on main.

Notes

  • en-US and pt-BR are held at strict key and placeholder parity, enforced by tests, so a missing or mis-arged translation fails the build instead of shipping English. The generic reflection-driven CatalogParityTests that covers every module catalog travels with the module PR.
  • The lost update behind the front-end hydration guard is a contract change to PUT /identity/profile and is tracked separately in #1359.

…ocale

Framework slice of the i18n work (split of fullstackhero#1344 as requested in review).

- `SharedResources` catalog (en + pt-BR) and `SupportedCultures` as the single
  source of supported tags.
- `CustomException` carries `MessageKey`, `MessageArgs` and `ResourceSource`;
  `Message` stays English so logs remain culture-independent.
  `ILocalizableMessage` subclasses keep `UnauthorizedAccessException` /
  `KeyNotFoundException` as base types so audit severity classification is
  unaffected.
- `GlobalExceptionHandler` localizes `title`/`detail` and surfaces the message
  key as a stable `code` extension on ProblemDetails.
- `UseHeroLocalization` request-localization chain, UI-culture-only:
  `CurrentCulture` stays invariant, only `CurrentUICulture` is negotiated.
  `UserLocaleRequestCultureProvider` reads the `locale` claim, so the
  middleware sits between `UseAuthentication` and `UseAuthorization`.
- `User.Locale` (`varchar(10)`, nullable, no database default; `en-US` is a
  code-level fallback) plus the `AddUserLocale` migration, the `locale` claim
  emission and the write-boundary validator rejecting tags outside
  `SupportedCultures.Tags`.
- `LogContext.PushProperty` scoped in `using` blocks, fixing a pre-existing
  AsyncLocal leak that contaminated later log entries in the same request.
- `SSH.NET` pin (`2026.0.0`), byte-identical to fullstackhero#1333, so `dotnet restore`
  passes while that PR is open.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant