AF-59: Extensions should support 'requiredProgram' property #47
Conversation
api/src/main/java/org/openmrs/module/appframework/context/ProgramConfiguration.java
Show resolved
Hide resolved
| ret.setProgram(programs.get(0)); | ||
| } else if (programs.size() > 1) { | ||
| ret.setProgram(programWithBestWorflowAndStateCombination(programs)); | ||
| } |
There was a problem hiding this comment.
I wouldn't do this if statement, I would just keep the shortlist of programs for now.
| } | ||
| } | ||
| if (StringUtils.isNotBlank(workflowRef)) { | ||
| List<ProgramWorkflow> workflows = getAllPossibleWorkflows(); |
There was a problem hiding this comment.
Then I would do:
List<ProgramWorkflow> workflows = getAllPossibleWorkflows(programs);That would give the shortlist of workflows that can work with the shortlist of programs.
There was a problem hiding this comment.
Hi @mks-d,
Did you intend to pass the programs shortlist as a parameter to the getAllPossibleWorkflows(..) method? If so, I think we had talked about this a couple of months back. Our conclusion was: First of all try getting all the metadata(program, workflow, state) independently. If we encounter an issue, then we fail.
In other-words, we want to look at what was provided as the "workflowRef", try to retrieve whatever object(s) the reference points to from the DB.
We do this for:
- Program
- Workflow
- State
If that operation yields many values for each ie. many programs, we try to get what we called ResolvedConfiguration by trying to get the best program-workflow-state tree. Otherwise we fail.
| ret.setWorkflow(workflows.get(0)); | ||
| } else if (workflows.size() > 1) { | ||
| ret.setWorkflow(workflowWithBestProgramAndStateCombination(workflows, ret.getProgram())); | ||
| } |
There was a problem hiding this comment.
So this if would not be needed either, you would just keep going with the shortlist of workflows.
| } | ||
| } | ||
| if (StringUtils.isNotBlank(stateRef)) { | ||
| List<ProgramWorkflowState> states = getAllPossibleStates(); |
There was a problem hiding this comment.
Same story:
List<ProgramWorkflowState> states = getAllPossibleStates(workflows);That would give the shortlist of states that can work with the shortlist of workflows already established.
There was a problem hiding this comment.
Same case here, getAllPossibleStates(...) should retrieve all possible states independent of the workflows.
There was a problem hiding this comment.
That would give the shortlist of states that can work with the shortlist of workflows already established.
Actually that was my initial design which we changed. It makes things less complicated
| } else if (states.size() > 1) { | ||
| ret.setState(stateWithBestProgramAndWorkflowCombination(states, ret.getProgram(), ret.getWorkflow())); | ||
| } | ||
| } |
There was a problem hiding this comment.
At this point you have three shortlists:
programsworkflows(that are working withprograms)states(that are working withworkflows)
And this is where you need a specialized routine that will take the three shortlists and reduce them to a single final triplet program/workflow/state or throw an IllegalArgumentException if that is not possible from the provided references.
Probably this existing stateWithBestProgramAndWorkflowCombination to be renamed findProgramWorkflowStateTriplet.
| if (programs.size() == 1) { | ||
| ret.setProgram(programs.get(0)); | ||
| } else { | ||
| throw new APIException("Failed to figure out the intended program identified by: " + programRef); |
There was a problem hiding this comment.
APIException("Failed to single out one program identified by: " + programRef);Same kind of message for other entities.
| protected String getPatientUuid(AppContextModel contextModel) { | ||
| Bindings bindings = new SimpleBindings(); | ||
| for (Map.Entry<String, Object> e : contextModel.entrySet()) { | ||
| bindings.put(e.getKey(), e.getValue()); | ||
| } | ||
| javascriptEngine.setBindings(bindings, ScriptContext.ENGINE_SCOPE); | ||
| try { | ||
| String uuid = (String) javascriptEngine.eval("patient.uuid;"); | ||
| if (StringUtils.isBlank(uuid)) { | ||
| throw new APIException("Patient uuid cannot be empty"); | ||
| } | ||
| return uuid; | ||
| } catch (ScriptException e) { | ||
| throw new APIException("Failed to get patient uuid", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Looks like the context model holds direct references to the patient programs. See here. Wouldn't that be enough?
Ticket: https://issues.openmrs.org/browse/AF-59