-
Notifications
You must be signed in to change notification settings - Fork 149
chore(i18n): remove out-of-scope language variants (fixes #1371) #1389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: enext
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRemoves out-of-scope or duplicate locale variants from both frontend (moment.js locale list) and backend (supported locales lists) to align available languages with the intended i18n scope and keep only supported variants like pt-br. Flow diagram for locale validation with updated language listsgraph TD
A[Admin opens admin config page] --> B[Frontend loads momentLocaleSet]
B --> C[Admin opens language dropdown]
C --> D[Admin selects locale]
D --> E{Locale in momentLocaleSet?}
E -- No --> F[Locale not selectable in UI]
E -- Yes --> G[Send selected locale to backend]
G --> H[Backend checks locale in supported_locales_list]
H --> I{Locale in supported_locales_list?}
I -- No --> J[Return validation error]
I -- Yes --> K[Save configuration]
subgraph Frontend
B
C
D
E
F
end
subgraph Backend
H
I
J
K
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- The same locale list now exists in three places (frontend
main.vue,helpers/i18n.py, andorga/utils/i18n.py); consider centralizing this into a single source of truth and importing/consuming it in each layer to avoid future drift. - In
main.vue, the comment about using a static moment locale list might be updated or expanded to reference the shared i18n source (if you centralize it), so future changes to supported locales are less likely to miss the frontend.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The same locale list now exists in three places (frontend `main.vue`, `helpers/i18n.py`, and `orga/utils/i18n.py`); consider centralizing this into a single source of truth and importing/consuming it in each layer to avoid future drift.
- In `main.vue`, the comment about using a static moment locale list might be updated or expanded to reference the shared i18n source (if you centralize it), so future changes to supported locales are less likely to miss the frontend.
## Individual Comments
### Comment 1
<location> `app/eventyay/webapp/src/views/admin/config/main.vue:55-56` </location>
<code_context>
- 'mn', 'mr', 'ms', 'ms-my', 'mt', 'my', 'nb', 'ne', 'nl', 'nl-be', 'nn', 'oc-lnc', 'pa-in', 'pl', 'pt', 'pt-br',
- 'ro', 'ru', 'sd', 'se', 'si', 'sk', 'sl', 'sq', 'sr', 'sr-cyrl', 'ss', 'sv', 'sw', 'ta', 'te', 'tet',
- 'tg', 'th', 'tk', 'tl-ph', 'tlh', 'tr', 'tzl', 'tzm', 'tzm-latn', 'ug-cn', 'uk', 'ur', 'uz', 'uz-latn', 'vi',
+ 'mn', 'mr', 'ms', 'ms-my', 'mt', 'my', 'nb', 'ne', 'nl', 'nn', 'pa-in', 'pl', 'pt-br',
+ 'ro', 'ru', 'sd', 'se', 'si', 'sl', 'sq', 'sr', 'sr-cyrl', 'ss', 'sv', 'sw', 'ta', 'te', 'tet',
+ 'tg', 'th', 'tk', 'tl-ph', 'tlh', 'tr', 'tzl', 'tzm', 'ug-cn', 'uk', 'ur', 'uz', 'vi',
'x-pseudo', 'yo', 'zh-cn', 'zh-hk', 'zh-mo', 'zh-tw',
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider whether dropping locales like `pt` and `sk` is expected in the UI and if any stored config values rely on them.
Since `pt`, `sk`, and other locales are being removed from `momentLocaleSet`, any existing config or persisted settings using those codes may no longer resolve correctly and could fall back to defaults. Please verify whether these locales are used in stored data and, if so, ensure a migration or mapping strategy (e.g., `pt` → `pt-br`) so settings don’t end up in an invalid state.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| 'mn', 'mr', 'ms', 'ms-my', 'mt', 'my', 'nb', 'ne', 'nl', 'nn', 'pa-in', 'pl', 'pt-br', | ||
| 'ro', 'ru', 'sd', 'se', 'si', 'sl', 'sq', 'sr', 'sr-cyrl', 'ss', 'sv', 'sw', 'ta', 'te', 'tet', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Consider whether dropping locales like pt and sk is expected in the UI and if any stored config values rely on them.
Since pt, sk, and other locales are being removed from momentLocaleSet, any existing config or persisted settings using those codes may no longer resolve correctly and could fall back to defaults. Please verify whether these locales are used in stored data and, if so, ensure a migration or mapping strategy (e.g., pt → pt-br) so settings don’t end up in an invalid state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes out-of-scope and duplicate language variants from i18n configuration files to align with supported locales. The changes clean up language codes that are redundant or outside the project's scope while preserving the correctly specified locale for Portuguese (Brazil).
Key changes:
- Removed 7 language codes from frontend moment.js locale configuration
- Removed 5 language codes from backend Python i18n helpers
- Maintained consistency across all three configuration files
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| app/eventyay/webapp/src/views/admin/config/main.vue | Updated frontend moment.js locale list by removing 'bn', 'nl-be', 'sk', 'pt', 'gom-latn', 'oc-lnc', 'tzm-latn', and 'uz-latn' |
| app/eventyay/orga/utils/i18n.py | Removed 'bn', 'sk', 'pt', and 'tzm-latn' from backend moment_locales set |
| app/eventyay/helpers/i18n.py | Removed 'bn', 'sk', 'pt', and 'tzm-latn' from backend moment_locales set |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks. Could you please add a screenshot of the language selection option and remove the translation files? |
|
Hi @Sak1012 @mariobehling |
mariobehling
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change as follows:
- Please remove information like "inofficial translation" or "translation in progress"
- Brazilian Portuguese should be Portoguese (Brazil)
- Finnish and Polish should be sorted alphabetically into the list
- Malay should be there only once
- German should be in the alphabetical list
- However "English" should stay on top as the default
Fixes #1371
This PR removes out-of-scope or duplicate language variants from the frontend and backend language lists.
Removed:
Kept:
Files updated:
No translation
.pofiles were touched, since these appear auto-generated.If needed, I can create a follow-up PR to clean translation folders after confirmation.
All changes validated and build/lint checks pass locally.
Summary by Sourcery
Prune unsupported or duplicate language variants from the application’s configured locale lists to align frontend and backend i18n settings.
Enhancements:
Screenshot
Here is the language selection dropdown after the update:

