Approach to allow skipping authorization step per program

[Sebastian Brudziński]

Hi Mary Jo and dev group,

I wanted to talk about the approach for OLMIS-3782, which deals with allowing the authorization step to be skipped per program. I see two ways in which we could approach this.

If the authorization step is skipped we could either:

1. Make "submit" operation bring the requisition to SUBMITTED status. Now, for every action/validation that requires the requisition to be ready for approval, we would need to have two checks: either the requisition is AUTHORIZED or the requisition is SUBMITTED and the authorization step is skipped.

2. Make "submit" operation check whether the authorization is skipped, and if that's the case, bring it directly to AUTHORIZED status. All the actions and validation checks that require requisition ready for approval would still need to only check for this single status.

Do you have any preferences? My opinion is that the second option is much cleaner and less error-prone in the implementation. On the other hand, it may be a little confusing for the end users that "submitting" a requisition brings it to the AUTHORIZED status (we could also potentially think of dropping the AUTHORIZED status and just having the IN_APPROVAL status).

I'm curious to hear your thoughts.

Best regards,
Sebastian.

--

Sebastian Brudziński
Software Developer / Team Leader
sbrud...@soldevelo.com

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

[Paweł Albecki]

Hi Sebastian,

I have a question. Is the new approach much different than current one based on .env variable SKIP_AUTHORIZATION? Maybe some code can be reused?

Regards,

Paweł.

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/41549af9-e454-7b11-afd4-b81b1aac58d5%40soldevelo.com.
For more options, visit https://groups.google.com/d/optout.

--

Paweł Albecki
Software Developer
palb...@soldevelo.com

[Łukasz Lewczyński]

My understanding is that It is easier to check the flag in a single place than checking it everywhere we need to. Also checking the flag is several places can create new bugs because there is possibility that someone will forget to add check for the flag. For me it is okay to move to the AUTHORIZED status but I think in this case we should add some information on submit modal to inform users why after submit a requisition has different status.

Łukasz Lewczyński
Software Developer
llewc...@soldevelo.com

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAAJzpfkTWit_CpbNJy7P8aJDmxtje_o9ao6kEfo5-K8d7w-LcQ%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Sebastian Brudziński]

Thanks for the answers.

Pawel, the first approach that I've described isn't very different than what we are currently doing with the setting from .env, however code-wise this is just a huge mess at the moment. There's no single place where all of this logic would be contained (we read directly from configuration settings in several places). Also, by briefly scanning the code, I believe some places don't even respect this setting (like requisitions for approval). That said, I believe no matter which approach we pick, we would like to write it mostly from scratch and do it in a DRY way.

Best regards,
Sebastian.

[Łukasz Lewczyński]

My another two coins:

because the flag is in the program how do we check if the flag is set? If we take a value directly from the DTO (like requisition.program.skipAuthorizeStep), a user would be able to change the endpoint behavior because of modifying the flag value. If we will ask the reference data service for program details, the endpoint performance will go down (because of additional request). It would be great to have a single method (like isApprovable) in the Requisition class to check if approve is available but because we have only program ID in this class we will probably need a method in service.

Łukasz Lewczyński
Software Developer
llewc...@soldevelo.com

[Sebastian Brudziński]

Hi Lukasz,

yes, we cannot read this from DTO. Besides, we don't even send a DTO on submit/approve - just the requisition ID. We will probably require a single, additional call to the reference data service and since it's just one call, I don't think it will affect performance that much.

Best regards,
Sebastian.

[Paweł Albecki]

It would be great if what Łukasz is raising could be made in domain layer. We can use e.g. Policy DDD buiding block for this (which is simple strategy pattern) or maybe just some transient "flag" field.

[Mary Jo Kochendorfer]

Sebastian,

Thanks for bring up the user’s potential confusion with the Authorized status. I agree it could be confusing and it brings up the paradigm around the requisition status/state not matching the “action” on the screen for the user.   How messy and complicated would it be to modify the current states? I’m a bit hesitant to change the state to In_approval given all the different dependencies. However, I do agree it is cleaner.

Error! Filename not specified.

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/41549af9-e454-7b11-afd4-b81b1aac58d5%40soldevelo.com.
For more options, visit https://groups.google.com/d/optout.

--

Paweł Albecki
Software Developer
palb...@soldevelo.com

Error! Filename not specified.

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAAJzpfkTWit_CpbNJy7P8aJDmxtje_o9ao6kEfo5-K8d7w-LcQ%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

 

