diff --git a/docs/changelog.rst b/docs/changelog.rst index e65877e..a4acf23 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -13,6 +13,8 @@ Change Logs * **Improvement** - # 110_: Introduce an iteration to keep track of the order of the transitions even the cycling ones. This comes with a migration that will assess the iteration of all of your existing approvals so far. According to the tests, 250 workflow objects that have 5 approvals each will take ~1 minutes with the slowest django `v1.11`. * **Improvement** - # 111_: Cancel all approvals that are not part of the possible future instead of only impossible the peers when something approved and re-create the whole rest of the pipeline when it cycles * **Improvement** - # 105_: More dynamic and better way for hooks.On the fly function and hook creations, update or delete are also supported now. It also comes with useful admin interfaces for hooks and functions. This is a huge improvement for callback lovers :-) + * **Improvement** - # 113_: Support defining an approval hook with a specific approval. + * **Improvement** - # 114_: Support defining a transition hook with a specific iteration. .. _105: https://github.com/javrasya/django-river/issues/105 @@ -23,6 +25,8 @@ Change Logs .. _110: https://github.com/javrasya/django-river/issues/110 .. _111: https://github.com/javrasya/django-river/issues/110 .. _112: https://github.com/javrasya/django-river/issues/112 +.. _113: https://github.com/javrasya/django-river/issues/113 +.. _114: https://github.com/javrasya/django-river/issues/114 2.0.0 (Stable) -------------- diff --git a/river/migrations/0008_auto_20191109_1130.py b/river/migrations/0008_auto_20191109_1130.py new file mode 100644 index 0000000..5d6f026 --- /dev/null +++ b/river/migrations/0008_auto_20191109_1130.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.25 on 2019-11-09 17:30 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('contenttypes', '0002_remove_content_type_name'), + ('river', '0007_transitionapproval_iteration'), + ] + + operations = [ + migrations.AddField( + model_name='onapprovedhook', + name='transition_approval', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='on_approved_hooks', to='river.TransitionApproval', verbose_name='Transition Approval'), + ), + migrations.AddField( + model_name='ontransithook', + name='iteration', + field=models.IntegerField(blank=True, default=0, null=True, verbose_name='Priority'), + ), + migrations.AlterField( + model_name='onapprovedhook', + name='transition_approval_meta', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='on_approved_hooks', to='river.TransitionApprovalMeta', verbose_name='Transition Approval Meta'), + ), + migrations.AlterUniqueTogether( + name='onapprovedhook', + unique_together=set([('callback_function', 'workflow', 'transition_approval_meta', 'content_type', 'object_id', 'transition_approval')]), + ), + migrations.AlterUniqueTogether( + name='ontransithook', + unique_together=set([('callback_function', 'workflow', 'source_state', 'destination_state', 'content_type', 'object_id', 'iteration')]), + ), + ] diff --git a/river/models/on_approved_hook.py b/river/models/on_approved_hook.py index b3d99bc..0c366ae 100644 --- a/river/models/on_approved_hook.py +++ b/river/models/on_approved_hook.py @@ -2,12 +2,13 @@ from django.db.models import CASCADE from django.utils.translation import ugettext_lazy as _ -from river.models import TransitionApprovalMeta +from river.models import TransitionApprovalMeta, TransitionApproval from river.models.hook import Hook class OnApprovedHook(Hook): class Meta: - unique_together = [('callback_function', 'workflow', 'transition_approval_meta', 'content_type', 'object_id')] + unique_together = [('callback_function', 'workflow', 'transition_approval_meta', 'content_type', 'object_id', 'transition_approval')] - transition_approval_meta = models.ForeignKey(TransitionApprovalMeta, verbose_name=_("Transition Approval"), related_name='on_approved_hooks', on_delete=CASCADE) + transition_approval_meta = models.ForeignKey(TransitionApprovalMeta, verbose_name=_("Transition Approval Meta"), related_name='on_approved_hooks', on_delete=CASCADE) + transition_approval = models.ForeignKey(TransitionApproval, verbose_name=_("Transition Approval"), related_name='on_approved_hooks', null=True, blank=True, on_delete=CASCADE) diff --git a/river/models/on_transit_hook.py b/river/models/on_transit_hook.py index 2b581dc..1741375 100644 --- a/river/models/on_transit_hook.py +++ b/river/models/on_transit_hook.py @@ -8,7 +8,8 @@ class OnTransitHook(Hook): class Meta: - unique_together = [('callback_function', 'workflow', 'source_state', 'destination_state', 'content_type', 'object_id')] + unique_together = [('callback_function', 'workflow', 'source_state', 'destination_state', 'content_type', 'object_id', 'iteration')] source_state = models.ForeignKey(State, verbose_name=_("Source State"), related_name='on_transition_hook_as_source', on_delete=CASCADE) destination_state = models.ForeignKey(State, verbose_name=_("Next State"), related_name='on_transition_hook_as_destination', on_delete=CASCADE) + iteration = models.IntegerField(default=0, verbose_name=_('Priority'), null=True, blank=True) diff --git a/river/signals.py b/river/signals.py index a6254f5..c29ef88 100644 --- a/river/signals.py +++ b/river/signals.py @@ -37,6 +37,7 @@ def __enter__(self): if self.status: for hook in OnTransitHook.objects.filter( (Q(object_id__isnull=True) | Q(object_id=self.workflow_object.pk, content_type=self.content_type)) & + (Q(iteration__isnull=True) | Q(iteration=self.transition_approval.iteration)) & Q( workflow__field_name=self.field_name, source_state=self.transition_approval.source_state, @@ -53,6 +54,7 @@ def __exit__(self, type, value, traceback): if self.status: for hook in OnTransitHook.objects.filter( (Q(object_id__isnull=True) | Q(object_id=self.workflow_object.pk, content_type=self.content_type)) & + (Q(iteration__isnull=True) | Q(iteration=self.transition_approval.iteration)) & Q( workflow=self.workflow, source_state=self.transition_approval.source_state, @@ -89,6 +91,7 @@ def __init__(self, workflow_object, field_name, transition_approval): def __enter__(self): for hook in OnApprovedHook.objects.filter( (Q(object_id__isnull=True) | Q(object_id=self.workflow_object.pk, content_type=self.content_type)) & + (Q(transition_approval__isnull=True) | Q(transition_approval=self.transition_approval)) & Q( workflow__field_name=self.field_name, transition_approval_meta=self.transition_approval.meta, @@ -103,6 +106,7 @@ def __enter__(self): def __exit__(self, type, value, traceback): for hook in OnApprovedHook.objects.filter( (Q(object_id__isnull=True) | Q(object_id=self.workflow_object.pk, content_type=self.content_type)) & + (Q(transition_approval__isnull=True) | Q(transition_approval=self.transition_approval)) & Q( workflow__field_name=self.field_name, transition_approval_meta=self.transition_approval.meta, diff --git a/river/tests/hooking/base_hooking_test.py b/river/tests/hooking/base_hooking_test.py index 9618aa8..d011425 100644 --- a/river/tests/hooking/base_hooking_test.py +++ b/river/tests/hooking/base_hooking_test.py @@ -13,7 +13,8 @@ from river.tests.hooking.base_hooking_test import callback_output def handle(context): print(context) - callback_output['%s'] = context + key = '%s' + callback_output[key] = callback_output.get(key,[]) + [context] """ @@ -26,42 +27,46 @@ def setUp(self): def get_output(self): return callback_output.get(self.identifier, None) - def hook_pre_transition(self, workflow, source_state, destination_state, workflow_object=None): + def hook_pre_transition(self, workflow, source_state, destination_state, workflow_object=None, iteration=None): OnTransitHook.objects.create( workflow=workflow, callback_function=self.callback_function, source_state=source_state, destination_state=destination_state, hook_type=BEFORE, - workflow_object=workflow_object + workflow_object=workflow_object, + iteration=iteration ) - def hook_post_transition(self, workflow, source_state, destination_state, workflow_object=None): + def hook_post_transition(self, workflow, source_state, destination_state, workflow_object=None, iteration=None): OnTransitHook.objects.create( workflow=workflow, callback_function=self.callback_function, source_state=source_state, destination_state=destination_state, hook_type=AFTER, - workflow_object=workflow_object + workflow_object=workflow_object, + iteration=iteration ) - def hook_pre_approve(self, workflow, transition_approval_meta, workflow_object=None): + def hook_pre_approve(self, workflow, transition_approval_meta, workflow_object=None, transition_approval=None): OnApprovedHook.objects.create( workflow=workflow, callback_function=self.callback_function, transition_approval_meta=transition_approval_meta, hook_type=BEFORE, - workflow_object=workflow_object + workflow_object=workflow_object, + transition_approval=transition_approval ) - def hook_post_approve(self, workflow, transition_approval_meta, workflow_object=None): + def hook_post_approve(self, workflow, transition_approval_meta, workflow_object=None, transition_approval=None): OnApprovedHook.objects.create( workflow=workflow, callback_function=self.callback_function, transition_approval_meta=transition_approval_meta, hook_type=AFTER, - workflow_object=workflow_object + workflow_object=workflow_object, + transition_approval=transition_approval ) def hook_pre_complete(self, workflow, workflow_object=None): diff --git a/river/tests/hooking/test__approved_hooking.py b/river/tests/hooking/test__approved_hooking.py index ac650c3..0a0246e 100644 --- a/river/tests/hooking/test__approved_hooking.py +++ b/river/tests/hooking/test__approved_hooking.py @@ -1,6 +1,7 @@ from django.contrib.contenttypes.models import ContentType -from hamcrest import equal_to, assert_that, none, has_entry, all_of, has_key +from hamcrest import equal_to, assert_that, none, has_entry, all_of, has_key, has_length, is_, is_not, has_item +from river.models import TransitionApproval from river.models.factories import PermissionObjectFactory, UserObjectFactory, StateObjectFactory, WorkflowFactory, TransitionApprovalMetaFactory from river.models.hook import BEFORE from river.tests.hooking.base_hooking_test import BaseHookingTest @@ -49,10 +50,11 @@ def test_shouldInvokeCallbackThatIsRegisteredWithInstanceWhenAnApprovingHappens( assert_that(workflow_object.model.my_field, equal_to(state1)) output = self.get_output() - assert_that(output, has_key("hook")) - assert_that(output["hook"], has_entry("type", "on-approved")) - assert_that(output["hook"], has_entry("when", BEFORE)) - assert_that(output["hook"], has_entry( + assert_that(output, has_length(1)) + assert_that(output[0], has_key("hook")) + assert_that(output[0]["hook"], has_entry("type", "on-approved")) + assert_that(output[0]["hook"], has_entry("when", BEFORE)) + assert_that(output[0]["hook"], has_entry( "payload", all_of( has_entry(equal_to("workflow_object"), equal_to(workflow_object.model)), @@ -65,10 +67,11 @@ def test_shouldInvokeCallbackThatIsRegisteredWithInstanceWhenAnApprovingHappens( assert_that(workflow_object.model.my_field, equal_to(state2)) output = self.get_output() - assert_that(output, has_key("hook")) - assert_that(output["hook"], has_entry("type", "on-approved")) - assert_that(output["hook"], has_entry("when", BEFORE)) - assert_that(output["hook"], has_entry( + assert_that(output, has_length(1)) + assert_that(output[0], has_key("hook")) + assert_that(output[0]["hook"], has_entry("type", "on-approved")) + assert_that(output[0]["hook"], has_entry("when", BEFORE)) + assert_that(output[0]["hook"], has_entry( "payload", all_of( has_entry(equal_to("workflow_object"), equal_to(workflow_object.model)), @@ -113,10 +116,11 @@ def test_shouldInvokeCallbackThatIsRegisteredWithoutInstanceWhenAnApprovingHappe assert_that(workflow_object.model.my_field, equal_to(state1)) output = self.get_output() - assert_that(output, has_key("hook")) - assert_that(output["hook"], has_entry("type", "on-approved")) - assert_that(output["hook"], has_entry("when", BEFORE)) - assert_that(output["hook"], has_entry( + assert_that(output, has_length(1)) + assert_that(output[0], has_key("hook")) + assert_that(output[0]["hook"], has_entry("type", "on-approved")) + assert_that(output[0]["hook"], has_entry("when", BEFORE)) + assert_that(output[0]["hook"], has_entry( "payload", all_of( has_entry(equal_to("workflow_object"), equal_to(workflow_object.model)), @@ -129,10 +133,11 @@ def test_shouldInvokeCallbackThatIsRegisteredWithoutInstanceWhenAnApprovingHappe assert_that(workflow_object.model.my_field, equal_to(state2)) output = self.get_output() - assert_that(output, has_key("hook")) - assert_that(output["hook"], has_entry("type", "on-approved")) - assert_that(output["hook"], has_entry("when", BEFORE)) - assert_that(output["hook"], has_entry( + assert_that(output, has_length(1)) + assert_that(output[0], has_key("hook")) + assert_that(output[0]["hook"], has_entry("type", "on-approved")) + assert_that(output[0]["hook"], has_entry("when", BEFORE)) + assert_that(output[0]["hook"], has_entry( "payload", all_of( has_entry(equal_to("workflow_object"), equal_to(workflow_object.model)), @@ -140,3 +145,56 @@ def test_shouldInvokeCallbackThatIsRegisteredWithoutInstanceWhenAnApprovingHappe ) )) + + def test_shouldInvokeCallbackForTheOnlyGivenApproval(self): + authorized_permission = PermissionObjectFactory() + authorized_user = UserObjectFactory(user_permissions=[authorized_permission]) + + state1 = StateObjectFactory(label="state1") + state2 = StateObjectFactory(label="state2") + state3 = StateObjectFactory(label="state3") + + content_type = ContentType.objects.get_for_model(BasicTestModel) + workflow = WorkflowFactory(initial_state=state1, content_type=content_type, field_name="my_field") + meta1 = TransitionApprovalMetaFactory.create( + workflow=workflow, + source_state=state1, + destination_state=state2, + priority=0, + permissions=[authorized_permission] + ) + + TransitionApprovalMetaFactory.create( + workflow=workflow, + source_state=state2, + destination_state=state3, + priority=0, + permissions=[authorized_permission] + ) + + TransitionApprovalMetaFactory.create( + workflow=workflow, + source_state=state3, + destination_state=state1, + priority=0, + permissions=[authorized_permission] + ) + + workflow_object = BasicTestModelObjectFactory() + workflow_object.model.river.my_field.approve(as_user=authorized_user) + workflow_object.model.river.my_field.approve(as_user=authorized_user) + workflow_object.model.river.my_field.approve(as_user=authorized_user) + + assert_that(TransitionApproval.objects.filter(meta=meta1), has_length(2)) + first_approval = TransitionApproval.objects.filter(meta=meta1, iteration=0).first() + assert_that(first_approval, is_not(none())) + + self.hook_pre_approve(workflow, meta1, transition_approval=first_approval) + + output = self.get_output() + assert_that(output, none()) + + workflow_object.model.river.my_field.approve(as_user=authorized_user) + + output = self.get_output() + assert_that(output, none()) diff --git a/river/tests/hooking/test__completed_hooking.py b/river/tests/hooking/test__completed_hooking.py index 8485cf4..889d8d8 100644 --- a/river/tests/hooking/test__completed_hooking.py +++ b/river/tests/hooking/test__completed_hooking.py @@ -1,5 +1,5 @@ from django.contrib.contenttypes.models import ContentType -from hamcrest import assert_that, equal_to, has_entry, none, has_key +from hamcrest import assert_that, equal_to, has_entry, none, has_key, has_length from river.models.factories import PermissionObjectFactory, StateObjectFactory, WorkflowFactory, TransitionApprovalMetaFactory, UserObjectFactory from river.models.hook import AFTER @@ -51,10 +51,11 @@ def test_shouldInvokeCallbackThatIsRegisteredWithInstanceWhenFlowIsComplete(self assert_that(workflow_object.model.my_field, equal_to(state3)) output = self.get_output() - assert_that(output, has_key("hook")) - assert_that(output["hook"], has_entry("type", "on-complete")) - assert_that(output["hook"], has_entry("when", AFTER)) - assert_that(output["hook"], has_entry( + assert_that(output, has_length(1)) + assert_that(output[0], has_key("hook")) + assert_that(output[0]["hook"], has_entry("type", "on-complete")) + assert_that(output[0]["hook"], has_entry("when", AFTER)) + assert_that(output[0]["hook"], has_entry( "payload", has_entry(equal_to("workflow_object"), equal_to(workflow_object.model)) )) @@ -99,10 +100,11 @@ def test_shouldInvokeCallbackThatIsRegisteredWithoutInstanceWhenFlowIsComplete(s assert_that(workflow_object.model.my_field, equal_to(state3)) output = self.get_output() - assert_that(output, has_key("hook")) - assert_that(output["hook"], has_entry("type", "on-complete")) - assert_that(output["hook"], has_entry("when", AFTER)) - assert_that(output["hook"], has_entry( + assert_that(output, has_length(1)) + assert_that(output[0], has_key("hook")) + assert_that(output[0]["hook"], has_entry("type", "on-complete")) + assert_that(output[0]["hook"], has_entry("when", AFTER)) + assert_that(output[0]["hook"], has_entry( "payload", has_entry(equal_to("workflow_object"), equal_to(workflow_object.model)) )) diff --git a/river/tests/hooking/test__transition_hooking.py b/river/tests/hooking/test__transition_hooking.py index 895f30a..410451c 100644 --- a/river/tests/hooking/test__transition_hooking.py +++ b/river/tests/hooking/test__transition_hooking.py @@ -1,5 +1,5 @@ from django.contrib.contenttypes.models import ContentType -from hamcrest import equal_to, assert_that, has_entry, none, all_of, has_key +from hamcrest import equal_to, assert_that, has_entry, none, all_of, has_key, has_length, is_not from river.models import TransitionApproval from river.models.factories import PermissionObjectFactory, UserObjectFactory, StateObjectFactory, WorkflowFactory, TransitionApprovalMetaFactory @@ -56,10 +56,11 @@ def test_shouldInvokeCallbackThatIsRegisteredWithInstanceWhenTransitionHappens(s last_approval = TransitionApproval.objects.get(object_id=workflow_object.model.pk, source_state=state2, destination_state=state3) output = self.get_output() - assert_that(output, has_key("hook")) - assert_that(output["hook"], has_entry("type", "on-transit")) - assert_that(output["hook"], has_entry("when", AFTER)) - assert_that(output["hook"], has_entry( + assert_that(output, has_length(1)) + assert_that(output[0], has_key("hook")) + assert_that(output[0]["hook"], has_entry("type", "on-transit")) + assert_that(output[0]["hook"], has_entry("when", AFTER)) + assert_that(output[0]["hook"], has_entry( "payload", all_of( has_entry(equal_to("workflow_object"), equal_to(workflow_object.model)), @@ -111,10 +112,11 @@ def test_shouldInvokeCallbackThatIsRegisteredWithoutInstanceWhenTransitionHappen last_approval = TransitionApproval.objects.get(object_id=workflow_object.model.pk, source_state=state2, destination_state=state3) output = self.get_output() - assert_that(output, has_key("hook")) - assert_that(output["hook"], has_entry("type", "on-transit")) - assert_that(output["hook"], has_entry("when", AFTER)) - assert_that(output["hook"], has_entry( + assert_that(output, has_length(1)) + assert_that(output[0], has_key("hook")) + assert_that(output[0]["hook"], has_entry("type", "on-transit")) + assert_that(output[0]["hook"], has_entry("when", AFTER)) + assert_that(output[0]["hook"], has_entry( "payload", all_of( has_entry(equal_to("workflow_object"), equal_to(workflow_object.model)), @@ -122,3 +124,56 @@ def test_shouldInvokeCallbackThatIsRegisteredWithoutInstanceWhenTransitionHappen ) )) + + def test_shouldInvokeCallbackForTheOnlyGivenIteration(self): + authorized_permission = PermissionObjectFactory() + authorized_user = UserObjectFactory(user_permissions=[authorized_permission]) + + state1 = StateObjectFactory(label="state1") + state2 = StateObjectFactory(label="state2") + state3 = StateObjectFactory(label="state3") + + content_type = ContentType.objects.get_for_model(BasicTestModel) + workflow = WorkflowFactory(initial_state=state1, content_type=content_type, field_name="my_field") + meta1 = TransitionApprovalMetaFactory.create( + workflow=workflow, + source_state=state1, + destination_state=state2, + priority=0, + permissions=[authorized_permission] + ) + + TransitionApprovalMetaFactory.create( + workflow=workflow, + source_state=state2, + destination_state=state3, + priority=0, + permissions=[authorized_permission] + ) + + TransitionApprovalMetaFactory.create( + workflow=workflow, + source_state=state3, + destination_state=state1, + priority=0, + permissions=[authorized_permission] + ) + + workflow_object = BasicTestModelObjectFactory() + workflow_object.model.river.my_field.approve(as_user=authorized_user) + workflow_object.model.river.my_field.approve(as_user=authorized_user) + workflow_object.model.river.my_field.approve(as_user=authorized_user) + + assert_that(TransitionApproval.objects.filter(meta=meta1), has_length(2)) + first_approval = TransitionApproval.objects.filter(meta=meta1, iteration=0).first() + assert_that(first_approval, is_not(none())) + + self.hook_pre_transition(workflow, state1, state2, iteration=0) + + output = self.get_output() + assert_that(output, none()) + + workflow_object.model.river.my_field.approve(as_user=authorized_user) + + output = self.get_output() + assert_that(output, none())