-
Notifications
You must be signed in to change notification settings - Fork 308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Charting Demo #4494
base: main
Are you sure you want to change the base?
Update Charting Demo #4494
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
Quality Gate passedIssues Measures |
examples/medplum-chart-demo/src/bots/core/general-encounter-note.ts
Outdated
Show resolved
Hide resolved
examples/medplum-chart-demo/src/bots/core/gynecology-encounter-note.ts
Outdated
Show resolved
Hide resolved
examples/medplum-chart-demo/src/bots/core/obstetric-encounter-note.ts
Outdated
Show resolved
Hide resolved
examples/medplum-chart-demo/src/bots/core/test-data/general-encounter-test-data.ts
Outdated
Show resolved
Hide resolved
examples/medplum-chart-demo/src/components/ClinicalImpressionDisplay.tsx
Outdated
Show resolved
Hide resolved
98d0730
to
652e1c2
Compare
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring: Next stepsTake a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with |
examples/medplum-chart-demo/data/example/example-patient-data.json
Outdated
Show resolved
Hide resolved
c262547
to
e2f816a
Compare
@SocketSecurity ignore npm/esbuild@0.21.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this is not a tsx file, the name should be lower case.
aslo, maybe something like measurement-constants
, to more clearly indicate that these are static values
const secondBackgroundColor = 'rgba(255, 119, 0, 0.7)'; | ||
const secondBorderColor = 'rgba(255, 119, 0, 1)'; | ||
|
||
export const measurementsMeta: Record<string, ObservationType> = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
measurementStyles
?
useEffect(() => { | ||
if (observations) { | ||
const labels: string[] = []; | ||
const datasets: ChartDataset<'line', number[]>[] = chartDatasets.map((item) => ({ ...item, data: [] })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add some comments to this file to explain how things are working here.
} | ||
checkForValidResponse(); | ||
|
||
function getNoteType(): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments
displayValues: ObstetricAnswers; | ||
} | ||
|
||
function ObstetricNoteDisplay({ displayValues }: ObstetricNoteDisplay): JSX.Element { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Break each of these into their own component file
* @param user - The user creating the Condition resource | ||
* @returns An array of bundle entries containing the Condition resource that can be created in a batch request. | ||
*/ | ||
export function createConditionEntries( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now this function does two jobs:
- takes a partial
Condition
and adds default fields to create a fullCondition
resoruce - Creates the bundle entry, with the appropriate search strings
Instead, for each of the resources Observation
, Condition
, ClinicalImpression
, we should have functions:
create*
that takes a partial entry and creates a full resource (or potentially 2 in the case of conditions)
Separately, we should just have 1 function that then takes all the output resources, and packages them into a transaction bundle (with the appropriate search string).
That way, we won't be repeating the bundle creation logic for each resource type
|
||
// Return the clinical impression in a bundle entry | ||
return { | ||
fullUrl: generateId(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure that generateId()
does the right thing here. Might be better to use `urn:uuid:${randomUUID()}`
}; | ||
} | ||
|
||
function getWeightInKilograms(weight: Quantity): number { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you break out all the BMI related functions into a new file called observation-utils
Conceptually, these are different from the functions above because they deal with computing single Observation values, rather than full bundles
key === 'selfReportedHistory' ? getSelfReportedCode(observationData.selfReportedHistory as string) : codes[key], | ||
}; | ||
|
||
switch (key) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can simplify these switch statements as follows:
const observationTypes = {
'height': 'valueQuantity',
'moodSwings': 'valueBoolean',
/...
}
if(key !== bloodPressure) {
resource[observationTypes[key]] = observationData[key]
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can then just handle the case of the bloodPressure
separately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also means you can combine all the createPartial***Observations
functions into a single utility function.
createObservations(
observationData,
codes,
observationTypes) {
// handle blood pressure
// else
// use `observationTypes` to fill `value**`
// Fill in the default fields
// return a full Observation
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should really reduce the amount of specialized code per-note type
}; | ||
|
||
// Add the appropriate code based on the answer provided for self-reported history | ||
switch (reportedHistory) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above, this switch statement could be converted to a map
const labels: string[] = []; | ||
const datasets: ChartDataset<'line', number[]>[] = chartDatasets.map((item) => ({ ...item, data: [] })); | ||
|
||
for (const observation of observations) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might make sense to break this into a helper function that just converts a list of observations into a list of numbers
<QuantityDisplay value={displayValues.totalWeightGain} /> | ||
</Group> | ||
)} | ||
{/* <Group> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove dead code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try to avoid the double extenson (.utils.ts
) where possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each of the note-specific functions should live in the files related to that component. Things in a utils
file should truly be general purpose
This will update the current charting demo to be based around the
Encounter
resource. Specifically, it will demo how to create encounter notes.