KitwareMedical / lungair-web-application

Web application based on VolView for AI-based BPD risk analysis
4 stars 0 forks source link

Add fhir-client and EHR login capability #2

Closed jadh4v closed 1 year ago

jadh4v commented 1 year ago

This PR does the following:

ebrahimebrahim commented 1 year ago

While following the instructions in the README for setting up the vite proxy server, I ran into issues at the npm install command. Here is the relevant snippet from the npm log:

74 verbose Linux 5.15.0-78-generic
75 verbose node v18.14.0
76 verbose npm  v9.3.1
77 error code 1
78 error path /home/ebrahim/lungair-web-application/lungair/orthanc-proxy/node_modules/esbuild
79 error command failed
80 error command sh -c node install.js
81 error /home/ebrahim/lungair-web-application/lungair/orthanc-proxy/node_modules/esbuild/install.js:132
81 error     throw new Error(`Expected ${JSON.stringify(versionFromPackageJSON)} but got ${JSON.stringify(stdout)}`);
81 error           ^
81 error
81 error Error: Expected "0.18.17" but got "0.17.19"
81 error     at validateBinaryVersion (/home/ebrahim/lungair-web-application/lungair/orthanc-proxy/node_modules/esbuild/install.js:132:11)
81 error     at /home/ebrahim/lungair-web-application/lungair/orthanc-proxy/node_modules/esbuild/install.js:285:5
81 error
81 error Node.js v18.14.0
82 verbose exit 1
ebrahimebrahim commented 1 year ago

Using the LungAIR EHR tab I got the Cerner popup and logged in as one of the test patients, then got the patient dump in the frame. Really neat!

When I do it again without refreshing, hitting the login button and logging in as a different patient, the displayed data did not change. Ultimately we are not going for a patient-facing app, so I am not sure if we should worry about this... but probably we should get it right anyway to ensure our early designs can support what we eventually want to do.

jadh4v commented 1 year ago

81 error Error: Expected "0.18.17" but got "0.17.19" 81 error at validateBinaryVersion (/home/ebrahim/lungair-web-application/lungair/orthanc-proxy/node_modules/esbuild/install.js:132:11) 81 error at /home/ebrahim/lungair-web-application/lungair/orthanc-proxy/node_modules/esbuild/install.js:285:5

Can you try deleting the existing node_modules directory, delete package-lock.json and try npm install again.

ebrahimebrahim commented 1 year ago

Can you try deleting the existing node_modules directory, delete package-lock.json and try npm install again.

That worked (for the lungair/orthanc-proxy node project -- deleting package-lock.json for the main (volview) node project causes it to fail to build on my machine)

jadh4v commented 1 year ago

Using the LungAIR EHR tab I got the Cerner popup and logged in as one of the test patients, then got the patient dump in the frame. Really neat!

When I do it again without refreshing, hitting the login button and logging in as a different patient, the displayed data did not change. Ultimately we are not going for a patient-facing app, so I am not sure if we should worry about this... but probably we should get it right anyway to ensure our early designs can support what we eventually want to do.

It looks like this is a weird use-case for fhirclient because it doesn't work with multiple authorization calls without refreshing the entire page first. It is fixable by patching the fhirclient, but I would wait to see if this is really a required flow. When working on provider facing app, we may not need to login multiple times, or just trigger a full refresh for another login.

ebrahimebrahim commented 1 year ago

It looks like this is a weird use-case for fhirclient because it doesn't work with multiple authorization calls without refreshing the entire page first. It is fixable by patching the fhirclient, but I would wait to see if this is really a required flow. When working on provider facing app, we may not need to login multiple times, or just trigger a full refresh for another login.

I see, agreed. It's not a required flow at all. Just wanted to be sure we weren't doing something wrong, and it seems we are not.

ebrahimebrahim commented 1 year ago

I ran a fresh test with the newly updated package-lock and .env and it worked quite well. Uncommenting the server module in ModulePanel.vue also worked and I was able to talk to the python server from the application :tada:

jadh4v commented 1 year ago

It looks like this is a weird use-case for fhirclient because it doesn't work with multiple authorization calls without refreshing the entire page first. It is fixable by patching the fhirclient, but I would wait to see if this is really a required flow. When working on provider facing app, we may not need to login multiple times, or just trigger a full refresh for another login.

I see, agreed. It's not a required flow at all. Just wanted to be sure we weren't doing something wrong, and it seems we are not.

To document the findings: Since v2.x, fhirclient doesn't support passing of code and state values as function parameters when calling FHIR.oauth2.Ready(). This means that these values are read by the FHIR module through the global location variable (see here). Currently, the global variable is only fetched the first time and the url string is cached into a member variable of BrowserAdapter. For future calls, instead of re-reading from the window's actual location, the cached value is used. This prevents the second login from succeeding. And, currently there is no public function exposed that could be used to reset the state of FHIR module. Refreshing the page is the only way to do this.

I am not certain if this is a bug in fhir-client or if this is simply not considered a valid use case where an app could allow a second login without resetting state.