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

GSoC 2019: Patient search criteria module #219

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

Conversation

Reyano132
Copy link

new UI of find patient widget, which supports the new patient search criteria.
GSoC 2019 Project:https://wiki.openmrs.org/display/projects/Patient+Search+Criteria

@HerbertYiga
Copy link
Contributor

did you see the conflicting files on this ticket!!!

@Reyano132 Reyano132 force-pushed the GSoC19 branch 7 times, most recently from 4f1d29c to 00b63a3 Compare August 31, 2019 05:23
@Reyano132
Copy link
Author

@HerbertYiga, I resolved conflict

@@ -255,6 +264,20 @@
<scope>provided</scope>
</dependency>

<!-- Patient search criteria module -->
<dependency>
<groupId>com.github.Reyano132.openmrs-module-patientsearchcriteria</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

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

please revise this groupid to match our conventions https://wiki.openmrs.org/display/docs/Module+Conventions

Copy link
Author

Choose a reason for hiding this comment

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

@kaweesi sir, here I used jitpack to host my remote repository. Jitpack added "com.github.Reyano132" prefix to the group ID. Once we officially host the patient search criteria module (https://github.com/openmrs/openmrs-module-patientsearchcriteria), I'll change the groupId value here.

<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.github.Reyano132.openmrs-module-patientsearchcriteria</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

<!-- Repository for patient search criteria -->
<repositories>
<repository>
<id>jitpack.io</id>
Copy link
Contributor

Choose a reason for hiding this comment

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

any idea why u wanna publish these on jitpack?

Copy link
Author

Choose a reason for hiding this comment

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

@kaweesi sir, I added this repository in PR so you can use the patient search criteria module(which is not merge yet). Once you merge patient search criteria module PR(openmrs/openmrs-module-patientsearchcriteria#1) and create its remote repository, I will replace this jitpack with that repository. Another thing is that the PR of patient search criteria also consists of a jitpack repository of openmrs-core(openmrs/openmrs-core#2980).

Overall, there are dependencies between the PRs as I explained in the talk.
please 1st merge openmrs core PR(openmrs/openmrs-core#2980).
Then merge patient search criteria module PR(openmrs/openmrs-module-patientsearchcriteria#1) and create a remote repository of that module and at last, after creating a repository of patient search criteria module I just need to replace jitpack repository with patient search criteria repository

@Irenyak1
Copy link
Member

Irenyak1 commented Dec 2, 2019

There is this error that occurs when you navigate to Find Patient Record
UI Framework Error
Root Error
groovy.lang.GroovyRuntimeException: Failed to parse template script (your template may contain an error or be trying to use expressions not currently supported): startup failed:
SimpleTemplateScript13.groovy: 108: unexpected token: < @ line 108, column 13.

<% if(patientSearchExtensions){

        <select id="patient-gender-search">

@Reyano132
Copy link
Author

@Irenyak1 ma'am, I'm not getting such error. Have you tried this on openmrs referenceapplication-standalone-2.9.0? because I just tried this one more time today and it works fine.

@kaweesi
Copy link
Contributor

kaweesi commented Dec 3, 2019 via email

@Irenyak1
Copy link
Member

Irenyak1 commented Dec 3, 2019

@Reyano132 I have tried it with Reference Application 2.10 SNAPSHOT. Could do like @kaweesi said and we see what comes out?

@Irenyak1
Copy link
Member

Irenyak1 commented Dec 3, 2019

@Reyano132 you may use this https://qa-refapp.openmrs.org/openmrs/login.htm.

Do you know how to build using the SDK?

@Reyano132 Reyano132 force-pushed the GSoC19 branch 4 times, most recently from 01dad2d to 8b8d822 Compare December 4, 2019 14:05
@Reyano132
Copy link
Author

Hey @Irenyak1 , I solved the problem of repetition of the result by calling reset function from updateSearchResult function (line 514). Problem related to the gender search, can you please see the rest response, maybe you are getting an error like "org.openmrs.Person is not an indexed type". This because openmrs core does not support indexing for Person class yet.

@Reyano132 Reyano132 closed this Dec 4, 2019
@Reyano132 Reyano132 reopened this Dec 4, 2019
@kaweesi
Copy link
Contributor

kaweesi commented Dec 4, 2019

please go ahead and test well enough including confirming that applying more than one filter doesn't break the widget/results.

You could also go ahead and do some styling on the new 2 element to kind of make them look more beautiful, perhaps from a vertical to a horizontal orientation

@Reyano132
Copy link
Author

Okay @kaweesi sir, I'll work on UI, but can you please help me with solving this Travis CI build problem?

@kaweesi
Copy link
Contributor

kaweesi commented Dec 5, 2019

@Irenyak1
Copy link
Member

Irenyak1 commented Dec 9, 2019

Hello @Reyano132 there's an issue that is still persisting, and that is the the replication of patient names when you do a search. When you time in a name of a patient and is filtered, if you remove that name, the same is duplicated as you remove the different characters. Please try to reproduce that error and look into it.
cc @kaweesi .

Look at this, Jamie Jones Jake is replicated and the gender filter has female but even the male patients are returned
search-patient

@Reyano132
Copy link
Author

Okay @Irenyak1, means whenever you are trying to apply a filter on the search result you are getting duplications in the result, Is it?

@Irenyak1
Copy link
Member

Yes that's it @Reyano132 . Have you tried to reproduce the error?

@Reyano132
Copy link
Author

Yes, @Irenyak1 ma'am, I tried to reproduce that error but whenever I apply gender filter it works fine.

@Irenyak1
Copy link
Member

@Reyano132 try selecting the gender then after type in a patient's name know exists. When the patient is returned, try removing the patients name one letter at a time, you may see the error. The names will be duplicated.

@Reyano132
Copy link
Author

@Irenyak1 and @kaweesi sir, can we first focus on merging OpenMRS core PR(openmrs/openmrs-core#2980). It will assure that we are using the same core and also we can use other filters also.

@Irenyak1
Copy link
Member

@kaweesi could you attend to the PR from @Reyano132 so we see if the bugs are fixed?

@kaweesi
Copy link
Contributor

kaweesi commented Jan 13, 2020

from a glance, i see a lot of feedback here unresolved.

@Irenyak1
Copy link
Member

@Reyano132 could you attend to @kaweesi 's comments

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

Successfully merging this pull request may close these issues.

4 participants