From 8e62feea71243e10ee22073c8eee2f9cf3f95915 Mon Sep 17 00:00:00 2001 From: Sukhwinder Dhillon Date: Tue, 23 Apr 2024 15:03:14 +0200 Subject: [PATCH] `EventRuleConfigForm::ON_SUCCESS`: Only save data if changes have been made --- .../controllers/EventRuleController.php | 18 ++++++++++++++---- .../EscalationRecipient.php | 14 +++++++------- application/forms/EventRuleConfigForm.php | 5 ++++- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php index 620b3b938..5c67ca377 100644 --- 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 index 6a30e906f..009667a24 100644 --- a/application/forms/EventRuleConfigElements/EscalationRecipient.php +++ b/application/forms/EventRuleConfigElements/EscalationRecipient.php @@ -217,6 +217,9 @@ public function getRecipients(): array $count += 1; } + // 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++) { if ($i === (int) $removePosition) { @@ -224,20 +227,17 @@ public function getRecipients(): array } $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 index f4d0317b7..fa4cf51bc 100644 --- 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)) {