Skip to content

Commit

Permalink
Check backup consistency before importing workflow (#196)
Browse files Browse the repository at this point in the history 
* Check backup consistency before importing workflow

* Fix PHPDocs

* Use existing category in backup_and_restore test

* lifecycletrigger_categories: A bit of general refactoring

* lifecycletrigger_categories: Don't fail if category doesn't exist

* Add possibility to force import of errorneous backup

* Use $force = false as default in restore_workflow:execute

* Display only unique errors when Workflows causes multiple errors

---------

Co-authored-by: Justus Dieckmann <[email protected]>
Co-authored-by: NinaHerrmann <[email protected]>
  • Loading branch information
@justusdieckmann @NinaHerrmann
3 people authored Apr 26, 2024
1 parent 33cbd05 commit 920d1f9
Show file tree
Hide file tree
Showing 11 changed files with 127 additions and 18 deletions.
50 changes: 47 additions & 3 deletions classes/local/backup/restore_lifecycle_workflow.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use tool_lifecycle\local\entity\step_subplugin;
use tool_lifecycle\local\entity\trigger_subplugin;
use tool_lifecycle\local\entity\workflow;
use tool_lifecycle\local\manager\lib_manager;
use tool_lifecycle\local\manager\workflow_manager;
use tool_lifecycle\local\manager\step_manager;
use tool_lifecycle\local\manager\trigger_manager;
Expand Down Expand Up @@ -65,20 +66,29 @@ public function __construct($xmldata) {
* Executes the restore process. It loads the workflow with all steps and triggers from the xml data.
* If all data is valid, it restores the workflow with all subplugins and settings.
* Otherwise an array with error strings is returned.
* @param bool $force force import, even if there are errors.
* @return string[] Errors, which occurred during the restore process.
* @throws \coding_exception
* @throws \moodle_exception
*/
public function execute() {
public function execute(bool $force = false) {
$this->reader->read();

$this->load_workflow();
// If the workflow could be loaded continue with the subplugins.
if ($this->workflow) {
$this->load_subplugins();

if (!$this->all_subplugins_installed()) {
return $this->errors;
}

// Validate the subplugin data.
if (empty($this->errors) && $this->all_subplugins_installed()) {
$this->check_subplugin_validity();
if (empty($this->errors) || $force) {
// If all loaded data is valid, the new workflow and the steps can be stored in the database.
// If we force the import, we empty the errors;
$this->errors = [];
$this->persist();
}
}
Expand All @@ -101,7 +111,6 @@ private function load_workflow() {
$this->workflow->timeactive = null;
$this->workflow->timedeactive = null;
$this->workflow->sortindex = null;
workflow_manager::insert_or_update($this->workflow);
}

/**
Expand Down Expand Up @@ -174,6 +183,41 @@ private function all_subplugins_installed() {
return true;
}

/**
* Calls the subplugins to check the consistency and validity of the step and trigger settings.
*/
private function check_subplugin_validity() {
foreach ($this->steps as $step) {
$steplib = lib_manager::get_step_lib($step->subpluginname);
$filteredsettings = [];
foreach ($this->settings as $setting) {
if ($setting->pluginid === $step->id) {
$filteredsettings[$setting->name] = $setting->value;
}
}
$errors = array_map(
fn($x) => get_string('restore_error_in_step', 'tool_lifecycle', $step->instancename) . $x,
$steplib->ensure_validity($filteredsettings)
);
$this->errors = array_merge($this->errors, $errors);
}

foreach ($this->trigger as $trigger) {
$steplib = lib_manager::get_trigger_lib($trigger->subpluginname);
$filteredsettings = [];
foreach ($this->settings as $setting) {
if ($setting->pluginid === $trigger->id) {
$filteredsettings[$setting->name] = $setting->value;
}
}
$errors = array_map(
fn($x) => get_string('restore_error_in_trigger', 'tool_lifecycle', $trigger->instancename) . $x,
$steplib->ensure_validity($filteredsettings)
);
$this->errors = array_merge($this->errors, $errors);
}
}

/**
* Stores all loaded data in the database.
* @throws \moodle_exception
Expand Down
6 changes: 6 additions & 0 deletions classes/local/form/form_upload_workflow.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ public function definition() {

$mform->addElement('filepicker', 'backupfile', get_string('file'), null,
['accepted_types' => 'xml']);

$showforce = isset($this->_customdata['showforce']) && $this->_customdata['showforce'];
$mform->addElement($showforce ? 'checkbox' : 'hidden', 'force', get_string('force_import', 'tool_lifecycle'));
$mform->setDefault('force', 0);
$mform->setType('force', PARAM_BOOL);

$this->add_action_buttons('true', get_string('upload'));
}

Expand Down
4 changes: 4 additions & 0 deletions lang/en/tool_lifecycle.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@
$string['restore_subplugins_invalid'] = 'Wrong format of the backup file. The structure of the subplugin elements is not as expected.';
$string['restore_step_does_not_exist'] = 'The step {$a} is not installed, but is included in the backup file. Please installed it first and try again.';
$string['restore_trigger_does_not_exist'] = 'The trigger {$a} is not installed, but is included in the backup file. Please installed it first and try again.';
$string['restore_error_in_step'] = 'An error occurred when importing step "{$a}": ';
$string['restore_error_in_trigger'] = 'An error occurred when importing trigger "{$a}": ';
$string['workflow_was_not_imported'] = 'The workflow was not imported!';
$string['force_import'] = 'Try ignoring errors and import the workflow anyway. <b>Use this at your own risk!</b>';

// Events.
$string['process_triggered_event'] = 'A process has been triggered';
Expand Down
8 changes: 2 additions & 6 deletions renderer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,11 @@ public function header($title = null) {
/**
* Renders the workflow upload form including errors, which occured during upload.
* @param \tool_lifecycle\local\form\form_upload_workflow $form
* @param array $errors
* @throws coding_exception
*/
public function render_workflow_upload_form($form, $errors = []) {
public function render_workflow_upload_form($form) {
$this->header(get_string('adminsettings_edit_workflow_definition_heading', 'tool_lifecycle'));
foreach ($errors as $error) {
\core\notification::add($error, \core\notification::ERROR);
}
echo $form->render();
$form->display();
$this->footer();
}

Expand Down
10 changes: 10 additions & 0 deletions step/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,16 @@ public function extend_add_instance_form_definition_after_data($mform, $settings
public function abort_course($process) {
}


/**
* Ensure validity of settings upon backup restoration.
* @param array $settings
* @return array List of errors with settings. If empty, the given settings are valid.
*/
public function ensure_validity(array $settings) : array {
return [];
}

}

/**
Expand Down
9 changes: 9 additions & 0 deletions tests/backup_and_restore_workflow_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
require_once(__DIR__ . '/generator/lib.php');
require_once(__DIR__ . '/../lib.php');

use mod_bigbluebuttonbn\settings;
use tool_lifecycle\local\backup\backup_lifecycle_workflow;
use tool_lifecycle\local\backup\restore_lifecycle_workflow;
use tool_lifecycle\local\manager\workflow_manager;
Expand Down Expand Up @@ -60,6 +61,14 @@ public function setUp() : void {
$this->resetAfterTest(true);
$generator = $this->getDataGenerator()->get_plugin_generator('tool_lifecycle');
$this->workflow = $generator->create_workflow(['startdatedelay', 'categories'], ['email', 'createbackup', 'deletecourse']);
$category = $this->getDataGenerator()->create_category();
foreach (trigger_manager::get_triggers_for_workflow($this->workflow->id) as $trigger) {
if ($trigger->subpluginname === 'categories') {
settings_manager::save_setting($trigger->id, settings_type::TRIGGER, 'categories',
'categories', $category->id);
}
}

foreach (workflow_manager::get_workflows() as $existingworkflow) {
$this->existingworkflows[] = $existingworkflow->id;
}
Expand Down
1 change: 1 addition & 0 deletions trigger/categories/lang/de/lifecycletrigger_categories.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@
$string['categories'] = 'Kategorien, für die der Workflow ausgelöst werden soll.';
$string['categories_noselection'] = 'Bitte wählen sie mindestens eine Kategorie aus.';
$string['exclude'] = 'Falls ausgewählt, werden gerade die Kurse der angegebenen Kategorien nicht ausgelöst.';
$string['categories_do_not_exist'] = 'Es gibt keine Kurskategorien mit den folgenden IDs: {$a}.';
1 change: 1 addition & 0 deletions trigger/categories/lang/en/lifecycletrigger_categories.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@
$string['categories'] = 'Categories, for which the workflow should be triggered';
$string['categories_noselection'] = 'Please choose at least one category.';
$string['exclude'] = 'If ticked, the named categories are excluded from triggering instead.';
$string['categories_do_not_exist'] = 'There are no categories with the following ids: {$a}.';
35 changes: 28 additions & 7 deletions trigger/categories/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public function check_course($course, $triggerid) {
public function get_course_recordset_where($triggerid) {
global $DB, $CFG;
$categories = settings_manager::get_settings($triggerid, settings_type::TRIGGER)['categories'];
$exclude = settings_manager::get_settings($triggerid, settings_type::TRIGGER)['exclude'] && true;
$exclude = settings_manager::get_settings($triggerid, settings_type::TRIGGER)['exclude'];

$categories = explode(',', $categories);
// Use core_course_category for moodle 3.6 and higher.
Expand All @@ -75,17 +75,17 @@ public function get_course_recordset_where($triggerid) {
}
$allcategories = [];
foreach ($categories as $category) {
array_push($allcategories , $category);
array_push($allcategories, $category);
if (!isset($categoryobjects[$category]) || !$categoryobjects[$category]) {
continue;
}
$children = $categoryobjects[$category]->get_all_children_ids();
$allcategories = array_merge($allcategories , $children);
$allcategories = array_merge($allcategories, $children);
}

list($insql, $inparams) = $DB->get_in_or_equal($allcategories, SQL_PARAMS_NAMED);
list($insql, $inparams) = $DB->get_in_or_equal($allcategories, SQL_PARAMS_NAMED, 'param', !$exclude);

$where = "{course}.category {$insql}";
if ($exclude) {
$where = "NOT " . $where;
}

return [$where, $inparams];
}
Expand Down Expand Up @@ -131,4 +131,25 @@ public function extend_add_instance_form_definition($mform) {
$mform->setType('exclude', PARAM_BOOL);
}

/**
* Ensure validity of settings upon backup restoration.
* @param array $settings
* @return array List of errors with settings. If empty, the given settings are valid.
*/
public function ensure_validity(array $settings): array {
$missingcategories = [];
$categories = explode(',', $settings['categories']);
$categoryobjects = \core_course_category::get_many($categories);
foreach ($categories as $category) {
if (!isset($categoryobjects[$category]) || !$categoryobjects[$category]) {
$missingcategories[] = $category;
}
}
if ($missingcategories) {
return [get_string('categories_do_not_exist', 'lifecycletrigger_categories', join(', ', $missingcategories))];
} else {
return [];
}
}

}
9 changes: 9 additions & 0 deletions trigger/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,15 @@ public function get_status_message() {
return get_string("workflow_started", "tool_lifecycle");
}

/**
* Ensure validity of settings upon backup restoration.
* @param array $settings
* @return array List of errors with settings. If empty, the given settings are valid.
*/
public function ensure_validity(array $settings) : array {
return [];
}

}

/**
Expand Down
12 changes: 10 additions & 2 deletions uploadworkflow.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/

use core\notification;
use tool_lifecycle\local\backup\restore_lifecycle_workflow;
use tool_lifecycle\local\form\form_upload_workflow;
use tool_lifecycle\permission_and_navigation;
Expand Down Expand Up @@ -51,11 +52,18 @@
if ($data = $form->get_data()) {
$xmldata = $form->get_file_content('backupfile');
$restore = new restore_lifecycle_workflow($xmldata);
$errors = $restore->execute();
$force = $data->force ?? false;
$errors = $restore->execute($force);
if (count($errors) != 0) {
notification::add(get_string('workflow_was_not_imported', 'tool_lifecycle'), notification::ERROR);
foreach (array_unique($errors) as $error) {
notification::add($error, notification::ERROR);
}
$form = new form_upload_workflow(null, ['showforce' => true]);

/** @var \tool_lifecycle_renderer $renderer */
$renderer = $PAGE->get_renderer('tool_lifecycle');
$renderer->render_workflow_upload_form($form, $errors);
$renderer->render_workflow_upload_form($form);
die();
} else {
// Redirect to workflow page.
Expand Down

0 comments on commit 920d1f9

Please sign in to comment.