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
| Comment | File | Size | Author |
|---|
Comments
Comment #2
eric_a commentedComment #3
eric_a commentedComment #4
eric_a commentedComment #5
eric_a commentedComment #6
eric_a commentedComment #7
eric_a commentedComment #8
victoria-marina commentedComment #9
victoria-marina commentedHi! I've fixed it.
Comment #10
victoria-marina commentedFixed failing tests
Comment #11
victoria-marina commentedFixed failing tests
Comment #12
anagomes commentedThe patch in #11 applies correctly on 9.4.x-dev version and removes the
@internaltag. 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.Comment #13
victoria-marina commentedThanks for the heads up, @anagomes.
Comment #16
catchCommitted/pushed to 10.0.x and cherry-picked to 9.4.x, thanks!