Closed sarahmcdougall closed 1 year ago
St.:grey_question: |
Category | Percentage | Covered / Total |
---|---|---|---|
🟢 | Statements | 86.25% | 2258/2618 |
🟡 | Branches | 75.4% | 2087/2768 |
🟢 | Functions | 89.41% | 422/472 |
🟢 | Lines | 86.57% | 2178/2516 |
424 tests passing in 31 suites.
Report generated by 🧪jest coverage report action from 673731dd97b7af5c6868c6eb8493a236228c0aa3
Do we actually want moment.defaultFormat
instead of moment.defaultFormatUtc
?
defaultFormatUtc
seems to actually ignore any timezone offsets present in the datetime and just always use Z
, whereas defaultFormat
accounts for the timezone offset when spitting out the UTC date:
I missed this when initially looking at the moment
docs
Do we actually want
moment.defaultFormat
instead ofmoment.defaultFormatUtc
?
Update: After some discussion, using defaultFormatUtc
seems more appropriate given the possible CQL logic that could be encountered during measure calculation. Added a sentence to the documentation explaining that timezone offsets are not taken into account when they are supplied on the measurement period.
I did some more testing with moment.defaultFormatUtc
with provided measurement periods that do specify offsets, and the offsets get ignored, as expected.
Summary
Fixes https://github.com/projecttacoma/fqm-execution/issues/241 Updates measurement period precision support down to the millisecond.
New behavior
Fqm-execution now supports precision of measurement period down to the millisecond rather than only down to the minute. This has potential impacts on the calculation results, depending on the measurement period precision being used (i.e. results may be different between the master branch and this branch when using millisecond precision in the
CalculationOptions
measurement period)Code changes
src/execution/ValueSetHelper.ts
—> update themoment
format tomoment.defaultFormatUtc
inparseTimeStringAsUTC
andparseTimeStringAsUTCConvertingToEndOfYear
(see the moment documentation for more info on formatting)parseTimeStringAsUTCConvertingToEndOfYear
does not get used anywhere in the code base, so open to thoughts on whether to keep itREADME.md
—> documentation updates for measurement period precisiondefaultFormatUtc
format seems to align with the values allowed by a FHIR dateTime, which I included in the documentation. Open to thoughts on whether to explicitly cite the type as adateTime
or not.Testing guidance
.zip
) with the measurement period specified in the issue:2024-01-01T00:00:00.000+00:00
2024-12-31T23:59:59.999+00:00
Example CLI command (assuming the bundles are at the root of fqm-execution):
Pay attention to the “Narcolepsy Exclusion” statement. On the master branch, the statement will contain red clauses due to minute precision of the measurement period (
Condition.onsetDateTime
is2024-12-31T23:59:01:000Z
):On this branch, the entire statement should be green due to millisecond precision:![Screenshot 2023-06-05 at 10 26 29 AM](https://github.com/projecttacoma/fqm-execution/assets/87094479/cf04ebed-a48e-44ae-b483-8d6ccd1c17ff)
Bundles from GH issue: precisionBundles.zip