Skip to content

feat(localization): module catalogs and localized messages (2/4) - #1382

Open
marcelo-maciel wants to merge 21 commits into
fullstackhero:mainfrom
marcelo-maciel:feat/i18n-modules
Open

marcelo-maciel wants to merge 21 commits into
fullstackhero:mainfrom
marcelo-maciel:feat/i18n-modules

Conversation

@marcelo-maciel

@marcelo-maciel marcelo-maciel commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Carries two infrastructure fixes that are not this PR's topic. Without them CI cannot even reach this PR's code.

  • Dependency bump: Microsoft.SourceLink.GitHub to 10.0.401 and Testcontainers to 4.14.0, clearing NU1902/NU1903 so restore succeeds. Those are the versions #1375 (SourceLink) and #1369 (Testcontainers) carry: this hunk is the union of the two, plus one comment per pin naming the advisory it answers.
  • MinIO (560111e0): minio/minio is gone from Docker Hub, so every Testcontainers-backed integration test dies on the image pull. Pulls from quay.io on a pinned tag instead. Same fix as #1388.

The MinIO hunk is byte-identical to #1388. The dependency hunk is not byte-identical to #1375 or #1369, which each carry half of it without the comments, but it is identical across all twelve PRs in this series: src/Directory.Packages.props resolves to the same blob (854deb95) at every head. Either way they merge in any order, and these copies can be dropped once the PRs that own them land.

Reopened from #1361. That PR was closed automatically on 2026-09-14, when the head fork
was deleted. It reopened at 7962ea4f, and review has added commits on top since then (the
commit list above is the current one). The earlier review history stays on #1361.


Module slice of the i18n work, split out of #1344. 229 files, and mechanical: one catalog per module plus the MessageKey / ResourceSource wiring at each throw and validation site.

This PR carries the framework slice as a merge commit, so it builds and the suite is green here. It cannot be based on #1381 — a PR opened from a fork cannot target another PR's branch — so the branch merges feat/i18n-framework instead. The two are therefore reviewed together in this diff; once #1381 lands, merging main in collapses that half to nothing. The rest of this note is what the dependency is. It uses SharedResources, SupportedCultures and the MessageKey / MessageArgs / ResourceSource members on CustomException, none of which exist on main yet. A PR opened from a fork cannot be based on another PR's branch, so there is no way to stack it. It is up now so you can see the whole shape at once; rebasing it on main after the framework PR lands turns it green with no content change. Verified rather than assumed: framework + this slice together build clean and run the full suite green, and the combined tree is byte-identical to #1344 on every backend path.

The split is four PRs rather than three, because these ~220 files fit none of your three and would have put the framework PR back at ~285.

Slice Files PR
Framework 56 #1381review this one first
Module catalogs and wiring 229 this one
clients/admin 111 #1383 — independent
clients/dashboard 134 #1384 — independent

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

One file: Web/Validation/PagedQueryValidator.cs. Its constructor now takes IStringLocalizer<SharedResources> so the paging messages localize. Its three callers are in modules — GetAuditsQueryValidator, GetTenantsQueryValidator, SearchUsersQueryValidator — so the class travels with them. (They are callers, not subclasses: the class is sealed and is pulled in with Include(...). The earlier wording was wrong.) Leaving it in the framework PR would either break that PR's build (its subclasses on main call the old constructor) or drag AuditingResources and GetAuditsQueryHandler in with it, for no review benefit. The other seventeen BuildingBlocks files are declared in the framework PR.

