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.
When outputting results as rendered entity in a specific view mode, the excerpt field for that entity is not available in the manage display screen.
We can provide this with an extra field, and fill the data dynamicly for that field.
Comment | File | Size | Author |
---|---|---|---|
#33 | 2983606-33--entity_excerpt_field.patch | 22.01 KB | drunken monkey |
|
Comments
Comment #2
StryKaizerComment #3
StryKaizerComment #4
StryKaizerStill todo: we need to disable the entity caching mechanism when rendering entities who have the excerpt field enabled and are being rendered in a search, to prevent showing the wrong excerpt.
Comment #6
StryKaizerComment #8
StryKaizerComment #10
borisson_The testfail is not related, but we should create a new integration test to fix this.
Comment #11
borisson_in #4 you mentioned that we still needed to disable the caching for that field, is that still the case?
Comment #12
lolandese CreditAttribution: lolandese commentedWorks as expected with the built-in Database backend (thus not using SOLR). Missing a field label though. See screenshot.
Comment #13
StryKaizerYes, we still need to address that.
Entities are being cached, and as this excerpt is a dynamic result, we need to flag the entity non-cachable when rendering it in a search result with this field enabled.
Comment #14
drunken monkeyAwesome idea, thanks!
Attached are a few small code style corrections for now. Otherwise, yes, we still need a test for this, plus apparently the caching fix (which should also be covered by the test, of course).
But great work so far!
Comment #15
drunken monkeyComment #16
sgurlt CreditAttribution: sgurlt commentedComment #17
drunken monkeyThanks!
In the future, though, please provide an interdiff when posting a new revision of an existing patch!
Regarding the changes: the
url.query_args
cache context is a good attempt to keep cacheability, but I don’t think this would be enough. Search keywords aren’t guaranteed to come via query parameters – e.g., in the Search API Pages module, they are passed as part of the path.So, until we have something like a
search_api.keys
context, I fear we have to stick with a max-age of 0.The test change seems not really related? That code tests the normal “Search: Excerpt” field, unless I’m mistaken. We still need code that tests this current patch.
Comment #18
judapriestHi,
I tested #14, with search API and Search API page.
On my "search api page", I switched to display result through view mode.
I do have the extrafield in the view mode, but the value is NULL: nothing is rendered for the extrafield. When debugging, it doesn't seem to go through the method render() in SearchApiRow class.
Is it right ?
Comment #19
iyyappan.govindHi
I just applied this patch to search_api module (version 8.x-1.12). But dynamic field is not created because of the below condition. I did following change in the patch then it shows the dynamic field.
from:
if ($entity_type instanceof ContentEntityType) {
to
if ($entity_type instanceof \Drupal\Core\Entity\ContentEntityType) {
Am I right here? Thanks
Comment #20
iyyappan.govindSorry I missed to add name space while applying the patch manually. I applied the patch manually because I got error while applying via GIT. Thanks
Comment #21
scott_euser CreditAttribution: scott_euser commentedI approached this slightly different in my own module to handle this for now in case it is useful for anyone:
Noting the following:
Comment #22
borisson_I couldn't figure out a reroll, so reapplied all individual hunks. Next up: removing test change and writing a new testcase.
Comment #23
borisson_Back to needs work.
Comment #24
borisson_I made an oopsie.
Comment #26
borisson_Forgot another
use
. Also made a change for readability already.Comment #28
borisson_Got my local test-env working again, thanks to @wannesderoy, should be green again now. Working on a test.
Comment #29
borisson_I have no clue how to write a test for this. I actually spent most of the last 2 days working on this but I don't know how to do this.
@drunken monkey please halp?
Comment #30
drunken monkeyThanks a lot for moving this along, Joris!
Oh, wow. Thanks again, then, for that!
Unfortunately, I don’t have the time right now to work on this, either (maybe later this week), but I think it would need a Functional test where you include a view mode config that uses the new
search_api_excerpt
field and some hook, processor or the test backend to add an excerpt to the results for whose presence you can then check on the search results page.If you tried that already: which part of it caused problems?
Also, something completely different: Can’t we also support this new field in the
RenderedItem
processor if called during search results rendering? See attached patch revision.Comment #31
drunken monkeyComment #32
drunken monkeyOK, took a bit but seems to work fine.
The (Views) test is still a bit bare, maybe it should be expanded to also test proper caching? (E.g., visit the page again and verify that the rendered node was retrieved from cache (not sure, how, though?), then use different query args and verify it didn’t touch the cache. (Probably need to adapt
search_api_test_excerpt_field_search_api_results_alter()
for that.))Comment #33
drunken monkeyImplemented my suggestions from #32.
Comment #34
borisson_Ok, I think this looks done now. +1 Thanks for helping out @drunken monkey :)
Comment #36
drunken monkeyAlright, then, thanks for reviewing again!
Committed.
Thanks a lot again, everyone!