Problem/Motivation

Classes that implement \Drupal\Core\Entity\RevisionableInterface and \Drupal\Core\Entity\TranslatableInterface are not just standalone revisionable or translatable objects, they are also Entity objects since they are in the Drupal\Core\Entity namespace.

Not extending the main interface has the drawback that when a RevisionableInterface or TranslatableInterface type hint is used in a method argument, IDEs can not provide a good developer experience because they don't offer suggestions for generic methods from EntityInterface, like id() or label().

Proposed resolution

Make \Drupal\Core\Entity\RevisionableInterface and \Drupal\Core\Entity\TranslatableInterface extend the generic \Drupal\Core\Entity\EntityInterface.

Same for the storage counterparts and all the other specialized interfaces.

Remaining tasks

Review.

User interface changes

Nope.

API changes

Nope.

Data model changes

Nope.

Release notes snippet

Specialized interfaces such as EntityPublishedInterface now extend \Drupal\Core\Entity\EntityInterface.

CommentFileSizeAuthor
#6 interdiff-6.txt2.31 KBamateescu
#6 3028319-6.patch6.65 KBamateescu
#4 interdiff-4.txt1.96 KBamateescu
#4 3028319-4.patch4.34 KBamateescu
#2 3028319.patch2.38 KBamateescu

Comments

amateescu created an issue. See original summary.

amateescu’s picture

amateescu commented
Status: Active » Needs review
StatusFileSize
new 3028319.patch2.38 KB

This should do it.

Status: Needs review » Needs work

The last submitted patch, 2: 3028319.patch, failed testing. View results

amateescu’s picture

amateescu commented
Status: Needs work » Needs review
StatusFileSize
new 3028319-4.patch4.34 KB
new interdiff-4.txt1.96 KB

Fixed those unit tests.

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented

This sounds like a very reasonable idea. However I think that it would be better to be consistent and cover all the interfaces that are placed unter the namespace Drupal\Core\Entity - for example SynchronizableInterface, EntityChangedInterface ....

amateescu’s picture

amateescu commented
Title: Translatable and Revisionable entity interfaces should extend the main EntityInterface » Specialized entity interfaces should extend the main EntityInterface
Issue summary: View changes
StatusFileSize
new 3028319-6.patch6.65 KB
new interdiff-6.txt2.31 KB

Good point, let's do that :)

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented
Status: Needs review » Reviewed & tested by the community

Thank you! Looks good to me :).

catch’s picture

catch
he/him
Primary language English
commented

Alex Pott asked for feedback on the bc aspects of this.

In theory we're changing the expectations when you implement one of these interfaces.

In practice we only expect these to be implemented on top of EntityInterface, and we have the Entity base class that you're supposed to implement with that, so I think the change is OK - although it should have a change record.

effulgentsia’s picture

effulgentsia commented

+1 to #8. Seems squarely in line with "we reserve the ability to add methods to [interfaces not tagged with @api] in minor releases".

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

So let's get a change record for this.

amateescu’s picture

amateescu commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs change record

Here it is: https://www.drupal.org/node/3029284

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Committed 5d1589b and pushed to 8.7.x. Thanks!

  • alexpott committed 5d1589b on 8.7.x 
    Issue #3028319 by amateescu: Specialized entity interfaces should extend...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

4 days to refactor entity interfaces and have it committed. That must be a record. Congrats!

amateescu’s picture

amateescu commented
Related issues: +#3029405: Clean up documentation and type hints that don't need to mention both EntityInterface and another specialized interface anymore

Thanks! It's all about a catchy title :P Jokes aside, here's a doxygen cleanup followup enabled by this patch: #3029405: Clean up documentation and type hints that don't need to mention both EntityInterface and another specialized interface anymore

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I forgot to credit @hchonov - ticking the box now so at least drupal.org is correct.

Status: Fixed » Closed (fixed)

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