Skip to content
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

Move rtn req. routes and session handling #597

Merged
merged 8 commits into from
Dec 19, 2023
Merged

Conversation

robertparkinson
Copy link
Collaborator

Having discussed and agreed a way forward for handling temporary sessions in our code base this demonstrates how we can create the initial session, then retrieve it in subsequent requests.

The other thing it does is break the dependence on the return requirement journey sitting under /licences. /licences is still the gateway to the journey, creating the temporary session for tracking the journey. But it then redirects away so /return-requirements can take over.

The benefit of this is without any additional controls, we have a way of 'clearing' the session. Essentially, the user simply has to leave the journey and start again and the session is cleared. But whilst in the journey the user can go forwards and backwards throughout and each page can validly assume sessionId refers to an existing record.

Having discussed and agreed a way forward for handling temporary sessions in our code base this demonstrates how we can create the initial session, then retrieve it in subsequent requests.

The other thing it does is break the dependence on the return requirement journey sitting under `/licences`. `/licences` is still the gateway to the journey, creating the temporary session for tracking the journey. But it then redirects away so `/return-requirements` can take over.

The benefit of this is without any additional controls, we have a way of 'clearing' the session. Essentially, the user simply has to leave the journey and start again and the session is cleared. But whilst in the journey the user can go forwards and backwards throughout and each page can validly assume `sessionId` refers to an existing record.
@robertparkinson robertparkinson changed the title Move rtn req. routes and session handling example Move rtn req. routes and session handling Dec 15, 2023
@robertparkinson robertparkinson marked this pull request as ready for review December 19, 2023 12:18
Copy link
Contributor

@Demwunz Demwunz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@Demwunz Demwunz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

})
}

async function saveReturnsHowDoYouWant (request, h) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sonarcloud is complaining about this function being the same as saveNote

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put in a small change to make it pass.

@Demwunz Demwunz self-requested a review December 19, 2023 13:07
Cruikshanks
Cruikshanks previously approved these changes Dec 19, 2023
Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not worried about SonarCloud at the moment as a lot of this code will change once we start adding in validation. So...

Demwunz
Demwunz previously approved these changes Dec 19, 2023
@robertparkinson robertparkinson dismissed stale reviews from Demwunz and Cruikshanks via 4a036e4 December 19, 2023 13:47
Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I spotted on the first pass I should have flagged but forgot. 😬

@@ -47,6 +47,7 @@ const AuthPlugin = {
},
redirectTo: '/signin',
validate: async (_request, session) => {
console.log('🚀 ~ file: auth.plugin.js:50 ~ validate: ~ session:', session)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh! Something I should have spotted the first time around.

Suggested change
console.log('🚀 ~ file: auth.plugin.js:50 ~ validate: ~ session:', session)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird that it's showing up on my PR.. it's not something I added. but i've removed it now

Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robertparkinson robertparkinson merged commit 16f7b70 into main Dec 19, 2023
6 checks passed
@robertparkinson robertparkinson deleted the lick-it-and-stick-it branch December 19, 2023 14:26
Demwunz pushed a commit that referenced this pull request Dec 20, 2023
Having discussed and agreed a way forward for handling temporary sessions in our code base this demonstrates how we can create the initial session, then retrieve it in subsequent requests.

The other thing it does is break the dependence on the return requirement journey sitting under `/licences`. `/licences` is still the gateway to the journey, creating the temporary session for tracking the journey. But it then redirects away so `/return-requirements` can take over.

The benefit of this is without any additional controls, we have a way of 'clearing' the session. Essentially, the user simply has to leave the journey and start again and the session is cleared. But whilst in the journey the user can go forwards and backwards throughout and each page can validly assume `sessionId` refers to an existing record.

---------

Co-authored-by: Alan Cruikshanks <[email protected]>
Demwunz pushed a commit that referenced this pull request Dec 20, 2023
Having discussed and agreed a way forward for handling temporary sessions in our code base this demonstrates how we can create the initial session, then retrieve it in subsequent requests.

The other thing it does is break the dependence on the return requirement journey sitting under `/licences`. `/licences` is still the gateway to the journey, creating the temporary session for tracking the journey. But it then redirects away so `/return-requirements` can take over.

The benefit of this is without any additional controls, we have a way of 'clearing' the session. Essentially, the user simply has to leave the journey and start again and the session is cleared. But whilst in the journey the user can go forwards and backwards throughout and each page can validly assume `sessionId` refers to an existing record.

---------

Co-authored-by: Alan Cruikshanks <[email protected]>
Demwunz pushed a commit that referenced this pull request Dec 20, 2023
Having discussed and agreed a way forward for handling temporary sessions in our code base this demonstrates how we can create the initial session, then retrieve it in subsequent requests.

The other thing it does is break the dependence on the return requirement journey sitting under `/licences`. `/licences` is still the gateway to the journey, creating the temporary session for tracking the journey. But it then redirects away so `/return-requirements` can take over.

The benefit of this is without any additional controls, we have a way of 'clearing' the session. Essentially, the user simply has to leave the journey and start again and the session is cleared. But whilst in the journey the user can go forwards and backwards throughout and each page can validly assume `sessionId` refers to an existing record.

---------

Co-authored-by: Alan Cruikshanks <[email protected]>
Demwunz pushed a commit that referenced this pull request Dec 20, 2023
Having discussed and agreed a way forward for handling temporary sessions in our code base this demonstrates how we can create the initial session, then retrieve it in subsequent requests.

The other thing it does is break the dependence on the return requirement journey sitting under `/licences`. `/licences` is still the gateway to the journey, creating the temporary session for tracking the journey. But it then redirects away so `/return-requirements` can take over.

The benefit of this is without any additional controls, we have a way of 'clearing' the session. Essentially, the user simply has to leave the journey and start again and the session is cleared. But whilst in the journey the user can go forwards and backwards throughout and each page can validly assume `sessionId` refers to an existing record.

---------

Co-authored-by: Alan Cruikshanks <[email protected]>
@Cruikshanks Cruikshanks added the housekeeping Refactoring, tidying up or other work which supports the project label Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Refactoring, tidying up or other work which supports the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants