-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: Google FHIR Store Integration #135
base: master
Are you sure you want to change the base?
Conversation
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.
❌ Changes requested. Reviewed everything up to c66a1fb in 2 minutes and 10 seconds
More details
- Looked at
458
lines of code in8
files - Skipped
2
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. src/app/api/google-fhir/sync/route.ts:50
- Draft comment:
Verify the return type of getAccessToken(); ensure that 'accessToken.token' is valid. If getAccessToken returns a string, adjust the code. - Reason this comment was not posted:
Comment did not seem useful.
2. .env.example:13
- Draft comment:
Environment variable name mismatch: The code expects 'NEXT_PUBLIC_GOOGLE_FHIR_STORE_URL' but the .env.example uses 'PUBLIC_NEXT_GOOGLE_FHIR_STORE_URL'. Please rename for consistency. - Reason this comment was not posted:
Marked as duplicate.
3. src/app/api/google-fhir/sync/route.ts:11
- Draft comment:
Ensure that the environment variable name 'NEXT_PUBLIC_GOOGLE_FHIR_STORE_URL' matches what is defined in your .env file. - Reason this comment was not posted:
Marked as duplicate.
4. src/app/api/google-fhir/sync/route.ts:49
- Draft comment:
Avoid reusing the variable name 'client'. It’s already used on line 29 for the FlexpaClient. Consider renaming one of them to prevent shadowing. - Reason this comment was not posted:
Marked as duplicate.
5. src/components/google-fhir-sync.tsx:56
- Draft comment:
The extraction of the resource id from the location URL may be incorrect. If the URL includes a '_history' segment (e.g., .../Patient/12345/_history/1), use parts[parts.length - 3] instead of parts[parts.length - 2] to obtain the actual resource id. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment raises a valid technical point about FHIR URL structures. However, without seeing the actual response format from the Google FHIR API or documentation about the expected URL format, we can't be certain if this edge case applies here. The code seems to be specifically handling Google FHIR store responses, which may have a standardized format.
I might be underestimating the importance of handling versioned FHIR resources. The _history segment is a standard FHIR feature that could be relevant even in Google's implementation.
While versioning is part of FHIR, this is a new component specifically for Google FHIR sync that may have known URL patterns. Without documentation confirming the _history segment is possible here, the comment remains speculative.
The comment should be removed as it makes assumptions about URL formats without clear evidence that this edge case applies to Google FHIR store responses.
Workflow ID: wflow_hLTC9Q3JUEog8hyM
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
# GOOGLE_APPLICATION_CREDENTIALS must be set to enable Google Cloud Healthcare API integration | ||
GOOGLE_APPLICATION_CREDENTIALS= | ||
PUBLIC_NEXT_GOOGLE_FHIR_STORE_URL= |
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.
Env var name mismatch: code expects NEXT_PUBLIC_GOOGLE_FHIR_STORE_URL, but .env uses PUBLIC_NEXT_GOOGLE_FHIR_STORE_URL.
}; | ||
|
||
try { | ||
const client = await auth.getClient(); |
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.
Avoid reusing 'client' for different purposes; consider renaming the GoogleAuth client variable to prevent confusion.
Why
What
Important
Adds Google FHIR Store integration for data sync using Google Cloud Healthcare API, with new API route, UI component, and environment configuration.
src/app/api/google-fhir/sync/route.ts
for syncing data with Google FHIR Store using Google Cloud Healthcare API.GoogleAuth
fromgoogle-auth-library
for authentication.NEXT_PUBLIC_GOOGLE_FHIR_STORE_URL
format and constructsbaseUrl
for API requests.GOOGLE_APPLICATION_CREDENTIALS
andPUBLIC_NEXT_GOOGLE_FHIR_STORE_URL
to.env.example
for Google FHIR integration.GoogleFhirSync
component insrc/components/google-fhir-sync.tsx
for handling sync UI in the dashboard.Dashboard
insrc/app/dashboard/page.tsx
to include Google FHIR sync card if configured.google-auth-library
topackage.json
dependencies.next.config.ts
to useexperimental.turbo.resolveAlias
for path resolution.This description was created by
for c66a1fb. It will automatically update as commits are pushed.