Directory: Fix directory migration crash

src/onegov/directory/migration.py

@@ -5,11 +5,12 @@
 from onegov.form import flatten_fieldsets
 from onegov.form import parse_form
 from onegov.form import parse_formcode
+from onegov.form.parser.core import OptionsField
 from sqlalchemy.orm import object_session, joinedload, undefer
 from sqlalchemy.orm.attributes import get_history
 
-
 from typing import Any, TYPE_CHECKING
+
 if TYPE_CHECKING:
     from collections.abc import Callable, Iterable
     from datetime import date, datetime, time
@@ -64,7 +65,8 @@ def possible(self) -> bool:
         if not self.changes:
             return True
 
-        if not self.changes.changed_fields:
+        if (not self.changes.changed_fields and
+                not self.changes.renamed_options):
             return True
 
         for changed in self.changes.changed_fields:
@@ -142,6 +144,8 @@ def migrate_values(self, values: dict[str, Any]) -> None:
         self.remove_old_fields(values)
         self.rename_fields(values)
         self.convert_fields(values)
+        self.rename_options(values)
+        self.remove_old_options(values)
 
     def add_new_fields(self, values: dict[str, Any]) -> None:
         for added in self.changes.added_fields:
@@ -170,6 +174,23 @@ def convert_fields(self, values: dict[str, Any]) -> None:
             changed = as_internal_id(changed)
             values[changed] = convert(values[changed])
 
+    def rename_options(self, values: dict[str, Any]) -> None:
+        for old_option, new_option in self.changes.renamed_options.items():
+            old_label = old_option[1]
+            new_label = new_option[1]
+            for key, val in list(values.items()):
+                if val == old_label:
+                    values[key] = new_label
+
+    def remove_old_options(self, values: dict[str, Any]) -> None:
+        for human_id, label in self.changes.removed_options:
+            id = as_internal_id(human_id)
+            if id in values:
+                if isinstance(values[id], list):
+                    values[id] = [v for v in values[id] if v != label]
+                elif values[id] == label:
+                    values[id] = None
+
 
 class FieldTypeMigrations:
     """ Contains methods to migrate fields from one type to another. """
