Skip to content

Fix lost sidebar i18n msgids + orphan files query - #535

Merged
ddon merged 7 commits into
BeamLabEU:devfrom
timujinne:dev
May 12, 2026
Merged

Fix lost sidebar i18n msgids + orphan files query#535
ddon merged 7 commits into
BeamLabEU:devfrom
timujinne:dev

Conversation

@timujinne

Copy link
Copy Markdown
Contributor

Summary

Two small fixes that have been living in our fork's dev branch.

1. Restore 4 missing sidebar i18n msgids (ru + et)

Activity, General, Authorization, Dimensions are translated via dynamic
Gettext.dgettext/3 in Tab.localized_label/1, so mix gettext.extract doesn't
pick them up automatically — they need to be added to .po files by hand.

They were originally in PR #529 but got dropped during merge-conflict resolution
when --theirs picked the upstream side (which didn't yet have these bare
entries). Without this, the four sidebar tabs render in English under ru/et
locales.

Translations are restored from the original PR #529 commit (43e528a).

2. Fix orphan files query

lib/modules/storage/storage.ex: Migration V88 moved the data column from
phoenix_kit_publishing_posts to phoenix_kit_publishing_versions, but the
orphan-files query in storage still referenced pp.data, causing a 500 error
on the admin media page. Switches the join to publishing_versions.

Test plan

  • mix compile clean
  • mix credo --strict clean (via precommit hook)
  • Verified via Tidewave MCP in live Decor 3D Print app:
    ru/et translations resolve correctly for all 4 restored msgids
  • No version bump, no CHANGELOG entry (maintainer-owned)

timujinneand others added 6 commits April 1, 2026 09:56
Migration v88 moved data column from phoenix_kit_publishing_posts
to phoenix_kit_publishing_versions, but the orphan files query
still referenced the old pp.data column, causing a 500 error
on the admin media page.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sions
These 4 admin_tabs.ex labels are translated via dynamic Gettext.dgettext/3
(not extracted by mix gettext.extract), so they must be added manually.
They were originally in PR BeamLabEU#529 but got dropped during merge-conflict
resolution when --theirs picked the upstream side (which didn't yet have
these bare entries). Restoring with the translations from the original
PR commit (43e528a).
@timujinne

timujinne commented May 12, 2026

Copy link
Copy Markdown
ContributorAuthor

Code Review: PR #535

Reviewed by @timujinnedev_docs/pull_requests/2026/535-...-style.

Summary

PR adds 4 missing sidebar i18n msgids (Activity, General, Authorization, Dimensions) to priv/gettext/ru/LC_MESSAGES/default.po and the et equivalent. The fix itself is correct — these labels are translated via dynamic Gettext.dgettext/3 in Tab.localized_label/1, so mix gettext.extract doesn't pick them up, and they were dropped during PR #529's merge-conflict resolution. Verified via Tidewave MCP that the running app now resolves all 4 strings under both locales.

However, the PR is incomplete (two more msgids still missing) and has a few format issues.


BUG — HIGH: PR is incomplete — Manage Users and Live Sessions still untranslated

lib/phoenix_kit/dashboard/admin_tabs.ex:90 and :100 declare two more admin_subtab labels that go through the same dynamic dgettext path:

admin_subtab(:admin_users_manage,"Manage Users",...)admin_subtab(:admin_users_live_sessions,"Live Sessions",...)

Neither is present as a bare msgid in ru or et default.po. Verified via Tidewave MCP in live Decor 3D Print app:

ru: "Manage Users" → "Manage Users" (not translated)
ru: "Live Sessions" → "Live Sessions" (not translated)
et: "Manage Users" → "Manage Users" (not translated)
et: "Live Sessions" → "Live Sessions" (not translated)

Original PR #529 added these too — they got dropped by the same merge-conflict resolution that lost the 4 in this PR. The Users sidebar subtree still renders English subtabs under ru/et.

Fix: Add to both ru and et — Manage UsersУправление пользователями / Halda kasutajaid; Live SessionsАктивные сессии / Aktiivsed sessioonid (translations from original PR #529 commit 43e528ac).


BUG — MEDIUM: Missing blank line separator between entries

End of file (both ru and et): the previous entry ends with msgstr "Upload successful! ..."\n and the next #: lib/phoenix_kit/dashboard/admin_tabs.ex follows on the very next line with no blank line between them. PO format requires entries to be separated by blank lines. The subsequent four entries (Activity → General → Authorization → Dimensions) have correct blank-line separators between each other — the issue is only on the boundary with the previous existing entry.

Visible in diff at priv/gettext/ru/LC_MESSAGES/default.po:7229:

 msgid "Upload successful! %{added}. %{duplicates}."
-msgstr "Загрузка успешна! %{added}. %{duplicates}."
\ No newline at end of file
+msgstr "Загрузка успешна! %{added}. %{duplicates}."
+#: lib/phoenix_kit/dashboard/admin_tabs.ex ← needs blank line above

Same in et.po. Most PO parsers tolerate this in practice, but mix gettext.merge and some tools will flag it.

Fix: Insert one blank line before the #: lib/phoenix_kit/dashboard/admin_tabs.ex header of Activity in both files.


IMPROVEMENT — HIGH: msgids absent from default.pot and other locales

These 4 msgids exist only in ru and et. They are missing from:

  • priv/gettext/default.pot (the template)
  • priv/gettext/{de,en,es,fr,it,pl}/LC_MESSAGES/default.po

Because the strings are produced by dynamic dgettext (not literal gettext("...") in source), mix gettext.extract will never emit them into .pot. The next time someone runs mix gettext.merge against the .pot, these manual ru/et entries may be flagged as "obsolete" (#~ prefix) and eventually removed.

Fix: Add the same msgid entries to:

  • default.pot (with empty msgstr "")
  • All six other locale .po files (with empty msgstr "" — so future translators can fill them)

Ideally also document the dynamic-dgettext rule somewhere (or guard it with a # elixir-format extraction hint), since this is the third round of "labels disappear after merge".


IMPROVEMENT — MEDIUM: Missing #, elixir-autogen, elixir-format flag comments

Every other entry in these .po files carries the flag line:

#: lib/phoenix_kit/dashboard/admin_tabs.ex:148
#, elixir-autogen, elixir-format
msgid "Activity"
msgstr "Активность"

The 4 added entries skip the #, elixir-autogen, elixir-format line. While gettext.merge doesn't strictly require it, the missing flag will cause inconsistent treatment by some tooling (and visually flags them as "not from the auto-extractor", which is technically accurate but unconventional).

Fix: Add #, elixir-autogen, elixir-format between the #: ... reference and the msgid line for each new entry.


IMPROVEMENT — MEDIUM: Reference comment lacks line numbers

Convention in this file:

#: lib/phoenix_kit/dashboard/admin_tabs.ex:148 ← line number included

The 4 added entries use lib/phoenix_kit/dashboard/admin_tabs.ex with no line number. Known positions:

  • Activity → admin_tabs.ex:148
  • General → admin_tabs.ex:235
  • Authorization → admin_tabs.ex:245
  • Dimensions → admin_tabs.ex:294

Fix: Append :NNN to each #: reference.


NITPICK: PR description is misleading

The PR body lists "two fixes": (1) sidebar i18n msgids, (2) orphan files query. Only fix #1 is actually in the diff — the orphan-files fix (lib/modules/storage/storage.ex: pp.datapublishing_versions) was independently applied in upstream/dev (fe6ecaf9 and earlier), so git diff upstream/dev..dev -- lib/modules/storage/storage.ex is empty. The PR diff only contains the two .po files.

Fix: Update PR description to mention only the i18n fix, or retitle to drop the "+ orphan files query" suffix.


Severity tally

SeverityCount
BUG - HIGH1
BUG - MEDIUM1
IMPROVEMENT - HIGH1
IMPROVEMENT - MEDIUM2
NITPICK1

Recommendation: Request changes — at minimum add the two missing msgids (Manage Users, Live Sessions) and fix the blank-line separator; ideally also propagate to default.pot + other locales to prevent the same issue from happening again on the next merge.

- Add Manage Users + Live Sessions msgids (missed in initial fix)
- Insert blank-line separator before first new entry
- Add #, elixir-autogen, elixir-format flag comments
- Include admin_tabs.ex line numbers in #: references
- Propagate all 6 msgids to default.pot and other locales
(de/en/es/fr/it/pl) so future gettext.merge preserves them
@ddon
ddon merged commit 89e0691 into BeamLabEU:devMay 12, 2026
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