Problem/Motivation

\Drupal\Core\Config\Entity\ConfigEntityListBuilder::getDefaultOperations supplies operations to Enable / Disable config entities if the entity defines a link template. This can be applied to the domain, so some boilerplate code can be removed from DomainListBuilder

Steps to reproduce

See https://git.drupalcode.org/project/drupal/-/blob/main/core/lib/Drupal/Co...
See https://git.drupalcode.org/project/domain/-/blob/3.x/domain/src/DomainLi...

Proposed resolution

Add link-templates for domain enable / disable, apply \Drupal\Core\Config\Entity\ConfigEntityListBuilder::getDefaultOperations

Remaining tasks

  1. Write a merge request
  2. Review
  3. Commit

User interface changes

None

API changes

None

Data model changes

None

Issue fork domain-3616766

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

idebr created an issue. See original summary.

mably’s picture

Worth doing, and it can be done without losing any current behavior, though the payoff is not line count. Sketching the shape here so whoever writes the merge request does not drop the rules the hand-rolled block carries.

Where the value is. After the change the module is roughly a wash in size: about thirty lines of hand-built operations go, about twelve lines of post-processing and ten lines of routing come back. What is actually gained is $domain->toUrl('enable') becoming part of the entity's public surface, so other modules can link to those operations without hardcoding a route name, plus the route names lining up with the entity.domain.* convention the module has been converging on since #3613844: The collection link template of the domain entity type points to a route that does not exist.

Suggested shape. Declare the enable and disable link templates, split domain.inline_action into entity.domain.enable and entity.domain.disable plus a separate route for the default operation, and add enable and disable cases to DomainAccessControlHandler::checkAccess(). The routes then require _entity_access: domain.enable and domain.disable, and DomainListBuilder only has to drop what core offered when $entity->access('enable') comes back FALSE. One rule, one place, enforced on the button and on the route alike. Core's search module is the precedent for the whole shape, including the disable case in SearchPageAccessControlHandler that protects the default search page.

A route name alias cannot be used here, since aliases resolve name to name and cannot supply the op parameter that domain.inline_action expects. The _csrf_token requirement carries over untouched: RouteProcessorCsrf stamps the token onto any URL generated for such a route, so toUrl() keeps producing tokenized links.

Four rules that have to be carried over. ConfigEntityListBuilder::getDefaultOperations() checks only hasKey('status') and hasLinkTemplate(), so everything else the current block knows has to be re-expressed somewhere:

  1. The default domain gets neither operation. Core would render Disable on it, and Domain::disable() refuses on the default record and only adds a warning, so the link would be a dead end. Core hits the same thing and answers it in the list builder: SearchPageListBuilder still calls unset($operations['disable'], $operations['delete']) for the default page, and still hand-builds its own "Set as default" operation.
  2. The permission gate. Today the operations additionally require administer domains or access inactive domains. Note that the route currently requires only domain.update, so the button is stricter than the route it points at; the _csrf_token requirement makes that hard to reach by hand, but moving the rule into the access handler removes the disagreement for free. Worth deciding explicitly whether the gate keeps that permission at all, since it is documented as "Access domain URLs for domains marked as inactive" and using it for a status toggle is an overload.
  3. Operation weights. The module uses 40 for Enable and 50 for Disable, core uses -10 and 40, so Enable would move above Edit in the dropdown unless the weights are re-set. Small, but it means "User interface changes: None" is not quite accurate.
  4. The order of work inside getOperations(). It calls parent::getOperations() first and only then returns early when $entity->access('update') is forbidden. That is harmless today because the parent contributes nothing, but once the link templates exist the parent adds Enable and Disable with no access check of their own, so they would already be in the array when the early return fires, and would reach users who cannot edit the record. The access check needs to move ahead of the parent call.

One more thing worth knowing while wiring the controller: Domain::enable() and Domain::disable() save the entity themselves, unlike ConfigEntityBase, so the core idiom $entity->$op()->save() would save twice here.

API changes. Dropping or renaming domain.inline_action changes a public route name. Inside the project only DomainListBuilder references it, and 4.x is the right place to break it, but the summary should say so rather than "None".

Test coverage. DomainListBuilderTest asserts only the edit and delete hrefs. Enable, Disable and Make default have no coverage at all, and nothing exercises domain.inline_action. This change rewrites untested behavior, so the merge request should bring tests for the default-domain case and for the permission matrix.

AI-Generated: Yes (Claude Code was used to analyze the issue and draft this comment. I reviewed it before posting; there is no code on this issue yet.)

mably’s picture

Status: Active » Needs work