Skip to content
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

Removing status and version presets on exported artifacts #1143

Merged
merged 6 commits into from
Oct 3, 2022

Conversation

julianxcarter
Copy link
Contributor

@julianxcarter julianxcarter commented Sep 7, 2022

Addresses #1022, removing SUSHI's default values for the version (previously defaulted to the IG version), and status (previously defaulted to active) properties on all exported artifacts. The purpose of this change is to allow the IG Publisher to set these fields automatically instead, as it provides users the option of whether or not to apply these presets where SUSHI does not.

In a regression, the only differences present should be the absence of status and version fields on exported artifacts, unless there is a rule present in the FSH that explicitly sets these fields.

The previous default for version provided by SUSHI was the version used in the IG, the IG Publisher uses the same default.

The previous default for status provided by SUSHI was active, the default used by IG Publisher is the status used in the IG.

@cmoesel
Copy link
Member

cmoesel commented Sep 7, 2022

@julianxcarter -- do you know if the IG publisher will automatically default status to the same value as in the IG JSON (which corresponds to the status value in our sushi-config.yaml)?

I see that there is an IG parameter for propagate-status, but I can't tell if it's talking about the same status as we are or if it's talking about a different status.

@cmoesel cmoesel self-assigned this Sep 13, 2022
@cmoesel
Copy link
Member

cmoesel commented Sep 13, 2022

I haven't tested this yet, but the code all looks good! My only concern (in hindsight) is for people who use SUSHI without the IG Publisher. I don't know how many people do that, but I guess they would end up having to set the version and status themselves.

And now that I'm looking at it -- I'm seeing that status is 1..1. This means that profiles that SUSHI creates would be invalid if we don't include a status. I'm not sure that's a good thing. We may need to think about that.

I also posted this on the Zulip thread to get peoples thoughts before we merge it. See: https://chat.fhir.org/#narrow/stream/179252-IG-creation/topic/Profiles.20Versioning/near/298581914

@julianxcarter julianxcarter force-pushed the cimpl-903-publisher-presets branch from 390d64a to cff559c Compare September 14, 2022 13:30
@julianxcarter
Copy link
Contributor Author

And now that I'm looking at it -- I'm seeing that status is 1..1. This means that profiles that SUSHI creates would be invalid if we don't include a status. I'm not sure that's a good thing. We may need to think about that.

In light of discussion on Zulip, I've changed all of our exporters to pull the status property from the in-memory configuration object. I've also updated the code in our configuration importers to always populate the status property (defaulting to draft if one isn't present on FSHOnly IGs or imported IG resources, this wasn't necessary in the base case as status is required in the sushi-config file).

Prior to this PR, the previous default for version provided by SUSHI was the version used in the IG, the IG Publisher uses the same default.

Prior to this PR, the previous default for status provided by SUSHI was active, status is now pulled from the IG resource (which itself now defaults to draft when a user does not include one in FSHOnly mode).

Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @julianxcarter! I left a few comments that I think need to be addressed -- but let me know if there is a reason you did what you did.

I'd also like to run a full regression on this, but haven't yet had a chance to. So, I may have more comments after that.

src/export/CodeSystemExporter.ts Outdated Show resolved Hide resolved
src/export/StructureDefinitionExporter.ts Outdated Show resolved Hide resolved
src/export/ValueSetExporter.ts Outdated Show resolved Hide resolved
@cmoesel
Copy link
Member

cmoesel commented Sep 28, 2022

I've run the full regression on this and I see that quite a few IGs will have all of their resources changed from active to draft (mainly because they have their sushi-config.yaml status set to draft). I expected this might happen, but I want to discuss it w/ @markkramerus before we merge this. I think it's the right thing to do, but it is a bit disruptive.

Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, @julianxcarter! I've discussed the status issue w/ Mark and we are in agreement to keep it as you've implemented here -- but we need to document the heck out of it when we write the release notes! We want people to know about this change and how to address it (by changing the status of their IG).

@guhanthuran guhanthuran self-assigned this Sep 30, 2022
Copy link
Contributor

@guhanthuran guhanthuran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - ran tests and regression and it seems all green to me.

