[cpruijsen]

Fixes #385

ToDateTime(String) now uses the execution timezone offset when the string has no offset, instead of the host machine's offset (new Date().getTimezoneOffset()). This matches the CQL specification (Appendix B, ToDateTime): "If no timezone offset is supplied, the timezone offset of the evaluation request timestamp is assumed." DateTime literals, the DateTime(...) selector and the ToDateTime(Date) overload already do this.

Changes

• src/datatypes/datetime.ts: DateTime.parse accepts an optional defaultTimezoneOffset, used only when the string has no Z and no explicit offset.
• src/elm/type.ts: ToDateTime.exec passes ctx.getExecutionDateTime().timezoneOffset for the String overload, the same value the Date overload uses.
• The other DateTime.parse callers (cql-patient.ts and the min/max constants) are unchanged because the parameter is optional.

This is the fix suggested in the issue. A default in the DateTime constructor would also work, but it would change every other caller, including patient data parsing.

Result

Execution offset +00:00, host at -06:00:

Expression	Before	After
ToDateTime('2014-01-01T12:05')	2014-01-01T12:05-06:00	2014-01-01T12:05+00:00
ToDateTime('2014-01-01T12:05') = @2014-01-01T12:05	false	true
ToDateTime('2014-01-01T12:05Z')	+00:00	+00:00
ToDateTime('2014-01-01T12:05:05.955+01:30')	+01:30	+01:30

Tests

• test/datatypes/datetime-test.ts: DateTime.parse uses the default offset only when the string has none.
• test/elm/convert/convert-test.ts: ToDateTime('2015-01-02') gets the execution offset. The test uses -3.25, which no real time zone uses, so it cannot pass by matching the host offset.

Both new offset tests fail without the source change and pass with it.

Follow-up

The four skip-list.txt entries (CqlTypeOperatorsTest.Convert.StringToDateTime and CqlTypeOperatorsTest.ToDateTime.ToDateTime1 to ToDateTime3) stay skipped. The skip markers live in the generated spec-test JSON, which needs regeneration with the translator, so I left that out of this PR. Evaluated directly, those expressions now return the execution offset.

Submitter:

• This pull request describes why these changes were made
• Code diff has been done and been reviewed (it does not contain: additional white space, not applicable code changes, debug statements, etc.)
• Tests are included and test edge cases
• Tests have been run locally and pass
• Code coverage has not gone down and all code touched or added is covered.
• Code passes lint and prettier (hint: use npm run check to run tests, lint, and prettier)
• All dependent libraries are appropriately updated or have a corresponding PR related to this change (N/A)

Reviewer:

Name:

• Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task's purpose
• The tests appropriately test the new code, including edge cases
• You have tried to break the code

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.19%. Comparing base (b1a4c3e) to head (c368a16).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #387   +/-   ##
=======================================
  Coverage   89.19%   89.19%           
=======================================
  Files          60       60           
  Lines        5062     5064    +2     
  Branches     1445     1446    +1     
=======================================
+ Hits         4515     4517    +2     
  Misses        315      315           
  Partials      232      232           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cmoesel]

Thank you for the contribution, @cpruijsen. This looks really good and fixes a bug that's probably been there for quite a long time!

I also confirmed that if I re-enable those four skipped tests, they all pass. If you have Java 17 and Gradle 9 installed on your system, please feel free to remove those tests from the skip file and regenerate the ELM bits via npm run build:spec-test-data. If you don't have Java 17 and Gradle 9, however, I can either push up a commit to your branch that re-enables them or we'll handle it post PR. What is your preference?

[cpruijsen]

Thanks! I removed the four tests from the skip list and regenerated the ELM in c368a16. They all pass locally.

[cmoesel]

Perfect. Thanks for the contribution, @cpruijsen!