What is in here

  • One {Module}Resources catalog per module, en-US neutral plus *.pt-BR.resx, for Auditing, Billing, Catalog, Chat, Files, Multitenancy, Notifications, Tickets and Webhooks, plus the Identity messages outside the Locale plumbing.
  • Throw sites carry MessageKey and ResourceSource. Message stays English, so logs and audit records remain culture-independent while the response body localizes. This is where the 41 Conflict throws across Billing and Catalog live — the ones that #1344 had briefly answering 409 with "an unexpected error occurred" until TitleKeyFor was fixed in the framework slice.
  • Validators resolve their messages through IStringLocalizer. Message arguments are kept culture-insensitive: GetAuditsQueryHandler.MaxWindowDays is a new int constant, replacing MaxWindow.TotalDays as a double in the two audit-window validators. The localizer formats with string.Format under CurrentCulture, and a double in a localized message is culture-sensitive by construction.
  • CatalogParityTests discovers every catalog by reflection and compares each culture's own key set with tryParents: false, plus the placeholder-index set per key. {1} present in one culture and not the other throws FormatException at render time, in that culture only. New module catalogs are covered without a new test — Notifications had shipped a catalog with no parity test and has no test project to hold one.
  • Tickets.Tests joins the solution, which had no test project for that module.
  • A built-in-validation test that asserted only the absence of the English string (satisfied equally by a blank message or a leaked resource key) now pins the Portuguese text.

Testing

Because this slice cannot build alone, the numbers below are framework + this slice, which is what will be on main once both land.

  • dotnet restore src/FSH.Starter.slnx with the audit on: exit 0, no NU1903.
  • dotnet build -warnaserror: exit 0.
  • Full suite, re-run on the merged branch: 15 test assemblies, 1915 passed / 0 failed / 1 skipped, including Integration (Testcontainers/Postgres, Docker) at 747 passed / 1 skipped. That is the 1891 first reported plus the 24 the review follow-ups added.
  • The earlier "1891 passed / 0 failed" was only true on an English machine. Twenty tests across four assemblies asserted an exact English message while the messages had just moved into a resx that resolves against CultureInfo.CurrentUICulture, so on a pt-BR developer machine they failed (11 in Identity, 6 in Generic, 2 in Webhooks, 1 in Catalog) and on CI they passed. The suite was asserting a property of the developer's operating system. Each of those assemblies now pins the UI culture in a [ModuleInitializer], and the numbers above are from a pt-BR machine.
  • The combined tree is byte-identical to #1344 on every path under src/, .agents/ and AGENTS.md: git diff between them is empty there.

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.

dotnet restore passes with the audit on. The explicit SSH.NET pin that used to sit in src/Directory.Packages.props is gone — Testcontainers 4.14.0 already depends on the patched version, so it pinned nothing. See the note at the end.

