If you try to create a static route for adding an entity you cannot provide the entity type as a default.
e.g.:
The following route is not working. You get an:
Drupal\Core\Entity\EntityStorageException: Missing bundle for entity type example in Drupal\Core\Entity\ContentEntityStorageBase->doCreate() (line 83 of core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php).
entity.example.add_onboarding_form:
path: '/example/onboarding'
defaults:
_entity_form: example.onboarding_add
_title: 'Create an onboarding example Entity'
example_type: 'my_bundle'
requirements:
_permission: 'edit example entities'
options:
parameters:
example_type:
type: entity:example_type
But this is working
e.g.:
entity.example.add_onboarding_form:
path: '/example/onboarding/{example_type}'
defaults:
_entity_form: example.onboarding_add
_title: 'Create an onboarding example Entity'
example_type: 'my_bundle'
requirements:
_permission: 'edit example entities'
options:
parameters:
example_type:
type: entity:example_type
Reason is in \Drupal\Core\Entity\EntityForm::getEntityFromRouteMatch
There is this line. Where we check getRawParameter. If the placeholder is not in the route definition this is empty. The processed parameter has the correct information. And if you remove this check the right example_type gets picked up in the next line.
if (($bundle_entity_type_id = $entity_type->getBundleEntityType()) && $route_match->getRawParameter($bundle_entity_type_id)) {
What is the reason for checking the Raw Parameter here? I have no idea about the intend so its hard to say if this is a bug and if and how it can be fixed without breaking other things. Maybe somebody can enlight me.
Complete method for more context:
public function getEntityFromRouteMatch(RouteMatchInterface $route_match, $entity_type_id) {
if ($route_match->getRawParameter($entity_type_id) !== NULL) {
$entity = $route_match->getParameter($entity_type_id);
}
else {
$values = [];
// If the entity has bundles, fetch it from the route match.
$entity_type = $this->entityTypeManager->getDefinition($entity_type_id);
if ($bundle_key = $entity_type->getKey('bundle')) {
if (($bundle_entity_type_id = $entity_type->getBundleEntityType()) && $route_match->getRawParameter($bundle_entity_type_id)) {
$match = $route_match->getParameter($bundle_entity_type_id);
$values[$bundle_key] = $route_match->getParameter($bundle_entity_type_id)->id();
}
elseif ($route_match->getRawParameter($bundle_key)) {
$values[$bundle_key] = $route_match->getParameter($bundle_key);
}
}
$entity = $this->entityTypeManager->getStorage($entity_type_id)->create($values);
}
return $entity;
}
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | 2921093-default-route-value-21.patch | 1.11 KB | mrshowerman |
| #19 | 2921093-19.patch | 839 bytes | anmolgoyal74 |
| #18 | 2921093-18.patch | 716 bytes | ohorbatiuk |
| #9 | 2921093-9.patch | 697 bytes | mglaman |
| #5 | drupal-2921093-5.patch | 645 bytes | rosk0 |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
marcusx commentedComment #3
marcusx commentedComment #4
tstoecklerYes, this is a legitimate bug report. I hit this before in #2723615: Use entity route providers for all content entity types. In patch #18 over there I added a workaround to
MenuLinkContentForm. However, solving this in a generic way makes more sense, so it's great you opened this issue.Can you provide a patch which simply changes the check in the if-clause from
getRawParameter()togetParameter()? Then we can see if anything breaks.Comment #5
rosk0Lets try the concept but it doesn't seem like a correct approach.
Comment #6
tstoecklerHmm... I thought this issue was about the following call:
not the one where the entity type ID is fetched?
Comment #7
rosk0I obviously done a mistake.
Also obvious that we need to write test first.
Comment #9
mglamanRan into this as well. Instead of adjusting existing code, a new
elseifis added to support default values if there are no raw parameters. Just to prevent and unknown repercussions.Comment #11
tstoecklerHmm.. raw parameters are always a subset of the parameters, so I still think changing
getRawParameter()togetParameter()makes sense.Comment #15
watergate commentedHaving the same problem and not understanding why explicitly using
getRawParameter()in the checks and then using the value from thegetParameter()call. Can't/shouldn't we simply usegetParameter()in the checks as well?Comment #18
ohorbatiuk#9 with fixed a path to modified file.
Comment #19
anmolgoyal74 commentedPatch for 9.2.x in a shorter way.
not able to generate the interdiff.
Comment #21
mrshowermanI agree with @tstoeckler.
If an entity type has been provided as a URL parameter, it will be returned by both
getRawParameter()(as a string) andgetParameter()(as an object). Thus, checking the raw parameter and returning the processed parameter works in this case.But if the parameter has just been set in the routing definition, it will only be present in
getParameter().Adding a patch which simply replaces 2 calls to
getRawParameter()withgetParameter().Comment #24
sjpeters79 commentedI ran into this issue recently while trying to create routes for node add and edit pages. For the longest time I couldn't figure out why my route was not working until I added node_type as parameter in my route path.
This patch seems like an appropriate fix. I've tested the latest patch and it works for me.
Comment #26
mxh commentedFor BC we should keep the call to
->getRawParameter(), and if that method doesn't return anything, "fallback" to ->getParameter(). Then it should be checked whether->getParameter()either returns a scalar (string or int) or already returns an entity (since this method can provide processed / converted values).That change also needs test coverage.
Comment #28
catchI'm not sure I understand #26.
The current patch looks like this:
Note the assignment doesn't change - that is calling ::getParameter(), it's only the if check that calls getRawParameter().
Tagging for re-roll - this needs conversion to an MR. We can probably base the test coverage on the issues found in #2723615: Use entity route providers for all content entity types or I wonder if that issue is close enough, we could rely on that issue for the test coverage and just go ahead here (once it's demonstrated that it fixes the problems over there).
Comment #30
mrshowermanCreated MR based on #21. Leaving as NW for the decision about test coverage.
Comment #31
mxh commentedMy comment #26 is too long ago to provide more context at this time. Guess I made that advise up to minimize potential compatibility breaks. Definitively it was not made up because of any variable assignments. It seems both methods are detached from each other on runtime, which is probably not reproducable by static code checks but will pop up once recognized on runtime.