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
- Write a merge request
- Review
- Commit
User interface changes
None
API changes
None
Data model changes
None
Issue fork domain-3616766
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
mably commentedWorth 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 theentity.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
enableanddisablelink templates, splitdomain.inline_actionintoentity.domain.enableandentity.domain.disableplus a separate route for thedefaultoperation, and addenableanddisablecases toDomainAccessControlHandler::checkAccess(). The routes then require_entity_access: domain.enableanddomain.disable, andDomainListBuilderonly 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 thedisablecase inSearchPageAccessControlHandlerthat protects the default search page.A route name alias cannot be used here, since aliases resolve name to name and cannot supply the
opparameter thatdomain.inline_actionexpects. The_csrf_tokenrequirement carries over untouched:RouteProcessorCsrfstamps the token onto any URL generated for such a route, sotoUrl()keeps producing tokenized links.Four rules that have to be carried over.
ConfigEntityListBuilder::getDefaultOperations()checks onlyhasKey('status')andhasLinkTemplate(), so everything else the current block knows has to be re-expressed somewhere: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:SearchPageListBuilderstill callsunset($operations['disable'], $operations['delete'])for the default page, and still hand-builds its own "Set as default" operation.administer domainsoraccess inactive domains. Note that the route currently requires onlydomain.update, so the button is stricter than the route it points at; the_csrf_tokenrequirement 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.getOperations(). It callsparent::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()andDomain::disable()save the entity themselves, unlikeConfigEntityBase, so the core idiom$entity->$op()->save()would save twice here.API changes. Dropping or renaming
domain.inline_actionchanges a public route name. Inside the project onlyDomainListBuilderreferences it, and 4.x is the right place to break it, but the summary should say so rather than "None".Test coverage.
DomainListBuilderTestasserts only the edit and delete hrefs. Enable, Disable and Make default have no coverage at all, and nothing exercisesdomain.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.)
Comment #3
mably commented