@@ -194,10 +215,6 @@ def get_converter(
 
         return getattr(self, explicit, getattr(self, generic, None))
 
-    # FIXME: A lot of these converters currently don't work if the value
-    #        happens to be None, which should be possible for every field
-    #        as long as its optional, or do we skip converting None
-    #        explicitly somewhere?!
     def any_to_text(self, value: Any) -> str:
         return str(value if value is not None else '').strip()
 
@@ -214,16 +231,16 @@ def text_to_code(self, value: str) -> str:
         return value
 
     def date_to_text(self, value: date) -> str:
-        return '{:%d.%m.%Y}'.format(value)
+        return '{:%d.%m.%Y}'.format(value) if value else ''
 
     def datetime_to_text(self, value: datetime) -> str:
-        return '{:%d.%m.%Y %H:%M}'.format(value)
+        return '{:%d.%m.%Y %H:%M}'.format(value) if value else ''
 
     def time_to_text(self, value: time) -> str:
-        return '{:%H:%M}'.format(value)
+        return '{:%H:%M}'.format(value) if value else ''
 
     def radio_to_checkbox(self, value: str) -> list[str]:
-        return [value]
+        return [value] if value else []
 
     def text_to_url(self, value: str) -> str:
         return value
@@ -255,13 +272,17 @@ def __init__(self, old_structure: str, new_structure: str) -> None:
         self.detect_removed_fields()
         self.detect_renamed_fields()  # modifies added/removed fields
         self.detect_changed_fields()
+        self.detect_added_options()
+        self.detect_removed_options()
+        self.detect_renamed_options()
 
     def __bool__(self) -> bool:
         return bool(
             self.added_fields
             or self.removed_fields
             or self.renamed_fields
             or self.changed_fields
+            or self.renamed_options  # radio and checkboxes
         )
 
     def detect_removed_fieldsets(self) -> None:
@@ -378,3 +399,56 @@ def detect_changed_fields(self) -> None:
             new = self.new[new_id]
             if old.required != new.required or old.type != new.type:
                 self.changed_fields.append(new_id)
+
+    def detect_added_options(self) -> None:
+        self.added_options = []
+
+        for old_id, old_field in self.old.items():
+            if isinstance(old_field, OptionsField) and old_id in self.new:
+                new_field = self.new[old_id]
+                if isinstance(new_field, OptionsField):
+                    new_labels = [r.label for r in new_field.choices]
+                    old_labels = [r.label for r in old_field.choices]
+
+                    for n in new_labels:
+                        if n not in old_labels:
+                            self.added_options.append((old_id, n))
+
+    def detect_removed_options(self) -> None:
+        self.removed_options = []
+
+        for old_id, old_field in self.old.items():
+            if isinstance(old_field, OptionsField) and old_id in self.new:
+                new_field = self.new[old_id]
+                if isinstance(new_field, OptionsField):
+                    new_labels = [r.label for r in new_field.choices]
+                    old_labels = [r.label for r in old_field.choices]
+
+                    for o in old_labels:
+                        if o not in new_labels:
+                            self.removed_options.append((old_id, o))
+
+    def detect_renamed_options(self) -> None:
+        self.renamed_options = {}
+
+        for old_id, old_field in self.old.items():
+            if isinstance(old_field, OptionsField) and old_id in self.new:
+                new_field = self.new[old_id]
+                if isinstance(new_field, OptionsField):
+                    old_labels = [r.label for r in old_field.choices]
+                    new_labels = [r.label for r in new_field.choices]
+
+                    if old_labels == new_labels:
+                        continue
+
+                    for r, a in zip(self.removed_options, self.added_options):
+                        self.renamed_options[r] = a
+
+                    self.added_options = [
+                        f for f in self.added_options
+                        if f not in self.renamed_options.values()
+                    ]
+                    self.removed_options = [
+                        f for f in self.removed_options
+                        if f not in self.renamed_options
+                    ]

src/onegov/org/locale/de_CH/LC_MESSAGES/onegov.org.po

@@ -2,7 +2,7 @@
 msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE 1.0\n"
-"POT-Creation-Date: 2025-12-29 09:08+0100\n"
+"POT-Creation-Date: 2025-12-29 12:33+0100\n"
 "PO-Revision-Date: 2022-03-15 10:21+0100\n"
 "Last-Translator: Marc Sommerhalder <marc.sommerhalder@seantis.ch>\n"
 "Language-Team: German\n"
@@ -6347,6 +6347,14 @@ msgstr "Ein neues Verzeichnis wurde hinzugefügt"
 msgid "New Directory"
 msgstr "Neues Verzeichnis"
 
+#, python-format
+msgid ""
+"Cannot convert field \"${field}\" from type \"${old_type}\" to "
+"\"${new_type}\"."
+msgstr ""
+"Feld \"${field}\" kann nicht von Typ \"${old_type}\" zu Typ \"${new_type}\" "
+"konvertiert werden."
+
 msgid ""
 "The requested change cannot be performed, as it is incompatible with "
 "existing entries"

src/onegov/org/locale/fr_CH/LC_MESSAGES/onegov.org.po

@@ -2,7 +2,7 @@
 msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE 1.0\n"
-"POT-Creation-Date: 2025-12-29 09:08+0100\n"
+"POT-Creation-Date: 2025-12-29 12:33+0100\n"
 "PO-Revision-Date: 2022-03-15 10:50+0100\n"
 "Last-Translator: Marc Sommerhalder <marc.sommerhalder@seantis.ch>\n"
 "Language-Team: French\n"
@@ -6361,6 +6361,14 @@ msgstr "Ajout d'un nouveau dossier"
 msgid "New Directory"
 msgstr "Nouveau dossier"
 
+#, python-format
+msgid ""
+"Cannot convert field \"${field}\" from type \"${old_type}\" to "
+"\"${new_type}\"."
+msgstr ""
+"Impossible de convertir le champ \"${field}\" du type \"${old_type}\" en "
+"\"${new_type}\"."
+
 msgid ""
 "The requested change cannot be performed, as it is incompatible with "
 "existing entries"

src/onegov/org/locale/it_CH/LC_MESSAGES/onegov.org.po

@@ -2,7 +2,7 @@
 msgid ""
 msgstr ""
 "Project-Id-Version: \n"
-"POT-Creation-Date: 2025-12-29 09:08+0100\n"
+"POT-Creation-Date: 2025-12-29 12:33+0100\n"
 "PO-Revision-Date: 2022-03-15 10:52+0100\n"
 "Last-Translator: \n"
 "Language-Team: \n"
@@ -6336,6 +6336,14 @@ msgstr "Aggiunta una nuova cartella"
 msgid "New Directory"
 msgstr "Nuova cartella"
 
+#, python-format
+msgid ""
+"Cannot convert field \"${field}\" from type \"${old_type}\" to "
+"\"${new_type}\"."
+msgstr ""
+"Impossibile convertire il campo \"${field}\" dal tipo \"${old_type}\" a "
+"\"${new_type}\"."
+
 msgid ""
 "The requested change cannot be performed, as it is incompatible with "
 "existing entries"

src/onegov/org/views/directory.py

@@ -51,6 +51,7 @@
 if TYPE_CHECKING:
     from collections.abc import Mapping, Sequence, Iterator
     from onegov.core.types import JSON_ro, RenderData, EmailJsonDict
+    from onegov.directory.migration import DirectoryMigration
     from onegov.directory.models.directory import DirectoryEntryForm
     from onegov.org.models.directory import ExtendedDirectoryEntryForm
     from onegov.org.request import OrgRequest
@@ -202,6 +203,10 @@ def handle_new_directory(
     }
 
 
+# no op call to make translators aware of this string used in migration.reason
+_('Cannot convert field "${field}" from type "${old_type}" to "${new_type}".')
+
+
 @OrgApp.form(model=ExtendedDirectoryEntryCollection, name='edit',
              template='directory_form.pt', permission=Secret,
              form=get_directory_form_class)
@@ -233,15 +238,15 @@ def handle_edit_directory(
                             'The requested change cannot be performed, '
                             'as it is incompatible with existing entries'
                         ))
