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

Remove redundant limit on number of search results #799

Merged
merged 1 commit into from
Sep 30, 2021

Conversation

bausmeier
Copy link
Contributor

@bausmeier bausmeier commented Sep 27, 2021

Fixes #788.

Description
Remove redundant limit on number of search results

Type
Bug fix.

Checklist

  • I have read and acknowledged the Code of conduct
  • I have read How to Contribute
  • I have read the Developer's guide
  • I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA
  • I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach
  • I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project
  • I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally
  • I have built and run the reference app(s) to verify my change fixes the issue and/or does not break the reference app(s)

@bausmeier
Copy link
Contributor Author

bausmeier commented Sep 27, 2021

@trevorgowing I can't request a review on the PR, but please will you take a look and confirm that this resolves the issue for you?

@trevorgowing
Copy link

@bausmeier Working for me. Nice one.

@codecov-commenter
Copy link

codecov-commenter commented Sep 27, 2021

Codecov Report

Merging #799 (777bfdd) into master (22dc506) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #799      +/-   ##
============================================
- Coverage     35.18%   35.16%   -0.03%     
+ Complexity      315      314       -1     
============================================
  Files           115      115              
  Lines          4360     4360              
  Branches        841      841              
============================================
- Hits           1534     1533       -1     
  Misses         2496     2496              
- Partials        330      331       +1     
Impacted Files Coverage Δ
...va/com/google/android/fhir/db/impl/DatabaseImpl.kt 58.69% <0.00%> (-1.09%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22dc506...777bfdd. Read the comment docs.

Copy link
Collaborator

@jingtang10 jingtang10 left a comment

Choose a reason for hiding this comment

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

Thanks @bausmeier! 👍

@jingtang10
Copy link
Collaborator

please feel free to merge

@jingtang10 jingtang10 closed this Sep 30, 2021
@jingtang10 jingtang10 reopened this Sep 30, 2021
Accessing the value of `patientCount` using the not-null assertion
operator was resulting in a Null Pointer Exception if the count
operation had not yet completed. Limiting the number of search results
based on the count was not necessary anyway since the set of filters
used for the search was narrower than those used for the count which
meant that the count would always be greater than or equal to the number
of search results.
@bausmeier
Copy link
Contributor Author

@jingtang10 Thanks, I've rebased against master so that the branch is up to date to pass the branch protection, but I don't have write access to the repo to be able to merge.

@jingtang10 jingtang10 merged commit 523a023 into google:master Sep 30, 2021
@bausmeier bausmeier deleted the ba/gh-788-fix-npe branch September 30, 2021 10:15
@Tarun-Bhardwaj Tarun-Bhardwaj added the type:bug Something isn't working label Sep 30, 2021
@Tarun-Bhardwaj Tarun-Bhardwaj added this to In progress in Demo applications via automation Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
Archived in project
Demo applications
  
In progress
Development

Successfully merging this pull request may close these issues.

PatientListViewModel Null Pointer Exception Line 77
5 participants