[#162] Don't cancel possible transitions even though it is the future…

… of one of those that is cancelled

features/issue162_1.feature

@@ -0,0 +1,43 @@
+Feature: An example #162 Flow that is set up with django-river (https://github.com/javrasya/django-river/issues/162)
+
+  Background: some requirement of this test
+    # Groups
+    Given a group with name "Authorized Group"
+
+    # Users
+    Given a user with name authorized_user with group "Authorized Group"
+
+    # States
+    Given a state with label "Draft"
+    And a state with label "Issued"
+    And a state with label "Part Received"
+    And a state with label "Received"
+    And a state with label "Closed"
+
+    # Workflow
+    Given a workflow with an identifier "#162 Flow" and initial state "Draft"
+
+    # Transitions
+    Given a transition "Draft" -> "Issued" in "#162 Flow"
+    And a transition "Issued" -> "Part Received" in "#162 Flow"
+    And a transition "Part Received" -> "Received" in "#162 Flow"
+    And a transition "Issued" -> "Received" in "#162 Flow"
+    And a transition "Received" -> "Issued" in "#162 Flow"
+    And a transition "Received" -> "Closed" in "#162 Flow"
+
+    # Authorization Rules
+    Given an authorization rule for the transition "Draft" -> "Issued" with group "Authorized Group" and priority 0
+    Given an authorization rule for the transition "Issued" -> "Part Received" with group "Authorized Group" and priority 0
+    Given an authorization rule for the transition "Part Received" -> "Received" with group "Authorized Group" and priority 0
+    Given an authorization rule for the transition "Issued" -> "Received" with group "Authorized Group" and priority 0
+    Given an authorization rule for the transition "Received" -> "Issued" with group "Authorized Group" and priority 0
+    Given an authorization rule for the transition "Received" -> "Closed" with group "Authorized Group" and priority 0
+
+  Scenario: Should allow multiple cyclic transitions when one of them goes through
+    Given a workflow object with identifier "object 1"
+    When "object 1" is attempted to be approved for next state "Issued" by authorized_user
+    And "object 1" is attempted to be approved for next state "Part Received" by authorized_user
+    And "object 1" is attempted to be approved for next state "Received" by authorized_user
+    And "object 1" is attempted to be approved for next state "Closed" by authorized_user
+    And get current state of "object 1"
+    Then return current state as "Closed"

river/core/instanceworkflowobject.py

@@ -158,55 +158,31 @@ def approve(self, as_user, next_state=None):
     @atomic
     def cancel_impossible_future(self, approved_approval):
         transition = approved_approval.transition
-        qs = Q(
-            workflow=self.workflow,
-            object_id=self.workflow_object.pk,
-            iteration=transition.iteration,
-            source_state=transition.source_state,
-        ) & ~Q(destination_state=transition.destination_state)
-
-        transitions = Transition.objects.filter(qs)
-        iteration = transition.iteration + 1
-        cancelled_transitions_qs = Q(pk=-1)
-        while transitions:
-            cancelled_transitions_qs = cancelled_transitions_qs | qs
-            qs = Q(
+
+        possible_transition_ids = {transition.pk}
+
+        possible_next_states = {transition.destination_state.label}
+        while possible_next_states:
+            possible_transitions = Transition.objects.filter(
                 workflow=self.workflow,
                 object_id=self.workflow_object.pk,
-                iteration=iteration,
-                source_state__pk__in=transitions.values_list("destination_state__pk", flat=True)
-            )
-            transitions = Transition.objects.filter(qs)
-            iteration += 1
+                status=PENDING,
+                source_state__label__in=possible_next_states
+            ).exclude(pk__in=possible_transition_ids)
+
+            possible_transition_ids.update(set(possible_transitions.values_list("pk", flat=True)))
 
-        uncancelled_transitions_qs = Q(pk=-1)
-        qs = Q(
+            possible_next_states = set(possible_transitions.values_list("destination_state__label", flat=True))
+
+        cancelled_transitions = Transition.objects.filter(
             workflow=self.workflow,
             object_id=self.workflow_object.pk,
-            iteration=transition.iteration,
-            source_state=transition.source_state,
-            destination_state=transition.destination_state
-        )
-        transitions = Transition.objects.filter(qs)
-        iteration = transition.iteration + 1
-        while transitions:
-            uncancelled_transitions_qs = uncancelled_transitions_qs | qs
-            qs = Q(
-                workflow=self.workflow,
-                object_id=self.workflow_object.pk,
-                iteration=iteration,
-                source_state__pk__in=transitions.values_list("destination_state__pk", flat=True),
-                status=PENDING
-            )
-            transitions = Transition.objects.filter(qs)
-            iteration += 1
-
-        actual_cancelled_transitions = Transition.objects.select_for_update(nowait=True).filter(cancelled_transitions_qs & ~uncancelled_transitions_qs)
-        for actual_cancelled_transition in actual_cancelled_transitions:
-            actual_cancelled_transition.status = CANCELLED
-            actual_cancelled_transition.save()
+            status=PENDING,
+            iteration__gte=transition.iteration
+        ).exclude(pk__in=possible_transition_ids)
 
-        TransitionApproval.objects.filter(transition__in=actual_cancelled_transitions).update(status=CANCELLED)
+        TransitionApproval.objects.filter(transition__in=cancelled_transitions).update(status=CANCELLED)
+        cancelled_transitions.update(status=CANCELLED)
 
     def _approve_signal(self, approval):
         return ApproveSignal(self.workflow_object, self.field_name, approval)

river/tests/core/test__instance_api.py

@@ -1953,3 +1953,81 @@ def test_shouldAllowMultipleCyclicTransitions(self):
 
         workflow_object.model.river.my_field.approve(as_user=authorized_user)
         assert_that(workflow_object.model.my_field, equal_to(cycle_state_1))
+
+    def test_shouldNotCancelDescendantsThatCanBeTransitedInTheFuture(self):
+        authorized_permission = PermissionObjectFactory()
+
+        authorized_user = UserObjectFactory(user_permissions=[authorized_permission])
+
+        state1 = StateObjectFactory(label="state_1")
+        state2 = StateObjectFactory(label="state_2")
+        state3 = StateObjectFactory(label="state_3")
+        final_state = StateObjectFactory(label="final_state")
+
+        workflow = WorkflowFactory(initial_state=state1, content_type=self.content_type, field_name="my_field")
+
+        transition_meta_1 = TransitionMetaFactory.create(
+            workflow=workflow,
+            source_state=state1,
+            destination_state=state2,
+        )
+
+        transition_meta_2 = TransitionMetaFactory.create(
+            workflow=workflow,
+            source_state=state1,
+            destination_state=state3,
+        )
+
+        transition_meta_3 = TransitionMetaFactory.create(
+            workflow=workflow,
+            source_state=state2,
+            destination_state=state3,
+        )
+
+        transition_meta_4 = TransitionMetaFactory.create(
+            workflow=workflow,
+            source_state=state3,
+            destination_state=final_state,
+        )
+
+        TransitionApprovalMetaFactory.create(
+            workflow=workflow,
+            transition_meta=transition_meta_1,
+            priority=0,
+            permissions=[authorized_permission]
+        )
+
+        TransitionApprovalMetaFactory.create(
+            workflow=workflow,
+            transition_meta=transition_meta_2,
+            priority=0,
+            permissions=[authorized_permission]
+        )
+
+        TransitionApprovalMetaFactory.create(
+            workflow=workflow,
+            transition_meta=transition_meta_3,
+            priority=0,
+            permissions=[authorized_permission]
+        )
+
+        finalTransitionApprovalMeta = TransitionApprovalMetaFactory.create(
+            workflow=workflow,
+            transition_meta=transition_meta_4,
+            priority=0,
+            permissions=[authorized_permission]
+        )
+
+        workflow_object = BasicTestModelObjectFactory()
+
+        assert_that(workflow_object.model.my_field, equal_to(state1))
+        workflow_object.model.river.my_field.approve(as_user=authorized_user, next_state=state2)
+        assert_that(workflow_object.model.my_field, equal_to(state2))
+
+        assert_that(
+            finalTransitionApprovalMeta.transition_approvals.all(),
+            all_of(
+                has_length(1),
+                has_item(has_property("status", PENDING))
+            )
+        ),