Now In Drupal 8 it is possible to disable Node and Taxonomy.
But in IndexListBuilder::checkDefaultsModuleCanBeInstalled() there is a direct call NodeType::loadMultiple().
It could lead to an error:

The website encountered an unexpected error. Please try again later.
Error: Class 'Drupal\node\Entity\NodeType' not found in Drupal\search_api\IndexListBuilder::checkDefaultsModuleCanBeInstalled() (line 67 of modules/contrib/search_api/src/IndexListBuilder.php).
CommentFileSizeAuthor
#7 3022693-7--check_for_node_module_existence.patch807 bytesdrunken monkey
#5 3022693-5--check_for_node_module_existence.patch2 KBdrunken monkey
#2 search_api_no_validation_node_nodule_3022693_2.patch2.9 KBkashandarash

Comments

kashandarash created an issue. See original summary.

kashandarash’s picture

kashandarash commented
StatusFileSize
new search_api_no_validation_node_nodule_3022693_2.patch2.9 KB

I propose to add moduleExists validation

kashandarash’s picture

kashandarash commented
Assigned: kashandarash » Unassigned
Status: Active » Needs review

Status: Needs review » Needs work

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

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs work » Needs review
StatusFileSize
new 3022693-5--check_for_node_module_existence.patch2 KB

Oh, right! Thanks a lot for noticing that!
I don’t know how you created that patch, but it fails to apply no matter what I try. But the approach is off anyways: instead, we should just check if the Node module is enabled right at the start, and bail immediately if it isn’t – no further checks possible/necessary anyways.
Patch attached, please test/review!

Status: Needs review » Needs work

The last submitted patch, 5: 3022693-5--check_for_node_module_existence.patch, failed testing. View results

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs work » Needs review
StatusFileSize
new 3022693-7--check_for_node_module_existence.patch807 bytes

Oops, talk about improperly created patches …
Luckily, there is PhpStorm’s “Local history” feature! (OK, in this specific case, re-writing the patch would also not have been an insurmountable problem.)

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
Status: Needs review » Reviewed & tested by the community

The only issue I've got with this patch is that it's lacking tests, but given the size of the patch and the edge-case of this issue I think that shouldn't be a blocker. Good to go as far as I'm concerned.

  • drunken monkey committed b7d6ddd on 8.x-1.x 
    Issue #3022693 by drunken monkey, kashandarash, legolasbo: Fixed fatal...
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Reviewed & tested by the community » Fixed

Alright, thanks for reviewing!
Committed.
Thanks again, everyone!

Status: Fixed » Closed (fixed)

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