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

Make LuceneSearchResultsServiceImpl the primary implementation for SearchResultsService interface #1218

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

rahul6603
Copy link
Contributor

Issue - #1049

Summary

  • This PR aims to make LuceneSearchResultsServiceImpl the primary implementation for SearchResultsService interface and LuceneSearchResultsDAOImpl the primary implementation for SearchResultsDAO interface.
  • I have also added Qualifier annotations to make it more readable about which specific implementation is being used.

@rahul6603
Copy link
Contributor Author

fuzzy-search-demo.mov

Hi @mozzy11, could you please review the changes? I have also attached a demo video of Patient Search from the UI demonstrating fuzzy search on first name and last name.
Thanks.

Copy link
Collaborator

@mozzy11 mozzy11 left a comment

Choose a reason for hiding this comment

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

Thanks @rahul6603 .
We will delay merging this PR as it depends on #1220

@rahul6603 rahul6603 requested a review from mozzy11 August 16, 2024 12:31
@rahul6603
Copy link
Contributor Author

Hi @mozzy11, since #1220 has been merged, this PR can also be merged to finalise the Lucene integration.

@mozzy11
Copy link
Collaborator

mozzy11 commented Sep 19, 2024

Hello @rahul6603 ,
Thanks for the PR.
Can you see the QA workfow fails especially whenever theres search by patient .

Copy link
Collaborator

@mozzy11 mozzy11 left a comment

Choose a reason for hiding this comment

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

Can you first ensure the Patient Serach works in the QA workflow

@rahul6603
Copy link
Contributor Author

Hi @mozzy11 , I have made a fix in the latest commit which resolves the failing QA workflows.
I have added a check to ensure the idList parameter is only added to the SQL query when there are more than 0 hits from the Lucene query. The actual conditionif (hitsCount > 1) accounts for the dummy value (-1) in the hits list.
Let me know if any other change is needed.

@rahul6603 rahul6603 requested a review from mozzy11 October 1, 2024 15:09
@mozzy11
Copy link
Collaborator

mozzy11 commented Oct 2, 2024

Thanks @rahul6603

@mozzy11
Copy link
Collaborator

mozzy11 commented Oct 8, 2024

hello @rahul6603 ,
just to confirm , is this endpoint /reindex meant to rebuild the search index in case its broken ??
can you add a link somewhere on the admin page for a user to just click and call the endpoint to rebuild the index incase its broken ??

@rahul6603
Copy link
Contributor Author

Hi @mozzy11, sorry for the late reply. Yeah, the endpoint /reindex is meant for rebuilding the index. Can you point me to the specific admin page where such functionality can be added?

@mozzy11
Copy link
Collaborator

mozzy11 commented Nov 7, 2024

@rahul6603 No worries.

You can add a new Side Nav Link on the Admin Page ie Search Index Management. whick links to a Search Index Management Page.

You can just put a button , for a user to click to rebuild the searxh index and it should display a loader until the Search index is completely re-build

@mozzy11
Copy link
Collaborator

mozzy11 commented Nov 7, 2024

And also , you can add more Tests for LuceneSearch for each single search Parameter to ensure nothing breaks.
The existing Lucene Search Test takes in all params supported by Lucene Search at ago

@mozzy11
Copy link
Collaborator

mozzy11 commented Nov 7, 2024

Again thanks for responding , i appreciate

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.

2 participants