Check for expected_modules before applying updates #14
Conversation
…e as a GLOBAL_CONDITION instead of GLOBAL_ACTION
| @@ -57,6 +58,8 @@ function _update_helper_checklist_checklistapi_checklist_items() { | |||
| $version_updates = []; | |||
| foreach ($updates as $update_key => $update) { | |||
| if (is_array($update)) { | |||
| $neededModules = $updateHelper->checkExpectedModules($module_name, $update_key); | |||
There was a problem hiding this comment.
I'm not sure, but I think that we don't need this in the checklist.
The checklist sub-module should not care about the execution of an update, it's there only to listen if some update failed or not and mark that for us in the list.
| * | ||
| * Current provided global conditions are "expected_modules". | ||
| */ | ||
| const GLOBAL_CONDITIONS = '__global_conditions'; |
There was a problem hiding this comment.
To be honest, I'm thinking that an introduction of an additional global property would make CUD file more complex. Maybe we should put everything under __global_actions and then in the future, we could deprecate it in favor of __global and eventually remove it.
For now, it would be:
__global_actions:
expected_modules:
- ...
expected_configs:
- ...
install_modules:
- ...
import_configs:
- ...
What do you think about it?
| */ | ||
| public function checkExpectedModulesArray(array $expected_modules) { | ||
| $needed_modules = []; | ||
| $moduleHandler = \Drupal::service('module_handler'); |
There was a problem hiding this comment.
We should inject this service.
And maybe we can avoid loop here if we just find diff between expected modules and installed modules. Something like this:
return array_diff($expected_modules, array_keys(\Drupal::service('module_handler')->getModuleList()));
| * @return array | ||
| * Returns array needed modules. | ||
| */ | ||
| public function checkExpectedModulesArray(array $expected_modules) { |
There was a problem hiding this comment.
If we remove usage of checkExpectedModules in checklist module, we can also remove checkExpectedModules function. And then we could rename checkExpectedModulesArray into checkExpectedModules, what would make function name nicer.
Some updates refer to modules that can be uninstalled in a site, which means that the update is dependent on the existence of certain modules. If the module is not installed, skip the update (and the checklist entry). Therefore this PR will allow you to add expected_modules check to the global_actions as follows:
__global_actions:
expected_modules:
- module_name1
- module_name2