Hello!
My bug can be reproduced with the following steps:
1. Create a module with a custom route that has two parameters:
example.routing_example:
path: '/example-page/{id1}/{id2}'
defaults:
_title: 'Routing Example'
_controller: '\Drupal\<module_name>\Controller\ControllerClass::action'
requirements:
_permission: 'access content'
2. Create Custom menu links (menu_link_content) that follow the above rule
3. Enable indexing on the menu where these menu links are added
4. Generate sitemap
On generation we get a 500 error, because the route parameters do not match and id1 is empty.
The snippet of code responsible for this is in Batch.php on line 176
$route_parameters = array(key($route_parameters) => $route_parameters[key($route_parameters)]);
This forces the route parameters to contain only the last parameter in the route.
Was there a specific reason for forcing this behaviour? Because after unserializing, the $route_parameters array should be in the correct format.
I can upload a patch that resolves this, but I just wanted to make sure that I'm aware of all the implications.
Thank you!
Comment | File | Size | Author |
---|---|---|---|
#5 | multiple_parameter_routes-2700981-5.patch | 2.44 KB | rosinegrean |
| |||
#5 | interdiff-3-5.txt | 549 bytes | rosinegrean |
#3 | multiple_parameter_routes-2700981-3.patch | 2.36 KB | benelori |
Comments
Comment #2
gbyte CreditAttribution: gbyte as a volunteer and commentedHi benelori, to be honest, I'm not quite sure anymore. Please test with a variety of menu links and submit a patch; I'll be happy to double check and commit. This is another thing we would actually need tests for...
Comment #3
benelori CreditAttribution: benelori commentedI've uploaded a patch for the issue and tested with multiple parameters (1, 2 and 3) for links generated as menu_link_content entities.
Comment #4
gbyte CreditAttribution: gbyte as a volunteer and commentedThanks benelori, I'm going to incorporate your findings into some refactoring the batch class badly needs. I'm going to take care of this next week as soon as I find the time.
Comment #5
rosinegrean CreditAttribution: rosinegrean at PitechPlus commentedHey,
I saw a small issue in the fix from the patch which made the logs flood with the same notice.
Thanks,
Comment #7
gbyte CreditAttribution: gbyte as a volunteer and commentedThanks benelori, sorry it took a bit longer. I didn't use your patch, but attributed the authorship to you, as you fixed the bug. Please test this in 8.x-2.x.
Comment #8
gbyte CreditAttribution: gbyte as a volunteer and commentedComment #9
benelori CreditAttribution: benelori commentedSure no problem!
I tested against 8.0.0 and 8.1.0 and works like a charm, plus the warnings that @prics mentioned are gone as well.
Thanks!