-
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 Hapi yar session manager to the project #926
Merged
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-4447 > This is the first and key part in a series of changes to properly support 'flash' banner notifications in the project. [Yar](https://hapi.dev/module/yar/) adds session support to hapi > a persistent state across multiple browser requests using an [iron](https://hapi.dev/module/iron) encrypted cookie and server-side storage. **yar** tries to fit session data into a session cookie based on a configured maximum size. If the content is too big to fit, it uses server storage via the [hapi plugin cache](https://hapi.dev/api#server.cache()) interface. There are times we need to persist state across multiple browser requests. A common reason is when you need to take users through a journey before committing the change, for example, setting up a bill run. We _do not_ intend to use **yar** in these situations. We persist journeys using our `SessionModel` (checkout `app/controllers/bill-runs-setup.controller.js` for an example). Most session managers like **yar** rely on creating a cookie. This will hold a session ID. Out-of-the-box they will also support you adding the rest of your session data to the cookie. But as it is common to 'blow' the cookie with too much data they will also support working with server-side storage. The legacy code uses *yar** in conjunction with {@link https://hapi.dev/module/catbox-redis/ | catbox-redis} to store session data in {@link https://redis.io/ | Redis} (the ID in the session is the ID for the record in Redis!) That's perfect for high volume sites and to be fair is considered 'the norm'. But it adds cost and complexity to our project that we just do not need. Hence we save 'journey' data to our PostgreSQL DB in the `sessions` table. The other need for a session manager is when you want to display a one-off notification banner to a user. For example, they've performed an action that has resulted in a change to the record. For example, - A licence review is currently not 'in progress' - The user clicks the 'Mark progress' button. This sends a POST request - The server updates the licence review to 'in progress' and redirects to the licence review page (as per the {@link https://www.geeksforgeeks.org/post-redirect-get-prg-design-pattern/ | PRG pattern}) - The redirect causes a GET request which results in the server getting the updated record - The licence review is displayed to the user In this situation we want to display a banner to the user confirming a change was just made. The problem is there is nothing to tell the GET request that this is so. We could do the following - Don't redirect but respond directly from the POST - This breaks PRG which you'll see if you hit refresh. The browser will resend your POST request which might have unintended consequences - Set a flag in the DB - You then have to add logic to the GET request to update the record and remove the flag. It is also storing something in the DB that we only care about from a UI/behaviour context - Add a query string param - This can be seen by the user and feels a bit 'ick'. Also, it could be abused. There is nothing to stop users adding the flag themselves to force the banner to appear. It also wouldn't clear when the browser is refreshed. We're not the first to encounter this problem. Hence why session cookies and **yar** exist. **yar** even has a feature specific to this use case. - In the POST request call `request.yar.flash('banner', 'This licence has been marked.') - In the GET request call `const [bannerMessage] = request.yar.flash('banner')` **yar** will store the message in the `wrlsSession` cookie. The call in the GET not only retrieves the value it also tell's **yar** to delete it from the cookie. If the user hits refresh the second GET will find `banner` is `null`. --- > Note about multiple sessions Unfortunately, adding **yar** to our service means there are now two session cookies. The legacy project being first means it gets 'dibs' on the default name 'session'. We did try to reuse the 'session' cookie. Unfortunately, you tell **yar** to use the server's cache instead of the cookie by setting its size to 0. This is how the legacy code has configured it which means any calls to `set()` or `flash()` result in nothing being persisted. So, we create our own 'wrlsSession' cookie. The upside is we're not bound by the legacy cookie and when the time comes can drop it completely without additional work or adverse affects to the user.
yar requires a password with a minimum of 32 characters. That is fine locally because we use the same one as the legacy service. In CI though we were just setting a simple one because that is all the auth cookie needed!
Cruikshanks
requested review from
Demwunz,
robertparkinson,
Jozzey,
Beckyrose200 and
rvsiyad
April 22, 2024 09:08
Jozzey
approved these changes
Apr 22, 2024
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.
robertparkinson
approved these changes
Apr 22, 2024
Cruikshanks
added a commit
that referenced
this pull request
Apr 23, 2024
https://eaflood.atlassian.net/browse/WATER-4447 We recently added [Hapi yar session manager to the project](#926). This was specifically to support flash notification banners (ones that appear on first GET request but then disappear). The Review licence screen has a number of these already. Ones for when you toggle the progress and and others for toggling the status. These have been achieved by breaking the [post-redirect-get pattern](https://www.geeksforgeeks.org/post-redirect-get-prg-design-pattern/) and responding directly from the POST request. As a first step to switching to using **yar** we need to refactor `ReviewLicenceService` into two; `ReviewLicenceService` and `SubmitReviewLicenceService`. This means it will also follow the pattern we've been using in other areas of the project. This change is that first step. We'll switch to **yar** in the next one.
Cruikshanks
added a commit
that referenced
this pull request
Apr 23, 2024
The keen eyed will see this is an exact copy of `ReviewLicenceService` before we refactored it! The scope of this PR is to split the GET and POST request handling to get us in a position we can re-write how the POST is handled to use [yar](#926) and fix it to support post-redirect-get again. So, we not only just copy the code across, we also copy the test as it was. We promise we'll be updating it in a subsequent change! ;-)
Cruikshanks
added a commit
that referenced
this pull request
Apr 23, 2024
https://eaflood.atlassian.net/browse/WATER-4447 We recently added [Hapi yar session manager to the project](#926). This was specifically to support flash notification banners (ones that appear on first GET request but then disappear). The Review licence screen has a number of these already. Ones for when you toggle the progress and and others for toggling the status. These have been achieved by breaking the [post-redirect-get pattern](https://www.geeksforgeeks.org/post-redirect-get-prg-design-pattern/) and responding directly from the POST request. As a first step to switching to using **yar** we need to refactor `ReviewLicenceService` into two; `ReviewLicenceService` and `SubmitReviewLicenceService`. This means it will also follow the pattern we've been using in other areas of the project. This change is that first step. We'll switch to **yar** in the next one.
Cruikshanks
added a commit
that referenced
this pull request
Apr 23, 2024
The keen eyed will see this is an exact copy of `ReviewLicenceService` before we refactored it! The scope of this PR is to split the GET and POST request handling to get us in a position we can re-write how the POST is handled to use [yar](#926) and fix it to support post-redirect-get again. So, we not only just copy the code across, we also copy the test as it was. We promise we'll be updating it in a subsequent change! ;-)
Cruikshanks
added a commit
that referenced
this pull request
Apr 24, 2024
https://eaflood.atlassian.net/browse/WATER-4447 We recently added [Hapi yar session manager to the project](#926). This was specifically to support flash notification banners (ones that appear on first GET request but then disappear). The Review licence screen has a number of these already. Ones for when you toggle the progress and and others for toggling the status. These have been achieved by breaking the [post-redirect-get pattern](https://www.geeksforgeeks.org/post-redirect-get-prg-design-pattern/) and responding directly from the POST request. As a first step to switching to using **yar** we need to refactor `ReviewLicenceService` into two; `ReviewLicenceService` and `SubmitReviewLicenceService`. This means it will also follow the pattern we've been using in other areas of the project. This change is that first step. We'll switch to **yar** in the next one.
Cruikshanks
added a commit
that referenced
this pull request
Apr 24, 2024
The keen eyed will see this is an exact copy of `ReviewLicenceService` before we refactored it! The scope of this PR is to split the GET and POST request handling to get us in a position we can re-write how the POST is handled to use [yar](#926) and fix it to support post-redirect-get again. So, we not only just copy the code across, we also copy the test as it was. We promise we'll be updating it in a subsequent change! ;-)
Cruikshanks
added a commit
that referenced
this pull request
Apr 24, 2024
https://eaflood.atlassian.net/browse/WATER-4447 We recently added [Hapi yar session manager to the project](#926). This was specifically to support flash notification banners (ones that appear on the first GET request but then disappear). The Review licence screen has a number of these already. Ones for when you toggle the progress and others for toggling the status. These have been achieved by breaking the [post-redirect-get pattern](https://www.geeksforgeeks.org/post-redirect-get-prg-design-pattern/) and responding directly to the POST request. As a first step to switching to using **yar** we need to refactor `ReviewLicenceService` into two; `ReviewLicenceService` and `SubmitReviewLicenceService`. This means it will also follow the pattern we've been using in other areas of the project. This change is the first step. We'll switch to **yar** in the next one.
Cruikshanks
added a commit
that referenced
this pull request
Apr 24, 2024
https://eaflood.atlassian.net/browse/WATER-4447 We recently added [Hapi yar session manager to the project](#926). This was specifically to support flash notification banners (ones that appear on the first GET request but then disappear). The Review licence screen has a number of these already. Ones for when you toggle the progress and others for toggling the status. These have been achieved by breaking the [post-redirect-get pattern](https://www.geeksforgeeks.org/post-redirect-get-prg-design-pattern/) and responding directly to the POST request. As a first step we [refactored `ReviewLicenceService`](#928) into two; `ReviewLicenceService` and `SubmitReviewLicenceService`. We can now complete the process. This change updates `SubmitReviewLicenceService` to use **yar** and have the associated controller redirect rather than respond.
Cruikshanks
added a commit
that referenced
this pull request
Apr 25, 2024
https://eaflood.atlassian.net/browse/WATER-4447 We recently added [Hapi yar session manager to the project](#926). This was specifically to support flash notification banners (ones that appear on the first GET request but then disappear). The Review licence screen has a number of these already. Ones for when you toggle the progress and others for toggling the status. These have been achieved by breaking the [post-redirect-get pattern](https://www.geeksforgeeks.org/post-redirect-get-prg-design-pattern/) and responding directly to the POST request. As a first step we [refactored `ReviewLicenceService`](#928) into two; `ReviewLicenceService` and `SubmitReviewLicenceService`. We can now complete the process. This change updates `SubmitReviewLicenceService` to use **yar** and have the associated controller redirect rather than respond.
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-4447
Yar adds session support to hapi
There are times we need to persist state across multiple browser requests. A common reason is when you need to take users through a journey before committing the change, for example, setting up a bill run.
We do not intend to use yar in these situations. We persist journeys using our
SessionModel
(check outapp/controllers/bill-runs-setup.controller.js
for an example). Most session managers like yar rely on creating a cookie. This will hold a session ID. Out-of-the-box they will also support you adding the rest of your session data to the cookie. But as it is common to 'blow' the cookie with too much data they will also support working with server-side storage.The legacy code uses yar in conjunction with catbox-redis to store session data in Redis (the ID in the session is the ID for the record in Redis!) That's perfect for high-volume sites and to be fair is considered 'the norm'. But it adds cost and complexity to our project that we just do not need. Hence we save 'journey' data to our PostgreSQL DB in the
sessions
table.The other need for a session manager is when you want to display a one-off notification banner to a user. For example, they've performed an action that has resulted in a change to the record.
In this situation we want to display a banner to the user confirming a change was just made. The problem is there is nothing to tell the GET request that this is so. We could do the following
We're not the first to encounter this problem. Hence why session cookies and yar exist. yar even has a feature specific to this use case.
request.yar.flash('banner', 'This licence has been marked.')
const [bannerMessage] = request.yar.flash('banner')
yar will store the message in the
wrlsSession
cookie. The call in the GET not only retrieves the value it also tells yar to delete it from the cookie. If the user hits refresh the second GET will findbannerMessage
isnull
.Unfortunately, adding yar to our service means there are now two session cookies. The legacy project being first means it gets 'dibs' on the default name 'session'. We did try to reuse the 'session' cookie. But you tell yar to use the server's cache instead of the cookie by setting its size to 0. This is how the legacy code has configured it which means any calls to
set()
orflash()
result in nothing being persisted.So, we create our own 'wrlsSession' cookie. The upside is we're not bound by the legacy cookie and when the time comes can drop it completely without additional work or adverse effects on the user.