Fix and update time-limited job #908
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-3486 In [Remove licences with charge elements with approaching time limits from billing](#443) we added a background job to do just that. Then in [Refactor to use new view based WATER models](#569) we updated how we interact with the DB. We updated the service which fetches the data for the job. But we didn't clock that this results in the job erroring because we are no longer passing in `licenceId`. Without it we error trying to create the `workflow` record in the DB because of a constraint on the field. We spotted this when working on [New job to add new & updated licences to workflow](#903) so this change is mainly about fixing the bug. But we also realised one of the reasons we'd failed to spot the issue was a lack of logging. So, we also updated the job to use our pattern of logging how long the process took. We also take an idea from the new job and set a flag on the `workflow` record so we can spot _why_ it was created.
This brings the job in line with `/export` and `/licence-updates` and means we should see something in the logs when the job has run.
Taking a lesson from the `licence-updates` job we want to apply something to the workflow `data` property that will indicate why it was added. We could have just gone with `timeLimited: true` and that would have told us why it was added. But if we include the charge version ID instead it gives us something we can build on, for example, directly linking the manage workflow screen to the charge version with the time limited element.
We bring it in line with what we have done in our licence updates job. For example, there is no need to check if the fetch service returned any results because we now process them in a loop (no results no looping). We process them in a loop because it drops the dependence on the fetch service returning results that directly map to workflow records (the thing we broke and why we're making these changes). Plus it gives us more flexibility, for example, adding the charge version ID with the time limited charge element to the result.
Whilst in there we also were able to correct some things that might have caused errors down the road; using `returns()` and `throws()` with a stubbed service that is async could cause problems. Instead `resolves()` and `rejects()` should be used.
Cruikshanks
added a commit
to DEFRA/water-abstraction-ui
that referenced
this pull request
Apr 15, 2024
https://eaflood.atlassian.net/browse/WATER-4437 For context, we found out legacy background jobs scheduled in BullMQ only run intermittently. One we found hadn't run for the last 2 years so agreed to bin. Two we need to deal with at some point but intermittent is fine for now. The critical job, which is putting updated licences into workflow, we solved by migrating to [water-abstraction-system](DEFRA/water-abstraction-system#903). As part of solving it we took the opportunity to add data to the workflow record to allow us to identify that the record was added because of an updated licence. We also spotted that our time-limited job was broken 😱 so [fixed that](DEFRA/water-abstraction-system#908) and updated it to _also_ add some info to the workflow record. With these handy bits of info now in the workflow record it seems a shame we don't make the reason why the record was added visible to the user. So, this change amends the link in the 'To setup' based on that extra info. In the pre-handler that fetches the to-setup workflow records we now iterate through the results and add the link details to use in the view. If it spots the `timeLimitedChargeVersionId` property we add to the data field it will generate a link that will take the user directly to the charge version with the time limited element. Else if the `chargeVersionExists` property is present we know the record was added by the licence updates job. If the property is true we direct the user to the charge information tab for the licence rather than drop them into a new one. That way they can see the existing ones and determine if a new one is needed. Else if `chargeVersionExists` property is missing or false we leave the link as it was and direct the user to create a new charge version.
Cruikshanks
added a commit
to DEFRA/water-abstraction-ui
that referenced
this pull request
Apr 15, 2024
https://eaflood.atlassian.net/browse/WATER-4437 For context, we found out legacy background jobs scheduled in BullMQ only run intermittently. One we found hadn't run for the last 2 years so agreed to bin. Two we need to deal with at some point but intermittent is fine for now. The critical job, which is putting updated licences into workflow, we solved by migrating to [water-abstraction-system](DEFRA/water-abstraction-system#903). As part of solving it we took the opportunity to add data to the workflow record to allow us to identify that the record was added because of an updated licence. We also spotted that our time-limited job was broken 😱 so [fixed that](DEFRA/water-abstraction-system#908) and updated it to _also_ add some info to the workflow record. With these handy bits of info now in the workflow record, it seems a shame we don't make the reason why the record was added visible to the user. So, this change amends the link in the 'To setup' based on that extra info. In the pre-handler that fetches the to-setup workflow records we now iterate through the results and add the link details to use in the view. If it spots the `timeLimitedChargeVersionId` property we add to the data field it will generate a link that will take the user directly to the charge version with the time-limited element. Else if the `chargeVersionExists` property is present we know the record was added by the licence updates job. If the property is true we direct the user to the charge information tab for the licence rather than drop them into a new one. That way they can see the existing ones and determine if a new one is needed. Else if `chargeVersionExists` property is missing or false we leave the link as it was and direct the user to create a new charge version.
Cruikshanks
added a commit
to DEFRA/water-abstraction-service
that referenced
this pull request
Apr 15, 2024
https://eaflood.atlassian.net/browse/WATER-4437 For context, we found out legacy background jobs scheduled in BullMQ only run intermittently. One we found hadn't run for the last 2 years so we agreed to bin it. Two we need to deal with at some point but intermittent is fine for now. The critical job, which is putting updated licences into workflow, we solved by migrating to [water-abstraction-system](DEFRA/water-abstraction-system#903). As part of solving it we took the opportunity to add data to the workflow record to allow us to identify that the record was added because of an updated licence. We also spotted that our time-limited job was broken 😱 so [fixed that](DEFRA/water-abstraction-system#908) and updated it to _also_ add some info to the workflow record. With these handy bits of info now in the workflow record, it seemed a shame we don't make the reason why the record was added visible to the user. That led to [Make manage workflow 'to setup' links intelligent](DEFRA/water-abstraction-ui#2549) in the legacy UI code. The last piece of the puzzle is the historic `to_setup` records. The new links would be less confusing if the existing records were updated to also identify if they are for new licences that need charge information or existing ones that need their details checking. This adds a one-time migration to do just that!
Cruikshanks
added a commit
to DEFRA/water-abstraction-service
that referenced
this pull request
Apr 15, 2024
https://eaflood.atlassian.net/browse/WATER-4437 For context, we found out legacy background jobs scheduled in BullMQ only run intermittently. One we found hadn't run for the last 2 years so we agreed to bin it. Two we need to deal with at some point but intermittent is fine for now. The critical job, which is putting updated licences into workflow, we solved by migrating to [water-abstraction-system](DEFRA/water-abstraction-system#903). As part of solving it we took the opportunity to add data to the workflow record to allow us to identify that the record was added because of an updated licence. We also spotted that our time-limited job was broken 😱 so [fixed that](DEFRA/water-abstraction-system#908) and updated it to _also_ add some info to the workflow record. With these handy bits of info now in the workflow record, it seemed a shame we didn't make the reason why the record was added visible to the user. That led to [Make manage workflow 'to setup' links intelligent](DEFRA/water-abstraction-ui#2549) in the legacy UI code. The last piece of the puzzle is the historic `to_setup` records. The new links would be less confusing if the existing records were updated to also identify if they are for new licences that need charge information or existing ones that need their details checking. This adds a one-time migration to do just that!
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-3486
In Remove licences with charge elements with approaching time limits from billing we added a background job to do just that. Then in Refactor to use new view-based WATER models we updated how we interact with the DB.
We updated the service which fetches the data for the job. But we didn't spot that this results in the job erroring because we are no longer passing in
licenceId. Without it, we error trying to create theworkflowrecord in the DB because of a constraint on the field.We spotted this when working on New job to add new & updated licences to workflow so this change is mainly about fixing the bug.
But we also realised one of the reasons we'd failed to spot the issue was a lack of logging. So, we also updated the job to use our pattern of logging how long the process took. We also take an idea from the new job and set a flag on the
workflowrecord so we can spot why it was created.