Move getting the licence holder to the model #683
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-4188 https://eaflood.atlassian.net/browse/WATER-4328 Much like in [Move licence end date logic to model](#681) we have another two tickets that need to identify the licence holder for a licence. We've already figured out how to do that in [Add licence ref. and holder to rtn. req. session](#639). But that implementation was specific to return requirements and unlike `myContact.$name()` and `myLicence.$ends()` to determine the licence holder you need to have pulled through a series of related models. So, for the same reasons we want to move this piece of logic to the model to make it easier to reuse. However, their will be differences in implementation. This change adds the logic to the model and then refactors `InitiateReturnRequirementSessionService` to demonstrate how to use it.
Cruikshanks
added
the
housekeeping
The first step to make this reusable is to make the query needed to fetch the additional records reusable. Thankfully, Objection.js has the concept of [modifiers](https://vincit.github.io/objection.js/recipes/modifiers.html), reusable query components we can chain to queries we build. We also tweak `InitiateReturnRequirementSessionService` to use the modifier and prove the query still works thanks to the unit tests still passing.
Again, because of the existing tests for `InitiateReturnRequirementSessionService` we can move the functionality to the model, update the service and confirm everything still works as expected.
We want the custom instance method tested within the model tests. Because these instance methods rely on the right data being available in the instance we also end up testing the modifier. This was the approach we took on [sroc-charging-module-api](https://github.com/DEFRA/sroc-charging-module-api); we add modifiers because numerous things depend on them and they are better tested through those things.
Because we cover the edge case for the query in our model tests we don't need to cover them in the `InitiateReturnRequirementSessionService` tests. So, we can simplify things there.
On review realised we didn't have test coverage for `$licenceHolder()` being called on an instance that has not been created with the `licenceDocument` and related properties instantiated. That flagged we'd forgotten the optional chaining! So, we fix that as well.
Cruikshanks
requested review from
Demwunz,
robertparkinson,
Jozzey and
Beckyrose200
robertparkinson
pushed a commit
that referenced
this pull request
Jan 24, 2024
https://eaflood.atlassian.net/browse/WATER-4188 https://eaflood.atlassian.net/browse/WATER-4328 Much like in [Move licence end date logic to model](#681) we have another two tickets that need to identify the licence holder for a licence. We've already figured out how to do that in [Add licence ref. and holder to rtn. req. session](#639). But that implementation was specific to return requirements and unlike `myContact.$name()` and `myLicence.$ends()` to determine the licence holder you need to have pulled through a series of related models. So, for the same reasons we want to move this piece of logic to the model to make it easier to reuse. However, there will be differences in implementation. This change adds the logic to the model and then refactors `InitiateReturnRequirementSessionService` to demonstrate how to use it.
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-4188
https://eaflood.atlassian.net/browse/WATER-4328
Much like in Move licence end date logic to model we have another two tickets that need to identify the licence holder for a licence. We've already figured out how to do that in Add licence ref. and holder to rtn. req. session.
But that implementation was specific to return requirements and unlike
myContact.$name()andmyLicence.$ends()to determine the licence holder you need to have pulled through a series of related models.So, for the same reasons we want to move this piece of logic to the model to make it easier to reuse. However, there will be differences in implementation.
This change adds the logic to the model and then refactors
InitiateReturnRequirementSessionServiceto demonstrate how to use it.