+                        alert_migration_type_errors(migration, request)
                     else:
                         if not request.params.get('confirm'):
                             form.action += '&confirm=1'
                             save_changes = False
 
             if save_changes:
-                form.populate_obj(self.directory)
-
                 try:
+                    form.populate_obj(self.directory)
                     self.session.flush()
                 except ValidationError as e:
                     error = e
@@ -298,6 +303,25 @@ def handle_edit_directory(
     }
 
 
+def alert_migration_type_errors(
+    migration: DirectoryMigration,
+    request: OrgRequest
+) -> None:
+    for changed in migration.changes.changed_fields:
+        old = migration.changes.old[changed]
+        new = migration.changes.new[changed]
+
+        if not migration.fieldtype_migrations.possible(old.type, new.type):
+            request.alert(_(
+                'Cannot convert field "${field}" from type "${old_type}" '
+                'to "${new_type}".', mapping={
+                    'field': changed,
+                    'old_type': old.type,
+                    'new_type': new.type
+                }
+            ))
+
+
 @OrgApp.view(
     model=ExtendedDirectoryEntryCollection,
     permission=Secret,

src/onegov/town6/templates/directory_form.pt

@@ -45,6 +45,9 @@
                 <li tal:repeat="field migration.changes.changed_fields">
                     <tal:b i18n:translate>Changed:</tal:b> ${field}
                 </li>
+                <li tal:repeat="(old, new) migration.changes.renamed_options.items()|nothing">
+                    <tal:b i18n:translate>Renamed:</tal:b> ${old[0]}: ${old[1]} -> ${new[0]}: ${new[1]}
+                </li>
             </ul>
 
             <div class="button primary" data-submits-form="#confirm-migration form" i18n:translate>