Problem/Motivation

Following #3394870: Allow controller service wiring via constructor parameter attributes we can autowire many classes in core that extend ControllerBase by deleting the create() method.

This provides a good example for contrib and custom code to follow, and should mean we can delete hundreds of lines of boilerplate code.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3396310

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:

About issue forks

Comments

longwave created an issue. See original summary.

mstrelan’s picture

mstrelan commented
Status: Postponed » Active

#3394870: Allow controller service wiring via constructor parameter attributes is in so setting to active

longwave opened merge request !5214

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review

First pass at this.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Seems to have CC failures.

Nitin shrivastava made their first commit to this issue’s fork.

Spokje made their first commit to this issue’s fork.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Very neat.

Ran the test-only feature

There was 1 failure:
1) Drupal\KernelTests\Core\DependencyInjection\AutowireTest::testCoreControllerAutowiring
The following core controllers can be autowired. Remove the create() method:
\Drupal\views_ui\Controller\ViewsUIController
\Drupal\update\Controller\UpdateController
\Drupal\tracker\Controller\TrackerController
\Drupal\system\CronController
\Drupal\system\Controller\ThemeController
\Drupal\system\FileDownloadController
\Drupal\shortcut\Controller\ShortcutSetController
\Drupal\search\Controller\SearchController
\Drupal\menu_ui\Controller\MenuController
\Drupal\help\Controller\HelpTopicPluginController
\Drupal\dblog\Controller\DbLogController
\Drupal\contact\Controller\ContactController
\Drupal\config_translation\Controller\ConfigTranslationListController
\Drupal\comment\Controller\CommentController
\Drupal\ckeditor5\Controller\CKEditor5MediaController
\Drupal\book\Controller\BookController
\Drupal\block\Controller\BlockController
\Drupal\block\Controller\BlockListController
\Drupal\announcements_feed\Controller\AnnounceController
Failed asserting that an array is empty.

So check seems good. And deletions didn't break anything. Think we can rely on the tests on this one. Controllers pretty important in the existing tests :)

  • larowlan committed 83752e6b on 11.x 
    Issue #3396310 by longwave, Spokje, smustgrave: Use autowiring for core...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Title: Use autowiring for core controllers » [Needs backport] Use autowiring for core controllers
Version: 11.x-dev » 10.2.x-dev

Committed to 11.x

Waiting on second opinion about whether this still qualifies for 10.2.x

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 10.2.x-dev » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Too late for 10.2.x now, marking as fixed in 11.x.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: [Needs backport] Use autowiring for core controllers » Use autowiring for core controllers

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.