Problem/Motivation

Contrib and custom content entity types need a way to just declare a standard content entity form delete handler. (Or need a way to extend a convenient base form if overrides are needed.)

Such a base class exists in core, even if its name doesn't end with "Base". The short description is "Provides a generic base class for a content entity deletion form."
Unfortunately it was declared an internal base class in #2873696: Add @internal to core form classes, methods, and properties. I think this might have been an oversight. Or a case of better safe than sorry. Exposing stuff (again) is much easier than marking things internal once it's out there.
It is extremely convenient to have a class that provides the standard content entity form deletion behavior.
However, the class being internal makes this problematic for contrib.

Examples module extends ContentEntityConfirmFormBase for its non-translatable content entity type. For translatable entity types things become difficult. I have no idea what else to do than copying ContentEntityDeleteForm, but that's a maintenance nightmare. No wonder contrib is just ignoring the @internal tag. Still a maintenance issue, but it seems the worst that could happen is disappearing delete functionality. Much better than buggy delete functionality and possible data loss.

For non-translatable entity types it's doable to extend ContentEntityConfirmFormBase, but this is still more work than just declaring ContentEntityDeleteForm if you just need a standard entity form delete handler. And extending ContentEntityConfirmFormBase could be risky if the entity type would one day be converted to be translatable. ContentEntityDeleteForm handles both cases since #2486177: Deleting an entity translation from the UI deletes the whole entity.

So unless I'm missing something, it appears to be problematic for contrib and custom code that ContentEntityDeleteForm is declared @internal.

I've looked at #2491057: Streamline the entity deletion form class hierarchy. Doesn't seem it would help. And doesn't look like a blocker.

Steps to reproduce

Proposed resolution

Remove @internal from the class.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

Eric_A created an issue. See original summary.

eric_a’s picture

Issue summary: View changes
eric_a’s picture

Issue summary: View changes
eric_a’s picture

Issue summary: View changes
eric_a’s picture

Issue summary: View changes
eric_a’s picture

Issue summary: View changes
eric_a’s picture

Title: Remove @internal from ContentEntityDeleteForm » Remove @internal from ContentEntityDeleteForm generic base class
Issue summary: View changes
victoria-marina’s picture

Assigned: Unassigned » victoria-marina
victoria-marina’s picture

Assigned: victoria-marina » Unassigned
Status: Active » Needs review
StatusFileSize
new541 bytes

Hi! I've fixed it.

victoria-marina’s picture

StatusFileSize
new533 bytes

Fixed failing tests

victoria-marina’s picture

StatusFileSize
new525 bytes

Fixed failing tests

anagomes’s picture

Status: Needs review » Reviewed & tested by the community

The patch in #11 applies correctly on 9.4.x-dev version and removes the @internal tag. And just a tip @victoria-marina, usually the patch receives the number of the comment in which you are posting it as a file name, so it would be 3256539-11.patch.

victoria-marina’s picture

Thanks for the heads up, @anagomes.

  • catch committed 0107a8b on 10.0.x
    Issue #3256539 by victoria-marina, Eric_A: Remove @internal from...

  • catch committed 5ac4e51 on 9.4.x
    Issue #3256539 by victoria-marina, Eric_A: Remove @internal from...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.0.x and cherry-picked to 9.4.x, thanks!

Status: Fixed » Closed (fixed)

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