Feature - Disable Frontend... #78
Conversation
…ue and just show a blank page.
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you un-nest this. Early returns are far better for human reading of logical intent, and there is rarely a time where an if statement inside another is valid.
src/JSONAPIExtension.php
Outdated
| ['error', 515], | ||
| ], | ||
| KernelEvents::CONTROLLER => [ | ||
| ['disableFrontend', -1025] |
There was a problem hiding this comment.
This is massively high (late in terms of priority, but not the point 😛 ). Does it really need to be?
src/JSONAPIExtension.php
Outdated
| public function disableFrontend(FilterControllerEvent $event) | ||
| { | ||
| $request = $event->getRequest(); | ||
| //$response = $event->getResponse(); |
There was a problem hiding this comment.
Remove commented code
src/JSONAPIExtension.php
Outdated
|
|
||
| $container = $this->getContainer(); | ||
|
|
||
| //Get route name |
There was a problem hiding this comment.
Comment serves no purpose 😉
src/JSONAPIExtension.php
Outdated
| if ($container['jsonapi.config']->isDisableFrontend()) { | ||
| //Only disable frontend routes, don't disable json routes | ||
| if (strpos($routeName, 'jsonapi') === false) { | ||
| $event->stopPropagation(); |
There was a problem hiding this comment.
Not needed. You're setting a non-null value below
src/JSONAPIExtension.php
Outdated
| $event->setController( | ||
| function() { | ||
| return new Response('', 200); | ||
| } |
There was a problem hiding this comment.
You're better off throwing a HttpException here with a 403 or similar. This is just a nightmare to debug if for some reason it is being unexpectedly called … the Samuel L. Jackson Law of Assumption in full flight.
|
@GawainLynch - You have reminded me once again why I submit PRs and have you review them. Do you have any job openings? I would love for you to be my boss. ;) :) Seriously though, thank you @GawainLynch for the review. I will revise my code as requested, and I hope to push some changes within the next few days. |
|
No worries mate … Being you, I thought I'd go a bit harder than normal. 😺 Also, @xiaohutai is on leave, so someone has to harass people for no good reason 😇 |
|
Hi @Raistlfiren ,
Also when doing: I think you need to add a |
|
@xiaohutai - I was wanting to add the feature because we are only using the Bolt instance for the API to feed child websites. We have no need for the frontend. It would be handy to be able to disable the frontend completely, and only include the backend and jsonapi routes. I tested having no value in my config for Is the check necessary? I bet @GawainLynch has his own two cents. ;) lol |
|
It's an array, it will give a notice if it's not there:
Maybe: what do you think? |
|
I agree @xiaohutai, but I did set the default to false instead of true. Since currently the frontend is always displayed. |
3 participants
Added the ability to disable the frontend through a configuration value and just show a blank page.