feat(ehr): adding athena / elation / healthie to lambda + canvas get resources #3882
Conversation
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
WalkthroughThis set of changes removes legacy EHR bundle refresh functions and related routes, refactors appointment fetching to use a new handler-based client-credential architecture with explicit tokenId and environment parameters, introduces new core modules for client creation and secret management, updates types and interfaces to support these changes, and adds new functions for appointment and bundle retrieval across multiple EHR sources. Changes
Sequence Diagram(s)EHR Client Creation and Appointment Fetching (Handler-Based)sequenceDiagram
participant API_Service
participant ClientFactory
participant SecretsManager
participant TokenService
participant EHR_Client
participant AppointmentsHandler
API_Service->>ClientFactory: createClientWithClientCredentials(cxId, practiceId)
ClientFactory->>SecretsManager: getSecrets(cxId, practiceId, ehrSource)
SecretsManager-->>ClientFactory: secrets (env, credentials)
alt Needs token
ClientFactory->>TokenService: getTokenInfo(tokenId)
TokenService-->>ClientFactory: tokenInfo
end
ClientFactory->>EHR_Client: create(credentials, env, tokenInfo)
ClientFactory-->>API_Service: { client, tokenId, environment }
API_Service->>AppointmentsHandler: getAppointments({ client, tokenId, environment, ... })
AppointmentsHandler->>EHR_Client: fetchAppointments(params)
EHR_Client-->>AppointmentsHandler: appointments
AppointmentsHandler-->>API_Service: appointments
Resource Diff Bundle Creation (With Optional Token)sequenceDiagram
participant JobStarter
participant ClientFactory
participant ResourceDiffHandler
JobStarter->>ClientFactory: getTwoLeggedClientWithTokenIdAndEnvironment(ehr, cxId, practiceId)
ClientFactory-->>JobStarter: { client, tokenId, environment }
loop resourceTypes
JobStarter->>ResourceDiffHandler: refreshEhrBundles({ ..., tokenId, ... })
ResourceDiffHandler->>ClientFactory: getBundleByResourceType({ ..., tokenId, ... })
ClientFactory-->>ResourceDiffHandler: bundle
end
Secret Retrieval and ValidationsequenceDiagram
participant Service
participant SecretsManager
participant SchemaValidator
Service->>SecretsManager: getSecrets(ehr, cxId, practiceId)
SecretsManager->>SchemaValidator: validate(secrets)
alt Valid
SchemaValidator-->>SecretsManager: secrets (typed)
SecretsManager-->>Service: secrets
else Invalid
SchemaValidator-->>SecretsManager: error
SecretsManager-->>Service: throw BadRequestError
end
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error code ERR_SSL_WRONG_VERSION_NUMBER 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
🧬 Code Graph Analysis (2)packages/core/src/external/ehr/elation/index.ts (2)
packages/core/src/external/ehr/healthie/index.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
packages/infra/lib/ehr-nested-stack.ts (1)
203-214:⚠️ Potential issueOverly-permissive CORS policy on
EhrBundleBucketThe bucket allows
GETfrom*(any origin). WhilepublicReadAccessisfalse, exposing CORS to every origin still lets any website consume presigned URLs without restriction and may violate data-residency or privacy constraints.Consider scoping this down – e.g. to your front-end domain(s) – or omitting the CORS block entirely until a concrete cross-origin requirement emerges.
- cors: [ - { - allowedOrigins: ["*"], - allowedMethods: [s3.HttpMethods.GET], - }, - ], + // ⚠️ Review CORS before production. Lock down to specific origins if required. + cors: [ + { + allowedOrigins: ["https://app.example.com"], // tighten as needed + allowedMethods: [s3.HttpMethods.GET], + }, + ],
🧹 Nitpick comments (19)
packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-cloud.ts (1)
3-4: Consider using TypeScript path aliases
Deeply nested relative imports can be fragile. As an optional refactor, you could define a path mapping intsconfig.json(e.g.,@metriport/core/util/config) to simplify these imports and improve maintainability.packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-factory.ts (1)
1-1: Optional: adopt path aliases for Config
To avoid deep relative imports, consider defining a TypeScript path alias (e.g.,@metriport/core/util/config) intsconfig.json.packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-factory.ts (1)
1-1: Optional: simplify with TS path mappings
Consider using a project-level alias intsconfig.jsonforutil/configto reduce the nesting in relative imports.packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-cloud.ts (1)
3-4: Import path update – correct adjustment.
The relative import paths forConfigandSQSClienthave been updated to reflect the new directory structure and they resolve correctly. Consider introducing path aliases (viatsconfig.json) to avoid deep relative paths in the future.packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments-factory.ts (1)
1-1: Import path update – valid change.
TheConfigimport path has been shortened appropriately to match the reorganized directory structure. Consider using a path alias for even clearer module resolution.packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments-cloud.ts (1)
2-3: Import path corrections – approved.
Paths forConfig,out, and Lambda utilities have been shortened one level and align with the new folder layout. As an enhancement, consider adopting path aliases to reduce relative traversal.Also applies to: 8-8
packages/core/src/external/ehr/elation/command/get-appointments.ts (1)
26-28: Consider returning early for empty results caseThe function could benefit from handling the case where no appointments are found, perhaps by logging this information or returning early.
const appointments = await client.getAppointments({ cxId, fromDate, toDate }); + if (!appointments.length) { + return []; + } return appointments;packages/core/src/external/ehr/healthie/command/get-appointments.ts (1)
25-30: API parameters naming differs from function parametersThe function receives
fromDateandtoDatebut usesstartAppointmentDateandendAppointmentDatewhen calling the client. While functional, this mapping could be made more explicit for maintainability.const appointments = await client.getAppointments({ cxId, - startAppointmentDate: fromDate, - endAppointmentDate: toDate, + startAppointmentDate: fromDate, // Maps from fromDate parameter + endAppointmentDate: toDate, // Maps from toDate parameter });packages/core/src/external/ehr/athenahealth/command/get-appointments.ts (1)
6-33: Function implementation looks good with proper error handling.The implementation correctly validates required parameters with detailed error context, creates the client using the shared utility, and properly handles optional parameters.
One observation: The function renames parameters from
fromDate/toDatetostartAppointmentDate/endAppointmentDatewhen calling the client. While this works, it creates a slight mapping complexity that future developers will need to understand.packages/core/src/external/ehr/athenahealth/shared.ts (1)
7-31: Add return type annotation to the functionThe function implementation is good, but would benefit from an explicit return type annotation for better type safety and documentation.
export async function createAthenaHealthClient({ environment, cxId, practiceId, tokenId, }: { environment: string; cxId: string; practiceId: string; tokenId?: string; -}) { +}): Promise<ReturnType<typeof AthenaHealthApi.create>> {Also, consider adding error handling around the client creation to provide more context if
AthenaHealthApi.createfails.packages/lambdas/src/ehr-get-bundle-by-resource-type.ts (1)
18-18: Address the TODO commentThe TODO comment suggests moving to
capture.wrapHandler()which is a more centralized approach for error handling. This should be implemented before finalizing this PR to maintain consistent error handling across all Lambda functions.-// TODO move to capture.wrapHandler() -export const handler = Sentry.AWSLambda.wrapHandler(async (event: SQSEvent) => { +export const handler = capture.wrapHandler(async (event: SQSEvent) => {packages/core/src/external/ehr/canvas/command/get-bundle-by-resource-type.ts (2)
5-7: Function signature is correct, but consider adding JSDocThe function signature looks good but adding JSDoc with param descriptions would improve code maintainability and developer experience.
+/** + * Fetches a FHIR Bundle filtered by resource type from Canvas EHR + * + * @param params - The request parameters including environment, patient IDs, and resource type + * @returns A Promise resolving to a FHIR Bundle containing the requested resources + */ export async function getBundleByResourceType( params: GetBundleByResourceTypeClientRequest ): Promise<Bundle> {
18-23: Use object spread correctly for optional propertiesThe code uses a conditional spread for
tokenIdwhich is a good pattern, but there's a more concise way to handle this with the nullish coalescing operator.const client = await createCanvasClient({ environment, cxId, practiceId, - ...(tokenId && { tokenId }), + tokenId, });Note: This assumes
createCanvasClienthandles undefined values properly. If it does not, the current approach is acceptable.packages/infra/lib/ehr-nested-stack.ts (1)
42-50: Duplicate hard-coded memory/timeout values – extract to a shared constantThe new get-bundle-by-resource-type Lambda repeats the same
4096 MB / 12 minconfiguration used elsewhere. Duplicating these literals makes fleet-wide tuning error-prone.Create a small helper (or extend
settings()) that centralises typical “heavy EHR Lambda” settings and reuse it here and ingetAppointments,computeResourceDiffBundles, etc.
This keeps infra DRY and guarantees consistent scaling behaviour.packages/api/src/external/ehr/healthie/command/process-patients-from-appointments.ts (1)
180-181: Avoid instantiating an unused API client just to obtain the environment
createHealthieClientWithEnvironmentreturns both a client and the env, but onlyenvironmentis consumed. Constructing the client may trigger auth/token fetch logic and unnecessary network I/O for every practice.If all you need is the environment payload, expose a lighter helper (e.g.
getHealthieEnvironment) or modify the factory to allow{ createClient: false }.-const { environment } = await createHealthieClientWithEnvironment({ cxId, practiceId }); +const { environment } = await getHealthieEnvironment({ cxId, practiceId }); // pseudo-codeThis will shave latency and memory for large
process-patientsruns.packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type.ts (1)
32-41: Freeze and type-check the handler map to prevent silent runtime gapsBecause
ehrGetBundleByResourceTypeMapis declared asRecord<string, …>the compiler cannot warn when a newEhrSourceis later added without adding the corresponding handler. Marking the mapas const(or usingsatisfies) keeps the object immutable and enforces exhaustiveness.-export const ehrGetBundleByResourceTypeMap: GetBundleByResourceTypeMethodsMap = { +export const ehrGetBundleByResourceTypeMap = { [EhrSources.canvas]: { [GetBundleByResourceTypeMethods.canvasGetBundleByResourceType]: getBundleByResourceTypeCanvas, }, -}; +} as const satisfies GetBundleByResourceTypeMethodsMap;This prevents accidental mutation and gives you a compiler error the moment a new source or method is introduced without a handler.
packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments.ts (1)
50-65: Make the handler map exhaustive at compile-timeUsing
Record<string, …>tolerates typos and lets new sources slip in without handlers. Tie the keys toEhrSourceto surface gaps during compilation:-export type AppointmentMethodsMap = Record< - string, - Record<string, (params: GetAppointmentsClientRequest) => Promise<Appointment[]>> ->; +export type AppointmentMethodsMap = { + [S in EhrSource]?: { + [M in AppointmentMethods]?: ( + params: GetAppointmentsClientRequest, + ) => Promise<Appointment[]>; + }; +};(or cast the constant with
satisfies AppointmentMethodsMapas shown in the bundle file).
This enforces completeness and prevents silent runtimeundefined.packages/core/src/external/ehr/elation/shared.ts (2)
7-31: Consider adding explicit return type annotation for improved type safetyThe function correctly creates an Elation Health client, but would benefit from an explicit return type annotation to enhance type safety and documentation. This is especially important for API-facing functions.
-export async function createElationHealthClient({ +export async function createElationHealthClient({ environment, cxId, practiceId, tokenId, }: { environment: string; cxId: string; practiceId: string; tokenId?: string; -}) { +}): Promise<ReturnType<typeof ElationHealthApi.create>> {
24-24: Consider using spread operator for conditional assignmentFor conditional inclusion of the tokenId parameter, consider using the spread operator pattern that appears in other files in this PR for consistency.
- const twoLeggedAuthTokenInfo = tokenId ? await getTokenInfo(tokenId) : undefined; + const twoLeggedAuthTokenInfo = tokenId + ? await getTokenInfo(tokenId) + : undefined;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
packages/api/src/external/ehr/athenahealth/command/process-patients-from-appointments.ts(5 hunks)packages/api/src/external/ehr/canvas/command/bundle/refresh-ehr-bundle.ts(2 hunks)packages/api/src/external/ehr/canvas/command/process-patients-from-appointments.ts(1 hunks)packages/api/src/external/ehr/elation/command/process-patients-from-appointments.ts(4 hunks)packages/api/src/external/ehr/healthie/command/process-patients-from-appointments.ts(5 hunks)packages/api/src/external/ehr/healthie/shared.ts(1 hunks)packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/start-job.ts(1 hunks)packages/core/src/external/ehr/athenahealth/command/get-appointments-from-subscription-events.ts(1 hunks)packages/core/src/external/ehr/athenahealth/command/get-appointments.ts(1 hunks)packages/core/src/external/ehr/athenahealth/shared.ts(1 hunks)packages/core/src/external/ehr/canvas/command/get-appointments.ts(1 hunks)packages/core/src/external/ehr/canvas/command/get-bundle-by-resource-type.ts(1 hunks)packages/core/src/external/ehr/elation/command/get-appointments.ts(1 hunks)packages/core/src/external/ehr/elation/shared.ts(1 hunks)packages/core/src/external/ehr/healthie/command/get-appointments.ts(1 hunks)packages/core/src/external/ehr/healthie/index.ts(1 hunks)packages/core/src/external/ehr/healthie/shared.ts(1 hunks)packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-cloud.ts(1 hunks)packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-factory.ts(1 hunks)packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.ts(1 hunks)packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-cloud.ts(1 hunks)packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-factory.ts(1 hunks)packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-local.ts(1 hunks)packages/core/src/external/ehr/lambdas/appointment/get-appointments/ehr-get-appointments-local.ts(0 hunks)packages/core/src/external/ehr/lambdas/appointment/get-appointments/ehr-get-appointments.ts(0 hunks)packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments-cloud.ts(1 hunks)packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments-factory.ts(1 hunks)packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments-local.ts(1 hunks)packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments.ts(1 hunks)packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts(1 hunks)packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-factory.ts(1 hunks)packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-local.ts(1 hunks)packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type.ts(1 hunks)packages/core/src/util/config.ts(1 hunks)packages/infra/lib/api-stack.ts(3 hunks)packages/infra/lib/api-stack/api-service.ts(4 hunks)packages/infra/lib/ehr-nested-stack.ts(6 hunks)packages/lambdas/src/ehr-compute-resource-diff-bundles.ts(1 hunks)packages/lambdas/src/ehr-get-appointments.ts(1 hunks)packages/lambdas/src/ehr-get-bundle-by-resource-type.ts(1 hunks)packages/lambdas/src/ehr-refresh-ehr-bundles.ts(1 hunks)packages/lambdas/src/shared/ehr.ts(2 hunks)
💤 Files with no reviewable changes (2)
- packages/core/src/external/ehr/lambdas/appointment/get-appointments/ehr-get-appointments-local.ts
- packages/core/src/external/ehr/lambdas/appointment/get-appointments/ehr-get-appointments.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-cloud.tspackages/lambdas/src/ehr-get-appointments.tspackages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-factory.tspackages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-factory.tspackages/core/src/external/ehr/canvas/command/get-appointments.tspackages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments-factory.tspackages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.tspackages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-cloud.tspackages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-local.tspackages/core/src/util/config.tspackages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments-cloud.tspackages/lambdas/src/ehr-compute-resource-diff-bundles.tspackages/lambdas/src/ehr-refresh-ehr-bundles.tspackages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-factory.tspackages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/start-job.tspackages/core/src/external/ehr/healthie/index.tspackages/core/src/external/ehr/healthie/command/get-appointments.tspackages/api/src/external/ehr/canvas/command/process-patients-from-appointments.tspackages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-local.tspackages/infra/lib/api-stack.tspackages/core/src/external/ehr/athenahealth/command/get-appointments.tspackages/api/src/external/ehr/healthie/shared.tspackages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments-local.tspackages/core/src/external/ehr/athenahealth/command/get-appointments-from-subscription-events.tspackages/lambdas/src/ehr-get-bundle-by-resource-type.tspackages/core/src/external/ehr/elation/shared.tspackages/core/src/external/ehr/canvas/command/get-bundle-by-resource-type.tspackages/core/src/external/ehr/healthie/shared.tspackages/infra/lib/api-stack/api-service.tspackages/core/src/external/ehr/athenahealth/shared.tspackages/core/src/external/ehr/elation/command/get-appointments.tspackages/api/src/external/ehr/canvas/command/bundle/refresh-ehr-bundle.tspackages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type.tspackages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.tspackages/lambdas/src/shared/ehr.tspackages/api/src/external/ehr/elation/command/process-patients-from-appointments.tspackages/api/src/external/ehr/healthie/command/process-patients-from-appointments.tspackages/api/src/external/ehr/athenahealth/command/process-patients-from-appointments.tspackages/infra/lib/ehr-nested-stack.tspackages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments.ts
🧠 Learnings (4)
packages/lambdas/src/ehr-refresh-ehr-bundles.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3788
File: packages/api/src/external/ehr/shared/utils/bundle.ts:83-93
Timestamp: 2025-05-08T19:41:36.533Z
Learning: In the Metriport codebase, the team prefers to let errors bubble up naturally in some cases rather than adding explicit error handling at every layer, as demonstrated in the refreshEhrBundle function in the bundle.ts file.
packages/core/src/external/ehr/canvas/command/get-bundle-by-resource-type.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
packages/api/src/external/ehr/canvas/command/bundle/refresh-ehr-bundle.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
🧬 Code Graph Analysis (10)
packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-local.ts (3)
packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type.ts (3)
EhrGetBundleByResourceTypeHandler(19-21)GetBundleByResourceTypeRequest(6-17)getEhrGetBundleByResourceTypeHandler(43-55)packages/api/src/routes/medical/schemas/fhir.ts (1)
Bundle(23-23)packages/lambdas/src/ehr-get-bundle-by-resource-type.ts (1)
handler(19-39)
packages/api/src/external/ehr/healthie/shared.ts (2)
packages/api/src/external/ehr/shared/utils/client.ts (1)
EhrPerPracticeParams(30-30)packages/core/src/external/ehr/healthie/index.ts (1)
HealthieEnv(47-47)
packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments-local.ts (2)
packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments.ts (4)
EhrGetAppointmentsHandler(25-27)Appointment(39-43)GetAppointmentsRequest(13-23)getEhrGetAppointmentsHandler(67-76)packages/lambdas/src/ehr-get-appointments.ts (1)
handler(19-39)
packages/core/src/external/ehr/athenahealth/command/get-appointments-from-subscription-events.ts (2)
packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments.ts (1)
GetAppointmentsClientRequest(29-29)packages/core/src/external/ehr/athenahealth/shared.ts (1)
createAthenaHealthClient(7-31)
packages/lambdas/src/ehr-get-bundle-by-resource-type.ts (5)
packages/lambdas/src/shared/capture.ts (1)
capture(18-103)packages/lambdas/src/shared/sqs.ts (1)
getSingleMessageOrFail(62-83)packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-local.ts (1)
EhrGetBundleByResourceTypeLocal(8-17)packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type.ts (1)
GetBundleByResourceTypeRequest(6-17)packages/lambdas/src/shared/ehr.ts (1)
ehrGetBundleByResourceTypeSchema(104-115)
packages/core/src/external/ehr/elation/shared.ts (4)
packages/core/src/external/ehr/elation/index.ts (1)
isElationEnv(57-59)packages/shared/src/index.ts (1)
BadRequestError(39-39)packages/core/src/external/ehr/api/get-token-info.ts (1)
getTokenInfo(17-39)packages/core/src/external/ehr/shared.ts (1)
getOauthSecrets(53-72)
packages/core/src/external/ehr/healthie/shared.ts (3)
packages/core/src/external/ehr/healthie/index.ts (1)
isHealthieEnv(48-50)packages/shared/src/index.ts (1)
BadRequestError(39-39)packages/core/src/external/ehr/shared.ts (2)
GetSecretsApiKeyResult(48-48)getSecretsApiKeySchema(45-47)
packages/core/src/external/ehr/athenahealth/shared.ts (4)
packages/core/src/external/ehr/athenahealth/index.ts (1)
isAthenaEnv(96-98)packages/shared/src/index.ts (1)
BadRequestError(39-39)packages/core/src/external/ehr/api/get-token-info.ts (1)
getTokenInfo(17-39)packages/core/src/external/ehr/shared.ts (1)
getOauthSecrets(53-72)
packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type.ts (3)
packages/shared/src/interface/external/ehr/source.ts (1)
EhrSource(8-8)packages/lambdas/src/ehr-get-bundle-by-resource-type.ts (1)
handler(19-39)packages/shared/src/index.ts (1)
BadRequestError(39-39)
packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments.ts (4)
packages/shared/src/interface/external/ehr/source.ts (1)
EhrSource(8-8)packages/shared/src/interface/external/ehr/healthie/appointment.ts (1)
AppointmentAttendee(10-10)packages/lambdas/src/ehr-get-appointments.ts (1)
handler(19-39)packages/shared/src/index.ts (1)
BadRequestError(39-39)
🔇 Additional comments (48)
packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-cloud.ts (1)
3-4: Import path adjustment is correct
The relative imports forConfigandSQSClienthave been shortened to match the updated directory layout and are consistent with the other steps in this job.packages/core/src/external/ehr/canvas/command/get-appointments.ts (1)
4-4: Import path updated to centralized handler
This change correctly points to the newlambdas/get-appointmentsmodule forGetAppointmentsClientRequest. The logic remains unchanged and aligns with the consolidated handler-based approach.packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-factory.ts (1)
1-1: Config import path shortened
The relative path toConfighas been shortened by one directory level to reflect the reorganized folder structure.packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-factory.ts (1)
1-1: Config import path update
The import path has been shortened to mirror the new directory hierarchy; no logic changes were made.packages/lambdas/src/ehr-get-appointments.ts (1)
1-2: Handler import path corrections
The imports now correctly reference the consolidatedlambdas/get-appointmentshandlers via the@metriport/corepackage. Confirm these paths resolve properly in the TypeScript build and in downstream consumers.packages/lambdas/src/ehr-refresh-ehr-bundles.ts (1)
1-2: Import path adjustment – looks good.
The absolute imports forRefreshEhrBundlesRequestandEhrRefreshEhrBundlesLocalnow point to the updatedjobs/create-resource-diff-bundles/steps/refreshdirectory. No issues spotted.packages/lambdas/src/ehr-compute-resource-diff-bundles.ts (1)
1-2: Adjusted import paths – approved.
The imports forComputeResourceDiffBundlesRequestandEhrComputeResourceDiffBundlesLocalcorrectly reflect the newjobs/.../computedirectory structure.packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/start-job.ts (1)
2-2: Import path updated correctly.The import path for
buildEhrRefreshEhrBundlesHandlerhas been updated to reflect the new directory structure where the bundle job was moved to the newjobsfolder, aligning with the PR objectives. This change maintains consistency across the codebase.packages/api/src/external/ehr/canvas/command/process-patients-from-appointments.ts (2)
1-2: Import paths updated correctly for handler-based approach.The updated import paths appropriately reference the new location of appointment-related functionality in the
lambdasdirectory, which is consistent with the architectural changes described in the PR objectives for retrieving appointments from various EHR systems.
108-118: Implementation follows handler pattern correctly.The code properly uses the new handler-based approach for retrieving appointments, which explicitly passes environment and tokenId parameters. This aligns with the architectural changes implemented across other EHR integrations (Athena, Elation, Healthie).
packages/core/src/util/config.ts (1)
218-220: New config method added for bundle retrieval Lambda.The new
getEhrGetBundleByResourceTypeLambdaNamemethod correctly follows the established pattern for retrieving environment variables. This addition supports the new Lambda-based EHR bundle retrieval functionality mentioned in the PR objectives.packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-local.ts (1)
3-6: Import paths updated to reflect new directory structure.The import paths have been correctly updated to follow the new directory structure, where bundle-related functionality has been moved to the
jobsfolder. This maintains consistency with the overall codebase restructuring described in the PR objectives.packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.ts (1)
4-11: Import paths updated in line with directory restructuring.The relative import paths have been updated to reduce directory depth, presumably reflecting a reorganization of the project structure. The changes maintain the same imported modules while adjusting their locations.
packages/core/src/external/ehr/healthie/index.ts (1)
40-40: Improved TypeScript type definition for optional property.Changed
apiKeyfrom explicitstring | undefinedto the cleaner TypeScript optional property syntaxapiKey?: string. This change maintains the same behavior while using more idiomatic TypeScript syntax.packages/infra/lib/api-stack.ts (3)
425-425: Added new EHR Lambda for retrieving bundles by resource type.The new Lambda function for getting bundles by resource type is correctly destructured from the EHR nested stack.
561-561: Lambda properly passed to API service creation.The new Lambda is correctly passed to the API service creation function, following the existing pattern for Lambda integration.
667-667: Lambda included in API_URL environment variable recipients.The Lambda is appropriately added to the list of Lambda functions that receive the API_URL environment variable, ensuring it can make requests back to the API when needed.
packages/infra/lib/api-stack/api-service.ts (4)
116-116: Added new Lambda parameter to createAPIService function.The new EHR Lambda for retrieving bundles by resource type is correctly included in the function parameters.
163-163: Type declaration added for new Lambda parameter.The type interface for createAPIService input parameters is properly updated to include the new Lambda.
308-309: Lambda function name added to container environment variables.The Lambda function name is correctly injected into the Fargate service environment variables, following the existing naming convention with the EHR_GET_BUNDLE_BY_RESOURCE_TYPE_LAMBDA_NAME key.
455-455: Invoke permissions granted to Fargate task role.The appropriate Lambda invoke permissions are granted to the Fargate task role, allowing the API service to call the new Lambda function.
packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-factory.ts (1)
1-12: Clean implementation of the factory patternThis implementation nicely abstracts the environment-specific handling logic, returning the appropriate handler based on runtime configuration. The function is small, focused, and follows the functional programming style recommended in the guidelines.
packages/core/src/external/ehr/elation/command/get-appointments.ts (2)
10-19: Clear and comprehensive error handlingGood validation of required parameters with a detailed error message. The error structure follows the guidelines by using a static message with dynamic data in additionalInfo.
20-25: Proper client creation with optional parametersThe client creation logic is clean and handles the optional tokenId parameter correctly using object spread syntax. This approach is elegant and aligns with the coding guidelines.
packages/core/src/external/ehr/healthie/command/get-appointments.ts (2)
10-19: Good error validation patternThe input validation follows the same pattern as other EHR implementations, providing consistency across the codebase. The error message is clear and includes appropriate context.
20-24: Clean client initializationGood use of object destructuring and proper environment handling in the client creation process.
packages/lambdas/src/shared/ehr.ts (2)
2-3: Import path correctionThe updated import path for
AppointmentMethodsand the addition ofGetBundleByResourceTypeMethodsensure proper module resolution.
104-115: Well-structured schema definitionThe new schema
ehrGetBundleByResourceTypeSchemais comprehensive and follows the established pattern in the file. It properly validates all required fields and uses appropriate Zod types.packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-local.ts (1)
1-17: Clean implementation of the handler interface!This class correctly implements the
EhrGetBundleByResourceTypeHandlerinterface, following the handler pattern that's being established across the EHR integrations. The pattern of extracting the EHR type and method, then delegating to the appropriate handler is clean and maintainable.packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments-local.ts (1)
1-17: Clean implementation with a minor type safety concernThis class correctly implements the
EhrGetAppointmentsHandlerinterface, following the handler pattern being established across the EHR integrations.There's a potential type safety issue with casting the result to
T[]. If the handler returns a type incompatible withT[], TypeScript won't catch this error at compile time. Consider if there's a way to make this safer or document the assumption being made.packages/api/src/external/ehr/healthie/shared.ts (2)
103-113: Good refactoring to return both client and environment.The function
createHealthieClientWithEnvironmenthas been properly updated to return both the client and environment in a consistent object structure. This change aligns with the broader architectural pattern established across other EHR integrations.
115-120: Well-implemented backward compatibility function.The new
createHealthieClientfunction maintains backward compatibility for existing code that only needs the client. This is an excellent approach that allows for incremental adoption of the new pattern while ensuring existing code continues to work.packages/lambdas/src/ehr-get-bundle-by-resource-type.ts (1)
19-25:✅ Verification successful
Verify early return behavior
When no message is found, the function returns undefined. Ensure this behavior is expected and properly handled by the caller. Consider returning an explicit response object with a status code to maintain consistent response formats.
🏁 Script executed:
#!/bin/bash # Check how other Lambda functions handle early returns rg -A 2 -B 2 "return undefined|return;|!message \S* return" --type=ts packages/lambdas/src/Length of output: 12654
Consistent early-return pattern across Lambda handlers
I’ve verified that this Lambda follows the same
if (!message) return;pattern as all other SQS-driven handlers in packages/lambdas/src (e.g. elation-link-patient.ts, healthie-link-patient.ts, ehr-sync-patient.ts, etc.). These functions are wrapped withSentry.AWSLambda.wrapHandler, so returningundefinedsimply ends execution without error, which is the intended behavior. No explicit HTTP-like response object is expected by the caller.Action: No changes required here.
packages/core/src/external/ehr/canvas/command/get-bundle-by-resource-type.ts (1)
24-31: LGTM - Client usage is consistent with other implementationsThe function correctly calls the Canvas client's
getBundleByResourceTypemethod with all the required parameters, and returns the bundle as is.packages/core/src/external/ehr/healthie/shared.ts (1)
7-33: Function implementation follows best practicesThe function correctly validates the environment before proceeding, throws appropriate errors with context, and creates the client with async secret retrieval. It adheres to the coding guidelines with proper typing, error handling, and functional style.
However, consider adding JSDoc comments to improve code documentation:
+/** + * Creates a Healthie API client for the specified environment and practice + * + * @param environment - The Healthie environment to connect to + * @param cxId - The customer ID for secret retrieval + * @param practiceId - The Healthie practice ID + * @returns A Promise resolving to a configured Healthie API client + * @throws {BadRequestError} When the environment is invalid + */ export async function createHealthieClient({ environment, cxId, practiceId, }: { environment: string; cxId: string; practiceId: string; }) {packages/api/src/external/ehr/elation/command/process-patients-from-appointments.ts (5)
2-3: Good use of specific importsThe imports follow best practices by importing specific modules rather than entire packages, which helps with tree-shaking and bundle size optimization.
13-13: Appropriate type importThe
BookedAppointmentimport is correctly added to support type checking for the refactored appointment handling logic.
27-27: Updated import for client creationThe import correctly switches from the previously used client creation function to the new one that also returns environment information.
149-152: Destructuring used appropriatelyThe code correctly destructures both
tokenIdandenvironmentfrom the client creation function, which aligns with the new handler-based pattern.
158-168: Well-structured handler usageThe code now uses the handler-based approach correctly, passing all necessary parameters including the EHR source, environment, token ID, and method enum.
One small suggestion - consider using explicit type parameters for better clarity:
-const handler = buildEhrGetAppointmentsHandler(); +const handler = buildEhrGetAppointmentsHandler<BookedAppointment>();packages/api/src/external/ehr/athenahealth/command/process-patients-from-appointments.ts (2)
154-157: ValidatetokenIdandenvironmentbefore proceeding
createAthenaClientWithTokenIdAndEnvironment()can theoretically returnundefinedvalues (e.g. if token refresh fails).
Passing them unchecked into the handler would surface only deep inside the call-chain and be hard to trace.-const { tokenId, environment, client } = - await createAthenaClientWithTokenIdAndEnvironment({ cxId, practiceId }); +const { + tokenId, + environment, + client, +} = await createAthenaClientWithTokenIdAndEnvironment({ cxId, practiceId }); + +if (!tokenId || !environment) { + log("Missing tokenId or environment – skipping appointment fetch"); + return { error: new Error("Missing auth context") }; +}A cheap guard here avoids multiple nested catches further down the stack.
214-224: ConfirmstartRange/endRangeareDateinstances, notdayjsobjects
GetAppointmentsRequestexpects plainDatetypes, yetgetLookForwardTimeRange()returnsdayjsinstances in several older helpers. Passing the wrong type compiles (due to structural typing) but will break JSON serialisation in the Lambda call.const { startRange, endRange } = getLookForwardTimeRange({ lookForward: appointmentsLookForward }); log(`Getting appointments from ${startRange} to ${endRange}`); return await handler.getAppointments<BookedAppointment>({ ...handlerParams, method: AppointmentMethods.athenaGetAppointments, fromDate: startRange.toDate?.() ?? startRange, toDate: endRange.toDate?.() ?? endRange, });Please verify the helper returns
Dateor convert explicitly.packages/core/src/external/ehr/elation/shared.ts (1)
7-31: Well-structured client creation with proper error handlingThe implementation follows good practices by:
- Validating the environment before proceeding
- Including contextual information in error messages
- Properly handling optional token retrieval
- Using async/await consistently
- Following the established factory pattern for client creation
packages/api/src/external/ehr/canvas/command/bundle/refresh-ehr-bundle.ts (2)
14-30: Good refactoring to handler-based architectureThe refactoring from direct client usage to a handler-based approach improves consistency across EHR integrations and enables serverless execution via Lambda. This architectural change:
- Makes environment and token handling more explicit
- Standardizes the approach across different EHR providers
- Improves separation of concerns
27-27: Improved parameter naming for better generalizationRenaming
canvasPatientIdtoehrPatientIdmakes the code more generic and reusable across different EHR providers, which aligns with the broader refactoring goals of this PR.packages/core/src/external/ehr/athenahealth/command/get-appointments-from-subscription-events.ts (3)
5-22: Consider adding explicit return type annotationThe function would benefit from an explicit return type annotation for improved clarity and type safety:
export async function getAppointmentsFromSubscriptionEvents( params: GetAppointmentsClientRequest -): Promise<BookedAppointment[]> { +): Promise<BookedAppointment[]> {Though the return type is already correctly specified, I'm highlighting this as good practice that should be maintained.
13-20: Clean conditional property inclusion patternThe use of spread operators with logical AND for conditional property inclusion (e.g.,
...(tokenId && { tokenId })) is a clean pattern that:
- Keeps the code concise
- Makes it clear which properties are optional
- Avoids undefined values in the resulting object
This pattern is used consistently for all optional parameters, which is excellent.
1-22: Consistent approach to client creation and appointment fetchingThis implementation follows the same architectural pattern established across other EHR providers:
- Takes standardized parameters
- Creates a client using the appropriate factory function
- Calls the specific client method
- Returns typed results
This consistency makes the codebase more maintainable and easier to understand.
...rc/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts
Outdated
Show resolved
Hide resolved
...core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type.ts
Outdated
Show resolved
Hide resolved
packages/core/src/external/ehr/lambdas/get-appointments/ehr-get-appointments.ts
Outdated
Show resolved
Hide resolved
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-direct.ts (1)
31-35: Improve type safety of the validation function.The type guard implementation could be more robust and follows functional programming principles well.
function isGetBundleByResourceTypeMethod(method: string): method is GetBundleByResourceTypeMethods { - return Object.values(GetBundleByResourceTypeMethods).includes( - method as GetBundleByResourceTypeMethods - ); + return (Object.values(GetBundleByResourceTypeMethods) as string[]).includes(method); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
packages/api/src/external/ehr/athenahealth/command/process-patients-from-appointments.ts(5 hunks)packages/api/src/external/ehr/canvas/command/bundle/refresh-ehr-bundle.ts(2 hunks)packages/api/src/external/ehr/canvas/command/process-patients-from-appointments.ts(1 hunks)packages/api/src/external/ehr/elation/command/process-patients-from-appointments.ts(4 hunks)packages/api/src/external/ehr/healthie/command/process-patients-from-appointments.ts(5 hunks)packages/core/src/external/ehr/athenahealth/command/get-appointments-from-subscription-events.ts(1 hunks)packages/core/src/external/ehr/athenahealth/command/get-appointments.ts(1 hunks)packages/core/src/external/ehr/canvas/command/get-appointments.ts(1 hunks)packages/core/src/external/ehr/canvas/command/get-bundle-by-resource-type.ts(1 hunks)packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments-cloud.ts(1 hunks)packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments-direct.ts(1 hunks)packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments-factory.ts(1 hunks)packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments.ts(1 hunks)packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts(1 hunks)packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-direct.ts(1 hunks)packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-factory.ts(1 hunks)packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type.ts(1 hunks)packages/core/src/external/ehr/elation/command/get-appointments.ts(1 hunks)packages/core/src/external/ehr/healthie/command/get-appointments.ts(1 hunks)packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.ts(1 hunks)packages/core/src/util/config.ts(1 hunks)packages/infra/lib/api-stack.ts(3 hunks)packages/infra/lib/api-stack/api-service.ts(4 hunks)packages/infra/lib/ehr-nested-stack.ts(6 hunks)packages/lambdas/src/ehr-get-bundle-by-resource-type.ts(1 hunks)packages/lambdas/src/shared/ehr.ts(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- packages/api/src/external/ehr/canvas/command/process-patients-from-appointments.ts
- packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments-cloud.ts
- packages/core/src/external/ehr/jobs/create-resource-diff-bundles/steps/compute/ehr-compute-resource-diff-bundles-local.ts
- packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-factory.ts
- packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments.ts
- packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments-factory.ts
- packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/core/src/external/ehr/canvas/command/get-appointments.ts
- packages/lambdas/src/shared/ehr.ts
- packages/core/src/util/config.ts
- packages/infra/lib/api-stack.ts
- packages/api/src/external/ehr/canvas/command/bundle/refresh-ehr-bundle.ts
- packages/core/src/external/ehr/athenahealth/command/get-appointments-from-subscription-events.ts
- packages/core/src/external/ehr/athenahealth/command/get-appointments.ts
- packages/infra/lib/api-stack/api-service.ts
- packages/infra/lib/ehr-nested-stack.ts
- packages/api/src/external/ehr/healthie/command/process-patients-from-appointments.ts
- packages/api/src/external/ehr/athenahealth/command/process-patients-from-appointments.ts
- packages/lambdas/src/ehr-get-bundle-by-resource-type.ts
- packages/core/src/external/ehr/elation/command/get-appointments.ts
- packages/core/src/external/ehr/healthie/command/get-appointments.ts
- packages/api/src/external/ehr/elation/command/process-patients-from-appointments.ts
- packages/core/src/external/ehr/canvas/command/get-bundle-by-resource-type.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.tspackages/core/src/external/ehr/command/get-appointments/ehr-get-appointments-direct.tspackages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-direct.ts
🧠 Learnings (1)
packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-direct.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
🔇 Additional comments (16)
packages/core/src/external/ehr/command/get-appointments/ehr-get-appointments-direct.ts (7)
1-13: LGTM: Clean import organizationThe imports are well-organized with proper aliasing to avoid naming conflicts and meaningful import paths.
15-27: LGTM: Well-implemented handler class with proper error handlingThe class follows the coding guidelines correctly:
- PascalCase naming
- Proper async/await usage
- Destructuring parameters
- Static error messages with dynamic data in
additionalInfo- Type-safe implementation with generic constraint
The type casting on line 25 is acceptable here since the handler is validated and type-safe through the mapping system.
29-35: LGTM: Well-defined enum following naming conventionsThe enum follows the coding guidelines with:
- PascalCase enum name
- camelCase enum values
- Descriptive naming that clearly indicates the EHR source and action
37-39: LGTM: Proper type predicate implementationThe type guard function correctly uses TypeScript's type predicate feature to narrow down the type, following the guideline about type predicates being used for type narrowing.
41-44: LGTM: Clean type definitionThe type definition uses proper TypeScript Record type and follows PascalCase naming convention for types.
46-61: LGTM: Well-structured mapping objectThe mapping object follows the coding guidelines:
- camelCase constant naming
- Consistent structure across EHR sources
- Clear separation of concerns for different appointment methods
63-72: LGTM: Robust handler retrieval with proper error handlingThe function demonstrates good practices:
- camelCase naming
- Optional chaining for safe property access
- Static error message with dynamic data in
additionalInfo- Clear return type specification
- Defensive programming with null/undefined checks
packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts (2)
16-25: LGTM: Well-designed constructor with proper dependency injection.The constructor follows good practices with optional dependency injection for the Lambda client and proper fallback to default AWS region configuration.
16-25: LGTM: Well-structured constructor with proper dependency injection.The constructor follows good dependency injection patterns with optional parameters and sensible defaults. The use of nullish coalescing operator (??) aligns with the coding guidelines preference over the OR operator.
packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-direct.ts (7)
11-25: LGTM: Clean implementation with proper validation and error handling.The method correctly validates the method parameter and delegates to the appropriate handler with proper error context.
27-29: Consider future extensibility of the enum.Currently only supports Canvas, but the pattern is well-designed for adding other EHR sources. The naming convention follows the coding guidelines.
37-46: Well-structured handler map with clear type definitions.The map structure properly separates concerns by EHR source and method, making it easy to extend for additional sources and methods.
11-25: LGTM: Excellent implementation following functional programming principles.The class method properly uses destructuring, validates input parameters, and maintains separation of concerns. The error handling follows the coding guidelines with proper use of
BadRequestErrorandadditionalInfofor dynamic data.
27-35: LGTM: Proper enum and type guard implementation.The enum follows PascalCase naming convention and the type guard function is implemented correctly using
Object.values()for runtime validation.
37-46: LGTM: Well-designed handler map with proper typing.The type definition and handler map follow good patterns for extensibility. The nested Record structure provides type safety while allowing for easy addition of new EHR sources and methods.
48-60: LGTM: Robust handler retrieval with proper error handling.The function uses optional chaining appropriately and provides clear error messages with relevant context in
additionalInfo. This follows the coding guidelines for error handling.
...rc/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts
Outdated
Show resolved
Hide resolved
...c/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-direct.ts
Outdated
Show resolved
Hide resolved
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net> F438
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
There was a problem hiding this comment.
moved to lambda directly
| }; | ||
|
|
||
| const bundleFunctionsByEhr: Record<EhrSources, BundleFunctions | undefined> = { | ||
| const bundleFunctionsByEhr: Record<EhrSource, BundleFunctions | undefined> = { | ||
| [EhrSources.canvas]: { |
There was a problem hiding this comment.
This is being fully deleted in the next PR so no need to flatten it here #3883
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
packages/core/src/external/ehr/command/ehr-get-bundle-by-resource-type.ts
Outdated
Show resolved
Hide resolved
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 2684 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/start-job.ts(3 hunks)packages/core/src/external/ehr/canvas/command/get-bundle-by-resource-type.ts(1 hunks)packages/core/src/external/ehr/command/get-bundle-by-resource-type.ts(1 hunks)packages/core/src/external/ehr/job/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-local.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/external/ehr/canvas/command/get-bundle-by-resource-type.ts
- packages/core/src/external/ehr/job/create-resource-diff-bundles/steps/refresh/ehr-refresh-ehr-bundles-local.ts
- packages/api/src/external/ehr/shared/job/bundle/create-resource-diff-bundles/start-job.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/core/src/external/ehr/command/get-bundle-by-resource-type.ts
🧠 Learnings (1)
packages/core/src/external/ehr/command/get-bundle-by-resource-type.ts (3)
Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:27-49
Timestamp: 2025-05-28T19:22:09.281Z
Learning: In packages/core/src/external/ehr/command/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.
Learnt from: thomasyopes
PR: metriport/metriport#3608
File: packages/core/src/external/ehr/canvas/index.ts:451-469
Timestamp: 2025-04-21T17:07:30.574Z
Learning: The `getMetriportOnlyBundleByResourceType` method in CanvasApi returns `Promise<Bundle | undefined>` because the Metriport-only bundle is a computed artifact that may not exist yet. In contrast, `getBundleByResourceType` returns `Promise<Bundle>` because it can always fetch from the EHR API if the S3 cached bundle doesn't exist.
Learnt from: thomasyopes
PR: metriport/metriport#3882
File: packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts:0-0
Timestamp: 2025-05-28T19:20:47.442Z
Learning: In packages/core/src/external/ehr/lambdas/get-bundle-by-resource-type/ehr-get-bundle-by-resource-type-cloud.ts, the EHR get bundle by resource type Lambda endpoint is guaranteed to return valid JSON, so JSON.parse() error handling is not necessary for this specific endpoint.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
🔇 Additional comments (4)
packages/core/src/external/ehr/command/get-bundle-by-resource-type.ts (4)
1-4: LGTM - Clean imports following coding guidelines.The imports are well-organized and follow the established patterns. The aliasing of the Canvas import to
getBundleByResourceTypeCanvasimproves clarity.
6-17: Well-structured type definitions.The type definitions follow TypeScript best practices with meaningful names and proper use of the
Omitutility type for the client request variant.
19-25: Clean implementation following functional programming principles.The function properly uses destructuring, async/await, and the handler pattern. The implementation is concise and follows the coding guidelines for functional programming style.
39-47: Excellent error handling following coding guidelines.The error handling implementation correctly follows the guidelines:
- Uses a static error message
- Passes dynamic data (
ehr) toadditionalInfo- Proper type safety with the handler lookup and validation
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
…riport/metriport into 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
| import AthenaHealthApi, { AthenaEnv } from "./athenahealth"; | ||
| import CanvasApi, { CanvasEnv } from "./canvas"; | ||
| import { EClinicalWorksEnv } from "./eclinicalworks"; | ||
| import ElationApi, { ElationEnv } from "./elation"; | ||
| import { HealthieEnv } from "./healthie"; | ||
|
|
||
| export type EhrEnv = AthenaEnv | ElationEnv | CanvasEnv | HealthieEnv | EClinicalWorksEnv; | ||
|
|
||
| const EhrSourcesWithClientCredentials = [ |
| import { EhrSources } from "@metriport/shared/interface/external/ehr/source"; | ||
| import { z } from "zod"; | ||
|
|
||
| const EhrSourcesWithDynamicSecrets = [ |
| @@ -31,7 +32,9 @@ const MAX_AGE = dayjs.duration(24, "hours"); | |||
| const region = Config.getAWSRegion(); | |||
| const responsesBucket = Config.getEhrResponsesBucketName(); | |||
|
|
|||
| export const paginateWaitTime = dayjs.duration(5, "seconds").asMilliseconds(); | |||
| export const paginateWaitTime = dayjs.duration(1, "seconds").asMilliseconds(); | |||
There was a problem hiding this comment.
I know this was approved on the previous PR, but if you send a new commit can you change this so we don't convert to millis when we create the const, please?
There was a problem hiding this comment.
Done.
… 199-elation-contribution-1.5.1 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-199 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Issues:
Dependencies
Description
Testing
Release Plan
Summary by CodeRabbit