From 3836edcea358bb369dbffdf1b7ea35b4b35784dd Mon Sep 17 00:00:00 2001 From: powderflask Date: Sat, 11 Nov 2023 13:07:41 -0800 Subject: [PATCH] improve API for working with approval processes --- signoffs/core/approvals.py | 16 ++++-- signoffs/core/models/fields.py | 25 ++-------- signoffs/core/process.py | 11 +++- signoffs/core/tests/test_view_actions.py | 50 +++++++++++++------ signoffs/core/views/actions.py | 24 ++++++--- signoffs/static/css/signoffs.css | 4 +- .../signoffs/process/approval_process.html | 7 ++- signoffs/templatetags/signoff_tags.py | 4 ++ 8 files changed, 91 insertions(+), 50 deletions(-) diff --git a/signoffs/core/approvals.py b/signoffs/core/approvals.py index 383913d..8a1d20c 100644 --- a/signoffs/core/approvals.py +++ b/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) diff --git a/signoffs/core/models/fields.py b/signoffs/core/models/fields.py index da52d42..ef1c735 100644 --- a/signoffs/core/models/fields.py +++ b/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 diff --git a/signoffs/core/process.py b/signoffs/core/process.py index 0b4c236..8057b5a 100644 --- a/signoffs/core/process.py +++ b/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 diff --git a/signoffs/core/tests/test_view_actions.py b/signoffs/core/tests/test_view_actions.py index 898e823..ecae4e3 100644 --- a/signoffs/core/tests/test_view_actions.py +++ b/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,7 +505,9 @@ 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()) @@ -511,14 +515,18 @@ def test_sign_signoff(self): 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()) diff --git a/signoffs/core/views/actions.py b/signoffs/core/views/actions.py index 991af88..2ed4eac 100644 --- a/signoffs/core/views/actions.py +++ b/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, diff --git a/signoffs/static/css/signoffs.css b/signoffs/static/css/signoffs.css index 495f98c..60de7f9 100644 --- a/signoffs/static/css/signoffs.css +++ b/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; diff --git a/signoffs/templates/signoffs/process/approval_process.html b/signoffs/templates/signoffs/process/approval_process.html index 67da252..8e41ce2 100644 --- a/signoffs/templates/signoffs/process/approval_process.html +++ b/signoffs/templates/signoffs/process/approval_process.html @@ -2,8 +2,13 @@ {% load signoff_tags %}
- {% 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 %}
diff --git a/signoffs/templatetags/signoff_tags.py b/signoffs/templatetags/signoff_tags.py index d2d8f25..4c2c60d 100644 --- a/signoffs/templatetags/signoff_tags.py +++ b/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)