-
Notifications
You must be signed in to change notification settings - Fork 1
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
Optimize location hierarchy #87
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #87 +/- ##
=============================================
+ Coverage 44.25% 60.89% +16.64%
- Complexity 138 195 +57
=============================================
Files 16 16
Lines 1358 1404 +46
Branches 160 158 -2
=============================================
+ Hits 601 855 +254
+ Misses 689 468 -221
- Partials 68 81 +13 ☔ View full report in Codecov by Sentry. |
Instead of fetching entire practitionerdetails, just get the locations.
25c2248
to
ec06253
Compare
- replace for loops with streams - use sets instead of lists to avoid duplication - switch from using DFS to BFS in getting locations deep in the hierarchy
- switch from for loop to streams in filterLocationsByAdminLevels
@@ -443,23 +453,25 @@ public List<String> generateAdminLevels( | |||
|
|||
public List<Location> filterLocationsByAdminLevels( | |||
List<Location> locations, List<String> postFetchAdminLevels) { | |||
if (postFetchAdminLevels == null) { | |||
|
|||
if (postFetchAdminLevels == null || postFetchAdminLevels.isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolves bug of filtering out all locations when adminlevels is not provided/is empty
PR looks good. Lets bump up the Release version as well cc @lincmba |
Resolves #82
Engineer Checklist
bug fixes
option(s) on the
README.md
mvn spotless:check
to check my code follows the project'sstyle guide
mvn clean test jacoco:report
to confirm the coverage reportwas generated at
plugins/target/site/jacoco/index.html
mvn clean package
right before creating this pull request.