Error! Filename not specified.

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

 

--

Sebastian Brudziński
Software Developer / Team Leader
sbrud...@soldevelo.com

Error! Filename not specified.

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

 

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

 

--

Paweł Albecki
Software Developer
palb...@soldevelo.com

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAAJzpf%3DmeKR1Nkve8P4Miygo%2B9AMTGQ1zxD5VB59iD1%2BJjeBPQ%40mail.gmail.com.

[Sebastian Brudziński]

Thanks for the answer Mary Jo.

We already do have the IN_APPROVAL status, so we wouldn't be adding anything new (and therefore the complexity isn't that huge). The requisition currently reaches this state when it requires more than one approval.

Essentially, when requisition is waiting for the first approval, it's marked as AUTHORIZED; when it's waiting for the 2nd, 3rd and nth approval it's marked as IN_APPROVAL. After the change that I've proposed, we wouldn't have AUTHORIZED state at all - we would only use IN_APPROVAL - no matter if we are waiting for the 1st, 2nd or nth approval.

If we go with the option I've proposed, we would also be reaching the IN_APROVAL state after submitting the requisition (provided the skip authorization is enabled for that program; otherwise it would still be just SUBMITTED).

I agree there's some risk involved, since requisition status has got a lot of dependencies, but if we are to make this change, I think it's better to do this now, when we are still a few months away from the next release.

FInally, I know the Malawi implementation depends on the AUTHORIZED status currently to distinguish between the first and the second approval, so small code changes would be required there as well (before incorporating the next release).

Best regards,

Sebastian.

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/41549af9-e454-7b11-afd4-b81b1aac58d5%40soldevelo.com.
For more options, visit https://groups.google.com/d/optout.

--

Paweł Albecki
Software Developer
palb...@soldevelo.com

Error! Filename not specified.
SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAAJzpf%3DmeKR1Nkve8P4Miygo%2B9AMTGQ1zxD5VB59iD1%2BJjeBPQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/3B986167-F0A4-4163-8BD1-A7D46FFAAFED%40villagereach.org.

For more options, visit https://groups.google.com/d/optout.

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com

[Brandon Bowersox-Johnson]

Sebastian,

 

Here is a variation on Option 2 that provides a bit more help for our end-users:

 

2B. Make "submit" operation check whether the authorization is skipped, and if that's the case, bring it directly to AUTHORIZED status. All the actions and validation checks that require requisition ready for approval would still need to only check for this single status. The UI will check the program setting and rename the button to say “Submit & Authorize” if this single action will be doing both steps. This will help end-users understand why the requisition appears as ‘Authorized’ after this.

 

Option 2 > 1

Generally, I agree that Option 2 (or this 2B) are much better than Option 1. We need to use this opportunity to make the code cleaner and DRY.

 

Setting in .env

Personally, I had no idea that we already had a .env variable SKIP_AUTHORIZATION. I see some code (in openlmis-requisition in src/main/java/org/openlmis/settings/service/ConfigurationSettingService.java) but I don’t know if it actually works. If we are making this configurable as a Program Setting, I think we should get rid of the .env variable. If we keep both, we need to clearly document which setting overrides the other one. (If .env says false but Program Setting says true, which way is it?) The .env file would be global, not per-program, so I think we should get rid of .env and make it a per-program setting with a toggle in the Program Settings Admin UI.

 

-Brandon

 

From: <openlm...@googlegroups.com> on behalf of Mary Jo Kochendorfer <maryjo.ko...@villagereach.org>
Date: Monday, December 11, 2017 at 12:47 PM
To: Paweł Albecki <palb...@soldevelo.com>, Łukasz Lewczyński <llewc...@soldevelo.com>
Cc: Sebastian Brudziński <sbrud...@soldevelo.com>, "openlm...@googlegroups.com" <openlm...@googlegroups.com>
Subject: Re: [openlmis-dev] Approach to allow skipping authorization step per program

 

Sebastian,

[Sebastian Brudziński]

Hi Brandon,

yes, we can definitely change the button message to "Submit and Authorize" if we decide to go with option 2 and want to keep the "AUTHORIZED" status by all means.

About the setting in .env - the plan was to remove that setting and any references to it from the start. I just noticed it isn't a part of the ticket description though, so I have just added that.

