From 0fb087eebfc5cb24efae116bf6f5f32845e43c0f Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 20 Jun 2024 18:22:12 -0500 Subject: [PATCH 1/4] Restore isaccepted business rule Fixes #5027 Caused by #4257 --- specifyweb/businessrules/rules/tree_rules.py | 4 ++-- specifyweb/specify/tree_extras.py | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/specifyweb/businessrules/rules/tree_rules.py b/specifyweb/businessrules/rules/tree_rules.py index 7723c578ca9..64382f39a0b 100644 --- a/specifyweb/businessrules/rules/tree_rules.py +++ b/specifyweb/businessrules/rules/tree_rules.py @@ -40,8 +40,8 @@ def post_tree_rank_deletion_handler(sender, obj): if is_instance_of_tree_def_item(obj): # is it a treedefitem? post_tree_rank_deletion(obj) -# @orm_signal_handler('pre_save') -def set_is_accepted_if_prefereed(sender, obj): +@orm_signal_handler('pre_save') +def set_is_accepted_if_preferred(sender, obj): if hasattr(obj, 'isaccepted'): obj.isaccepted = obj.accepted_id == None diff --git a/specifyweb/specify/tree_extras.py b/specifyweb/specify/tree_extras.py index fe2a232b0cb..549da10c9a7 100644 --- a/specifyweb/specify/tree_extras.py +++ b/specifyweb/specify/tree_extras.py @@ -701,8 +701,6 @@ class Meta: def save(self, *args, **kwargs): # pre_save - if hasattr(self, 'isaccepted'): - self.isaccepted = self.accepted_id == None if self.pk is None: # is it a new object? pre_tree_rank_init(self) verify_rank_parent_chain_integrity(self, RankOperation.CREATED) From 707da82be0636d41ae96ab9babb162e15020c93a Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 20 Jun 2024 19:36:19 -0500 Subject: [PATCH 2/4] Write tests for business rule --- specifyweb/businessrules/tests/geography.py | 32 ++++++++- specifyweb/businessrules/tests/taxon.py | 29 +++++++- specifyweb/specify/tree_extras.py | 74 ++++++++++----------- specifyweb/specify/tree_ranks.py | 2 +- 4 files changed, 95 insertions(+), 42 deletions(-) diff --git a/specifyweb/businessrules/tests/geography.py b/specifyweb/businessrules/tests/geography.py index 7c5b9da7eb0..cb82b875931 100644 --- a/specifyweb/businessrules/tests/geography.py +++ b/specifyweb/businessrules/tests/geography.py @@ -5,6 +5,7 @@ from specifyweb.specify import models from specifyweb.specify.tests.test_api import ApiTests + class GeographyTests(ApiTests): def test_delete_blocked_by_locality(self): geography = models.Geography.objects.create( @@ -39,6 +40,31 @@ def test_delete_blocked_by_agentgeography(self): models.Agentgeography.objects.filter(geography=geography).delete() geography.delete() + def test_isaccepted_on_save(self): + earth = models.Geography.objects.create( + name="Earth", + definition=self.geographytreedef, + definitionitem=self.geographytreedef.treedefitems.all()[0]) + + continent = earth.definitionitem.children.create( + name="Continent", + treedef=earth.definition, + rankid=earth.definitionitem.rankid+100) + + na = earth.children.create( + name="North America", + definition=earth.definition, + definitionitem=continent) + + other_na = earth.children.create( + name="Americas", + acceptedgeography=na, + definition=earth.definition, + definitionitem=continent) + + self.assertTrue(na.isaccepted) + self.assertFalse(other_na.isaccepted) + @skip("this behavior was eliminated by https://github.com/specify/specify7/issues/136") def test_delete_cascades_to_deletable_children(self): earth = models.Geography.objects.create( @@ -73,7 +99,8 @@ def test_delete_cascades_to_deletable_children(self): earth.delete() - self.assertEqual(models.Geography.objects.filter(id__in=(na.id, sa.id)).count(), 0) + self.assertEqual(models.Geography.objects.filter( + id__in=(na.id, sa.id)).count(), 0) @skip("not clear if this is correct.") def test_accepted_children_acceptedparent_set_to_null_on_delete(self): @@ -109,4 +136,5 @@ def test_accepted_children_acceptedparent_set_to_null_on_delete(self): definitionitem=country) asia.delete() - self.assertEqual(models.Geography.objects.get(id=lugash.id).acceptedgeography, None) + self.assertEqual(models.Geography.objects.get( + id=lugash.id).acceptedgeography, None) diff --git a/specifyweb/businessrules/tests/taxon.py b/specifyweb/businessrules/tests/taxon.py index 019bbd323df..4b369e2782b 100644 --- a/specifyweb/businessrules/tests/taxon.py +++ b/specifyweb/businessrules/tests/taxon.py @@ -6,6 +6,7 @@ from specifyweb.specify import models from specifyweb.specify.tests.test_api import ApiTests + class TaxonTests(ApiTests): def setUp(self): super(TaxonTests, self).setUp() @@ -39,7 +40,8 @@ def test_delete_cascades_to_commonnames(self): name="test") self.roottaxon.delete() - self.assertEqual(models.Commonnametx.objects.filter(id=commonname.id).count(), 0) + self.assertEqual(models.Commonnametx.objects.filter( + id=commonname.id).count(), 0) def test_delete_blocked_by_determinations(self): det = self.collectionobjects[0].determinations.create( @@ -95,6 +97,28 @@ def test_delete_blocked_by_hybridchildren(self): tax2.delete() self.roottaxon.delete() + def test_isaccepted_on_save(self): + kingdom = self.roottaxontreedefitem.children.create( + name="Kingdom", + treedef=self.taxontreedef, + rankid=self.roottaxontreedefitem.rankid+100) + + animalia = self.roottaxon.children.create( + name="Animalia", + definition=self.taxontreedef, + definitionitem=kingdom + ) + + metazoa = self.roottaxon.children.create( + name="Metazoa", + acceptedtaxon=animalia, + definition=self.taxontreedef, + definitionitem=kingdom + ) + + self.assertTrue(animalia.isaccepted) + self.assertFalse(metazoa.isaccepted) + @skip("not sure if rule is valid") def test_delete_blocked_by_taxoncitations(self): rw = models.Referencework.objects.create( @@ -139,7 +163,8 @@ def test_delete_cascades_to_deletable_children(self): det.delete() self.roottaxon.delete() - self.assertEqual(models.Taxon.objects.filter(id__in=(animal.id, plant.id)).count(), 0) + self.assertEqual(models.Taxon.objects.filter( + id__in=(animal.id, plant.id)).count(), 0) @skip("not clear if this is correct.") def test_accepted_children_acceptedparent_set_to_null_on_delete(self): diff --git a/specifyweb/specify/tree_extras.py b/specifyweb/specify/tree_extras.py index 549da10c9a7..b6d66bbb420 100644 --- a/specifyweb/specify/tree_extras.py +++ b/specifyweb/specify/tree_extras.py @@ -128,6 +128,43 @@ def accepted_id(self): def accepted_id(self, value): setattr(self, self.accepted_id_attr(), value) +class TreeRank(models.Model): + class Meta: + abstract = True + + def save(self, *args, **kwargs): + # pre_save + if self.pk is None: # is it a new object? + pre_tree_rank_init(self) + verify_rank_parent_chain_integrity(self, RankOperation.CREATED) + else: + verify_rank_parent_chain_integrity(self, RankOperation.UPDATED) + + # save + super(TreeRank, self).save(*args, **kwargs) + + # post_save + post_tree_rank_save(self.__class__, self) + + def delete(self, *args, **kwargs): + # pre_delete + if self.__class__.objects.get(id=self.id).parent is None: + raise TreeBusinessRuleException( + "cannot delete root level tree definition item", + {"tree": self.__class__.__name__, + "localizationKey": 'deletingTreeRoot', + "node": { + "id": self.id + }}) + pre_tree_rank_deletion(self.__class__, self) + verify_rank_parent_chain_integrity(self, RankOperation.DELETED) + + # delete + super(TreeRank, self).delete(*args, **kwargs) + + # post_delete + post_tree_rank_deletion(self) + def open_interval(model, parent_node_number, size): @@ -694,40 +731,3 @@ def is_instance_of_tree_def_item(obj): spmodels.Taxontreedefitem, ] return any(isinstance(obj, cls) for cls in tree_def_item_classes) - -class TreeRank(models.Model): - class Meta: - abstract = True - - def save(self, *args, **kwargs): - # pre_save - if self.pk is None: # is it a new object? - pre_tree_rank_init(self) - verify_rank_parent_chain_integrity(self, RankOperation.CREATED) - else: - verify_rank_parent_chain_integrity(self, RankOperation.UPDATED) - - # save - super(TreeRank, self).save(*args, **kwargs) - - # post_save - post_tree_rank_save(self.__class__, self) - - def delete(self, *args, **kwargs): - # pre_delete - if self.__class__.objects.get(id=self.id).parent is None: - raise TreeBusinessRuleException( - "cannot delete root level tree definition item", - {"tree": self.__class__.__name__, - "localizationKey": 'deletingTreeRoot', - "node": { - "id": self.id - }}) - pre_tree_rank_deletion(self.__class__, self) - verify_rank_parent_chain_integrity(self, RankOperation.DELETED) - - # delete - super(TreeRank, self).delete(*args, **kwargs) - - # post_delete - post_tree_rank_deletion(self) diff --git a/specifyweb/specify/tree_ranks.py b/specifyweb/specify/tree_ranks.py index 47f9dd30781..d8910577cac 100644 --- a/specifyweb/specify/tree_ranks.py +++ b/specifyweb/specify/tree_ranks.py @@ -175,7 +175,7 @@ def set_rank_id(new_rank): # Get tree def item model tree_def_item_model_name = (tree + 'treedefitem').lower().title() - tree_def_item_model = getattr(spmodels, tree_def_item_model_name.lower().title()) + tree_def_item_model = getattr(spmodels, tree_def_item_model_name) # Handle case where the parent rank is not given, and it is not the first rank added. # This is happening in the UI workflow of Treeview->Treedef->Treedefitems->Add From 6e9104c7cd04ca5b6ec36c394116ec34aacb8338 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 24 Jun 2024 08:12:50 -0500 Subject: [PATCH 3/4] Remove rankid declarations from tests --- specifyweb/businessrules/tests/geography.py | 3 +-- specifyweb/businessrules/tests/taxon.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/specifyweb/businessrules/tests/geography.py b/specifyweb/businessrules/tests/geography.py index cb82b875931..0a280415ca4 100644 --- a/specifyweb/businessrules/tests/geography.py +++ b/specifyweb/businessrules/tests/geography.py @@ -48,8 +48,7 @@ def test_isaccepted_on_save(self): continent = earth.definitionitem.children.create( name="Continent", - treedef=earth.definition, - rankid=earth.definitionitem.rankid+100) + treedef=earth.definition) na = earth.children.create( name="North America", diff --git a/specifyweb/businessrules/tests/taxon.py b/specifyweb/businessrules/tests/taxon.py index 4b369e2782b..9435dc0543c 100644 --- a/specifyweb/businessrules/tests/taxon.py +++ b/specifyweb/businessrules/tests/taxon.py @@ -100,8 +100,7 @@ def test_delete_blocked_by_hybridchildren(self): def test_isaccepted_on_save(self): kingdom = self.roottaxontreedefitem.children.create( name="Kingdom", - treedef=self.taxontreedef, - rankid=self.roottaxontreedefitem.rankid+100) + treedef=self.taxontreedef) animalia = self.roottaxon.children.create( name="Animalia", From fc9774b23a8ba04800506ddb455f7ec003874c20 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 24 Jun 2024 11:50:04 -0500 Subject: [PATCH 4/4] Rename root taxontreedef in test --- specifyweb/businessrules/tests/taxon.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specifyweb/businessrules/tests/taxon.py b/specifyweb/businessrules/tests/taxon.py index 9435dc0543c..a3f77384fdd 100644 --- a/specifyweb/businessrules/tests/taxon.py +++ b/specifyweb/businessrules/tests/taxon.py @@ -15,7 +15,7 @@ def setUp(self): name="Test Taxon tree def") self.roottaxontreedefitem = self.taxontreedef.treedefitems.create( - name="root", + name="taxonomy_root", rankid=0) self.roottaxon = self.roottaxontreedefitem.treeentries.create(