Problem/Motivation
See #3485030: Avoid saving menu links through node form when they do not change and #3499181: Disallow saving the current default revision as a non-default revision There is code copied from menu_ui and also additional extra savings that will break those performance optimizations and for the second issue, likely result in fatal errors.
Should extend \Drupal\Tests\menu_ui\Functional\MenuUiNodeTest and \Drupal\Tests\menu_ui\Functional\MenuUiContentModerationTest and run them with token.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 3511768-1.patch | 8.06 KB | bingol@ciandt.com |
Issue fork token-3511768
Show commands
Start 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:
- 3511768-avoid-incorrect-and
changes, plain diff MR !90
Comments
Comment #3
berdirNeat, we already have the subclass for content_moderation and that fails on 11.x with an exception, as expected. Added the other one too and refactored the code to be similar to core.
Really wish all of this wouldn't be necessary. At least it should only save it once now and we have extensive test coverage.
Comment #4
andy_w commentedI think I came to a very similar conclusion (https://www.drupal.org/project/drupal/issues/3486604), but then got side-tracked by the rabbit hole of the menu name return issue, I like the self-contained nature of this patch, looks good, I'll try and find the time to give it a test later today on some of our sites.
Comment #5
slbrassard commentedThe patch works for me on a Drupal 11.2 site.
Comment #6
defcon0 commentedWorks for me as well in 11.2. Thanks a lot!
Comment #7
berdirFor context, the blocker here is getting tests and logic to work on both old and new core versions. If it's just tests then I'm OK with skipping them on older versions, but it still needs to somewhat work on older versions.
Comment #8
timfletcher commentedI've tested MR !90 on Core 11.2.2 and it works nicely.
I was originally getting this error:
Drupal\Core\Entity\EntityStorageException: An existing default revision of the 'menu_link_content' entity type can not be changed to a non-default revision. in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 815 of /var/www/html/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).This was blocking saving draft revisions of published content with a menu position assigned. My URLs use Token to generate the path based on parent menu item URLs.
There are some side effects though - when explicitly updating the Menu item content in an unpublished revision of a live node, it breaks the menu interface. I'm including it here for posterity:
This warning appears (hard to read, at least in the Gin theme):
<Menu name> contains 1 menu link with pending revisions. Manipulation of a menu tree having links with pending revisions is not supported, but you can re-enable manipulation by getting each menu link to a published state.Here's an example menu after publishing a node:

I then published another Page, but then changed the menu item title manually and saved the node as a Draft revision:

The same menu then shows a broken structure:

Comment #9
timfletcher commentedThere seems to be another issue arising here; if the menu item title saved in the draft revision doesn't match the node title in the published revision, the menu nav stays broken when the node is published:
I changed the title of the menu item via the Menu Item UI, and it saved fine:
I then edited the Page node and the updated menu link title appeared correctly. I changed the menu item title and saved the Page as a Published revision:
This should have resolved the moderation state of the menu item and allowed the Menu UI to work again but it stayed broken until I explicitly unchecked 'Provide a menu link' and re-checked it, which resets the menu item title text to match the node's title.
Comment #11
bingol@ciandt.com commentedComment #13
damienmckennaComment #14
tregonia commentedEncountered this issue today and found that the patch does not apply cleanly. Ended up using the patch from #3540288 as saving any entity that had a menu item resulted in errors, and this needed to be hotfixed today.
Comment #16
berdirEnded up having to copy the test and add some version checks to account for the changing core behavior.
Re #9, not sure I follow, but I recommend you test with the problem you're seeing with just core. If it happens there as well, open a core issue, if not, open a new token issue. Token should not affect the behavior at all, we just have to apply some pretty ugly workarounds so that saving new entities with menu link based tokens work as expected.
Same for other possible problems. This is now passing the existing test coverage, if there are more issues then please open new issues.
Comment #18
andy_w commentedWhile I appreciate the massive time and effort put into this, and the fact that we have a resolution is really impressive. I just wanted to ask whether those initial conversations on slack and discussions around approaches on other related tickets would count towards credit on this issue. (If not I completely understand, just wanted to ask the question).
(See https://www.drupal.org/project/drupal/issues/3485030#comment-15886593)
Comment #20
nicholassThis change seems to have caused an issue with a module like add_child_page maybe one of you can review the fix at https://www.drupal.org/project/token/issues/3548518
Comment #22
webflo commentedI experienced the same issue an in #9. I think its a core bug, triggered with Content Moderation and Token module: Filled issue #3564492: Menu link content stuck in pending revision mode for it.