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

Issue fork token-3511768

Command icon 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:

Comments

berdir created an issue. See original summary.

berdir’s picture

Status: Active » Needs review

Neat, 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.

andy_w’s picture

I 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.

slbrassard’s picture

The patch works for me on a Drupal 11.2 site.

defcon0’s picture

Works for me as well in 11.2. Thanks a lot!

berdir’s picture

For 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.

timfletcher’s picture

I'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):

Warning text

<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:
The menu structure of a Drupal 11.2.2 site after applying MR !90 and saving a node with a menu item

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

The same menu then shows a broken structure:
Broken menu UI

timfletcher’s picture

StatusFileSize
new53.59 KB
new54.44 KB

There 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:

menu ui

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:

Revision moderation history

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.

bingol@ciandt.com changed the visibility of the branch 3511768-avoid-incorrect-and to hidden.

bingol@ciandt.com’s picture

StatusFileSize
new8.06 KB

berdir changed the visibility of the branch 3511768-avoid-incorrect-and to active.

tregonia’s picture

Encountered 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.

  • berdir committed ff3af0f8 on 8.x-1.x
    [#3511768] feat: Avoid incorrect and unecessary menu_link_content entity...
berdir’s picture

Status: Needs review » Fixed

Ended 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.

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

andy_w’s picture

While 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)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

nicholass’s picture

This 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

bingol@ciandt.com changed the visibility of the branch 3511768-avoid-incorrect-and to hidden.

webflo’s picture

I 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.