add RiskOfBiasScoreOverrideObject to admin (#374)

- gracefully handle case where the RiskOfBiasScoreOverrideObject.content_object is deleted in serializations
    - URL redirects to /404/
    - text indicates the object is deleted
- update admin for review of RiskOfBiasScoreOverrideObject
- update docs for how to setup celery tasks
- add method for finding orphaned RiskOfBiasScoreOverrideObject and optionally deleting them
- added celery task to check periodically and Log findings (may automatically delete at some point)

docs/source/development.rst

@@ -394,19 +394,28 @@ To test asynchronous functionality in development, modify your ``hawc/main/setti
 
 .. code-block:: python
 
-    CELERY_BROKER_URL = "redis://localhost:6379/1"
-    CELERY_RESULT_BACKEND = "redis://localhost:6379/2"
+    CELERY_BROKER_URL = "redis://:default-password@localhost:6379/1"
+    CELERY_RESULT_BACKEND = "redis://:default-password@localhost:6379/2"
     CELERY_TASK_ALWAYS_EAGER = False
     CELERY_TASK_EAGER_PROPAGATES = False
 
 Then, create the example docker container and start a celery worker instance:
 
 .. code-block:: bash
 
+    # build container
     docker-compose build redis
     docker-compose up -d redis
-    celery worker --app=hawc.main.celery --loglevel=INFO --logfile=celery-worker.log --soft-time-limit=90 --time-limit=120
-    celery beat --app=hawc.main.celery --loglevel=INFO --logfile=celery-beat.log
+
+    # check redis is up and can be pinged successfully
+    redis-cli -h localhost -a default-password ping
+
+    # start workers
+    celery worker --app=hawc.main.celery --loglevel=INFO
+    celery beat --app=hawc.main.celery --loglevel=INFO
+
+    # stop redis when you're done
+    docker-compose down
 
 Asynchronous tasks will no be executed by celery workers instead of the main thread.
 

hawc/apps/assessment/tasks.py

@@ -18,6 +18,16 @@ def delete_old_jobs():
     models.Job.objects.filter(last_updated__lte=week_old).delete()
 
 
+@shared_task
+def delete_orphan_relations(delete: bool = False):
+    # remove orphan relations in cases where the db cannot do so directly
+    Log = apps.get_model("assessment", "Log")
+    RiskOfBiasScoreOverrideObject = apps.get_model("riskofbias", "RiskOfBiasScoreOverrideObject")
+    message = RiskOfBiasScoreOverrideObject.get_orphan_relations(delete=delete)
+    if message:
+        Log.objects.create(message=message)
+
+
 @shared_task(bind=True)
 def run_job(self):
     job = models.Job.objects.get(pk=self.request.id)

hawc/apps/riskofbias/admin.py

@@ -86,6 +86,12 @@ def num_override_scores(self, obj):
     raw_id_fields = ("study",)
 
 
+class RiskOfBiasScoreOverrideObjectInline(admin.TabularInline):
+    model = models.RiskOfBiasScoreOverrideObject
+    fk_name = "score"
+    extra = 0
+
+
 @admin.register(models.RiskOfBiasScore)
 class RiskOfBiasScoreAdmin(admin.ModelAdmin):
     list_display = (
@@ -104,3 +110,11 @@ class RiskOfBiasScoreAdmin(admin.ModelAdmin):
         "riskofbias",
         "metric",
     )
+    inlines = [RiskOfBiasScoreOverrideObjectInline]
+
+
+@admin.register(models.RiskOfBiasScoreOverrideObject)
+class RiskOfBiasScoreOverrideObjectAdmin(admin.ModelAdmin):
+    list_display = ("id", "content_type", "object_id")
+    list_filter = (("content_type", admin.RelatedOnlyFieldListFilter),)
+    raw_id_fields = ("score",)

hawc/apps/riskofbias/models.py

@@ -625,15 +625,59 @@ class RiskOfBiasScoreOverrideObject(models.Model):
     object_id = models.PositiveIntegerField()
     content_object = GenericForeignKey("content_type", "object_id")
 
+    def __str__(self):
+        return f"id={self.id};score={self.score_id};obj_ct={self.content_type_id};obj_id={self.object_id}"
+
     def get_content_type_name(self) -> str:
         return f"{self.content_type.app_label}.{self.content_type.model}"
 
     def get_object_url(self) -> str:
+        if self.content_object is None:
+            return reverse("404")
         return self.content_object.get_absolute_url()
 
     def get_object_name(self) -> str:
+        if self.content_object is None:
+            return f"<deleted {self}>"
         return str(self.content_object)
 
+    @classmethod
+    def get_orphan_relations(cls, delete: bool = False) -> str:
+        """Determine if any relations are orphaned and optionally delete.
+
+        Args:
+            delete (bool, optional): Delete found instances. Defaults to False.
+
+        Returns:
+            str: A log message of relations found and what as done.
+        """
+        cts = RiskOfBiasScoreOverrideObject.objects.values_list(
+            "content_type", flat=True
+        ).distinct()
+
+        deletions = []
+        for ct in cts:
+            RelatedClass = ContentType.objects.get_for_id(ct).model_class()
+            all_ids = cls.objects.filter(content_type=ct).values_list("object_id", flat=True)
+            matched_ids = RelatedClass.objects.filter(id__in=all_ids).values_list("id", flat=True)
+            deleted_ids = list(set(all_ids) - set(matched_ids))
+            if deleted_ids:
+                deletions.extend(
+                    list(cls.objects.filter(content_type=ct, object_id__in=deleted_ids))
+                )
+
+        message = ""
+        if deletions:
+            message = "\n".join([str(item) for item in deletions])
+            ids_to_delete = [item.id for item in deletions]
+            if delete:
+                message = f"Deleting orphaned RiskOfBiasScoreOverrideObjects:\n{message}"
+                cls.objects.filter(id__in=ids_to_delete).delete()
+            else:
+                message = f"Found orphaned RiskOfBiasScoreOverrideObjects:\n{message}"
+
+        return message
+
 
 DEFAULT_QUESTIONS_OHAT = 1
 DEFAULT_QUESTIONS_EPA = 2

hawc/main/celery.py

@@ -33,4 +33,9 @@ def debug_task(self):
         "task": "hawc.apps.assessment.tasks.delete_old_jobs",
         "schedule": timedelta(days=1),
     },
