add post signoff/revoke hooks to action committer.

signoffs/core/tests/test_view_actions.py

@@ -2,6 +2,7 @@
 App-independent tests for view.actions - no app logic
 """
 import django_fsm as fsm
+from django.db import transaction
 from django.db.models import Model, TextChoices
 from django.test import TestCase
 
@@ -66,6 +67,46 @@ def test_invalid_data(self):
         self.assertIsNone(bf)
 
 
+class SignoffCommitterTests(TestCase):
+    def setUp(self):
+        self.user = fixtures.get_user(perms=("add_signoff",))
+        self.signoff = signoff_type()
+
+    def test_create(self):
+        committer = actions.BasicSignoffCommitter(self.user)
+        self.assertEqual(committer.user, self.user)
+
+    def test_sign(self):
+        committer = actions.BasicSignoffCommitter(self.user)
+        committer.sign(self.signoff)
+        self.assertTrue(self.signoff.is_signed())
+
+    def test_revoke(self):
+        self.signoff.sign(self.user)
+        committer = actions.BasicSignoffCommitter(self.user)
+        committer.revoke(self.signoff)
+        self.assertFalse(self.signoff.is_signed())
+
+    def test_post_signoff_hook(self):
+        obj = lambda: None
+        obj.called = False
+        def hook(signoff):
+            obj.called = True
+        committer = actions.BasicSignoffCommitter(self.user, post_signoff_hook=hook)
+        committer.sign(self.signoff)
+        self.assertTrue(obj.called)
+
+    def test_post_revoke_hook(self):
+        obj = lambda: None
+        obj.called = False
+        def hook(signoff):
+            obj.called = True
+        self.signoff.sign(self.user)
+        committer = actions.BasicSignoffCommitter(self.user, post_revoke_hook=hook)
+        committer.revoke(self.signoff)
+        self.assertTrue(obj.called)
+
+
 class BasicUserSignoffActionsTests(TestCase):
     def setUp(self):
         self.user = fixtures.get_user(perms=("add_signoff",))
@@ -192,6 +233,62 @@ class ActionsApproval(BaseApproval):
     signing_order = so.SigningOrder(first_signoff, final_signoff)
 
 
+class ApprovalSignoffCommitterTests(TestCase):
+    def setUp(self):
+        self.user = fixtures.get_user(perms=("add_signoff",))
+        self.approval = ActionsApproval.create()
+        self.signoff = ActionsApproval.first_signoff(stamp=self.approval.stamp)
+
+    def test_create(self):
+        committer = actions.ApprovalSignoffCommitter(self.user, self.approval)
+        self.assertEqual(committer.user, self.user)
+        self.assertTrue(isinstance(committer.signoff_committer, actions.BasicSignoffCommitter))
+        self.assertEqual(committer.signoff_committer.user, self.user)
+
+    def test_sign(self):
+        committer = actions.ApprovalSignoffCommitter(self.user, self.approval)
+        committer.sign(self.signoff)
+        self.assertTrue(self.signoff.is_signed())
+
+    def test_revoke(self):
+        self.signoff.sign(self.user)
+        committer = actions.ApprovalSignoffCommitter(self.user, self.approval)
+        committer.revoke(self.signoff)
+        self.assertFalse(self.signoff.is_signed())
+
+    def test_default_post_signoff_hook(self):
+        """Default post_signoff_hook approves the approval when signing order complete"""
+        committer = actions.ApprovalSignoffCommitter(self.user, self.approval,
+                                                     post_signoff_hook=lambda s,a: self.approval.approve_if_ready())
+        committer.sign(self.approval.first_signoff(stamp=self.approval.stamp))
+        committer.sign(self.approval.final_signoff(stamp=self.approval.stamp))
+        self.assertTrue(self.approval.is_approved())
+
+    def test_post_signoff_hook(self):
+        """Custom post_signoff_hook runs in an atomic transaction to maintain integrity of triggered DB ops"""
+        obj = lambda: None
+        obj.called = 0
+        def hook(signoff, approval):
+            self.assertFalse(transaction.get_autocommit())
+            obj.called +=1
+        committer = actions.ApprovalSignoffCommitter(self.user, self.approval, post_signoff_hook=hook)
+        committer.sign(self.approval.first_signoff(stamp=self.approval.stamp))
+        committer.sign(self.approval.final_signoff(stamp=self.approval.stamp))
+        self.assertFalse(self.approval.is_approved())
+        self.assertEqual(obj.called, 2)
+
+    def test_post_revoke_hook(self):
+        obj = lambda: None
+        obj.called = False
+        def hook(signoff, approval):
+            self.assertFalse(transaction.get_autocommit())
+            obj.called = True
+        self.signoff.sign(self.user)
+        committer = actions.ApprovalSignoffCommitter(self.user, self.approval, post_revoke_hook=hook)
+        committer.revoke(self.signoff)
+        self.assertTrue(obj.called)
+
+
 class BasicUserApprovalActionsTests(TestCase):
     def setUp(self):
         self.user = fixtures.get_user(perms=("add_signoff",))

signoffs/core/views/actions.py

@@ -102,14 +102,23 @@ class BasicSignoffCommitter:
     """
 
     user: User
