Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement approvals using form_stages construct within __config__ #62

Closed
signebedi opened this issue Mar 25, 2024 · 19 comments
Closed

Implement approvals using form_stages construct within __config__ #62

signebedi opened this issue Mar 25, 2024 · 19 comments

Comments

@signebedi
Copy link
Owner

signebedi commented Mar 25, 2024

The objective is to support arbitrary, group-based approval chains (eg. on submission, form X gets routed to group Y for approval, and then to group Z; if any reviewer returns the form, it goes back to the original creator). We may also be able to add assigned roles - the idea being that a proponent can identify an approver, and/or an organizational unit can identify specific individuals to play certain roles. These are general thoughts on the routing of forms for approval, disapproval, and return.

The question then becomes: are we measuring this the right way? In a manner that is sufficient? We have three placeholder fields in the document database metadata fields:

  1. _approval_signature
  2. _approved
  3. _approved_by

These are written to using the api approve form route, see #58. We also have a planned "approval chains" table to the relational data model, see #39. This should be able to measure the three kinds of situations identified in the beginning.

@signebedi
Copy link
Owner Author

One question is how to represent multiple stages of review in the document database model. I think that we can set it so that only one signature (the most recent) shows up. Older signatures will still show up in the journal.

@signebedi
Copy link
Owner Author

Ok, as I think through things, it may make sense to use the following fields in the document database metadata section:

_latest_approval_signature: str (encrypted string using app secret key?)
_approval_status: str
_approved_by: list
_current_approval_stage:str

The _approval_status should be an enum eg. of "Submitted for approval", "Currently being reviewed", or "Approved" / "Denied" / "Returned" + substantive reviewer comments.

The _approval_status should map perfectly to a corresponding named stage in the ApprovalChains table, and the _approved_by should be a list that gets appended with approver names.

@signebedi
Copy link
Owner Author

signebedi commented Mar 26, 2024

Additionally, the ApprovalChains to define behavior on Denial and behavior on Return. For example, denial from some arbitrary stage in the chain COULD send the form back to the initiator, wiping out past approvals. Or, it could simply send it back to the last stage. These should all be behavior that can be defined in the ApprovalChains table.

So the relational data model might look something like:

  1. ID: A unique identifier for each approval chain configuration (Primary Key).
  2. Name: A descriptive name for the approval chain (e.g., "Expense Report Approval").
  3. Stage: A named stage in the approval process (e.g., "Department Review").
  4. Sequence: An integer representing the order of this stage within the approval process. This should be an int, I think, but might be redundant with Stage?
  5. ApproverRole: The role or group required to approve at this stage. Groups are straightforward. But what if you want a specific reviewal by users based on defined roles / relationships to the initiating user?
  6. OnApprove: Action or reference to the next stage upon approval (could be the next Sequence number or a specific stage name).
  7. OnDeny: Defines the action taken when a denial occurs at this stage (e.g., "Reset", "Return to Previous", specific stage name/ID).
  8. OnReturn: Defines the action taken when a document is returned for revision (e.g., "Return to Initiator", "Return to Previous", specific stage name/ID).
  9. CommentsRequired: A boolean indicating whether comments are required for approval, denial, or return actions.

The OnDeny and OnReturn fields should be flexible to accommodate different processes. For example, some documents may need to go back several stages for a comprehensive review, while others might only require minor revisions and thus, a step back or direct feedback to the initiator would suffice.

*** We should add a generate_approval_chain (form_name) factory func / table method that returns some kind of representation of the approval chain for a given form.

*** How do we represent multiple chains for a given form, eg. when the initiator is from Group 1 vs. Group 2? Maybe it makes sense to add an AppliesTo or AppliesWhen condition to the data model, which defaults to ALL / ALWAYS, but can also define more scoped application eg. to a list of groups (so, maybe a nullable foreign key > Group.name that, when null, applies to all users...)?

This also raises a consideration as to how to track approval history in the document data model. Is the journal tracking method sufficient? Or should we add eg. an _approval_history field. And how will actions taken based on the OnDeny and OnReturn configurations will be recorded in the document's _approval_history? We will want to capture approvals, sure, but also the denials and returns, along with any comments provided, to maintain a complete audit trail.

