Add valid chronostrat filter to geo_time relativeage and paleocontext queries

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:

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(

specifyweb/specify/tests/test_geotime.py

@@ -22,48 +22,48 @@ 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,
         )
 
         root_chronostrat = Geologictimeperiod.objects.create(
             name='Root',
             rankid=0,
-            definitionitem=root_rank,
+            definitionitem=self.root_rank,
             definition=self.geologictimeperiodtreedef,
             startperiod=100000,
             endperiod=0,
         )
         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()