Allow datetime values to be treated as strings #3900
Conversation
rogordon01
added
Enhancement
rogordon01
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/Microsoft.Health.Fhir.Shared.Api/Controllers/ConvertDataController.cs
Fixed
Show fixed
Hide fixed
| @@ -92,7 +93,7 @@ public async Task GivenCcdaConvertDataRequest_WithADefaultTemplates_CorrectResul | |||
| public async Task GivenJsonConvertDataRequest_WithADefaultTemplates_CorrectResultShouldReturn() | |||
| { | |||
| var convertDataEngine = GetDefaultEngine(); | |||
| var request = GetJsonRequestWithDefaultTemplates(); | |||
| var request = GetJsonRequestWithDefaultTemplates(true); | |||
There was a problem hiding this comment.
nit: add param name for values passed as is.
| @@ -180,6 +189,7 @@ private static HashSet<string> GetSupportedParams() | |||
| ConvertDataProperties.InputDataType, | |||
| ConvertDataProperties.TemplateCollectionReference, | |||
| ConvertDataProperties.RootTemplate, | |||
| ConvertDataProperties.JsonDeserializationTreatDatesAsStrings, | |||
There was a problem hiding this comment.
By adding it in the supported param list, upon validation of params we are enforcing this parameter is provided in the request, right? Is that intended or can this simply not be provided in the request and default if so. If it is the former, I suppose we will need an api version update.
There was a problem hiding this comment.
The supported param list contains all of the params that this controller works with. So if a parameter provided by the end user doesn't exist within this list then the user will get an error. It doesn't enforce that these params need to exist, though. The various ReadXXXParameter method seem to do that.
As you've stated, I've opted to allow a default value to be used if one isn't provided. I added a new ReadBoolParameter which will return false if the parameter is not present.
src/Microsoft.Health.Fhir.Core/Data/OperationDefinition/convert-data.json
Outdated
Show resolved
Hide resolved
Currently, default Newtonsoft JsonSerialization settings are used when performing Json deserialization. This impacts datetime values, as the default behavior is to convert them into .Net
DateTimeobjects. This causes unwanted formatting issues and leads to loss of TimeZone information on the converted data.This PR allows end users to instruct Newtonsoft to simply treat dates as strings during deserialization. This allows the datetime information to be preserved as-is. They can then be assured that any filters are applied on the original value, and not a value modified by Newtownsoft.
End users must opt-in to this new behavior by specifying a new Parameter.
Current behavior:
When enabled:
Description
Describe the changes in this PR.
Related issues
Addresses AB#120779.
Testing
Describe how this change was tested.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)