[PP-1] Use entity route providers for all content entity types

Really great work!

+++ b/core/modules/shortcut/src/Entity/ShortcutHtmlRouteProvider.php
@@ -0,0 +1,62 @@
+        ->setDefault('_title_callback', FALSE)
...
+        ->setDefault('_title_callback', FALSE)

Is there any way to properly unset a value?

That is certainly super nice work!

1. +++ b/core/modules/block_content/block_content.routing.yml
   @@ -6,26 +6,6 @@ entity.block_content_type.collection:
   -    _title_callback: 'Drupal\block_content\Controller\BlockContentController::getAddFormTitle'
   
   +++ b/core/modules/comment/comment.routing.yml
   @@ -36,24 +27,6 @@ comment.approve:
   -    _title_callback: '\Drupal\comment\Controller\CommentController::commentPermalinkTitle'
   

   Should we mark this method as deprecated as well?

2. +++ b/core/modules/block_content/src/Entity/BlockContentHtmlRouteProvider.php
   @@ -0,0 +1,70 @@
   +    if ($add_page_route = $collection->get("entity.{$entity_type_id}.add_page")) {
   +      $collection->add('block_content.add_page', $add_page_route);
   +      $collection->remove("entity.{$entity_type_id}.add_page");
   +    }
   +    if ($add_form_route = $collection->get("entity.{$entity_type_id}.add_form")) {
   +      $collection->add('block_content.add_form', $add_form_route);
   +      $collection->remove("entity.{$entity_type_id}.add_form");
   +    }
   

   I'm wondering whether we should maybe copy the route definition or so?

This definitely needs a re-roll.

I am rerolling for SprintWeekend2018

whoops

Thanks, @robpowell and @tstoeckler! This is a pretty straightforward patch, and a very nice improvement to the entity system.

1. +++ b/core/modules/block_content/src/Entity/BlockContentHtmlRouteProvider.php
   @@ -0,0 +1,70 @@
   +    // Rename the add page and add form routes for backwards compatibility.
   +    $entity_type_id = $entity_type->id();
   +    if ($add_page_route = $collection->get("entity.{$entity_type_id}.add_page")) {
   +      $collection->add('block_content.add_page', $add_page_route);
   +      $collection->remove("entity.{$entity_type_id}.add_page");
   +    }
   +    if ($add_form_route = $collection->get("entity.{$entity_type_id}.add_form")) {
   +      $collection->add('block_content.add_form', $add_form_route);
   +      $collection->remove("entity.{$entity_type_id}.add_form");
   +    }
   

   Is there a way to mark these routes as deprecated?

2. +++ b/core/modules/comment/src/Controller/CommentController.php
   @@ -153,6 +153,9 @@ public function commentPermalink(Request $request, CommentInterface $comment) {
   +   *
   +   * @deprecated in 8.2.x, will be removed in Drupal 9. Use
   +   *   \Drupal\Core\Entity\Controller\EntityController::title() instead.
       */
      public function commentPermalinkTitle(CommentInterface $comment) {
   

   Should be trigger a deprecation notice in the method itself?

3. +++ b/core/modules/menu_link_content/src/Controller/MenuController.php
   @@ -18,6 +18,10 @@ class MenuController extends ControllerBase {
   +   * @deprecated in 8.2.x, will be removed in Drupal 9. Use
   +   *   '_entity_form: menu_link_content.add' as a route default instead and
   +   *   specify the bundle as a route default additionally.
       */
      public function addLink(MenuInterface $menu) {
   

   Ditto here.

4. +++ b/core/modules/menu_link_content/src/Entity/MenuLinkContent.php
   @@ -379,4 +386,17 @@ public function setRequiresRediscovery($rediscovery) {
   +    // Account for legacy route names.
   +    /** @var \Drupal\Core\Url $url */
   +    $url = parent::toUrl($rel, $options);
   +    if ($rel === 'add-form') {
   +      $url = Url::fromRoute('entity.menu.add_link_form', $url->getRouteParameters(), $url->getOptions());
   +    }
   +    return $url;
   

   We should probably trigger a deprecation notice here.

5. +++ b/core/modules/menu_link_content/src/Entity/MenuLinkContentHtmlRouteProvider.php
   @@ -0,0 +1,66 @@
   +    if ($add_form_route = $collection->get("entity.{$entity_type_id}.add_form")) {
   +      $collection->add('entity.menu.add_link_form', $add_form_route);
   +      $collection->remove("entity.{$entity_type_id}.add_form");
   

   Why is this deviating from the generic entity.FOOBAR.add_form pattern?

6. +++ b/core/modules/shortcut/src/Controller/ShortcutController.php
   @@ -19,6 +19,9 @@ class ShortcutController extends ControllerBase {
   +   *
   +   * @deprecated in 8.2.x, will be removed in 9.x. Use
   +   *   '_entity_form: shortcut.add' in route declarations instead.
       */
      public function addForm(ShortcutSetInterface $shortcut_set) {
   

   We should fire a deprecation notice here.

7. +++ b/core/modules/shortcut/src/Entity/ShortcutHtmlRouteProvider.php
   @@ -0,0 +1,62 @@
   +/**
   + * Provides HTML routes for block contents.
   + */
   +class ShortcutHtmlRouteProvider extends DefaultHtmlRouteProvider {
   

   Inaccurate doc comment.

8. +++ b/core/modules/shortcut/src/Entity/ShortcutHtmlRouteProvider.php
   @@ -0,0 +1,62 @@
   +    if ($add_form_route = $collection->get("entity.{$entity_type_id}.add_form")) {
   +      $collection->add('shortcut.link_add', $add_form_route);
   +      $collection->remove("entity.{$entity_type_id}.add_form");
   +    }
   

   Again, why is this deviating from the entity.TYPE.add_form pattern?

9. +++ b/core/modules/taxonomy/src/Controller/TaxonomyController.php
   @@ -20,6 +20,9 @@ class TaxonomyController extends ControllerBase {
   +   *
   +   * @deprecated in 8.2.x, will be removed in 9.x. Use
   +   *   '_entity_form: taxonomy_term.add' in route declarations instead.
       */
      public function addForm(VocabularyInterface $taxonomy_vocabulary) {
   

   Should fire a deprecation notice here.

10. +++ b/core/modules/taxonomy/src/Entity/TermHtmlRouteProvider.php
    @@ -0,0 +1,37 @@
    +/**
    + * Provides HTML routes for block contents.
    + */
    +class TermHtmlRouteProvider extends AdminHtmlRouteProvider {
    

    Needs a doc comment update.

Added #2956385: Use route provider for action entities to summary

Is there policy about deprecation of route names?

According to the issue summary:

Except for aggregator_feed, node and user

node and user could benefit from using or extending the DefaultHtmlRouteProvider class, as currently they do provide their own Route Provider classes but those classes don't extend DefaultHtmlRouteProvider resulting in unnecessary code and YAML entries, this issue seems like a good place to also resolve that.

For clarity aggregator_feed does provide a Route Provider class, but it does extend DefaultHtmlRouteProvider so nothing needs done there.

Found #2723579: [PP-1] NodeRouteProvider should extend DefaultHtmlRouteProvider, last activity over there seems older than on this issue though, we can either leave that one open and fix over there, or close that one and fix over here, any preferences?

@tstoeckler yeah fair enough, it'll also mean this issue will likely land in Core quicker if the scope remains as it is.

Rerolled for 9.3.x, #28 not addressed yet.

1. +++ b/core/modules/comment/src/Controller/CommentController.php
   @@ -171,6 +171,9 @@ public function commentPermalink(Request $request, CommentInterface $comment) {
   +   * @deprecated in 8.2.x, will be removed in Drupal 9. Use
   +   *   \Drupal\Core\Entity\Controller\EntityController::title() instead.
   

   needs separate issue

2. +++ b/core/modules/shortcut/src/Controller/ShortcutController.php
   @@ -19,6 +19,9 @@ class ShortcutController extends ControllerBase {
   +   * @deprecated in 8.2.x, will be removed in 9.x. Use
   +   *   '_entity_form: shortcut.add' in route declarations instead.
   

   needs own issue

3. +++ b/core/modules/taxonomy/src/Controller/TaxonomyController.php
   @@ -20,6 +20,9 @@ class TaxonomyController extends ControllerBase {
   +   * @deprecated in 8.2.x, will be removed in 9.x. Use
   +   *   '_entity_form: taxonomy_term.add' in route declarations instead.
   

   the same

Probably it should be postponed on #3159210: Support route aliasing (Symfony 5.4) and allow deprecating the route name to provide BC for route names

Agree with #47, let's postpone until we can provide BC.

Block content module has support for deprecating routes, see \Drupal\block_content\Controller\BlockContentController::blockContentTypeRedirect

this can be active again I think