Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
0be851e
Adds test
Tschuppi81 Dec 16, 2025
d304594
Fix field type migration for radio to checkbox
Tschuppi81 Dec 16, 2025
9f5ca2e
Adds another test
Tschuppi81 Dec 16, 2025
437df27
Supports renaming radio or checkbox label
Tschuppi81 Dec 18, 2025
32a4ef6
Resolve related fixme
Tschuppi81 Dec 18, 2025
87beef2
Ensure form validation lead to user error message
Tschuppi81 Dec 22, 2025
560ef88
Detect added, removed and renamed options
Tschuppi81 Dec 22, 2025
ced7192
Revert
Tschuppi81 Dec 22, 2025
db829ed
Extend test
Tschuppi81 Dec 22, 2025
43a3be9
Merge branch 'master' into bugfix/ogc-2353-fix-directory-migration-crash
Tschuppi81 Dec 22, 2025
1e3ed4a
Merge branch 'master' into bugfix/ogc-2353-fix-directory-migration-crash
Tschuppi81 Dec 22, 2025
58eec5d
Extend test
Tschuppi81 Dec 29, 2025
e1efe9f
Alert user about migration issues
Tschuppi81 Dec 29, 2025
c0d9f5c
Workaround to keep translation
Tschuppi81 Dec 29, 2025
85f426d
Minor changes
Tschuppi81 Dec 29, 2025
68fe93a
Merge branch 'master' into bugfix/ogc-2353-fix-directory-migration-crash
Tschuppi81 Dec 29, 2025
f9571da
Rename method
Tschuppi81 Dec 29, 2025
9416f14
Disable alert invalid conversion
Tschuppi81 Dec 30, 2025
f4a88e6
Move alert logic to view preventing hierarchy issue
Tschuppi81 Dec 30, 2025
3e342ac
Refactor type migration alerting
Tschuppi81 Dec 30, 2025
527bab9
Merge master
Tschuppi81 Jan 8, 2026
a49d45e
Remove useless set conversion
Tschuppi81 Jan 8, 2026
423d47b
Fix renamed option recognition
Tschuppi81 Jan 9, 2026
7583354
Merge master
Tschuppi81 Jan 9, 2026
8abbd8c
Adjust test
Tschuppi81 Jan 12, 2026
7ee83e3
Adds missing translation and adds new once
Tschuppi81 Jan 12, 2026
98226df
Shows changed options for migrations
Tschuppi81 Jan 12, 2026
20c81f1
Fix linter issue
Tschuppi81 Jan 12, 2026
6c428e4
Align translations
Tschuppi81 Jan 12, 2026
3702432
Improve alerting for newly added fields marked as required
Tschuppi81 Jan 16, 2026
2b635f8
Adds translations
Tschuppi81 Jan 16, 2026
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
149 changes: 138 additions & 11 deletions src/onegov/directory/migration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -58,14 +59,21 @@

@property
def possible(self) -> bool:
""" Returns True if the migration is possible, False otherwise. """
if not self.directory.entries:
return True

if not self.changes:
return True

if not self.changes.changed_fields:
return True
if len(self.changes.renamed_options) > 1:
return False

if self.multiple_option_changes_in_one_step():
return False

if self.added_required_fields():
return False

for changed in self.changes.changed_fields:
old = self.changes.old[changed]
Expand Down Expand Up @@ -142,6 +150,8 @@
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:
Expand Down Expand Up @@ -170,8 +180,57 @@
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 isinstance(val, list):
values[key] = [
new_label if v == old_label else v for v in val
]

elif 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
Comment on lines +196 to +203
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems slightly controversial to delete existing selections. We might want to add an additional warning/confirmation step, if we detect that removing/renaming an option would modify the recorded data of existing entries (it would always be fine if none of the existing entries have selected this option).

Copy link
Contributor Author

@Tschuppi81 Tschuppi81 Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is actually the same as with removing fields.
This is what you currently need to confirm:
image

Is that what you meant (A) or do you think of an extra step (B) where each entry would need to be changed in a separate window before an option can be removed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, although it might be nice to be a bit more explicit that some data will be lost if there are existing entries that use the removed option.


def multiple_option_changes_in_one_step(self) -> bool:
"""
Returns True if there are multiple changes e.g. added and
removed options.
"""

if (
(self.changes.added_options and self.changes.removed_options)
or (self.changes.added_options and self.changes.renamed_options)
or (self.changes.removed_options and self.changes.renamed_options)
):
return True
return False

def added_required_fields(self) -> bool:
"""
Identify newly added fields that are set to be required. Newly added
fields shall not be required if entries exist, make them required
in a separate migration step.
"""
if self.directory.entries:
return any(
f.required for f in self.changes.new.values()
if f.human_id in self.changes.added_fields
)

return False

class FieldTypeMigrations:

Check failure on line 233 in src/onegov/directory/migration.py

View workflow job for this annotation

GitHub Actions / Lint

Ruff (E302)

src/onegov/directory/migration.py:233:1: E302 Expected 2 blank lines, found 1
""" Contains methods to migrate fields from one type to another. """

def possible(self, old_type: str, new_type: str) -> bool:
Expand All @@ -194,10 +253,6 @@

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()

Expand All @@ -214,16 +269,16 @@
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
Expand Down Expand Up @@ -255,13 +310,19 @@
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.added_options
or self.removed_options
or self.renamed_options # radio and checkboxes
)

def detect_removed_fieldsets(self) -> None:
Expand Down Expand Up @@ -378,3 +439,69 @@
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

# test if re-ordered
if set(old_labels) == set(new_labels):
continue

# only consider renames if the number of options
# remains the same
if len(old_labels) != len(new_labels):
continue

for o_label, n_label in zip(old_labels, new_labels):
if o_label != n_label:
self.renamed_options[(old_id, o_label)] = (
old_id,
n_label
)

self.added_options = [
ao for ao in self.added_options
if ao not in self.renamed_options.values()
]
self.removed_options = [
ro for ro in self.removed_options
if ro not in self.renamed_options.keys()
]
Loading
Loading