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

TRUNK-3780 Searching for patient with patient name/new feature #865

Merged
merged 1 commit into from May 8, 2014

Conversation

Projects
None yet
2 participants
@haychris
Contributor

haychris commented Apr 12, 2014

https://issues.openmrs.org/browse/TRUNK-3780
These are our minimal possible changes to fix this. We tested in the webapp and minSearchCharacters (in Administration/Advanced Settings) was indeed now 2, allowing 2-character prefix search as requested. If this change is alright, then we can also add more comprehensive tests.

Possible optimizations could be: prioritizing exact match listed before prefix match before substring match, allowing user to choose between exact/prefix/substring match, or phonetic match.

How does that sound?
Christopher Hay and Katherine Ye

@dkayiwa

This comment has been minimized.

Show comment
Hide comment
@dkayiwa

dkayiwa May 2, 2014

Do you mind explaining the reason for changing this?

Do you mind explaining the reason for changing this?

This comment has been minimized.

Show comment
Hide comment
@haychris

haychris May 3, 2014

Owner

A two character search is no longer considered a short name due to how we fixed the bug, so "Jo" would not work in this test. Instead, we made it test to make sure the short name test still works with a one char search (which is still a short name).

Owner

haychris replied May 3, 2014

A two character search is no longer considered a short name due to how we fixed the bug, so "Jo" would not work in this test. Instead, we made it test to make sure the short name test still works with a one char search (which is still a short name).

@dkayiwa

This comment has been minimized.

Show comment
Hide comment
@dkayiwa

dkayiwa May 2, 2014

Do you mind explaining the reason for changing this?

Do you mind explaining the reason for changing this?

This comment has been minimized.

Show comment
Hide comment
@haychris

haychris May 3, 2014

Owner

While 'Jonathan' is still a long name (i.e., 2 or more characters), it doesn't effectively test the boundary conditions for a long name. With a two character search "Jo", we test the boundary condition for what is to be considered a long name.

Owner

haychris replied May 3, 2014

While 'Jonathan' is still a long name (i.e., 2 or more characters), it doesn't effectively test the boundary conditions for a long name. With a two character search "Jo", we test the boundary condition for what is to be considered a long name.

@dkayiwa

This comment has been minimized.

Show comment
Hide comment
@dkayiwa

dkayiwa May 2, 2014

Did you remove this test intentionally? If yes, do you mind sharing with me the reason why?

Did you remove this test intentionally? If yes, do you mind sharing with me the reason why?

This comment has been minimized.

Show comment
Hide comment
@haychris

haychris May 3, 2014

Owner

We no longer want an exact search for two character queries, but instead a prefix search. So this test is no longer necessary. The other tests make sure that two character prefix search works.

Owner

haychris replied May 3, 2014

We no longer want an exact search for two character queries, but instead a prefix search. So this test is no longer necessary. The other tests make sure that two character prefix search works.

dkayiwa added a commit that referenced this pull request May 8, 2014

Merge pull request #865 from haychris/TRUNK-3780
TRUNK-3780 Searching for patient with patient name/new feature

@dkayiwa dkayiwa merged commit 9b3e6f8 into openmrs:master May 8, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment