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

Support disease filter on results CSV download #8052

Merged
merged 3 commits into from
Aug 23, 2024

Conversation

mpbrown
Copy link
Collaborator

@mpbrown mpbrown commented Aug 22, 2024

BACKEND PULL REQUEST

Related Issue

Changes Proposed

  • Adds support for disease filter parameter on the results CSV download

Additional Information

  • The main results list page and the csv download use different but very similar queries and search filters.
  • The key difference between the two is that the csv download has some additional data from the test event AOE responses while the main results list page does not.
  • Main results page calls the query mapping to ResultResolver.resultsPage which then calls ResultService.getFacilityResults or ResultService.getOrganizationResults. These service methods then call ResultRepository.findAll and pass in a JPA Specification<Results> that is created with the Jakarta criteria builder in buildResultSearchFilter.
  • The CSV download calls the query mapping to TestResultResolver.testResultsPage which then calls TestOrderService.getOrganizationTestEventsResults or TestOrderService.getFacilityTestEventsResults. These service methods then call TestEventRepository.findAll and pass in a JPA Specification<TestEvent> that is created with the Jakarta criteria builder in buildTestEventSearchFilter.
  • For both buildTestEventSearchFilter and buildResultSearchFilter there is fairly identical logic for applying the filters on the joins.
  • A future ticket should refactor some of this to ideally use one query and support both use cases whether or not it requires the additional AOE data.

GraphQL backwards compatibility

Testing

  • Deployed on dev5

@mpbrown mpbrown changed the title Mike/7948 results download filters frontend Support disease filter on CSV download Aug 22, 2024
@mpbrown mpbrown changed the title Support disease filter on CSV download Support disease filter on results CSV download Aug 22, 2024
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@mpbrown
Copy link
Collaborator Author

mpbrown commented Aug 22, 2024

Since these queries are already called out as candidates for refactoring in the future, I think we could probably be safe with "Accepting" the SonarCloud issues that identify the method using 8 parameters instead of 8. But let me know if anyone would prefer this addressed in this PR instead of the follow up refactor ticket.

@@ -2504,27 +2549,30 @@ private List<TestEvent> makedata() {
return testEvents;
}

private List<TestEvent> makeSpecificDiseaseData(SupportedDisease disease) {
private List<TestEvent> makeSpecificDiseaseData(SupportedDisease disease, Facility site) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a parameter to pass in a specific site so that one of the tests can use the global instance variable facility site rather than a new disease specific facility created below

Copy link
Collaborator

@emyl3 emyl3 left a comment

Choose a reason for hiding this comment

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

LGTM! tested on dev5 and filters worked as expected!

@fzhao99 fzhao99 added this pull request to the merge queue Aug 23, 2024
Merged via the queue into main with commit bbc80df Aug 23, 2024
41 of 42 checks passed
@fzhao99 fzhao99 deleted the mike/7948-results-download-filters-frontend branch August 23, 2024 17:03
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.

[BUG] Fix download csv test results filters
3 participants