TP2000-1582 Enable customisable queue field name on QueueItem subclas…

…ses (#1343)

* Amend Queue model docstring

* Enable customisable queue field name of QueueItem concrete subclasses

* Add QueueItem metaclass validation tests

* Add type hint to QueueItem.queue_field and update its docstring

* Get field_name post-init to include inherited attributes from parent class

* Rename queue field on TaskItem and TaskItemTemplate models

* Update references to named queue

* Recommend QueueItem subclasses redefine model Meta class following queue field renaming

common/util.py

@@ -796,7 +796,7 @@ def get_related_names(instance, related_model) -> list[str]:
 
     If a reverse foreign-key relationship exists but no related name has been
     defined, a default name in the format `relatedmodel_set` will be returned.
-    If no such relationships exist, an empty list if returned.
+    If no such relationships exist, an empty list is returned.
     """
     related_names = []
     for field in instance._meta.get_fields():

tasks/migrations/0017_rename_queue_taskitem_workflow_and_more.py

@@ -0,0 +1,51 @@
+# Generated by Django 4.2.15 on 2024-12-05 15:46
+
+import django.db.models.deletion
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("tasks", "0016_taskworkflowtemplate_creator"),
+    ]
+
+    operations = [
+        migrations.RenameField(
+            model_name="taskitem",
+            old_name="queue",
+            new_name="workflow",
+        ),
+        migrations.AlterField(
+            model_name="taskitem",
+            name="workflow",
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="workflow_items",
+                to="tasks.taskworkflow",
+            ),
+        ),
+        migrations.AlterModelOptions(
+            name="taskitem",
+            options={"ordering": ["workflow", "position"]},
+        ),
+        migrations.RenameField(
+            model_name="taskitemtemplate",
+            old_name="queue",
+            new_name="workflow_template",
+        ),
+        migrations.AlterField(
+            model_name="taskitemtemplate",
+            name="workflow_template",
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="workflow_template_items",
+                to="tasks.taskworkflowtemplate",
+            ),
+        ),
+        migrations.AlterModelOptions(
+            name="taskitemtemplate",
+            options={"ordering": ["workflow_template", "position"]},
+        ),
+    ]

tasks/models/queue.py

@@ -2,6 +2,7 @@
 
 from typing import Self
 
+from django.core.exceptions import FieldDoesNotExist
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import models
 from django.db.transaction import atomic
@@ -18,11 +19,8 @@ class Queue(models.Model):
     """
     A (FIFO) queue.
 
-    Note: This abstract class only supports a single reverse foreign-key relationship
-    to `QueueItem` for each instance. As such, all instance methods in this class use
-    the first-returned related name to access related `QueueItem` objects. This means that
-    if there are multiple relationships, only the first one will be considered when
-    retrieving items from the queue,
+    Note: This abstract class only supports a single, reverse foreign-key relationship
+    to `QueueItem` for each instance (i.e `QueueItem` instances are assumed to belong only to a single `Queue` instance).
     """
 
     class Meta:
@@ -65,18 +63,31 @@ class QueueItemMetaClass(models.base.ModelBase):
     def __new__(cls, name, bases, attrs):
         new_class = super().__new__(cls, name, bases, attrs)
 
-        if (
-            "QueueItem" in [base.__name__ for base in bases]
-            and not new_class._meta.abstract
-        ):
-            queue_field = attrs.get("queue", None)
-            if not queue_field or not isinstance(queue_field, models.ForeignKey):
-                raise RequiredFieldError(
-                    f"{name} must have a 'queue' ForeignKey field.",
-                )
+        if not new_class._meta.abstract:
+            queue_field_name = getattr(new_class, "queue_field", None)
+            cls.validate_queue_field(new_class, queue_field_name)
 
         return new_class
 
+    @staticmethod
+    def validate_queue_field(new_class: type[Self], queue_field_name: str) -> None:
+        """Validate that concrete subclasses of `QueueItem` have a ForeignKey
+        field to a subclass of `Queue` model."""
+        try:
+            queue_field = new_class._meta.get_field(queue_field_name)
+        except FieldDoesNotExist:
+            queue_field = None
+
+        if not queue_field or not isinstance(queue_field, models.ForeignKey):
+            raise RequiredFieldError(
+                f"{new_class.__name__} must have a 'queue' ForeignKey field. The name of the field must match the value given to the 'queue_field' attribute on the model.",
+            )
+
+        if not issubclass(queue_field.remote_field.model, Queue):
+            raise RequiredFieldError(
+                f"{queue_field} must be a ForeignKey field to a subclass of 'Queue' model.",
+            )
+
 
 class QueueItemManager(models.Manager):
     @atomic
@@ -85,7 +96,8 @@ def create(self, **kwargs) -> QueueItem:
         param, and place it in last position."""
 
         with TableLock(self.model, lock=TableLock.EXCLUSIVE):
-            queue = kwargs.pop("queue")
+            queue_field = self.model.queue_field
+            queue = kwargs.pop(queue_field)
             position = kwargs.pop("position", (queue.get_items().count() + 1))
 
             if position <= 0:
@@ -94,8 +106,8 @@ def create(self, **kwargs) -> QueueItem:
                 )
 
             return super().create(
-                queue=queue,
                 position=position,
+                **{queue_field: queue},
                 **kwargs,
             )
 
