-
Notifications
You must be signed in to change notification settings - Fork 180
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
Create filter to format time with a colon #515
base: main
Are you sure you want to change the base?
Conversation
public static string FormatTimeWithColon(string input, string inputFormat = "HHmm") | ||
{ | ||
// For backwards compatibility, if the input is not a 4 digit numeric value then just return the value | ||
if (string.IsNullOrEmpty(input) || (!(input.All(char.IsDigit) && (input.Length == 4)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wi-y Is it better to ignore (input.Length == 4)
to make it more as a general filter because there are cases when time is represented as HHmmssSS
e.g. when OBX type(OBX.2.1 ) is time(TM) value(OBX.5.1) has seconds and ms. Also there is inputFormat
as filter arg which helps parsing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shabanlushaj agree - it is better to make this more generic for the other HL7v2 times. I've updated the pull request so that the filter is more generic.
Question: Is HHmmssSS
supported by HL7v2 for TM time? I was looking at the spec and it seemed to be HHMM[SS[.SSSS]][+/-ZZZZ]
with the milliseconds following a decimal. But either way both formats HHmmssff and HHmmss.ff should be supported now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wi-y Whenever it is UTC, timezone can be discarded. I believe same applies for time. It would be nice to have this filter support timezone.
20240107111232-0300
for datetime this is supported by converter (MSH.7 - Date/Time Of Message).
Tried something that might do the work, but we have to ignore check for input length and inputFormat having same length.
var input = "144100.0000+0300";
var inputFormat = "HHmmss.ffffzzzz";
var dt = DateTime.ParseExact(input, inputFormat, CultureInfo.InvariantCulture).TimeOfDay; // might need also an arg for preserve/local/utc output (same as datetime)
In this issue: #513 it was reported that the time being returned by a HL7v2 to FHIR liquid template was not in the correct format.
The input time from the HL7v2 message was "0730" and the template was returning a FHIR resource with a time value of "0730". This is technically not a valid time value for FHIR. The FHIR spec states that the format should be in HH:mm:ss format, so the templates technically should be updated to return "07:30:00" in this specific case and in other locations where HL7v2 - 2.8.35.2 Explicit time interval (ST) is used.
In this PR we are creating a new filter which can be used to format the time in the various places where the templates should be formatting the time.
The most common place is in the
timeOfDay
FHIR element like in this case of ServiceRequest: https://github.com/microsoft/FHIR-Converter/blob/370f5c1964c6e317ba406dad1b057b4399187b92/data/Templates/Hl7v2/Resource/_ServiceRequest.liquid#L533C1-L534C1Currently the liquid template passes the value from the HL7v2 message to the FHIR output without any formatting. But after this new time formatting filter is rolled out then the templates could be updated to use the filter to format the time correctly.
The templates could be updated as follows to add the formatting filter
format_time_with_colon