-
Notifications
You must be signed in to change notification settings - Fork 4
Support Explicitly Excluding Locales From Smartling Sync #65
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -365,9 +365,24 @@ def get_or_create_from_source_and_translation_data( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # TODO only submit locales that match Smartling target locales | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # TODO make sure the source locale matches the Smartling project's language | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| translations_list = list(translations) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Filter out translations for excluded locales | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| excluded = smartling_settings.EXCLUDE_LOCALES | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if excluded: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| excluded_translations = [t for t in translations_list if t.target_locale.language_code in excluded] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if excluded_translations: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Excluding %d translation(s) for locales: %s", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| len(excluded_translations), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ", ".join(t.target_locale.language_code for t in excluded_translations), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| translations_list = [t for t in translations_list if t.target_locale.language_code not in excluded] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+373
to
+380
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| excluded_translations = [t for t in translations_list if t.target_locale.language_code in excluded] | |
| if excluded_translations: | |
| logger.info( | |
| "Excluding %d translation(s) for locales: %s", | |
| len(excluded_translations), | |
| ", ".join(t.target_locale.language_code for t in excluded_translations), | |
| ) | |
| translations_list = [t for t in translations_list if t.target_locale.language_code not in excluded] | |
| # Bulk-load locales to avoid N+1 queries on t.target_locale | |
| target_locale_ids = { | |
| t.target_locale_id for t in translations_list if getattr(t, "target_locale_id", None) | |
| } | |
| locales_by_id = Locale.objects.in_bulk(target_locale_ids) if target_locale_ids else {} | |
| def _is_excluded(translation: Translation) -> bool: | |
| locale = locales_by_id.get(getattr(translation, "target_locale_id", None)) | |
| return bool(locale and locale.language_code in excluded) | |
| excluded_translations = [t for t in translations_list if _is_excluded(t)] | |
| if excluded_translations: | |
| excluded_language_codes = sorted( | |
| { | |
| locales_by_id[t.target_locale_id].language_code | |
| for t in excluded_translations | |
| if getattr(t, "target_locale_id", None) in locales_by_id | |
| } | |
| ) | |
| logger.info( | |
| "Excluding %d translation(s) for locales: %s", | |
| len(excluded_translations), | |
| ", ".join(excluded_language_codes), | |
| ) | |
| translations_list = [t for t in translations_list if not _is_excluded(t)] |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -42,6 +42,7 @@ class SmartlingSettings: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ADD_APPROVAL_TASK_TO_DASHBOARD: bool = True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MAX_APPROVAL_TASKS_ON_DASHBOARD: int = 7 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SEND_EMAIL_ON_TRANSLATION_IMPORT: bool = True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EXCLUDE_LOCALES: frozenset[str] = dataclasses.field(default_factory=frozenset) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _init_settings() -> SmartlingSettings: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -191,6 +192,27 @@ def _init_settings() -> SmartlingSettings: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "SEND_EMAIL_ON_TRANSLATION_IMPORT" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if "EXCLUDE_LOCALES" in settings_dict: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exclude = settings_dict["EXCLUDE_LOCALES"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not isinstance(exclude, (list, tuple, set, frozenset)): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise ImproperlyConfigured( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"{setting_name}['EXCLUDE_LOCALES'] must be a list, tuple, or set " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"of locale code strings" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+199
to
+200
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"{setting_name}['EXCLUDE_LOCALES'] must be a list, tuple, or set " | |
| f"of locale code strings" | |
| f"{setting_name}['EXCLUDE_LOCALES'] must be an iterable " | |
| f"(list, tuple, set, or frozenset) of locale code strings" |
Copilot
AI
Mar 12, 2026
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.
invalid_codes = set(exclude) - valid_locale_codes will raise a TypeError if EXCLUDE_LOCALES contains any unhashable items (e.g. a nested list) and currently doesn't validate that each entry is a string. Consider explicitly validating each element is a str (and rejecting non-hashable values) so misconfiguration consistently raises ImproperlyConfigured with a clear message instead of crashing during settings init.
| valid_locale_codes = { | |
| code | |
| for code, _ in getattr( | |
| django_settings, "WAGTAIL_CONTENT_LANGUAGES", [] | |
| ) | |
| } | |
| invalid_codes = set(exclude) - valid_locale_codes | |
| if invalid_codes: | |
| raise ImproperlyConfigured( | |
| f"{setting_name}['EXCLUDE_LOCALES'] contains locale codes not in " | |
| f"WAGTAIL_CONTENT_LANGUAGES: {sorted(invalid_codes)}" | |
| ) | |
| settings_kwargs["EXCLUDE_LOCALES"] = frozenset(exclude) | |
| # Validate that each entry is a string so we can safely use it in a set | |
| exclude_codes: list[str] = [] | |
| for value in exclude: | |
| if not isinstance(value, str): | |
| raise ImproperlyConfigured( | |
| f"{setting_name}['EXCLUDE_LOCALES'] must contain only strings; " | |
| f"got {value!r} (type {type(value).__name__})" | |
| ) | |
| exclude_codes.append(value) | |
| valid_locale_codes = { | |
| code | |
| for code, _ in getattr( | |
| django_settings, "WAGTAIL_CONTENT_LANGUAGES", [] | |
| ) | |
| } | |
| invalid_codes = set(exclude_codes) - valid_locale_codes | |
| if invalid_codes: | |
| raise ImproperlyConfigured( | |
| f"{setting_name}['EXCLUDE_LOCALES'] contains locale codes not in " | |
| f"WAGTAIL_CONTENT_LANGUAGES: {sorted(invalid_codes)}" | |
| ) | |
| settings_kwargs["EXCLUDE_LOCALES"] = frozenset(exclude_codes) |
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.
Optimisation nit, nonblocking: could this be done more efficiently with a
set.difference, I'm wondering