Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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.
11 changes: 10 additions & 1 deletion backend/src/opendlp/domain/respondent_field_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
ChoiceOption,
FieldOnRegistrationPage,
FieldType,
FixedFieldError,
RespondentFieldDefinition,
RespondentFieldGroup,
humanise_field_key,
Expand All @@ -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

Expand Down Expand Up @@ -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] = {}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
]
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()

Expand Down
2 changes: 2 additions & 0 deletions backend/tests/e2e/test_backoffice_respondent_field_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
5 changes: 4 additions & 1 deletion backend/tests/unit/test_respondent_field_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
ChoiceOption,
FieldOnRegistrationPage,
FieldType,
FixedFieldError,
RespondentFieldDefinition,
RespondentFieldGroup,
humanise_field_key,
Expand Down Expand Up @@ -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:
Expand Down
70 changes: 55 additions & 15 deletions backend/translations/hu/LC_MESSAGES/messages.po
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading