Replace BlockContentController::add with EntityController::addPage via the AdminHtmlRouteProvider route provider

I'm not sure if the function \Drupal\Core\Entity\Controller\EntityController::addPage() fits for that because this function has an argument $entity_type_id that should be passed from the route.

Or, I just missed something important here.

There is no need to deprecate - core controllers aren't part of the api unless marked as such.

I think we'll need to make sure we keep the same route name for BC sake so if we move to a route provider, overriding the default name of entity.block_content.add_page to be the same as now will be required.

This has a lot of conflicts since #3461316: Redirect block/add if there is only one block type (we probably should've just done this issue instead since EntityController::addPage has the same redirect logic XD)

It might be easiest to start from scratch here, we also need to ensure we remove the route definition in block_content.routing.yml and rename the route as per #12

I think what you were seeing in #14 was because this wasn't done.

We can also remove the following routes from block_content.routing.yml:

- entity.block_content.canonical
- entity.block_content.edit_form
- entity.block_content.delete_form

Those match what DefaultHtmlRouteProvider provide

Then we'll just have to alias the block_content.add_page and block_content.add_form routes

Pushed a WIP of what I've got so far, things are a bit more complicated because of 2 things:

1. BlockContentController::addForm has special handling for query parameters that allow a theme to be set automatically when going from the block library to add a block content entity. These query params come from template_preprocess_block_content_add_list currently which is not compatible with the generic entity_add_list that EntityController uses.

There's no easy way to override this so I've gone for a preprocess instead, we'll have to probably deprecate the block_content_add_list theme hook although I'd be worried if anyone was using this outside of core.

2. As per above all of the entity.block_content.* routes are safe to remove because they match what the route provider is generating. The 2 aliases are going to be problematic for BC even if we don't deprecate them because people might be checking the string based route name, etc. I think the easiest for this issue may be to override ::getRoutes to swap the route names.

Based on this search it looks like block_content_permissions and xp modules are the only contrib modules using this theme hook.
https://git.drupalcode.org/search?group_id=2&scope=blobs&search=block_co...

block_content_permissions is obselete according to its project page and xp is marked unsupported

Postponed now on

#3196798: Fix EntityController::addPage so bundles are sorted by label and not ID
#3530654: BlockContentType should implement EntityDescriptionInterface

This should be green now, we just need to action the route rename.

Updated the route names, also added test coverage for the query param redirection stuff in EntityController::addPage

The route renaming is causing some issues with JSON:API Rest tests and Link relations.

So here's something kinda sneaky, we can just alias the routes the other way to make everything work again.

I've opened #3534380: [PP-2] Remove route renames and aliases for block_content to reverse this once we can do it properly.

template_preprocess_block_content_add_list() is being removed now, but the template is just deprecated. It won't be working without that preprocess, so we need to keep it. The same was done with template_preprocess_authorize_report for example.

To prepare for #1177762: Deprecate automatic template_preprocess discovery and 'includes'/'file' support from hook_theme(), we might still want to move it to .module. Also, in #3504381-25: [meta] Convert Template Preprocess hooks to OOP equivalent, we discussed that for .inc files, we should keep them with a deprecation message, see for example #3535680: Convert template_preprocess in views. That's not really done consistently, and it might not block getting it committed.

Forget about the moving part, this does enough. I'll take care of that in #1177762: Deprecate automatic template_preprocess discovery and 'includes'/'file' support from hook_theme()

Good point, added it back with a deprecation

Added a review.

Addressed all feedback, thanks for catching the template issue.

BC is complicated, but I think this looks fine now. Seems a bit overkill to go to all that trouble for a template that almost certainly nobody uses, but makes sense to me that we either do it properly or not at all. If we can remove routes and controllers, then possibly we can just directly remove such a template, but that's a decision for whoever is going to commit this.

Back to RTBC.

Had one question on the MR about the parameter change on EntityController - but also there are merge conflicts in the MR.

Had a tricky test fail on workspaces update tests, The module install it does was running before the router table was updated and then that failed. Not exactly sure why this is triggered now.

Committed/pushed to 11.x, thanks!