Problem/Motivation
In #2852067: Add support for rendering computed fields to the "field" views field handler a fix was added for computed fields in the views field handler. The filter handler needs this fix as well to be able to filter views on computed fields.
I ran into this while creating a search API based view that indexes a computed entity reference field. While trying to filter on the computed entity reference field, the filter options for entity references are not very useful. It would be nice to make the entity references work like TaxonomyIndexTid. This is being fixed in #2429699: Add Views EntityReference filter to be available for all entity reference fields.
This does not automatically work for computed entity reference fields. I traced this to \Drupal\views\FieldAPIHandlerTrait::getFieldStorageDefinition(). This method should also return the computedd base fields.
Proposed resolution
Work for this started in #2852067: Add support for rendering computed fields to the "field" views field handler but since that issue originally intended to fix only the field handler, this is a follow up to make the fix more generic. I would propose the move the code in EntityField::getFieldStorageDefinition() to FieldAPIHandlerTrait::getFieldStorageDefinition().
Remaining tasks
Discuss the proposed patch.
Add tests.
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #22 | 2936737-22-10.3.x.patch | 5.26 KB | chewi3 |
| #12 | 2936737-12-8.8.x.patch | 4.31 KB | rjacobs |
| #11 | 2936737-11-8.7.x.patch | 5.04 KB | rjacobs |
| #2 | 2936737-2.patch | 4.27 KB | seanb |
Comments
Comment #2
seanbProposed solution.
Comment #3
lendudeSo we are changing the return type here, but we are just making it a little more generic since FieldStorageConfigInterface extends FieldStorageDefinitionInterface. Since the method name already promises getFieldStorageDefinition, making it return a FieldStorageDefinition sounds very reasonable to me.
But it is an API change I guess. So that would need a CR.
The fix doesn't really align well with the title and IS here. It's nice that this also fixes the described issue but the fix does more then that. I would update the title and point out that this is removing a workaround in one Views plugin and making it a generic fix.
Since this is currently a fix in field, lets move it there to start with.
Comment #4
seanbUpdated the IS and title. Todo:
Comment #5
borisson_NW based on #4.
Comment #7
twodThis also needs to support bundle fields like in #2981047: Allow adding computed bundle fields in Views.
Comment #9
rjacobs commentedRe-rolling #2 for interim reference against 8.7.x-dev. Temporarily adjusting version and status to trigger testbot.
Comment #11
rjacobs commentedOops. It looks like service support for the Entity Manager was dropped in views test classes for 8.7 onward. Here's another attempt at re-rolling #2 against 8.7.x-dev.
Comment #12
rjacobs commentedBack to 8.8.x-dev. Here's a re-roll of #2 for that as well.
Comment #13
rjacobs commentedBack to needs work based on #3-5
Comment #22
chewi3 commentedRerolled latest patch for 10.3.