Don't update queue positions if packaged workbasket is no longer a queue member

Author: dalecannon

publishing/models/packaged_workbasket.py

@@ -669,7 +669,7 @@ def promote_to_top_position(self) -> "PackagedWorkBasket":
         """Promote the instance to the top position of the package processing
         queue so that it occupies position 1."""
 
-        if self.position == 1:
+        if self.position <= 1:
             return self
 
         position = self.position
@@ -680,6 +680,7 @@ def promote_to_top_position(self) -> "PackagedWorkBasket":
 
         self.position = 1
         self.save()
+        self.refresh_from_db()
 
         return self
 
@@ -689,8 +690,8 @@ def promote_position(self) -> "PackagedWorkBasket":
         """Promote the instance by one position up the package processing
         queue."""
 
-        if self.position == 1:
-            return
+        if self.position <= 1:
+            return self
 
         obj_to_swap = PackagedWorkBasket.objects.select_for_update(nowait=True).get(
             position=self.position - 1,
@@ -711,8 +712,8 @@ def demote_position(self) -> "PackagedWorkBasket":
         """Demote the instance by one position down the package processing
         queue."""
 
-        if self.position == PackagedWorkBasket.objects.max_position():
-            return
+        if self.position in {0, PackagedWorkBasket.objects.max_position()}:
+            return self
 
         obj_to_swap = PackagedWorkBasket.objects.select_for_update(nowait=True).get(
             position=self.position + 1,

publishing/tests/test_models.py

@@ -333,6 +333,37 @@ def test_demote_position():
     assert initially_second.position == 1
 
 
+def test_cannot_promote_or_demote_removed_packaged_workbasket():
+    """Tests that packaged workbasket positions remain unchanged after
+    attempting to reposition a packaged workbasket that has since been removed
+    from the queue."""
+    with patch(
+        "publishing.tasks.create_xml_envelope_file.apply_async",
+        return_value=MagicMock(id=factory.Faker("uuid4")),
+    ):
+        factories.PackagedWorkBasketFactory()
+
+    with patch(
+        "publishing.tasks.create_xml_envelope_file.apply_async",
+        return_value=MagicMock(id=factory.Faker("uuid4")),
+    ):
+        factories.PackagedWorkBasketFactory()
+
+    queued_pwb = PackagedWorkBasket.objects.get(position=1)
+    removed_pwb = PackagedWorkBasket.objects.get(position=2)
+    removed_pwb.abandon()
+
+    removed_pwb = removed_pwb.promote_to_top_position()
+    assert removed_pwb.position == 0
+    queued_pwb.refresh_from_db()
+    assert queued_pwb.position == 1
+
+    removed_pwb = removed_pwb.demote_position()
+    assert removed_pwb.position == 0
+    queued_pwb.refresh_from_db()
+    assert queued_pwb.position == 1
+
+
 def test_pause_and_unpause_queue(unpause_queue):
     assert not OperationalStatus.is_queue_paused()
     OperationalStatus.pause_queue(user=None)