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.I checked and unchecked "Rendered item" option in Processor setting. I found that in search_api_page module
$index = Index::load($this->index);
$fields_info = $index->getFields();
foreach ($index->getFulltextFields() as $field_id) {
var_dump($field_id);
$fields[$field_id] = $fields_info[$field_id]->getPrefixedLabel();
}
always get string(16) "entity:node/body" string(17) "entity:node/title" string(13) "rendered_item" .
It will cause
Fatal error: Call to a member function getPrefixedLabel() on null in search_api_page/src/Entity/SearchApiPage.php on line 165.
Estimated Value and Story Points
This issue was identified as a Beta Blocker for Drupal 8. We sat down and figured out the value proposition and amount of work (story points) for this issue.
Value and Story points are in the scale of fibonacci. Our minimum is 1, our maximum is 21. The higher, the more value or work a certain issue has.
Value : 1
Story Points: 2
| Comment | File | Size | Author |
|---|---|---|---|
| #24 | 2612104-24--check_for_property_removal.patch | 16.13 KB | drunken monkey |
| |||
Comments
Comment #2
Nick_vhComment #3
AntonnaviGot the same issue.
Fixed it with adding "rendered_item" field remove code to /src/Form/IndexProcessorsForm.php file submitForm() method, see attached patch.
Don't know is this correct way. If not correct - describe plz how better fix this issue.
Comment #6
AntonnaviFixed path to patched file.
Comment #7
AntonnaviComment #8
andypostworks for me
Comment #9
drunken monkeyNo, this is much too specialized. We need to deal with this in general: when a processor which provides new properties is removed, also remove all fields using those properties. Or, upon saving, ensure the underlying properties of all fields actually exist (probably better, since that would potentially catch more such situations – like #2724569: Improve handling of fields that no longer exist).
If we want to do this the former way, by dealing with processors, it should probably wait for #2575003: Make adding new properties/fields more a first-class operation of processors, though – which might actually solve this problem anyways. But, as said, probably it's smarter to just validate the fields' requirements/dependencies when saving the index.
Comment #10
drunken monkeyThis should do it.
Had to remove the static caching for
Index::getPropertyDefinitions(), though, since that concept was flawed – while we correctly reset the cache when adding/removing processors, changing the configuration of a processor or datasource can also already invalidate this cache. And since that now happens directl on that plugin object, it's not so easy to invalidate a static cache on the index in reaction to such a change.All in all I felt this was too much hassle for something that probably won't really ever be an issue.
I also found some old code we still had in there relating to caches we don't even use anymor, and removed that, too.
Comment #15
drunken monkeyOh god, what happened there …
Comment #16
drunken monkeyStill no idea what broke, or why, but this should at least fix the aggregated fields test.
Comment #19
drunken monkeyNot a serious patch proposal, just checking whether this is the actual issue here.
Unfortunately, I'm on my laptop this week and tests are slow and broken there, so I can't really debug this properly this week.
If someone else wants to give it a try, that would be great. Otherwise it'll most likely have to wait a week or two.
Comment #22
drunken monkeyThe problem was that at the point the index is first saved the bundles whose fields the index uses haven't been created yet, so the index can't find the properties and subsequently removes the field (as it should).
The simplest solution I could come up with is this. (Change of parent class is unrelated, was just unnecessary baggage.) However, it's not very clean, so other suggestions are very welcome.
Comment #24
drunken monkeyComment #25
borisson_So much tests for this, so great!
Comment #27
drunken monkeyThanks for reviewing!
Committed.