Repository navigation
feat: extract translatable strings from the authz schema #509
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: main
Are you sure you want to change the base?
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 |
|---|---|---|
| @@ -0,0 +1,170 @@ | ||
| """Extract translatable strings from the static authz schema (ADR 0020). | ||
|
|
||
| The authz schema's ``display_name`` and ``description`` fields are source-language | ||
| text that end users see through APIs and frontend clients, so they need to go | ||
| through the Open edX translation process (OEP-58). Standard extraction tools only | ||
| scan Python and JavaScript source, so they can't find strings written in YAML. | ||
|
|
||
| This module reads every discovered schema resource directly with ``yaml.safe_load``, | ||
| independent of the schema ``load``/``validate``/``compile`` pipeline: it only needs | ||
| the raw ``display_name``/``description`` fields, not resolved role extensions, | ||
| priority conflicts, or any of the richer typed documents those steps build. That | ||
| keeps it usable (and testable) even before the rest of the pipeline exists, and | ||
| immune to changes in how that pipeline models conflicts or precedence. | ||
|
|
||
| It emits a generated Python module containing one ``pgettext()`` call per | ||
| translatable field, which ``i18n_tool extract`` (a thin wrapper around Django's | ||
| ``makemessages``) then picks up like any other source file. ``pgettext`` is used | ||
| instead of plain ``gettext`` because ADR 0020 requires a permission's display name | ||
| and description to remain separate translation messages even when they contain | ||
| the same English text; a plain ``gettext`` call would merge identical msgids into | ||
| one catalog entry. The context string also carries the field's stable identifier, | ||
| so two different roles that happen to share a one-word name like "Admin" stay | ||
| separate messages too. | ||
|
|
||
| This generated module is never imported or executed: its only purpose is to be | ||
| valid Python source for ``makemessages`` to scan. At runtime, the API (once it | ||
| exists, see openedx-authz#432's own note that this depends on the still-unmerged | ||
| schema loader and catalog API) looks up a field's source-language value directly | ||
| in the compiled catalog with the same ``pgettext(context, value)`` pair used here, | ||
| so the generated calls must stay in sync with however that lookup builds its | ||
| context string. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from dataclasses import dataclass | ||
|
|
||
| import yaml | ||
|
|
||
| from openedx_authz.engine.schema.discovery import DiscoveredResource, SchemaDiscovery, SchemaDiscoveryError | ||
|
|
||
| GENERATED_FILE_HEADER = '''"""Generated by extract_schema_translations. Do not edit, do not import. | ||
|
|
||
| This file exists only so that ``i18n_tool extract`` (Django's ``makemessages``) | ||
| can find the authz schema's source-language display_name/description strings, | ||
| which otherwise live in YAML and are invisible to it. Regenerate it with: | ||
|
|
||
| python manage.py extract_schema_translations | ||
|
|
||
| See openedx_authz.engine.schema.translation for why pgettext is used here. | ||
| """ | ||
| from django.utils.translation import pgettext | ||
| ''' | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class TranslatableMessage: | ||
| """One ``pgettext(context, message)`` pair extracted from the schema. | ||
|
|
||
| Attributes: | ||
| context: Disambiguates otherwise-identical messages, formatted as | ||
| ``"{field_kind}.{field_name}:{stable_id}"``, e.g. | ||
| ``"permission.display_name:courses.view_course"``. | ||
| message: The source-language (English) string to translate. | ||
| """ | ||
|
|
||
| context: str | ||
| message: str | ||
|
|
||
|
|
||
| class SchemaTranslationExtractionError(Exception): | ||
| """A schema resource could not be read or parsed for extraction.""" | ||
|
|
||
|
|
||
| def extract_messages(resources: list[DiscoveredResource] | None = None) -> list[TranslatableMessage]: | ||
| """Extract every translatable schema string from the given (or discovered) resources. | ||
|
|
||
| Args: | ||
| resources: Resources to extract from. Defaults to everything | ||
| ``SchemaDiscovery`` finds (every registered schema directory), which | ||
| is what the management command uses. | ||
|
|
||
| Returns: | ||
| Deduplicated messages, sorted by context, so the generated file's | ||
| diffs are stable across runs regardless of discovery order. | ||
|
|
||
| Raises: | ||
| SchemaTranslationExtractionError: A resource's bytes couldn't be read, | ||
| or weren't valid YAML, or the document wasn't a mapping at the top | ||
| level. A translation build must fail loudly here (ADR 0020) rather | ||
| than silently skip a file, the same way schema validation does. | ||
| """ | ||
| if resources is None: | ||
| try: | ||
| resources = SchemaDiscovery().discover() | ||
| except SchemaDiscoveryError as exc: | ||
| raise SchemaTranslationExtractionError(str(exc)) from exc | ||
|
|
||
| messages: dict[str, TranslatableMessage] = {} | ||
| for resource in resources: | ||
| for extracted in _extract_from_resource(resource): | ||
| # Last one wins on a context collision; in practice this only happens | ||
| # if two files genuinely define the same stable id, which validation | ||
| # (a separate step) is responsible for catching. | ||
| messages[extracted.context] = extracted | ||
|
|
||
| return sorted(messages.values(), key=lambda m: m.context) | ||
|
|
||
|
|
||
| def _extract_from_resource(resource: DiscoveredResource) -> list[TranslatableMessage]: | ||
| """Parse one resource's YAML and pull its translatable fields out.""" | ||
| try: | ||
| contents = resource.read_bytes() | ||
| except SchemaDiscoveryError as exc: | ||
| raise SchemaTranslationExtractionError(str(exc)) from exc | ||
|
|
||
| try: | ||
| document = yaml.safe_load(contents) | ||
| except yaml.YAMLError as exc: | ||
| raise SchemaTranslationExtractionError( | ||
| f"Invalid YAML in {resource.package}:{resource.resource_path}: {exc}" | ||
| ) from exc | ||
|
|
||
| if document is None: | ||
| return [] | ||
| if not isinstance(document, dict): | ||
| raise SchemaTranslationExtractionError( | ||
| f"Schema file {resource.package}:{resource.resource_path} must be a mapping " | ||
| f"at the top level, got {type(document).__name__}." | ||
| ) | ||
|
|
||
| messages: list[TranslatableMessage] = [] | ||
| for category in document.get("permission_categories") or []: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the for loops here, if for some reason the collections are malformed, it's possible that we may end up raising an exception different than SchemaTranslationExtractionError. For example, if a file has We should catch these cases and make sure we always rise a SchemaTranslationExtractionError. |
||
| messages.extend(_messages_for_entry("permission_category", category.get("id", ""), category)) | ||
| for permission in document.get("permissions") or []: | ||
| stable_id = f"{permission.get('namespace', '')}.{permission.get('name', '')}" | ||
| messages.extend(_messages_for_entry("permission", stable_id, permission)) | ||
| for role in document.get("roles") or []: | ||
| messages.extend(_messages_for_entry("role", role.get("id", ""), role)) | ||
| for extension in document.get("role_extensions") or []: | ||
| # display_name/description are optional on an extension (it may only | ||
| # touch permissions or the icon), so only emit the fields it actually | ||
| # overrides instead of a pair of empty messages for every extension. | ||
| messages.extend(_messages_for_entry("role_extension", extension.get("role", ""), extension, optional=True)) | ||
|
|
||
| return messages | ||
|
|
||
|
|
||
| def _messages_for_entry( | ||
| field_kind: str, stable_id: str, entry: dict, *, optional: bool = False | ||
| ) -> list[TranslatableMessage]: | ||
| """Build the display_name/description messages for one schema entry.""" | ||
| messages = [] | ||
| for field_name in ("display_name", "description"): | ||
| value = entry.get(field_name) | ||
| if value is None: | ||
| if optional: | ||
| continue | ||
| value = "" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we fall into this case, this will result in a Perhaps we should validate for this, if not here, on the render step? |
||
| messages.append(TranslatableMessage(context=f"{field_kind}.{field_name}:{stable_id}", message=str(value))) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If for some reason stable_id is missing or "", this would result in potentially duplicated context ids. I think we should validate and error out if this happens. |
||
| return messages | ||
|
|
||
|
|
||
| def render_module(messages: list[TranslatableMessage]) -> str: | ||
| """Render the extracted messages as a generated Python module's source text.""" | ||
| lines = [GENERATED_FILE_HEADER] | ||
| for extracted in messages: | ||
| lines.append(f"pgettext({extracted.context!r}, {extracted.message!r})") | ||
| lines.append("") # Trailing newline. | ||
| return "\n".join(lines) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| """Django management command to extract translatable strings from the authz schema. | ||
|
|
||
| See openedx_authz.engine.schema.translation for why this exists and how it works. | ||
| """ | ||
|
|
||
| import os | ||
|
|
||
| from django.core.management.base import BaseCommand, CommandError | ||
|
|
||
| from openedx_authz import ROOT_DIRECTORY | ||
| from openedx_authz.engine.schema.translation import ( | ||
| SchemaTranslationExtractionError, | ||
| extract_messages, | ||
| render_module, | ||
| ) | ||
|
|
||
| DEFAULT_OUTPUT_PATH = os.path.join(ROOT_DIRECTORY, "engine", "schema", "_generated_translations.py") | ||
|
|
||
|
|
||
| class Command(BaseCommand): | ||
| """Generate a Python module with one pgettext() call per translatable schema string. | ||
|
|
||
| Run this before ``i18n_tool extract`` (see the ``extract_translations`` Makefile | ||
| target), so the generated calls are on disk for ``makemessages`` to scan. | ||
|
|
||
| Example Usage: | ||
| python manage.py extract_schema_translations | ||
| python manage.py extract_schema_translations --output /path/to/file.py | ||
| """ | ||
|
|
||
| help = "Extract translatable display_name/description strings from the authz schema into a generated module." | ||
|
|
||
| def add_arguments(self, parser) -> None: | ||
| """Add command-line arguments to the argument parser. | ||
|
|
||
| Args: | ||
| parser: The Django argument parser instance to configure. | ||
| """ | ||
| parser.add_argument( | ||
| "--output", | ||
| type=str, | ||
| default=DEFAULT_OUTPUT_PATH, | ||
| help="Path to write the generated module to.", | ||
| ) | ||
|
|
||
| def handle(self, *args, **options) -> None: | ||
| """Extract schema translation messages and write the generated module.""" | ||
| try: | ||
| messages = extract_messages() | ||
| except SchemaTranslationExtractionError as exc: | ||
| raise CommandError(str(exc)) from exc | ||
|
|
||
| output_path = options["output"] | ||
| with open(output_path, "w", encoding="utf-8") as output_file: | ||
| output_file.write(render_module(messages)) | ||
|
|
||
| self.stdout.write( | ||
| self.style.SUCCESS(f"Extracted {len(messages)} translatable schema string(s) to {output_path}") | ||
| ) |
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.
Nit: I think these comments on PR dependencies shouldn't be kept in code to be merged, only on PR comments, at this will become stale as soon as these are merged.