Move slug generation onto locale_slug - #693
Merged
Merged
Conversation
PhoenixKit.Utils.Slug keeps its public shape — slugify/2, transliterate/1,
ensure_unique/2 — and delegates the hard part. Two live bugs go with it:
"Größe Fußball" -> gro-e-fu-ball (NFD strips the umlaut; ß has no
canonical decomposition, so the ASCII
pass deleted it)
"Цветокоррекция" -> "" (wherever a caller omitted
transliterate: true — 5 sites in
entities, 2 in ai)
The empty-slug case was the worse one: callers read empty as "no slug yet"
and regenerate forever, which is how a CSV re-import of a Cyrillic catalogue
inserted the whole feed again on every run.
Neither was fixable in the old table, because one table cannot be both German
(ö -> oe) and Estonian (ö -> o). That is why this moved to a package that
takes a locale.
Transliteration is now the DEFAULT. :transliterate is accepted and ignored so
existing call sites keep compiling. Stored slugs are not rewritten — only
newly generated ones change, so live URLs are unaffected.
transliterate/1 keeps its exact old contract: unmapped characters pass
through. generate_username_from_email/1 depends on it — it does
String.replace(".", "_") afterwards, so folding punctuation into a separator
would turn ülo.kask@ into ulokask instead of ulo_kask.
One test previously asserted "without transliteration a Cyrillic title still
collapses to empty". That was the bug, not the contract; it now asserts the
fix.
NOT MERGEABLE until locale_slug is published to Hex — the dep pins ~> 0.1 and
nothing satisfies it yet. Verified locally with:
LOCALE_SLUG_PATH=../../../Elixir/locale_slug mix test # 3189 tests, 0 failures
The package is published, so the dep now resolves from Hex rather than a local
path and this branch is mergeable.
Verified against the PUBLISHED artifact, not the path dep — a path dep can hide
packaging faults (a file missing from the `files:` list, a dev-only dep that is
actually needed at compile time). Checks:
- clean project, `{:locale_slug, "~> 0.1"}`: resolves, compiles, and pulls in
ZERO transitive deps
- compiles with yamerl absent, confirming the apply/3 indirection in the
generator task keeps it dev-only for consumers
- 11/11 slug behaviours correct from the Hex build
- phoenix_kit: 38 doctests, 3189 tests, 0 failures
mdon
added a commit
to mdon/phoenix_kit_entities
that referenced
this pull request
Aug 9, 2026
data_form.ex had the language available at every slug site and was discarding it:
compute_slug_and_data(socket, title, true, current_lang, changeset, data)
slug_text = title |> Slug.slugify() # current_lang is a parameter
The secondary-language path had `current_lang` bound as a parameter; the primary
path had `socket.assigns[:primary_language]` sitting there unused. So a German
entry slugged with the neutral rule ("grosse") where German orthography wants
"groesse", and a Ukrainian one romanized Russian-style.
auto_generate_entity_slug/3 becomes /4 with a default, so the two param-level
call sites can pass the primary language.
NOT changed, deliberately:
- mirror/importer.ex — imports carry no language, so neutral is correct
- entity_form.ex — slugs a machine-facing key with separator: "_"
The empty-slug bug at these sites is already fixed by core defaulting
transliteration on; this is the locale-tuning half.
Requires the phoenix_kit release carrying the locale-aware Slug
(BeamLabEU/phoenix_kit#693). Against local core: 1032 tests, 21 failures — all
21 PRE-EXISTING (identical count with this file reverted).
ddon
pushed a commit
that referenced
this pull request
Aug 9, 2026
Post-merge review of #693 (slug generation moved onto locale_slug). The change is right and the diagnosis behind it is right — I restored the pre-PR implementation and ran both over ~40 inputs; every claim checks out and the old code was worse than the PR says (`Café` slugged to `caf`, `Ünïcödé Tëst` to `n-c-d-t-st`). Three findings, all documentation or dependency hygiene rather than logic. - locale_slug was pinned `~> 0.1`, which admits every 0.x. The exposure is not the API but the OUTPUT: a revised romanization table changes the slug a host derives from the same title, and slugs are persisted in URLs and compared for uniqueness. That is a content change arriving as a dependency bump. Pinned `~> 0.1.0`; mix.lock is unchanged. - transliterate/1's docstring claims "Contract preserved exactly" while the new implementation lower-cases. The punctuation example it gives IS preserved (verified: `ülo.kask` -> `ulo.kask`), but `MiXeD CaSe` now comes back `mixed case`. Core's only caller downcases first; external callers do not necessarily. - "Existing URLs are unaffected" holds for STORED slugs only. A caller that re-derives a slug from a title to find a row now derives a different string, and not only for Cyrillic content. Tests: accented-Latin default change, ASCII-only output invariant over path traversal / markup / emoji / zero-width / CJK, punctuation-and-spacing survival, and the lower-casing. 15 tests, 0 failures — this file needs no database, so it genuinely ran. Review: dev_docs/pull_requests/2026/693-move-slug-generation-onto-locale-slug/CLAUDE_REVIEW.md Co-Authored-By: Claude Opus 5 (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
2 participants
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
PhoenixKit.Utils.Slugnow delegates to thelocale_slugpackage. It keeps its exact publicshape —
slugify/2,transliterate/1,ensure_unique/2— so no caller changes.Two live bugs go with it:
The empty-slug case was the worse one: callers read empty as "no slug yet" and regenerate
forever, which is how a CSV re-import of a Cyrillic catalogue inserted the whole feed
again on every run.
Neither was fixable in the old table, because one table cannot be both German
(
ö → oe) and Estonian (ö → o). That is why this moved to a package that takes alocale. No other Elixir slug package does —
sluggerapplies German rules to everything,slugifythe reverse, and the maintained transliterators expose arity-1 functions thatstructurally cannot take one.
Behaviour changes
:transliterateis accepted and ignored so theexisting call sites keep compiling. This is what closes the empty-slug bug.
unaffected.
transliterate/1keeps its exact old contract: unmapped characters pass through.generate_username_from_email/1depends on it — it doesString.replace(".", "_")afterwards, so folding punctuation into a separator would turn
ülo.kask@intoulokaskinstead ofulo_kask.The package
Pure Elixir, zero dependencies. Verified against the published artifact rather than a
path dep: a clean project resolves it with no transitive deps and compiles without
yamerlpresent. Romanization comes from cited standards (BGN/PCGN for Cyrillic, ISO 843Type 2 for Greek), including their contextual rules — Greek
μπisbat a word edge andmpinside one; Russianеisyeword-initially and after a vowel or sign.Note on one test
A test previously asserted "without transliteration a Cyrillic title still collapses to
empty". That was the bug, not the contract; it now asserts the fix.
Verification
38 doctests, 3189 tests, 0 failuresagainst the publishedlocale_slug 0.1.0.Rebased onto current
mainafter #692 merged.Downstream
Seven module PRs depend on the release that carries this: ecommerce#19, posts#15,
newsletters#31, projects#36, publishing#39, dashboards#5, document_creator#31.