Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Investigate if this is needed for D8. If so, port it. If not, add an update on the project page.
Comment | File | Size | Author |
---|---|---|---|
#23 | interdiff_2767151-19-23.txt | 737 bytes | xavier.masson |
#23 | port-to-drupal-8-2767151-23.patch | 5.59 KB | xavier.masson |
#19 | port-to-drupal-8-2767151-19.patch | 5.42 KB | johnwebdev |
#17 | port-to-drupal-8-2767151--17.patch | 2.24 KB | rli |
#16 | port-to-drupal-8-2767151--16.patch | 2.16 KB | 8thom |
Comments
Comment #2
weseze CreditAttribution: weseze commentedIt is needed in D8. (tested in 8.2)
Comment #3
opdaviesThanks @weseze.
I'd tested it recently myself and come to the same conclusion, but forgot to post a comment. :(
Comment #4
johnwebdev CreditAttribution: johnwebdev commentedYep, I agree.
I did some quick skimming, as we don't have
hook_menu_link_update
anymore, we'll have to usehook_entity_update()
.From, there we could do something like:
Now we have the node id, and from there we can pretty much do like in Drupal 7. Is there something that SHOULD be different for the 8 version though?
MenuTreeStorage::getAllChildIds
gets us all the child ids.https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Menu%21Me...
I'd love to contribute on porting this to D8, any chance we can get it started?
Comment #5
opdaviesComment #6
opdaviesComment #8
opdaviesThanks @johndevman. This is a great start.
I've started a 8.x-1.x branch, and added the code as the second commit. :)
Comment #9
johnwebdev CreditAttribution: johnwebdev commentedHmm. I'm stuck at the moment. The problem is that the menu tree is updated after all Menu link entities have been updated. So the proposed solution at the moment (or our idea) will work. But, if the Path contains Menu tokens, it won't get the right one, as the tree storage hasn't been updated when we generate the new alias.
So if we have this menu:
And we move in Bar 2 in Bar 1. The alias won't "change", but when we move in Bar 2 into Foo 1, it will get bar1/bar2
One way to work around these issues, is that we could store the entity id(s) in some way (State API, Queue, Database) and process them asap after Menu tree has been updated.Another idea is that we hook in to the Menu UI and add an callback that resaves the Menu links entities after the menu tree has been stored. But then we are LIMITED to people updating through Menu UI, and it won't react to programmatically updates in the menu.Edit: I got an idea. We could use hook_menu_update() hook to perform the updates of node, at that point menu should be updated. I'll give it a shot and hopefully provide a initial working patch.
Comment #10
johnwebdev CreditAttribution: johnwebdev commentedHere's an initial working patch:
Comment #11
realityloopThe patch in #10 only updates the dragged menu item, not any of it's children
Comment #12
realityloopThis patch updates node paths for child menu links of the moved menu item as well
Comment #13
realityloopwas an issue with patch in #12
Comment #14
johnwebdev CreditAttribution: johnwebdev commented#13 The updating should be done in the latter hook since the parent item haven't been updated yet.
Comment #15
realityloop@johndevman thats strange, I've tested this a fair bit and it's working as I'd expect (updates all children node paths).
Comment #16
8thom CreditAttribution: 8thom as a volunteer commentedUpdating patch in #13 to include is_array check
Comment #17
rliUpdated patch in #16 to add condition check to rule out
<front>
and invalid URLs.Comment #18
johnwebdev CreditAttribution: johnwebdev commentedCool!
Looks like we're getting somewhere, to move ahead we should add at least a Kernel test to ensure it works as expected.
Covering following scenarios:
- That the moved menu link items path alias has been updated
- That the moved items children alias has been updated
-Ensure menu tokens works as expected
Edit: I'm working on the test.
Comment #19
johnwebdev CreditAttribution: johnwebdev commentedHere's an W.I.P on the Kernel tests that currently verifies:
- That the moved menu link items path alias has been updated
The key part is:
It updates the second menu link to have the first one created as its parents, and then the Menu entity gets updated to trigger the hook, as done in the Menu UI module.
Not entirely sure, if this is the right way to go? Perhaps we should make an additional Web test to use the most common scenario (rearranging through the Menu UI) - or just scrap this one, and go for only a Web test.
Comment #20
johnwebdev CreditAttribution: johnwebdev commentedEdit: Missposted in the wrong issue.
Comment #21
johnwebdev CreditAttribution: johnwebdev commentedComment #22
joelpittet#19 looks good enough to commit:) Nice job adding a test there too. The test class name should probably match the filename and the modules don't need to be all included I bet but otherwise good work flushing this out.
Comment #23
xavier.massonThe following code from pathauto_menu_link.module line 25 show warning when using path like
route:<current>
:Proposed resolution
Avoid to use
and use instead :