[IMP] queue_job: prevent invalid change of job status

Author: florentx

queue_job/models/queue_job.py

@@ -17,6 +17,7 @@
 from ..job import (
     CANCELLED,
     DONE,
+    ENQUEUED,
     FAILED,
     PENDING,
     STARTED,
@@ -324,19 +325,24 @@ def _change_job_state(self, state, result=None):
             else:
                 raise ValueError(f"State not supported: {state}")
 
-    def button_done(self):
+    def button_done(self, __state_in=(WAIT_DEPENDENCIES, PENDING, ENQUEUED, FAILED)):
+        # If job was set to STARTED or CANCELLED, do not set it to DONE
         result = _("Manually set to done by {}").format(self.env.user.name)
-        self._change_job_state(DONE, result=result)
+        records = self.filtered(lambda job_: job_.state in __state_in)
+        records._change_job_state(DONE, result=result)
         return True
 
-    def button_cancelled(self):
+    def button_cancelled(self, __state_in=(PENDING, ENQUEUED, FAILED)):
+        # If job was set to DONE or WAIT_DEPENDENCIES, do not cancel it
         result = _("Cancelled by {}").format(self.env.user.name)
-        self._change_job_state(CANCELLED, result=result)
+        records = self.filtered(lambda job_: job_.state in __state_in)
+        records._change_job_state(CANCELLED, result=result)
         return True
 
-    def requeue(self):
-        jobs_to_requeue = self.filtered(lambda job_: job_.state != WAIT_DEPENDENCIES)
-        jobs_to_requeue._change_job_state(PENDING)
+    def requeue(self, __state_in=(FAILED, DONE, CANCELLED)):
+        # If job is already in queue or started, do not requeue it
+        records = self.filtered(lambda job_: job_.state in __state_in)
+        records._change_job_state(PENDING)
         return True
 
     def _message_post_on_failure(self):

queue_job/tests/test_wizards.py

@@ -46,3 +46,60 @@ def test_03_done(self):
         wizard = self._wizard("queue.jobs.to.done")
         wizard.set_done()
         self.assertEqual(self.job.state, "done")
+
+    def test_04_requeue_forbidden(self):
+        wizard = self._wizard("queue.requeue.job")
+
+        # State WAIT_DEPENDENCIES is not requeued
+        self.job.state = "wait_dependencies"
+        wizard.requeue()
+        self.assertEqual(self.job.state, "wait_dependencies")
+
+        # State PENDING, ENQUEUED or STARTED are ignored too
+        for test_state in ("pending", "enqueued", "started"):
+            self.job.state = test_state
+            wizard.requeue()
+            self.assertEqual(self.job.state, test_state)
+
+        # States CANCELLED, DONE or FAILED will change status
+        self.job.state = "cancelled"
+        wizard.requeue()
+        self.assertEqual(self.job.state, "pending")
+
+    def test_05_cancel_forbidden(self):
+        wizard = self._wizard("queue.jobs.to.cancelled")
+
+        # State WAIT_DEPENDENCIES is not cancelled
+        self.job.state = "wait_dependencies"
+        wizard.set_cancelled()
+        self.assertEqual(self.job.state, "wait_dependencies")
+
+        # State DONE is not cancelled
+        self.job.state = "done"
+        wizard.set_cancelled()
+        self.assertEqual(self.job.state, "done")
+
+        # State PENDING, ENQUEUED or FAILED will be cancelled
+        for test_state in ("pending", "enqueued"):
+            self.job.state = test_state
+            wizard.set_cancelled()
+            self.assertEqual(self.job.state, "cancelled")
+
+    def test_06_done_forbidden(self):
+        wizard = self._wizard("queue.jobs.to.done")
+
+        # State STARTED is not set DONE manually
+        self.job.state = "started"
+        wizard.set_done()
+        self.assertEqual(self.job.state, "started")
+
+        # State CANCELLED is not cancelled
+        self.job.state = "cancelled"
+        wizard.set_done()
+        self.assertEqual(self.job.state, "cancelled")
+
+        # State WAIT_DEPENDENCIES, PENDING, ENQUEUED or FAILED will be set to DONE
+        for test_state in ("wait_dependencies", "pending", "enqueued"):
+            self.job.state = test_state
+            wizard.set_done()
+            self.assertEqual(self.job.state, "done")

queue_job/wizards/queue_jobs_to_cancelled.py

@@ -10,8 +10,8 @@ class SetJobsToCancelled(models.TransientModel):
     _description = "Cancel all selected jobs"
 
     def set_cancelled(self):
-        jobs = self.job_ids.filtered(
-            lambda x: x.state in ("pending", "failed", "enqueued")
-        )
+        # Only jobs with state PENDING, FAILED, ENQUEUED
+        # will change to CANCELLED
+        jobs = self.job_ids
         jobs.button_cancelled()
         return {"type": "ir.actions.act_window_close"}

queue_job/wizards/queue_jobs_to_done.py

@@ -10,6 +10,8 @@ class SetJobsToDone(models.TransientModel):
     _description = "Set all selected jobs to done"
 
     def set_done(self):
+        # Only jobs with state WAIT_DEPENDENCIES, PENDING, ENQUEUED or FAILED
+        # will change to DONE
         jobs = self.job_ids
         jobs.button_done()
         return {"type": "ir.actions.act_window_close"}

queue_job/wizards/queue_requeue_job.py

@@ -20,6 +20,7 @@ def _default_job_ids(self):
     )
 
     def requeue(self):
+        # Only jobs with state FAILED, DONE or CANCELLED will change to PENDING
         jobs = self.job_ids
         jobs.requeue()
         return {"type": "ir.actions.act_window_close"}