diff --git a/backend/docs/agent/610-registration-page-html/add-remove-fields-plan.md b/backend/docs/agent/610-registration-page-html/add-remove-fields-plan.md index c11587c3..05f2fee0 100644 --- a/backend/docs/agent/610-registration-page-html/add-remove-fields-plan.md +++ b/backend/docs/agent/610-registration-page-html/add-remove-fields-plan.md @@ -281,6 +281,9 @@ Per repo convention (docs separate from code): - [x] **Phase 4 — Template.** Bottom-of-page "Add a field" form. - [x] **Phase 5 — e2e tests.** `TestAddField`. - [x] **Phase 6 — Wrap-up.** `just translate-regen`, tests, lint/type/dep checks, commit code. +- [x] **Phase 7 — Error i18n.** Translate the Fields-page `FieldDefinitionConflictError` + messages; introduce `FixedFieldError` so the fixed-field case is recognised by type + rather than the brittle `"fixed" in str(exc)` check (Option C, service-owned message). > Note: `just check`'s prek step can't run in this sandbox (read-only uv tools dir), so > the equivalent tools were run directly — `mypy`, `deptry src`, `ruff check`, `ruff format` @@ -291,6 +294,24 @@ Per repo convention (docs separate from code): ## Follow-up (after this round) -- Translate the service-layer `FieldDefinitionConflictError` messages so the duplicate/empty - cases don't `flash(str(e))` untranslated (decision 5). Affects the existing - update/option/delete routes too, so do it as a separate pass. +- **Done (Phase 7):** the Fields-page `FieldDefinitionConflictError` messages are now + `_l()`-wrapped, and the fixed-field guard is a typed `FixedFieldError` (domain) caught in + the service, which raises a translated conflict. The `NotFoundError`/`AssemblyNotFoundError` + messages were intentionally left English — the blueprint flashes its own translated strings + and only logs `str(e)` — as was the `reorder_group` programming guard (never user-facing). + +- **`InvalidSelection` half-translated flash (separate, CSV-upload feature).** + `_parse_csv_headers` raises `InvalidSelection("CSV file is empty or has no header row")` + (in this service), and `respondents.py` surfaces it as + `flash(_("Invalid CSV format: %(error)s", error=str(e)))` — the wrapper is translated but + the interpolated message is not, so users see a half-translated flash. Fixing it belongs + with the CSV-upload / reconciliation work, not the Fields page. + +- **Broader flash-message i18n audit (its own ticket).** The `InvalidSelection` case above is + an instance of a general pattern: `flash(_("… %(error)s", error=str(e)))` where the inner + exception message is plain English. The same shape appears across `respondents.py`, + `gsheets*.py`, `targets*.py`, and `db_selection*.py` (many `except InvalidSelection as e` + sites). A focused pass should audit every such site, decide which inner messages are + genuinely user-facing, and `_l()`-wrap them at the raise site in the relevant services + (`respondent_service`, `assembly_service`, `sortition`, …). Scope it separately — it spans + several features and is larger than this branch. diff --git a/backend/src/opendlp/domain/respondent_field_schema.py b/backend/src/opendlp/domain/respondent_field_schema.py index b8256c1f..89c8047e 100644 --- a/backend/src/opendlp/domain/respondent_field_schema.py +++ b/backend/src/opendlp/domain/respondent_field_schema.py @@ -32,6 +32,15 @@ _UNSET: Any = object() +class FixedFieldError(ValueError): + """Raised when an edit is rejected because the field is a fixed field. + + Subclasses ``ValueError`` so existing ``except ValueError`` callers still + catch it; the distinct type lets the service layer recognise this case and + surface a translated message without matching on the message text. + """ + + class RespondentFieldGroup(Enum): """Fixed catalogue of groups that a respondent field can belong to. @@ -247,7 +256,7 @@ def update( changed = True if field_type is not None or options is not _UNSET: if self.is_fixed: - raise ValueError("Cannot change field_type or options on a fixed field") + raise FixedFieldError("Cannot change field_type or options on a fixed field") new_type = field_type if field_type is not None else self.field_type # When the caller didn't pass options explicitly, preserve the # current list across choice<->choice transitions, but drop it diff --git a/backend/src/opendlp/service_layer/respondent_field_schema_service.py b/backend/src/opendlp/service_layer/respondent_field_schema_service.py index c66ae53e..8e723a74 100644 --- a/backend/src/opendlp/service_layer/respondent_field_schema_service.py +++ b/backend/src/opendlp/service_layer/respondent_field_schema_service.py @@ -20,6 +20,7 @@ ChoiceOption, FieldOnRegistrationPage, FieldType, + FixedFieldError, RespondentFieldDefinition, RespondentFieldGroup, humanise_field_key, @@ -34,6 +35,7 @@ from opendlp.service_layer.permissions import can_manage_assembly, can_view_assembly from opendlp.service_layer.respondent_field_schema_heuristics import classify_field_key from opendlp.service_layer.unit_of_work import AbstractUnitOfWork +from opendlp.translations import lazy_gettext as _l _MAX_RADIO_OPTIONS = 6 @@ -244,12 +246,12 @@ def add_field( field_key = field_key.strip() if not field_key: - raise FieldDefinitionConflictError("Field key cannot be empty") + raise FieldDefinitionConflictError(_l("Field key cannot be empty")) # Check for duplicate field_key existing = uow.respondent_field_definitions.list_by_assembly(assembly_id) if any(f.field_key == field_key for f in existing): - raise FieldDefinitionConflictError(f"Field '{field_key}' already exists in this assembly") + raise FieldDefinitionConflictError(_l("Field '%(key)s' already exists in this assembly", key=field_key)) # Compute next sort_order for this group per_group_next: dict[RespondentFieldGroup, int] = {} @@ -304,10 +306,8 @@ def update_field( options=options, on_registration_page=on_registration_page, ) - except ValueError as exc: - if "fixed" in str(exc): - raise FieldDefinitionConflictError(str(exc)) from exc - raise + except FixedFieldError as exc: + raise FieldDefinitionConflictError(_l("You can't change the type or options of a fixed field")) from exc uow.commit() detached: RespondentFieldDefinition = field.create_detached_copy() return detached @@ -418,13 +418,13 @@ def add_choice_option( if field is None or field.assembly_id != assembly_id: raise FieldDefinitionNotFoundError(f"Field {field_id} not found in assembly {assembly_id}") if field.field_type not in {FieldType.CHOICE_RADIO, FieldType.CHOICE_DROPDOWN}: - raise FieldDefinitionConflictError("Options can only be set on choice fields") + raise FieldDefinitionConflictError(_l("Options can only be set on choice fields")) value = value.strip() if not value: - raise FieldDefinitionConflictError("Option value cannot be blank") + raise FieldDefinitionConflictError(_l("Option value cannot be blank")) new_options = list(field.options or []) if any(o.value == value for o in new_options): - raise FieldDefinitionConflictError(f"Option '{value}' already exists") + raise FieldDefinitionConflictError(_l("Option '%(value)s' already exists", value=value)) new_options.append(ChoiceOption(value=value, help_text=help_text)) field.update(options=new_options) uow.commit() @@ -452,12 +452,12 @@ def update_choice_option( raise FieldDefinitionNotFoundError(f"Field {field_id} not found in assembly {assembly_id}") new_value = new_value.strip() if not new_value: - raise FieldDefinitionConflictError("Option value cannot be blank") + raise FieldDefinitionConflictError(_l("Option value cannot be blank")) existing = list(field.options or []) if not any(o.value == old_value for o in existing): raise FieldDefinitionNotFoundError(f"Option '{old_value}' not found on field {field_id}") if new_value != old_value and any(o.value == new_value for o in existing): - raise FieldDefinitionConflictError(f"Option '{new_value}' already exists") + raise FieldDefinitionConflictError(_l("Option '%(value)s' already exists", value=new_value)) updated_options = [ ChoiceOption(value=new_value, help_text=new_help_text) if o.value == old_value else o for o in existing ] @@ -485,7 +485,7 @@ def remove_choice_option( if len(remaining) == len(existing): raise FieldDefinitionNotFoundError(f"Option '{value}' not found on field {field_id}") if not remaining: - raise FieldDefinitionConflictError("A choice field must keep at least one option") + raise FieldDefinitionConflictError(_l("A choice field must keep at least one option")) field.update(options=remaining) uow.commit() detached: RespondentFieldDefinition = field.create_detached_copy() @@ -538,7 +538,7 @@ def delete_field( if field is None or field.assembly_id != assembly_id: raise FieldDefinitionNotFoundError(f"Field {field_id} not found in assembly {assembly_id}") if field.is_fixed: - raise FieldDefinitionConflictError(f"Fixed field '{field.field_key}' cannot be deleted") + raise FieldDefinitionConflictError(_l("Fixed field '%(key)s' cannot be deleted", key=field.field_key)) uow.respondent_field_definitions.delete(field) uow.commit() diff --git a/backend/tests/e2e/test_backoffice_respondent_field_schema.py b/backend/tests/e2e/test_backoffice_respondent_field_schema.py index 4e56d4c5..9f255531 100644 --- a/backend/tests/e2e/test_backoffice_respondent_field_schema.py +++ b/backend/tests/e2e/test_backoffice_respondent_field_schema.py @@ -290,6 +290,8 @@ def test_add_duplicate_key_is_rejected( follow_redirects=True, ) assert response.status_code == 200 + # The translated conflict message (a LazyString) renders via flash(str(e)). + assert b"already exists" in response.data with SqlAlchemyUnitOfWork(postgres_session_factory) as uow: after = len(respondent_field_schema_service.get_schema(uow, admin_user.id, existing_assembly.id)) diff --git a/backend/tests/unit/test_respondent_field_schema.py b/backend/tests/unit/test_respondent_field_schema.py index 600abebe..2af44017 100644 --- a/backend/tests/unit/test_respondent_field_schema.py +++ b/backend/tests/unit/test_respondent_field_schema.py @@ -16,6 +16,7 @@ ChoiceOption, FieldOnRegistrationPage, FieldType, + FixedFieldError, RespondentFieldDefinition, RespondentFieldGroup, humanise_field_key, @@ -396,7 +397,9 @@ def test_accepts_choice_dropdown_with_options(self) -> None: def test_update_refuses_type_change_on_fixed_row(self) -> None: field = self._field(field_key="email", is_fixed=True) - with pytest.raises(ValueError, match="fixed"): + # FixedFieldError is a ValueError subclass, but the distinct type lets the + # service layer translate the message without matching on its text. + with pytest.raises(FixedFieldError): field.update(field_type=FieldType.TEXT) def test_update_changes_type_and_options_together_for_non_fixed_row(self) -> None: diff --git a/backend/translations/hu/LC_MESSAGES/messages.po b/backend/translations/hu/LC_MESSAGES/messages.po index eacf6c4d..eec6ad0c 100644 --- a/backend/translations/hu/LC_MESSAGES/messages.po +++ b/backend/translations/hu/LC_MESSAGES/messages.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: PROJECT VERSION\n" "Report-Msgid-Bugs-To: EMAIL@ADDRESS\n" -"POT-Creation-Date: 2026-06-11 13:53+0100\n" +"POT-Creation-Date: 2026-06-11 14:30+0100\n" "PO-Revision-Date: 2025-10-06 11:07+0200\n" "Last-Translator: \n" "Language: hu\n" @@ -61,64 +61,64 @@ msgstr "" msgid "Processing…" msgstr "" -#: src/opendlp/domain/respondent_field_schema.py:62 +#: src/opendlp/domain/respondent_field_schema.py:71 msgid "Eligibility" msgstr "" -#: src/opendlp/domain/respondent_field_schema.py:63 +#: src/opendlp/domain/respondent_field_schema.py:72 msgid "Name and contact" msgstr "" -#: src/opendlp/domain/respondent_field_schema.py:64 +#: src/opendlp/domain/respondent_field_schema.py:73 #, fuzzy msgid "Address" msgstr "Email cím" -#: src/opendlp/domain/respondent_field_schema.py:65 +#: src/opendlp/domain/respondent_field_schema.py:74 msgid "About you" msgstr "" -#: src/opendlp/domain/respondent_field_schema.py:66 +#: src/opendlp/domain/respondent_field_schema.py:75 #: templates/backoffice/assembly_view_respondent.html:177 #: templates/respondents/view_respondents.html:165 msgid "Consent" msgstr "" -#: src/opendlp/domain/respondent_field_schema.py:67 +#: src/opendlp/domain/respondent_field_schema.py:76 msgid "Other" msgstr "" -#: src/opendlp/domain/respondent_field_schema.py:96 +#: src/opendlp/domain/respondent_field_schema.py:105 #, fuzzy msgid "Text" msgstr "Kezdés dátuma" -#: src/opendlp/domain/respondent_field_schema.py:97 +#: src/opendlp/domain/respondent_field_schema.py:106 msgid "Long text" msgstr "" -#: src/opendlp/domain/respondent_field_schema.py:98 +#: src/opendlp/domain/respondent_field_schema.py:107 msgid "Yes / No" msgstr "" -#: src/opendlp/domain/respondent_field_schema.py:99 +#: src/opendlp/domain/respondent_field_schema.py:108 msgid "Yes / No / Not set" msgstr "" -#: src/opendlp/domain/respondent_field_schema.py:100 +#: src/opendlp/domain/respondent_field_schema.py:109 msgid "Choice (radios)" msgstr "" -#: src/opendlp/domain/respondent_field_schema.py:101 +#: src/opendlp/domain/respondent_field_schema.py:110 msgid "Choice (dropdown)" msgstr "" -#: src/opendlp/domain/respondent_field_schema.py:102 +#: src/opendlp/domain/respondent_field_schema.py:111 #, fuzzy msgid "Whole number" msgstr "Módosítsa a számot." -#: src/opendlp/domain/respondent_field_schema.py:103 +#: src/opendlp/domain/respondent_field_schema.py:112 #: templates/admin/invite_view.html:47 templates/admin/invites.html:66 #: templates/admin/user_view.html:22 templates/admin/users.html:113 #: templates/backoffice/assembly_members.html:53 @@ -2688,6 +2688,46 @@ msgstr "" msgid "login" msgstr "Bejelentkezés" +#: src/opendlp/service_layer/respondent_field_schema_service.py:249 +#, fuzzy +msgid "Field key cannot be empty" +msgstr "Utoljára módosítva" + +#: src/opendlp/service_layer/respondent_field_schema_service.py:255 +#, python-format +msgid "Field '%(key)s' already exists in this assembly" +msgstr "" + +#: src/opendlp/service_layer/respondent_field_schema_service.py:313 +#, fuzzy +msgid "You can't change the type or options of a fixed field" +msgstr "Jelenleg egy közösségi gyűlés eléréséhez sincs jogosultságod" + +#: src/opendlp/service_layer/respondent_field_schema_service.py:425 +msgid "Options can only be set on choice fields" +msgstr "" + +#: src/opendlp/service_layer/respondent_field_schema_service.py:428 +#: src/opendlp/service_layer/respondent_field_schema_service.py:459 +#, fuzzy +msgid "Option value cannot be blank" +msgstr "Kiválasztás" + +#: src/opendlp/service_layer/respondent_field_schema_service.py:431 +#: src/opendlp/service_layer/respondent_field_schema_service.py:464 +#, fuzzy, python-format +msgid "Option '%(value)s' already exists" +msgstr "A felhasználó már létezik" + +#: src/opendlp/service_layer/respondent_field_schema_service.py:492 +msgid "A choice field must keep at least one option" +msgstr "" + +#: src/opendlp/service_layer/respondent_field_schema_service.py:545 +#, python-format +msgid "Fixed field '%(key)s' cannot be deleted" +msgstr "" + #: src/opendlp/service_layer/selection_report.py:193 #: templates/backoffice/patterns/_dropdown.html:148 #: templates/backoffice/service_docs.html:41