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.
Problem/Motivation
In #2301239: MenuLinkNG part1 (no UI or conversions): plugins (static + MenuLinkContent) + MenuLinkManager + MenuTreeStorage it was noticed that a more specific exception might be useful.
#35 8 and #39 14
Proposed resolution
There might be two distinct kinds.
Remaining tasks
Look at the ones that are there
propose new exceptions
User interface changes
No.
API changes
Just new ones.
Postponed until
Comment | File | Size | Author |
---|---|---|---|
#23 | have_a_more_specific-2302805-23.patch | 4.88 KB | anmolgoyal74 |
#23 | interdiff_11_23.txt | 2.33 KB | anmolgoyal74 |
#11 | have_a_more_specific-2302805-11.patch | 5.11 KB | cilefen |
#11 | interdiff-10-11.txt | 3.44 KB | cilefen |
#10 | have_a_more_specific-2302805-10.patch | 3.96 KB | cilefen |
Comments
Comment #1
YesCT CreditAttribution: YesCT commentedThese look like the ones that Part 1 issue were adding:
core/lib/Drupal/Core/Menu/MenuLinkBase.php
194: throw new PluginException(String::format('Menu link plugin with ID @id does not support deletion', array('@id' => $this->getPluginId())));
core/lib/Drupal/Core/Menu/MenuLinkManager.php
280: throw new PluginException(String::format('Menu link plugin with ID @id does not support deletion', array('@id' => $id)));
348: throw new PluginException(String::format('The ID @id already exists as a plugin definition or is not valid', array('@id' => $id)));
395: throw new PluginException(String::format('Menu link %id is not resetable', array('%id' => $id)));
core/lib/Drupal/Core/Menu/MenuTreeStorage.php
398: throw new PluginException(String::format('The link with ID @id or its children exceeded the maximum depth of @depth', array('@id' => $link['id'], '@depth' => $this->maxDepth())));
1152: throw new PluginException($e->getMessage(), NULL, $e);
Comment #2
xjmComment #3
YesCT CreditAttribution: YesCT commentedComment #4
cilefen CreditAttribution: cilefen commentedComment #5
cilefen CreditAttribution: cilefen commentedComment #6
dawehnerThis does not have to be postponed any longer.
Comment #7
cilefen CreditAttribution: cilefen commentedTrying a PluginNotDeletableException. Is this the sort of thing we want?
Comment #8
dawehnerThank you for working on this issue.
I don't really like "hardcoding" the exception message.
There is no such concept as deletion of plugins outside of menus
Comment #9
cilefen CreditAttribution: cilefen commented@dawehner: Thank you for reviewing. So, it should be MenuLinkPluginNotDeletableException or something like that? Or, should we go for a MenuLinkPluginException to be more generic.
Comment #10
cilefen CreditAttribution: cilefen commentedreroll
Comment #11
cilefen CreditAttribution: cilefen commentedI think MenuLinkManager->addDefinition() could use InvalidPluginDefinitionException.
Comment #12
cilefen CreditAttribution: cilefen commentedShould these exceptions live in Drupal\Core\Menu?
Comment #19
borisson_I think this is a good idea.
String::format should be removed and this can just use string interpolation here.
Comment #23
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedRe-rolled for 9.2.x and addressed #19.
Comment #24
borisson_The interdiff looks confusing but the patch looks great. Setting to rtbc.
Comment #25
catchThis needs a change record and release notes mention.
Also beyond that, I wonder whether we can actually do this in a minor release - we might need to document that it will throw the more narrow exception in 9.x and make the change itself in 10.x