-
Notifications
You must be signed in to change notification settings - Fork 324
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
PatientSummary
Problem List uses US Core profile
#4535
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -589,7 +594,17 @@ export function matchDiscriminant( | |||
if (!value || !sliceElement) { | |||
return false; | |||
} | |||
if (matchesSpecifiedValue(value, sliceElement)) { | |||
if (sliceElement.pattern) { |
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 is a subtle but potentially validation-breaking change for malformed slice definitions. Since matchesSpecifiedValue
returns true
by default, checks on elements containing neither .fixed
nor .pattern
now return false
whereas they used to return true
. The only time fixed and pattern shouldn't be defined is when we're dealing with slicing by ValueSet which is handled below and retains the permissiveness for now.
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.
@mattwiller - can you comment on this? Seems ok to me, but this is kinda out of my comfort zone.
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.
Yep, Matt and I discussed this and I think this change should be fine. In general, the possibility of malformed profile StructureDefinition
resources makes validation tricky unless we assume the underlying profile is valid. Eventually, we may want to add explicit validation checks on the profile before using it for validation of another resource: at least then we could abort and display the issue with the profile
45af05e
to
a20514f
Compare
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.
lgtm. I think we should wait for @mattwiller to clarify the matchesSpecifiedValue()
change
export interface SliceDefinition { | ||
export interface SliceDefinition extends InternalSchemaElement { | ||
// narrowed InternalSchemaElement fields | ||
slicing?: never; |
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.
Would this be more accurate?
export interface SliceDefinition extends Omit<InternalSchemaElement, 'slicing'> {
@@ -580,7 +580,12 @@ export function matchDiscriminant( | |||
return false; | |||
} | |||
|
|||
const sliceElement: InternalSchemaElement = (elements ?? slice.elements)[discriminator.path]; | |||
let sliceElement: InternalSchemaElement | undefined; | |||
if (discriminator.path === '$this') { |
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.
Clarifying question: My understanding is that discriminator.path
can be a full FHIRPath expression, so we're currently adding support for the special case of === '$this'
, but still have open future work here?
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.
Yes, that's correct; the Restricted Subset ("Simple"):
- All statements SHALL start with the name of the context element (e.g. on a Patient resource, Patient.contact.name.), or SHALL be simply "$this" to refer to the element that has focus
- Operators SHALL NOT be used
- Only the following functions may be used:
- .resolve()
- .extension("url")
- .ofType(type)
- All other functions SHALL NOT be used
@@ -589,7 +594,17 @@ export function matchDiscriminant( | |||
if (!value || !sliceElement) { | |||
return false; | |||
} | |||
if (matchesSpecifiedValue(value, sliceElement)) { | |||
if (sliceElement.pattern) { |
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.
@mattwiller - can you comment on this? Seems ok to me, but this is kinda out of my comfort zone.
cb9418a
to
99579fb
Compare
|
Fix age display in PatientSummary (#4484)Fixes #4471 - DetectedIssue.status valueset and search param (#4483) Qualify columns with table name in generated SQL (#4487) fix(migrations): only take lock if migrating (#4490) Document updating profiles (#4402) Run expand tests against old and new (#4503) Adding OpenCareHub support post (#4504) [Medplum Provider app] Various fixes and touchups (#4500) ci(agent): add workflow for building agent outside of a release (#4512) feat(agent): `Agent/$reload-config` operation (#4457) Remove broken links to Foo Provider (#4518) Dependency upgrades 2024-05-06 (#4515) PatientSummary and provider app tweaks (#4521) Fixes #4509 - Improve exact match search support for token parameters in `matchesTokenFilter` (#4516) Link to new Demo Applications (#4514) Clarify that autobatching only applies to `GET` requests (#4479) Added missing useEffect dependency in chat demo (#4527) fix: allows CORS for `keyvalue` API (#4476) Fixes #4508 - MeasureReport-subject search param backport (#4530) Fix CLI update-server version flag (#4534) Patient summary appointments and encounters links (#4524) Remove unused DB columns (#4532) fix(agent): unwrap response for `$reload-config` by id (#4542) `PatientSummary` Problem List uses US Core profile (#4535) fix(cli): always exit with exit code 1 after error occurs during command (#4536) Add failing test about validating nested extensions (#4548) Add error message when `cli` fails on login (#4507) Remove functions moved to core (#4547) fix: AttachmentDisplay use uncached url for download link (#4501) feat(agent): respect `Agent.status` and `Agent.channel.endpoint.status` being `off` (#4523) CMS 1500 and Superbill (#4543) Demo Bot: Agent Setup (#4555) feat(Subscription): add `author` as a `SearchParameter` (#4540) Dependency upgrades 2024-05-13 (#4544) Full linked Project ordering in CodeSystem lookup (#4522) Disable super admin refresh tokens (#4492) Minor fixes for the agent setup bot (#4560) docs(agent): document how logging works with `Bot` and `Agent` (#4563) Split rate limits into two buckets (#4568) Properly detect array elements (#4569) Apply filter to ValueSet with expansion.contains (#4570) More efficiently validate included concepts (#4573) Dependency upgrades 2024-05-20 (#4574) tweak(agent): add timezone in status `lastUpdated` time (#4564) fix(client/keyvalue): set keyvalue content-type text (#4575) Allow configuring server default rate limits (#4491) feat(cli): add `token` command to get access token (#4579) Updating device resources and videos (#4578) fix(subscriptions): don't retry ws subs if sub is deleted (#4577) Add support for 'pr' filter operation (#4584) Super admin endpoint for database stats (#4443)
Fix age display in PatientSummary (#4484)Fixes #4471 - DetectedIssue.status valueset and search param (#4483) Qualify columns with table name in generated SQL (#4487) fix(migrations): only take lock if migrating (#4490) Document updating profiles (#4402) Run expand tests against old and new (#4503) Adding OpenCareHub support post (#4504) [Medplum Provider app] Various fixes and touchups (#4500) ci(agent): add workflow for building agent outside of a release (#4512) feat(agent): `Agent/$reload-config` operation (#4457) Remove broken links to Foo Provider (#4518) Dependency upgrades 2024-05-06 (#4515) PatientSummary and provider app tweaks (#4521) Fixes #4509 - Improve exact match search support for token parameters in `matchesTokenFilter` (#4516) Link to new Demo Applications (#4514) Clarify that autobatching only applies to `GET` requests (#4479) Added missing useEffect dependency in chat demo (#4527) fix: allows CORS for `keyvalue` API (#4476) Fixes #4508 - MeasureReport-subject search param backport (#4530) Fix CLI update-server version flag (#4534) Patient summary appointments and encounters links (#4524) Remove unused DB columns (#4532) fix(agent): unwrap response for `$reload-config` by id (#4542) `PatientSummary` Problem List uses US Core profile (#4535) fix(cli): always exit with exit code 1 after error occurs during command (#4536) Add failing test about validating nested extensions (#4548) Add error message when `cli` fails on login (#4507) Remove functions moved to core (#4547) fix: AttachmentDisplay use uncached url for download link (#4501) feat(agent): respect `Agent.status` and `Agent.channel.endpoint.status` being `off` (#4523) CMS 1500 and Superbill (#4543) Demo Bot: Agent Setup (#4555) feat(Subscription): add `author` as a `SearchParameter` (#4540) Dependency upgrades 2024-05-13 (#4544) Full linked Project ordering in CodeSystem lookup (#4522) Disable super admin refresh tokens (#4492) Minor fixes for the agent setup bot (#4560) docs(agent): document how logging works with `Bot` and `Agent` (#4563) Split rate limits into two buckets (#4568) Properly detect array elements (#4569) Apply filter to ValueSet with expansion.contains (#4570) More efficiently validate included concepts (#4573) Dependency upgrades 2024-05-20 (#4574) tweak(agent): add timezone in status `lastUpdated` time (#4564) fix(client/keyvalue): set keyvalue content-type text (#4575) Allow configuring server default rate limits (#4491) feat(cli): add `token` command to get access token (#4579) Updating device resources and videos (#4578) fix(subscriptions): don't retry ws subs if sub is deleted (#4577) Add support for 'pr' filter operation (#4584) Super admin endpoint for database stats (#4443)
A notable underlying change to facilitate this:
In order to get discriminators with a path of
$this
working,SliceDefinition
now extendsInternalSchemaElement
. Previously they already had an awful lot of overlap, so this feels like a low risk change to make. The redundancy ofSliceDefinition.definition
andInternalSchemaElement.description
is unfortunate. We could get rid ofSliceDefinition.definition
and normalize ondescription
, but that would of course be a breaking change which would probably be best to leave for the next major release. If/when this PR gets merged, I'll make an issue to track that.