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

Refactor au-core-medreq-03 (FHIR-46498, FHIR-48362) #260

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

JayMurdoch
Copy link
Collaborator

@JayMurdoch JayMurdoch commented Sep 24, 2024

FHIR-46498: medreq-03 allows non-precise date if Data Absent Reason is present
FHIR-48362: correct to require min length of 10

  • Refactored au-core-medreq-03 for precise date or DAR, increased min length to 10

@dt-r dt-r requested a review from dbojicic September 24, 2024 23:05
@dt-r dt-r closed this Sep 24, 2024
@dt-r
Copy link
Collaborator

dt-r commented Sep 24, 2024

The PR should include both technical corrections on this invariant.

@JayMurdoch JayMurdoch removed the request for review from dbojicic October 2, 2024 10:58
@JayMurdoch JayMurdoch reopened this Oct 2, 2024
@JayMurdoch JayMurdoch changed the title Refactor au-core-medreq-03 (FHIR-46498) Refactor au-core-medreq-03 (FHIR-46498, FHIR-48362) Oct 2, 2024
input/pagecontent/changes.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@dt-r dt-r left a comment

Choose a reason for hiding this comment

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

See inline comments.

@JayMurdoch JayMurdoch requested a review from dt-r October 10, 2024 03:54
Copy link
Collaborator

@dt-r dt-r left a comment

Choose a reason for hiding this comment

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

See continued conversation

@JayMurdoch JayMurdoch requested review from dt-r and dbojicic and removed request for dt-r October 15, 2024 01:43
Copy link
Collaborator

@dbojicic dbojicic left a comment

Choose a reason for hiding this comment

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

Changes requested.

@@ -248,7 +248,7 @@
<key value="au-core-medreq-03"/>
<severity value="error"/>
<human value="Date shall be precise to the day or, if not available, the Data Absent Reason extension shall be present"/>
<expression value="(toString().length() &gt;= 8) xor extension('http://hl7.org/fhir/StructureDefinition/data-absent-reason').exists()"/>
<expression value="(value.exists() and value.toString().length() &gt;= 10 and extension('http://hl7.org/fhir/StructureDefinition/data-absent-reason').exists().not()) or (value.empty() and extension('http://hl7.org/fhir/StructureDefinition/data-absent-reason').exists())"/>
<xpath value="(f:matches(f:effectiveDateTime, /\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d([+-][0-2]\d:[0-5]\d|Z)) and not(exists(f:extension[@url='http://hl7.org/fhir/StructureDefinition/data-absent-reason']))) or (not(f:matches(f:effectiveDateTime, /\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d([+-][0-2]\d:[0-5]\d|Z))) and exists(f:extension[@url='http://hl7.org/fhir/StructureDefinition/data-absent-reason']))"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

please update xpath to match the updated expression.

Copy link
Collaborator

@dbojicic dbojicic Oct 25, 2024

Choose a reason for hiding this comment

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

I am not convinced this is the solution. Could not get it to work with FHIRPath Lab either.
I've raised a question in Zulip: MedicationRequest.authoredOn allow precise date or DAR ext; @JayMurdoch, could you please follow up if there is no response by early next week?

Copy link
Collaborator

@dbojicic dbojicic left a comment

Choose a reason for hiding this comment

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

Please see inline comments

@dbojicic
Copy link
Collaborator

dbojicic commented Oct 30, 2024

@JayMurdoch
based on the feedback in Zulip, here is a refined expression that seem to be working:

<expression value ="($this.hasValue().not() and extension('http://hl7.org/fhir/StructureDefinition/data-absent-reason').value.exists()) or ($this.hasValue() and $this.toString().length() >= 10 and extension('http://hl7.org/fhir/StructureDefinition/data-absent-reason').value.exists().not())"/>

@heathfrankel provided a simplified version of this:
<expression value ="$this.hasValue() implies (extension('http://hl7.org/fhir/StructureDefinition/data-absent-reason').value.exists().not() and $this.toString().length() >= 10)"/>

Initial test in Fhirpath-lab using HAPI validator passed, see
🧪 Test with FHIRPath-Lab

Could you please test both expressions?

@JayMurdoch
Copy link
Collaborator Author

JayMurdoch commented Oct 30, 2024

@dbojicic Test results - behaved as expected

<expression value ="($this.hasValue().not() and extension('http://hl7.org/fhir/StructureDefinition/data-absent-reason').value.exists()) or ($this.hasValue() and $this.toString().length() >= 10 and extension('http://hl7.org/fhir/StructureDefinition/data-absent-reason').value.exists().not())"/>

Test Outcome
Valid DAR pass
Valid date (to day) pass
Valid date (to day) + DAR fail
Valid date (to seconds) pass
Valid date (to seconds) + DAR fail
Invalid date (to month) fail
Invalid date (to month) + DAR fail

Test results - behaved as expected

<expression value ="$this.hasValue() implies (extension('http://hl7.org/fhir/StructureDefinition/data-absent-reason').value.exists().not() and $this.toString().length() >= 10)"/>

Test Outcome
Valid DAR pass
Valid date (to day) pass
Valid date (to day) + DAR fail
Valid date (to seconds) pass
Valid date (to seconds) + DAR fail
Invalid date (to month) fail
Invalid date (to month) + DAR fail

Proceeding with 2nd option as discussed with @dbojicic, @brettesler-ext, @johnscarter

Copy link
Collaborator

@dbojicic dbojicic left a comment

Choose a reason for hiding this comment

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

Thanks @JayMurdoch , just a few minor changes.

Copy link
Collaborator

@dbojicic dbojicic left a comment

Choose a reason for hiding this comment

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

One minor change

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