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

Update Ret. vers. end-dates query to handle new #1015

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Cruikshanks
Copy link
Member

https://eaflood.atlassian.net/browse/WATER-4514

Part of the work to migrate return version management from NALD to WRLS

In Add missing return version end dates during import, we added a query to the import that will fill in the missing end dates for return versions imported from NALD. WRLS requires a 'complete' history in the versions (no gaps), which isn't enforced in NALD.

It works great! The problem is it didn't consider returns versions added directly to WRLS. In fairness, it didn't need to. The intent is that the import would be killed on the day WRLS took over return version management.

In practice, though, the team and our users are trying to conduct testing of the new functionality, but then they see what they've added messed with when the import runs during the night.

This change updates the query to ignore stuff added directly in WRLS and just look at the records imported from NALD to prevent problems.

https://eaflood.atlassian.net/browse/WATER-4514

> Part of the work to migrate return version management from NALD to WRLS

In [Add missing return version end dates during import](#954) we added a query to the import that will fill in the missing end dates for return versions imported from NALD. WRLS requires a 'complete' history in the versions (no gaps) which isn't enforced in NALD.

It works great! The problem is it didn't consider returns versions added directly to WRLS. In fairness, it didn't need to. The intent is that the import would be killed on the day WRLS took over return version management.

In practice though, the team and our users are trying to conduct testing of the new functionality, but then seeing what they've added messed with when the import runs during the night.

This change updates the query to ignore stuff added directly in WRLS, and just look at the records imported from NALD to stop things getting messed up.
@Cruikshanks Cruikshanks added the housekeeping Refactoring, tidying up or other work which supports the project label Aug 30, 2024
@Cruikshanks Cruikshanks self-assigned this Aug 30, 2024
So, if we had the following, the query later wouldn't be trying to compare those where end date is null against a max version number that is greater than any on the imported ones.

| ID | version no | end date   | external ID |
|----|------------|------------|-------------|
| A  | 103        |            |             |
| B  | 102        | 2022/06/01 | 6:100356    |
| C  | 101        |            | 6:100341    |
| D  | 100        | 2018/07/19 | 6:100341    |

> Max version number just considering imported records is 102. Not 103.
We're only trying to fill in the missing end dates for 'current' records. Those with a status of 'draft' are incomplete return versions we consider to be invalid and ignore.
The external ID being null indicates that the record was created in WRLS. If it has no end date it is intended. So, we also want to exclude these from any records getting updated.
This is the equivalent of wearing suspenders and a belt! But just make sure whatever date we pull only comes from records that are valid and were imported. `draft` and those created in WRLS should be ignored.
@Cruikshanks Cruikshanks force-pushed the update-ret-version-end-dates-query branch from e20d0eb to 331f0a9 Compare August 30, 2024 13:21
@Cruikshanks Cruikshanks force-pushed the update-ret-version-end-dates-query branch from 331f0a9 to 485ad8e Compare August 30, 2024 13:21
Realised in testing these changes that there is another source of data getting messed up.

Where we have not added any new return versions to a licence the following has been happening;

- import runs
- finds existing records so updates their dates and status because of the ON CONFLICT()
- our clean up scripts run and repopulate missing end dates

We expect the last imported return version to not have an end date and our clean up queries are not filling them in. So, the latest return versions would have a blank end date.

Then, a user adds a new return version via WRLS. When that is added it will populate the end date of the last return version imported to be start date minus one day.

A bit later, it is night and the import runs again

- import runs
- finds existing records so updates their dates and status because of the ON CONFLICT()
- argh! It has reset the end date to NULL
- our clean up scripts run but don't consider this record as having a missing end date

Now the user goes in and finds the second record no longer has an end date. Plus our downstream functions start failing because they can no longer determine which return version covers a particular period.
@Cruikshanks Cruikshanks added the do not merge Used for spikes and experiments label Aug 30, 2024
@Cruikshanks
Copy link
Member Author

I got the fix dates query to ignore WRLS stuff. But then we found/remembered that the import will overwrite end dates we've set when adding new return versions.

The last commit removes the update from the import. But if this got dropped into prod we might miss a user going into NALD, reopening a version, and making a change (NALD lets them do that!)

If I remove all my changes to the fix dates query, yes the import overwrites the end date we add, but then the query fixes it! Or it did in the cases I was testing.

So, decided to park this until either we understand the original NALD vs WRLS records issue better, or we go live and can drop the import altogether!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Used for spikes and experiments housekeeping Refactoring, tidying up or other work which supports the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant