Problem/Motivation
Several defects and some accumulated staleness were found while updating the
module for Drupal 11.
The user-visible bug: `domain_menu_links.links.menu.yml` parents the settings
link to `domain.admins`, which does not exist — the Domain module defines
`domain.admin`. Drupal cannot resolve the parent, so `MenuTreeStorage::preSave()`
moves the link to the top level and it inherits the default `tools` menu. The
settings page is therefore not where README says it is
(Admin > Configuration > Domains).
Other issues found:
* `hook_page_attachments()` varies its output by permission but declares no
cache context, so Dynamic Page Cache can serve the toolbar library to a user
who lacks `view toolbar domain menu`.
* Domain links are built from `getRawPath()`, which returns only scheme and
hostname. Sites installed in a subdirectory, or using `domain_alias` path
prefixes, get links that drop the prefix. `getRawPath()` is also not part of
`DomainInterface`.
* The domain menu is enabled whenever any domain record exists, even when every
domain is disabled and no child links are derived.
* The derivative definitions carry a `cache` key, which is not a menu link
definition field and is discarded by `MenuTreeStorage`.
* `DomainMenuLink` loads a domain entity in its constructor, so building a menu
tree queries storage for every link even when nothing reads its cacheability.
* Both menu link plugins carry an `@Menu` annotation. No such annotation exists
in Drupal core — these plugins come from YAML discovery — and both declare the
same plugin ID.
* The hook implementations use `\Drupal::` static calls rather than injected
services.
* `parent_menu_link_weight` is declared as a string in the config schema, but it
is semantically an integer.
* The settings route is gated on `administer site configuration` rather than the
more appropriate `administer domains`.
* The test suite has a dead `addModules()` helper that would corrupt the module
list, a `getDomainsSorted()` that re-indexes before filtering (so keys develop
gaps), a cache assertion that silently skips the first domain, a helper named
with the `test` prefix, and no response status assertions.
Steps to reproduce
1. Install the module.
2. Go to Admin > Configuration > Domains.
3. The "Domain Menu Links Settings" link is not there. It appears in the tools
menu instead.
Proposed resolution
Fix each of the defects above, convert the hook implementations to OOP hooks
with dependency injection, mark the plugin and form classes final with proper
return types, retype the configuration as an integer with an accompanying update
hook, and repair and extend the test suite.
User interface changes
* The settings link now appears under Admin > Configuration > Domains, as
documented.
* The settings form requires the `administer domains` permission instead of
`administer site configuration`.
* The parent menu link weight field is now bounded to -50..50.
API changes
* `DomainMenu`, `DomainMenuLink`, `DomainMenuDeriver` and
`DomainMenuLinksSettingsForm` are now `final`.
* Hook implementations moved to `Drupal\domain_menu_links\Hook\DomainMenuLinksHooks`,
which now takes `AccountProxyInterface` and `MenuLinkManagerInterface` as
constructor arguments.
* `domainInsert()`, `domainUpdate()` and `domainDelete()` are replaced by a
single `domainChanged()`.
* `core_version_requirement` is narrowed to `^10.2 || ^11`, matching what
Domain 3.x requires.
Data model changes
`domain_menu_links.settings:parent_menu_link_weight` changes from `string` to
`integer`. `domain_menu_links_update_10201()` casts existing values.
Issue fork domain_menu_links-3615522
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
Comment #2
brunoalmeida commentedComment #4
brunoalmeida commentedComment #8
brunoalmeida commentedComment #10
brunoalmeida commentedComment #12
brunoalmeida commented