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

RA-1533 Update the registration module to permit editing all sections #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tmvumbi2
Copy link

@tmvumbi2 tmvumbi2 commented Sep 14, 2018

This PR updates RegisterPatientFragmentController, and adds a new method that handles the edit of the entire patient registration form (without modifying the existing code that handles adding a new registration). When an extra query parameter (patientIdNumber) is included, the controller edits the existing patient. This solution is backward compatible: it doesn't alter the current patient registration flow and adds the possibility to also edit a patient.
Jira ticket: https://issues.openmrs.org/browse/RA-1533

…ions

This commit updates RegisterPatientFragmentController, and adds a new method that handles the edit of the entire patient registration form (without modifying the existing code that handles adding a new registration). When an extra query parameter (patientIdNumber) is included, the controller edits the existing patient. This solution is backward compatible: it doesn't alter the current patient registration flow and adds the possibility to also edit a patient
@dkayiwa
Copy link
Member

dkayiwa commented Oct 10, 2018

@tmvumbi2 can you include a link to the ticket as advised at? https://wiki.openmrs.org/display/docs/Pull+Request+Tips

@tmvumbi2
Copy link
Author

@tmvumbi2 can you include a link to the ticket as advised at? https://wiki.openmrs.org/display/docs/Pull+Request+Tips

@dkayiwa done

@dkayiwa
Copy link
Member

dkayiwa commented Nov 4, 2018

@tmvumbi2 can you follow the JIRA workflow process here? https://wiki.openmrs.org/display/docs/Pull+Request+Tips

@dkayiwa
Copy link
Member

dkayiwa commented Nov 21, 2018

@tmvumbi2 are you still working on this?

@dkayiwa
Copy link
Member

dkayiwa commented Nov 27, 2018

@tmvumbi2 ping

@tmvumbi2
Copy link
Author

Sorry @dkayiwa for the late reply. Thx for the ping, I'll check the PR tips and update where needed.

@dkayiwa
Copy link
Member

dkayiwa commented Jan 11, 2019

@tmvumbi2 are you still working on this?


// Assertion
Patient result = (Patient) pageModel.getAttribute("patient");
Assert.assertEquals(existingPatient.getId(), result.getId());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that you are trying to get 2 checks done in assert.equal check. It would be a better coding convention and improve readability if there was assert.null right before the assert.equals function.

@@ -105,6 +107,7 @@ public FragmentActionResult importMpiPatient(@RequestParam("mpiPersonId") String
public FragmentActionResult submit(UiSessionContext sessionContext, @RequestParam(value="appId") AppDescriptor app,
@SpringBean("registrationCoreService") RegistrationCoreService registrationService,
@ModelAttribute("patient") @BindParams Patient patient,
@RequestParam(value = "patientIdNumber", required = false) String patientIdNumber,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not try to inject the Patient instance directly like here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever you end up doing, make sure it works with patient UUIDs.

@@ -121,6 +124,130 @@ public FragmentActionResult submit(UiSessionContext sessionContext, @RequestPara
@SpringBean("emrApiProperties") EmrApiProperties emrApiProperties,
@SpringBean("patientValidator") PatientValidator patientValidator, UiUtils ui) throws Exception {

if (patientIdNumber != null && !patientIdNumber.equals("")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should you stick to a String param, then use

!StringUtils.isEmpty(patientIdNumber)

@brandones
Copy link
Contributor

@mks-d Is it worth us trying to resurrect this code and get this in? Seems like it was a non-trivial piece of work.

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