Deprecation BlockController::Add

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.

Pushed up some attempts. But currently have an issue where

Create 2 block types
Create a test editor and assign permissions for just 1 block type
Login and try and create a block
Button doesn't do anything.

smustgrave → credited larowlan → .

For the slack help.

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.

block_content.add_form: '/block/add/{block_content_type}'
block_content.add_page: /block/add
block_content.type_add: /admin/structure/block-content/add
block_content.type_add.bc: /admin/structure/block/block-content/types/add

block_content.add_form: '/block/add/{block_content_type}'
block_content.add_page: /block/add
block_content.type_add: /admin/structure/block-content/add
block_content.type_add.bc: /admin/structure/block/block-content/types/add

So routes end up staying the same, so we are good.

For #11 I removed the line adding the destination parameter to the "Add content block" button. Adding a block still redirect

Will pick back up tomorrow

smustgrave → credited AaronMcHale → .

Closed 📌 [PP-1] Provide a "add" link template for the block_content_type entity type Closed: duplicate as a duplicate

Think need approval on the removal of the destination parameter.

The failures I currently have no clue about. Clearly missing a route setting

This has a lot of conflicts since 🐛 The block/add path is not redirecting Active (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

📌 Fix \Drupal\Core\Entity\Controller\EntityController so bundles are sorted by label and not ID Needs work
📌 BlockContentType should implement EntityDescriptionInterface Active

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 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 📌 [PP-2] Remove route renames and aliases for block_content Active to reverse this once we can do it properly.

Changes look good to me but I don't think I can mark it.

Don't want it to stale so since the latest changes weren't me going to mark it now.

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 📌 Remove 'includes' support from hook_theme() Postponed , 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 📌 Convert template_preprocess in views Active . 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 📌 Remove 'includes' support from hook_theme() Postponed

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.