This also places pretty significant pressure on the application logic to synchronize approval status with the relational database. What if there is a change to the ApprovalChain by an admin? We need to think through that behavior. Does it render previously approved forms "unapproved"? (bad idea, from a bureaucratic perspective!) Is there a background sync process to ensure changes in the approval chain (especially denials and returns) properly update the _approval_status and potentially _current_approval_stage in the document's metadata? The bottom line is that we need to make sure that the document's status always reflects its actual position in the approval process, unless it has already been fully approved.

@signebedi
Copy link
Owner Author

signebedi commented Mar 26, 2024

As I think through things, I am starting to wonder whether we should just represent all approvals and signatures as "signatures", and then user the ApprovalChains (SignatureChains?) table to represent ordering of signatures on a form / doc in order to complete it. This shifts the process from a linear / sequential one, to one that has the possibility of being more flat and simultaneous in the manner of signature/approval aggregation. Just a thought, which needs to be processed a little more before we move to execute.

The classic example is mortgage paperwork, and contracts more generally, which need multiple signatures from parties on equal footing.

A minor problem is that this approach will simplify data structures but place greater pressure on application logic to deconflict / reconstruct...

@signebedi
Copy link
Owner Author

With the introduction of user-to-user relationships in #173, we can shift how we think about the approval table. For example, we can now make the "approval type" one of an enumerated list of options including "static", "group_based", and "relationship_based". We should also revise how signatures are stored in the form data - read: consolidate signatures into a single object - before we proceed.

@signebedi
Copy link
Owner Author

signebedi commented Apr 16, 2024

PXL_20240416_155731319

This captures much of my thinking at this point. The signatures field becomes a list of tuples containing the signature, username, timestamp, and a role_id. The role_id links to the unique identifier of a SignatureChains table entry. We also capture signatures by the form owner using this method, meaning that we get rid of the sign_own group privilege.

Then, we modify the RESTful API to include two validation routes: validate_all/{form_name}/{document_id} and validate_one/{form_name}/{document_id}/{role_id}...

@signebedi
Copy link
Owner Author

Right now, we've devised a signature roles table, but we are not actually tracking signatures required as part of the data model. Doing so would make "pushes" to the next level of review less computationally costly and complex. But, we would also more closely embed the document database with the relational database at the expense of separation of concerns principles and, in the long-run, maintainability.

Part of the question we need to ask here is what the scalability of an application like this is intended to be. Thousands of users? Tens of thousands? My hunch is that a maintainable code base for a simple form management app for small organizations is the way to go. On those grounds, then, we would prioritize maintainable, if somewhat inefficient, code.

@signebedi
Copy link
Owner Author

signebedi commented Jul 29, 2024

Now that we've added the __config__ to the form config in #315, we have an option to represent this in the form config itself. This comes with a number of tradeoffs. However, the advantage is that is the more intuitive place to put it, and requires the least custom training.

form_name:
  __config__:
    form_stages:
      initial_stage:
        label: some stage name
        action: approval
        initial_stage: true
        required: true
        method: group
        target: group_name
        on_approve: send_to_some_stage_name
        on_deny: send_to_some_stage_name
        on_pushback: send_to_some_stage_name

Supported actions include sign, approve, approved, and denied. These are rather akin to "gateways" in the sense that they describe a step through which the form will be processed, with "approved" and "denied" being terminal stages.... Alas, this leaves much to be desired, but it is a start. The stage name should be computer-friendly, but the label can override this - the label is what the end user will see.

@signebedi signebedi changed the title Think through the approval process Implement approvals using form_stages construct within __config__ Aug 3, 2024
@signebedi
Copy link
Owner Author

signebedi commented Aug 3, 2024

How do we need to represent this in the form config?

"metadata": {
  "form_stage": "current_stage_name",
  "signatures": [
    (signature, username, stage_name, timestamp),
    (signature, username, stage_name, timestamp),
  ]
}

I need to encode these tuples in json, See https://stackoverflow.com/a/715601.

@signebedi
Copy link
Owner Author

signebedi commented Aug 21, 2024

We need to fully implement the form approval API and UI routes. It's not clear which of those are within scope for this issue ... we ought to set some guardrails, but that might be a vanity this late in the game, at least as far as it relates to the implementation of an approval process. We have removed the "Sign Form" option from the read_one UI and added a boilerplate UI page to show all the forms needing review by the current user via a datatable, as well as a route that extends read_one to include an approval interface. This is clunky because it retains all of the dropdown options of the read_one, but saves us from having two separate files with essentially the same information. We can probably solve this down the road by creating an upstream jinja2 blueprint that read_one and individual review and approval pages inherit from, but which allows us eg. exclude the drop-down options from the latter....

