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 6ab316a15f8..0c791d2f070 100644 --- a/specifyweb/specify/models.py +++ b/specifyweb/specify/models.py @@ -25,48 +25,32 @@ 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. - 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 custom_save(self, *args, **kwargs): try: # Custom save logic here, if necessary 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