Docs (Golden Rule #10)

fullstackhero/docs#238, kept as a single PR covering all four slices. The String resources section of internationalization.mdx is this slice: the per-module {Module}Resources catalogs and how a localized message is wired at a throw or validation site. That PR should merge after the last of the four.

Review follow-ups

An independent review of this slice produced the following. All are in the branch:

  • The suite was culture-dependent (detail above): four test assemblies now pin the UI culture.
  • Enum arguments reached the user in English. Seven messages take an enum as an argument and
    string.Format wrote the C# member name straight into the translated sentence: "Não é possível
    atribuir um chamado no status Closed", "... porque está Pending". The catalogs were fully
    translated and the visible result still was not. GlobalExceptionHandler now resolves an enum
    argument as "{EnumType}.{Member}" against the same catalog as the message, falling back to
    ToString() when there is no entry — the behaviour every argument has today, so a new enum reaching
    a message before its keys do degrades rather than breaks. Keys added for TicketStatus,
    FileAssetStatus, Visibility, TopupRequestStatus and TenantProvisioningStatus, and each
    module's resource test now asserts every member of its enums is present in both catalogs.
  • Ticket.Assign passed the verb as an argument (ThrowIfClosedOrResolved("assign")), so the verb
    stayed English whatever the culture. It has one caller, so the action moved into the key
    (Tickets.CannotAssignInStatus) and the argument list keeps only the status.
  • Audit.RealExceptionType's single BaseType step was flagged as fragile. It is correct while
    the localization wrappers are sealed, which they are, so the walk is not worth writing — the premise
    is gated instead: every ILocalizableMessage that is not a CustomException must be sealed.
    Verified by mutation.

Infra carve-outs, corrected after review. Two things in the out-of-topic hunks were wrong and
are fixed on the branch:

  • The MinIO carve-out only moved minio/minio to quay.io. minio/mc is gone from Docker Hub too
    (hub.docker.com/v2/repositories/minio/mc/ answers 404) and it is what minio-init runs, so both
    dotnet run --project src/Host/FSH.Starter.AppHost and docker compose up died on the pull and the
    fsh bucket was never created. Now pinned to the same quay tag #1388 uses.
  • The SSH.NET pin is gone: it pinned nothing. Its own comment claimed bumping Testcontainers
    does not help, but 4.14.0 — which this branch also carries — declares SSH.NET >= 2026.0.0.
    Measured rather than argued: with the pin removed, dotnet restore src/FSH.Starter.slnx --force
    reports zero NU1902/NU1903 and exits 0. (The MessagePack pin next to it stays; removing that one
    does bring its advisory straight back.)

With both applied, deploy/docker/docker-compose.yml and src/Directory.Packages.props are now
genuinely byte-identical to #1388 (git diff --exit-code, checked today), which the earlier claim
was not.

The new test project was building but not running. This PR adds Tickets.Tests and registers it in FSH.Starter.slnx, which turns out not to be the wiring: the unit-test job in backend.yml enumerates test projects by name rather than running the solution, so the entry put the project in the build and nowhere else. Every run on this branch has been green without executing one of its four tests. Tickets is in that list now. Checked locally before wiring it in: dotnet test src/Tests/Tickets.Tests passes 4/4.

…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.
Module slice of the i18n work (split of fullstackhero#1344 as requested in review).
Depends on the framework slice: it uses `SharedResources`, `SupportedCultures`
and the `MessageKey` / `MessageArgs` / `ResourceSource` plumbing on
`CustomException`, none of which exist on `main` yet.

- One `{Module}Resources` catalog per module (en + pt-BR) for Auditing,
  Billing, Catalog, Chat, Files, Multitenancy, Notifications, Tickets and
  Webhooks, plus the Identity messages that were not part of the framework
  slice.
- Domain and handler throws carry `MessageKey` and `ResourceSource` so the
  response body is localized while `Message` stays English for the logs.
- Validators resolve their messages through `IStringLocalizer`, with message
  arguments kept culture-insensitive (`MaxWindowDays` as `int` rather than
  `MaxWindow.TotalDays` as `double`, which `string.Format` would render with a
  culture-dependent decimal separator).
- `CatalogParityTests` pins key parity and placeholder parity across every
  catalog, so a missing or malformed translation fails the build rather than
  reaching a user.
- `Tickets.Tests` joins the solution, which previously had no test project.

Protected code (Golden Rule fullstackhero#4): this slice touches one file under
`src/BuildingBlocks` — `Web/Validation/PagedQueryValidator.cs`. Its constructor
now takes `IStringLocalizer<SharedResources>` so paging messages localize. All
three subclasses live in modules (`GetAuditsQueryValidator`,
`GetTenantsQueryValidator`, `SearchUsersQueryValidator`), so the base class
travels with its callers rather than breaking them from another PR. Every other
`BuildingBlocks` change is in the framework slice.

The `SSH.NET` pin (`2026.0.0`) is byte-identical to fullstackhero#1333, so `dotnet restore`
passes while that PR is open.
…est culture

No exception message this PR localizes was actually translated at runtime.
Every detail resolved from a MessageKey and every title mapped from a status
code came back from the neutral resx, whatever the client asked for.

UseExceptionHandler is registered above UseHeroLocalization, and
RequestLocalizationMiddleware assigns CultureInfo.CurrentUICulture inside its
own async frame. That assignment belongs to the ExecutionContext of that frame
and is gone by the time an exception unwinds up to the handler, so every
localizer there resolved under the culture of the host process -- the invariant
one in a container with no LANG, hence the neutral resx.

GlobalExceptionHandler now reads the culture from
HttpContext.Features.Get<IRequestCultureFeature>(), which the middleware sets
on the request itself and therefore survives the unwind. Reading the negotiated
culture rather than re-reading Accept-Language keeps the whole provider chain,
including the user locale claim. Only CurrentUICulture is touched:
AddHeroLocalization pins CurrentCulture to invariant on purpose. The previous
value is restored in a finally so no request culture leaks onto the thread. When
no feature is present -- an exception escaping before localization runs -- the
ambient culture stands and no Content-Language is claimed.

Also restores Content-Language on the problem body. ExceptionHandlerMiddleware
clears the response before re-executing, which drops the header the
localization middleware had already written, leaving the culture of the prose
undeclared.

GlobalExceptionHandlerLocalizationTests could not catch this: they assign
CurrentUICulture by hand and call the handler directly, never through a
pipeline. ExceptionLocalizationPipelineTests build the real pipeline and pin the
ambient culture to invariant, which is what a container gives the API -- without
that pin a developer machine whose own culture is the tested one reports a false
pass. Against the handler as it stood before this commit, five of those cases
fail; the negotiation baseline and the no-localization case pass either way.
…advisories

`dotnet restore` fails for the whole solution under `TreatWarningsAsErrors`, on
`main` and on every open PR alike. Advisory-database drift, not a regression from
any change: a commit green on 2026-08-10 is red today with no edits.

- `Testcontainers.PostgreSql` / `.Redis` / `.Minio` 4.11.0 -> 4.14.0 (NU1903,
  GHSA-q939-rpr3-3284). 4.11.0 depends on `SSH.NET` 2025.1.0; 4.14.0 already
  depends on the patched 2026.0.0, so the advisory clears with no transitive pin
  to remember to remove later. Same fix as fullstackhero#1369, so the two do not conflict.
- `Microsoft.SourceLink.GitHub` 8.0.0 -> 10.0.401 (NU1902,
  GHSA-23fw-v26w-5fgq). 8.0.0 drags in `Microsoft.Build.Tasks.Git` 8.0.0 and the
  8.x line has no patched release, so a transitive pin cannot fix it; the package
  itself has to move. 10.0.401 depends on `Microsoft.Build.Tasks.Git` 10.0.401,
  past the patched 10.0.303. Build-time only (`PrivateAssets="all"`), referenced
  only where `IsPackable == true`, which is the CLI alone - and `src/Tools/**` is
  excluded from the template, so the scaffold never sees it.

Verified: `dotnet restore src/FSH.Starter.slnx` exits 0 with no NU19xx, and
`dotnet build src/FSH.Starter.slnx -c Release -warnaserror` reports 0 warnings
and 0 errors.
…advisories

`dotnet restore` fails for the whole solution under `TreatWarningsAsErrors`, on
`main` and on every open PR alike. Advisory-database drift, not a regression from
any change: a commit green on 2026-08-10 is red today with no edits.

- `Testcontainers.PostgreSql` / `.Redis` / `.Minio` 4.11.0 -> 4.14.0 (NU1903,
  GHSA-q939-rpr3-3284). 4.11.0 depends on `SSH.NET` 2025.1.0; 4.14.0 already
  depends on the patched 2026.0.0, so the advisory clears with no transitive pin
  to remember to remove later. Same fix as fullstackhero#1369, so the two do not conflict.
- `Microsoft.SourceLink.GitHub` 8.0.0 -> 10.0.401 (NU1902,
  GHSA-23fw-v26w-5fgq). 8.0.0 drags in `Microsoft.Build.Tasks.Git` 8.0.0 and the
  8.x line has no patched release, so a transitive pin cannot fix it; the package
  itself has to move. 10.0.401 depends on `Microsoft.Build.Tasks.Git` 10.0.401,
  past the patched 10.0.303. Build-time only (`PrivateAssets="all"`), referenced
  only where `IsPackable == true`, which is the CLI alone - and `src/Tools/**` is
  excluded from the template, so the scaffold never sees it.

Verified: `dotnet restore src/FSH.Starter.slnx` exits 0 with no NU19xx, and
`dotnet build src/FSH.Starter.slnx -c Release -warnaserror` reports 0 warnings
and 0 errors.
MinIO withdrew `minio/minio` from Docker Hub. Docker Hub's API now answers
`object not found` for the repository, and a pull fails with:

    pull access denied for minio/minio, repository does not exist or may
    require 'docker login'

That takes down every Testcontainers-backed integration test (the harness boots
a MinIO container per fixture, so all 724 tests in `Integration.Tests` fail at
container start), the Aspire AppHost, and the Docker Compose deployment. The
image is still published at `quay.io/minio/minio`:

- `Integration.Tests` and `Integration.Middleware.Tests` harnesses
- `AppHost.cs`, via Aspire's `WithImageRegistry` / `WithImageTag`
- `deploy/docker/docker-compose.yml` and the image table in its README

The tag is pinned to `RELEASE.2025-09-07T16-13-09Z` rather than `:latest`. quay
has not moved `:latest` since 2025-09-07, so the two resolve to the same digest
today; pinning only removes the surprise of a silent move later, and keeps the
test harness off a floating tag. Whether to track a newer release, or a different
S3-compatible image, is a separate call.

While in the README's image table: `postgres` and `redis` rows had drifted from
what compose actually ships (`postgres:18-alpine`, `valkey/valkey:9.1.0-alpine`).

Verified: `docker pull minio/minio:latest` fails with the error above;
`docker pull quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z` succeeds
(`sha256:14cea493d9a34af32f524e538b8346cf79f3321eff8e708c1e2960462bd8936e`, the
same digest `:latest` resolves to). `dotnet test Integration.Tests -c Release`
passes against the pinned image, and the Aspire manifest renders the container
as `quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z`.
MinIO withdrew `minio/minio` from Docker Hub. Docker Hub's API now answers
`object not found` for the repository, and a pull fails with:

    pull access denied for minio/minio, repository does not exist or may
    require 'docker login'

That takes down every Testcontainers-backed integration test (the harness boots
a MinIO container per fixture, so all 724 tests in `Integration.Tests` fail at
container start), the Aspire AppHost, and the Docker Compose deployment. The
image is still published at `quay.io/minio/minio`:

- `Integration.Tests` and `Integration.Middleware.Tests` harnesses
- `AppHost.cs`, via Aspire's `WithImageRegistry` / `WithImageTag`
- `deploy/docker/docker-compose.yml` and the image table in its README

The tag is pinned to `RELEASE.2025-09-07T16-13-09Z` rather than `:latest`. quay
has not moved `:latest` since 2025-09-07, so the two resolve to the same digest
today; pinning only removes the surprise of a silent move later, and keeps the
test harness off a floating tag. Whether to track a newer release, or a different
S3-compatible image, is a separate call.

While in the README's image table: `postgres` and `redis` rows had drifted from
what compose actually ships (`postgres:18-alpine`, `valkey/valkey:9.1.0-alpine`).

Verified: `docker pull minio/minio:latest` fails with the error above;
`docker pull quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z` succeeds
(`sha256:14cea493d9a34af32f524e538b8346cf79f3321eff8e708c1e2960462bd8936e`, the
same digest `:latest` resolves to). `dotnet test Integration.Tests -c Release`
passes against the pinned image, and the Aspire manifest renders the container
as `quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z`.
UpdateUserCommandValidator guards its allow-list rule with
.When(!IsNullOrWhiteSpace), so "" never reaches the allow-list. The
service disagreed: `if (locale is not null)` wrote the empty string
straight onto the user, so a body carrying locale: "" silently wiped a
language the user had chosen. A form that serialises an untouched locale
field as "" clears the preference on every unrelated save, and the
allow-list never sees the value that got stored.

Aligns the service with the validator's reading. The integration-test
UserDto mirror gains Locale, which is why no existing test caught this.

Gates: the new integration test fails on the previous code
(Shouldly: dto.Locale) and passes after; UserProfileTests 8/8,
Identity.Tests 330/330.
@marcelo-maciel

marcelo-maciel commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Backend CI here is red for an ordering reason, not a code one: src/BuildingBlocks/Web/Validation/PagedQueryValidator.cs references FSH.Framework.Core.Localization.SharedResources, which arrives with #1381, slice 1 of 4. Against main the build cannot resolve it (CS0234, CS0246), and it will stay that way until #1381 merges.

Retargeting this PR at feat/i18n-framework would have been the clean fix and GitHub refuses it: the base of a cross-fork PR must be a branch in this repository, and slice 1 lives in a fork. So this branch now carries #1381 instead, the same pattern the rest of the batch uses for the infrastructure fixes it carries, which gets Backend CI compiling. Details in the follow-up comment.

Full merge order for the batch, with the pairwise conflicts measured rather than guessed: #1369 comment.

…ding on the machine

The localized validator messages resolve against CurrentUICulture, so the eleven tests
that assert the exact English string pass on an English machine and fail on any other.
On a pt-BR workstation the suite reports 11 failures for code that is correct; CI, being
en-US, never sees it.

Pinning the culture once for the assembly keeps the assertions as they read and makes the
result a property of the code rather than of the developer's OS.
@marcelo-maciel

Copy link
Copy Markdown
Contributor Author

Two updates on this PR.

It now carries #1381 until that one merges. Retargeting this PR at feat/i18n-framework was the cleaner option and GitHub rejects it: the base of a cross-fork PR has to be a branch in this repository, and slice 1 lives in a fork (Proposed base branch 'feat/i18n-framework' was not found). So feat/i18n-framework is merged into this branch instead, the same pattern the rest of this batch already uses for the infrastructure fixes it carries. Backend CI can now compile the code this slice depends on, and once #1381 lands, the carried commits drop out and the diff here collapses back to slice 2.

Merging the two halves surfaced a real test defect, now fixed here. With slice 1 present, Identity.Tests runs for the first time against the localized validators, and eleven assertions failed on my workstation while being green on CI. Cause: the validator messages resolve through the embedded resx against CultureInfo.CurrentUICulture, so a test asserting the exact English string ("Group name is required.") asserts a property of the developer's operating system. On an en-US runner it passes; on a pt-BR machine the same correct code reports eleven failures, and the resx we ship has a pt-BR translation for every one of those keys, so this is reproducible for any non-English contributor.

Identity.Tests now pins the UI culture once for the assembly (Support/TestUiCulture.cs, a [ModuleInitializer]), which leaves the assertions as they read and makes the result a property of the code. 332/332 green on a pt-BR machine, where it was 321/11 before.

Worth noting that the comment on SharedResourcesLocalizerFactory in #1381 claims the default culture resolves to neutral English. It does not; it resolves to whatever the machine is set to.

The earlier fix landed in Identity.Tests only, and the same defect was still in
Catalog.Tests, Generic.Tests and Webhooks.Tests: a real IStringLocalizer resolves the
message against CurrentUICulture while the assertion spells out the English string, so
nine tests fail on a pt-BR workstation and pass on an en-US runner.

Catalog 70/70, Generic 43/43, Webhooks 71/71 on a pt-BR machine, where they were
69/1, 37/6 and 69/2.
…main

The migration was generated as 20260720062947, which sorts *before*
20260807063015_DropIdentityOutbox that has since landed on main. The history
was therefore inconsistent with itself: DropIdentityOutbox's designer snapshot,
the later of the two by name, knows nothing about the Locale column the earlier
one adds, so anything generated on top of that snapshot would try to add it
again.

Regenerated against main's snapshot rather than hand-edited: the two files were
deleted, the snapshot restored from origin/main, and `dotnet ef migrations add
AddUserLocale` re-run. The Up/Down bodies are unchanged (AddColumn Locale,
varchar(10), nullable) and the resulting model snapshot is byte-identical to the
one already committed (`git diff HEAD` empty), so this is purely a reordering.
Matching key sets were the only gate, and they do not catch a translation that
drops {0} or renumbers it: the argument is either swallowed or the message
throws FormatException at the point it is built, and neither shows up as a
missing key. The new assertion compares the placeholder index set per key,
ignoring alignment and format specifier ({0,-10}, {0:N2} are the same argument).

Verified by mutation: collapsing "{0}/{1} bytes" to "{0} bytes" in the pt-BR
catalog turns it red.
SupportedCultures.Tags was a public static string[], so the whitelist that a
validator, a culture provider and the request-localization setup all trust was
writable by any caller holding a reference. FrozenSet with an ordinal comparer
keeps the exact matching semantics (a wrong-case tag is still rejected) and
makes the set immutable.
The 401 that JwtBearer's OnChallenge writes is the one error response the global
exception handler never sees, so it stayed English while every other error was
negotiated: a pt-BR reader got "Authentication is required to access this
resource." in the middle of an otherwise translated app.

It resolves IStringLocalizer<SharedResources> per request. That works because of
the pipeline order this PR already relies on: UseRequestLocalization sits ahead
of UseAuthorization, which is where the challenge is emitted, so the negotiated
UI culture is in place by then. Error.AuthenticationRequired is new; the title
reuses Error.Unauthorized.

ChallengeLocalizationTests pins both the body and that ordering, hitting a
protected endpoint with and without Accept-Language.
…ages

Seven messages take an enum as an argument, and string.Format wrote the C#
member name straight into the translated sentence: a pt-BR reader got "Não é
possível atribuir um chamado no status Closed", "Cota ... Quarantined",
"... porque está Pending". The catalogs were fully translated and the visible
result still was not.

GlobalExceptionHandler now resolves an enum argument as "{EnumType}.{Member}"
against the same catalog as the message. A member with no entry keeps ToString(),
which is exactly what every argument does today, so nothing regresses if a new
enum reaches a message before its keys do.

Keys added for the five enums that get there: TicketStatus, FileAssetStatus,
Visibility, TopupRequestStatus, TenantProvisioningStatus. Each module's resource
test now asserts every member of its enums has an entry in both catalogs, so a
new member cannot ship half-translated.

Ticket.Assign carried the other half of the problem: the guard took the action as
a string ("assign") and fed it to a generic key, so the verb was English no matter
the culture. It has one caller, so the action moves into the key
(Tickets.CannotAssignInStatus) and the argument list keeps only the status.
Review flagged the single `BaseType` step in Audit.RealExceptionType as fragile:
a subclass of a localization wrapper would report the wrapper as its type, and
one BCL exception would then appear under two names in the exceptionType filter.

That cannot happen today — both wrappers are sealed — so the walk is not worth
writing. The premise is what needs protecting, so it is now a gate: every
ILocalizableMessage that is not a CustomException must be sealed. Verified by
mutation: dropping `sealed` from LocalizedKeyNotFoundException turns it red.
The MinIO carve-out this branch carries only moved `minio/minio`. `minio/mc` is
gone from Docker Hub as well (`hub.docker.com/v2/repositories/minio/mc/` answers
404), and it is what `minio-init` runs: without it `dotnet run --project
src/Host/FSH.Starter.AppHost` and `docker compose up` both die on the image pull,
and the `fsh` bucket is never created, so the first upload fails with
NoSuchBucket.

Same pinned tag as fullstackhero#1388, which owns the fix, so the copy stays byte-identical
to it and can be dropped once that lands.
The pin's own comment says "Testcontainers 4.11.0 and 4.13.0 both depend on
2025.1.0, so bumping Testcontainers does not help", but the branch also bumps
Testcontainers to 4.14.0, whose nuspec declares `SSH.NET >= 2026.0.0`. The two
statements cannot both be true, and the bump is the one that is: with the pin
removed, `dotnet restore src/FSH.Starter.slnx --force` reports zero NU1902/NU1903
and exits 0. It was carrying a transitive pin that no longer pins anything.

The MessagePack pin above it stays: that one is still load-bearing (removing it
brings GHSA-hv8m-jj95-wg3x straight back, verified in the same probe).
This PR adds `Tickets.Tests` and registers it in `FSH.Starter.slnx`, but the
unit-test job enumerates test projects by name rather than running the
solution. The slnx entry puts the project in the build and nowhere else: every
run on this branch has been green without executing one of its four tests.
A gate CI never runs is not a gate.

Verified locally before wiring it in: `dotnet test src/Tests/Tickets.Tests`
passes 4/4 on this head.
@marcelo-maciel

Copy link
Copy Markdown
Contributor Author

Merge order for the three PRs that share the Identity profile path. #1381, #1382 and #1387 all touch the same eight files. None of the three declared an order against the others, so this is what the merges actually do, measured rather than guessed.

#1381 + #1382 is clean. Same series, and #1382 carries #1381's changes to those files verbatim (+94/-14 in both); they differ only by two later commits on #1381.

#1387 conflicts with both, on all eight:

src/Modules/Identity/Modules.Identity.Contracts/DTOs/UserDto.cs
src/Modules/Identity/Modules.Identity.Contracts/Services/IUserProfileService.cs
src/Modules/Identity/Modules.Identity.Contracts/Services/IUserService.cs
src/Modules/Identity/Modules.Identity.Contracts/v1/Users/UpdateUser/UpdateUserCommand.cs
src/Modules/Identity/Modules.Identity/Features/v1/Users/UpdateUser/UpdateUserCommandHandler.cs
src/Modules/Identity/Modules.Identity/Services/UserProfileService.cs
src/Modules/Identity/Modules.Identity/Services/UserService.cs
src/Tests/Identity.Tests/Handlers/UpdateUserCommandHandlerTests.cs

Twelve hunks, and every one is additive: one PR adds Locale, the other adds ExpectedConcurrencyStamps / ConcurrencyStamp, at the same place in the same property list, signature or argument list. Nothing contradicts anything. The single hunk that genuinely interleaves is the error path in UserProfileService.UpdateAsync, where #1387 inserts a ConcurrencyFailure branch in front of the throw that #1381 localizes; both survive.

The part worth knowing about is not in the conflicts. src/Tests/Identity.Tests/Services/UserLocaleTests.cs is a file #1381 adds and #1387 never touches, so git merges it with zero conflicts and the result does not compile: both PRs add a constructor parameter to UserProfileService (IHttpContextAccessor and IdentityErrorDescriber), so the merged constructor takes seven arguments and that test passes six, and its two UpdateAsync calls are one argument short. Three CS7036/CS1503 errors in a file the merge reports as clean. Resolving the twelve marked hunks and pushing is not enough.

Recommended order: #1387#1381#1382#1383#1384.

Three reasons, in order of weight. #1387 is the fix for a silent lost update, and it should not queue behind a four-part feature. #1384 already declares it depends on #1387 landing first, so any other order serializes the same way with an extra step. And the side that rebases re-applies its own changes: #1381's footprint in the shared files is +94/-14, #1387's is +364/-13, so this direction is the cheaper rebase by a factor of four.

Verified end to end on a scratch worktree off main: merge #1387, merge #1381, resolve the twelve hunks and the three compile errors above, then dotnet build src/FSH.Starter.slnx exits 0 with 0 warnings, Identity.Tests 331/331 and Architecture.Tests 55/55. #1382 then merges on top with no conflicts at all.

One follow-up that is not a merge problem. UserProfileService.StaleProfileException(), added by #1387, builds a CustomException with no MessageKey / ResourceSource, while every neighbouring Identity exception gets one from #1381. After these merge, the 412 a stale profile update returns is the only Identity error still hardcoded in English. Whoever rebases should give it a key.

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