Updated: Comment #0

Problem/Motivation

Most config entity delete forms do largely the same thing with the exception of the labels/paths etc.
If we add an admin_path annotation to config entities, really all of the meta-data required for the delete form is found in the entity annotation - eg the entity type label forms the 'Are you sure you want to delete the %entity_type_label %label' the module and entity type label can power the watchdog entry and the message displayed. The admin path can handle the redirect/cancel logic too.

This dovetails nicely with #2089757: Auto generate routing entries for entities based on an 'admin_path' and 'admin_permission' annotation but the relevant bits of that patch are included here, including the change to the route id.

Proposed resolution

Provide a ConfigDeleteFormBase that meets most common requirements

Remaining tasks

Decide if this is a worthwhile inclusion. It may well be that its not.
See if tests pass and fix if needed
Identify other config entity delete forms that might be candidates for removal
Review

User interface changes

None

API changes

New base class

#2089757: Auto generate routing entries for entities based on an 'admin_path' and 'admin_permission' annotation
#2085429: Provide a simple list controller for config entities.

CommentFileSizeAuthor
config-entity-delete.patch5 KBlarowlan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

I'm +1 on the idea.

  1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDeleteFormBase.php
    @@ -0,0 +1,60 @@
    +    return t('Delete');
    

    Here and elsewhere: $this->t()

  2. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDeleteFormBase.php
    @@ -0,0 +1,60 @@
    +      'route_name' => $info['module'] . '.' . $this->entity->entityType() . '_list',
    

    Use 'provider', not 'module'

  3. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDeleteFormBase.php
    @@ -0,0 +1,60 @@
    +      throw new \Exception("Entity types using ConfigDeleteFormBase as their delete form controller must define the 'admin_path' property.");
    

    This seems a little late to be throwing this. Maybe in buildForm?

Status: Needs review » Needs work

The last submitted patch, config-entity-delete.patch, failed testing.

larowlan’s picture

Issue tags: +Prague DX smackdown
Berdir’s picture