@@ -107,6 +119,18 @@ class Meta:
         abstract = True
         ordering = ["queue", "position"]
 
+    queue_field: str = "queue"
+    """
+    The name of the required ForeignKey field relating this instance to a Queue
+    instance.
+
+    The value of this attribute can be inherited as is or overridden in
+    subclasses to reflect the specific purpose or role of the queue.
+
+    If this value is overridden, the subclass must redefine the `Meta` class
+    to maintain ordering based on the queue field.
+    """
+
     position = models.PositiveSmallIntegerField(
         db_index=True,
         editable=False,
@@ -117,6 +141,14 @@ class Meta:
 
     objects = QueueItemManager()
 
+    def get_queue_field(self) -> str:
+        """Return the queue field name on this instance."""
+        return self.__class__.queue_field
+
+    def get_queue(self) -> type[Queue]:
+        """Return the queue instance related to this instance."""
+        return getattr(self, self.get_queue_field())
+
     @atomic
     def delete(self):
         """Remove and delete instance from its queue, shuffling all successive
@@ -125,7 +157,7 @@ def delete(self):
 
         self.__class__.objects.select_for_update(nowait=True).filter(
             position__gt=instance.position,
-            queue=instance.queue,
+            **{self.get_queue_field(): self.get_queue()},
         ).update(position=models.F("position") - 1)
 
         return super().delete()
@@ -146,7 +178,7 @@ def promote(self) -> Self:
 
         item_to_demote = self.__class__.objects.select_for_update(nowait=True).get(
             position=instance.position - 1,
-            queue=instance.queue,
+            **{self.get_queue_field(): self.get_queue()},
         )
         item_to_demote.position += 1
         instance.position -= 1
@@ -166,12 +198,18 @@ def demote(self) -> Self:
         """
         instance = self.__class__.objects.select_for_update(nowait=True).get(pk=self.pk)
 
-        if instance.position == self.queue.max_position:
+        queue_field = self.get_queue_field()
+        queue = self.get_queue()
+        queue_kwarg = {
+            queue_field: queue,
+        }
+
+        if instance.position == queue.max_position:
             return instance
 
         item_to_promote = self.__class__.objects.select_for_update(nowait=True).get(
             position=instance.position + 1,
-            queue=instance.queue,
+            **queue_kwarg,
         )
         item_to_promote.position -= 1
         instance.position += 1
@@ -198,7 +236,7 @@ def promote_to_first(self) -> Self:
 
         self.__class__.objects.select_for_update(nowait=True).filter(
             position__lt=instance.position,
-            queue=instance.queue,
+            **{self.get_queue_field(): self.get_queue()},
         ).update(position=models.F("position") + 1)
 
         instance.position = 1
@@ -219,13 +257,19 @@ def demote_to_last(self) -> Self:
         """
         instance = self.__class__.objects.select_for_update(nowait=True).get(pk=self.pk)
 
-        last_place = self.queue.max_position
+        queue_field = self.get_queue_field()
+        queue = self.get_queue()
+        queue_kwarg = {
+            queue_field: queue,
+        }
+
+        last_place = queue.max_position
         if instance.position == last_place:
             return instance
 
         self.__class__.objects.select_for_update(nowait=True).filter(
             position__gt=instance.position,
-            queue=instance.queue,
+            **queue_kwarg,
         ).update(position=models.F("position") - 1)
 
         instance.position = last_place

tasks/models/workflow.py

@@ -48,7 +48,9 @@ def get_tasks(self) -> models.QuerySet:
         """Get a QuerySet of the Tasks associated through their TaskItem
         instances to this TaskWorkflow, ordered by the position of the
         TaskItem."""
-        return Task.objects.filter(taskitem__queue=self).order_by("taskitem__position")
+        return Task.objects.filter(taskitem__workflow=self).order_by(
+            "taskitem__position",
+        )
 
     def get_url(self, action: str = "detail"):
         if action == "detail":
@@ -73,9 +75,11 @@ class TaskItem(QueueItem):
     """Task item queue management for Task instances (these should always be
     subtasks)."""
 
-    queue = models.ForeignKey(
+    queue_field = "workflow"
+
+    workflow = models.ForeignKey(
         TaskWorkflow,
-        related_name="queue_items",
+        related_name="workflow_items",
         on_delete=models.CASCADE,
     )
     task = models.OneToOneField(
@@ -84,6 +88,9 @@ class TaskItem(QueueItem):
     )
     """The Task instance managed by this TaskItem."""
 
+    class Meta:
+        ordering = ["workflow", "position"]
+
 
 # ----------------------------------------
 # - Template workflows and template tasks.
@@ -129,7 +136,9 @@ def get_task_templates(self) -> models.QuerySet:
         """Get a QuerySet of the TaskTemplates associated through their
         TaskItemTemplate instances to this TaskWorkflowTemplate, ordered by the
         position of the TaskItemTemplate."""
-        return TaskTemplate.objects.filter(taskitemtemplate__queue=self).order_by(
+        return TaskTemplate.objects.filter(
+            taskitemtemplate__workflow_template=self,
+        ).order_by(
             "taskitemtemplate__position",
         )
 
@@ -155,7 +164,7 @@ def create_task_workflow(
 
         task_item_templates = TaskItemTemplate.objects.select_related(
             "task_template",
-        ).filter(queue=self)
+        ).filter(workflow_template=self)
         for task_item_template in task_item_templates:
             task_template = task_item_template.task_template
             task = Task.objects.create(
@@ -165,7 +174,7 @@ def create_task_workflow(
             )
             TaskItem.objects.create(
                 position=task_item_template.position,
-                queue=task_workflow,
+                workflow=task_workflow,
                 task=task,
             )
 
@@ -202,16 +211,21 @@ def get_url(self, action: str = "detail"):
 class TaskItemTemplate(QueueItem):
     """Queue item management for TaskTemplate instances."""
 
-    queue = models.ForeignKey(
+    queue_field = "workflow_template"
+
+    workflow_template = models.ForeignKey(
         TaskWorkflowTemplate,
-        related_name="queue_items",
+        related_name="workflow_template_items",
         on_delete=models.CASCADE,
     )
     task_template = models.OneToOneField(
         "tasks.TaskTemplate",
         on_delete=models.CASCADE,
     )
 
+    class Meta:
+        ordering = ["workflow_template", "position"]
+
 
 class TaskTemplate(TaskBase):
     """Template used to create Task instances from within a template
@@ -226,7 +240,7 @@ def get_url(self, action: str = "detail"):
             return reverse(
                 "workflow:task-template-ui-delete",
                 kwargs={
-                    "workflow_template_pk": self.taskitemtemplate.queue.pk,
+                    "workflow_template_pk": self.taskitemtemplate.workflow_template.pk,
                     "pk": self.pk,
                 },
             )

tasks/tests/conftest.py

@@ -69,7 +69,7 @@ def task_workflow_template_single_task_template_item(
 
     task_template = factories.TaskTemplateFactory.create()
     factories.TaskItemTemplateFactory.create(
-        queue=task_workflow_template,
+        workflow_template=task_workflow_template,
         task_template=task_template,
     )
 
@@ -89,15 +89,15 @@ def task_workflow_template_three_task_template_items(
     for _ in range(3):
         task_template = factories.TaskTemplateFactory.create()
         task_item_template = factories.TaskItemTemplateFactory.create(
-            queue=task_workflow_template,
+            workflow_template=task_workflow_template,
             task_template=task_template,
         )
         task_item_templates.append(task_item_template)
 
     assert task_workflow_template.get_items().count() == 3
     assert (
         TaskItemTemplate.objects.filter(
-            queue=task_workflow_template,
+            workflow_template=task_workflow_template,
         ).count()
         == 3
     )
@@ -123,7 +123,7 @@ def task_workflow_single_task_item(task_workflow) -> TaskWorkflow:
     associated Task instance."""
 
     task_item = factories.TaskItemFactory.create(
-        queue=task_workflow,
+        workflow=task_workflow,
     )
 
     assert task_workflow.get_items().count() == 1
@@ -143,11 +143,11 @@ def task_workflow_three_task_items(
 
     task_items = factories.TaskItemFactory.create_batch(
         expected_count,
-        queue=task_workflow,
+        workflow=task_workflow,
     )
 
     assert task_workflow.get_items().count() == expected_count
-    assert TaskItem.objects.filter(queue=task_workflow).count() == expected_count
+    assert TaskItem.objects.filter(workflow=task_workflow).count() == expected_count
     assert (
         Task.objects.filter(taskitem__in=[item.pk for item in task_items]).count()
         == expected_count