From 6308fbfcd76d45caefba50e2b3bbfe3a764681ad Mon Sep 17 00:00:00 2001 From: Ross Date: Mon, 24 Nov 2025 10:18:26 +0000 Subject: [PATCH] Refactor Finding and Structure models: remove legacy `primary` and `synonym` fields, introduce `canonical` field for aliasing. Update filters and forms accordingly. --- atlas/filters.py | 41 +++++- atlas/forms.py | 8 +- ...primary_remove_finding_synonym_and_more.py | 40 ++++++ atlas/models.py | 120 +++++++++++++----- 4 files changed, 169 insertions(+), 40 deletions(-) create mode 100644 atlas/migrations/0088_remove_finding_primary_remove_finding_synonym_and_more.py diff --git a/atlas/filters.py b/atlas/filters.py index a86ed2b8..9daa9e95 100755 --- a/atlas/filters.py +++ b/atlas/filters.py @@ -209,9 +209,15 @@ class ConditionFilter(django_filters.FilterSet): class FindingFilter(django_filters.FilterSet): + # Replace legacy `primary`/`synonym` with canonical-based filters + is_canonical = django_filters.BooleanFilter(method="filter_is_canonical", label="Canonical") + synonym = django_filters.ModelChoiceFilter(queryset=Finding.objects.all(), method="filter_synonym", label="Synonym group") + class Meta: model = Finding - fields = ("name", "primary", "synonym") + fields = { + "name": ["icontains"], + } def __init__( self, @@ -227,11 +233,29 @@ class FindingFilter(django_filters.FilterSet): ) pass + def filter_is_canonical(self, queryset, name, value): + if value in (True, "True", "true", 1, "1"): + return queryset.filter(canonical__isnull=True) + if value in (False, "False", "false", 0, "0"): + return queryset.filter(canonical__isnull=False) + return queryset + + def filter_synonym(self, queryset, name, value): + # value is a Finding instance selected in the filter. Return all + # Findings that belong to the same canonical group as `value`. + if not value: + return queryset + master = value.canonical if value.canonical else value + return queryset.filter(Q(canonical=master) | Q(pk=master.pk)) + class StructureFilter(django_filters.FilterSet): + is_canonical = django_filters.BooleanFilter(method="filter_is_canonical", label="Canonical") + synonym = django_filters.ModelChoiceFilter(queryset=Structure.objects.all(), method="filter_synonym", label="Synonym group") + class Meta: model = Structure - fields = {"name": ["icontains"], "primary": ["exact"], "synonym": ["exact"]} + fields = {"name": ["icontains"]} def __init__( self, @@ -247,6 +271,19 @@ class StructureFilter(django_filters.FilterSet): ) pass + def filter_is_canonical(self, queryset, name, value): + if value in (True, "True", "true", 1, "1"): + return queryset.filter(canonical__isnull=True) + if value in (False, "False", "false", 0, "0"): + return queryset.filter(canonical__isnull=False) + return queryset + + def filter_synonym(self, queryset, name, value): + if not value: + return queryset + master = value.canonical if value.canonical else value + return queryset.filter(Q(canonical=master) | Q(pk=master.pk)) + class PresentationFilter(django_filters.FilterSet): class Meta: diff --git a/atlas/forms.py b/atlas/forms.py index 60fbda07..7640554d 100755 --- a/atlas/forms.py +++ b/atlas/forms.py @@ -327,9 +327,10 @@ class FindingForm(ModelForm): class Meta: model = Finding exclude = [] - + # Use the canonical FK in forms (if users want to mark this Finding + # as an alias of another). The old `synonym` M2M has been removed. widgets = { - "synonym": autocomplete.ModelSelect2Multiple( + "canonical": autocomplete.ModelSelect2( url="atlas:finding-autocomplete" ), } @@ -339,9 +340,8 @@ class StructureForm(ModelForm): class Meta: model = Structure exclude = [] - widgets = { - "synonym": autocomplete.ModelSelect2Multiple( + "canonical": autocomplete.ModelSelect2( url="atlas:structure-autocomplete" ), } diff --git a/atlas/migrations/0088_remove_finding_primary_remove_finding_synonym_and_more.py b/atlas/migrations/0088_remove_finding_primary_remove_finding_synonym_and_more.py new file mode 100644 index 00000000..fc121c35 --- /dev/null +++ b/atlas/migrations/0088_remove_finding_primary_remove_finding_synonym_and_more.py @@ -0,0 +1,40 @@ +# Generated by Django 5.2.7 on 2025-11-24 10:17 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('atlas', '0087_alter_condition_canonical'), + ] + + operations = [ + migrations.RemoveField( + model_name='finding', + name='primary', + ), + migrations.RemoveField( + model_name='finding', + name='synonym', + ), + migrations.RemoveField( + model_name='structure', + name='primary', + ), + migrations.RemoveField( + model_name='structure', + name='synonym', + ), + migrations.AddField( + model_name='finding', + name='canonical', + field=models.ForeignKey(blank=True, help_text='If set, this Finding is an alias and points to the canonical Finding.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='aliases', to='atlas.finding'), + ), + migrations.AddField( + model_name='structure', + name='canonical', + field=models.ForeignKey(blank=True, help_text='If set, this Structure is an alias and points to the canonical Structure.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='aliases', to='atlas.structure'), + ), + ] diff --git a/atlas/models.py b/atlas/models.py index 03c31d98..7ebd9427 100644 --- a/atlas/models.py +++ b/atlas/models.py @@ -110,7 +110,10 @@ class SynMixin(object): # abstract = True def __str__(self) -> str: - if self.primary: + # Use getattr to avoid AttributeError if the model no longer + # defines `primary` (we are migrating some models away from the + # old primary/synonym fields). + if getattr(self, "primary", False): return self.name else: return f"{self.name} [syn]" @@ -122,11 +125,13 @@ class SynMixin(object): except Exception: syns = None - if self.primary: + # Use getattr for safety when models no longer have `primary`. + if getattr(self, "primary", False): if syns is None: - if getattr(self, "synonym", None) is None or self.synonym.count() == 0: + synonyms_field = getattr(self, "synonym", None) + if synonyms_field is None or (hasattr(synonyms_field, "count") and synonyms_field.count() == 0): return self.name - synonyms = ",".join([i.name for i in self.synonym.all()]) + synonyms = ",".join([i.name for i in synonyms_field.all()]) return f"{self.name} ({synonyms})" else: if not syns: @@ -143,39 +148,48 @@ class SynMixin(object): syns = self.get_synonyms() except Exception: syns = None - if getattr(self, "primary", False): return "[Primary]" - else: - if syns is None: - s = self.synonym.filter(primary=True).values_list("name", flat=True) - return ", ".join(s) - else: - # prefer primary names among synonyms if present - primary_names = [i.name for i in syns if getattr(i, "primary", False)] - if primary_names: - return ", ".join(primary_names) - return ", ".join([i.name for i in syns]) + + if syns is None: + synonyms_field = getattr(self, "synonym", None) + if synonyms_field is None: + return "" + # If the old M2M exists, prefer those marked primary if such a + # attribute exists on the related objects. + try: + s = synonyms_field.filter(primary=True).values_list("name", flat=True) + except Exception: + s = synonyms_field.values_list("name", flat=True) + return ", ".join(s) + + # prefer primary names among synonyms if present + primary_names = [i.name for i in syns if getattr(i, "primary", False)] + if primary_names: + return ", ".join(primary_names) + return ", ".join([i.name for i in syns]) def get_synonym_link(self): try: syns = self.get_synonyms() except Exception: syns = None - if getattr(self, "primary", False): return "[Primary]" - else: - if syns is None: - # fall back to all M2M synonyms (not just primary) for models - # that still use the old synonym field (e.g. Finding, Structure) - syns_qs = self.synonym.all() - items = [f"{s.name}" for s in syns_qs] - return ", ".join(items) - else: - # render links for synonyms/canonical group - items = [f"{s.name}" for s in syns] - return ", ".join(items) + + if syns is None: + # fall back to all M2M synonyms (not just primary) for models + # that still use the old synonym field (e.g. Finding, Structure) + synonyms_field = getattr(self, "synonym", None) + if synonyms_field is None: + return "" + syns_qs = synonyms_field.all() + items = [f"{s.name}" for s in syns_qs] + return ", ".join(items) + + # render links for synonyms/canonical group + items = [f"{s.name}" for s in syns] + return ", ".join(items) def get_link(self): return format_html("{}", self.get_absolute_url(), self.name) @@ -183,13 +197,37 @@ class SynMixin(object): class Finding(SynMixin, models.Model): name = models.CharField(max_length=255, unique=True) - synonym = models.ManyToManyField("self", blank=True) - - primary = models.BooleanField(default="True") + # New canonical/alias field: if set, this Finding is an alias and points + # to the canonical/master Finding. We remove the old M2M `synonym` and + # `primary` boolean in favour of this single canonical FK. + canonical = models.ForeignKey( + "self", + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name="aliases", + help_text="If set, this Finding is an alias and points to the canonical Finding.", + ) def get_absolute_url(self): return reverse("atlas:finding_detail", kwargs={"pk": self.pk}) + @property + def canonical_finding(self): + return self.canonical if self.canonical else self + + def get_synonyms(self): + """Return other Findings that are aliases/synonyms for this concept. + + Behaviour mirrors Condition.get_synonyms: if this Finding is an alias + (has canonical set) return the canonical and other aliases (excluding + self). If this Finding is canonical, return all aliases (excluding + self). + """ + master = self.canonical_finding + qs = Finding.objects.filter(models.Q(canonical=master) | models.Q(pk=master.pk)).exclude(pk=self.pk) + return qs + class Condition(SynMixin, models.Model): name = models.CharField(max_length=255, unique=True) @@ -339,14 +377,28 @@ class Differential(models.Model): class Structure(SynMixin, models.Model): name = models.CharField(max_length=255, unique=True) - - synonym = models.ManyToManyField("self", blank=True) - - primary = models.BooleanField(default="True") + # Migrate to canonical/aliases model like Condition and Finding + canonical = models.ForeignKey( + "self", + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name="aliases", + help_text="If set, this Structure is an alias and points to the canonical Structure.", + ) def get_absolute_url(self): return reverse("atlas:structure_detail", kwargs={"pk": self.pk}) + @property + def canonical_structure(self): + return self.canonical if self.canonical else self + + def get_synonyms(self): + master = self.canonical_structure + qs = Structure.objects.filter(models.Q(canonical=master) | models.Q(pk=master.pk)).exclude(pk=self.pk) + return qs + @reversion.register class Case(models.Model, AuthorMixin, QuestionMixin):