Fix return req frequency fields being imported #2556
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://eaflood.atlassian.net/browse/WATER-4473 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're 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, this change adds a migration to add a new `reporting_frequency` field to `water.return_requirements`.
Cruikshanks
added a commit
to DEFRA/water-abstraction-system
that referenced
this pull request
Jun 10, 2024
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
added a commit
to DEFRA/water-abstraction-system
that referenced
this pull request
Jun 10, 2024
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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_frequencywe 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_frequencyrepresents. We thought it was the reporting frequency; however it turns out NALD has 3 frequency levels.ARTC_RET_FREQ_CODE)ARTC_REC_FREQ_CODE)ARTC_CODE)Because we assumed
ARTC_RET_FREQ_CODEwas reporting, we're importingARTC_REC_FREQ_CODEas collection. Now we know it should have beenARTC_REC_FREQ_CODEis reporting frequencyARTC_CODEis collection frequencyWe don't want to touch
ARTC_RET_FREQ_CODEbecause we're still not 100% sure that nothing in the legacy code is using these tables. So, this change adds a migration to add a newreporting_frequencyfield towater.return_requirements.