improve API for working with approval processes

signoffs/core/approvals.py

@@ -100,9 +100,11 @@ def ready_to_approve(self, approval):
         return not approval.is_approved() and approval.is_complete()
 
     def approve_if_ready(self, approval, commit=True):
-        """Approve and save the approval is it meets all ready conditions"""
+        """Approve and save the approval is it meets all ready conditions; return True iff this was done. """
         if self.ready_to_approve(approval):
             self.approve(approval, commit)
+            return True
+        return False
 
     def approve(self, approval, commit=True, **kwargs):
         """
@@ -400,6 +402,10 @@ def has_signoff(self, signoff_id_or_type):
 
     # Approval Business Logic Delegation
 
+    def is_signable(self, by_user=None):
+        """Return True iff this approval is signable"""
+        return self.logic.is_signable(self, by_user)
+
     def can_sign(self, user, signoff=None):
         """
         return True iff the given user can sign given signoff on this approval,
@@ -423,9 +429,9 @@ def approve(self, commit=True, **kwargs):
         """
         return self.logic.approve(self, commit=commit, **kwargs)
 
-    def is_revokable(self):
-        """Return True iff this approval is in a state it could be revoked"""
-        return self.logic.is_revokable(self)
+    def is_revokable(self, by_user=None):
+        """Return True iff this approval is in a state it could be revoked, optionally by given user"""
+        return self.logic.is_revokable(self, by_user)
 
     @classmethod
     def is_permitted_revoker(cls, user):
@@ -523,7 +529,7 @@ def next_signoffs(self, for_user=None):
         Most applications will define custom business logic for ordering signoffs, restricting duplicate signs, etc.
             - ideally, use ApprovalLogic and SigningOrder to handle these, but this gives total control!
         """
-        if not self.logic.is_signable(self, for_user):
+        if not self.is_signable(for_user):
             return []
         signoffs = (
             signoff(stamp=self.stamp, subject=self, user=for_user)

signoffs/core/models/fields.py

@@ -333,32 +333,17 @@ def __get__(self, instance, owner=None):
         # Can't validate until models are fully loaded :-/
         self._validate_related_model(self.stamp_field, self.approval_type)
 
-        base_approval_type = registry.get_approval_type(self.approval_type)
-
-        class BaseRelatedApproval(base_approval_type):
-            """An Approval that is aware of a "reverse" one-to-one relation to the instance object"""
-
-            stamp_field = self.stamp_field
-
-            def save(self, *args, **kwargs):
-                """Save the related stamp and then the instance relation"""
-                approval = super().save(*args, **kwargs)
-                if not getattr(instance, self.stamp_field.name) == approval.stamp:
-                    setattr(instance, self.stamp_field.name, approval.stamp)
-                    instance.save()
-                return approval
-
-        RelatedApproval = type(
-            f"Related{base_approval_type.__name__}", (BaseRelatedApproval,), {}
-        )
+        approval_type = registry.get_approval_type(self.approval_type)
 
         if not instance:
-            return base_approval_type
+            return approval_type
         else:
             stamp = getattr(instance, self.stamp_field.name)
-            approval = RelatedApproval(stamp=stamp, subject=instance)
+            approval = approval_type(stamp=stamp, subject=instance)
             if not stamp:
                 approval.save()
+                setattr(instance, self.stamp_field.name, approval.stamp)
+                instance.save()
             setattr(instance, self.accessor_attr, approval)
             return approval
 

signoffs/core/process.py

@@ -239,6 +239,9 @@ def _validate_registry(self):
 
     # access to approval transition sequencing
 
+    def __iter__(self):
+        return iter(self.get_all_approvals())
+
     def get_all_approvals(self):
         """Return list of all Approval instances defined in this sequence for the process_model"""
         return list(self.seq.values())
@@ -279,6 +282,10 @@ def get_next_available_approval(self):
         except IndexError:
             return None
 
+    def is_signable_approval(self, approval):
+        """Return True iff the given approval is signable within the process"""
+        return approval in self.get_available_approvals()
+
     def get_revokable_approvals(self):
         """Return list of approvals that can be revoked based on available state transitions"""
         return [
@@ -751,7 +758,7 @@ def approve_the_thing(self, approval, ...):
 
         def approval_transition_decorator(transition_method):
             """Decorate & register a transition_method with the fsm_decorator + logic to complete approval"""
-            return register(fsm_decorator(self.do_approval(transition_method)))
+            return register(self.do_approval(fsm_decorator(transition_method)))
 
         return approval_transition_decorator
 
@@ -793,7 +800,7 @@ def revoke_the_thing(self, approval, ...):
 
         def revoke_transition_decorator(transition_method):
             """Decorate & register a transition_method with the fsm_decorator + logic to complete approval"""
-            return register(fsm_decorator(self.do_revoke(transition_method)))
+            return register(self.do_revoke(fsm_decorator(transition_method)))
 
         return revoke_transition_decorator
 

signoffs/core/tests/test_view_actions.py

@@ -493,7 +493,9 @@ def get_data(self, signoff=None, stamp=None, signed_off="on"):
 
     def test_create(self):
         action = actions.ApprovalProcessUserActions(
-            self.user, self.get_data(), self.approval_process
+            self.user, self.get_data(),
+            approval_process=self.approval_process,
+            approval=self.approval_process.get_next_available_approval()
         )
         self.assertEqual(type(action.signoff_actions), actions.BasicUserSignoffActions)
         self.assertEqual(
@@ -503,22 +505,28 @@ def test_create(self):
 
     def test_sign_signoff(self):
         action = actions.ApprovalProcessUserActions(
-            self.user, self.get_data(), self.approval_process
+            self.user, self.get_data(),
+            approval_process=self.approval_process,
+            approval=self.approval_process.get_next_available_approval()
         )
         self.assertTrue(action.sign_signoff())
         self.assertTrue(action.signoff.is_signed())
         self.assertEqual(action.approval.signoffs.count(), 1)
 
     def test_sign_to_approval(self):
         action = actions.ApprovalProcessUserActions(
-            self.user, self.get_data(), self.approval_process
+            self.user, self.get_data(),
+            approval_process=self.approval_process,
+            approval=self.approval_process.get_next_available_approval()
         )
         action.sign_signoff()
         self.assertFalse(action.approval.is_approved())
         self.assertEqual(self.fsm.state, self.fsm.States.STATE0)
 
         action = actions.ApprovalProcessUserActions(
-            self.user, self.get_data(), self.approval_process
+            self.user, self.get_data(),
+            approval_process=self.approval_process,
+            approval=self.approval_process.get_next_available_approval()
         )
         action.sign_signoff()
         self.assertTrue(action.approval.is_approved())
@@ -528,14 +536,18 @@ def test_sign_to_approval2(self):
         self.fsm.sign_and_approve(self.user)
         self.assertEqual(self.fsm.state, self.fsm.States.STATE1)
         action = actions.ApprovalProcessUserActions(
-            self.user, self.get_data(), self.approval_process
+            self.user, self.get_data(),
+        approval_process = self.approval_process,
+        approval = self.approval_process.get_next_available_approval()
         )
         action.sign_signoff()
         self.assertFalse(action.approval.is_approved())
         self.assertEqual(self.fsm.state, self.fsm.States.STATE1)
 
         action = actions.ApprovalProcessUserActions(
-            self.user, self.get_data(), self.approval_process
+            self.user, self.get_data(),
+            approval_process=self.approval_process,
+            approval=self.approval_process.get_next_available_approval()
         )
         action.sign_signoff()
         self.assertTrue(action.approval.is_approved())
@@ -556,11 +568,15 @@ def revoke_data(self, signoff=None):
 
     def test_is_valid_approval_revoke_request(self):
         action = actions.ApprovalProcessUserActions(
-            self.user, self.get_data(), self.approval_process
+            self.user, self.get_data(),
+            approval_process=self.approval_process,
+            approval=self.approval_process.get_next_available_approval()
         )
         action.sign_signoff()
         r_action = actions.ApprovalProcessUserActions(
-            self.user, {}, self.approval_process
+            self.user, {},
+            approval_process=self.approval_process,
+            approval=self.approval_process.get_next_available_approval()
         )
         self.assertFalse(r_action.is_valid_approval_revoke_request())
 
@@ -569,18 +585,22 @@ def test_is_valid_approval_revoke_request(self):
         r_action = actions.ApprovalProcessUserActions(
             self.user,
             {},
-            self.approval_process,
+            approval_process=self.approval_process,
             approval=self.approval_process.get_approved_approvals()[-1],
         )
         self.assertTrue(r_action.is_valid_approval_revoke_request())
 
     def test_revoke_signoff(self):
         action = actions.ApprovalProcessUserActions(
-            self.user, self.get_data(), self.approval_process
+            self.user, self.get_data(),
+            approval_process=self.approval_process,
+            approval=self.approval_process.get_next_available_approval()
         )
         action.sign_signoff()
         r_action = actions.ApprovalProcessUserActions(
-            self.user, self.revoke_data(action.signoff), self.approval_process
+            self.user, self.revoke_data(action.signoff),
+            approval_process=self.approval_process,
+            approval=self.approval_process.get_next_available_approval()
         )
         self.assertTrue(r_action.revoke_signoff())
         self.assertFalse(r_action.signoff.is_signed())
@@ -591,7 +611,9 @@ def test_approve_request(self):
         approval.stamp.approved = False
         self.fsm.state = self.fsm.States.STATE0
         action = actions.ApprovalProcessUserActions(
-            self.user, {}, self.approval_process
+            self.user, {},
+            approval_process=self.approval_process,
+            approval=self.approval_process.get_next_available_approval()
         )
         action.approve()
         self.assertTrue(approval.is_approved())
@@ -605,7 +627,7 @@ def test_revoke_approve_request(self):
         action = actions.ApprovalProcessUserActions(
             self.user,
             {},
-            self.approval_process,
+            approval_process=self.approval_process,
             approval=self.approval_process.get_approved_approvals()[-1],
         )
         self.assertTrue(action.is_valid_approval_revoke_request())
@@ -617,7 +639,7 @@ def test_revoke_approve_request(self):
         action = actions.ApprovalProcessUserActions(
             self.user,
             {},
-            self.approval_process,
+            approval_process=self.approval_process,
             approval=self.approval_process.get_approved_approvals()[-1],
         )
         self.assertTrue(action.revoke_approval())

signoffs/core/views/actions.py

@@ -410,7 +410,9 @@ class BasicUserApprovalActions:
     """
     Concrete implementation that handles simple approvals that don't trigger any follow-up actions
 
-    An extensible base class for extending this simple case that
+    An extensible base class for extending this simple case that...
+    provides a default post_signoff_hook to approve approval when ready and adds it to default committer
+    provides a stub for post_revoke_hook adds it to default committer
     implements `SignoffRequestActions` and `ApprovalRequestActions` Protocols
     Facade: delegation & orchestration of signoff_actions_class, and signoff & approval business logic.
     """
@@ -454,8 +456,9 @@ def __init__(
             verify_signet=get_verify_signet(kwargs),
             verify_stamp=self.verify_stamp,
         )
-        committer = committer or self.committer_class(
-            user, post_signoff_hook=lambda s: self.approve()
+        committer = committer or self.committer_class(user,
+            post_signoff_hook=self.process_signoff,
+            post_revoke_hook=self.process_revoked_signoff
         )
         forms = self.signoff_actions_class.form_handler_class(
             self.data, signoff_subject=self.approval
@@ -510,6 +513,11 @@ def revoke_signoff(self, commit=True):
 
     # ApprovalActions Protocol
 
+    def process_signoff(self, signoff):
+        """Trigger an approval if the signoff completed approval's signing order"""
+        if self.approval.ready_to_approve():
+            self.approve()
+
     def is_valid_approve_request(self):
         """Return True iff the approval is complete and ready to be approved"""
         return self.verify_stamp() and self.approval.ready_to_approve()
@@ -523,6 +531,10 @@ def approve(self, commit=True):
         self.approval.approve(commit=commit)
         return self.approval.is_approved()
 
+    def process_revoked_signoff(self, signoff):
+        """Take additional approval-related actions when given signoff has been revoked"""
+        pass
+
     def is_valid_approval_revoke_request(self):
         """Return True iff user is allowed to revoke the given approval"""
         return self.verify_stamp() and self.approval.can_revoke(self.user)
@@ -629,7 +641,7 @@ def __init__(
         user,
         data,
         approval_process,
-        approval=None,
+        approval,
         signoff_actions: SignoffRequestActions = None,
         validator: SignoffValidator = None,
         committer: SignoffCommitter = None,
@@ -646,8 +658,8 @@ def __init__(
         :param kwargs: additional parameters passed through to `BasicUserApprovalActions` initializer
         """
         self.approval_process = approval_process
-        self.approval = approval or self.approval_process.get_next_available_approval()
-        if not self.approval.subject:
+        self.approval = approval
+        if approval and not self.approval.subject:
             self.approval.subject = self.approval_process.process_model
         validator = validator or self.validator_class(
             user,

signoffs/static/css/signoffs.css

@@ -19,8 +19,8 @@
 .signoffs.signet .signet-field-group .signet-field-label {
     position: absolute;
     left: 0;
-    top: 2em;
-    margin: 0 5%;
+    top: 2.4em;
+    width: 100%;
     color: grey;
     font-variant: small-caps;
     line-height: 1em;

signoffs/templates/signoffs/process/approval_process.html

@@ -2,8 +2,13 @@
 {% load signoff_tags %}
 
 <div class="signoffs approval_process">
-  {% for approval in process.get_all_approvals %}
+  {# All the approved ones... #}
+  {% for approval in process.get_approved_approvals %}
     {% render_process_approval process approval %}
   {% endfor %}
+  {# ... and the next one in sequence... #}
+  {% with approval=process.get_next_approval %}{% if approval %}
+    {% render_process_approval process approval %}
+  {% endif %}{% endwith %}
 </div>
 

signoffs/templatetags/signoff_tags.py

@@ -57,6 +57,8 @@ def render_approval_signoff(
     user = _get_request_user(context, **kwargs)
     if not approval.can_revoke_signoff(signoff_instance, user):
         kwargs["show_revoke"] = False
+    if not approval.can_sign(user, signoff_instance):
+        kwargs["show_form"] = False
     return render_signoff(context, signoff_instance, action=action, **kwargs)
 
 
@@ -79,6 +81,8 @@ def render_process_approval(context, approval_process, approval_instance, **kwar
     """
     if not approval_process.is_revokable_approval(approval_instance):
         kwargs["show_revoke"] = False
+    if not approval_process.is_signable_approval(approval_instance):
+        kwargs["render_signoff_forms"] = False
     return approval_instance.render(**kwargs, context=context)