EventRuleConfigForm::ON_SUCCESS: Only save data if changes have bee…

…n made
Icinga · Apr 23, 2024 · e08b25b · e08b25b
1 parent 913e324
commit e08b25b
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 12 deletions.
diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php
@@ -64,10 +64,20 @@ public function indexAction(): void
             ->populate($eventRuleConfigValues)
             ->on(Form::ON_SUCCESS, function (EventRuleConfigForm $form) use ($ruleId, $eventRuleConfigValues) {
                 $config = $form->getValues();
-                $config['object_filter'] = $eventRuleConfigValues['object_filter'];
-                $form->addOrUpdateRule($ruleId, $config);
-                Notification::success((sprintf(t('Successfully saved event rule %s'), $eventRuleConfigValues['name'])));
-                $this->redirectNow(Links::eventRule((int) $ruleId));
+               $hasChanges = $config !== array_intersect_key($eventRuleConfigValues, $config);
+               $redirectLink = Links::eventRules();
+
+               if ($hasChanges) {
+                   $config['object_filter'] = $eventRuleConfigValues['object_filter'];
+                   $form->addOrUpdateRule($ruleId, $config);
+                   $redirectLink = Links::eventRule((int) $ruleId);
+                   Notification::success(sprintf(
+                       t('Successfully saved event rule %s'),
+                       $eventRuleConfigValues['name']
+                   ));
+               }
+
+               $this->redirectNow($redirectLink);
             })
             ->on(
                 EventRuleConfigForm::ON_DELETE,

diff --git a/application/forms/EventRuleConfigElements/EscalationRecipient.php b/application/forms/EventRuleConfigElements/EscalationRecipient.php
@@ -213,23 +213,23 @@ public function getRecipients(): array
         /** @var int $count */
         $count = $this->getValue('recipient-count');
 
+        // Order of keys in $values maters, When comparing stored values with newly submitted values,
+        // to get the difference. The order of keys should be same as the columns order in the db,
+        // to compute the diff correctly when ON_SUCCESS is emitted.
         $values = [];
         for ($i = 1; $i <= $count; $i++) {
             $value = [];
-            $value['channel_id'] = $this->getValue('val_' . $i);
             $value['id'] = $this->getValue('id_' . $i);
 
             /** @var ?string $columnName */
             $columnName = $this->getValue('column_' . $i);
 
-            if ($columnName === null) {
-                $values[] = $value;
-                continue;
+            if ($columnName !== null) {
+                [$columnName, $id] = explode('_', $columnName, 2);
+                $value[$columnName . '_id'] = $id;
             }
 
-            [$columnName, $id] = explode('_', $columnName, 2);
-
-            $value[$columnName . '_id'] = $id;
+            $value['channel_id'] = $this->getValue('val_' . $i);
 
             $values[] = $value;
         }

diff --git a/application/forms/EventRuleConfigForm.php b/application/forms/EventRuleConfigForm.php
@@ -363,6 +363,9 @@ public function populate($values): self
      */
     public function getValues(): array
     {
+        // Order of keys in $values maters, When comparing stored values with newly submitted values,
+        // to get the difference. The order of keys should be same as the columns order in the db,
+        // to compute the diff correctly when ON_SUCCESS is emitted.
         $values = [];
         $escalations = [];
         /** @var string $prefixesString */
@@ -375,8 +378,8 @@ public function getValues(): array
             if ($this->hasElement('escalation-condition_' . $prefixMap)) {
                 /** @var EscalationCondition $escalationCondition */
                 $escalationCondition = $this->getElement('escalation-condition_' . $prefixMap);
-                $escalations[$i]['condition'] = $escalationCondition->getCondition();
                 $escalations[$i]['id'] = $escalationCondition->getValue('id');
+                $escalations[$i]['condition'] = $escalationCondition->getCondition();
             }
 
             if ($this->hasElement('escalation-condition_' . $prefixMap)) {