This is a cross-post of https://www.drupal.org/node/2691929 since the issue lies in Pathauto rather than Token.
As described on that issue, re: using a Pathauto pattern with ':menu-link:parent' token/s, when updating the alias for an entity that has a menu link that is a parent, the aliases for the child menu links are not updated.
The attached patch adds a call at the end of PathautoGenerator::createEntityAlias() for insert or update operations (if alias changed) to update the aliases for any child menu link/s (if any) of the entity's menu link/s (if any). It only runs this update if any saved pathauto pattern contains ':menu-link:parent/s' tokens. Perhaps an admin setting for this might be preferred?
NB this only operates on entity insert/update, not on menu link hierarchy restructuring.
NB tested successfully with pattern '[node:menu-link:parent:url:relative]/[node:title]'. The pattern '[node:menu-link:parents:join-path]/[node:title]' seemed to replace with stale aliases, not sure of the issue.
Comment | File | Size | Author |
---|---|---|---|
#19 | interdiff-3016532-16-19-do-not-test.diff | 505 bytes | Daniel Korte |
#19 | pathauto-update_child_aliases-3016532-19.patch | 7.45 KB | Daniel Korte |
#13 | pathauto-update_child_aliases-3016532-13.patch | 7.45 KB | 0livier |
| |||
#6 | 3016532-pathauto-update_child_aliases-5.patch | 7.22 KB | ducktape |
| |||
#2 | 3016532-pathauto-update_child_aliases.patch | 6.33 KB | bgilhome |
|
Comments
Comment #2
bgilhome CreditAttribution: bgilhome commentedComment #3
bgilhome CreditAttribution: bgilhome commentedComment #4
ducktape CreditAttribution: ducktape at District09 commentedThe use case of menu hierarchy changes need to be covered as well.
I don't think the for-each will scale very well, maybe switch to a queue for processing.
Comment #5
mpp CreditAttribution: mpp at AmeXio for District09 commentedHi @bgilhome, thanks for your patch.
I consider this a major bug (or missing feature). Changing the priority.
Like @ducktape mentioned:
Note that the foreach code in #2 will cause performance issues for sites with large menu's (I've seen sites with 50k+ menu items) where a webmaster can move a large subtree in the menu. I would consider using a queue for this: fill it up with all the children when a node's parent menu is moved, then process a configurable amount on cron.
Comment #6
ducktape CreditAttribution: ducktape at District09 commentedI rerolled that patch against the latest dev. I have also added support for menu tree restructuring or menu link updates.
I have not added a queue yet, as the processing seems to be fine, even for large trees, however I did not have a 50k+ menu to my disposal to test. @mpp, can you have that tested? :)
Comment #7
Fernly CreditAttribution: Fernly at Dropsolid for District09 commentedThe patch in #6 outputs a fatal error when there is no parsable request url because the exception class in the catch statement is not recognised, namespacewise (Exception => \Exception).
Added an updated patch.
Comment #8
Fernly CreditAttribution: Fernly at Dropsolid for District09 commentedRemoved the txt extension from the patch file.
Comment #9
gremy CreditAttribution: gremy at Kalamuna commentedWe were were running into this problem with one of our clients, and decided to write a custom module to solve this issue. After running into some issues with our custom module, we found this issue and tried pathauto-update_child_aliases-3016532-8.patch instead of our module.
It works great! Thank you!
Comment #10
0livier CreditAttribution: 0livier for Michelin Travel Partner commentedPatch updated for 8.x-1.5
Comment #11
BerdirThis is quite a lot of non-trivial code, I didn't review in depth yet but some test coverage would be very helpful in getting this committed. to ensure that this doesn't get broken in the future.
Comment #12
almunningsGetting some strange behaviour with this patch with pathauto 1.8 & Drupal 8.9.6
Removing patch #10 stops this happening.
On creating a new node:
Select 'Provide a menu link' and save.
Node saves, but the link in the menu gets the url node/[uuid] rather than the alias. Menu seems to save incorrectly.
So not sure whats going on here. Race condition?
Edit. My issue appears to be unrelated to this patch. Issue could be memcached 2.2.
https://www.drupal.org/project/memcache/issues/3176451
Comment #13
0livier CreditAttribution: 0livier commentedFix child recursivity loop
Comment #14
sylvain lavielle CreditAttribution: sylvain lavielle commentedThe #13 patch worked for me but I had to do something more to make the whole thing to work in my case.
The problem that I faced was the Pathauto generation for content related to menu item was triggered to early while menu item update and before the menu tree was updated. Hence, the ":menu-link:parent" pathauto token generated an old version of the alias due to a non-updated menu tree structure.
I fixed it in our project by applying a patch to the core in the menu link manager (core/lib/Drupal/Core/Menu/MenuLinkManager.php), updating the
updateDefinition
this way :I'm aware that is certainly not the good manner. That's only a quick fix in order to make it work. But there is somehow a tricky point there that appears only in case of using
:menu-link:parent
token/s :Is pathauto have to change the way it works to handle it ?
Is the core have to change something about the way it works while updating menu tree / menu items ?
an what is the best way to do this (or that)
I thing I'm not deeply aware about those subjects to handle this further right now.
I let you - the aware guys :) - react on it and suggest the best solution
Comment #15
sylvain lavielle CreditAttribution: sylvain lavielle commentedComment #16
kevineinarsson CreditAttribution: kevineinarsson as a volunteer commentedIn #13, the logic that determines whether or not to update a child looks like this:
($child_entity->id() !== $entity->id && $child_entity->getEntityTypeId() !== $entity->getEntityTypeId())
By changing the condition to be an
||
instead makes more sense to me. That would mean entities of different types with the same ID would evaluate to true as well as entities of the same type but with different ID:s.Comment #17
kevineinarsson CreditAttribution: kevineinarsson as a volunteer commentedComment #18
fengtanA combination of #16 with #3181070-2: Possible need to change the way MenuLinkManager update menu-item and menu-tree seems to work.
However, given that the latter requires to patch core, we implemented the following workaround instead as suggested by #3068943-2: Menu navigation change does not affect node path:
The paths get updated only on cron (instead of right after updating a node or menu item), but that is acceptable for us.
Comment #19
Daniel KorteI’m seeing duplicate path aliases created when updating a node by adding it to a menu via the “Menu settings” fieldset on the node edit page. This is because there is an “insert” operation on the
MenuLinkContentInterface
entity which preventscreateEntityAlias()
from setting$existing_alias
causing a duplicate alias to be created.Changing to “Needs work” because tests are still needed and I’m not sure that hardcoding the operation to “update” here is the best solution.