fix(core): uses buildDayjs and UTC for medication resources, adds "outpatient" category, and minor fixes #4048
Conversation
metriport/metriport-internal#1040 Signed-off-by: Keshav Saharia <keshav@metriport.com>
…from HIEs metriport/metriport-internal#1040 Signed-off-by: Keshav Saharia <keshav@metriport.com>
… pharmacy data sources metriport/metriport-internal#1040 Signed-off-by: Keshav Saharia <keshav@metriport.com>
WalkthroughThe updates standardize date handling in medication-related modules by replacing direct Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant MedicationRequestModule
Caller->>MedicationRequestModule: getMedicationRequest(detail)
MedicationRequestModule->>MedicationRequestModule: getAuthoredOn(detail)
MedicationRequestModule->>MedicationRequestModule: buildDayjs(detail.dateWritten).toISOString()
MedicationRequestModule->>MedicationRequestModule: getDispenseCategory()
MedicationRequestModule-->>Caller: MedicationRequest object (with category and authoredOn)
sequenceDiagram
participant Caller
participant MedicationDispenseModule
Caller->>MedicationDispenseModule: getWhenHandedOver(detail)
MedicationDispenseModule->>MedicationDispenseModule: buildDayjs(detail.dateWritten).toISOString()
MedicationDispenseModule-->>Caller: ISO date string
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 ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 0
🧹 Nitpick comments (2)
packages/core/src/external/surescripts/fhir/medication-request.ts (2)
19-19: Optimize unnecessary conditional check.Since
getDispenseCategory()always returns a non-empty array, the conditional check is unnecessary and adds complexity.- const category = getDispenseCategory(); + const category = getDispenseCategory();- ...(category ? { category } : undefined), + category,Also applies to: 26-26
44-55: Consider more descriptive function naming.The function name
getDispenseCategorymight be misleading since it returns a medication request category, not a dispense-specific category. The implementation correctly provides the outpatient category as specified in the PR objectives.Consider renaming to be more descriptive:
-function getDispenseCategory(): MedicationRequest["category"] { +function getMedicationRequestCategory(): MedicationRequest["category"] {And update the usage accordingly:
- const category = getDispenseCategory(); + const category = getMedicationRequestCategory();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/external/surescripts/fhir/medication-dispense.ts(2 hunks)packages/core/src/external/surescripts/fhir/medication-request.ts(3 hunks)
🧰 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/surescripts/fhir/medication-dispense.tspackages/core/src/external/surescripts/fhir/medication-request.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: keshavsaharia
PR: metriport/metriport#3885
File: packages/core/src/external/surescripts/client.ts:0-0
Timestamp: 2025-05-30T01:09:55.013Z
Learning: In packages/core/src/external/surescripts/client.ts, the Surescripts date formatting should use local time according to their documentation ("message-date-must-be-local"), which means buildDayjs() cannot be used since it defaults to UTC. The specific use case for transmission date formatting requires dayjs to be used directly to preserve local time behavior.
packages/core/src/external/surescripts/fhir/medication-dispense.ts (1)
Learnt from: keshavsaharia
PR: metriport/metriport#3885
File: packages/core/src/external/surescripts/client.ts:0-0
Timestamp: 2025-05-30T01:09:55.013Z
Learning: In packages/core/src/external/surescripts/client.ts, the Surescripts date formatting should use local time according to their documentation ("message-date-must-be-local"), which means buildDayjs() cannot be used since it defaults to UTC. The specific use case for transmission date formatting requires dayjs to be used directly to preserve local time behavior.
packages/core/src/external/surescripts/fhir/medication-request.ts (2)
Learnt from: thomasyopes
PR: metriport/metriport#3970
File: packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts:17-17
Timestamp: 2025-06-06T16:45:31.832Z
Learning: The writeMedicationToChart function in packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts returns a response that is not currently used by any consumers, so changes to its return type are not breaking changes in practice.
Learnt from: keshavsaharia
PR: metriport/metriport#3885
File: packages/core/src/external/surescripts/client.ts:0-0
Timestamp: 2025-05-30T01:09:55.013Z
Learning: In packages/core/src/external/surescripts/client.ts, the Surescripts date formatting should use local time according to their documentation ("message-date-must-be-local"), which means buildDayjs() cannot be used since it defaults to UTC. The specific use case for transmission date formatting requires dayjs to be used directly to preserve local time behavior.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/core/src/external/surescripts/fhir/medication-dispense.ts (2)
8-8: LGTM: Import follows coding guidelines.Adding the buildDayjs import aligns with the coding guideline to "Always use buildDayjs() to create dayjs instances."
53-53: LGTM: Date handling standardization.The change from direct
toISOString()tobuildDayjs().toISOString()correctly standardizes date handling for FHIR resources and ensures UTC formatting as intended by the PR objectives.packages/core/src/external/surescripts/fhir/medication-request.ts (2)
2-2: LGTM: Import follows coding guidelines.Adding the buildDayjs import aligns with the coding guideline to "Always use buildDayjs() to create dayjs instances."
59-59: LGTM: Date handling standardization.The change from direct
toISOString()tobuildDayjs().toISOString()correctly standardizes date handling for FHIR resources and ensures UTC formatting as intended by the PR objectives.
|
|
metriport/metriport-internal#1040 Signed-off-by: Keshav Saharia <keshav@metriport.com>
metriport/metriport-internal#1040 Signed-off-by: Keshav Saharia <keshav@metriport.com>
metriport/metriport-internal#1040 Signed-off-by: Keshav Saharia <keshav@metriport.com>
metriport/metriport-internal#1040 Signed-off-by: Keshav Saharia <keshav@metriport.com>
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 (12)
packages/core/src/external/sftp/command/sftp-action/sftp-action-cloud.ts(0 hunks)packages/core/src/external/sftp/command/sftp-action/sftp-action-direct.ts(1 hunks)packages/core/src/external/sftp/command/sftp-action/sftp-action.ts(1 hunks)packages/core/src/external/surescripts/fhir/coverage.ts(1 hunks)packages/core/src/external/surescripts/fhir/medication.ts(5 hunks)packages/core/src/external/surescripts/fhir/provenance.ts(0 hunks)packages/core/src/external/surescripts/fhir/shared.ts(4 hunks)packages/core/src/external/term-server/index.ts(2 hunks)packages/lambdas/src/surescripts/convert-batch-response.ts(1 hunks)packages/lambdas/src/surescripts/convert-patient-response.ts(1 hunks)packages/utils/src/surescripts/analysis.ts(2 hunks)packages/utils/src/surescripts/convert-customer-response.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/core/src/external/sftp/command/sftp-action/sftp-action-cloud.ts
- packages/core/src/external/surescripts/fhir/provenance.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/utils/src/surescripts/convert-customer-response.tspackages/core/src/external/term-server/index.tspackages/core/src/external/sftp/command/sftp-action/sftp-action-direct.tspackages/core/src/external/surescripts/fhir/coverage.tspackages/utils/src/surescripts/analysis.tspackages/lambdas/src/surescripts/convert-batch-response.tspackages/lambdas/src/surescripts/convert-patient-response.tspackages/core/src/external/sftp/command/sftp-action/sftp-action.tspackages/core/src/external/surescripts/fhir/medication.tspackages/core/src/external/surescripts/fhir/shared.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: keshavsaharia
PR: metriport/metriport#3885
File: packages/core/src/external/surescripts/client.ts:0-0
Timestamp: 2025-05-30T01:09:55.013Z
Learning: In packages/core/src/external/surescripts/client.ts, the Surescripts date formatting should use local time according to their documentation ("message-date-must-be-local"), which means buildDayjs() cannot be used since it defaults to UTC. The specific use case for transmission date formatting requires dayjs to be used directly to preserve local time behavior.
packages/core/src/external/surescripts/fhir/medication.ts (1)
Learnt from: thomasyopes
PR: metriport/metriport#3970
File: packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts:17-17
Timestamp: 2025-06-06T16:45:31.832Z
Learning: The writeMedicationToChart function in packages/api/src/external/ehr/athenahealth/command/write-back/medication.ts returns a response that is not currently used by any consumers, so changes to its return type are not breaking changes in practice.
packages/core/src/external/surescripts/fhir/shared.ts (1)
Learnt from: keshavsaharia
PR: metriport/metriport#4045
File: packages/core/src/external/surescripts/fhir/shared.ts:87-93
Timestamp: 2025-06-18T18:50:40.948Z
Learning: The `getResourceFromResourceMap` function in `packages/core/src/external/surescripts/fhir/shared.ts` works correctly as implemented. The comparison `resourceValue === resourceMap[key]` where `resourceValue = resource[key]` is intentional and functions as designed, despite appearing to compare different types.
⏰ 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: Analyze (javascript)
🔇 Additional comments (21)
packages/utils/src/surescripts/analysis.ts (3)
40-40: LGTM: Clean addition of output file option.The new
--out-fileoption is properly added to the command configuration.
46-46: LGTM: Parameter handling is correct.The parameter destructuring and type definition properly include the new optional
outFileparameter.Also applies to: 52-52
175-179: LGTM: Directory creation and file writing implementation is solid.The approach properly creates the target directory recursively and writes the file to the correct location. The use of
path.join()ensures proper path handling across platforms.packages/core/src/external/sftp/command/sftp-action/sftp-action.ts (1)
26-26: LGTM: Improved type semantics for write action result.The change from
undefinedtonumberfor the write action result type provides more meaningful feedback by returning the length of written content. This aligns well with the coordinated changes in the SFTP action implementations.packages/core/src/external/sftp/command/sftp-action/sftp-action-direct.ts (1)
33-34: LGTM: Consistent implementation of write action result.The change to return
action.content.lengthinstead of the client write result provides a more predictable and useful return value. This implementation aligns perfectly with the updated type definition and ensures consistent behavior across different SFTP implementations.packages/core/src/external/term-server/index.ts (1)
96-98: LGTM: Added network retry resilience.Wrapping the term server HTTP request with
executeWithNetworkRetriesimproves reliability when interacting with external services. This follows best practices for handling transient network issues without changing the function's interface or response processing logic.packages/utils/src/surescripts/convert-customer-response.ts (1)
50-50: LGTM: Performance optimization by reusing handler instance.Moving the handler instantiation outside the loop is a good optimization that reduces unnecessary object creation. Since the
SurescriptsConvertPatientResponseHandlerDirectappears to be stateless, reusing the same instance across all transmissions is both safe and efficient.packages/core/src/external/surescripts/fhir/coverage.ts (2)
15-18: LGTM: Enhanced Coverage resource with patient context.The function signature enhancement to accept
SurescriptsContextand the inclusion ofbeneficiaryreference improve the FHIR Coverage resource construction. The TODO comment appropriately tracks the future enhancement for payor references under the same issue (ENG-377).Also applies to: 21-21, 28-30
1-68: Inconsistency with PR objectives.The PR objectives mention "uses buildDayjs and UTC for medication resources, adds 'outpatient' category" but the medication-related files (
medication-request.tsandmedication-dispense.ts) mentioned in the AI summaries are not included in this review. This creates a disconnect between the stated PR objectives and the files being reviewed.#!/bin/bash # Description: Verify if medication-related files exist and contain buildDayjs usage # Expected: Find medication-request.ts and medication-dispense.ts files with buildDayjs usage # Search for medication-related files in the surescripts FHIR directory fd "medication.*\.ts$" packages/core/src/external/surescripts/fhir/ # Search for buildDayjs usage in any surescripts files rg -A 3 -B 3 "buildDayjs" packages/core/src/external/surescripts/Likely an incorrect or invalid review comment.
packages/lambdas/src/surescripts/convert-patient-response.ts (2)
2-2: LGTM: Type consolidation improves maintainability.The consolidation from intersection types to a single
SurescriptsJobtype simplifies the type system and improves code maintainability.
8-13: LGTM: Handler logic remains intact.The handler function logic is preserved while benefiting from the simplified type signature.
packages/lambdas/src/surescripts/convert-batch-response.ts (2)
2-2: LGTM: Consistent type consolidation.The type consolidation matches the pattern used in the patient response handler, maintaining consistency across the codebase.
8-13: LGTM: Simplified handler maintains functionality.The handler function is appropriately simplified while preserving the core conversion logic.
packages/core/src/external/surescripts/fhir/shared.ts (3)
11-11: LGTM: Import cleanup removes unused types.The removal of unused
ResourceandResourceMaptype imports cleanses the import statement.
55-69: Verify the behavioral change in coding deduplication.Similar to the system identifier deduplication, this function now returns the last found existing resource instead of the first. Please confirm this behavioral change is intentional.
33-47: Verify the behavioral change in deduplication logic.The deduplication logic has changed from returning the first found existing resource to returning the last found existing resource. This is a significant behavioral change.
Previous behavior: Return immediately upon finding the first existing resource
New behavior: Continue iteration and return the last found existing resourcePlease confirm this change is intentional and aligns with the expected deduplication semantics.
#!/bin/bash # Description: Search for usages of deduplicateBySystemIdentifier to understand the expected behavior # Expected: Find code that relies on this function to understand if the behavior change is breaking rg -A 10 -B 5 "deduplicateBySystemIdentifier"packages/core/src/external/surescripts/fhir/medication.ts (5)
75-76: LGTM: Conditional display field prevents undefined values.The conditional inclusion of the display field using the ternary operator prevents undefined values from being included in the coding object.
84-85: LGTM: Consistent conditional display handling.Consistent with the NDC code handling above, this properly handles optional display fields.
99-111: LGTM: Improved return type aligns with FHIR standards.The change from returning a single
Codingto aCodeableConceptwith a coding array better aligns with FHIR standards and provides a more structured representation of medication form data.
146-150: LGTM: Refactored ingredient handling improves modularity.The extraction of ingredient CodeableConcept creation to a dedicated helper function improves code modularity and readability.
170-183: LGTM: Well-structured helper function.The new helper function properly encapsulates the creation of medication ingredient CodeableConcept with appropriate validation and conditional display field handling.
| @@ -72,7 +72,7 @@ function getMedicationNdcCode(detail: ResponseDetail): Coding | undefined { | |||
| return { | |||
| system: NDC_URL, | |||
| code: detail.ndcNumber, | |||
| display: detail.drugDescription ?? "", | |||
| ...(detail.drugDescription ? { display: detail.drugDescription } : undefined), | |||
metriport/metriport-internal#1040
Issues:
Dependencies
Description
This uses UTC time via
buildDayjsfor theMedicationDispenseandMedicationRequestresources (on thedateWrittenandwhenHandedOverdates). It also adds an"outpatient"category to the MedicationRequest since all Surescripts data is retrieve from PBMs (pharmacy benefits managers) who are collecting data from pharmacies.Testing
npm run surescripts -- convert-customer-response ...(see other PRs)Release Plan
Summary by CodeRabbit
New Features
Bug Fixes
Chores