🌵[spike] Fixed redundant member data loading for static assets #20031
Conversation
ref CFR-21 - Spike WIP - Added testing to spy that will determine when the loadMemberSession is executed on a request. - Added known routes where we don't need this middleware from executing. - The method currently used is to reorganise the pattern in which the middleware loads, ensuring we only execute `loadMemberSession` where needed, after the the paths that don't require it.
ronaldlangeveld
changed the title
| const membersService = require('../../../server/services/members'); | ||
|
|
||
| function conditionalMemberSessionMiddleware(req, res, next) { | ||
| const bypassMembersSession = [ |
There was a problem hiding this comment.
🤔 I'm not sure I'm too keen on this - I don't think it should be the responsibility of this middleware to know the specifics of defined routes (and this would likely turn into a maintenance burden)
Would be better to somehow inverse this - Let the route decide if it needs the member session?
Or could it be as simple as attempting to load the member session later in the request cycle (after the middleware processing the static assets has been executed)?
There was a problem hiding this comment.
Thanks for the feedback! I would be great to rubber duck a bit around this.
My initial idea was to pass in the member session middleware passed into routes, but after investigation, I realised there may be too many variables involved in the frontend app (it's not quite like the admin/content API with each route explicitly defined).
That lead me to do it at middleware level, since the member session is already passed in globally, it would be simpler to filter out the routes by the time they are already defined, in the middleware.
Either way, I think we have some decent testing on this PR, so we can play around and see what works best in terms of maintainability and best practices.
ronaldlangeveld
changed the title
ref CFR-21