Skip to content

Add i18n coverage for User Management, Settings, Auth, and admin sidebar - #569

Merged
ddon merged 22 commits into
BeamLabEU:devfrom
timujinne:dev
May 25, 2026
Merged

Add i18n coverage for User Management, Settings, Auth, and admin sidebar#569
ddon merged 22 commits into
BeamLabEU:devfrom
timujinne:dev

Conversation

@timujinne

Copy link
Copy Markdown
Contributor

Summary

Comprehensive i18n coverage pass across PhoenixKit admin and auth surfaces. Wraps ~280 user-visible strings in gettext/ngettext, populates ru + et translations, defensively wraps all admin sidebar labels in gettext_noop/1, and de-fuzzies all user-touched pre-existing fuzzy entries that rendered English at runtime.

Changes

User Management pages (/admin/users/*)

  • users.ex + users.html.heex — 50+ strings: page title, stat cards, role-management modal, column-customization modal, filter selects, badges (Confirmed/Pending/System/No roles), empty states, all flash messages, data-confirm dialogs, dynamic state strings split into separate gettext entries
  • sessions.ex + sessions.html.heex — stat cards, revoke modals, filter options, pagination text with bindings (%{from}/%{to}/%{total}), ngettext for revoke count (3 ru plural forms), all flash messages
  • live_sessions.ex + live_sessions.html.heex — full coverage (file had 0 gettext calls before): table headers, badges, "Anonymous User", filter tabs, empty states
  • user_details.exformat_timezone(nil) "Not set" fallback wrapped

User Settings (/admin/settings/users)

  • All flash messages wrapped, JS confirm() calls migrated to data-confirm pattern, page title and section headers

Sidebar labels (lib/phoenix_kit/dashboard/admin_tabs.ex)

  • All 17 raw label literals wrapped in gettext_noop/1 (Dashboard, Users, Manage Users, Live Sessions, Sessions, Roles, Permissions, Referral Codes, Activity, Media, Modules, Settings, General, Authorization, Organization, Integrations, Dimensions, Health) — previously worked only via incidental extraction from LV files
  • Manage Users was missing from PO entirely; now added in ru + et

Auth pages (/users/*)

  • login, registration, magic_link, magic_link_registration, forgot_password, reset_password, confirmation, confirmation_instructions — full i18n coverage
  • Language switcher added across auth flow

Activity module + Modules overview + General settings tab

  • i18n coverage for Activity admin pages, Modules overview, and General settings tab

Translations

  • ru + et populated for all new msgids (~280 entries)
  • ngettext plural forms: ru (3 forms), et (2 forms)
  • de-fuzzy: 28 ru + 18 et user-module fuzzy entries fixed (correcting wrong autoguesses like Save Permissions "Активные сессии" → "Сохранить разрешения", Activate as action not state, Username as proper translation not parent menu)
  • de-fuzzy: 12 ru + 9 et mixed-ref entries affecting user-touched pages (Username, Expires, User, Organization, Person, Integrations, Modules, Dimensions, etc.)
  • de-fuzzy: 5 user_form fuzzy entries (Failed to add member, Failed to cancel invitation, Failed to send invitation [had wrong ru autoguess], Organization Name *, Select a user to add...)

Test Plan

  • mix format --check-formatted — clean
  • mix compile --warnings-as-errors — clean
  • mix credo --strict — 7699 mods/funs, no issues
  • mix dialyzer — passes
  • Live runtime verification via Tidewave MCP on Decor 3D Print parent app — all spot-checked msgids return correct ru + et translations
  • User-touched fuzzy count = 0 in both ru and et after de-fuzzy passes
  • Sidebar tabs render translated labels on /admin/users, /admin/users/live_sessions, /admin/settings/users, etc.

timujinne added 15 commits May 24, 2026 20:27
- Hoist language switcher and dev-mailbox notice into AuthPageWrapper
so all 9 auth pages get them for free (previously only login had
the switcher). New attrs: current_path, dev_mailbox_message;
show_language_switcher computed internally from Languages module.
- Wrap hardcoded strings in gettext across registration, forgot_password,
reset_password, magic_link, magic_link_registration,
magic_link_registration_request, confirmation,
confirmation_instructions (login was already partial — finished).
- Remove duplicate switcher/show_language_switcher wiring from
login.ex / login.html.heex.
- Extract POT + merge: 51 new msgids, 15 reworded fuzzies de-fuzzied
for ru + et with correct translations (66 entries each locale).
Other locales (de/es/fr/it/pl) ship empty msgstr; maintainer fills.
Verified via decor-shop-tidewave MCP: 8 page+locale combinations
return status=200 with translated copy, English fallback intact,
language switcher rendered on every variant.
Wrap all hardcoded strings in Activity index/show LiveViews with
gettext/ngettext: table headers, filter labels, toolbar, empty
states, page titles, flashes, and relative-time formatter.
Register "Authorization" sidebar tab label via gettext_noop in
admin_tabs so it gets extracted into the default domain.
Add Russian and Estonian translations for ~22 new msgids; correct
8 pre-existing fuzzy translations that the merge had guessed wrong
(Activity, Action:, All Modes/Modules/Actions, Mode:, Module:, Key).
Russian "Role" and "System" fuzzy translations also corrected — both
are used by Activity now and were rendering as "Роли"/"Владельцы
системы" at runtime. Plural forms for %{count}m/h/d ago completed
for ru (3 forms) and et (2 forms).
# Conflicts:
#	priv/gettext/et/LC_MESSAGES/default.po
#	priv/gettext/ru/LC_MESSAGES/default.po
Wrap external module names/descriptions (from module_name/0 and
permission_metadata.description) and Available Packages entries
(name/description from Hex.pm metadata) in runtime gettext lookup
so they localize per user locale. The strings are loaded at runtime
from external module callbacks and Hex metadata, so they're not
visible to mix gettext.extract — we register all known module names
and descriptions via gettext_noop/1 inside modules.ex to make them
extractable.
Add ru and et translations for 20 module names plus their full
descriptions, including AI, Billing, Catalogue, CRM, Comments,
Customer Support, Document Creator, E-Commerce, Emails, Entities,
Hello World, Legal, Locations, Newsletters, Posts, Projects,
Publishing, Staff, Sync, User Connections. De-fuzzy entries the
extract had guessed wrong (Emails, External module, Projects, Sync,
User Connections, Locations, Legal).
Translate "Current Configuration" stats section title via gettext.
Hide the stats section entirely when a module has no module_stats/0
callback (via :if on the <:stats> slot) — previously the title
rendered with an empty body for plain external modules.
Allow long localized button labels to wrap inside module-card action
buttons via Tailwind arbitrary selector on the card-actions
container: whitespace-normal text-left h-auto min-h-fit py-2
leading-tight applied to all .btn descendants — fixes overflow when
short English labels become longer Russian/Estonian phrases.
Wrap the "General" tab label in gettext_noop so it gets extracted
into the default domain. Same pattern as the Authorization tab fix.
Add ru ("Общие") and et ("Üldine") translations.
Wrap all hardcoded strings in users.html.heex and users.ex:
- Page title, heading, subtitle, filter/role/status labels
- All stat card titles and subtitles
- Role modal, column modal, confirmation modal content
- Error messages, flash messages, badge labels (you, Confirmed, Pending)
- data-confirm dialogs replacing onclick JS confirms
- Dynamic flash messages split into separate gettext strings
- ngettext for session count pluralization
- Preserve all existing gettext calls
Wrap all hardcoded strings in sessions.html.heex and sessions.ex:
- Page title, heading, subtitle, stat card labels
- Revoke session/user-all modals including dynamic email binding
- Filter options, pagination text with bindings (%{from}, %{to}, %{total})
- Previous/Next pagination buttons
- ngettext for plural revoke count
- All flash messages wrapped
Wrap all hardcoded strings in live_sessions.html.heex and live_sessions.ex:
- Page title, heading, auto-refresh ON/OFF button labels
- Stat card titles and subtitles
- Card fields labels and values inside closure
- Filter tab labels (All, Anonymous, Authenticated)
- Table headers, badge labels (Auth, Anon), Anonymous User text
- Empty state heading and description
- Add use Gettext import to live_sessions.ex
- settings/users.html.heex: wrap page_title suffix, replace JS
onclick confirms with data-confirm={gettext(...)} pattern
- settings/users.ex: wrap page_title, all flash messages including
dynamic field label bindings (%{label})
- user_details.ex: wrap "Not set" fallback in format_timezone/1
Extract 107 new msgids from user management pages into default.pot.
Translate all new entries in ru and et; de-fuzzy the 55 reworded
entries generated by gettext.extract.
New strings include: user/session management labels, flash messages,
modal content, filter options, column customizer UI, badge text,
empty states, confirmation dialogs with dynamic bindings.
Plural forms: ngettext for session revoke count (ru: 3 forms, et: 2).
De-fuzzy 28 ru and 18 et entries whose #: references point to
live/users/ and live/settings/users files.
Correct wrong autoguess translations in ru:
- "Save Permissions": "Активные сессии" -> "Сохранить разрешения"
- "Activate": "Активно" -> "Активировать" (action, not state)
- "Enable": "Включено" -> "Включить" (action, not state)
- "System Roles": "Владельцы системы" -> "Системные роли"
- "Total": "Всего пользователей" -> "Итого"
- "users": "Пользователи" -> "пользователей" (genitive count)
- "roles": "Роли" -> "ролей" (genitive count)
- "Registered": "Зарегистрированные аккаунты" -> "Зарегистрирован"
- "Admin Only": "Только администратор" -> "Только для администраторов"
- "Custom User Fields": removed redundant "пользователя" suffix
All 18 et entries had correct translations; fuzzy flag removed only.
User-module fuzzy count: ru=0, et=0 after this commit.
Replace raw "Manage Users" string in admin_tabs.ex with
gettext_noop() so the extractor registers the msgid. Raw literals
in tab declarations are never seen by mix gettext.extract.
Add translations:
- ru: "Список пользователей" (distinct from parent "Управление
пользователями" to preserve subtab hierarchy readability)
- et: "Halda kasutajaid"
All tab and subtab labels in admin_tabs.ex now use gettext_noop() so
the Gettext extractor registers them as translatable msgids. Previously
only General, Authorization, and Manage Users were wrapped; Dashboard,
Users, Activity, Media, Modules, Settings, Live Sessions, Sessions,
Roles, Permissions, Referral Codes, Organization, Integrations,
Dimensions, Health were raw literals invisible to the extractor.
Also updated source refs via mix gettext.extract --merge (no new msgids
added — all strings were already translated; only #: comment lines
updated).
De-fuzzied 12 ru and 9 et entries that had source refs in user-module
files (live/users/, settings/users, dashboard/admin_tabs) and were
silently falling back to English at runtime.
Corrected wrong autoguesses in ru:
- 'Username' was 'Управление пользователями' -> now 'Имя пользователя'
- 'Expires' was 'Истекшие сессии' -> now 'Истекает'
- 'User' was 'Пользователи' -> now 'Пользователь'
De-fuzzied (translation already correct): Inactive, Failed to remove
member, Organization, Organization Members, Organization Name, Person,
Integrations, Modules, Dimensions (ru); User, Failed to remove member,
Organization, Organization Members, Organization Name, Person,
Integrations, Modules, Dimensions (et).
Five fuzzy entries in lib/phoenix_kit_web/users/user_form.{ex,html.heex}
were not caught by the previous de-fuzzy pass. Four had correct
autoguesses (only fuzzy flag needed removal); one had a wrong autoguess:
- "Failed to send invitation" ru: "Не удалось сохранить настройки"
-> "Не удалось отправить приглашение"
Fuzzy entries cleared: Failed to add member, Failed to cancel invitation,
Failed to send invitation, Organization Name *, Select a user to add...
User-touched fuzzy count after this commit: ru=0, et=0.
#: lib/phoenix_kit_web/live/users/sessions.html.heex:154
#, elixir-autogen, elixir-format
msgid "Search by email or token..."
msgstr "Поиск по имени файла…"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUG MEDIUM: Wrong ru translation "Search by email or token..." -> "Поиск по имени файла…" (= Search by filename). Expected: "Поиск по email или токену...".

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6ce125d: msgstr corrected to "Поиск по email или токену...".

#: lib/phoenix_kit_web/live/users/sessions.html.heex:154
#, elixir-autogen, elixir-format
msgid "Search by email or token..."
msgstr "Otsi failinime järgi…"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUG MEDIUM: Wrong et translation "Search by email or token..." -> "Otsi failinime järgi…" (= Search by filename). Expected: "Otsi e-posti või tokeni järgi...".

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6ce125d: msgstr corrected to "Otsi e-posti või tokeni järgi...".

#: lib/phoenix_kit_web/live/users/users.html.heex:787
#, elixir-autogen, elixir-format
msgid "Standard Fields"
msgstr "Добавить поле"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUG MEDIUM: Wrong ru translation "Standard Fields" -> "Добавить поле" (= Add field). Used as section heading at users.html.heex:755. Expected: "Стандартные поля".

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6ce125d: msgstr corrected to "Стандартные поля".

#: lib/phoenix_kit_web/live/users/users.html.heex:787
#, elixir-autogen, elixir-format
msgid "Standard Fields"
msgstr "Lisa väli"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUG MEDIUM: Wrong et translation "Standard Fields" -> "Lisa väli" (= Add field). Expected: "Standardväljad".

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6ce125d: msgstr corrected to "Standardväljad".

#: lib/phoenix_kit_web/live/users/users.html.heex:363
#, elixir-autogen, elixir-format
msgid "No data"
msgstr "Нет метаданных"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUG MEDIUM: Wrong ru translation "No data" -> "Нет метаданных" (= No metadata). Used at users.html.heex:363. Expected: "Нет данных".

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6ce125d: msgstr corrected to "Нет данных".

#: lib/phoenix_kit_web/live/users/users.html.heex:363
#, elixir-autogen, elixir-format
msgid "No data"
msgstr "Metaandmed puuduvad"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUG MEDIUM: Wrong et translation "No data" -> "Metaandmed puuduvad" (= Metadata missing). Expected: "Andmed puuduvad".

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6ce125d: msgstr corrected to "Andmed puuduvad".

<p class="text-sm text-base-content/60 mt-0.5">
Real-time monitoring of all active sessions with live updates.
Last updated: {Calendar.strftime(@last_updated, "%H:%M:%S")}
{gettext("Real-time monitoring of all active sessions with live updates.")} Last updated: {Calendar.strftime(

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUG — MEDIUM

The English literal Last updated: is stranded outside the gettext wrap. Russian/Estonian users see:

«Мониторинг в реальном времени всех активных сессий с обновлениями.» Last updated: 12:34:56

The English label leaks into the translated UI.

Fix:

{gettext("Real-time monitoring of all active sessions with live updates.")}{gettext("Last updated:")}{Calendar.strftime(@last_updated,"%H:%M:%S")}

— or combine the whole thing into one gettext with %{time} binding so translators control word order.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in eb9eb04: wrapped in gettext("Last updated:"); ru: "Обновлено:", et: "Viimati uuendatud:".

ngettext("%{count}d ago", "%{count}d ago", n, count: n)

true ->
Calendar.strftime(datetime, "%b %d, %Y")

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUG — MEDIUM

Calendar.strftime(datetime, "%b %d, %Y") produces English month abbreviations (May, Jun, Jul) regardless of Gettext.get_locale/1. A Russian-locale user viewing an activity older than 7 days sees May 15, 2026 instead of 15 мая 2026 г..

Three fixes:

  1. Use PhoenixKit.Utils.Date.format_date_with_user_format/1 — same as elsewhere in the codebase.
  2. Wrap month names: gettext(Calendar.strftime(datetime, "%B")) after registering each month name with gettext_noop/1.
  3. Use Cldr.DateTime.to_string/2 if CLDR is a dependency.

The PR introduced ngettext for the recent branches but left this final branch un-localized.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a540a3a: replaced Calendar.strftime("%b %d, %Y") with UtilsDate.format_date_with_user_format/1 and format_time_with_user_format/1 in both table column and card-view Date field.

<%= for stat <- ext.module.module_stats() do %>
<div>
<span class="text-base-content/70">
{Gettext.gettext(PhoenixKitWeb.Gettext, stat.label)}:

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUG — MEDIUM

Gettext.gettext(PhoenixKitWeb.Gettext, stat.label) crashes with FunctionClauseError if stat.label is not a binary.

Gettext.dpgettext/5 guards when is_binary(msgid). An external module implementing module_stats/0 returning [%{label: :active_users, value: 5}] — or any module returning nil — crashes the Modules overview page for all users.

Fix:

{ifis_binary(stat.label),do: Gettext.gettext(PhoenixKitWeb.Gettext,stat.label),else: to_string(stat.label)}

Or document in PhoenixKit.Module.module_stats/0 callback spec that labels must be binaries, and enforce it via Dialyzer.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5866dd5: added is_binary/1 guard; falls back to to_string/1 for non-binary labels.


diff < 3600 ->
n = div(diff, 60)
ngettext("%{count}m ago", "%{count}m ago", n, count: n)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUG — MEDIUM

Msgid collision between ngettext and gettext for the same string.

This PR adds:

ngettext("%{count}m ago","%{count}m ago",n,count: n)

But lib/phoenix_kit_web/components/core/time_display.ex:188 already uses:

gettext("%{count}m ago",count: div(diff_seconds,60))

They collide on the same msgid. After mix gettext.extract --merge the PO entry becomes a plural entry (msgid_plural populated, msgstr[0/1/2]). The gettext call in time_display then resolves against a plural entry and may return only msgstr[0] regardless of count — silently wrong for languages with multiple plural forms.

Fix: consolidate by replacing the local format_time_ago/1 in activity/index.ex with the existing <.time_display datetime={entry.inserted_at} /> component, or change both call sites to use the same macro.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a540a3a: removed format_time_ago/1 entirely; the ngettext collision is gone. Template now calls UtilsDate.format_date/time_with_user_format/1.

#: lib/phoenix_kit/dashboard/admin_tabs.ex:92
#, elixir-autogen, elixir-format
msgid "Manage Users"
msgstr "Список пользователей"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMPROVEMENT — MEDIUM

"Manage Users" translated to "Список пользователей" (= "List of users", noun phrase).

The sidebar parent tab Users already translates to "Пользователи". The child tab is a verb-like action ("Manage Users"), and User Management already maps to "Управление пользователями". Same translation for both would be more consistent.

Suggest: "Управление пользователями" or, to distinguish from the page heading, "Все пользователи".

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6ce125d: "Manage Users" ru updated to "Управление пользователями" to match page heading and parent tab convention.

for user <strong>{@selected_user.email}</strong>?
{gettext("Are you sure you want to revoke")}
<strong>
{ngettext("all %{count} session", "all %{count} sessions", @user_sessions_count,

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMPROVEMENT — MEDIUM

The revoke confirmation sentence is split across three independent translation units:

{gettext("Are you sure you want to revoke")}<strong>{ngettext("all %{count} session","all %{count} sessions",n,count: n)}</strong>{gettext("for user")}<strong>{@selected_user.email}</strong>?

For inflected/SOV languages this produces ungrammatical output — verb-object agreement, noun cases, and word order can't be expressed when fragments translate in isolation. Russian translation has to force English word order and accept clumsy phrasing.

Fix: one full sentence with bindings:

ngettext("Are you sure you want to revoke all %{count} session for user %{email}?","Are you sure you want to revoke all %{count} sessions for user %{email}?",n,count: n,email: @selected_user.email)

The <strong> tag is lost in this approach, but you can re-add styling via a span around the whole <p> or via HEEX interpolation.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 104d3d1: consolidated into single ngettext with %{count} and %{email} interpolation. ru 3-form and et 2-form translations added.

Only accessible to users with Owner or Admin roles.
"""
use PhoenixKitWeb, :live_view
use Gettext, backend: PhoenixKitWeb.Gettext

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NITPICK

use Gettext, backend: PhoenixKitWeb.Gettext is redundant here — use PhoenixKitWeb, :live_view (at line 16) already injects use Gettext, backend: PhoenixKitWeb.Gettext (see lib/phoenix_kit_web.ex:53).

Elixir's import is idempotent so this compiles cleanly, but it's noise and confuses a reader. Remove the explicit line.

Same applies to users.ex, sessions.ex, user_details.ex, activity/index.ex, activity/show.ex, settings/users.ex if they have the same duplication.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6f7cc67: removed redundant use Gettext line from live_sessions.ex (and also users.ex, sessions.ex, user_details.ex, settings/users.ex).

gettext_noop("Catalogue"),
gettext_noop("Comments"),
gettext_noop("Customer Support"),
gettext_noop("Document Creator"),

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NITPICK

Both "E-Commerce" and "Ecommerce" are registered via gettext_noop/1. Each produces a separate msgid that translators must localize.

In practice only one is the actual module_name/0 return value from PhoenixKitEcommerce — the other is dead weight in the PO files. Either:

  1. Drop the variant the module doesn't return, or
  2. Add a comment naming the canonical form and explain the alias exists for backward-compat.

Minor: @compile {:no_warn_undefined, :_register_module_translations} at line 94 uses the wrong form — the conventional Elixir form is {__MODULE__, :_register_module_translations, 0}. As-is the attribute may not actually suppress anything.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipped (uncertain): could not safely determine which of 'E-Commerce'/'Ecommerce' is the live module_name/0 return value without running the external module. Left both to avoid silently dropping a valid translation key.

diff < 604_800 -> "#{div(diff, 86_400)}d ago"
true -> Calendar.strftime(datetime, "%b %d, %Y")
diff < 60 ->
gettext("just now")

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMPROVEMENT — MEDIUM

format_time_ago/1 (this private helper) is a near-verbatim duplicate of PhoenixKitWeb.Components.Core.TimeDisplay.format_time_ago/2. The duplication caused the msgid collision flagged on line 202.

Replace this local helper with <.time_display datetime={entry.inserted_at} /> in the template. One source of truth, no msgid collision, free updates when TimeDisplay is improved.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a540a3a: removed local format_time_ago/1; replaced with UtilsDate.format_date/time_with_user_format/1 in both template locations.

@timujinne

Copy link
Copy Markdown
ContributorAuthor

Code-review summary (15 findings)

Automated /code-review pass: 5 parallel finder angles + targeted verification. All findings posted as inline comments.

Severity breakdown

BUG — MEDIUM (10):

  • 6 wrong translations leaked in: Search by email or token..., Standard Fields, No data — each wrong in both ru and et (autoguesses copied from unrelated msgids)
  • Last updated: hardcoded English stranded outside gettext in live_sessions.html.heex:14
  • Calendar.strftime(datetime, "%b %d, %Y") in activity/index.ex:213 produces English month names for the >7-day fallback
  • Gettext.gettext(backend, stat.label) in modules.html.heex:750 can crash if external module_stats/0 returns non-binary labels
  • Msgid collision between ngettext in activity/index.ex:202 and gettext in time_display.ex:188 (same string, different macros)

IMPROVEMENT — MEDIUM (3):

  • "Manage Users" ru translation could match the parent group's "Управление пользователями" for consistency
  • Revoke confirmation sentence split across 3 gettext fragments (sessions.html.heex:94) is ungrammatical for inflected languages — combine into one ngettext with %{email} binding
  • format_time_ago/1 in activity/index.ex:193 duplicates TimeDisplay.format_time_ago/2 — use the existing component

NITPICK (2):

  • Redundant use Gettext, backend: PhoenixKitWeb.Gettext in several LV files (live_sessions.ex:17 etc.) — use PhoenixKitWeb, :live_view already injects it
  • modules.ex:104: both "E-Commerce" and "Ecommerce" registered; pick one. Also @compile {:no_warn_undefined, :_register_module_translations} uses non-standard form.

Not flagged (verified safe)

  • ngettext("...%{count}...", ..., n, email: ...) without explicit count: binding — Gettext auto-injects count from the 3rd arg, so this is NOT a bug.

Pre-existing fuzzy entries

~178 ru + ~176 et fuzzy entries remain in other modules (ecommerce/projects/dashboard/etc.) — out of scope for this PR.

Copy-paste errors introduced when these user-module strings were first
extracted:
- "No data" ru: "Нет метаданных" → "Нет данных"
- "No data" et: "Metaandmed puuduvad" → "Andmed puuduvad"
- "Search by email or token..." ru: "Поиск по имени файла…" → "Поиск по email или токену..."
- "Search by email or token..." et: "Otsi failinime järgi…" → "Otsi e-posti või tokeni järgi..."
- "Standard Fields" ru: "Добавить поле" → "Стандартные поля"
- "Standard Fields" et: "Lisa väli" → "Standardväljad"
Also improve "Manage Users" ru: "Список пользователей" → "Управление
пользователями" to match the page heading and parent group convention.
The English literal "Last updated:" was outside gettext and rendered
as-is in Russian/Estonian UI. Wrap in gettext() and add ru/et msgstrs:
- ru: "Обновлено:"
- et: "Viimati uuendatud:"
The revoke confirmation was split into three independent gettext calls
("Are you sure you want to revoke" + ngettext("all %{count} session")
+ gettext("for user")), which breaks grammar in inflected languages.
Replace with a single ngettext that expresses the full sentence:
msgid: "Are you sure you want to revoke all %{count} session for user %{email}?"
msgid_plural: "Are you sure you want to revoke all %{count} sessions for user %{email}?"
Translations:
- ru (3 forms): 1 сессию / %{count} сессии / %{count} сессий
- et (2 forms): sessiooni / kõik %{count} sessiooni
Run gettext.extract --merge to remove the three obsolete msgids and
register the new combined one. Also updates de/en/es/fr/it/pl pot stubs.
Gettext.gettext/2 guards when is_binary(msgid) and raises
FunctionClauseError if called with an atom or other non-binary. An
external module returning %{label: :active_users} would crash the
modules page. Wrap with is_binary guard, falling back to to_string/1.
Calendar.strftime produces English month names regardless of locale.
Replace with UtilsDate.format_date/time_with_user_format/1 in both
the full table date column and the card-view Date field.
Also removes the local format_time_ago/1 helper which duplicated
PhoenixKitWeb.Components.Core.TimeDisplay and caused a msgid collision:
the local ngettext("%{count}m ago", ...) conflicted with the singular
gettext("%{count}m ago", ...) already registered in TimeDisplay.
use PhoenixKitWeb, :live_view already injects
use Gettext, backend: PhoenixKitWeb.Gettext (see phoenix_kit_web.ex:53).
The explicit line in live_sessions, users, sessions, user_details, and
settings/users was a no-op duplicate.
Late-loading :phoenix_kit_<x> deps can miss ModuleRegistry's init scan
because the dep's beams aren't yet available when :phoenix_kit starts
its supervision tree. Symptom: the module's admin tabs and Modules page
entry are absent until the registry is poked (e.g. server restart).
- PhoenixKit.boot/1 wraps the result of Supervisor.start_link/2, calls
ModuleRegistry.rescan/0 to absorb late discoveries, then runs
run_all_legacy_migrations/0. Returns the supervisor tuple unchanged
so it composes at the end of start/2 via |>. {:error, _} passes
through without side effects.
- mix phoenix_kit.install and mix phoenix_kit.update wire the call into
parent's application.ex automatically. Non-standard Supervisor.start_link
forms emit an Igniter warning with manual-edit instructions.
- New ModuleRegistry.rescan/0 public API is also useful for dev
hot-reload recovery and dynamic-fixture tests.
@timujinne

Copy link
Copy Markdown
ContributorAuthor

Additional commit: f3f8d298PhoenixKit.boot/1 hook for late-loaded modules

Unrelated to the i18n scope of this PR but lands on the same branch.

Problem

:phoenix_kit_<x> deps can miss ModuleRegistry's init scan because their .beam files aren't yet loaded when :phoenix_kit starts its supervision tree. Symptom: the module's admin tabs and Modules-page entry are absent until something pokes the registry (e.g. a server restart that happens to win the timing race).

Caught it during a Decor 3D Print session — phoenix_kit_emails was installed but invisible in the admin Modules page. ModuleDiscovery.discover_external_modules/0 returned PhoenixKit.Modules.Emails, but ModuleRegistry.all_modules/0 did not.

Solution

  • PhoenixKit.boot/1 — wraps the result of Supervisor.start_link/2, calls ModuleRegistry.rescan/0 to absorb late discoveries, then runs run_all_legacy_migrations/0. Returns the supervisor tuple unchanged so it composes at the end of start/2:

    Supervisor.start_link(children,opts)|>PhoenixKit.boot()

    {:error, _} short-circuits without side effects.

  • mix phoenix_kit.install and mix phoenix_kit.update both wire the call into the parent's application.ex automatically via the new PhoenixKit.Install.BootHook helper. Idempotent (skips if PhoenixKit.boot already present). Non-standard Supervisor.start_link forms emit an Igniter.add_warning with manual-edit instructions.

  • PhoenixKit.ModuleRegistry.rescan/0 — new synchronous public API. Also useful for dev hot-reload recovery after recompiling a module package, or in tests that dynamically load fixture modules.

Why not a background polling loop in ModuleRegistry.init/1

Considered and rejected on review:

  • Adds 250ms+ to every boot even when no race occurred
  • Magic-number tuning attributes feel like ops config for a workaround, not domain logic
  • Process.send_after timers leak into tests (global registered name, async: false)
  • Only covers the boot window — hot-reload staleness later is unsolved
  • Diverges from the existing run_all_legacy_migrations/0 parent-callback pattern documented in PhoenixKit.Module

The parent-callback path is deterministic, zero-cost when not needed, idiomatic (matches the established pattern), and auto-installs via update/install — addressing all the concerns surfaced in review.

Test plan

  • mix format, mix credo --strict, mix compile --warnings-as-errors, mix dialyzer — clean
  • mix test test/phoenix_kit_test.exs test/phoenix_kit/module_registry_test.exs — 61 tests, 0 failures
  • Verified in live Decor 3D Print via MCP: discovery finds PhoenixKit.Modules.Emails; manual ModuleRegistry.register/1 makes it appear; rescan/0 is the deterministic equivalent

@ddon
ddon merged commit 6b4f160 into BeamLabEU:devMay 25, 2026
ddon pushed a commit that referenced this pull request May 25, 2026
Release rollup since 1.7.120:
- PR #568: native <dialog> modal (PkDialog), core list-UI toolkit
(BulkSelect, Sortable, ReorderModal, load_more), race-free sort_selector
- PR #568 post-merge review fixes (untranslated reorder label, named group/row)
- PR #569: PhoenixKit.boot/1 hook, locale-aware Activity dates, broad i18n sweep
- PR #550/#552/#554/#557/#558/#559 follow-ups
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for freeto 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.

2 participants

@timujinne@ddon