@julianxcarter julianxcarter merged commit 1dfe866 into next Oct 3, 2022
jafeltra pushed a commit that referenced this pull request Oct 7, 2022
* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package
mint-thompson added a commit that referenced this pull request Nov 30, 2022
* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Do not rename choice types on StructureDefinitions (#972)

* Do not rename choice types on StructureDefinitions

A specific choice type on a StructureDefinition should not have its id
or path renamed. Instead, put a slice name on the choice element.
Specific choice types on Instances are not changed.

* Remove extra id assignment

* Move shortcut id function to where it is used

The shortcut id is only ever needed by setImpliedPropertiesOnInstance,
so the function to get the shortcut id is moved to be in the same file.
The constants used by this function are still in the ElementDefinition
file, since I feel like that is where I would expect to find them if I
were looking for them.

* Move choice type slice name postfixes

* Always include type on diff for choice element slice

The IG publisher requires that the differential element representing a
slice of a choice element always contain a type. This is the case even
if its type is the same as the type of the choice element. Add a check
when calculating the differential so that the type is always included
for these elements.

Added TODO note to ElementDefinitionType.fromJSON regarding undefined
properties. There may be upcoming changes to requirements regarding
these types, which are currently included due to the implementation
difference between how ElementDefinitionType's constructor and fromJSON
methods handle undefined properties.

* Update antlr4 to 4.9.2 (#933)

* Update antlr4 to 4.9.2

The updated antlr4 library generates classes with a different export
style. Update the imports of those classes to use the default import
rather than a destructured import.

The directory structure of the antlr4 package adds two new directories.
Add package path mapping to the TypeScript compiler configuration and
the Jest configuration.

* Update antlr4 to 4.9.3

Update year in test fixtures, since they haven't been run since last
year.

* Handle antlr path resolution at runtime

Setting paths in the tsconfig is good for helping tsc find modules, but
doesn't help node out at runtime. Add the tsconfig-paths module so that
the modules will resolve. This works, but it adds a new library, so I'm
upset about it.

* Resolve antlr4 submodules relative to app

* Fix path registration in index.ts

When using SUSHI as a module, the entry point is index, not app. So,
both files need to have the antlr4 path registration applied.

* Replace package loading with FPL (#1026)

* Remove load functionality that is replace with fhir-package-load library. Update tests and fixtures.

* Silence fhir-package-load logger in tests

* Use updated exported FHIRDefinition class from fhir-package-load

* Pass in a log function to loadDependency

* Rename to fhir-package-loader

* Remove import of FPL logger for tests

* Remove cloneJsonValues and addDefinition imports from FPL and replace functions

* Add fishForMetadata and corresponding tests back

* Update enum to use string values

* Keep track of packages loaded through local variable. Remove property from FHIRDefs.

* Remove unused loadPackages array. Fix stray comma.

* Use mergeDependency function

* Add FPL package

* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Don't slice single-type choices (unless already sliced) (#1116)

If a user refers to a specific choice type (e.g., valueQuantity), but the choice (e.g., value[x]) only allows one type anyway, then don't create a type slicing and slice, because they're not necessary.

On the other hand, if the choice has already been sliced, then go ahead and create the slice for consistency.

* Handle null array elements when assigning complex values

* Add slice name enforcement to configuration

* Instance Exporter uses configured slice name enforcement

When resolving soft indexing, a boolean parameter can be passed to
indicate whether "strict soft indexing" should be used. The value of
this parameter is taken from the SUSHI configuration. When this value is
false (the default), the behavior is consistent with previous versions
of SUSHI. When this value is true, soft indexing is resolved by
enforcing the practice of referring to slices by name.

* Reslices in soft indexing increment parent slices

When a path uses soft indexing to increment the value of an element with
reslices, the path part with the soft index will contain multiple
slices. The tracked index for each less-sliced version of the element
should be incremented in order to maintain the correct index when
less-sliced versions of the element are used.

* Don't warn on numeric index usage when enforcing slice name usage

When enforcing slice name usage, referring to a numeric index
necessarily refers to an element that is not one of the named slices.
Therefore, it is not referring to a named slice via numeric index, and
there is no need to emit a warning.

* Allow name/description in Instances (#1104)

* Init commit

* Tests added

* Undeleted instancemeta, moved logic to be under the if statement for #definition usage, added tests

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Removed name and status for patient

* Update test case so id/title/description are consistent

Co-authored-by: gthuran <Suma1-dumont>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>

* 3.0.0 Beta 1 Candidate: Update Node and Dev Tools (#1127)

* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1

* Allow reslices to fulfill base slice cardinality

When a slice with a minimum cardinality greater than 0 is resliced,
reslices should count towards fulfilling that cardinality.

Add logic to ensure that implied properties on a slice are set, even
when that slice's cardinality is fulfilled by reslices.

Fix typo in soft index conversion functions.

* Order IG resources based on config (#1118)

* Order IG resources based on config

If all generated resources are present in the config resources section,
use the order from that section when adding resources to the IG.
Otherwise, if all generated resources are present in the config groups
section, use the order from that section when adding resources to the
IG. If neither config section contains all resources, add the resources
sorted by title or id.

* Fix Minifier Dependency and Location Type (#1113)

Changed dependency html-minifier to html-minifier-terser

* Add test for using title to sort resource

* Sort resources after all resources are added

Resources in the IG are sorted after adding generated, configured, and
predefined resources. A temporary sort key attribute is added to the
resources when adding them. This attribute is used for the default
sorting method.

* Refactor IG resource sorting

Test if a sort should be used and use it in the same function. This
avoids having to iterate and search over the set of resources additional
times.

* Sort resources by name

The sort key that was used to sort IG resources is removed. Resources
are now sorted by name. The name attribute should always exist, but if
it doesn't, use the reference key instead.

* Default resource order is case-insensitive

Co-authored-by: Alex Walley <[email protected]>

* messy commit to share with julia

things are not working quite yet. but i promise. we are very close. we
are so very close.

* Update some tests to hopefully work better

* Change element ordering in test

* Correctly create helpyblock for paths with slicenames

* Failing test case for siblings with multiple slices causing extra entries to be added

* Add tests from reslicing to ensure duplicate entries aren't added

* Slice minimums include numeric indices

When calculating the minimum number of elements for sliced elements,
include numeric indices for parts of the path before the last part. This
helps avoid creating unnecessary sibling elements.

A side effect of this is that implicit extension slices created on
instances will no longer automatically fill in their URL on elements in
the list prior to the ones actually referred to in rules. However, both
the previous and new behaviors are equally erroneous, and indicate that
the author should revise their FSH to create valid extension elements.

Some cleanup still needed.

* Removing status and version presets on exported artifacts (#1143)

* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package

* Traverse SD elements to find implied properties

A new function for setting implied properties is added. Rather than
iterating over the set of rule paths and using those paths to find
elements, this function iterates over the elements. The rule paths are
still used for certain checks.

This implementation is not yet complete, and is in need of significant
cleanup. The new function is only used when exporting instances, so the
previous function will remain for now.

* Remove old setImpliedPropertiesOnInstance and replace with new version

* Farewell to helpy. Refactor create and determine slices functions

* Remove old sort function and rootRequired property

* Clean up setImpliedPropertiesOnInstance

* Clean up findConnectedElements

* Rename configuration property to manualSliceOrdering and move into instanceOptions

* Minor updates based on PR review

- Add comment above confusing line of code for primitive properties
- Add comments to distinguish the strict and non-strict convertSoftIndices functions
- Only check for slice indices on extension elements

* Access the correct array element with pop

* Remove code that is no longer reached or needed

* Create a variable for manualSliceOrdering config property

* Eliminate old copy and merge approach for assigning values on Instances

* Update getSlices to properly handle getting reslices from an already sliced element

* Rename strict to manualSliceOrdering for clarity

* Separate out slice tree related functions and add comments explaining why we need slice trees

* Add comment explaining sliceIndices and ruleIndices when we createUsefulSlices

* Add README file explaining the approach taken in setImpliedPropertiesOnInstance

* Clean up test

* Added a few formatting updates to new README file

* Revert "Eliminate old copy and merge approach for assigning values on Instances"

This reverts commit f8f39d8.

Co-authored-by: Julia Afeltra <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Guhan B. Thuran <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Alex Walley <[email protected]>
Co-authored-by: Julia Afeltra <[email protected]>
Co-authored-by: Julian Carter <[email protected]>
cmoesel pushed a commit that referenced this pull request Feb 10, 2023
* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package
cmoesel added a commit that referenced this pull request Feb 10, 2023
* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Do not rename choice types on StructureDefinitions (#972)

* Do not rename choice types on StructureDefinitions

A specific choice type on a StructureDefinition should not have its id
or path renamed. Instead, put a slice name on the choice element.
Specific choice types on Instances are not changed.

* Remove extra id assignment

* Move shortcut id function to where it is used

The shortcut id is only ever needed by setImpliedPropertiesOnInstance,
so the function to get the shortcut id is moved to be in the same file.
The constants used by this function are still in the ElementDefinition
file, since I feel like that is where I would expect to find them if I
were looking for them.

* Move choice type slice name postfixes

* Always include type on diff for choice element slice

The IG publisher requires that the differential element representing a
slice of a choice element always contain a type. This is the case even
if its type is the same as the type of the choice element. Add a check
when calculating the differential so that the type is always included
for these elements.

Added TODO note to ElementDefinitionType.fromJSON regarding undefined
properties. There may be upcoming changes to requirements regarding
these types, which are currently included due to the implementation
difference between how ElementDefinitionType's constructor and fromJSON
methods handle undefined properties.

* Update antlr4 to 4.9.2 (#933)

* Update antlr4 to 4.9.2

The updated antlr4 library generates classes with a different export
style. Update the imports of those classes to use the default import
rather than a destructured import.

The directory structure of the antlr4 package adds two new directories.
Add package path mapping to the TypeScript compiler configuration and
the Jest configuration.

* Update antlr4 to 4.9.3

Update year in test fixtures, since they haven't been run since last
year.

* Handle antlr path resolution at runtime

Setting paths in the tsconfig is good for helping tsc find modules, but
doesn't help node out at runtime. Add the tsconfig-paths module so that
the modules will resolve. This works, but it adds a new library, so I'm
upset about it.

* Resolve antlr4 submodules relative to app

* Fix path registration in index.ts

When using SUSHI as a module, the entry point is index, not app. So,
both files need to have the antlr4 path registration applied.

* Replace package loading with FPL (#1026)

* Remove load functionality that is replace with fhir-package-load library. Update tests and fixtures.

* Silence fhir-package-load logger in tests

* Use updated exported FHIRDefinition class from fhir-package-load

* Pass in a log function to loadDependency

* Rename to fhir-package-loader

* Remove import of FPL logger for tests

* Remove cloneJsonValues and addDefinition imports from FPL and replace functions

* Add fishForMetadata and corresponding tests back

* Update enum to use string values

* Keep track of packages loaded through local variable. Remove property from FHIRDefs.

* Remove unused loadPackages array. Fix stray comma.

* Use mergeDependency function

* Add FPL package

* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Don't slice single-type choices (unless already sliced) (#1116)

If a user refers to a specific choice type (e.g., valueQuantity), but the choice (e.g., value[x]) only allows one type anyway, then don't create a type slicing and slice, because they're not necessary.

On the other hand, if the choice has already been sliced, then go ahead and create the slice for consistency.

* Handle null array elements when assigning complex values

* Add slice name enforcement to configuration

* Instance Exporter uses configured slice name enforcement

When resolving soft indexing, a boolean parameter can be passed to
indicate whether "strict soft indexing" should be used. The value of
this parameter is taken from the SUSHI configuration. When this value is
false (the default), the behavior is consistent with previous versions
of SUSHI. When this value is true, soft indexing is resolved by
enforcing the practice of referring to slices by name.

* Reslices in soft indexing increment parent slices

When a path uses soft indexing to increment the value of an element with
reslices, the path part with the soft index will contain multiple
slices. The tracked index for each less-sliced version of the element
should be incremented in order to maintain the correct index when
less-sliced versions of the element are used.

* Don't warn on numeric index usage when enforcing slice name usage

When enforcing slice name usage, referring to a numeric index
necessarily refers to an element that is not one of the named slices.
Therefore, it is not referring to a named slice via numeric index, and
there is no need to emit a warning.

* Allow name/description in Instances (#1104)

* Init commit

* Tests added

* Undeleted instancemeta, moved logic to be under the if statement for #definition usage, added tests

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Removed name and status for patient

* Update test case so id/title/description are consistent

Co-authored-by: gthuran <Suma1-dumont>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>

* 3.0.0 Beta 1 Candidate: Update Node and Dev Tools (#1127)

* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1

* Allow reslices to fulfill base slice cardinality

When a slice with a minimum cardinality greater than 0 is resliced,
reslices should count towards fulfilling that cardinality.

Add logic to ensure that implied properties on a slice are set, even
when that slice's cardinality is fulfilled by reslices.

Fix typo in soft index conversion functions.

* Order IG resources based on config (#1118)

* Order IG resources based on config

If all generated resources are present in the config resources section,
use the order from that section when adding resources to the IG.
Otherwise, if all generated resources are present in the config groups
section, use the order from that section when adding resources to the
IG. If neither config section contains all resources, add the resources
sorted by title or id.

* Fix Minifier Dependency and Location Type (#1113)

Changed dependency html-minifier to html-minifier-terser

* Add test for using title to sort resource

* Sort resources after all resources are added

Resources in the IG are sorted after adding generated, configured, and
predefined resources. A temporary sort key attribute is added to the
resources when adding them. This attribute is used for the default
sorting method.

* Refactor IG resource sorting

Test if a sort should be used and use it in the same function. This
avoids having to iterate and search over the set of resources additional
times.

* Sort resources by name

The sort key that was used to sort IG resources is removed. Resources
are now sorted by name. The name attribute should always exist, but if
it doesn't, use the reference key instead.

* Default resource order is case-insensitive

Co-authored-by: Alex Walley <[email protected]>

* messy commit to share with julia

things are not working quite yet. but i promise. we are very close. we
are so very close.

* Update some tests to hopefully work better

* Change element ordering in test

* Correctly create helpyblock for paths with slicenames

* Failing test case for siblings with multiple slices causing extra entries to be added

* Add tests from reslicing to ensure duplicate entries aren't added

* Slice minimums include numeric indices

When calculating the minimum number of elements for sliced elements,
include numeric indices for parts of the path before the last part. This
helps avoid creating unnecessary sibling elements.

A side effect of this is that implicit extension slices created on
instances will no longer automatically fill in their URL on elements in
the list prior to the ones actually referred to in rules. However, both
the previous and new behaviors are equally erroneous, and indicate that
the author should revise their FSH to create valid extension elements.

Some cleanup still needed.

* Removing status and version presets on exported artifacts (#1143)

* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package

* Traverse SD elements to find implied properties

A new function for setting implied properties is added. Rather than
iterating over the set of rule paths and using those paths to find
elements, this function iterates over the elements. The rule paths are
still used for certain checks.

This implementation is not yet complete, and is in need of significant
cleanup. The new function is only used when exporting instances, so the
previous function will remain for now.

* Remove old setImpliedPropertiesOnInstance and replace with new version

* Farewell to helpy. Refactor create and determine slices functions

* Remove old sort function and rootRequired property

* Clean up setImpliedPropertiesOnInstance

* Clean up findConnectedElements

* Rename configuration property to manualSliceOrdering and move into instanceOptions

* Minor updates based on PR review

- Add comment above confusing line of code for primitive properties
- Add comments to distinguish the strict and non-strict convertSoftIndices functions
- Only check for slice indices on extension elements

* Access the correct array element with pop

* Remove code that is no longer reached or needed

* Create a variable for manualSliceOrdering config property

* Eliminate old copy and merge approach for assigning values on Instances

* Update getSlices to properly handle getting reslices from an already sliced element

* Rename strict to manualSliceOrdering for clarity

* Separate out slice tree related functions and add comments explaining why we need slice trees

* Add comment explaining sliceIndices and ruleIndices when we createUsefulSlices

* Add README file explaining the approach taken in setImpliedPropertiesOnInstance

* Clean up test

* Added a few formatting updates to new README file

* Revert "Eliminate old copy and merge approach for assigning values on Instances"

This reverts commit f8f39d8.

Co-authored-by: Julia Afeltra <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Guhan B. Thuran <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Alex Walley <[email protected]>
Co-authored-by: Julia Afeltra <[email protected]>
Co-authored-by: Julian Carter <[email protected]>
cmoesel pushed a commit that referenced this pull request Feb 21, 2023
* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package
cmoesel added a commit that referenced this pull request Feb 21, 2023
* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Do not rename choice types on StructureDefinitions (#972)

* Do not rename choice types on StructureDefinitions

A specific choice type on a StructureDefinition should not have its id
or path renamed. Instead, put a slice name on the choice element.
Specific choice types on Instances are not changed.

* Remove extra id assignment

* Move shortcut id function to where it is used

The shortcut id is only ever needed by setImpliedPropertiesOnInstance,
so the function to get the shortcut id is moved to be in the same file.
The constants used by this function are still in the ElementDefinition
file, since I feel like that is where I would expect to find them if I
were looking for them.

* Move choice type slice name postfixes

* Always include type on diff for choice element slice

The IG publisher requires that the differential element representing a
slice of a choice element always contain a type. This is the case even
if its type is the same as the type of the choice element. Add a check
when calculating the differential so that the type is always included
for these elements.

Added TODO note to ElementDefinitionType.fromJSON regarding undefined
properties. There may be upcoming changes to requirements regarding
these types, which are currently included due to the implementation
difference between how ElementDefinitionType's constructor and fromJSON
methods handle undefined properties.

* Update antlr4 to 4.9.2 (#933)

* Update antlr4 to 4.9.2

The updated antlr4 library generates classes with a different export
style. Update the imports of those classes to use the default import
rather than a destructured import.

The directory structure of the antlr4 package adds two new directories.
Add package path mapping to the TypeScript compiler configuration and
the Jest configuration.

* Update antlr4 to 4.9.3

Update year in test fixtures, since they haven't been run since last
year.

* Handle antlr path resolution at runtime

Setting paths in the tsconfig is good for helping tsc find modules, but
doesn't help node out at runtime. Add the tsconfig-paths module so that
the modules will resolve. This works, but it adds a new library, so I'm
upset about it.

* Resolve antlr4 submodules relative to app

* Fix path registration in index.ts

When using SUSHI as a module, the entry point is index, not app. So,
both files need to have the antlr4 path registration applied.

* Replace package loading with FPL (#1026)

* Remove load functionality that is replace with fhir-package-load library. Update tests and fixtures.

* Silence fhir-package-load logger in tests

* Use updated exported FHIRDefinition class from fhir-package-load

* Pass in a log function to loadDependency

* Rename to fhir-package-loader

* Remove import of FPL logger for tests

* Remove cloneJsonValues and addDefinition imports from FPL and replace functions

* Add fishForMetadata and corresponding tests back

* Update enum to use string values

* Keep track of packages loaded through local variable. Remove property from FHIRDefs.

* Remove unused loadPackages array. Fix stray comma.

* Use mergeDependency function

* Add FPL package

* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Don't slice single-type choices (unless already sliced) (#1116)

If a user refers to a specific choice type (e.g., valueQuantity), but the choice (e.g., value[x]) only allows one type anyway, then don't create a type slicing and slice, because they're not necessary.

On the other hand, if the choice has already been sliced, then go ahead and create the slice for consistency.

* Handle null array elements when assigning complex values

* Add slice name enforcement to configuration

* Instance Exporter uses configured slice name enforcement

When resolving soft indexing, a boolean parameter can be passed to
indicate whether "strict soft indexing" should be used. The value of
this parameter is taken from the SUSHI configuration. When this value is
false (the default), the behavior is consistent with previous versions
of SUSHI. When this value is true, soft indexing is resolved by
enforcing the practice of referring to slices by name.

* Reslices in soft indexing increment parent slices

When a path uses soft indexing to increment the value of an element with
reslices, the path part with the soft index will contain multiple
slices. The tracked index for each less-sliced version of the element
should be incremented in order to maintain the correct index when
less-sliced versions of the element are used.

* Don't warn on numeric index usage when enforcing slice name usage

When enforcing slice name usage, referring to a numeric index
necessarily refers to an element that is not one of the named slices.
Therefore, it is not referring to a named slice via numeric index, and
there is no need to emit a warning.

* Allow name/description in Instances (#1104)

* Init commit

* Tests added

* Undeleted instancemeta, moved logic to be under the if statement for #definition usage, added tests

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Removed name and status for patient

* Update test case so id/title/description are consistent

Co-authored-by: gthuran <Suma1-dumont>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>

* 3.0.0 Beta 1 Candidate: Update Node and Dev Tools (#1127)

* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1

* Allow reslices to fulfill base slice cardinality

When a slice with a minimum cardinality greater than 0 is resliced,
reslices should count towards fulfilling that cardinality.

Add logic to ensure that implied properties on a slice are set, even
when that slice's cardinality is fulfilled by reslices.

Fix typo in soft index conversion functions.

* Order IG resources based on config (#1118)

* Order IG resources based on config

If all generated resources are present in the config resources section,
use the order from that section when adding resources to the IG.
Otherwise, if all generated resources are present in the config groups
section, use the order from that section when adding resources to the
IG. If neither config section contains all resources, add the resources
sorted by title or id.

* Fix Minifier Dependency and Location Type (#1113)

Changed dependency html-minifier to html-minifier-terser

* Add test for using title to sort resource

* Sort resources after all resources are added

Resources in the IG are sorted after adding generated, configured, and
predefined resources. A temporary sort key attribute is added to the
resources when adding them. This attribute is used for the default
sorting method.

* Refactor IG resource sorting

Test if a sort should be used and use it in the same function. This
avoids having to iterate and search over the set of resources additional
times.

* Sort resources by name

The sort key that was used to sort IG resources is removed. Resources
are now sorted by name. The name attribute should always exist, but if
it doesn't, use the reference key instead.

* Default resource order is case-insensitive

Co-authored-by: Alex Walley <[email protected]>

* messy commit to share with julia

things are not working quite yet. but i promise. we are very close. we
are so very close.

* Update some tests to hopefully work better

* Change element ordering in test

* Correctly create helpyblock for paths with slicenames

* Failing test case for siblings with multiple slices causing extra entries to be added

* Add tests from reslicing to ensure duplicate entries aren't added

* Slice minimums include numeric indices

When calculating the minimum number of elements for sliced elements,
include numeric indices for parts of the path before the last part. This
helps avoid creating unnecessary sibling elements.

A side effect of this is that implicit extension slices created on
instances will no longer automatically fill in their URL on elements in
the list prior to the ones actually referred to in rules. However, both
the previous and new behaviors are equally erroneous, and indicate that
the author should revise their FSH to create valid extension elements.

Some cleanup still needed.

* Removing status and version presets on exported artifacts (#1143)

* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package

* Traverse SD elements to find implied properties

A new function for setting implied properties is added. Rather than
iterating over the set of rule paths and using those paths to find
elements, this function iterates over the elements. The rule paths are
still used for certain checks.

This implementation is not yet complete, and is in need of significant
cleanup. The new function is only used when exporting instances, so the
previous function will remain for now.

* Remove old setImpliedPropertiesOnInstance and replace with new version

* Farewell to helpy. Refactor create and determine slices functions

* Remove old sort function and rootRequired property

* Clean up setImpliedPropertiesOnInstance

* Clean up findConnectedElements

* Rename configuration property to manualSliceOrdering and move into instanceOptions

* Minor updates based on PR review

- Add comment above confusing line of code for primitive properties
- Add comments to distinguish the strict and non-strict convertSoftIndices functions
- Only check for slice indices on extension elements

* Access the correct array element with pop

* Remove code that is no longer reached or needed

* Create a variable for manualSliceOrdering config property

* Eliminate old copy and merge approach for assigning values on Instances

* Update getSlices to properly handle getting reslices from an already sliced element

* Rename strict to manualSliceOrdering for clarity

* Separate out slice tree related functions and add comments explaining why we need slice trees

* Add comment explaining sliceIndices and ruleIndices when we createUsefulSlices

* Add README file explaining the approach taken in setImpliedPropertiesOnInstance

* Clean up test

* Added a few formatting updates to new README file

* Revert "Eliminate old copy and merge approach for assigning values on Instances"

This reverts commit f8f39d8.

Co-authored-by: Julia Afeltra <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Guhan B. Thuran <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Alex Walley <[email protected]>
Co-authored-by: Julia Afeltra <[email protected]>
Co-authored-by: Julian Carter <[email protected]>
cmoesel pushed a commit that referenced this pull request Feb 24, 2023
* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package
cmoesel added a commit that referenced this pull request Feb 24, 2023
* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Do not rename choice types on StructureDefinitions (#972)

* Do not rename choice types on StructureDefinitions

A specific choice type on a StructureDefinition should not have its id
or path renamed. Instead, put a slice name on the choice element.
Specific choice types on Instances are not changed.

* Remove extra id assignment

* Move shortcut id function to where it is used

The shortcut id is only ever needed by setImpliedPropertiesOnInstance,
so the function to get the shortcut id is moved to be in the same file.
The constants used by this function are still in the ElementDefinition
file, since I feel like that is where I would expect to find them if I
were looking for them.

* Move choice type slice name postfixes

* Always include type on diff for choice element slice

The IG publisher requires that the differential element representing a
slice of a choice element always contain a type. This is the case even
if its type is the same as the type of the choice element. Add a check
when calculating the differential so that the type is always included
for these elements.

Added TODO note to ElementDefinitionType.fromJSON regarding undefined
properties. There may be upcoming changes to requirements regarding
these types, which are currently included due to the implementation
difference between how ElementDefinitionType's constructor and fromJSON
methods handle undefined properties.

* Update antlr4 to 4.9.2 (#933)

* Update antlr4 to 4.9.2

The updated antlr4 library generates classes with a different export
style. Update the imports of those classes to use the default import
rather than a destructured import.

The directory structure of the antlr4 package adds two new directories.
Add package path mapping to the TypeScript compiler configuration and
the Jest configuration.

* Update antlr4 to 4.9.3

Update year in test fixtures, since they haven't been run since last
year.

* Handle antlr path resolution at runtime

Setting paths in the tsconfig is good for helping tsc find modules, but
doesn't help node out at runtime. Add the tsconfig-paths module so that
the modules will resolve. This works, but it adds a new library, so I'm
upset about it.

* Resolve antlr4 submodules relative to app

* Fix path registration in index.ts

When using SUSHI as a module, the entry point is index, not app. So,
both files need to have the antlr4 path registration applied.

* Replace package loading with FPL (#1026)

* Remove load functionality that is replace with fhir-package-load library. Update tests and fixtures.

* Silence fhir-package-load logger in tests

* Use updated exported FHIRDefinition class from fhir-package-load

* Pass in a log function to loadDependency

* Rename to fhir-package-loader

* Remove import of FPL logger for tests

* Remove cloneJsonValues and addDefinition imports from FPL and replace functions

* Add fishForMetadata and corresponding tests back

* Update enum to use string values

* Keep track of packages loaded through local variable. Remove property from FHIRDefs.

* Remove unused loadPackages array. Fix stray comma.

* Use mergeDependency function

* Add FPL package

* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Don't slice single-type choices (unless already sliced) (#1116)

If a user refers to a specific choice type (e.g., valueQuantity), but the choice (e.g., value[x]) only allows one type anyway, then don't create a type slicing and slice, because they're not necessary.

On the other hand, if the choice has already been sliced, then go ahead and create the slice for consistency.

* Handle null array elements when assigning complex values

* Add slice name enforcement to configuration

* Instance Exporter uses configured slice name enforcement

When resolving soft indexing, a boolean parameter can be passed to
indicate whether "strict soft indexing" should be used. The value of
this parameter is taken from the SUSHI configuration. When this value is
false (the default), the behavior is consistent with previous versions
of SUSHI. When this value is true, soft indexing is resolved by
enforcing the practice of referring to slices by name.

* Reslices in soft indexing increment parent slices

When a path uses soft indexing to increment the value of an element with
reslices, the path part with the soft index will contain multiple
slices. The tracked index for each less-sliced version of the element
should be incremented in order to maintain the correct index when
less-sliced versions of the element are used.

* Don't warn on numeric index usage when enforcing slice name usage

When enforcing slice name usage, referring to a numeric index
necessarily refers to an element that is not one of the named slices.
Therefore, it is not referring to a named slice via numeric index, and
there is no need to emit a warning.

* Allow name/description in Instances (#1104)

* Init commit

* Tests added

* Undeleted instancemeta, moved logic to be under the if statement for #definition usage, added tests

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <[email protected]>

* Removed name and status for patient

* Update test case so id/title/description are consistent

Co-authored-by: gthuran <Suma1-dumont>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>

* 3.0.0 Beta 1 Candidate: Update Node and Dev Tools (#1127)

* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1

* Allow reslices to fulfill base slice cardinality

When a slice with a minimum cardinality greater than 0 is resliced,
reslices should count towards fulfilling that cardinality.

Add logic to ensure that implied properties on a slice are set, even
when that slice's cardinality is fulfilled by reslices.

Fix typo in soft index conversion functions.

* Order IG resources based on config (#1118)

* Order IG resources based on config

If all generated resources are present in the config resources section,
use the order from that section when adding resources to the IG.
Otherwise, if all generated resources are present in the config groups
section, use the order from that section when adding resources to the
IG. If neither config section contains all resources, add the resources
sorted by title or id.

* Fix Minifier Dependency and Location Type (#1113)

Changed dependency html-minifier to html-minifier-terser

* Add test for using title to sort resource

* Sort resources after all resources are added

Resources in the IG are sorted after adding generated, configured, and
predefined resources. A temporary sort key attribute is added to the
resources when adding them. This attribute is used for the default
sorting method.

* Refactor IG resource sorting

Test if a sort should be used and use it in the same function. This
avoids having to iterate and search over the set of resources additional
times.

* Sort resources by name

The sort key that was used to sort IG resources is removed. Resources
are now sorted by name. The name attribute should always exist, but if
it doesn't, use the reference key instead.

* Default resource order is case-insensitive

Co-authored-by: Alex Walley <[email protected]>

* messy commit to share with julia

things are not working quite yet. but i promise. we are very close. we
are so very close.

* Update some tests to hopefully work better

* Change element ordering in test

* Correctly create helpyblock for paths with slicenames

* Failing test case for siblings with multiple slices causing extra entries to be added

* Add tests from reslicing to ensure duplicate entries aren't added

* Slice minimums include numeric indices

When calculating the minimum number of elements for sliced elements,
include numeric indices for parts of the path before the last part. This
helps avoid creating unnecessary sibling elements.

A side effect of this is that implicit extension slices created on
instances will no longer automatically fill in their URL on elements in
the list prior to the ones actually referred to in rules. However, both
the previous and new behaviors are equally erroneous, and indicate that
the author should revise their FSH to create valid extension elements.

Some cleanup still needed.

* Removing status and version presets on exported artifacts (#1143)

* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package

* Traverse SD elements to find implied properties

A new function for setting implied properties is added. Rather than
iterating over the set of rule paths and using those paths to find
elements, this function iterates over the elements. The rule paths are
still used for certain checks.

This implementation is not yet complete, and is in need of significant
cleanup. The new function is only used when exporting instances, so the
previous function will remain for now.

* Remove old setImpliedPropertiesOnInstance and replace with new version

* Farewell to helpy. Refactor create and determine slices functions

* Remove old sort function and rootRequired property

* Clean up setImpliedPropertiesOnInstance

* Clean up findConnectedElements

* Rename configuration property to manualSliceOrdering and move into instanceOptions

* Minor updates based on PR review

- Add comment above confusing line of code for primitive properties
- Add comments to distinguish the strict and non-strict convertSoftIndices functions
- Only check for slice indices on extension elements

* Access the correct array element with pop

* Remove code that is no longer reached or needed

* Create a variable for manualSliceOrdering config property

* Eliminate old copy and merge approach for assigning values on Instances

* Update getSlices to properly handle getting reslices from an already sliced element

* Rename strict to manualSliceOrdering for clarity

* Separate out slice tree related functions and add comments explaining why we need slice trees

* Add comment explaining sliceIndices and ruleIndices when we createUsefulSlices

* Add README file explaining the approach taken in setImpliedPropertiesOnInstance

* Clean up test

* Added a few formatting updates to new README file

* Revert "Eliminate old copy and merge approach for assigning values on Instances"

This reverts commit f8f39d8.

Co-authored-by: Julia Afeltra <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Guhan B. Thuran <[email protected]>
Co-authored-by: Chris Moesel <[email protected]>
Co-authored-by: Alex Walley <[email protected]>
Co-authored-by: Julia Afeltra <[email protected]>
Co-authored-by: Julian Carter <[email protected]>
@jafeltra jafeltra deleted the cimpl-903-publisher-presets branch April 12, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants