Problem/Motivation

\Drupal\Core\Entity\EntityDeleteForm is marked as being a member of Drupal's internal-only API, which is subject to the possibility of a higher frequency of backward-incompatible changes between releases. Curiously, \Drupal\Core\Entity\EntityDeleteFormTrait is not internal. Also, contrib seems to be making use of this internal-only API (likely by mistake).

Proposed resolution

Make EntityDeleteForm part of Drupal's public API (similar to EntityDeleteFormTrait).

User interface changes

n/a

API changes

This issue would only change the visibility of internal API to public API.

Data model changes

n/a

Issue fork drupal-3196168

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

clayfreeman created an issue. See original summary.

clayfreeman opened merge request !288

clayfreeman’s picture

clayfreeman
he/him
Primary language English
CreditAttribution: clayfreeman commented
Status: Active » Needs review
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

@internal was added on a blanket basis to all core forms in #2873696: Add @internal to core form classes, methods, and properties. However I agree there doesn't seem to be a good reason in this case, and many contrib modules are already extending it anyway: http://grep.xnddx.ru/search?text=extends+EntityDeleteForm&filename=

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented

Unsure if #2491057: Streamline the entity deletion form class hierarchy is still relevant but I suppose it should be considered before committing this. Alternatively maybe that just needs to be closed as won't fix, especially if contrib already has expectations here.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Re #5 that issue suggests removing the trait and moving the methods to EntityDeleteForm - which definitely makes this less @internal. Also the current docs for EntityDeleteForm say * Provides a generic base class for an entity deletion form. - I don't think a generic base class should be @internal.

Committed ed76f2c and pushed to 9.2.x. Thanks!

  • alexpott committed ed76f2c on 9.2.x 
    Issue #3196168 by clayfreeman, longwave: Should remove @internal from...

Status: Fixed » Closed (fixed)

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

TR’s picture

TR CreditAttribution: TR commented

Shouldn't there be a change record for this?