Menu changes from node form leak into live site when creating draft revision [#3065121]

Problem/Motivation

The Token module adds a token_node_menu_link_submit() entity builder to the node form that largely duplicates logic from the menu_ui module.

The menu_ui module has recently been updated to work with draft revisions, see:
- #2858434: Menu changes from node form leak into live site when creating draft revision
- #3041326: Remove 'title' and 'description' from MenuSettingsConstraintValidator when used with content moderation by creating a draft of menu link content when a draft of it's parent content is created

The duplicated code in the Token module has not yet been updated with these changes. As a result, sites that have the Token module installed still leak changes from draft revisions.

Priority critical in line with the related core issue #2858434: Menu changes from node form leak into live site when creating draft revision

Proposed resolution

Update token_node_menu_link_submit() with the recent changes in Drupal Core.

Remaining tasks

Write a patch
Review
Commit

User interface changes

To be determined.

API changes

To be determined.

Data model changes

To be determined.

Comment	File	Size	Author
#5	3065121-5.patch	1.21 KB	idebr
#5	8.x-1.x: PHP 7.1 & MySQL 5.7, D8.7 45 pass
#5	3065121-5-test-only.patch	634 bytes	idebr
#5	8.x-1.x: PHP 7.1 & MySQL 5.7, D8.7 44 pass, 2 fail
#5	interdiff-2-5.txt	321 bytes	idebr
#2	3065121-2.patch	1.52 KB	idebr
#2	8.x-1.x: PHP 7.1 & MySQL 5.7, D8.7 55 pass PHP 7 & PostgreSQL 9.1, D8.8 55 pass PHP 7 & SQLite 3.14, D8.8 55 pass
#2	3065121-2-test-only.patch	955 bytes	idebr
#2	8.x-1.x: PHP 7.1 & MySQL 5.7, D8.7 54 pass, 2 fail

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

1 July 2019 at 10:04

idebr created an issue. See original summary.

Comment #2

idebr CreditAttribution: idebr at ezCompany commented 1 July 2019 at 10:40

Status:

Active

» Needs review

File	Size
3065121-2-test-only.patch	955 bytes
8.x-1.x: PHP 7.1 & MySQL 5.7, D8.7 54 pass, 2 fail
3065121-2.patch	1.52 KB
8.x-1.x: PHP 7.1 & MySQL 5.7, D8.7 55 pass PHP 7 & PostgreSQL 9.1, D8.8 55 pass PHP 7 & SQLite 3.14, D8.8 55 pass

Attached patch updates token_node_menu_link_submit() with the recent changes in Drupal Core.

Comment #3

1 July 2019 at 10:48

The last submitted patch, 2: 3065121-2-test-only.patch, failed testing. View results

Comment #4

Berdir

German

Switzerland

CreditAttribution: Berdir at MD Systems GmbH commented 28 July 2019 at 09:38

Status:

Needs review

» Needs work

+++ b/token.info.yml
@@ -4,3 +4,6 @@ description: Provides a user interface for the Token API and some missing core t
   - drupal:system (>= 8.5)
+
+test_dependencies:
+  - drupal:content_moderation

This is not necessary, test dependencies are only needed for contrib modules to fetch the dependencies on testbot, this always exists. It's also basically replaced by having a composer.json with require-dev dependencies, which are more flexible and work immediately including in patches.

Comment #5

idebr CreditAttribution: idebr at ezCompany commented 31 July 2019 at 12:37

Status:

Needs work

» Needs review

File	Size
interdiff-2-5.txt	321 bytes
3065121-5-test-only.patch	634 bytes
8.x-1.x: PHP 7.1 & MySQL 5.7, D8.7 44 pass, 2 fail
3065121-5.patch	1.21 KB
8.x-1.x: PHP 7.1 & MySQL 5.7, D8.7 45 pass

2 files were hidden/shown/deleted

File	Size
3065121-2-test-only.patch	955 bytes
8.x-1.x: PHP 7.1 & MySQL 5.7, D8.7 54 pass, 2 fail
3065121-2.patch	1.52 KB
8.x-1.x: PHP 7.1 & MySQL 5.7, D8.7 55 pass PHP 7 & PostgreSQL 9.1, D8.8 55 pass PHP 7 & SQLite 3.14, D8.8 55 pass

#4 Removed the test_dependencies

Comment #6

31 July 2019 at 12:47

The last submitted patch, 5: 3065121-5-test-only.patch, failed testing. View results

Comment #7

31 July 2019 at 18:23

Berdir committed 53c5208 on 8.x-1.x authored by idebr

Issue #3065121 by idebr: Menu changes from node form leak into live site...

Comment #8

Berdir

German

Switzerland

CreditAttribution: Berdir at MD Systems GmbH commented 31 July 2019 at 18:24

Status:

Needs review

» Fixed

This adds a new public $modules property, but since this is a critical bugfix, I'll commit this and we can fix that in #3040520: Make all $modules properties protected in tests