-
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 auth & user details to view context by default #477
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-4155 When working on [Update page header with user links](#474) we hit a blocker. The header needs to display links but only if a user is authenticated (logged in). So, we need to pass this information into the 'context' used when compiling (rendering) a view. Commonly, what you will see in controllers that return views is something like this ```javascript // NOTE: naming the object which contains the data collated by the controller `pageData` is a convention we use return h.view('info.njk', { pageTitle: 'Info', ...pageData }) ``` The first arg is the name of the view to nunjucks template to render, the second is the 'context' containing the data the template will use. We could have just done the following. ```javascript const pageData = { authenticated: request.auth.isAuthenticated, user: request.auth.credentials?.user } return h.view('info.njk', { pageTitle: 'Info', ...pageData }) ``` If we did that though we'd need to do it in _every_ controller that returns a view. A better solution is to include it in the global context which is always passed in via the engine. ```javascript // app/plugins/views.plugin.js plugin: require('@hapi/vision'), options: { engines: { // The 'engine' is the file extension this applies to; in this case, .njk njk: { compile: (src, options) => { const template = nunjucks.compile(src, options.environment) return context => template.render(context) }, // ... ``` Prior to this change the context is a static object also found in `app/plugins/views.plugin.js`. ```javascript context: { appVersion: pkg.version, assetPath: '/assets', serviceName: SERVICE_NAME, pageTitle: `${SERVICE_NAME} - GOV.UK` } ``` This change replaces that with a function that that will generate the global context including auth details found in the request each time a view is rendered. Whilst we're at it we'll update the plugin to match our conventions rather than being a straight copy of the **Vision's** example.
If the service name is a `const` in the plugin there is no reason not to include it directly in the layout. Adding it to the context means we have the option to override it but we can't foresee a time we ever would (famous last words!) Conversely, we _always_ provide a page title from our controllers when returning a view. So, there seems little point in adding something we never use.
We also move it to the top of the layout where it would more likely be in a html page.
What we had is taken directly from the example Vision provides for Nunjucks. But it depends on all the clever JavaScript stuff our conventions have us avoid. So, this moves it to its own function where we also get a but more room to do our best to describe what is going on.
Again, we just think it makes it clearer what we are actually assigning in the Vision's `options` object. Plus it gives us more space to explain what is going on. We also do a bit of tidying up. The options we were providing default to those values so are unneeded and as we clearly set `relativeTo` in Vision's options a few lines above there is no need for protective coding to generate the path.
There is no need to require it inline and this is only something we'll have to deal with should we ever manage to move to ES6 modules.
I think this is something that just got copied across. We never use app version in this context so there is no point having this extra logic.
Finally, we get to the reason we started this PR! Replace the global context object with a function that will extract auth details from the `request` object Vision helpfully passes in and build an object that includes this info.
Cruikshanks
added a commit
that referenced
this pull request
Oct 21, 2023
Thanks to [Add auth & user details to view context by default](#477) we now have access to whether a user is authenticated in the global view context. Using some Nunjucks _stuff_ (?? code, macros, filters - i dunno!) we can now use this to determine whether to generate nav links or not. Note - Ordinarily we would so this kind of logic outside of the template. Our standard is to minimise logic in the view to the bare minimum, for example, if thing show x else show y. But this is logic that is going to apply to _every_ page we display. So, it made sense to make an exception and include the logic in the layout template.
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-4155
When working on Update page header with user links we hit a blocker.
The header needs to display links but only if a user is authenticated (logged in). So, we need to pass this information into the 'context' used when compiling (rendering) a view.
Commonly, what you will see in controllers that return views is something like this
The first arg is the path to the Nunjucks template to be rendered. The second is the 'context' containing the data the template will use.
We could have just done the following.
If we did that though we'd need to do it in every controller that returns a view. A better solution is to include it in the global context which is always passed in via the engine.
Prior to this change, the context is a static object also found in
app/plugins/views.plugin.js
.This change replaces that with a function that will generate the global context including auth details found in the request each time a view is rendered. Whilst we're at it we'll update the plugin to match our conventions rather than being a straight copy of the Vision's example.