Site-wide default_language_no_prefix setting + sitemap honors it#552
Merged
Conversation
Lifts the locale-prefix-on-primary-language behavior from a publishing-module setting into a single site-wide toggle on the core Languages admin page (`/admin/settings/languages`). All URL generators in the workspace — `Routes.path/1`, `Routes.admin_path/2`, the sitemap, publishing's HTML builders, and publishing's canonical-redirect controller — now honor the same source of truth. ## Why PR BeamLabEU#551 made admin URLs unconditionally prefixless for the primary language. Publishing's public URLs were governed by a separate toggle (`publishing_default_language_no_prefix`, default off), and the sitemap honored neither — it always emitted `/en/blog/post` in multilang mode regardless of the setting. The boss flagged both: the setting should be site-wide, and the sitemap should respect it. ## What changed - **New core setting** `default_language_no_prefix` (boolean, default `false`) on `PhoenixKit.Modules.Languages`. Surfaced as a daisyUI toggle in a new "URL Behavior" card on the Languages admin page, above the existing "Frontend Language Switcher" card. - **`Routes.path/1` + `admin_path/2`** (`lib/phoenix_kit/utils/routes.ex`): the unconditional primary-locale strip from PR BeamLabEU#551 is now gated on `Languages.default_language_no_prefix?/0` via a new `prefixless_primary?/0` private helper. Same defensive rescue + mix-task fallback as `get_default_language_base/0`. - **Sitemap fix** (`lib/modules/sitemap/sources/publishing.ex`): `build_group_path/3` now skips the language segment for the primary language when the setting is on. Previously it always emitted the prefix in multilang mode. - **Legacy migration** (`Languages.migrate_legacy/0`): on first call, copies the value from the legacy `publishing_default_language_no_prefix` key to the new `default_language_no_prefix` key. Idempotent — once the new key is set, subsequent runs are no-ops. Pairs with the publishing- side delegate in the matching publishing PR. ## Migration safety Default is `false`, matching the historical publishing default and keeping existing installs' indexed public URLs stable on upgrade. Sites that previously toggled the publishing setting get migrated on first `migrate_legacy/0` run — they keep their explicit choice without admin action. Admin URL emission flips: PR BeamLabEU#551 emitted prefixless unconditionally for the primary locale; this PR honors the new setting (default off), so primary admin URLs grow back the `/en/` prefix unless the admin opts in. Both URL shapes still resolve at the router (dual-scope admin emission), so the only visible change is the *emitted* shape — bookmarks and external links keep working. ## Tests - `test/phoenix_kit/utils/routes_test.exs` updated to cover the new default-off behavior (no-DB rescue → primary keeps prefix). - `test/integration/languages/default_language_no_prefix_test.exs` added (DB-backed): setter round-trips, Routes helpers across both setting states, `migrate_legacy/0` no-op + backfill + idempotency. - `test/phoenix_kit_web/components/core/language_switcher_test.exs` assertions updated to match the new default. `mix test` clean (1308 tests, 0 failures, 11 doctests). `mix compile --warnings-as-errors`, `mix credo --strict`, `mix deps.unlock --check-unused`, `mix format --check-formatted` all clean. ## Follow-up surfaced (not in this PR) `phoenix_kit_entities/lib/phoenix_kit_entities/url_resolver.ex` has the same site-wide-setting blind spot in two helpers + three call sites — `build_path_with_language/3` (sitemap, always prefixes primary) and `add_public_locale_prefix/2` (public URLs, always strips primary). Surface to the entities owner before claiming site-wide unification is complete.
ddon
pushed a commit
that referenced
this pull request
May 19, 2026
- `Languages.default_language_no_prefix?/0` docstring repointed at the real migrator (`migrate_legacy/0`); the previous reference to `PhoenixKit.Migration.migrate_default_language_no_prefix/0` named a function that doesn't exist anywhere in the codebase. - `Routes.admin_path/2` doctest restructured: deterministic `"uk"` and `nil` cases stay inside `## Examples`; the two setting-dependent `"en"` cases move into a narrative block with `#=>` markers so ExDoc never evaluates them. Wiring up `doctest PhoenixKit.Utils.Routes` later is now safe — previously the second `"en"` example would have failed against the default-off setting. - Adds `dev_docs/pull_requests/2026/552-.../CLAUDE_REVIEW.md` covering the PR, with both findings marked ✅ FIXED plus a new BUG - LOW surfaced during the fix pass (dialyzer `unknown_type` on PR #552's own `@spec set_default_language_no_prefix/1`, left open). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ddon
pushed a commit
that referenced
this pull request
May 19, 2026
PR #552 introduced `@spec set_default_language_no_prefix/1` returning `{:ok, PhoenixKit.Settings.Setting.t()} | {:error, Ecto.Changeset.t()}` but the schema never declared `@type t`, so `mix precommit` failed with `unknown_type: PhoenixKit.Settings.Setting.t/0` from dialyzer. Adds the missing `@type t :: %__MODULE__{...}` with every field typed against its column. Fixes the warning at the source — any future spec that references `Settings.Setting.t()` type-checks cleanly without extra work. Verified: `mix precommit` clean (dialyzer 161 errors / 161 skipped, same as baseline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ddon
pushed a commit
that referenced
this pull request
May 19, 2026
Rolls up PR #552 (site-wide `default_language_no_prefix` setting + sitemap honors it) and the post-merge review follow-ups: docstring `@see` fix, `Routes.admin_path/2` doctest restructure, and a missing `@type t/0` on `PhoenixKit.Settings.Setting` that was tripping dialyzer. Also picks up the `ecto`/`ecto_sql`/`fresco`/`hammer` mix.lock bumps that landed on dev today. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mdon
added a commit
to mdon/phoenix_kit
that referenced
this pull request
May 19, 2026
…wrapper + tests Phase 2 triage on PR BeamLabEU#552 surfaced three actionable items: ## Dedup `build_path_with_language/3` across 3 sitemap sources The site-wide `default_language_no_prefix` setting gate logic was byte-identical across `publishing.ex`, `static.ex`, and `posts.ex` sitemap sources, alongside three byte-identical `single_language_mode?/0` private helpers. Extracted to a new `PhoenixKit.Modules.Sitemap.LocalePath` module: - `emit_prefix?/2` — single canonical decision rule - `single_language_mode?/0` — shared defensive lookup Each source still owns its locale-segment **formatting** (publishing uses `get_display_code/2` for hreflang-aware dialect emission; static + posts use `DialectMapper.extract_base/1` for clean base codes), so the dedup is decision-only. Three sources lose ~30 LOC each. ## Dedup `prefixless_primary?/0` boot-safe wrapper `PhoenixKit.Utils.Routes` and `PhoenixKitWeb.Users.Auth` each had a private `prefixless_primary?/0` wrapper around `Languages.default_language_no_prefix?/0` with defensive rescue — the Routes one also guarded against mix-task context, the Auth one didn't. Promoted to a public `Languages.prefixless_primary_safe?/0` with both guards (mix-task context check + broad rescue → false). Both callers now delegate to the canonical implementation. ## New test coverage The original integration test covered setter round-trip, Routes helpers, and `migrate_legacy/0`. Filled the gaps surfaced by triage: - `test/integration/sitemap/locale_path_test.exs` (NEW, 8 tests) — the shared `LocalePath.emit_prefix?/2` policy across single- language, multi-language, setting OFF, setting ON, primary, non-primary, and `is_default` mismatch cases. - `test/integration/users/auth_locale_test.exs` (NEW, 4 tests) — `redirect_invalid_locale/2` swap-vs-strip behaviour gated on the setting. Was previously untested; the gating only landed in this PR (commit `41a70b36`) and would have regressed undetected. - `default_language_no_prefix_test.exs` adds 2 tests for the new `prefixless_primary_safe?/0` wrapper covering both the runtime path and the mix-task-context fallback. ## Triage findings classified NOT-A-FIX - **Three-repo `prefixless_primary?/0` triplication** (core, auth, entities) — each repo has a distinct rescue scope and boot timing. Entities can't depend on core's mix-task-context sentinel because it doesn't run during core's install path. Leaving the entities copy as-is is the right call; the dedup target was the two **in core** which now share `prefixless_primary_safe?/0`. - **`default_language_no_prefix_key/0` public exposure** — flagged by triage as "looks like internal API". It's intentionally public for the legacy-migration system; the `@doc` says so. No change. - **Async UX on the Languages toggle** — single phx-click on a checkbox; round-trip is a fast Settings write + ETS-cache invalidate. No `phx-disable-with` needed. ## Verification `mix test`: 1322 tests, 0 failures, 11 doctests. `mix compile --warnings-as-errors`, `mix credo --strict`, `mix deps.unlock --check-unused`, `mix format --check-formatted` all clean.
mdon
added a commit
to mdon/phoenix_kit
that referenced
this pull request
May 20, 2026
…wrapper + tests Phase 2 triage on PR BeamLabEU#552 surfaced three actionable items: ## Dedup `build_path_with_language/3` across 3 sitemap sources The site-wide `default_language_no_prefix` setting gate logic was byte-identical across `publishing.ex`, `static.ex`, and `posts.ex` sitemap sources, alongside three byte-identical `single_language_mode?/0` private helpers. Extracted to a new `PhoenixKit.Modules.Sitemap.LocalePath` module: - `emit_prefix?/2` — single canonical decision rule - `single_language_mode?/0` — shared defensive lookup Each source still owns its locale-segment **formatting** (publishing uses `get_display_code/2` for hreflang-aware dialect emission; static + posts use `DialectMapper.extract_base/1` for clean base codes), so the dedup is decision-only. Three sources lose ~30 LOC each. ## Dedup `prefixless_primary?/0` boot-safe wrapper `PhoenixKit.Utils.Routes` and `PhoenixKitWeb.Users.Auth` each had a private `prefixless_primary?/0` wrapper around `Languages.default_language_no_prefix?/0` with defensive rescue — the Routes one also guarded against mix-task context, the Auth one didn't. Promoted to a public `Languages.prefixless_primary_safe?/0` with both guards (mix-task context check + broad rescue → false). Both callers now delegate to the canonical implementation. ## New test coverage The original integration test covered setter round-trip, Routes helpers, and `migrate_legacy/0`. Filled the gaps surfaced by triage: - `test/integration/sitemap/locale_path_test.exs` (NEW, 8 tests) — the shared `LocalePath.emit_prefix?/2` policy across single- language, multi-language, setting OFF, setting ON, primary, non-primary, and `is_default` mismatch cases. - `test/integration/users/auth_locale_test.exs` (NEW, 4 tests) — `redirect_invalid_locale/2` swap-vs-strip behaviour gated on the setting. Was previously untested; the gating only landed in this PR (commit `41a70b36`) and would have regressed undetected. - `default_language_no_prefix_test.exs` adds 2 tests for the new `prefixless_primary_safe?/0` wrapper covering both the runtime path and the mix-task-context fallback. ## Triage findings classified NOT-A-FIX - **Three-repo `prefixless_primary?/0` triplication** (core, auth, entities) — each repo has a distinct rescue scope and boot timing. Entities can't depend on core's mix-task-context sentinel because it doesn't run during core's install path. Leaving the entities copy as-is is the right call; the dedup target was the two **in core** which now share `prefixless_primary_safe?/0`. - **`default_language_no_prefix_key/0` public exposure** — flagged by triage as "looks like internal API". It's intentionally public for the legacy-migration system; the `@doc` says so. No change. - **Async UX on the Languages toggle** — single phx-click on a checkbox; round-trip is a fast Settings write + ETS-cache invalidate. No `phx-disable-with` needed. ## Verification `mix test`: 1322 tests, 0 failures, 11 doctests. `mix compile --warnings-as-errors`, `mix credo --strict`, `mix deps.unlock --check-unused`, `mix format --check-formatted` all clean.
3 tasks
ddon
added a commit
that referenced
this pull request
May 20, 2026
PR #552 follow-ups: dedup + 3 missed sites + 22 new tests
ddon
pushed a commit
that referenced
this pull request
May 20, 2026
#554 ships the PR #552 locale-prefix follow-ups (login-redirect gate, LocalePath dedup, redirect_invalid_locale swap-vs-strip, 22 new tests). Verdict: ship. Notes are nitpicks — double prefixless_primary? read in redirect_invalid_locale, moduledoc "three rules" / four-item list, mix_task_context? sentinel duplicated between Languages + Routes. #555 restores the bare-label rendering for single-dialect bases in the language switcher (and inherits it into admin/user dashboard nav via the public dedupe_names/1). Verdict: ship. Notes are nitpicks — unrun doctests on the new helpers, the dedup rule is duplicated between display_name_for/3 and dedupe_one_name/2, extract_base_language_name/1 returns "" on "" input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lifts the primary-language URL prefix toggle into a single site-wide
setting on the Languages admin page (
/admin/settings/languages).All URL generators in the workspace — core
Routes.path/1,Routes.admin_path/2, the sitemap, publishing's HTML builders, andpublishing's canonical-redirect controller — now honor the same
source of truth.
This pairs with phoenix_kit_publishing PR #TBD which delegates
publishing's existing setter and replaces the publishing-settings
toggle with a read-only status link.
Why
PR #551 made admin URLs unconditionally prefixless for the primary
language. Publishing's public URLs were governed by a separate
toggle (
publishing_default_language_no_prefix, default off), andthe sitemap honored neither — it always emitted
/en/blog/postinmultilang mode regardless of the setting. The boss flagged both: the
setting should be site-wide, and the sitemap should respect it.
What changed
default_language_no_prefix(boolean, defaultfalse) onPhoenixKit.Modules.Languages. Surfaced as a daisyUItoggle in a new "URL Behavior" card on the Languages admin page,
above the existing "Frontend Language Switcher" card.
Routes.path/1+admin_path/2(lib/phoenix_kit/utils/routes.ex):the unconditional primary-locale strip from PR URL-driven locale resolution + admin-URL prefixless + module_assigns generalization #551 is now gated
on
Languages.default_language_no_prefix?/0via a newprefixless_primary?/0private helper. Same defensive rescue +mix-task fallback as
get_default_language_base/0.lib/modules/sitemap/sources/publishing.ex):build_group_path/3now skips the language segment for theprimary language when the setting is on. Previously it always
emitted the prefix in multilang mode —
/en/blog/postshowed upin the sitemap even when publishing was set to emit
/blog/postin served URLs.
Languages.migrate_legacy/0): on firstcall, copies the value from the legacy
publishing_default_language_no_prefixkey to the newdefault_language_no_prefixkey. Idempotent. Run byPhoenixKit.ModuleRegistry.run_all_legacy_migrations/0from thehost app's
Application.start/2.Migration / behavior change
Default is
false, matching the historical publishing default andkeeping existing installs' indexed public URLs stable. Sites that
previously toggled the publishing setting get migrated on first
migrate_legacy/0run — they keep their explicit choice withoutadmin action.
Admin URL emission flips: PR #551 emitted prefixless
unconditionally for the primary locale; this PR honors the new
setting (default off), so primary admin URLs grow back the
/en/prefix unless the admin opts in. Both URL shapes still resolve at
the router (dual-scope admin emission), so the only visible change
is the emitted shape — bookmarks and external links keep working.
Tests
test/phoenix_kit/utils/routes_test.exsupdated to cover the newdefault-off behavior (no-DB rescue path → primary keeps prefix).
test/integration/languages/default_language_no_prefix_test.exsadded (DB-backed, 11 tests): setter round-trips, Routes helpers
across both setting states,
migrate_legacy/0no-op + backfill +idempotency.
test/phoenix_kit_web/components/core/language_switcher_test.exsassertions updated to match the new default.
mix testclean (1308 tests, 0 failures, 11 doctests).mix compile --warnings-as-errors,mix credo --strict,mix deps.unlock --check-unused,mix format --check-formattedallclean.
Follow-up surfaced (not in this PR)
phoenix_kit_entities/lib/phoenix_kit_entities/url_resolver.exhasthe same site-wide-setting blind spot in two helpers + three call
sites —
build_path_with_language/3(sitemap, always prefixesprimary) and
add_public_locale_prefix/2(public URLs, alwaysstrips primary). Should be addressed in entities to claim full
site-wide unification.
Test plan
/admin/settings/languages, verifyadmin URLs in the navigation and sidebar flip prefix shape
language entries don't carry
/en/without indexed URLs shifting