Merge pull request #134 from justusdieckmann/feature/edit-active-workflows

Add the possibility of editing steps and triggers after workflow activation

adminlib.php

@@ -636,17 +636,18 @@ class workflow_settings {
                 \core\notification::add(
                     get_string('active_workflow_not_changeable', 'tool_lifecycle'),
                     \core\notification::WARNING);
-            } else {
+            }
             if (!empty($data->id)) {
                 $step = step_manager::get_step_instance($data->id);
+                if (isset($data->instancename)) {
                     $step->instancename = $data->instancename;
+                }
             } else {
                 $step = step_subplugin::from_record($data);
             }
             step_manager::insert_or_update($step);
             // Save local subplugin settings.
-                settings_manager::save_settings($step->id, settings_type::STEP, $form->subpluginname, $data);
-            }
+            settings_manager::save_settings($step->id, settings_type::STEP, $form->subpluginname, $data, true);
         } else {
             $this->view_step_instance_form($form);
             return true;

classes/local/form/form_step_instance.php

@@ -129,13 +129,8 @@ class form_step_instance extends \moodleform {
             $this->lib->extend_add_instance_form_definition($mform);
         }
 
-        // For active workflows, we do not want the form to be editable.
-        if ($this->workflowid && !workflow_manager::is_editable($this->workflowid)) {
-            $this->add_cancel_button();
-        } else {
         $this->add_action_buttons();
     }
-    }
 
     /**
      * In case of read only mode only the cancel button is rendered.
@@ -183,7 +178,13 @@ class form_step_instance extends \moodleform {
         // For active workflows, we do not want the form to be editable.
         if ($this->workflowid && !workflow_manager::is_editable($this->workflowid)) {
             // The group buttonar is the array of submit buttons. For inactive workflows this is only a cancel button.
-            $mform->hardFreezeAllVisibleExcept(array('buttonar'));
+            $notfreeze = ['buttonar'];
+            foreach ($this->lib->instance_settings() as $setting) {
+                if ($setting->editable) {
+                    $notfreeze[] = $setting->name;
+                }
+            }
+            $mform->hardFreezeAllVisibleExcept($notfreeze);
         }
     }
 

classes/local/form/form_trigger_instance.php

@@ -135,13 +135,8 @@ class form_trigger_instance extends \moodleform {
             $this->lib->extend_add_instance_form_definition($mform);
         }
 
-        // For active workflows, we do not want the form to be editable.
-        if ($this->workflowid && !workflow_manager::is_editable($this->workflowid)) {
-            $this->add_cancel_button();
-        } else {
         $this->add_action_buttons();
     }
-    }
 
     /**
      * In case of read only mode only the cancel button is rendered.
@@ -186,7 +181,13 @@ class form_trigger_instance extends \moodleform {
         // For active workflows, we do not want the form to be editable.
         if ($this->workflowid && !workflow_manager::is_editable($this->workflowid)) {
             // The group buttonar is the array of submit buttons. For inactive workflows this is only a cancel button.
-            $mform->hardFreezeAllVisibleExcept(array('buttonar'));
+            $notfreeze = ['buttonar'];
+            foreach ($this->lib->instance_settings() as $setting) {
+                if ($setting->editable) {
+                    $notfreeze[] = $setting->name;
+                }
+            }
+            $mform->hardFreezeAllVisibleExcept($notfreeze);
         }
     }
 

classes/local/manager/settings_manager.php

@@ -23,6 +23,7 @@
  */
 namespace tool_lifecycle\local\manager;
 
+use tool_lifecycle\local\entity\workflow;
 use tool_lifecycle\settings_type;
 
 defined('MOODLE_INTERNAL') || die();
@@ -57,9 +58,11 @@ class settings_manager {
      * @param 'step'|'trigger' $type type of the subplugin.
      * @param string $subpluginname name of the subplugin.
      * @param mixed $data submitted data of the form.
+     * @param bool $accessvalidation whether to do only change settings that are editable once the workflow has started.
+     *          Then also calls the on_setting_changed listener. Defaults to false.
      * @throws \moodle_exception
      */
-    public static function save_settings($instanceid, $type, $subpluginname, $data) {
+    public static function save_settings($instanceid, $type, $subpluginname, $data, $accessvalidation = false) {
         global $DB;
         self::validate_type($type);
 
@@ -74,8 +77,12 @@ class settings_manager {
 
         if ($type == settings_type::TRIGGER) {
             $lib = lib_manager::get_trigger_lib($subpluginname);
+            $trigger = trigger_manager::get_instance($instanceid);
+            $wfeditable = workflow_manager::is_editable($trigger->workflowid);
         } else {
             $lib = lib_manager::get_step_lib($subpluginname);
+            $step = step_manager::get_step_instance($instanceid);
+            $wfeditable = workflow_manager::is_editable($step->workflowid);
         }
 
         $settingsfields = $lib->instance_settings();
@@ -83,6 +90,9 @@ class settings_manager {
             throw new \moodle_exception('id of the step instance has to be set!');
         }
         foreach ($settingsfields as $setting) {
+            if ($accessvalidation && !$wfeditable && !$setting->editable) {
+                continue;
+            }
             if (array_key_exists($setting->name, $data)) {
                 $value = $data[$setting->name];
                 // Needed for editor support.
@@ -105,8 +115,14 @@ class settings_manager {
                         'name' => $setting->name)
                 );
                 if ($record) {
+                    if ($record->value != $cleanedvalue) {
+                        $oldvalue = $record->value;
                         $record->value = $cleanedvalue;
                         $DB->update_record('tool_lifecycle_settings', $record);
+                        if ($accessvalidation && !$wfeditable) {
+                            $lib->on_setting_changed($setting->name, $cleanedvalue, $oldvalue);
+                        }
+                    }
                 } else {
                     $newrecord = new \stdClass();
                     $newrecord->instanceid = $instanceid;

lang/de/tool_lifecycle.php

@@ -51,7 +51,7 @@ $string['general_settings_header'] = 'Allgemeine Einstellungen';
 $string['followedby_none'] = 'Keine';
 $string['invalid_workflow'] = 'Ungültige Workflowkonfiguration';
 $string['invalid_workflow_details'] = 'Gehe zur Detailanzeige, um einen Trigger für diesen Workflow zu erstellen.';
-$string['active_workflow_not_changeable'] = 'Die Workflow-Instanz wurde bereits aktiviert. Es ist nicht mehr möglich, Schritte zu ändern.';
+$string['active_workflow_not_changeable'] = 'Die Workflow-Instanz wurde bereits aktiviert. Je nach Schritt-Typ können dessen Einstellungen eventuell noch geändert werden. Änderungen an Trigger-Instanzen wirken sich nicht auf bereits getriggerte Kurse aus.';
 $string['active_workflow_not_removeable'] = 'Die Workflow-Instanz ist aktiv. Es ist nicht möglich, sie zu entfernen.';
 $string['workflow_not_removeable'] = 'Es ist nicht möglich, diese Workflow-Instanz zu entfernen. Vielleicht hat sie noch laufende Prozesse?';
 $string['invalid_workflow_cannot_be_activated'] = 'Der Workflow kann nicht aktiviert werden, da die Workflowdefinition ungültig ist';

lang/en/tool_lifecycle.php

@@ -54,7 +54,7 @@ $string['general_settings_header'] = 'General settings';
 $string['followedby_none'] = 'None';
 $string['invalid_workflow'] = 'Invalid workflow configuration';
 $string['invalid_workflow_details'] = 'Go to details view, to create a trigger for this workflow';
-$string['active_workflow_not_changeable'] = 'The workflow instance was already activated. It is not possible to change any of its steps anymore.';
+$string['active_workflow_not_changeable'] = 'The workflow instance was already activated. Depending on the step type, some of its settings might be still editable. Changes to triggers will not affect already triggered courses.';
 $string['active_workflow_not_removeable'] = 'The workflow instance is active. It is not possible to remove it.';
 $string['workflow_not_removeable'] = 'It is not possible to remove this workflow instance. Maybe it still has running processes?';
 $string['invalid_workflow_cannot_be_activated'] = 'The workflow definition is invalid, thus it cannot be activated.';

step/createbackup/lib.php

@@ -101,7 +101,7 @@ class createbackup extends libbase {
      */
     public function instance_settings() {
         return array(
-            new instance_setting('maximumbackupspercron', PARAM_INT),
+            new instance_setting('maximumbackupspercron', PARAM_INT, true),
         );
     }
 

step/deletecourse/lib.php

@@ -95,7 +95,7 @@ class deletecourse extends libbase {
      */
     public function instance_settings() {
         return array(
-            new instance_setting('maximumdeletionspercron', PARAM_INT),
+            new instance_setting('maximumdeletionspercron', PARAM_INT, true),
         );
     }
 

step/email/lib.php

@@ -221,10 +221,10 @@ class email extends libbase {
      */
     public function instance_settings() {
         return array(
-            new instance_setting('responsetimeout', PARAM_INT),
-            new instance_setting('subject', PARAM_TEXT),
-            new instance_setting('content', PARAM_RAW),
-            new instance_setting('contenthtml', PARAM_RAW),
+            new instance_setting('responsetimeout', PARAM_INT, false),
+            new instance_setting('subject', PARAM_TEXT, true),
+            new instance_setting('content', PARAM_RAW, true),
+            new instance_setting('contenthtml', PARAM_RAW, true),
         );
     }
 

step/lib.php

@@ -107,6 +107,16 @@ abstract class libbase {
         return array();
     }
 
+    /**
+     * Is called when a setting is changed after a workflow is activated.
+     * @param string $settingname name of the setting
+     * @param mixed $newvalue the new value
+     * @param mixed $oldvalue the old value
+     */
+    public function on_setting_changed($settingname, $newvalue, $oldvalue) {
+
+    }
+
     /**
      * This method can be overriden, to add form elements to the form_step_instance.
      * It is called in definition().
@@ -149,14 +159,19 @@ class instance_setting {
     /** @var string param type of the setting, e.g. PARAM_INT */
     public $paramtype;
 
+    /** @var bool if editable after activation */
+    public $editable;
+
     /**
      * Create a local settings object.
      * @param string $name name of the setting
      * @param string $paramtype param type. Used for cleansing and parsing, e.g. PARAM_INT.
+     * @param bool $editable if setting is editable after activation
      */
-    public function __construct($name, $paramtype) {
+    public function __construct(string $name, string $paramtype, bool $editable = false) {
         $this->name = $name;
         $this->paramtype = $paramtype;
+        $this->editable = $editable;
     }
 
 }

trigger/categories/lib.php

@@ -103,8 +103,8 @@ class categories extends base_automatic {
      */
     public function instance_settings() {
         return array(
-            new instance_setting('categories', PARAM_SEQUENCE),
-            new instance_setting('exclude', PARAM_BOOL),
+            new instance_setting('categories', PARAM_SEQUENCE, true),
+            new instance_setting('exclude', PARAM_BOOL, true),
         );
     }
 

trigger/lib.php

@@ -53,6 +53,16 @@ abstract class base {
         return array();
     }
 
+    /**
+     * Is called when a setting is changed after a workflow is activated.
+     * @param string $settingname name of the setting
+     * @param mixed $newvalue the new value
+     * @param mixed $oldvalue the old value
+     */
+    public function on_setting_changed($settingname, $newvalue, $oldvalue) {
+
+    }
+
     /**
      * This method can be overriden, to add form elements to the form_step_instance.
      * It is called in definition().
@@ -179,14 +189,19 @@ class instance_setting {
     /** @var string param type of the setting, e.g. PARAM_INT */
     public $paramtype;
 
+    /** @var bool if editable after activation */
+    public $editable;
+
     /**
      * Create a local settings object.
      * @param string $name name of the setting
      * @param string $paramtype param type. Used for cleansing and parsing, e.g. PARAM_INT.
+     * @param bool $editable if setting is editable after activation
      */
-    public function __construct($name, $paramtype) {
+    public function __construct(string $name, string $paramtype, bool $editable = false) {
         $this->name = $name;
         $this->paramtype = $paramtype;
+        $this->editable = $editable;
     }
 
 }

trigger/startdatedelay/lib.php

@@ -80,7 +80,7 @@ class startdatedelay extends base_automatic {
      */
     public function instance_settings() {
         return array(
-            new instance_setting('delay', PARAM_INT)
+            new instance_setting('delay', PARAM_INT, true)
         );
     }