We also need to implement checking on the review and approval individual page to ensure that it is timely and within scope for the current user's approval, or else return an empty content section (in the UI) or 404 (from the server side ... not recommended because it increases server/client coupling... see #329).

@signebedi
Copy link
Owner Author

Add parameter-based selective cache invalidation
This is necessary in order to have any semblance of useful caching as it relates to stored lists of documents at each stage.

@signebedi
Copy link
Owner Author

Implement relationship-based form approval
Currently, there are two ways to approve forms: using static and group based approval. These are straightforward enough to implement. Relationship-based approval is highly complex because of the reciprocal nature of relationships and because of how arbitrary they can be. This is a point in the application where the document and relational databases intermix, which is always a tricky business but especially so when there are additional layers of complexity (relationships) added on top of the intermixed concerns of the two data stores.

@signebedi
Copy link
Owner Author

Add form_stage instruction validation
We've settled on using the following rules to specify form stages:

form_name:
  __config__:
    form_stages:
      initial_stage:
        label: some stage name
        action: approval
        initial_stage: true
        required: true
        method: group
        target: group_name
        on_approve: send_to_some_stage_name
        on_deny: send_to_some_stage_name
        on_pushback: send_to_some_stage_name

Some of these can have default behavior (like required); most fields will need to be specified in order for the logic to work; some will need to have their types validated; others still will need to have their values validated: for example, on_approve, on_deny, and on_pushback all need to point to another, valid stage within the same form config...

@signebedi
Copy link
Owner Author

Re-implement unsign / remove signature API route
Adding an approval process necessitated the temporary deprecation of the unsign API route. This is a note to re-implement some API method for removing a signature from a document.

@signebedi
Copy link
Owner Author

Figure out how to handle pushbacks
When we implement the approval process, we don't have a way to handle pushbacks in the sense that, with denial and approvals, there are obvious terminal stages, or stages back to which a document can be sent. Whereas, for pushback, the theoretical intermediary stage that it needs to be sent back to would not entail the original user "approving" their form again. So, I suppose the trigger events of on_approve, on_disapprove, and on_pushback are insufficient for such cases, as they just require resubmission, presumably with edits. Maybe: we can send the document to the same stage, but provide instructions as a message? This likely means that signatures and messages need to be stored in the approval interface for a form, so others don't approve while another pushes back, eg. for group-based approval where multiple people can approve a document... so, I think a discrete pushback behavior would be needed here... maybe a bool config like "edit_and_resubmit" that sends back to the initial_stage once completed.

@signebedi
Copy link
Owner Author

Add support for reviewer comments
In the form metadata, add reviewer comments. These need not be part of the signature - especially since comments will usually be given when approval has been withheld... Further, they need not be retained, since the journal will store past comments for us. As such, a reviewer_comments that can be easily re-written & cleared on the next approval stage will probably do the trick. We will want to link these ostensibly to some type of email mechanism, too..

@signebedi
Copy link
Owner Author

Add stage_label and badge_color override option to form_stages
This way end users can specify how they want arbitrary form_stage badges to appear. For example, it can display a user-friendly title, and use a badge color by class, such as those described here: https://bootswatch.com/darkly. So: Primary Secondary Success Danger Warning Info Light Dark.

@signebedi
Copy link
Owner Author

Allow admins to set a form submission's stage manually
This is a bit tricky, but there are situations where admins should be able to manually set a form's stage based on those available for that specific form. Obviously, setting a form to approved manually will not come with any signatures... so this is more of a fix-in-place tool than a component of a sustainable workflow.

@signebedi
Copy link
Owner Author

Implement group permission for viewing form action needed
Service accounts need to be able to see actions needed on a form submission. It would be straightforward to implement this feature as an admin route, but generally service accounts will not be given admin permissions, cf. #329. This can be implemented by adding a new form permission called something like read_actions_needed. However, it may actually be that this is not the most effective way to implement this feature, given the likely business process that organizations will follow when aggregating action-needed data. Instead, we might want to consider expanding group permissions from just encapsulating forms, to also encapsulating users. For example: form:example_form:read_all could be one permission scope, whereas group:group_name:read_actions_needed could be an additional permission. Obviously, a group will be able to see its own actions needed... so this risks being a little clunky if we don't approach it carefully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant