Handle timezones in bulk uploader and Fhir converter #5985

mpbrown · 2023-06-14T18:52:01Z

BACKEND PULL REQUEST

Related Issue

Resolves Handle time zones in CSV uploader #6025 and Fill blank fields (specimen collection date) in CSV uploader #5987
Related to Add test coverage to BulkUploadResultsToFhir #5844, Mike/5844 tests bulk upload results fhir #5946, and Add test date to Observation.issued for single-entry and bulk upload #5982

Changes Proposed

Convert datetime strings from CSV to ZonedDateTimes so that datetimes in FHIR bundles refer to the correct instant of time
- For datetimes that are not associated with any timezone, use UTC time with Z suffix.
  - Example: a timestamp of when an entry was updated
  - Format: 2021-12-20T19:00:00Z
- For datetimes that are associated with a timezone, use the UTC offset.
  - Example: datetime of specimen collection
  - Format: 2021-12-20T14:00:00-05:00
Updates DateTime regex to allow optional timezone code
- Some US codes like ET, EST, EDT, CT, MST, PDT, AKST, HST, etc are allowed
- Any TZ identifier string defined by ICANN tz database
If timezone is not specified in the datetime, use the relevant address to retrieve timezone metadata from Smarty.
If address validation fails, use fallback timezone of US/Eastern
Updates unit tests and removes now obsolete code in FhirConverter and BulkUploadResultsToFhir

Additional Information

Before we can convert it to a UTC timestamp, we need to convert the local test_result_date to the correct Date instant of time which requires knowing the data's time offset from UTC and whether it obeys daylight savings.

This PR attempts to validate the address of the testing lab and use the time zone metadata returned from Smarty for the correct UTC offset.

Testing

Check updated unit tests
Test on dev7 to make sure both the bulk uploader and single entry FHIR conversions work as expected

Use Z for system timestamps with setTimeZoneZulu

mpbrown · 2023-06-22T17:06:40Z

Created a new issue to track the time zone work specifically of referring to the bulk uploader test coverage issue. #6025

Will push some additional changes based on this soon

… into Mike/5844-handle-fhir-timezones

nathancrtr · 2023-07-06T21:02:44Z

@mpbrown are the datetime format requirements from the ticket up-to-date?

If so, I'm encountering a problem with (at least) the M/D/YYYY HH:mm TZ format. When using 12/20/2021 14:00 PDT as the value, the upload fails with the following errors:

mpbrown · 2023-07-06T22:07:03Z

@mpbrown are the datetime format requirements from the ticket up-to-date?
If so, I'm encountering a problem with (at least) the `M/D/YYYY HH:mm TZ` format. When using `12/20/2021 14:00 PDT` as the value, the upload fails with the following errors:

Nathan and I messaged about this in Slack and we may have to check with ReportStream to see what datetime formats they consider valid.

Test data csv includes:

Specimen_collection_date - 12/21/2021 14:00 PDT
Testing_lab_specimen_received_date - 12/22/2021 14:00 PDT

FHIR bundle gets created successfully with the following Specimen entry, correctly parsing the datetime strings as Pacific Time with UTC offset during December as -8

{
    "fullUrl": "Specimen/326117e3-f4f5-44cc-9387-407cd3601e44",
    "resource": {
        "resourceType": "Specimen",
        "id": "326117e3-f4f5-44cc-9387-407cd3601e44",
        "identifier": [
            {
                "value": "367f31bf-3bf2-43cd-8be4-dcab1daeb256"
            }
        ],
        "subject": {
            "reference": "Patient/1234"
        },
        "receivedTime": "2021-12-22T14:00:00-08:00",
        "collection": {
            "collectedDateTime": "2021-12-21T14:00:00-08:00"
        }
    }
},

… into Mike/5844-handle-fhir-timezones

Added requested caching feature

mpbrown · 2023-07-10T22:51:11Z

Added a data transformation step for the covid csv pipeline that converts the user's uploaded datetime strings to an offset format that is accepted by ReportStream

Re-deployed on dev7 with a successful ReportStream response

zdeveloper

LGTM! thank you for all this work

zdeveloper · 2023-07-11T14:28:04Z

backend/src/main/java/gov/cdc/usds/simplereport/service/errors/InvalidAddressException.java

+import java.io.Serial;
+
+public class InvalidAddressException extends RuntimeException {
+  @Serial private static final long serialVersionUID = 1L;


I am not familiar with @serial, what does it do?

That particular @Serial annotation was recommended by Intellij to allow compile-time checking of serialization-related declarations, similar to the checking enabled by the @Override annotation type to validate method overriding

zdeveloper · 2023-07-11T14:28:35Z

backend/src/main/java/gov/cdc/usds/simplereport/utils/BulkUploadResultsToFhir.java

@@ -56,12 +55,11 @@
 public class BulkUploadResultsToFhir {

  private static final String ALPHABET_REGEX = "^[a-zA-Z\\s]+$";
-  private static final String SNOMED_REGEX = "(^[0-9]{9}$)|(^[0-9]{15}$)";
-  private final ResultsUploaderDeviceValidationService resultsUploaderDeviceValidationService;
+  private static final String SNOMED_REGEX = "(^\\d{9}$)|(^\\d{15}$)";


whats the reason for this change?

SonarLint complaining that the digits could be replaced by \d, I could revert this if we prefer it to explicitly still use [0-9]

zdeveloper · 2023-07-11T14:31:44Z

backend/src/test/java/gov/cdc/usds/simplereport/test_util/JsonTestUtils.java

@@ -75,7 +77,8 @@ public static void assertJsonNodesEqual(
              fieldPath + "[" + i + "]");
        }
      } else { // Otherwise assert values are equal
-        assertThat(actualFieldValue.toString()).isEqualTo(expectedFieldValue.toString());
+        log.debug("Comparing " + fieldPath);


do we need this?

Useful when debugging the expected and actual JSON output for the FHIR bundler, but not necessary to keep in so I could comment out or remove

nathancrtr

I re-tried my test file in dev7 and the upload succeeded as expected. Thanks for investigating that issue! This LGTM 🚀

sonarcloud · 2023-07-11T19:16:06Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

94.4% Coverage
0.0% Duplication

Handle timezones in bulk uploader and Fhir converter

3e572e6

mpbrown mentioned this pull request Jun 14, 2023

Add test date to Observation.issued for single-entry and bulk upload #5982

Merged

Handle timezones in bulk uploader and Fhir converter

e1d4e6a

mpbrown force-pushed the Mike/5844-handle-fhir-timezones branch from e9e126b to e1d4e6a Compare June 14, 2023 20:56

mpbrown added 13 commits June 14, 2023 17:02

Fix intellij auto conversion to * import

6ad8c36

Check testResultTimeZoneInfo for null

89c1afb

Use ZonedDateTime for test result date

9025823

Use ZonedDateTime in DiagnosticReport

896b8f2

Use Z for system timestamps with setTimeZoneZulu

Fix intellij wildcard import

9eece4d

Handle null date tested on DiagnosticReport

5cf97ba

Update FhirConverter JSON test date format

b9d0afe

Merge branch 'main' into Mike/5844-handle-fhir-timezones

17774bf

Update issued as UTC instant from zoned time

46d8bdd

Use UTC Date instant for DiagnosticReport

1e26bcc

Remove ZonedDateTime helper method

1cfc271

Revert intellij wildcard import

9121ad7

Test invalid address exception

4fa2b6d

mpbrown force-pushed the Mike/5844-handle-fhir-timezones branch from 72cec4e to 4fa2b6d Compare June 20, 2023 13:13

Merge branch 'main' into Mike/5844-handle-fhir-timezones

e492dcb

mpbrown temporarily deployed to dev7 June 20, 2023 13:39 — with GitHub Actions Inactive

mpbrown marked this pull request as ready for review June 20, 2023 14:56

mpbrown requested review from mehansen, DanielSass, BobanL, nathancrtr and zdeveloper June 20, 2023 15:34

Merge branch 'main' into Mike/5844-handle-fhir-timezones

6e7ab9c

mpbrown added 2 commits June 23, 2023 10:30

Use timezone with ET fallback for date tested

b14fca0

Merge branch 'main' into Mike/5844-handle-fhir-timezones

cdd0821

Merge branch 'main' into Mike/5844-handle-fhir-timezones

6049b81

mpbrown linked an issue Jul 5, 2023 that may be closed by this pull request

Handle time zones in CSV uploader #6025

Closed

mpbrown temporarily deployed to dev7 July 5, 2023 15:46 — with GitHub Actions Inactive

mpbrown added 6 commits July 6, 2023 15:10

Fill specimen received time with order test date

a09a0f8

Add caching test and rename service

0b3674b

Fill date result released with test result date

dd27205

Fill blank ordering facility with testing lab

c8e93d6

Merge remote-tracking branch 'origin/Mike/5844-handle-fhir-timezones'…

275bdce

… into Mike/5844-handle-fhir-timezones

Merge branch 'main' into Mike/5844-handle-fhir-timezones

6f95836

mpbrown temporarily deployed to dev7 July 6, 2023 19:23 — with GitHub Actions Inactive

Update caching mock in DateTimeUtilsTest

e49d62d

Remove flaky test for not caching unique addresses

28a8a2c

mpbrown added 2 commits July 6, 2023 22:47

Add sonarlint fixes

6b0f975

Transform zoned datetimes for covid csv pipeline

31e4ba7

mpbrown temporarily deployed to dev7 July 10, 2023 18:24 — with GitHub Actions Inactive

mpbrown added 3 commits July 10, 2023 16:01

Merge branch 'main' into Mike/5844-handle-fhir-timezones

bd26d60

Use offset datetime string format

be9f247

Merge remote-tracking branch 'origin/Mike/5844-handle-fhir-timezones'…

3f07c8d

… into Mike/5844-handle-fhir-timezones

mpbrown temporarily deployed to dev7 July 10, 2023 21:43 — with GitHub Actions Inactive

mpbrown linked an issue Jul 10, 2023 that may be closed by this pull request

Fill blank fields (specimen collection date) in CSV uploader #5987

Closed

zdeveloper approved these changes Jul 11, 2023

View reviewed changes

nathancrtr approved these changes Jul 11, 2023

View reviewed changes

Merge branch 'main' into Mike/5844-handle-fhir-timezones

dc6bf59

mpbrown merged commit f09b3ff into main Jul 11, 2023
31 checks passed

mpbrown deleted the Mike/5844-handle-fhir-timezones branch July 11, 2023 20:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle timezones in bulk uploader and Fhir converter #5985

Handle timezones in bulk uploader and Fhir converter #5985

mpbrown commented Jun 14, 2023 •

edited

Loading

mpbrown commented Jun 22, 2023

nathancrtr commented Jul 6, 2023

mpbrown commented Jul 6, 2023

mpbrown commented Jul 10, 2023

zdeveloper left a comment

zdeveloper Jul 11, 2023

mpbrown Jul 11, 2023

zdeveloper Jul 11, 2023

mpbrown Jul 11, 2023

zdeveloper Jul 11, 2023

mpbrown Jul 11, 2023

nathancrtr left a comment

sonarcloud bot commented Jul 11, 2023

Handle timezones in bulk uploader and Fhir converter #5985

Handle timezones in bulk uploader and Fhir converter #5985

Conversation

mpbrown commented Jun 14, 2023 • edited Loading

BACKEND PULL REQUEST

Related Issue

Changes Proposed

Additional Information

Testing

mpbrown commented Jun 22, 2023

nathancrtr commented Jul 6, 2023

mpbrown commented Jul 6, 2023

mpbrown commented Jul 10, 2023

zdeveloper left a comment

Choose a reason for hiding this comment

zdeveloper Jul 11, 2023

Choose a reason for hiding this comment

mpbrown Jul 11, 2023

Choose a reason for hiding this comment

zdeveloper Jul 11, 2023

Choose a reason for hiding this comment

mpbrown Jul 11, 2023

Choose a reason for hiding this comment

zdeveloper Jul 11, 2023

Choose a reason for hiding this comment

mpbrown Jul 11, 2023

Choose a reason for hiding this comment

nathancrtr left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Jul 11, 2023

mpbrown commented Jun 14, 2023 •

edited

Loading