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.

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

brunoalmeida created an issue. See original summary.

brunoalmeida’s picture

Issue summary: View changes

brunoalmeida’s picture

Issue summary: View changes
Status: Active » Needs review

  • brunoalmeida committed 82d212f0 on 2.0.x
    fix: #3615522 Fix broken settings menu link and domain link URLs
    

brunoalmeida changed the visibility of the branch 3615522-fix-broken-settings-0 to hidden.

brunoalmeida’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

  • brunoalmeida committed b2cabd60 on 2.0.x
    fix: #3615522 Fix broken settings menu link and modernize hooks, plugins...
brunoalmeida’s picture

Status: Reviewed & tested by the community » Fixed

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

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

Maintainers, credit people who helped resolve this issue.

brunoalmeida’s picture

Status: Fixed » Closed (fixed)

brunoalmeida changed the visibility of the branch 3615522-fix-broken-settings-0 to active.