Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Trying to mock the EntityDisplayInterface will lack the entity methods. PhpStorm doesn't find them either.
Proposed resolution
EntityDisplayInterface should extend ConfigEntityInterface. As the implementations already do this is not a problem (as the passing patch shows).
Comment | File | Size | Author |
---|---|---|---|
#27 | 2175517_27.patch | 806 bytes | alimac |
#14 | 2175517_14.patch | 681 bytes | benjy |
#1 | 2175517_1.patch | 680 bytes | chx |
Comments
Comment #1
chx CreditAttribution: chx commentedComment #2
chx CreditAttribution: chx commentedComment #3
aspilicious CreditAttribution: aspilicious commentedDon't see anything wrong with this
Comment #4
swentel CreditAttribution: swentel commented+1 from me
Comment #5
webchickCommitted and pushed to 8.x. Thanks!
Comment #6
BerdirHm, #2031725: Move all entity display interfaces to the core component did explicitly remove this with a lot of discussions around this.
Comment #7
webchickOh, thanks. Was not aware.
Rolled back for now, pending further discussion.
Comment #8
BerdirNot sure if it was necessary, I commented over there to point the involved people to this issue
Comment #9
yched CreditAttribution: yched commentedYou don't necessarily have to be a configEntity to do the job of an EntityDisplay ("host display settings for the components of an entity")
Why do we want to mock an EntityDisplayInterface rather than an EntityDisplay or an EntityFormDisplay specifically ?
Comment #10
chx CreditAttribution: chx commentedBecause mocking classes is worse than interfaces, you need to disable the constructor, etc.
Comment #11
Berdir@yched: The thing is that we need a single interface to type hint in e.g. entity crud hooks, where we want both the ConfigEntityInterface methods and those of EntityDisplay.
The only thing that could work is having both this interface and one for the entity, that extends from this and ConfigEntityInterface, but that also seems fairly complicated.
Comment #12
Berdir1: 2175517_1.patch queued for re-testing.
Comment #14
benjy CreditAttribution: benjy commentedNew patch.
Comment #15
BerdirHere's why I think this makes sense:
Code from EmailFieldTest, but any other snippet is just the same:
PhpStorm analysis: "Method 'save' not found in class \Drupal\Core\Entity\Display\EntityDisplayInterface".
Comment #17
benjy CreditAttribution: benjy commented14: 2175517_14.patch queued for re-testing.
Comment #19
benjy CreditAttribution: benjy commented14: 2175517_14.patch queued for re-testing.
Comment #20
benjy CreditAttribution: benjy commentedBack to RTBC, it was just the test bot playing up.
Comment #22
benjy CreditAttribution: benjy commented14: 2175517_14.patch queued for re-testing.
Comment #23
BerdirAw testbot...
Comment #24
fagohm, that was changed by purpose to be decoupled, but #15 and the entity_get_display() documentation provide good reasons to re-introduce the coupling - thus agreed.
Comment #25
catchCommitted/pushed to 8.x, thanks!
Comment #27
alimac CreditAttribution: alimac commentedPatch in #14 was never committed. Here is the patch for the latest HEAD.
Comment #28
alimac CreditAttribution: alimac commentedComment #29
BerdirWeird, but +1 to the latest patch.
Comment #30
webchickCommitted and pushed to 8.x. For real this time. :)