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

Fix return req. frequency fields in view #1083

Merged
merged 2 commits into from
Jun 10, 2024

Conversation

Cruikshanks
Copy link
Member

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

We recently extended the import of return requirements from NALD to include the frequency with which returns need to be collected. We thought with this and returns_frequency we would have the data we need to support the return requirements setup journey.

However, we now realise we made an incorrect assumption about what returns_frequency represents. We thought it was the reporting frequency; however it turns out NALD has 3 frequency levels.

  • Return to agency (ARTC_RET_FREQ_CODE)
  • Recording (ARTC_REC_FREQ_CODE)
  • Collection (ARTC_CODE)

Because we assumed ARTC_RET_FREQ_CODE was reporting, we were importing ARTC_REC_FREQ_CODE as collection. Now we know it should have been

  • ARTC_REC_FREQ_CODE is reporting frequency
  • ARTC_CODE is collection frequency

We don't want to touch ARTC_RET_FREQ_CODE because we're still not 100% sure that nothing in the legacy code is using these tables. So, we have made changes to water-abstraction-service and water-abstraction-import to add a new reporting_frequency field to water.return_requirements.

This change corrects the migrations we wrote (because they are not yet live) for return requirements to include the new field.

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

We recently extended the import of return requirements from NALD to include the frequency with which returns need to be collected. We thought with this and `returns_frequency` we would have the data we need to support the return requirements setup journey.

However, we now realise we made an incorrect assumption about what `returns_frequency` represents. We thought it was the reporting frequency; however it turns out NALD has 3 frequency levels.

- Return to agency (`ARTC_RET_FREQ_CODE`)
- Recording (`ARTC_REC_FREQ_CODE`)
- Collection (`ARTC_CODE`)

Because we assumed `ARTC_RET_FREQ_CODE` was reporting, we were importing `ARTC_REC_FREQ_CODE` as collection. Now we know it should have been

- `ARTC_REC_FREQ_CODE` is reporting frequency
- `ARTC_CODE` is collection frequency

We don't want to touch `ARTC_RET_FREQ_CODE` because we're still not 100% sure that nothing in the legacy code is using these tables. So, we have made changes to [water-abstraction-service](DEFRA/water-abstraction-service#2556) and [water-abstraction-import](DEFRA/water-abstraction-import#947) to add a new `reporting_frequency` field to `water.return_requirements`.

This change corrects the migrations we wrote (because they are not yet live) for return requirements to include the new field.
@Cruikshanks Cruikshanks added the bug Something isn't working label Jun 10, 2024
@Cruikshanks Cruikshanks self-assigned this Jun 10, 2024
@Cruikshanks Cruikshanks marked this pull request as ready for review June 10, 2024 11:19
@Cruikshanks Cruikshanks merged commit 007a491 into main Jun 10, 2024
6 checks passed
@Cruikshanks Cruikshanks deleted the fix-return-requirements-view-frequencies branch June 10, 2024 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant