diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index ce4f4c64b5c..74861fb6b9c 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -15,6 +15,7 @@ from django import forms from django.db import transaction +from django.db.models import F from django.apps import apps from django.http import (HttpResponse, HttpResponseBadRequest, Http404, HttpResponseNotAllowed, QueryDict) @@ -51,7 +52,7 @@ def strict_get_model(name: str): if model._meta.model_name == name: return model raise e - + def get_model(name: str): try: return strict_get_model(name) @@ -949,6 +950,15 @@ def apply_filters(logged_in_collection, params, model, control_params=GetCollect val = val.split(',') filters.update({param: val}) + + if model.__name__ == 'Geologictimeperiod': + # Filter out invalid chronostrats + filters.update({ + 'startperiod__isnull': False, + 'endperiod__isnull': False, + 'startperiod__gte': F('endperiod') + }) + try: objs = model.objects.filter(**filters) except (ValueError, FieldError) as e: diff --git a/specifyweb/specify/geo_time.py b/specifyweb/specify/geo_time.py index 4434f9c8c08..c9831ef886e 100644 --- a/specifyweb/specify/geo_time.py +++ b/specifyweb/specify/geo_time.py @@ -15,12 +15,31 @@ ) from specifyweb.stored_queries import models as sq_models -# Paths from CollectionObject to Absoluteage or GeologicTimePeriod: +# Table paths from CollectionObject to Absoluteage or GeologicTimePeriod: +# - collectionobject->absoluteage +# - collectionobject->relativeage->chronostrat # - collectionobject->paleocontext->chronostrat # - collectionobject->collectionevent->paleocontext->chronostrat # - collectionobject->collectionevent->locality->paleocontext->chronostrat -# - collectionobject->relativeage->chronostrat -# - collectionobject->absoluteage + +# Field Paths from CollectionObject to Absoluteage or GeologicTimePeriod: +# - collectionobject__absoluteage__absoluteage +# - collectionobject__relativeage__agename__startperiod +# - collectionobject__relativeage__agename__endperiod +# - collectionobject__relativeage__agenameend__startperiod +# - collectionobject__relativeage__agenameend__endperiod +# - collectionobject__paleocontext__chronosstrat__startperiod +# - collectionobject__paleocontext__chronosstrat__endperiod +# - collectionobject__paleocontext__chronosstratend__startperiod +# - collectionobject__paleocontext__chronosstratend__endperiod +# - collectionobject__collectingevent__paleocontext__chronosstrat__startperiod +# - collectionobject__collectingevent__paleocontext__chronosstrat__endperiod +# - collectionobject__collectingevent__paleocontext__chronosstratend__startperiod +# - collectionobject__collectingevent__paleocontext__chronosstratend__endperiod +# - collectionobject__collectingevent__locality__paleocontext__chronosstrat__startperiod +# - collectionobject__collectingevent__locality__paleocontext__chronosstrat__endperiod +# - collectionobject__collectingevent__locality__paleocontext__chronosstratend__startperiod +# - collectionobject__collectingevent__locality__paleocontext__chronosstratend__endperiod def assert_valid_time_range(start_time: float, end_time: float): """ @@ -29,7 +48,6 @@ def assert_valid_time_range(start_time: float, end_time: float): """ assert start_time >= end_time, "Start time must be greater than or equal to end time." - def search_co_ids_in_time_range( start_time: float, end_time: float, require_full_overlap: bool = False ) -> Set[int]: @@ -71,6 +89,35 @@ def get_overlap_filter(require_full_overlap, start_time, end_time): else: return Q(co_end_time_low__lte=start_time, co_start_time_high__gte=end_time) + def get_valid_chronostrat_filter(field_name): + """ + Generate a filter to exclude records with invalid Chronostrat records, where startperiod < endperiod. + + :param field_name: The base name of the field to filter on. + :return: A Q object representing the filter. + """ + start_field = f'{field_name}__startperiod' + end_field = f'{field_name}__endperiod' + end_start_field = f'{field_name}end__startperiod' + end_end_field = f'{field_name}end__endperiod' + + start_period_isnull = f'{field_name}__startperiod__isnull' + end_period_isnull = f'{field_name}__endperiod__isnull' + end_isnull_field = f'{field_name}end__isnull' + end_start_period_isnull = f'{field_name}end__startperiod__isnull' + end_end_period_isnull = f'{field_name}end__endperiod__isnull' + + return Q(**{start_period_isnull: False}) \ + & Q(**{end_period_isnull: False}) \ + & Q(**{f'{start_field}__gte': F(end_field)}) \ + & ( + (Q(**{end_isnull_field: True}) | Q(**{f'{end_start_field}__gte': F(end_end_field)})) | + (Q(**{end_start_period_isnull: False}) & Q(**{end_end_period_isnull: False})) + ) + + valid_relative_age_chronostrat_filter = get_valid_chronostrat_filter('agename') + valid_paleocontext_chronostrat_filter = get_valid_chronostrat_filter('chronosstrat') + # Adjusted absolute ages absolute_ages = get_annotated_ages(Absoluteage, 'absoluteage', 'absoluteage', require_full_overlap) absolute_overlap_filter = get_overlap_filter(require_full_overlap, start_time, end_time) @@ -81,7 +128,9 @@ def get_overlap_filter(require_full_overlap, start_time, end_time): # Adjusted relative ages if require_full_overlap: - relative_ages = Relativeage.objects.annotate( + relative_ages = Relativeage.objects.filter( + valid_relative_age_chronostrat_filter + ).annotate( co_start_time_low=Cast(F("agename__startperiod"), FloatField()) - get_uncertainty_value("agename__startuncertainty") - get_uncertainty_value("ageuncertainty"), @@ -110,7 +159,9 @@ def get_overlap_filter(require_full_overlap, start_time, end_time): ) ) else: - relative_ages = Relativeage.objects.annotate( + relative_ages = Relativeage.objects.filter( + valid_relative_age_chronostrat_filter + ).annotate( co_start_time_high=Cast(F("agename__startperiod"), FloatField()) + get_uncertainty_value("agename__startuncertainty") + get_uncertainty_value("ageuncertainty"), @@ -146,7 +197,9 @@ def get_overlap_filter(require_full_overlap, start_time, end_time): ) if require_full_overlap: - paleocontexts = Paleocontext.objects.annotate( + paleocontexts = Paleocontext.objects.filter( + valid_paleocontext_chronostrat_filter + ).annotate( co_start_time_low=Cast(F("chronosstrat__startperiod"), FloatField()) - get_uncertainty_value("chronosstrat__startuncertainty"), co_end_time_high=Cast(F("chronosstrat__endperiod"), FloatField()) @@ -171,7 +224,9 @@ def get_overlap_filter(require_full_overlap, start_time, end_time): ) ) else: - paleocontexts = Paleocontext.objects.annotate( + paleocontexts = Paleocontext.objects.filter( + valid_paleocontext_chronostrat_filter + ).annotate( co_start_time_high=Cast(F("chronosstrat__startperiod"), FloatField()) + get_uncertainty_value("chronosstrat__startuncertainty"), co_end_time_low=Cast(F("chronosstrat__endperiod"), FloatField()) @@ -254,6 +309,10 @@ def search_co_ids_in_time_period( return set() start_time = time_period.startperiod end_time = time_period.endperiod + if start_time is None: + start_time = 13800 + if end_time is None: + end_time = 0 return search_co_ids_in_time_range(start_time, end_time, require_full_overlap) def query_co_in_time_range_with_joins( diff --git a/specifyweb/specify/tests/test_geotime.py b/specifyweb/specify/tests/test_geotime.py index 3d7905173c5..36f73b57340 100644 --- a/specifyweb/specify/tests/test_geotime.py +++ b/specifyweb/specify/tests/test_geotime.py @@ -22,32 +22,32 @@ class GeoTimeTests(ApiTests): def setUp(self): super().setUp() - root_rank, _ = Geologictimeperiodtreedefitem.objects.get_or_create( + self.root_rank, _ = Geologictimeperiodtreedefitem.objects.get_or_create( name='Root', rankid=0, treedef=self.geologictimeperiodtreedef, ) - erathem_rank, _ = Geologictimeperiodtreedefitem.objects.get_or_create( + self.erathem_rank, _ = Geologictimeperiodtreedefitem.objects.get_or_create( name='Erathem', - parent=root_rank, + parent=self.root_rank, rankid=100, treedef=self.geologictimeperiodtreedef, ) - period_rank, _ = Geologictimeperiodtreedefitem.objects.get_or_create( + self.period_rank, _ = Geologictimeperiodtreedefitem.objects.get_or_create( name='Period', - parent=erathem_rank, + parent=self.erathem_rank, rankid=200, treedef=self.geologictimeperiodtreedef, ) - epoch_rank, _ = Geologictimeperiodtreedefitem.objects.get_or_create( + self.epoch_rank, _ = Geologictimeperiodtreedefitem.objects.get_or_create( name='Series/Epoch', - parent=period_rank, + parent=self.period_rank, rankid=300, treedef=self.geologictimeperiodtreedef, ) - stage_rank, _ = Geologictimeperiodtreedefitem.objects.get_or_create( + self.stage_rank, _ = Geologictimeperiodtreedefitem.objects.get_or_create( name='Stage/Age', - parent=epoch_rank, + parent=self.epoch_rank, rankid=400, treedef=self.geologictimeperiodtreedef, ) @@ -55,7 +55,7 @@ def setUp(self): root_chronostrat = Geologictimeperiod.objects.create( name='Root', rankid=0, - definitionitem=root_rank, + definitionitem=self.root_rank, definition=self.geologictimeperiodtreedef, startperiod=100000, endperiod=0, @@ -63,7 +63,7 @@ def setUp(self): cenozoic_erathem_chronostrat = Geologictimeperiod.objects.create( name='Cenozoic', rankid=100, - definitionitem=erathem_rank, + definitionitem=self.erathem_rank, definition=self.geologictimeperiodtreedef, startperiod=None, startuncertainty=None, @@ -73,7 +73,7 @@ def setUp(self): paelozoic_erathem_chronostrat = Geologictimeperiod.objects.create( name='Paleozoic', rankid=100, - definitionitem=erathem_rank, + definitionitem=self.erathem_rank, definition=self.geologictimeperiodtreedef, startperiod=570, startuncertainty=None, @@ -83,7 +83,7 @@ def setUp(self): null_erathem_chronostrat = Geologictimeperiod.objects.create( name='Null', rankid=100, - definitionitem=erathem_rank, + definitionitem=self.erathem_rank, definition=self.geologictimeperiodtreedef, startperiod=None, startuncertainty=None, @@ -93,7 +93,7 @@ def setUp(self): paleogene_period_chronostrat = Geologictimeperiod.objects.create( name='Paleogene', rankid=200, - definitionitem=period_rank, + definitionitem=self.period_rank, definition=self.geologictimeperiodtreedef, startperiod=66, startuncertainty=None, @@ -103,7 +103,7 @@ def setUp(self): devonian_period_chronostrat = Geologictimeperiod.objects.create( name='Devonian', rankid=200, - definitionitem=period_rank, + definitionitem=self.period_rank, definition=self.geologictimeperiodtreedef, startperiod=419.2, startuncertainty=None, @@ -113,7 +113,7 @@ def setUp(self): jurassic_period_chronostrat = Geologictimeperiod.objects.create( name='Jurassic', rankid=200, - definitionitem=period_rank, + definitionitem=self.period_rank, definition=self.geologictimeperiodtreedef, startperiod=208, startuncertainty=18, @@ -123,7 +123,7 @@ def setUp(self): paleocene_epoch_chronostrat = Geologictimeperiod.objects.create( name='Paleocene', rankid=300, - definitionitem=epoch_rank, + definitionitem=self.epoch_rank, definition=self.geologictimeperiodtreedef, startperiod=66, startuncertainty=None, @@ -133,7 +133,7 @@ def setUp(self): eocene_epoch_chronostrat = Geologictimeperiod.objects.create( name='Eocene', rankid=300, - definitionitem=epoch_rank, + definitionitem=self.epoch_rank, definition=self.geologictimeperiodtreedef, startperiod=56, startuncertainty=None, @@ -143,7 +143,7 @@ def setUp(self): test_epoch_chronostrat = Geologictimeperiod.objects.create( name='Test Epoch', rankid=300, - definitionitem=epoch_rank, + definitionitem=self.epoch_rank, definition=self.geologictimeperiodtreedef, startperiod=100, startuncertainty=11, @@ -153,7 +153,7 @@ def setUp(self): late_jurassic_epoch_chronostrat = Geologictimeperiod.objects.create( name='Late Jurassic', rankid=300, - definitionitem=epoch_rank, + definitionitem=self.epoch_rank, definition=self.geologictimeperiodtreedef, startperiod=163, startuncertainty=15, @@ -163,7 +163,7 @@ def setUp(self): selandian_stage_chronostrat = Geologictimeperiod.objects.create( name='Selandian', rankid=400, - definitionitem=stage_rank, + definitionitem=self.stage_rank, definition=self.geologictimeperiodtreedef, startperiod=61.6, startuncertainty=None, @@ -173,7 +173,7 @@ def setUp(self): franconian_stage_chronostrat = Geologictimeperiod.objects.create( name='Franconian', rankid=400, - definitionitem=stage_rank, + definitionitem=self.stage_rank, definition=self.geologictimeperiodtreedef, startperiod=523, startuncertainty=36, @@ -183,7 +183,7 @@ def setUp(self): oxfordian_stage_chronostrat = Geologictimeperiod.objects.create( name='Oxfordian', rankid=400, - definitionitem=stage_rank, + definitionitem=self.stage_rank, definition=self.geologictimeperiodtreedef, startperiod=163, startuncertainty=15, @@ -423,6 +423,30 @@ def test_geotime_simple(self): collecting_event_2 = Collectingevent.objects.create(locality=locality_1, discipline=self.discipline) co_6 = Collectionobject.objects.create(collection=self.collection, collectingevent=collecting_event_2) + def test_invalid_chronostrat(self): + bad_chronostrat = Geologictimeperiod.objects.create( + name='BadBoyz', + rankid=100, + definitionitem=self.erathem_rank, + definition=self.geologictimeperiodtreedef, + startperiod=10, + endperiod=90, + parent=self.geo_time_period_dict['root'] + ) + co_1 = Collectionobject.objects.create(collection=self.collection) + relative_age = Relativeage.objects.create( + agename=bad_chronostrat, + collectionobject=co_1 + ) + + self.assertFalse(co_1.id in geo_time.search_co_ids_in_time_range(200, 10)) + + bad_chronostrat.startperiod = 100 + bad_chronostrat.name = 'GoodBoyz' # important, don't change this :) + bad_chronostrat.save() + + self.assertTrue(co_1.id in geo_time.search_co_ids_in_time_range(200, 10)) + @skip('Fix API test call') def test_geotime_any(self): c = Client() diff --git a/specifyweb/specify/tree_views.py b/specifyweb/specify/tree_views.py index 08117b66c49..2ede1d53787 100644 --- a/specifyweb/specify/tree_views.py +++ b/specifyweb/specify/tree_views.py @@ -2,6 +2,7 @@ from django import http from typing import Literal, Tuple from django.db import connection, transaction +from django.db.models import F, Q from django.http import HttpResponse from django.views.decorators.http import require_POST from sqlalchemy import sql, distinct @@ -15,7 +16,7 @@ from specifyweb.stored_queries.execution import set_group_concat_max_len from specifyweb.stored_queries.group_concat import group_concat from specifyweb.specify.tree_utils import get_search_filters -from specifyweb.specify import models +from specifyweb.specify import models as spmodels from specifyweb.specify.tree_ranks import tree_rank_count from . import tree_extras from .api import get_object_or_404, obj_to_data, toJson @@ -160,7 +161,7 @@ def tree_view(request, treedef, tree: TREE_TABLE, parentid, sortfield): def get_tree_rows(treedef, tree, parentid, sortfield, include_author, session): - tree_table = models.datamodel.get_table(tree) + tree_table = spmodels.datamodel.get_table(tree) parentid = None if parentid == 'null' else int(parentid) node = getattr(sqlmodels, tree_table.name) @@ -391,7 +392,7 @@ def repair_tree(request, tree: TREE_TABLE): check_permission_targets(request.specify_collection.id, request.specify_user.id, [perm_target(tree).repair]) - tree_model = models.datamodel.get_table(tree) + tree_model = spmodels.datamodel.get_table(tree) table = tree_model.name.lower() tree_extras.renumber_tree(table) tree_extras.validate_tree_numbering(table) @@ -402,8 +403,8 @@ def add_root(request, tree, treeid): tree_name = tree.title() tree_target = get_object_or_404(f"{tree_name}treedef", id=treeid) - tree_def_item_model = getattr(models, f"{tree_name}treedefitem") - item = getattr(models, tree_name) + tree_def_item_model = getattr(spmodels, f"{tree_name}treedefitem") + item = getattr(spmodels, tree_name) tree_def_item, create = tree_def_item_model.objects.get_or_create( treedef=tree_target, @@ -505,7 +506,7 @@ def has_tree_read_permission(tree: TREE_TABLE) -> bool: for tree in accessible_trees: result[tree] = [] - treedef_model = getattr(models, f'{tree.lower().capitalize()}treedef') + treedef_model = getattr(spmodels, f'{tree.lower().capitalize()}treedef') tree_defs = treedef_model.objects.filter(get_search_filters(request.specify_collection, tree)).distinct() for definition in tree_defs: ranks = definition.treedefitems.order_by('rankid') @@ -516,7 +517,6 @@ def has_tree_read_permission(tree: TREE_TABLE) -> bool: return HttpResponse(toJson(result), content_type='application/json') - class TaxonMutationPT(PermissionTarget): resource = "/tree/edit/taxon" merge = PermissionTargetAction()