Refactor Finding and Structure models: remove legacy primary and synonym fields, introduce canonical field for aliasing. Update filters and forms accordingly.

2025-11-24 10:18:26 +00:00
parent 8a871d6332
commit 6308fbfcd7
4 changed files with 169 additions and 40 deletions
@@ -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:
@@ -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"
            ),
        }
@@ -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'),
+        ),
+    ]
@@ -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"<a href='{s.get_absolute_url()}'>{s.name}</a>" for s in syns_qs]
-                return ", ".join(items)
-            else:
-                # render links for synonyms/canonical group
-                items = [f"<a href='{s.get_absolute_url()}'>{s.name}</a>" 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"<a href='{s.get_absolute_url()}'>{s.name}</a>" for s in syns_qs]
+            return ", ".join(items)
+
+        # render links for synonyms/canonical group
+        items = [f"<a href='{s.get_absolute_url()}'>{s.name}</a>" for s in syns]
+        return ", ".join(items)

    def get_link(self):
        return format_html("<a href='{}'>{}</a>", 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):