fix: handle UNAPPROVED_ABOVE status - #18
Conversation
|
|
||
| case 'UNAPPROVED_WAITING': | ||
| case 'UNAPPROVED_ELSEWHERE': | ||
| case 'UNAPPROVED_ABOVE': |
There was a problem hiding this comment.
If I get these docs (search for "UNAPPROVED_ABOVE") right, the this means that it's already been approved and the next level has to approve it, but I'm really not sure if I'm interpreting the docs correctly
There was a problem hiding this comment.
What I mean is: Is "waiting" the right label?
There was a problem hiding this comment.
Yeah, I’m not sure about the correct interpretation either, but we need to get these confirmed anyway, so I guess we can add this state to this case (waiting) for now.
Now that we already encountered this undocumented state on day 1 already, I think we should:
- Definitely talk to Lars/Jim about the correct interpretation of these states
- Probably scan the dhis-core source code for a complete overview of the statuses
Probably it makes sense to ensure we have a complete list before we talk to one of them…
There was a problem hiding this comment.
PS: I am referring to this state as undocumented because I don’t see it in the list of approval states. Could be I’m not looking properly or in the wrong place…
There was a problem hiding this comment.
I had a look in the dhis2-core code-base and was expecting the relevant section of code to be in the DataApprovalState.java file. But that only contained the 7 states and we already had comprised a list of 8 from the docs:
UNAPPROVABLE
APPROVED_ELSEWHERE // in docs, but not in DataApprovalState.java
UNAPPROVED_ABOVE
UNAPPROVED_WAITING
UNAPPROVED_READY
APPROVED_ABOVE
APPROVED_HERE
ACCEPTED_HERE
Then looked in dataApproval.js (I think the is the Javascript part of the old struts app) and there I found 14 states:
ACCEPTED_ELSEWHERE
ACCEPTED_HERE
APPROVED_ABOVE
APPROVED_ELSEWHERE
APPROVED_HERE
PARTIALLY_ACCEPTED_ELSEWHERE
PARTIALLY_ACCEPTED_HERE
PARTIALLY_APPROVED_ELSEWHERE
PARTIALLY_APPROVED_HERE
UNAPPROVABLE
UNAPPROVED_ABOVE
UNAPPROVED_ELSEWHERE
UNAPPROVED_READY
UNAPPROVED_WAITING
What was puzzling to is how the JavaScript file could have to deal with more approval states than defined in the Java code. I think we need to get more info on this.
For now, I guess we could just add the case UNAPPROVED_ABOVE as you suggested, but perhaps it would be good to also add a default case which throws a descriptive error message?
|
🎉 This PR is included in version 1.4.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
4 participants
Handle the
UNAPPROVED_ABOVEstatus in the same way we handleUNAPPROVED_WAITINGandUNAPPROVED_ELSEWHERE.