CDCgov / dibbs-query-connector

A FHIR client allowing public health agencies to query health care organizations directly or via a TEFCA QHIN
Creative Commons Zero v1.0 Universal
4 stars 0 forks source link

Add function to check whether valueset data exists in the DB #152

Closed m-goggins closed 1 week ago

m-goggins commented 1 week ago

PULL REQUEST

Summary

This PR adds a function called checkDBForData to, well, check whether the DB already contains valuesets. We need this function to determine whether we should proceed with fetching data from the eRSD and inserting custom valuesets in createDibbsDB.

Since we moved createDibbsDB to called every time an admin user clicks on Create Query on the My Queries page, it is especially important that the check operate very quickly and move to the next part of query creation if the DB already contains data. To accomplish this, I used an estimation of the number of rows rather than a straight COUNT(), which dramatically increases the speed. Right now, the function just checks valuesets and checks whether the table has more than 0, but we could increase to check whether the estimated number of rows is greater than say 1000 since we know that the eRSD contains more than 1000 valuesets.

Related Issue

Fixes #150

Additional Information

Anything else the review team should know?

Checklist

m-goggins commented 1 week ago

@robertandremitchell Ah, yeah, testing requires a little finagling. The easiest way is to drop all the migrations except the first one. And then un-comment this function. You can run the start up twice to ensure it runs the first time and then does not run the second time. Note: you might get errors on the createDibbsDB function

OR

You could keep the migrations as they currently are and uncomment the function. You should see that the insertions via createDibbsDB do not run a second time.

robertandremitchell commented 1 week ago

@robertandremitchell Ah, yeah, testing requires a little finagling. The easiest way is to drop all the migrations except the first one. And then un-comment this function. You can run the start up twice to ensure it runs the first time and then does not run the second time. Note: you might get errors on the createDibbsDB function

OR

You could keep the migrations as they currently are and uncomment the function. You should see that the insertions via createDibbsDB do not run a second time.

Thanks! tried for option 1 and am getting this error after hitting the Create query button:

query-connector-1  | TypeError: Cannot read properties of undefined (reading '0')
query-connector-1  |     at /app/.next/server/chunks/72.js:9:1686
query-connector-1  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
query-connector-1  |     at async _ (/app/.next/server/chunks/72.js:9:1656)
query-connector-1  |     at async /app/.next/server/app/queryBuilding/page.js:1:5159
query-connector-1  |     at async Promise.all (index 10)
query-connector-1  |     at async u (/app/.next/server/app/queryBuilding/page.js:1:5115)
query-connector-1  |     at async c (/app/.next/server/app/queryBuilding/page.js:1:5870)
query-connector-1  |     at async /app/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:16:418
query-connector-1  |     at async rE (/app/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:15:7978)
query-connector-1  |     at async r7 (/app/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:18:1144)
query-connector-1  | TypeError: Cannot read properties of undefined (reading '0')

tried a few different times with a clean cache, dropping orphans, etc., updating to have the same setup for the button as @bamader has, but still no dice 😕

bamader commented 1 week ago

@robertandremitchell Ah I think you might have fallen for a slight trap in the migrations. Migration 6 adds a single column to the valuesets table, so to successfully use Option 1, not only do you have to delete migrations 2-6, you also have to add the column dibbs_concept_type TEXT as the last entry in the valuesets table in Migration 1. Does that fix things for you?

m-goggins commented 1 week ago

I'm going to remove the migrations that we don't need since they are becoming a pain to current PRs.

robertandremitchell commented 1 week ago

@robertandremitchell Ah I think you might have fallen for a slight trap in the migrations. Migration 6 adds a single column to the valuesets table, so to successfully use Option 1, not only do you have to delete migrations 2-6, you also have to add the column dibbs_concept_type TEXT as the last entry in the valuesets table in Migration 1. Does that fix things for you?

hrm, that makes sense, but still getting the same error unfortunately, both with docker and npm run dev after a clean build.