From 0c46393b32e9d1ba91f5523f12f75b1f2957cc8c Mon Sep 17 00:00:00 2001 From: Ahmet Dal Date: Mon, 20 Jul 2020 11:32:37 +0200 Subject: [PATCH 1/2] [#162] Don't cancel possible transitions even though it is the future of one of those that is cancelled --- features/issue162_1.feature | 43 ++++++++++++++ river/core/instanceworkflowobject.py | 62 +++++++------------- river/tests/core/test__instance_api.py | 78 ++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 43 deletions(-) create mode 100644 features/issue162_1.feature diff --git a/features/issue162_1.feature b/features/issue162_1.feature new file mode 100644 index 0000000..fb0d5e1 --- /dev/null +++ b/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 the state to transit all the way to Closed + 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" diff --git a/river/core/instanceworkflowobject.py b/river/core/instanceworkflowobject.py index cdd27ba..ceccb1b 100644 --- a/river/core/instanceworkflowobject.py +++ b/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) diff --git a/river/tests/core/test__instance_api.py b/river/tests/core/test__instance_api.py index caaa29a..ac0976e 100644 --- a/river/tests/core/test__instance_api.py +++ b/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)) + ) + ), From 58b0eab1a812c2fb47d4f7c2c9c0b526a4140d39 Mon Sep 17 00:00:00 2001 From: Ahmet Dal Date: Mon, 20 Jul 2020 16:40:29 +0200 Subject: [PATCH 2/2] [#162] Don't cancel those transitions that are actually in the possible future just because they are also in the future of impossible transition path --- docs/changelog.rst | 8 +++++++- docs/conf.py | 4 ++-- setup.py | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 475fd88..fbf4801 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -3,8 +3,14 @@ Change Logs =========== -3.2.1 (Stable): +3.2.2 (Stable): --------------- + * **Bug** - # 162_: Fix a bug that is causing some possible future transitions to turn to CANCELLED for some workflows. + +.. _162: https://github.com/javrasya/django-river/issues/159 + +3.2.1: +------ * **Bug** - # 159_: A bug that is with having multiple cyclic dependencies in a workflow that happens when one of tem goes through has been fixed. * **Drop** - : Drop Python3.4 support since it is having incompatibilities with the module ``six`` diff --git a/docs/conf.py b/docs/conf.py index 5985b51..8aad04a 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -57,9 +57,9 @@ # built documents. # # The short X.Y version. -version = '3.2.1' +version = '3.2.2' # The full version, including alpha/beta/rc tags. -release = '3.2.1' +release = '3.2.2' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.py b/setup.py index e917152..bc4a918 100644 --- a/setup.py +++ b/setup.py @@ -13,7 +13,7 @@ setup( name='django-river', - version='3.2.1', + version='3.2.2', author='Ahmet DAL', author_email='ceahmetdal@gmail.com', packages=find_packages(),