Problem/Motivation

This is a followup to #2388905: menu link storage override requires too much copy-paste and deals with the same problem: while the menu tree storage is pluggable a lot of logic is inside MenuTreeStorage. If one wants to override this class huge chunks need to be copy pasted. This issue deals with preSave().

Proposed resolution

Break out the parent setting logic out to a separate method. This opens the door to things like #2398983: Switch from base 36 to base 128 and use it for menu tree in contrib if core doesn't want it / schedules don't match etc.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Prioritized changes This is a trivial bugfix.
Disruption Absolutely none.
CommentFileSizeAuthor
#4 2396761_4.patch4.14 KBchx
#4 interdiff.txt1.31 KBchx
#2 2396761_2.patch4.13 KBchx
setparent.patch4.07 KBchx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, setparent.patch, failed testing.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs work » Needs review
FileSize
2396761_2.patch4.13 KB

Eh, PHP and typehinting.

Status: Needs review » Needs work

The last submitted patch, 2: 2396761_2.patch, failed testing.

chx’s picture

chx CreditAttribution: chx commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
interdiff.txt1.31 KB
2396761_4.patch4.14 KB

Some minor cleanup needed.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

+1 from me.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: 2396761_4.patch, failed testing.

Status: Needs work » Needs review

isntall queued 4: 2396761_4.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 4: 2396761_4.patch, failed testing.

Status: Needs work » Needs review

chx queued 4: 2396761_4.patch for re-testing.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Reviewed & tested by the community

Sigh. Bot fails.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update

This issue is a normal task so we need to outline how it fits within the allowable Drupal 8 beta criteria. Can someone add Drupal 8 beta phase evaluation template to the issue summary. I can see that this is zero disruption but are there other benefits in doing this?

chx’s picture

chx CreditAttribution: chx commented
Title: Move the whole parent setting logic under setParent » menu tree storage override requires too much copy-paste
Category: Task » Bug report
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs issue summary update

Committed 1777332 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed 1777332 on 8.0.x 
    Issue #2396761 by chx: menu tree storage override requires too much copy...

Status: Fixed » Closed (fixed)

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