+    """The user who is signing the signoff"""
+    post_signoff_hook: Callable[[AbstractSignoff], None] = lambda s: None
+    """A function that takes the signed signoff as argument, called in atomic transaction after signing"""
+    post_revoke_hook: Callable[[AbstractSignoff], None] = lambda s: None
+    """A function that takes the revoked signoff as argument, called in atomic transaction after revoking"""
 
     def sign(self, signoff: AbstractSignoff):
         """Sign and commit the signoff for given user - no validation, just do it!"""
-        signoff.sign(self.user)
+        with transaction.atomic():
+            signoff.sign(self.user)
+            self.post_signoff_hook(signoff)
 
     def revoke(self, signoff: AbstractSignoff):
         """Revoke the signoff for given user and commit changes to DB - no validation, just do it!"""
-        signoff.revoke(self.user)
+        with transaction.atomic():
+            signoff.revoke(self.user)
+            self.post_revoke_hook(signoff)
 
 
 class SignoffRequestFormHandler(Protocol):
@@ -360,24 +369,43 @@ def is_valid_revoke_request(self, signoff: AbstractSignoff) -> bool:
 
 
 @dataclass
-class ApprovalSignoffCommitter(BasicSignoffCommitter):
+class ApprovalSignoffCommitter:
     """
     Default commit logic for requests to Signoff on an Approval.
 
-    Approves and commits the approval if it is ready after the current signoff is committed.
+    Implements `SignoffCommitter` Protocol
+    Signoff on a signoff of this approval.
     :::{caution}
-    Potential sync. issue here if approval has cached / prefetched signoffs - don't do that!
+    Potential sync. issue if post_signoff_hook tries to approve the approval (i.e., the default hook!)
+    but approval has cached / prefetched signoffs - don't do that!
     :::
     """
 
+    user: User
+    """The user who is signing the signoff"""
     approval: AbstractApproval
-    approve_if_ready: Callable[[AbstractApproval], bool]
+    """The approval that will be signed off (or have signoff revoked)"""
+    signoff_committer: SignoffCommitter = None
+    """The SingoffComitter object used to do the actual signoff, None to use a `BasisSignoffCommitter`"""
+    post_signoff_hook: Callable[[AbstractSignoff, AbstractApproval], None] = lambda s,a: None
+    """A function that takes the signed signoff and approval as arguments, called in atomic transaction after signing"""
+    post_revoke_hook: Callable[[AbstractSignoff, AbstractApproval], None] = lambda s,a: None
+    """A function that takes the revoked signoff and approval as argument, called in atomic transaction after revoking"""
+
+    def __post_init__(self):
+        self.signoff_committer = self.signoff_committer or BasicSignoffCommitter(self.user)
 
     def sign(self, signoff: AbstractSignoff):
-        """Sign and commit the signoff, and approve approval if ready - no validation, just do it!"""
+        """Sign and commit the signoff, and call post_signoff_hook - no validation, just do it!"""
         with transaction.atomic():
-            super().sign(signoff)
-            self.approve_if_ready(self.approval)
+            self.signoff_committer.sign(signoff)
+            self.post_signoff_hook(signoff, self.approval)
+
+    def revoke(self, signoff: AbstractSignoff):
+        """Revoke the signoff and commit changes to DB - no validation, just do it!"""
+        with transaction.atomic():
+            self.signoff_committer.revoke(signoff)
+            self.post_revoke_hook(signoff, self.approval)
 
 
 class ApprovalRequestActions(Protocol):
@@ -428,8 +456,14 @@ class BasicUserApprovalActions:
     """
 
     validator_class: SignoffValidator = ApprovalSignoffValidator
+    """Default `validator` uses get_verify_signet and get_verify_stamp as verifiers"""
     committer_class: SignoffCommitter = ApprovalSignoffCommitter
+    """Default `committer` approves the approval when it is `ready_to_approve`
+       
+       Override `approve()` method if you just need to extend the post_signoff_hook action
+    """
     signoff_actions_class = BasicUserSignoffActions
+    """Default `signoff_actions` is constructed from the other components."""
 
     def __init__(
         self,
@@ -461,7 +495,7 @@ def __init__(
             verify_stamp=self.verify_stamp,
         )
         committer = committer or self.committer_class(
-            user, approval, approve_if_ready=lambda a: self.approve()
+            user, approval, post_signoff_hook=lambda s,a: self.approve()
         )
         forms = self.signoff_actions_class.form_handler_class(
             self.data, signoff_subject=self.approval