-
Notifications
You must be signed in to change notification settings - Fork 55
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
Add result and condition filters to test results list #6682
Merged
nathancrtr
merged 42 commits into
main
from
nac/6303-results-table-add-condition-result-filters
Oct 25, 2023
Merged
Changes from 41 commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
5775897
Refactor results table - each multiplex result on own row
nathancrtr c986570
Rename TestResultsList test titles to reflect refactor; remove obsole…
nathancrtr ca0c17f
Merge branch 'main' into nac/6104-refactor-results-table-addtl-diseases
nathancrtr d85a7a3
Add skeleton structure for returning Results by facility & organization
nathancrtr 3fb4d9f
Add result service methods to return results with various filters app…
nathancrtr 8985b84
Adds support for GraphQL query to filter and return granular test result
nathancrtr 018b787
Defines query operation to retrieve filtered test results from server…
nathancrtr 360dc13
Fix page & result count display; Tidy comments & un-used code
nathancrtr 056d15c
Merge branch 'main' into nac/6303-results-table-add-condition-result-…
nathancrtr 85ef1e2
Remove reference to non-existent generated GraphQL method
nathancrtr 3fcb9df
Test changes - update many, many mocks and fudge some typing to get t…
nathancrtr 9ac8f73
Fix reference to test id in mock
nathancrtr bd83caa
Update some more test results mocks
nathancrtr cbae0e4
Merge branch 'main' into nac/6303-results-table-add-condition-result-…
nathancrtr 672eb13
Update sooooo many mocks, other test tweaks
nathancrtr 83ca095
Merge branch 'main' into nac/6303-results-table-add-condition-result-…
nathancrtr 543031a
comment out some junk so I can build
nathancrtr bfe6c5e
Broke out new ResultService tests into nested block to keep setup sep…
nathancrtr 791eb0b
Correct mocks for ResultsTable unit tests
nathancrtr 861e88d
Merge branch 'main' into nac/6303-results-table-add-condition-result-…
nathancrtr 5b5b151
Update test result CSV download mock
nathancrtr ca25a6e
Actually correct mock response for CSV results download
nathancrtr d98c65b
Cleanup....
nathancrtr 7c2fd17
Add ResultService test for date range filter; clean up some code smells
nathancrtr c221c12
Remove unused file
nathancrtr 169a96a
More code smell cleanup, on the frontend this time
nathancrtr e2f9607
Remove unread local variables in unit test file
nathancrtr db26d6e
Add some (not super-useful) unit tests for ResultResolver
nathancrtr 6b86674
Get patient link via GraphQL query for test result row in e2e tests
nathancrtr 55fad84
Merge branch 'main' into nac/6303-results-table-add-condition-result-…
nathancrtr cba21bd
Un-comment things from testing
nathancrtr 93a470a
Run login hooks for patient link e2e test - needed for one-off GraphQ…
nathancrtr a101869
Add and employ a GraphQL query called from Cypress to get patient lin…
nathancrtr 526d4ee
Try to get the right envvar for the SR frontend...
nathancrtr 2b1be0b
Ditto
nathancrtr eab7484
once more, with feeling
nathancrtr fbdd80c
Get rid of some stuff that should not be there
nathancrtr 34346c5
Remove some commented-out code and unused test result item properties
nathancrtr cc7f1bf
Remove unnecessary assertion in TestResultsList unit tests
nathancrtr 224a1e9
Codegen
nathancrtr 76e3733
Remove some unused properties on mock objects for ResultsTable
nathancrtr 8152475
Merge branch 'main' into nac/6303-results-table-add-condition-result-…
nathancrtr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
15 changes: 15 additions & 0 deletions
15
backend/src/main/java/gov/cdc/usds/simplereport/api/testresult/ResultDataResolver.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package gov.cdc.usds.simplereport.api.testresult; | ||
|
||
import gov.cdc.usds.simplereport.api.model.ApiFacility; | ||
import gov.cdc.usds.simplereport.db.model.auxiliary.TestResultsListItem; | ||
import org.springframework.graphql.data.method.annotation.SchemaMapping; | ||
import org.springframework.stereotype.Controller; | ||
|
||
@Controller | ||
public class ResultDataResolver { | ||
|
||
@SchemaMapping(typeName = "Result", field = "facility") | ||
public ApiFacility getFacility(TestResultsListItem result) { | ||
return new ApiFacility(result.getFacility()); | ||
} | ||
} |
74 changes: 74 additions & 0 deletions
74
backend/src/main/java/gov/cdc/usds/simplereport/api/testresult/ResultResolver.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
package gov.cdc.usds.simplereport.api.testresult; | ||
|
||
import gov.cdc.usds.simplereport.api.Translators; | ||
import gov.cdc.usds.simplereport.db.model.SupportedDisease; | ||
import gov.cdc.usds.simplereport.db.model.auxiliary.TestResultsListItem; | ||
import gov.cdc.usds.simplereport.service.DiseaseService; | ||
import gov.cdc.usds.simplereport.service.ResultService; | ||
import gov.cdc.usds.simplereport.service.TestOrderService; | ||
import java.util.Date; | ||
import java.util.UUID; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.data.domain.Page; | ||
import org.springframework.graphql.data.method.annotation.Argument; | ||
import org.springframework.graphql.data.method.annotation.QueryMapping; | ||
import org.springframework.stereotype.Controller; | ||
|
||
@Controller | ||
@RequiredArgsConstructor | ||
public class ResultResolver { | ||
|
||
private final ResultService service; | ||
private final DiseaseService diseaseService; | ||
|
||
@QueryMapping | ||
public Page<TestResultsListItem> resultsPage( | ||
@Argument UUID facilityId, | ||
@Argument UUID patientId, | ||
@Argument String result, | ||
@Argument String role, | ||
@Argument String disease, | ||
@Argument Date startDate, | ||
@Argument Date endDate, | ||
@Argument int pageNumber, | ||
@Argument int pageSize) { | ||
|
||
if (pageNumber < 0) { | ||
pageNumber = TestOrderService.DEFAULT_PAGINATION_PAGEOFFSET; | ||
} | ||
|
||
if (pageSize < 1) { | ||
pageSize = TestOrderService.DEFAULT_PAGINATION_PAGESIZE; | ||
} | ||
|
||
SupportedDisease supportedDisease = | ||
disease != null ? diseaseService.getDiseaseByName(disease) : null; | ||
|
||
if (facilityId == null) { | ||
return service | ||
.getOrganizationResults( | ||
patientId, | ||
Translators.parseTestResult(result), | ||
Translators.parsePersonRole(role, true), | ||
supportedDisease, | ||
startDate, | ||
endDate, | ||
pageNumber, | ||
pageSize) | ||
.map(TestResultsListItem::new); | ||
} | ||
|
||
return service | ||
.getFacilityResults( | ||
facilityId, | ||
patientId, | ||
Translators.parseTestResult(result), | ||
Translators.parsePersonRole(role, true), | ||
supportedDisease, | ||
startDate, | ||
endDate, | ||
pageNumber, | ||
pageSize) | ||
.map(TestResultsListItem::new); | ||
} | ||
} |
41 changes: 41 additions & 0 deletions
41
backend/src/main/java/gov/cdc/usds/simplereport/db/model/auxiliary/TestResultsListItem.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
package gov.cdc.usds.simplereport.db.model.auxiliary; | ||
|
||
import gov.cdc.usds.simplereport.db.model.ApiUser; | ||
import gov.cdc.usds.simplereport.db.model.DeviceType; | ||
import gov.cdc.usds.simplereport.db.model.Facility; | ||
import gov.cdc.usds.simplereport.db.model.Person; | ||
import gov.cdc.usds.simplereport.db.model.Result; | ||
import java.util.Date; | ||
import java.util.UUID; | ||
import lombok.Getter; | ||
|
||
@Getter | ||
public class TestResultsListItem { | ||
private final UUID id; | ||
private final Facility facility; | ||
private final Person patient; | ||
private final Date dateAdded; | ||
private final Date dateUpdated; | ||
private final DeviceType deviceType; | ||
private final String disease; | ||
private final TestResult testResult; | ||
private final Date dateTested; | ||
private final TestCorrectionStatus correctionStatus; | ||
private final String reasonForCorrection; | ||
private final ApiUser createdBy; | ||
|
||
public TestResultsListItem(Result result) { | ||
this.id = result.getTestEvent().getInternalId(); | ||
this.facility = result.getTestEvent().getFacility(); | ||
this.patient = result.getTestEvent().getPatient(); | ||
this.dateAdded = result.getTestEvent().getDateTested(); | ||
this.dateUpdated = result.getUpdatedAt(); | ||
this.deviceType = result.getTestEvent().getDeviceType(); | ||
this.disease = result.getDisease().getName(); | ||
this.testResult = result.getTestResult(); | ||
this.dateTested = result.getTestEvent().getDateTested(); | ||
this.correctionStatus = result.getTestEvent().getCorrectionStatus(); | ||
this.reasonForCorrection = result.getTestEvent().getReasonForCorrection(); | ||
this.createdBy = result.getTestEvent().getCreatedBy(); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
remove if statement and only use
getFacilityResults
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.
Is there a clean way to move this null conditional into the service instead of having it in the resolver? This was related to our other comment about combining
getOrganizationResults
andgetFacilityResults
, although not sure whether the different auth requirements complicates this thenThere 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.
I think having this conditional in the resolver makes sense for the auth reasons discussed elsewhere.
Fwiw, this pattern (and the associated auth requirements) are lifted one-to-one from the existing
TestResultResolver
andTestOrderService
classes, which these changes largely supplant:prime-simplereport/backend/src/main/java/gov/cdc/usds/simplereport/api/testresult/TestResultResolver.java
Lines 46 to 65 in 5140abc