Migrate Patients page to using a direct Neo4j query #167
Conversation
There was a problem hiding this comment.
This is a new version of RecordsList.tsx that's adapted for the Patients page. Because of the new query method, there were several changes made to RecordsList.tsx for the Patients page, so I rewrote it in a new file to make the development process less hairy.
The alternative would have been updating RecordsList.tsx to work for both Patients page and the other pages (Requests & Cohorts), and the RecordsList component's props would end up quite long and confusing.
Meanwhile, you can still see a diff between these files here.
Below, I'm going to clarify only the logic that are new/different (not many), but happy to clarity existing codes as well if you'd like.
| const extraSearchVals = searchInterceptor | ||
| ? await searchInterceptor(userSearchVal) | ||
| : []; | ||
| const searchVals = [ | ||
| ...parseUserSearchVal(userSearchVal), | ||
| ...extraSearchVals, | ||
| ]; |
There was a problem hiding this comment.
searchInterceptor function is a new component prop that runs some logic before the standard search logic kicks in. For the Patients page, this function returns additional values to be searched by: patient IDs corresponding to the MRNs that are entered in the search bar.
| const MAX_ROWS_EXPORT = 10000; | ||
| const MAX_ROWS_EXPORT_EXCEED_ALERT = | ||
| "You can only download up to 5,000 rows of data at a time. Please refine your search and try again. If you need the full dataset, contact the SMILE team at [email protected]."; | ||
| "You can only download up to 10,000 rows of data at a time. Please refine your search and try again. If you need the full dataset, contact the SMILE team at [email protected]."; |
There was a problem hiding this comment.
Pavitra requested for this change 1-2 weeks ago (context). I made this quick change to the code running in prod but haven't officially commit-ed it until now.
There was a problem hiding this comment.
Most code changes here are simple renames, either to make names more generic for reuse (like DashboardSampleSort to DashboardRecordSort) or follow better JS naming practices (like handleDownload to onDownload).
| // Mirror the field types in the CRDB, where CMO_ID is stored without the "C-" prefix | ||
| export type PatientIdsTriplet = { | ||
| CMO_ID: string; | ||
| PT_MRN: string; | ||
| DMP_ID: string | null; | ||
| }; |
There was a problem hiding this comment.
Removed because we can pull this type directly from the file generated by the codegen process.
| async function getExtraCmoIdsFromMrnInputs(userSearchVal: string) { | ||
| let extraCmoIds: string[] = []; | ||
| if (phiEnabled && userSearchVal !== "") { | ||
| const parsedSearchVals = parseUserSearchVal(userSearchVal); | ||
|
|
||
| if (phiEnabled) { | ||
| parsedSearchVals = parsedSearchVals.map((query) => | ||
| query.startsWith("C-") ? query.slice(2) : query | ||
| const patientIdsTriplets = await fetchPatientIdsTriplets( | ||
| parsedSearchVals | ||
| ); | ||
| setPatientIdsTriplets(patientIdsTriplets); | ||
|
|
||
| const customSearchVals = await fetchPatientIdsTriplets(parsedSearchVals); | ||
|
|
||
| if (customSearchVals.length > 0) { | ||
| setParsedSearchVals(customSearchVals); | ||
| if (patientIdsTriplets.length > 0) { | ||
| patientIdsTriplets.forEach((triplet) => { | ||
| // Add back C- to CMO IDs because they are stored without it in the CRDB | ||
| const cmoIdWithCDash = addCDashToCMOId(triplet.CMO_ID); | ||
| if ( | ||
| !parsedSearchVals.includes(cmoIdWithCDash) && | ||
| !parsedSearchVals.includes(triplet.DMP_ID ?? "") | ||
| ) { | ||
| extraCmoIds.push(cmoIdWithCDash); | ||
| } | ||
| }); | ||
| } else if (userEmail) { | ||
| setAlertModal({ | ||
| show: true, | ||
| ...NO_PHI_SEARCH_RESULTS, | ||
| }); | ||
|
|
||
| setParsedSearchVals([]); | ||
| } | ||
| } else { | ||
| setParsedSearchVals(parsedSearchVals); | ||
| } | ||
| return extraCmoIds; | ||
| } |
There was a problem hiding this comment.
Renamed this function to be more descriptive. Its logic mostly remains the same. Both functions query the CRDB for the patient IDs corresponding to the MRN inputs.
However, the new function simply returns those patient IDs, whereas the old function triggers the search process as well. The old function used a bad React practice that involves using redundant states.
| if (phiEnabled) { | ||
| window.addEventListener("message", handleLogin); | ||
| if (!userEmail) openLoginPopup(); | ||
| return () => { | ||
| window.removeEventListener("message", handleLogin); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Only listen for the result of the login popup/modal if the "PHI-enabled" toggle is on. Helps us avoid adding listeners excessively and slowing down the page.
| { | ||
| field: "totalSampleCount", | ||
| headerName: "# Samples", | ||
| }, | ||
| { | ||
| field: "cmoSampleIds", | ||
| headerName: "Sample IDs", | ||
| }, |
There was a problem hiding this comment.
I moved the "Sample IDs" column to the right-most side because I thought this looks more natural given how wide this column is. Also moved the "# Samples" next to it because they're both samples-related. Happy to move them back if you think otherwise.
There was a problem hiding this comment.
Most code changes here are simple renames to follow better JS naming practices (like handleDownload to onDownload).
There was a problem hiding this comment.
Most code changes here are simple renames to follow better JS naming practices (like handleDownload to onDownload).
For card #1292. This PR migrates the Patients page from OGM to using a direct Neo4j query, improving the performance of the page.