Follow-up to #1906810: Require type hints for automatic entity upcasting
Problem/Motivation
Commit from #1906810: Require type hints for automatic entity upcasting breaks possibility to pass function name to '_content' property of the route.
For example :
test.route:
path: 'some/path'
defaults:
_title: 'Title'
_content: 'functionName'
From here : This could be a PHP 4 style function, but best practice in Drupal 8 is to use a controller class.
It is all because of EntityResolverManager::setRouteOptions method. If u look at it you will see that $this->getController
call should return array with controller instance and method name as its values. (docblock comment)
Next. Looking at the getController method.
if (isset($defaults['_content'])) {
$controller = $this->controllerResolver->getControllerFromDefinition($defaults['_content']);
}
And here is it. getControllerFromDefinition method can return just callback function name instead of array.
This scenario is not covered in implementation and u will get en error : "Argument 1 passed to Drupal\Core\Entity\EntityResolverManager::setParametersFromReflection() must be of the type array, string given, called in /home/yarik/www/drupal8/dev/core/lib/Drupal/Core/Entity/EntityResolverManager.php on line 188"
Proposed resolution
Fix it.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | interdiff-2321393-6.txt | 8.04 KB | damiankloip |
| #6 | 2321393-6.patch | 8.93 KB | damiankloip |
| #6 | 2321393-6-tests-only-FAIL.patch | 7.36 KB | damiankloip |
Comments
Comment #1
undertext commentedMaybe something like this patch.
Comment #2
dawehnerLet's ensure that we continue to have full test coverage for this class ...
Comment #3
m1r1k commentedComment #4
jibranIt also needs test only patch.
typehint missing.
Comment #5
dawehnerSo, needs work.
Comment #6
damiankloip commentedYep, let's add this case to the test coverage too.
Comment #7
damiankloip commentedBumping as it doesn't look good if you just enter a function name and get a nice error like that.
Comment #9
dawehnerNice!!
Comment #10
alexpottCommitted 6d8b0b8 and pushed to 8.0.x. Thanks!