From 9328a3858daadf7cff4e1142a2361bf292489d16 Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Mon, 5 Oct 2026 13:12:13 +0200 Subject: [PATCH 1/3] Fix: Do not reparent rank one delete blocker check --- specifyweb/specify/models.py | 46 +++++++++++++------ .../specify/tests/test_delete_blockers.py | 25 ++++++++++ 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/specifyweb/specify/models.py b/specifyweb/specify/models.py index 6ab316a15f8..4733facc068 100644 --- a/specifyweb/specify/models.py +++ b/specifyweb/specify/models.py @@ -1,5 +1,6 @@ from functools import partialmethod from django.db import models +from django.db.models.signals import pre_delete from django.db.models import Q, CheckConstraint from django.utils import timezone from specifyweb.backend.businessrules.exceptions import AbortSave @@ -41,32 +42,43 @@ def delete_taxon_rank_parent_with_context(collector, field, sub_objs, using): """ Use CASCADE while deleting an entire TaxonTreeDef. - For single-rank deletion, reparent child ranks before deleting so - ParentItemID remains valid. + For single-rank deletion, leave children in place during collection and + reparent them when the rank is actually deleted. """ deleting_models = getattr(collector, 'data', {}) is_tree_delete = any( getattr(model, '__name__', '').lower() == 'taxontreedef' for model in deleting_models.keys() ) + ranks_to_delete = _get_collector_model_instances(collector, 'taxontreedefitem') + if is_tree_delete: + for rank in ranks_to_delete: + rank._taxon_rank_delete_from_tree = True return models.CASCADE(collector, field, sub_objs, using) - processed_ids = getattr(collector, '_taxon_rank_delete_prepared_ids', set()) - ranks_to_delete = [ - rank - for rank in _get_collector_model_instances(collector, 'taxontreedefitem') - if rank.id not in processed_ids - ] - _reparent_taxon_rank_children(ranks_to_delete) - - if ranks_to_delete: - collector._taxon_rank_delete_prepared_ids = processed_ids.union( - {rank.id for rank in ranks_to_delete} - ) + deleting_rank_ids = {rank.id for rank in ranks_to_delete} + for rank in ranks_to_delete: + rank._taxon_rank_deleting_ids = deleting_rank_ids return None + +def reparent_taxon_rank_children_before_delete(sender, instance, using, **kwargs): + if getattr(instance, '_taxon_rank_delete_from_tree', False): + return + + deleting_rank_ids = getattr(instance, '_taxon_rank_deleting_ids', {instance.id}) + _reparent_taxon_rank_children([instance], using, deleting_rank_ids) + + +def _reparent_taxon_rank_children(ranks, using, deleting_rank_ids): + for rank in ranks: + Taxontreedefitem.objects.using(using).filter(parent_id=rank.id)\ + .exclude(id__in=deleting_rank_ids)\ + .update(parent_id=rank.parent_id) + + def custom_save(self, *args, **kwargs): try: # Custom save logic here, if necessary @@ -7402,6 +7414,12 @@ class Meta: save = partialmethod(custom_save) +pre_delete.connect( + reparent_taxon_rank_children_before_delete, + sender=Taxontreedefitem, + dispatch_uid='specify.reparent_taxon_rank_children_before_delete', +) + class Treatmentevent(models.Model): specify_model = datamodel.get_table_strict('treatmentevent') diff --git a/specifyweb/specify/tests/test_delete_blockers.py b/specifyweb/specify/tests/test_delete_blockers.py index 75d5a9e348b..96fc20b1d45 100644 --- a/specifyweb/specify/tests/test_delete_blockers.py +++ b/specifyweb/specify/tests/test_delete_blockers.py @@ -70,6 +70,31 @@ def test_children_dont_block_deletion(self): for node in self._node_list: self._assertSame(self._get_blockers(node), []) + def test_taxon_rank_blockers_do_not_reparent_children(self): + tree = models.Taxontreedef.objects.create(name='Delete blocker taxon tree') + root_rank = models.Taxontreedefitem.objects.create( + name='Root', + rankid=0, + treedef=tree, + ) + rank = models.Taxontreedefitem.objects.create( + name='Order', + rankid=100, + parent=root_rank, + treedef=tree, + ) + child_rank = models.Taxontreedefitem.objects.create( + name='Family', + rankid=200, + parent=rank, + treedef=tree, + ) + + self.assertEqual(self._get_blockers(rank), []) + + child_rank.refresh_from_db() + self.assertEqual(child_rank.parent_id, rank.id) + def test_many_to_many_join_blockers_are_normalized(self): export_schema = models.Spexportschema.objects.create( discipline=self.discipline From 6d670811ff5cc661fad0874f198aee970ac397f2 Mon Sep 17 00:00:00 2001 From: "Caroline D." <108160931+CarolineDenis@users.noreply.github.com> Date: Mon, 5 Oct 2026 15:06:49 +0200 Subject: [PATCH 2/3] Update specifyweb/specify/models.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- specifyweb/specify/models.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/specifyweb/specify/models.py b/specifyweb/specify/models.py index 4733facc068..01fc898caf1 100644 --- a/specifyweb/specify/models.py +++ b/specifyweb/specify/models.py @@ -74,9 +74,17 @@ def reparent_taxon_rank_children_before_delete(sender, instance, using, **kwargs def _reparent_taxon_rank_children(ranks, using, deleting_rank_ids): for rank in ranks: + parent_id = rank.parent_id + while parent_id in deleting_rank_ids: + parent_id = ( + Taxontreedefitem.objects.using(using) + .filter(id=parent_id) + .values_list('parent_id', flat=True) + .first() + ) Taxontreedefitem.objects.using(using).filter(parent_id=rank.id)\ .exclude(id__in=deleting_rank_ids)\ - .update(parent_id=rank.parent_id) + .update(parent_id=parent_id) def custom_save(self, *args, **kwargs): From da9c5a71730ea3b240241a02789efa957cd56fa2 Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Mon, 5 Oct 2026 15:24:38 +0200 Subject: [PATCH 3/3] Refactor: Move logic to tree_rules Preserve adjacent-rank reparenting and avoid foreign-key failures during deletion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../backend/businessrules/rules/tree_rules.py | 27 +++++++++++- .../tests/test_taxontreedefitem.py | 32 ++++++++++++++ specifyweb/specify/models.py | 42 ------------------- 3 files changed, 58 insertions(+), 43 deletions(-) diff --git a/specifyweb/backend/businessrules/rules/tree_rules.py b/specifyweb/backend/businessrules/rules/tree_rules.py index d23a1e0d831..b6b09a4e32a 100644 --- a/specifyweb/backend/businessrules/rules/tree_rules.py +++ b/specifyweb/backend/businessrules/rules/tree_rules.py @@ -7,6 +7,32 @@ logger = logging.getLogger(__name__) +@orm_signal_handler( + 'pre_delete', + 'Taxontreedefitem', + dispatch_uid='specify.reparent_taxon_rank_children_before_delete', +) +def reparent_taxon_rank_children_before_delete(rank): + """Reparent surviving children only when the rank is actually deleted.""" + if getattr(rank, '_taxon_rank_delete_from_tree', False): + return + + deleting_rank_ids = getattr(rank, '_taxon_rank_deleting_ids', {rank.id}) + parent_id = rank.parent_id + while parent_id in deleting_rank_ids: + parent_id = ( + rank.__class__.objects.using(rank._state.db) + .filter(id=parent_id) + .values_list('parent_id', flat=True) + .first() + ) + if rank.parent_id in deleting_rank_ids: + rank.__class__.objects.using(rank._state.db).filter(id=rank.id)\ + .update(parent_id=parent_id) + rank.__class__.objects.using(rank._state.db).filter(parent_id=rank.id)\ + .exclude(id__in=deleting_rank_ids)\ + .update(parent_id=parent_id) + # @orm_signal_handler('pre_save') def pre_tree_rank_initiation_handler(sender, obj): if is_treedefitem(obj) and obj.pk is None: # is it a treedefitem? @@ -45,4 +71,3 @@ def post_tree_rank_deletion_handler(sender, obj): def set_is_accepted_if_preferred(sender, obj): if hasattr(obj, 'isaccepted') and hasattr(obj, 'accepted_id') : obj.isaccepted = obj.accepted_id == None - diff --git a/specifyweb/backend/businessrules/tests/test_taxontreedefitem.py b/specifyweb/backend/businessrules/tests/test_taxontreedefitem.py index 60e1c7552bc..941e387ef14 100644 --- a/specifyweb/backend/businessrules/tests/test_taxontreedefitem.py +++ b/specifyweb/backend/businessrules/tests/test_taxontreedefitem.py @@ -60,3 +60,35 @@ def test_full_tree_delete_still_cascades(self): self.taxontreedef.delete() self.assertFalse(models.Taxontreedef.objects.filter(id=self.taxontreedef.id).exists()) + + def test_instance_delete_unused_rank_reparents_children(self): + kingdom = self.roottaxontreedefitem.children.create( + name="Kingdom", + treedef=self.taxontreedef, + rankid=100) + phylum = kingdom.children.create( + name="Phylum", + treedef=self.taxontreedef, + rankid=200) + kingdom_id = kingdom.id + + kingdom.delete() + + phylum.refresh_from_db() + self.assertEqual(phylum.parent_id, self.roottaxontreedefitem.id) + self.assertFalse(models.Taxontreedefitem.objects.filter(id=kingdom_id).exists()) + + def test_delete_adjacent_ranks_reparents_to_surviving_ancestor(self): + kingdom = self.roottaxontreedefitem.children.create( + name="Kingdom", treedef=self.taxontreedef, rankid=100) + phylum = kingdom.children.create( + name="Phylum", treedef=self.taxontreedef, rankid=200) + class_rank = phylum.children.create( + name="Class", treedef=self.taxontreedef, rankid=300) + deleting_ids = [kingdom.id, phylum.id] + + models.Taxontreedefitem.objects.filter(id__in=deleting_ids).delete() + + class_rank.refresh_from_db() + self.assertEqual(class_rank.parent_id, self.roottaxontreedefitem.id) + self.assertFalse(models.Taxontreedefitem.objects.filter(id__in=deleting_ids).exists()) diff --git a/specifyweb/specify/models.py b/specifyweb/specify/models.py index 01fc898caf1..0c791d2f070 100644 --- a/specifyweb/specify/models.py +++ b/specifyweb/specify/models.py @@ -1,6 +1,5 @@ from functools import partialmethod from django.db import models -from django.db.models.signals import pre_delete from django.db.models import Q, CheckConstraint from django.utils import timezone from specifyweb.backend.businessrules.exceptions import AbortSave @@ -26,18 +25,6 @@ def _get_collector_model_instances(collector, model_name): return [] -def _reparent_taxon_rank_children(ranks): - rank_list = list(ranks) - if not rank_list: - return - - deleting_rank_ids = {rank.id for rank in rank_list} - for rank in rank_list: - Taxontreedefitem.objects.filter(parent_id=rank.id)\ - .exclude(id__in=deleting_rank_ids)\ - .update(parent_id=rank.parent_id) - - def delete_taxon_rank_parent_with_context(collector, field, sub_objs, using): """ Use CASCADE while deleting an entire TaxonTreeDef. @@ -64,29 +51,6 @@ def delete_taxon_rank_parent_with_context(collector, field, sub_objs, using): return None -def reparent_taxon_rank_children_before_delete(sender, instance, using, **kwargs): - if getattr(instance, '_taxon_rank_delete_from_tree', False): - return - - deleting_rank_ids = getattr(instance, '_taxon_rank_deleting_ids', {instance.id}) - _reparent_taxon_rank_children([instance], using, deleting_rank_ids) - - -def _reparent_taxon_rank_children(ranks, using, deleting_rank_ids): - for rank in ranks: - parent_id = rank.parent_id - while parent_id in deleting_rank_ids: - parent_id = ( - Taxontreedefitem.objects.using(using) - .filter(id=parent_id) - .values_list('parent_id', flat=True) - .first() - ) - Taxontreedefitem.objects.using(using).filter(parent_id=rank.id)\ - .exclude(id__in=deleting_rank_ids)\ - .update(parent_id=parent_id) - - def custom_save(self, *args, **kwargs): try: # Custom save logic here, if necessary @@ -7422,12 +7386,6 @@ class Meta: save = partialmethod(custom_save) -pre_delete.connect( - reparent_taxon_rank_children_before_delete, - sender=Taxontreedefitem, - dispatch_uid='specify.reparent_taxon_rank_children_before_delete', -) - class Treatmentevent(models.Model): specify_model = datamodel.get_table_strict('treatmentevent')