Skip to content
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

Do not amend local date time value from HMRC to UTC #121

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

tjpeel-ee
Copy link
Contributor

@tjpeel-ee tjpeel-ee commented Feb 6, 2025

The current persistence model when writing a DateTime to MongoDB will convert any Local or Unspecified Kind to UTC. This means any date times in BTMS that should be local are not actually local when they are read from the DB or returned via the API.

This PR amends the GMR data structure so it does not amend the type from the GVMS open API spec being used to generate the internal types. It preserves string? instead of assuming DateTime?.

This is the correct approach as we do not want to alter the behaviour in any way.

These statements have now been superseded following feedback from @craigedmunds in relation to using strong types.

Original discussion:

We could use DateTimeOffset instead as this persists correctly in MongoDB and can be retrieved correctly, however second, millisecond and offset values are appended to the values HMRC gives us during serialisation, which then contradicts the documentation from their API that we are using in BTMS as seconds should not be included.

We could also add a specific serialiser for these values so they are returned correctly via the BTMS API, however this starts to get convoluted for what should be a simple solution - retaining the type we are given.

Other options also exist such as using concepts like [BsonDateTimeOptions(Kind = DateTimeKind.Unspecified)] but they are equally convoluted.

The amended field names within BTMS have also been updated to include a "local" name prefix as this level of detail was also lost during integration.

When the goods movement is departing, the date time is local to the originating port. When it is arriving, it is local to the destination port. Therefore, this information should not be lost.

The new approach taken:

DateTimeOffset is used for the strong type so saving via MongoDB does not introduce any change of value.

A new JsonConverter called LocalDateTimeNoSecondsJsonConverter has been added to be explicit about the how the value will be serialised.

t11omas
t11omas previously approved these changes Feb 7, 2025
craigedmunds
craigedmunds previously approved these changes Feb 7, 2025
@tjpeel-ee tjpeel-ee dismissed stale reviews from craigedmunds and t11omas via 00ebd5b February 11, 2025 09:16
@tjpeel-ee tjpeel-ee force-pushed the PHAIN-74-gmr-datatypes branch from 3bccaba to 00ebd5b Compare February 11, 2025 09:16
Copy link

Code Coverage

Package Line Rate Branch Rate Health
Btms.Business 88% 84%
Btms.SyncJob 63% 45%
Btms.Metrics 84% 0%
Btms.Analytics 74% 73%
Btms.Backend.Data 64% 58%
Btms.Types.Ipaffs 95% 62%
TestDataGenerator 86% 80%
Btms.Backend 62% 39%
Btms.BlobService 38% 27%
Btms.Emf 11% 0%
Btms.Common 74% 56%
Btms.Azure 43% 100%
Btms.Consumers 87% 79%
Btms.Types.Alvs 85% 67%
Btms.Types.Gvms.Mapping 38% 25%
Btms.Types.Alvs.Mapping 81% 0%
Btms.Types.Ipaffs.Mapping 77% 50%
Btms.Types.Gvms 61% 100%
Btms.SensitiveData 79% 83%
Btms.Types.Alvs.Mapping.V1.Tests 0% 0%
Btms.Model 92% 94%
TestGenerator.IntegrationTesting.Backend 83% 71%
Summary 79% (9957 / 12540) 64% (977 / 1518)

@tjpeel-ee tjpeel-ee force-pushed the PHAIN-74-gmr-datatypes branch from 00ebd5b to 807577f Compare February 11, 2025 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants