Early Bird Registration for DrupalCon Atlanta is now open! By registering during our Early Bird Registration window, you’ll save $100. This window ends on 19 January 2025 and will go by quickly, so don’t wait!On review of the module code, I spotted that $updated_config in SavedSearchType::onDependencyRemoval() is never read, pointing to a definitive bug. (We don’t save the updated configuration for notification plugins that removed a dependency.)
Also, as we copied the code from Search API indexes, which have multiple types of plugins, we use $plugin_type in several places where it is unnecessary. (It is still necessary in the basic dependency data array, to distinguish between dependencies of plugins and dependencies of the type entity itself.)
Anyways, in addition to fixing that code, this definitely needs code coverage, too.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 3262824-5--fix_dependency_removal_handling.patch | 10.43 KB | drunken monkey |
| #5 | 3262824-5--fix_dependency_removal_handling--tests_only.patch | 7.21 KB | drunken monkey |
Comments
Comment #2
drunken monkeyComment #3
drunken monkeyNW for the tests.
Comment #4
drunken monkeyComment #5
drunken monkeyTurns out this wasn’t broken to begin with, it was just unnecessary to store
$updated_config– updating the configuration within the plugin suffices, as that gets written to$type->notification_settingsupon save anyways.So, this just cleans up the code a bit and adds tests.
Comment #7
drunken monkeyMerged.