+    "delete-orphan-relations": {
+        "task": "hawc.apps.assessment.tasks.delete_orphan_relations",
+        "schedule": timedelta(hours=6),
+        "kwargs": dict(delete=False),
+    },
 }

tests/data/api/api-visual-barchart.json

@@ -381,6 +381,14 @@
                   "object_id": 3,
                   "object_name": "sd rats",
                   "object_url": "/ani/animal-group/3/"
+                },
+                {
+                  "id": 3,
+                  "score_id": 16,
+                  "content_type_name": "animal.animalgroup",
+                  "object_id": 99999,
+                  "object_name": "<deleted id=3;score=16;obj_ct=49;obj_id=99999>",
+                  "object_url": "/404/"
                 }
               ],
               "riskofbias_id": 6,

tests/data/api/api-visual-rob-heatmap.json

@@ -383,6 +383,14 @@
                   "object_id": 3,
                   "object_name": "sd rats",
                   "object_url": "/ani/animal-group/3/"
+                },
+                {
+                  "id": 3,
+                  "score_id": 16,
+                  "content_type_name": "animal.animalgroup",
+                  "object_id": 99999,
+                  "object_name": "<deleted id=3;score=16;obj_ct=49;obj_id=99999>",
+                  "object_url": "/404/"
                 }
               ],
               "riskofbias_id": 6,

tests/data/fixtures/db.yaml

@@ -4178,6 +4178,14 @@
     - animal
     - animalgroup
     object_id: 3
+- model: riskofbias.riskofbiasscoreoverrideobject
+  pk: 3
+  fields:
+    score: 16
+    content_type:
+    - animal
+    - animalgroup
+    object_id: 99999
 - model: riskofbias.riskofbiasassessment
   pk: 1
   fields:

tests/hawc/apps/riskofbias/test_riskofbias_models.py

@@ -0,0 +1,33 @@
+from textwrap import dedent
+
+import pytest
+from django.urls import reverse
+
+from hawc.apps.riskofbias.models import RiskOfBiasScoreOverrideObject
+
+
+@pytest.mark.django_db
+class TestRiskOfBiasScoreOverrideObject:
+    def test_get_object_url(self):
+        valid = RiskOfBiasScoreOverrideObject.objects.get(id=2)
+        assert valid.get_object_url() == valid.content_object.get_absolute_url()
+
+        invalid = RiskOfBiasScoreOverrideObject.objects.get(id=3)
+        assert invalid.get_object_url() == reverse("404")
+
+    def test_get_object_name(self):
+        valid = RiskOfBiasScoreOverrideObject.objects.get(id=2)
+        assert valid.get_object_name() == "sd rats"
+
+        invalid = RiskOfBiasScoreOverrideObject.objects.get(id=3)
+        assert "deleted" in invalid.get_object_name()
+
+    def test_get_orphan_relations(self):
+        actual = RiskOfBiasScoreOverrideObject.get_orphan_relations()
+        expected = dedent(
+            """
+            Found orphaned RiskOfBiasScoreOverrideObjects:
+            id=3;score=16;obj_ct=49;obj_id=99999
+            """
+        ).strip()
+        assert actual == expected