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

Add ability for primary linked data to be updated when associated data is changing #72

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

Conversation

tjpeel-ee
Copy link

@tjpeel-ee tjpeel-ee commented Jan 22, 2025

Primarily for the benefit of the PHA API, we need to signal when primary linked data is changing.

When a movement is linked to an import notification, the relationship is created between the two entities and saved, therefore triggering a change to the notification's Updated field.

Should a change to the movement be processed, it does not need to link as it's already linked and therefore on the movement is updated.

The PHA API queries import notifications that have changed within a period of time by looking at the notification's Updated field. If an associated movement is changed then currently the linked notification will not be updated. This PR changes that behaviour and it updates each linked notification if the movement is deemed to be changing too.

The logic for this has been kept within the Linking business space but included as a separate service.

The AssociatedDataService would also be used when GVMS data is handled by BTMS and linked to import notifications, although it's unclear at this stage if GVMS data will actually change once it has been created.

This PR proposes the changes for the BTMS team to review and critique.

A working assuming is that the AssociatedDataService's Update method will only be called with import notifications that already exist.

The BTMS integration tests have not been run yet either. They look to have run as part of the build, despite quite a few being SKIPPED.

@craigedmunds
Copy link
Contributor

craigedmunds commented Jan 23, 2025

I didn't envisage using the updated field for this, as i think it's important we retain when this resource itself has changed. I'd intended to store an updated field on the relationship, as that seems the right place for it

Something like this:

notification-with-updated.json

We'd then need to make sure you can query by that updated field, something like import-notifications?filter=?filter=or(greaterThan(updated,'2025-01-23T15:00'),greaterThan(relationships.meta.updated,'2025-01-23T15:00'))

I don't think a query like that would work out of the box due to the nested filtering described here https://eaflood.atlassian.net/browse/CDMS-281. But I think we'd be able to easily add this capability.

Copy link

Code Coverage

Package Line Rate Branch Rate Health
Btms.Consumers 76% 55%
Btms.Azure 43% 100%
Btms.Business 85% 70%
Btms.Types.Alvs.Mapping.V1.Tests 0% 0%
Btms.Types.Ipaffs 95% 62%
Btms.Types.Gvms 67% 100%
Btms.Types.Alvs.Mapping 76% 0%
Btms.Types.Gvms.Mapping 38% 26%
Btms.Model 91% 94%
Btms.Common 73% 25%
TestDataGenerator 86% 79%
Btms.Types.Ipaffs.Mapping 76% 59%
Btms.BlobService 39% 29%
Btms.Metrics 87% 0%
Btms.Backend.Data 69% 61%
Btms.Types.Alvs 84% 50%
Btms.SensitiveData 79% 83%
Btms.SyncJob 73% 62%
Btms.Backend 61% 42%
TestGenerator.IntegrationTesting.Backend 83% 70%
Btms.Analytics 70% 68%
Btms.Emf 11% 0%
Summary 78% (9576 / 12210) 62% (979 / 1591)

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.

2 participants