RA-121: Add picture to Registration #71
Conversation
| } | ||
|
|
||
| private void buildObs(ConceptService conceptService, List<Obs> obsToCreate, String conceptId, String[] parameterValues) throws ParseException { | ||
| private void buildObs(ConceptService conceptService, List<Obs> obsToCreate, String conceptId, String[] parameterValues,HttpServletRequest request,String name) throws ParseException { |
There was a problem hiding this comment.
hi @reagan-meant thanks for the works here,do we really need this around there HttpServletRequest request,checkout that
There was a problem hiding this comment.
Thanks @HerbertYiga for the review...I have changed the approach..see new PR
| } else if (param.startsWith("obs.")) { | ||
| String conceptUuid = param.substring("obs.".length()); | ||
| buildObs(conceptService, obsToCreate, conceptUuid, request.getParameterValues(param)); | ||
| buildObs(conceptService, obsToCreate, conceptUuid, request.getParameterValues(param),request,name.getFullName()); |
|
Is this still a work-in-progress, or is it ready for review? Thanks! |
|
@mogoodrich I will be pushed a better commit tomorrow for review...left with a few adjustments |
|
@mogoodrich I need a review now...cc @dkayiwa @mks-d @Ruhanga @ibacher |
mogoodrich
left a comment
mogoodrich
left a comment
There was a problem hiding this comment.
Thanks @Ruhanga ... I took a quick look... I didn't review the workings of the new functionality in detail, was basically looking for any "red flags". I added some questions inline, and a couple follow-on here:
- Do you need to include jquery in registration app? I believe it's provided by uicommons?
- How is this configured? It should be "opt-in" functionality that people should need to "enable". (Maybe this is how ti works, but wanted to confirm)
.../src/main/java/org/openmrs/module/registrationapp/web/controller/patientImageController.java
Show resolved
Hide resolved
| return new SuccessResult(redirectUrl); | ||
| SimpleObject result = SimpleObject.create("redirectUrl", redirectUrl, "uuid", patient.getUuid()); | ||
| ObjectResult response = new ObjectResult(result); | ||
| return response; |
There was a problem hiding this comment.
I needed to get the patient uuid in the response since the saving of the image is done via the ajax success call back here
| assertTrue(result instanceof SuccessResult); | ||
| assertThat(((SuccessResult) result).getMessage(), is("url.html?patient=" + patient.getUuid())); | ||
| assertTrue(result instanceof ObjectResult); | ||
| //assertThat(((ObjectResult) result).getMessage(), is("url.html?patient=" + patient.getUuid())); |
There was a problem hiding this comment.
Is there any way to fix these tests instead of just commenting them out.
There was a problem hiding this comment.
I will add a new assertion here and remove this.
|
Closing as stale. |
4 participants
Work in progress.
https://issues.openmrs.org/browse/RA-121
Together with openmrs/openmrs-module-referenceapplication#77