Validate shortcode controllername service configurations (Case 173693)#40
Validate shortcode controllername service configurations (Case 173693)#40FabianSchmick merged 14 commits intomasterfrom
Conversation
| ?LoggerInterface $logger = null | ||
| LoggerInterface $logger = null | ||
| ) { | ||
| $callableFragments = explode('::', $controllerName); |
There was a problem hiding this comment.
I am not sure if I should also check for single : if I remember correctly, this was valid some time ago, too?
In the docs https://www.php.net/manual/en/language.oop5.paamayim-nekudotayim.php there is no : alternate
There was a problem hiding this comment.
AFAIK not in PHP, you might be thinking about Symfonys homebrew "bundle notation", which got deprecated in 4.1.
More importantly, I think we should allow invokable controllers as well. I'll add some changes shortly.
| /** | ||
| * @test | ||
| */ | ||
| public function throws_exception_for_shortcode_with_unresolvable_controller(): void | ||
| { | ||
| self::expectException(InvalidArgumentException::class); | ||
| $this->helper->resolveShortcodeController('test-config-invalid-controller'); | ||
| } |
There was a problem hiding this comment.
With this Test, the execution in an PHP8.1 and composer update --prefer-lowest environment failed, because the config for the shortcode named "test-config-invalid-controller" was loaded every time. Like in this run
In this PR this should also be captured by the new test EmbeddedShortcodeHandlerTest::throws_exception_on_invalid_controller_names()
MalteWunsch
left a comment
There was a problem hiding this comment.
Thank you! Do you want to have a look at my additions?
From my point of view: good to merge and release!
Resolves #35