It sounds like we are all leaning towards the second option. The only outstanding question is whether we want to remove or keep the AUTHORIZED status.

Thanks for all the replies!

Best regards,

Sebastian.

--

You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/ADD17FE1-F245-4BDF-B482-4906497A62DA%40villagereach.org.

For more options, visit https://groups.google.com/d/optout.

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com

[Paweł Albecki]

I doubt that we need both environment and program settings unless someone really ask for this.

I would vote for get rid of Authorized status. Does someone remember why Authorized wasn't just changed to In Approval in the first place and instead new status was added?

On Mon, Dec 11, 2017 at 9:06 PM, Sebastian Brudziński <sbrud...@soldevelo.com> wrote:

Hi Brandon,

yes, we can definitely change the button message to "Submit and Authorize" if we decide to go with option 2 and want to keep the "AUTHORIZED" status by all means.

About the setting in .env - the plan was to remove that setting and any references to it from the start. I just noticed it isn't a part of the ticket description though, so I have just added that.

It sounds like we are all leaning towards the second option. The only outstanding question is whether we want to remove or keep the AUTHORIZED status.

Thanks for all the replies!

Best regards,

Sebastian.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAPGMO-ztxa6wufACydFCtc5XLrfnXQf7dFudh_-BJApFdpb3ow%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Paweł Albecki
Software Developer
palb...@soldevelo.com

[Josh Zamor]

 I agree with option 2 and believe the env setting should be consolidated into a program specific setting.  Also I like the button naming change.

As to removing the AUTHORIZED status, I disagree.  Authorized is distinctly separate from approval:  https://openlmis.atlassian.net/wiki/x/EYCeAw

1. In Authorized status the authorizer may DELETE the requisition.  In approval status the approver may only reject.
2. In authorized status the authorizer may change anything in the Requisition.  In requisition status the approver may only change approved qty fields (plus some comment fields).

Put another way:  approve and authorize are two distinctly different sets of actions.  Authorize =/= the first approval.

Best,

Josh

[Sebastian Brudziński]

Hey Josh,
thanks for your input.

I think you may be mixing up the authorization step with the actual "AUTHORIZED" status. The "AUTHORIZED" status means the requisition is PAST the authorization step and is awaiting approval. Put anoher way - in the diagram you have attached we are in the "approval process" box when the status is AUTHORIZED.

Best regards,

Sebastian.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/1b6b0e0a-2776-44f6-91a9-689d31cea7fd%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Josh Zamor]

Ah!  You’re right Sebastian, now I see what you mean.

This is a good point.  Nor can I off the top of my head rationalize why this status needs to exist.  Looking at the v2 code, I also don’t see much usage of the AUTHORIZED status without the IN_APPROVAL status, suggesting it is in-fact duplicative.  I’d wonder about upcoming migrations from v1 and v2 systems should it be removed, however.

Before we go removing it, lets bring it up at technical committee tomorrow.  I’d like to get this in front of Elias’ eyes for any corner cases we might be missing.  In the meantime this decision doesn’t seem like a blocking issue for the rest of the work, is that correct?

Best,

Josh

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAPGMO-xBPDWoTXLrxm-LU6nerQ8qGzyQJLtGzX8CEZgFicwr8w%40mail.gmail.com.

[Sebastian Brudziński]

Thanks Josh,

this is not blocking us. In fact, we can even come back to this later on, in a separate ticket. I'll be happy to talk about it on the tech call.

Thanks again for all the replies.

Kind regards,
Sebastian.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/5C93964A-7ACB-4452-9364-9E44D626DE09%40villagereach.org.

For more options, visit https://groups.google.com/d/optout.

--

Sebastian Brudziński
Software Developer / Team Leader
sbrud...@soldevelo.com

[Sebastian Brudziński]

Hi everyone,

here's a quick recap of what has been discussed on the tech committee:

 * We will not be removing the AUTHORIZED status as a part of the ticket that allows skipping the authorize step. Instead, I'll create a new ticket so we can come back to this later.
 * We have agreed to go with option 2 - submit step moves the requisition directly to the AUTHORIZED state
 * We will also be renaming the button, based on the program setting to say either "Submit & Authorize" or just "Authorize". This is a small thing, but I'm in favour of "Submit & Authorize", since it would be a little weird to have the button say "Authorize" when authorization step is to be skipped. If we go with just "Authorize" on the button, we may need to rename the "Skip authorization step" flag as well. 

Best regards,
Sebastian.