From efd11b6b4b1cc230ca56bf990f209f60c7f8bc4e Mon Sep 17 00:00:00 2001
From: Justus Dieckmann <justusdieckmann@wwu.de>
Date: Tue, 2 Nov 2021 09:30:07 +0100
Subject: [PATCH] Only validate editablity of settings when user changes
 settings in the interface; Add editability for trigger

---
 adminlib.php                                 | 2 +-
 classes/local/form/form_trigger_instance.php | 8 +++++++-
 classes/local/manager/settings_manager.php   | 8 +++++---
 lang/de/tool_lifecycle.php                   | 2 +-
 lang/en/tool_lifecycle.php                   | 2 +-
 trigger/categories/lib.php                   | 4 ++--
 trigger/startdatedelay/lib.php               | 2 +-
 7 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/adminlib.php b/adminlib.php
index 77cf55b..fa1aaaf 100644
--- a/adminlib.php
+++ b/adminlib.php
@@ -647,7 +647,7 @@ class workflow_settings {
             }
             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;
diff --git a/classes/local/form/form_trigger_instance.php b/classes/local/form/form_trigger_instance.php
index 71653a7..036e256 100644
--- a/classes/local/form/form_trigger_instance.php
+++ b/classes/local/form/form_trigger_instance.php
@@ -186,7 +186,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);
         }
     }
 
diff --git a/classes/local/manager/settings_manager.php b/classes/local/manager/settings_manager.php
index 2b6bf91..4db7be7 100644
--- a/classes/local/manager/settings_manager.php
+++ b/classes/local/manager/settings_manager.php
@@ -58,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);
 
@@ -88,7 +90,7 @@ class settings_manager {
             throw new \moodle_exception('id of the step instance has to be set!');
         }
         foreach ($settingsfields as $setting) {
-            if (!$wfeditable && !$setting->editable) {
+            if ($accessvalidation && !$wfeditable && !$setting->editable) {
                 continue;
             }
             if (array_key_exists($setting->name, $data)) {
@@ -117,7 +119,7 @@ class settings_manager {
                         $oldvalue = $record->value;
                         $record->value = $cleanedvalue;
                         $DB->update_record('tool_lifecycle_settings', $record);
-                        if (!$wfeditable) {
+                        if ($accessvalidation && !$wfeditable) {
                             $lib->on_setting_changed($setting->name, $cleanedvalue, $oldvalue);
                         }
                     }
diff --git a/lang/de/tool_lifecycle.php b/lang/de/tool_lifecycle.php
index 5ce7b49..d4857f1 100644
--- a/lang/de/tool_lifecycle.php
+++ b/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.';
 $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';
diff --git a/lang/en/tool_lifecycle.php b/lang/en/tool_lifecycle.php
index dcf69f4..155f270 100644
--- a/lang/en/tool_lifecycle.php
+++ b/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.';
 $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.';
diff --git a/trigger/categories/lib.php b/trigger/categories/lib.php
index acb2961..6438175 100644
--- a/trigger/categories/lib.php
+++ b/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),
         );
     }
 
diff --git a/trigger/startdatedelay/lib.php b/trigger/startdatedelay/lib.php
index 7d0cd64..01e3d0e 100644
--- a/trigger/startdatedelay/lib.php
+++ b/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)
         );
     }
 
-- 
GitLab