-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add notifications setup ad hoc licence fetch #1718
Merged
jonathangoulding
merged 8 commits into
main
from
feature-notifications-setup-ad-hoc-licence-review
Feb 14, 2025
Merged
Add notifications setup ad hoc licence fetch #1718
jonathangoulding
merged 8 commits into
main
from
feature-notifications-setup-ad-hoc-licence-review
Feb 14, 2025
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-4900 This change introduces the 'journey' context into the notifications setup fetch services. Due to the complexity of the queries we have decided to pass the session into the fetch services. The session contains a journey key which we use to determine what query to make. When the journey is for an ad hoc licence we only want to fetch the recipients for that licence (we are not concerned with the returns period, due date and if is a summer return). This change replaces some of the existing fetch requests 'where' clauses. As the query is a template literal string we can 'inject' the relevant where clause into the query. The previous returns periods query results should not be affected.
I will update the page title and any other issues (not related to the fetch) in another pr for the ad hoc returns journey. |
jonathangoulding
added a commit
that referenced
this pull request
Feb 14, 2025
https://eaflood.atlassian.net/browse/WATER-4900 This change introduces the 'journey' context into the notifications setup review service and presenter. This journey context is used to determine what text is shown to the user relative to the journey. The ad hoc journey is a 'notification' and the invitations (and reminder - not yet implemented) is an 'invitations'. The ad hoc journey does not show the remove licences link and has been blanked to be hidden in the view. The previous static text for additional information and button text has been moved into the presenter to enable dynamic content. Previous work has been done for the fetching of recipients #1718.
Cruikshanks
requested changes
Feb 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some NITS in the comments. Love how the where
has been incorporated!
app/services/notifications/setup/fetch-download-recipients.service.js
Outdated
Show resolved
Hide resolved
…vice.js Co-authored-by: Alan Cruikshanks <[email protected]>
Co-authored-by: Alan Cruikshanks <[email protected]>
Cruikshanks
approved these changes
Feb 14, 2025
jonathangoulding
added a commit
that referenced
this pull request
Feb 14, 2025
#1721) * Update notifications review to present ad-hoc and invitations journeys https://eaflood.atlassian.net/browse/WATER-4900 This change introduces the 'journey' context into the notifications setup review service and presenter. This journey context is used to determine what text is shown to the user relative to the journey. The ad hoc journey is a 'notification' and the invitations (and reminder - not yet implemented) is an 'invitations'. The ad hoc journey does not show the remove licences link and has been blanked to be hidden in the view. The previous static text for additional information and button text has been moved into the presenter to enable dynamic content. Previous work has been done for the fetching of recipients #1718.
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-4900
This change introduces the 'journey' context into the notifications setup fetch services.
Due to the complexity of the queries we have decided to pass the session into the fetch services. The session contains a journey key which we use to determine what query to make.
When the journey is for an ad hoc licence we only want to fetch the recipients for that licence (we are not concerned with the returns period, due date and if is a summer return).
This change replaces some of the existing fetch requests 'where' clauses. As the query is a template literal string we can 'inject' the relevant where clause into the query.
The previous returns periods query results should not be affected.