From cf49a7cabc20f59282b87f5e446bf308e7206099 Mon Sep 17 00:00:00 2001 From: raviks789 Date: Fri, 5 Apr 2024 12:39:26 +0200 Subject: [PATCH 01/14] Remove event rule session storage --- .../controllers/EventRuleController.php | 221 ++++++++++-------- .../controllers/EventRulesController.php | 120 ++++++---- .../EscalationCondition.php | 6 +- .../EscalationRecipient.php | 9 +- .../EventRuleConfigFilter.php | 2 +- application/forms/EventRuleConfigForm.php | 123 ++++------ library/Notifications/Common/Links.php | 5 + public/css/detail/event-rule-detail.less | 16 +- public/css/event-rule-config.less | 10 +- 9 files changed, 260 insertions(+), 252 deletions(-) diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php index 652c5e45..b39ba523 100644 --- a/application/controllers/EventRuleController.php +++ b/application/controllers/EventRuleController.php @@ -7,13 +7,13 @@ use Icinga\Module\Notifications\Common\Auth; use Icinga\Module\Notifications\Common\Database; use Icinga\Module\Notifications\Common\Links; +use Icinga\Module\Notifications\Forms\EventRuleConfigElements\EventRuleConfigFilter; use Icinga\Module\Notifications\Forms\EventRuleConfigForm; use Icinga\Module\Notifications\Forms\EventRuleForm; use Icinga\Module\Notifications\Model\Incident; use Icinga\Module\Notifications\Model\Rule; use Icinga\Module\Notifications\Web\Control\SearchBar\ExtraTagSuggestions; use Icinga\Web\Notification; -use Icinga\Web\Session; use ipl\Html\Attributes; use ipl\Html\Form; use ipl\Html\FormElement\SubmitButtonElement; @@ -31,93 +31,67 @@ class EventRuleController extends CompatController { use Auth; - /** @var Session\SessionNamespace */ - private $sessionNamespace; - - public function init() + public function init(): void { - $this->sessionNamespace = Session::getSession()->getNamespace('notifications'); $this->assertPermission('notifications/config/event-rule'); } public function indexAction(): void { - $this->sessionNamespace->delete('-1'); + // Add an empty container and set it as the X-Icinga-Container when sending extra updates + // from the modal for filter or event rule + $this->addContent(new HtmlElement( + 'div', + Attributes::create(['class' => 'container', 'id' => 'dummy-event-rule-container']) + )); $this->addTitleTab(t('Event Rule')); $this->controls->addAttributes(['class' => 'event-rule-detail']); - $ruleId = $this->params->getRequired('id'); - $configValues = $this->sessionNamespace->get($ruleId); - $this->controls->addAttributes(['class' => 'event-rule-detail']); - $disableSave = false; - if ($configValues === null) { - $configValues = $this->fromDb((int) $ruleId); - $disableSave = true; - } + $eventRuleConfigValues = $this->fromDb((int) $ruleId); - $eventRuleConfig = new EventRuleConfigForm( - $configValues, - Url::fromPath('notifications/event-rule/search-editor', ['id' => $ruleId]) - ); + if ($this->getRequest()->isPost()) { + if ($this->getRequest()->has('searchbar')) { + $eventRuleConfigValues['object_filter'] = $this->getRequest()->get('searchbar'); + } else { + $eventRuleConfigValues['object_filter'] = null; + } + } - $eventRuleConfig - ->populate($configValues) - ->on(Form::ON_SUCCESS, function (EventRuleConfigForm $form) use ($ruleId, $configValues) { - $form->addOrUpdateRule((int) $ruleId, $configValues); - $this->sessionNamespace->delete($ruleId); - Notification::success((sprintf(t('Successfully saved event rule %s'), $configValues['name']))); + $eventRuleConfig = (new EventRuleConfigForm( + $eventRuleConfigValues, + Url::fromPath( + 'notifications/event-rule/search-editor', + ['id' => $ruleId, 'object_filter' => $eventRuleConfigValues['object_filter']] + ) + )) + ->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)); }) - ->on(EventRuleConfigForm::ON_DELETE, function (EventRuleConfigForm $form) use ($ruleId, $configValues) { - $form->removeRule((int) $ruleId); - $this->sessionNamespace->delete($ruleId); - Notification::success(sprintf(t('Successfully deleted event rule %s'), $configValues['name'])); - $this->redirectNow(Links::eventRules()); - }) - ->on(EventRuleConfigForm::ON_DISCARD, function () use ($ruleId, $configValues) { - $this->sessionNamespace->delete($ruleId); - Notification::success( - sprintf( - t('Successfully discarded changes to event rule %s'), - $configValues['name'] - ) - ); - $this->redirectNow(Links::eventRule((int) $ruleId)); - }) - ->on(EventRuleConfigForm::ON_CHANGE, function (EventRuleConfigForm $form) use ($ruleId, $configValues) { - $formValues = $form->getValues(); - $configValues = array_merge($configValues, $formValues); - $configValues['rule_escalation'] = $formValues['rule_escalation']; - $this->sessionNamespace->set($ruleId, $configValues); - }) + ->on( + EventRuleConfigForm::ON_DELETE, + function (EventRuleConfigForm $form) use ($ruleId, $eventRuleConfigValues) { + $form->removeRule((int) $ruleId); + Notification::success( + sprintf(t('Successfully deleted event rule %s'), $eventRuleConfigValues['name']) + ); + $this->redirectNow('__CLOSE__'); + } + ) ->handleRequest($this->getServerRequest()); - $cache = $this->sessionNamespace->get($ruleId); - $discardChangesButton = null; - if ($cache !== null) { - $this->addContent(Html::tag('div', ['class' => 'cache-notice'], t('There are unsaved changes.'))); - $discardChangesButton = new SubmitButtonElement( - 'discard_changes', - [ - 'label' => t('Discard Changes'), - 'form' => 'event-rule-config-form', - 'class' => 'btn-discard-changes', - 'formnovalidate' => true, - ] - ); - - $disableSave = false; - } - $buttonsWrapper = new HtmlElement('div', Attributes::create(['class' => ['icinga-controls', 'save-config']])); $eventRuleConfigSubmitButton = new SubmitButtonElement( 'save', [ 'label' => t('Save'), 'form' => 'event-rule-config-form', - 'disabled' => $disableSave ] ); @@ -131,7 +105,7 @@ public function indexAction(): void ] ); - $buttonsWrapper->add([$eventRuleConfigSubmitButton, $discardChangesButton, $deleteButton]); + $buttonsWrapper->addHtml($eventRuleConfigSubmitButton, $deleteButton); if ($ruleId > 0) { $incidentCount = Incident::on(Database::get()) @@ -147,8 +121,8 @@ public function indexAction(): void } } - $eventRuleForm = Html::tag('div', ['class' => 'event-rule-form'], [ - Html::tag('h2', $configValues['name']), + $eventRuleForm = Html::tag('div', ['class' => 'event-rule-form', 'id' => 'event-rule-form'], [ + Html::tag('h2', $eventRuleConfigValues['name']), (new Link( new Icon('edit'), Url::fromPath('notifications/event-rule/edit', [ @@ -163,6 +137,47 @@ public function indexAction(): void $this->addContent($eventRuleConfig); } + public function ruleAction(): void + { + /** @var int $ruleId */ + $ruleId = $this->params->getRequired('id'); + $query = Rule::on(Database::get()) + ->withoutColumns('timeperiod_id') + ->filter(Filter::equal('id', $ruleId)); + + $rule = $query->first(); + + $this->getDocument()->add([ + Html::tag('h2', $rule->name), + (new Link( + new Icon('edit'), + Url::fromPath('notifications/event-rule/edit', [ + 'id' => $ruleId + ]), + ['class' => 'control-button'] + ))->openInModal() + ]); + } + + public function configFilterAction(): void + { + $ruleId = $this->params->getRequired('id'); + $objectFilter = $this->params->get('object_filter'); + $eventRuleFilterFieldset = new EventRuleConfigFilter( + Url::fromPath( + 'notifications/event-rule/search-editor', + ['id' => $ruleId, 'object_filter' => $objectFilter] + ), + $objectFilter + ); + + if (! $objectFilter) { + $eventRuleFilterFieldset->getAttributes()->add('class', 'empty-filter'); + } + + $this->getDocument()->addHtml($eventRuleFilterFieldset); + } + /** * Create config from db * @@ -231,33 +246,28 @@ public function searchEditorAction(): void { /** @var string $ruleId */ $ruleId = $this->params->shiftRequired('id'); - - $eventRule = $this->sessionNamespace->get($ruleId); - - if ($eventRule === null) { - $eventRule = $this->fromDb((int) $ruleId); - } - $editor = new SearchEditor(); - $objectFilter = $eventRule['object_filter'] ?? ''; + /** @var string $objectFilter */ + $objectFilter = $this->params->shift('object_filter', ''); $editor->setQueryString($objectFilter) ->setAction(Url::fromRequest()->getAbsoluteUrl()) ->setSuggestionUrl( Links::ruleFilterSuggestionUrl($ruleId)->addParams(['_disableLayout' => true, 'showCompact' => true]) ); - $editor->on(SearchEditor::ON_SUCCESS, function (SearchEditor $form) use ($ruleId, $eventRule) { - $eventRule['object_filter'] = self::createFilterString($form->getFilter()); - $this->sessionNamespace->set($ruleId, $eventRule); - $this->getResponse() - ->setHeader('X-Icinga-Container', '_self') - ->redirectAndExit( - Url::fromPath( - 'notifications/event-rule', - ['id' => $ruleId] + $editor->on(SearchEditor::ON_SUCCESS, function (SearchEditor $form) use ($ruleId) { + $this->sendExtraUpdates( + [ + '#filter-wrapper' => Url::fromPath( + 'notifications/event-rule/config-filter', + ['id' => $ruleId, 'object_filter' => self::createFilterString($form->getFilter())] ) - ); + ] + ); + $this->getResponse() + ->setHeader('X-Icinga-Container', 'dummy-event-rule-container') + ->redirectAndExit('__CLOSE__'); }); $editor->handleRequest($this->getServerRequest()); @@ -293,31 +303,42 @@ public function editAction(): void { /** @var string $ruleId */ $ruleId = $this->params->getRequired('id'); - $config = $this->sessionNamespace->get($ruleId); - if ($config === null) { - if ($ruleId === '-1') { - $config = ['id' => $ruleId]; - } else { - $config = $this->fromDb((int) $ruleId); - } + $db = Database::get(); + if ($ruleId === '-1') { + $config = ['id' => $ruleId]; + } else { + // Casting to array is required as Connection::fetchOne actually returns stdClass and not array + $config = (array) $db->fetchOne( + Rule::on($db)->withoutColumns('timeperiod_id')->filter(Filter::equal('id', $ruleId))->assembleSelect() + ); } $eventRuleForm = (new EventRuleForm()) ->populate($config) ->setAction(Url::fromRequest()->getAbsoluteUrl()) - ->on(Form::ON_SUCCESS, function ($form) use ($ruleId, $config) { + ->on(Form::ON_SUCCESS, function ($form) use ($ruleId, $config, $db) { $config['name'] = $form->getValue('name'); $config['is_active'] = $form->getValue('is_active'); + if ($ruleId === '-1') { $redirectUrl = Url::fromPath('notifications/event-rules/add', ['id' => '-1']); + $this->getResponse()->setHeader('X-Icinga-Container', 'col2'); + $this->redirectNow($redirectUrl); } else { - $redirectUrl = Url::fromPath('notifications/event-rule', ['id' => $ruleId]); - $this->sendExtraUpdates(['#col1']); + $db->update('rule', [ + 'name' => $config['name'], + 'is_active' => $config['is_active'] ?? 'n' + ], ['id = ?' => $ruleId]); + + $this->sendExtraUpdates([ + '#event-rule-form' => Url::fromPath( + 'notifications/event-rule/rule', ['id' => $ruleId] + )->getAbsoluteUrl() + ]); + + $this->getResponse()->setHeader('X-Icinga-Container', 'dummy-event-rule-container') + ->redirectAndExit('__CLOSE__'); } - - $this->sessionNamespace->set($ruleId, $config); - $this->getResponse()->setHeader('X-Icinga-Container', 'col2'); - $this->redirectNow($redirectUrl); })->handleRequest($this->getServerRequest()); if ($ruleId === '-1') { diff --git a/application/controllers/EventRulesController.php b/application/controllers/EventRulesController.php index a1a3fa78..5fe22a5b 100644 --- a/application/controllers/EventRulesController.php +++ b/application/controllers/EventRulesController.php @@ -6,11 +6,11 @@ use Icinga\Module\Notifications\Common\Database; use Icinga\Module\Notifications\Common\Links; +use Icinga\Module\Notifications\Forms\EventRuleConfigElements\EventRuleConfigFilter; use Icinga\Module\Notifications\Forms\EventRuleConfigForm; use Icinga\Module\Notifications\Model\Rule; use Icinga\Module\Notifications\Widget\ItemList\EventRuleList; use Icinga\Web\Notification; -use Icinga\Web\Session; use ipl\Html\Attributes; use ipl\Html\Form; use ipl\Html\FormElement\SubmitButtonElement; @@ -25,6 +25,7 @@ use ipl\Web\Widget\ButtonLink; use ipl\Web\Widget\Icon; use ipl\Web\Widget\Link; +use ipl\Web\Widget\Tabs; class EventRulesController extends CompatController { @@ -33,20 +34,14 @@ class EventRulesController extends CompatController /** @var Filter\Rule Filter from query string parameters */ private $filter; - /** @var Session\SessionNamespace */ - private $sessionNamespace; - - public function init() + public function init(): void { $this->assertPermission('notifications/config/event-rules'); - $this->sessionNamespace = Session::getSession()->getNamespace('notifications'); } public function indexAction(): void { $eventRules = Rule::on(Database::get()); - $this->sessionNamespace->delete('-1'); - $limitControl = $this->createLimitControl(); $paginationControl = $this->createPaginationControl($eventRules); $sortControl = $this->createSortControl( @@ -105,11 +100,20 @@ public function addAction(): void { $this->addTitleTab(t('Add Event Rule')); $this->getTabs()->setRefreshUrl(Url::fromPath('notifications/event-rules/add', ['id' => '-1'])); + // Add an empty container which set as the X-Icinga-Container when sending extra updates + // from the modal for filter + $this->addContent(Html::tag('div', ['class' => 'container', 'id' => 'dummy-event-rule-container'])); $this->controls->addAttributes(['class' => 'event-rule-detail']); $ruleId = $this->params->get('id'); - $config = $this->sessionNamespace->get($ruleId); - $config['object_filter'] = $config['object_filter'] ?? null; + + if ($this->getRequest()->isPost()) { + if ($this->getRequest()->has('searchbar')) { + $eventRule['object_filter'] = $this->getRequest()->get('searchbar'); + } else { + $eventRule['object_filter'] = null; + } + } $eventRuleConfigSubmitButton = (new SubmitButtonElement( 'save', @@ -120,40 +124,42 @@ public function addAction(): void ))->setWrapper(new HtmlElement('div', Attributes::create(['class' => ['icinga-controls', 'save-config']]))); $eventRuleConfig = new EventRuleConfigForm( - $config, + $eventRule, Url::fromPath( 'notifications/event-rules/search-editor', - ['id' => $ruleId] + ['id' => $ruleId, 'object_filter' => $eventRule['object_filter'] ?? ''] ) ); $eventRuleConfig - ->populate($config) - ->on(Form::ON_SUCCESS, function (EventRuleConfigForm $form) use ($config) { - $ruleId = (int) $config['id']; - $ruleName = $config['name']; - $insertId = $form->addOrUpdateRule($ruleId, $config); - $this->sessionNamespace->delete($ruleId); + ->on(Form::ON_SUCCESS, function (EventRuleConfigForm $form) use ($eventRule) { + /** @var string $ruleId */ + $ruleId = $eventRule['id']; + /** @var string $ruleName */ + $ruleName = $eventRule['name']; + $eventRuleConfig = array_merge($eventRule, $form->getValues()); + $insertId = $form->addOrUpdateRule($ruleId, $eventRuleConfig); Notification::success(sprintf(t('Successfully add event rule %s'), $ruleName)); $this->redirectNow(Links::eventRule($insertId)); }) - ->on(EventRuleConfigForm::ON_CHANGE, function (EventRuleConfigForm $form) use ($config) { - $formValues = $form->getValues(); - $config = array_merge($config, $formValues); - $config['rule_escalation'] = $formValues['rule_escalation']; - $this->sessionNamespace->set('-1', $config); - }) ->handleRequest($this->getServerRequest()); $eventRuleForm = Html::tag('div', ['class' => 'event-rule-form'], [ - Html::tag('h2', $config['name'] ?? ''), - (new Link( - new Icon('edit'), - Url::fromPath('notifications/event-rule/edit', [ - 'id' => -1 - ]), - ['class' => 'control-button'] - ))->openInModal() + Html::tag('h2', $eventRule['name'] ?? ''), + Html::tag( + 'div', + [ + 'class' => 'not-allowed', + 'title' => $this->translate('Cannot edit event rule until it is saved to database') + ], + (new Link( + new Icon('edit'), + Url::fromPath('notifications/event-rule/edit', [ + 'id' => -1 + ]), + ['class' => ['control-button', 'disabled']] + ))->openInModal() + ) ]); $this->addControl($eventRuleForm); @@ -161,34 +167,50 @@ public function addAction(): void $this->addContent($eventRuleConfig); } - public function searchEditorAction(): void + public function configFilterAction(): void { - $ruleId = $this->params->shiftRequired('id'); - $eventRule = $this->sessionNamespace->get($ruleId); + $ruleId = $this->params->getRequired('id'); + $objectFilter = $this->params->get('object_filter', ''); + $eventRuleFilterFieldset = new EventRuleConfigFilter( + Url::fromPath( + 'notifications/event-rule/search-editor', + ['id' => $ruleId, 'object_filter' => $objectFilter] + ), + $objectFilter + ); - if ($eventRule === null) { - $eventRule = ['id' => '-1']; + if ($objectFilter === '') { + $eventRuleFilterFieldset->getAttributes()->add('class', 'empty-filter'); } + $this->getDocument()->addHtml($eventRuleFilterFieldset); + } + + public function searchEditorAction(): void + { + $ruleId = $this->params->shiftRequired('id'); + + /** @var string $objectFilter */ + $objectFilter = $this->params->shift('object_filter', ''); $editor = new SearchEditor(); - $objectFilter = $eventRule['object_filter'] ?? ''; $editor->setQueryString($objectFilter) ->setAction(Url::fromRequest()->getAbsoluteUrl()) ->setSuggestionUrl(Links::ruleFilterSuggestionUrl($ruleId)); - $editor->on(SearchEditor::ON_SUCCESS, function (SearchEditor $form) use ($ruleId, $eventRule) { - $eventRule['object_filter'] = self::createFilterString($form->getFilter()); + $editor->on(SearchEditor::ON_SUCCESS, function (SearchEditor $form) use ($ruleId) { + $this->sendExtraUpdates( + [ + '#config-filter' => Url::fromPath( + 'notifications/event-rules/config-filter', + ['id' => $ruleId, 'object_filter' => self::createFilterString($form->getFilter())] + ) + ] + ); - $this->sessionNamespace->set($ruleId, $eventRule); $this->getResponse() - ->setHeader('X-Icinga-Container', '_self') - ->redirectAndExit( - Url::fromPath( - 'notifications/event-rules/add', - ['id' => $ruleId] - ) - ); + ->setHeader('X-Icinga-Container', 'dummy-event-rule-container') + ->redirectAndExit('__CLOSE__'); }); $editor->handleRequest($this->getServerRequest()); @@ -233,7 +255,7 @@ protected function getFilter(): Filter\Rule return $this->filter; } - public function getTabs() + public function getTabs(): Tabs { if ($this->getRequest()->getActionName() === 'index') { return parent::getTabs() diff --git a/application/forms/EventRuleConfigElements/EscalationCondition.php b/application/forms/EventRuleConfigElements/EscalationCondition.php index f9ae52b4..8ed56bec 100644 --- a/application/forms/EventRuleConfigElements/EscalationCondition.php +++ b/application/forms/EventRuleConfigElements/EscalationCondition.php @@ -104,7 +104,7 @@ protected function assemble(): void 'select', 'operator_' . $i, [ - 'class' => ['operator-input', 'autosubmit'], + 'class' => 'operator-input', 'options' => array_combine($operators, $operators), 'required' => true ] @@ -117,7 +117,7 @@ protected function assemble(): void 'select', $valName, [ - 'class' => ['autosubmit', 'right-operand'], + 'class' => 'right-operand', 'options' => [ 'ok' => $this->translate('Ok', 'notification.severity'), 'debug' => $this->translate('Debug', 'notification.severity'), @@ -139,7 +139,7 @@ protected function assemble(): void $valName, [ 'required' => true, - 'class' => ['autosubmit', 'right-operand'], + 'class' => 'right-operand', 'validators' => [ new CallbackValidator(function ($value, $validator) { if (! preg_match('~^\d+(?:\.?\d*)?[hms]{1}$~', $value)) { diff --git a/application/forms/EventRuleConfigElements/EscalationRecipient.php b/application/forms/EventRuleConfigElements/EscalationRecipient.php index 0900cec1..56a58bb1 100644 --- a/application/forms/EventRuleConfigElements/EscalationRecipient.php +++ b/application/forms/EventRuleConfigElements/EscalationRecipient.php @@ -72,7 +72,7 @@ protected function assemble(): void 'select', 'val_' . $i, [ - 'class' => ['autosubmit', 'right-operand'], + 'class' => 'right-operand', 'options' => $options, 'disabledOptions' => [''], 'value' => $this->getPopulatedValue('val_' . $i) @@ -94,13 +94,6 @@ protected function assemble(): void } else { $val->addAttributes(['required' => true]); } - } else { - $val = $this->createElement('text', 'val_' . $i, [ - 'class' => 'right-operand', - 'placeholder' => $this->translate('Please make a decision'), - 'disabled' => true, - 'value' => $this->getPopulatedValue('val_' . $i) - ]); } $this->registerElement($val); diff --git a/application/forms/EventRuleConfigElements/EventRuleConfigFilter.php b/application/forms/EventRuleConfigElements/EventRuleConfigFilter.php index b723be2a..c0608e4e 100644 --- a/application/forms/EventRuleConfigElements/EventRuleConfigFilter.php +++ b/application/forms/EventRuleConfigElements/EventRuleConfigFilter.php @@ -19,7 +19,7 @@ class EventRuleConfigFilter extends FieldsetElement /** @var ?string Event rule's object filter */ protected $objectFilter; - protected $defaultAttributes = ['class' => 'config-filter']; + protected $defaultAttributes = ['id' => 'config-filter', 'class' => 'config-filter']; public function __construct(Url $searchEditorUrl, ?string $filter) { diff --git a/application/forms/EventRuleConfigForm.php b/application/forms/EventRuleConfigForm.php index 654de5c4..12b25853 100644 --- a/application/forms/EventRuleConfigForm.php +++ b/application/forms/EventRuleConfigForm.php @@ -34,10 +34,6 @@ class EventRuleConfigForm extends Form public const ON_DELETE = 'delete'; - public const ON_DISCARD = 'discard'; - - public const ON_CHANGE = 'change'; - protected $defaultAttributes = [ 'class' => ['event-rule-config', 'icinga-form', 'icinga-controls'], 'name' => 'event-rule-config-form', @@ -57,23 +53,15 @@ class EventRuleConfigForm extends Form * Create a new EventRuleConfigForm * * @param array $config - * @param Url $searchEditorUrl + * @param Url $searchEditorUrl */ public function __construct(array $config, Url $searchEditorUrl) { $this->config = $config; $this->searchEditorUrl = $searchEditorUrl; - - $this->on(self::ON_SENT, function () { - $config = array_merge($this->config, $this->getValues()); - - if ($config !== $this->config) { - $this->emit(self::ON_CHANGE, [$this]); - } - }); } - public function hasBeenSubmitted() + public function hasBeenSubmitted(): bool { $pressedButton = $this->getPressedSubmitElement(); @@ -82,8 +70,6 @@ public function hasBeenSubmitted() if ($buttonName === 'delete') { $this->emit(self::ON_DELETE, [$this]); - } elseif ($buttonName === 'discard_changes') { - $this->emit(self::ON_DISCARD, [$this]); } elseif ($buttonName === 'save') { return true; } @@ -137,10 +123,9 @@ protected function assemble(): void ); $defaultEscalationPrefix = bin2hex('1'); - $this->addElement('hidden', 'zero-condition-escalation'); - if (! isset($this->config['rule_escalation'])) { + if (! $this->hasBeenSent() && ! isset($this->config['rule_escalation'])) { $this->getElement('zero-condition-escalation')->setValue($defaultEscalationPrefix); } @@ -158,12 +143,12 @@ protected function assemble(): void ); $this->registerElement($addEscalationButton); + $prefixesElement = $this->createElement('hidden', 'prefixes-map', ['value' => $defaultEscalationPrefix]); $this->addElement($prefixesElement); $this->handleAdd(); - $prefixesMapString = $prefixesElement->getValue(); - $prefixesMap = explode(',', $prefixesMapString); + $prefixesMap = explode(',', $prefixesElement->getValue()); $escalationCount = count($prefixesMap); $zeroConditionEscalation = $this->getValue('zero-condition-escalation'); $removePosition = null; @@ -221,10 +206,14 @@ protected function assemble(): void $this->getElement('zero-condition-escalation')->setValue($zeroConditionEscalation); $this->addHtml( - (new HtmlElement('div', Attributes::create(['class' => 'filter-wrapper']))) + (new HtmlElement('div', Attributes::create(['class' => 'filter-pipeline']))) ->addHtml( (new FlowLine())->getRightArrow(), - $configFilter, + new HtmlElement( + 'div', + Attributes::create(['id' => 'filter-wrapper', 'class' => 'filter-wrapper']), + $configFilter + ), (new FlowLine())->getHorizontalLine() ) ); @@ -244,8 +233,7 @@ protected function handleAdd(): void if ($pressedButton && $pressedButton->getName() === 'add-escalation') { $this->clearPopulatedValue('prefixes-map'); - $prefixesMapString = $this->getValue('prefixes-map', ''); - $prefixesMap = explode(',', $prefixesMapString); + $prefixesMap = explode(',', $this->getValue('prefixes-map', '')); $escalationFakePos = bin2hex(random_bytes(4)); $prefixesMap[] = $escalationFakePos; $this->getElement('prefixes-map') @@ -346,13 +334,19 @@ public function getValues(): array $prefixesMap = explode(',', $prefixesString); $i = 1; foreach ($prefixesMap as $prefixMap) { - /** @var EscalationCondition $escalationCondition */ - $escalationCondition = $this->getElement('escalation-condition_' . $prefixMap); - /** @var EscalationRecipient $escalationRecipient */ - $escalationRecipient = $this->getElement('escalation-recipient_' . $prefixMap); - $escalations[$i]['condition'] = $escalationCondition->getCondition(); - $escalations[$i]['id'] = $escalationCondition->getValue('id'); - $escalations[$i]['recipients'] = $escalationRecipient->getRecipients(); + 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'); + } + + if ($this->hasElement('escalation-recipient_' . $prefixMap)) { + /** @var EscalationRecipient $escalationRecipient */ + $escalationRecipient = $this->getElement('escalation-recipient_' . $prefixMap); + $escalations[$i]['recipients'] = $escalationRecipient->getRecipients(); + } + $i++; } @@ -418,9 +412,7 @@ protected function createRemoveButton(string $prefix): SubmitButtonElement public function addOrUpdateRule(int $id, array $config): int { $db = Database::get(); - $db->beginTransaction(); - if ($id < 0) { $db->insert('rule', [ 'name' => $config['name'], @@ -432,17 +424,14 @@ public function addOrUpdateRule(int $id, array $config): int $id = $db->lastInsertId(); } else { $db->update('rule', [ - 'name' => $config['name'], - 'timeperiod_id' => $config['timeperiod_id'] ?? null, 'object_filter' => $config['object_filter'] ?? null, - 'is_active' => $config['is_active'] ?? 'n' ], ['id = ?' => $id]); } $escalationsFromDb = RuleEscalation::on($db) ->filter(Filter::equal('rule_id', $id)); - $escalationsInCache = $config['rule_escalation']; + $escalationsInForm = $config['rule_escalation']; $escalationsToUpdate = []; $escalationsToRemove = []; @@ -450,19 +439,19 @@ public function addOrUpdateRule(int $id, array $config): int /** @var RuleEscalation $escalationFromDB */ foreach ($escalationsFromDb as $escalationFromDB) { $escalationId = $escalationFromDB->id; - $escalationInCache = array_filter($escalationsInCache, function (array $element) use ($escalationId) { - /** @var string $idInCache */ - $idInCache = $element['id'] ?? null; - return (int) $idInCache === $escalationId; + $escalationInForm = array_filter($escalationsInForm, function (array $element) use ($escalationId) { + /** @var string $idInForm */ + $idInForm = $element['id'] ?? null; + return (int) $idInForm === $escalationId; }); - if ($escalationInCache) { - $position = array_key_first($escalationInCache); + if ($escalationInForm) { + $position = array_key_first($escalationInForm); // Escalations in DB to update - $escalationsToUpdate[$position] = $escalationInCache[$position]; + $escalationsToUpdate[$position] = $escalationInForm[$position]; - unset($escalationsInCache[$position]); + unset($escalationsInForm[$position]); } else { // Escalation in DB to remove $escalationsToRemove[] = $escalationId; @@ -470,7 +459,7 @@ public function addOrUpdateRule(int $id, array $config): int } // Escalations to add - $escalationsToAdd = $escalationsInCache; + $escalationsToAdd = $escalationsInForm; if (! empty($escalationsToRemove)) { $db->delete('rule_escalation_recipient', ['rule_escalation_id IN (?)' => $escalationsToRemove]); @@ -532,17 +521,17 @@ private function insertOrUpdateEscalations(int $ruleId, array $escalations, bool /** @var RuleEscalationRecipient $recipient */ foreach ($recipients as $recipient) { $recipientId = $recipient->id; - $recipientInCache = array_filter( + $recipientInForm = array_filter( $recipientsFromConfig, function (array $element) use ($recipientId) { - /** @var string $idFromCache */ - $idFromCache = $element['id']; - return (int) $idFromCache === $recipientId; + /** @var string $idFromForm */ + $idFromForm = $element['id']; + return (int) $idFromForm === $recipientId; } ); - if (empty($recipientInCache)) { - // Recipients to remove from Db not in cache + if (empty($recipientInForm)) { + // Recipients to remove from Db not in form $recipientsToRemove[] = $recipientId; } } @@ -555,30 +544,12 @@ function (array $element) use ($recipientId) { foreach ($recipientsFromConfig as $recipientConfig) { $data = [ 'rule_escalation_id' => $escalationId, - 'channel_id' => $recipientConfig['channel_id'] + 'channel_id' => $recipientConfig['channel_id'], + 'contact_id' => $recipientConfig['contact_id'] ?? null, + 'contactgroup_id' => $recipientConfig['contactgroup_id'] ?? null, + 'schedule_id' => $recipientConfig['schedule_id'] ?? null, ]; - switch (true) { - case isset($recipientConfig['contact_id']): - $data['contact_id'] = $recipientConfig['contact_id']; - $data['contactgroup_id'] = null; - $data['schedule_id'] = null; - - break; - case isset($recipientConfig['contactgroup_id']): - $data['contact_id'] = null; - $data['contactgroup_id'] = $recipientConfig['contactgroup_id']; - $data['schedule_id'] = null; - - break; - case isset($recipientConfig['schedule_id']): - $data['contact_id'] = null; - $data['contactgroup_id'] = null; - $data['schedule_id'] = $recipientConfig['schedule_id']; - - break; - } - if (! isset($recipientConfig['id'])) { $db->insert('rule_escalation_recipient', $data); } else { @@ -588,9 +559,9 @@ function (array $element) use ($recipientId) { } } - public function isValidEvent($event) + public function isValidEvent($event): bool { - if (in_array($event, [self::ON_CHANGE, self::ON_DELETE, self::ON_DISCARD])) { + if ($event === self::ON_DELETE) { return true; } diff --git a/library/Notifications/Common/Links.php b/library/Notifications/Common/Links.php index b2f040d3..cf8d033e 100644 --- a/library/Notifications/Common/Links.php +++ b/library/Notifications/Common/Links.php @@ -115,4 +115,9 @@ public static function ruleFilterSuggestionUrl(int $id): Url { return Url::fromPath("notifications/event-rule/complete", ['id' => $id]); } + + public static function addRule(): Url + { + return Url::fromPath('notifications/event-rule/add'); + } } diff --git a/public/css/detail/event-rule-detail.less b/public/css/detail/event-rule-detail.less index 0e3c83e1..522ee23a 100644 --- a/public/css/detail/event-rule-detail.less +++ b/public/css/detail/event-rule-detail.less @@ -3,14 +3,6 @@ align-items: baseline; } -.cache-notice { - margin: 1em; - padding: 1em; - background-color: @gray-lighter; - text-align: center; - .rounded-corners(); -} - .new-event-rule { margin-bottom: 1em; } @@ -38,6 +30,14 @@ width: 25em; } } + + .control-button.disabled { + pointer-events: none; + } + + .not-allowed { + cursor: not-allowed; + } } .save-config { diff --git a/public/css/event-rule-config.less b/public/css/event-rule-config.less index eb886b33..94c41f90 100644 --- a/public/css/event-rule-config.less +++ b/public/css/event-rule-config.less @@ -37,12 +37,12 @@ align-self: flex-end; } - .filter-wrapper { + .filter-pipeline { display: inline-flex; align-self: flex-start; } - .filter-wrapper:has(.config-filter .search-controls) { + .filter-pipeline:has(.config-filter .search-controls) { align-items: baseline; } @@ -135,7 +135,7 @@ } } -.filter-wrapper, +.filter-pipeline, .escalations { .horizontal-line, .right-arrow { @@ -183,10 +183,6 @@ } } - .default-channel { - color: @disabled-gray; - } - select, input { min-width: 10em; text-align-last: center; // text-align does not work in safari for select tags From a2022e6739b40fce3017ceb0342d61535828eb94 Mon Sep 17 00:00:00 2001 From: raviks789 Date: Wed, 24 Apr 2024 16:59:55 +0200 Subject: [PATCH 02/14] EventRulesController: Move `addAction` to `EventRuleController` --- .../controllers/EventRuleController.php | 72 ++++++++ .../controllers/EventRulesController.php | 157 ------------------ 2 files changed, 72 insertions(+), 157 deletions(-) diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php index b39ba523..b57703aa 100644 --- a/application/controllers/EventRuleController.php +++ b/application/controllers/EventRuleController.php @@ -299,6 +299,78 @@ public static function createFilterString(Filter\Rule $filters): ?string return $filterStr !== '' ? rawurldecode($filterStr) : null; } + public function addAction(): void + { + $this->addTitleTab(t('Add Event Rule')); + $this->getTabs()->setRefreshUrl(Url::fromRequest()); + + // Add an empty container and set it as the X-Icinga-Container when sending extra updates + // from the modal for filter or event rule + $this->addContent(Html::tag('div', ['class' => 'container', 'id' => 'dummy-event-rule-container'])); + + $this->controls->addAttributes(['class' => 'event-rule-detail']); + $ruleId = $this->params->get('id'); + + if ($this->getRequest()->isPost()) { + if ($this->getRequest()->has('searchbar')) { + $eventRule['object_filter'] = $this->getRequest()->get('searchbar'); + } else { + $eventRule['object_filter'] = null; + } + } + + $eventRuleConfigSubmitButton = (new SubmitButtonElement( + 'save', + [ + 'label' => t('Add Event Rule'), + 'form' => 'event-rule-config-form' + ] + ))->setWrapper(new HtmlElement('div', Attributes::create(['class' => ['icinga-controls', 'save-config']]))); + + $eventRuleConfig = new EventRuleConfigForm( + $eventRule, + Url::fromPath( + 'notifications/event-rule/search-editor', + ['id' => $ruleId, 'object_filter' => $eventRule['object_filter']] + ) + ); + + $eventRuleConfig + ->on(Form::ON_SUCCESS, function (EventRuleConfigForm $form) use ($eventRule) { + /** @var string $ruleId */ + $ruleId = $eventRule['id']; + /** @var string $ruleName */ + $ruleName = $eventRule['name']; + $eventRuleConfig = array_merge($eventRule, $form->getValues()); + $form->addOrUpdateRule($ruleId, $eventRuleConfig); + Notification::success(sprintf(t('Successfully add event rule %s'), $ruleName)); + $this->redirectNow('__CLOSE__'); + }) + ->handleRequest($this->getServerRequest()); + + $eventRuleForm = Html::tag('div', ['class' => 'event-rule-form'], [ + Html::tag('h2', $eventRule['name'] ?? ''), + Html::tag( + 'div', + [ + 'class' => 'not-allowed', + 'title' => $this->translate('Cannot edit event rule until it is saved to database') + ], + (new Link( + new Icon('edit'), + Url::fromPath('notifications/event-rule/edit', [ + 'id' => -1 + ]), + ['class' => ['control-button', 'disabled']] + ))->openInModal() + ) + ]); + + $this->addControl($eventRuleForm); + $this->addControl($eventRuleConfigSubmitButton); + $this->addContent($eventRuleConfig); + } + public function editAction(): void { /** @var string $ruleId */ diff --git a/application/controllers/EventRulesController.php b/application/controllers/EventRulesController.php index 5fe22a5b..efbee30a 100644 --- a/application/controllers/EventRulesController.php +++ b/application/controllers/EventRulesController.php @@ -5,26 +5,14 @@ namespace Icinga\Module\Notifications\Controllers; use Icinga\Module\Notifications\Common\Database; -use Icinga\Module\Notifications\Common\Links; -use Icinga\Module\Notifications\Forms\EventRuleConfigElements\EventRuleConfigFilter; -use Icinga\Module\Notifications\Forms\EventRuleConfigForm; use Icinga\Module\Notifications\Model\Rule; use Icinga\Module\Notifications\Widget\ItemList\EventRuleList; -use Icinga\Web\Notification; -use ipl\Html\Attributes; -use ipl\Html\Form; -use ipl\Html\FormElement\SubmitButtonElement; -use ipl\Html\Html; -use ipl\Html\HtmlElement; use ipl\Stdlib\Filter; use ipl\Web\Compat\CompatController; use ipl\Web\Compat\SearchControls; -use ipl\Web\Control\SearchEditor; use ipl\Web\Filter\QueryString; use ipl\Web\Url; use ipl\Web\Widget\ButtonLink; -use ipl\Web\Widget\Icon; -use ipl\Web\Widget\Link; use ipl\Web\Widget\Tabs; class EventRulesController extends CompatController @@ -96,151 +84,6 @@ public function indexAction(): void $this->getTabs()->activate('event-rules'); } - public function addAction(): void - { - $this->addTitleTab(t('Add Event Rule')); - $this->getTabs()->setRefreshUrl(Url::fromPath('notifications/event-rules/add', ['id' => '-1'])); - // Add an empty container which set as the X-Icinga-Container when sending extra updates - // from the modal for filter - $this->addContent(Html::tag('div', ['class' => 'container', 'id' => 'dummy-event-rule-container'])); - - $this->controls->addAttributes(['class' => 'event-rule-detail']); - $ruleId = $this->params->get('id'); - - if ($this->getRequest()->isPost()) { - if ($this->getRequest()->has('searchbar')) { - $eventRule['object_filter'] = $this->getRequest()->get('searchbar'); - } else { - $eventRule['object_filter'] = null; - } - } - - $eventRuleConfigSubmitButton = (new SubmitButtonElement( - 'save', - [ - 'label' => t('Add Event Rule'), - 'form' => 'event-rule-config-form' - ] - ))->setWrapper(new HtmlElement('div', Attributes::create(['class' => ['icinga-controls', 'save-config']]))); - - $eventRuleConfig = new EventRuleConfigForm( - $eventRule, - Url::fromPath( - 'notifications/event-rules/search-editor', - ['id' => $ruleId, 'object_filter' => $eventRule['object_filter'] ?? ''] - ) - ); - - $eventRuleConfig - ->on(Form::ON_SUCCESS, function (EventRuleConfigForm $form) use ($eventRule) { - /** @var string $ruleId */ - $ruleId = $eventRule['id']; - /** @var string $ruleName */ - $ruleName = $eventRule['name']; - $eventRuleConfig = array_merge($eventRule, $form->getValues()); - $insertId = $form->addOrUpdateRule($ruleId, $eventRuleConfig); - Notification::success(sprintf(t('Successfully add event rule %s'), $ruleName)); - $this->redirectNow(Links::eventRule($insertId)); - }) - ->handleRequest($this->getServerRequest()); - - $eventRuleForm = Html::tag('div', ['class' => 'event-rule-form'], [ - Html::tag('h2', $eventRule['name'] ?? ''), - Html::tag( - 'div', - [ - 'class' => 'not-allowed', - 'title' => $this->translate('Cannot edit event rule until it is saved to database') - ], - (new Link( - new Icon('edit'), - Url::fromPath('notifications/event-rule/edit', [ - 'id' => -1 - ]), - ['class' => ['control-button', 'disabled']] - ))->openInModal() - ) - ]); - - $this->addControl($eventRuleForm); - $this->addControl($eventRuleConfigSubmitButton); - $this->addContent($eventRuleConfig); - } - - public function configFilterAction(): void - { - $ruleId = $this->params->getRequired('id'); - $objectFilter = $this->params->get('object_filter', ''); - $eventRuleFilterFieldset = new EventRuleConfigFilter( - Url::fromPath( - 'notifications/event-rule/search-editor', - ['id' => $ruleId, 'object_filter' => $objectFilter] - ), - $objectFilter - ); - - if ($objectFilter === '') { - $eventRuleFilterFieldset->getAttributes()->add('class', 'empty-filter'); - } - - $this->getDocument()->addHtml($eventRuleFilterFieldset); - } - - public function searchEditorAction(): void - { - $ruleId = $this->params->shiftRequired('id'); - - /** @var string $objectFilter */ - $objectFilter = $this->params->shift('object_filter', ''); - $editor = new SearchEditor(); - - $editor->setQueryString($objectFilter) - ->setAction(Url::fromRequest()->getAbsoluteUrl()) - ->setSuggestionUrl(Links::ruleFilterSuggestionUrl($ruleId)); - - $editor->on(SearchEditor::ON_SUCCESS, function (SearchEditor $form) use ($ruleId) { - $this->sendExtraUpdates( - [ - '#config-filter' => Url::fromPath( - 'notifications/event-rules/config-filter', - ['id' => $ruleId, 'object_filter' => self::createFilterString($form->getFilter())] - ) - ] - ); - - $this->getResponse() - ->setHeader('X-Icinga-Container', 'dummy-event-rule-container') - ->redirectAndExit('__CLOSE__'); - }); - - $editor->handleRequest($this->getServerRequest()); - - $this->getDocument()->addHtml($editor); - $this->setTitle($this->translate('Adjust Filter')); - } - - /** - * Create filter string from the given filter rule - * - * @param Filter\Rule $filters - * - * @return ?string - */ - public static function createFilterString(Filter\Rule $filters): ?string - { - if ($filters instanceof Filter\Chain) { - foreach ($filters as $filter) { - self::createFilterString($filter); - } - } elseif ($filters instanceof Filter\Condition && empty($filters->getValue())) { - $filters->setValue(true); - } - - $filterStr = QueryString::render($filters); - - return $filterStr !== '' ? rawurldecode($filterStr) : null; - } - /** * Get the filter created from query string parameters * From f65f877ad0deaa61725fb3c94962aec4c63923f7 Mon Sep 17 00:00:00 2001 From: raviks789 Date: Mon, 8 Apr 2024 14:15:19 +0200 Subject: [PATCH 03/14] PHPStan: Update baseline --- phpstan-baseline-standard.neon | 5 ----- 1 file changed, 5 deletions(-) diff --git a/phpstan-baseline-standard.neon b/phpstan-baseline-standard.neon index bfb875b2..5b38c9b7 100644 --- a/phpstan-baseline-standard.neon +++ b/phpstan-baseline-standard.neon @@ -110,11 +110,6 @@ parameters: count: 2 path: application/controllers/EventRuleController.php - - - message: "#^Cannot access offset 'object_filter' on mixed\\.$#" - count: 2 - path: application/controllers/EventRuleController.php - - message: "#^Cannot access property \\$position on mixed\\.$#" count: 2 From 4366b6543879dafa50a0fe84f6a178f688f684f8 Mon Sep 17 00:00:00 2001 From: Sukhwinder Dhillon Date: Tue, 23 Apr 2024 15:03:14 +0200 Subject: [PATCH 04/14] `EventRuleConfigForm::ON_SUCCESS`: Only save data if changes have been made --- .../controllers/EventRuleController.php | 25 +++++++++++----- application/forms/EventRuleConfigForm.php | 29 ++++++++++++++++++- 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php index b57703aa..2f83d5be 100644 --- a/application/controllers/EventRuleController.php +++ b/application/controllers/EventRuleController.php @@ -50,12 +50,15 @@ public function indexAction(): void $ruleId = $this->params->getRequired('id'); $eventRuleConfigValues = $this->fromDb((int) $ruleId); + $filter = &$eventRuleConfigValues['object_filter']; // Assignment by reference to is used as search editor is a + // different form and the config must have the updated + // object_filter as soon as the search editor is closed if ($this->getRequest()->isPost()) { if ($this->getRequest()->has('searchbar')) { - $eventRuleConfigValues['object_filter'] = $this->getRequest()->get('searchbar'); + $filter = $this->getRequest()->get('searchbar'); } else { - $eventRuleConfigValues['object_filter'] = null; + $filter = null; } } @@ -63,15 +66,22 @@ public function indexAction(): void $eventRuleConfigValues, Url::fromPath( 'notifications/event-rule/search-editor', - ['id' => $ruleId, 'object_filter' => $eventRuleConfigValues['object_filter']] + ['id' => $ruleId, 'object_filter' => $filter] ) )) ->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']))); + $diff = $form->getChanges(); + if (empty($diff)) { + return; + } + + $form->addOrUpdateRule($ruleId, $diff); + Notification::success(sprintf( + t('Successfully saved event rule %s'), + $eventRuleConfigValues['name'] + )); + $this->redirectNow(Links::eventRule((int) $ruleId)); }) ->on( @@ -197,6 +207,7 @@ public function fromDb(int $ruleId): array } $config = iterator_to_array($rule); + $config['object_filter'] = $config['object_filter'] ?? null; foreach ($rule->rule_escalation as $re) { foreach ($re as $k => $v) { diff --git a/application/forms/EventRuleConfigForm.php b/application/forms/EventRuleConfigForm.php index 12b25853..8f389f14 100644 --- a/application/forms/EventRuleConfigForm.php +++ b/application/forms/EventRuleConfigForm.php @@ -422,12 +422,18 @@ public function addOrUpdateRule(int $id, array $config): int ]); $id = $db->lastInsertId(); - } else { + } elseif (isset($config['object_filter'])) { $db->update('rule', [ 'object_filter' => $config['object_filter'] ?? null, ], ['id = ?' => $id]); } + if (! isset($config['rule_escalation'])) { + $db->commitTransaction(); + + return $id; + } + $escalationsFromDb = RuleEscalation::on($db) ->filter(Filter::equal('rule_id', $id)); @@ -599,6 +605,27 @@ public function removeRule(int $id): void $db->commitTransaction(); } + /** + * Get the newly made changes + * + * @return array + */ + public function getChanges(): array + { + $values = $this->getValues(); + $dbValuesToCompare = array_intersect_key($this->config, $values); + + $checker = static function ($a, $b) use (&$checker) { + if (! is_array($a) || ! is_array($b)) { + return $a <=> $b; + } + + return empty(array_udiff_assoc($a, $b, $checker)) ? 0 : 1; + }; + + return array_udiff_assoc($values, $dbValuesToCompare, $checker); + } + /** * Get the prefix map * From 1acaf25edae5b26abc1ed6ed91b213093962ec1b Mon Sep 17 00:00:00 2001 From: raviks789 Date: Thu, 16 May 2024 14:48:56 +0200 Subject: [PATCH 05/14] Update column 1 after successful changes to event rule configuration --- application/controllers/EventRuleController.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php index 2f83d5be..0b3fbca9 100644 --- a/application/controllers/EventRuleController.php +++ b/application/controllers/EventRuleController.php @@ -82,6 +82,7 @@ public function indexAction(): void $eventRuleConfigValues['name'] )); + $this->sendExtraUpdates(['#col1']); $this->redirectNow(Links::eventRule((int) $ruleId)); }) ->on( @@ -348,14 +349,12 @@ public function addAction(): void $eventRuleConfig ->on(Form::ON_SUCCESS, function (EventRuleConfigForm $form) use ($eventRule) { - /** @var string $ruleId */ - $ruleId = $eventRule['id']; - /** @var string $ruleName */ - $ruleName = $eventRule['name']; $eventRuleConfig = array_merge($eventRule, $form->getValues()); - $form->addOrUpdateRule($ruleId, $eventRuleConfig); - Notification::success(sprintf(t('Successfully add event rule %s'), $ruleName)); - $this->redirectNow('__CLOSE__'); + $ruleId = $form->addOrUpdateRule((int) $eventRule['id'], $eventRuleConfig); + Notification::success(sprintf(t('Successfully add event rule %s'), $eventRule['name'])); + + $this->sendExtraUpdates(['#col1']); + $this->redirectNow(Links::eventRule($ruleId)); }) ->handleRequest($this->getServerRequest()); @@ -416,7 +415,8 @@ public function editAction(): void $this->sendExtraUpdates([ '#event-rule-form' => Url::fromPath( 'notifications/event-rule/rule', ['id' => $ruleId] - )->getAbsoluteUrl() + )->getAbsoluteUrl(), + '#col1' ]); $this->getResponse()->setHeader('X-Icinga-Container', 'dummy-event-rule-container') From 2cd3a0871bad1ac9aa7411f96d713cea03c801a0 Mon Sep 17 00:00:00 2001 From: raviks789 Date: Thu, 16 May 2024 15:25:46 +0200 Subject: [PATCH 06/14] EventRuleConfigForm::getChanges: Do not check for changes in config filter --- application/controllers/EventRuleController.php | 2 +- application/forms/EventRuleConfigForm.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php index 0b3fbca9..fdfa7569 100644 --- a/application/controllers/EventRuleController.php +++ b/application/controllers/EventRuleController.php @@ -186,7 +186,7 @@ public function configFilterAction(): void $eventRuleFilterFieldset->getAttributes()->add('class', 'empty-filter'); } - $this->getDocument()->addHtml($eventRuleFilterFieldset); + $this->getDocument()->add($eventRuleFilterFieldset); } /** diff --git a/application/forms/EventRuleConfigForm.php b/application/forms/EventRuleConfigForm.php index 8f389f14..6d7730e8 100644 --- a/application/forms/EventRuleConfigForm.php +++ b/application/forms/EventRuleConfigForm.php @@ -422,7 +422,7 @@ public function addOrUpdateRule(int $id, array $config): int ]); $id = $db->lastInsertId(); - } elseif (isset($config['object_filter'])) { + } else { $db->update('rule', [ 'object_filter' => $config['object_filter'] ?? null, ], ['id = ?' => $id]); From b9e95196fe4369f6f9a7fa9e956e3c8cc35dafde Mon Sep 17 00:00:00 2001 From: raviks789 Date: Fri, 17 May 2024 12:43:12 +0200 Subject: [PATCH 07/14] Improve event rule configuration layout --- public/css/detail/event-rule-detail.less | 8 -------- public/css/event-rule-config.less | 26 +++++++++++++++++------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/public/css/detail/event-rule-detail.less b/public/css/detail/event-rule-detail.less index 522ee23a..f4377e17 100644 --- a/public/css/detail/event-rule-detail.less +++ b/public/css/detail/event-rule-detail.less @@ -30,14 +30,6 @@ width: 25em; } } - - .control-button.disabled { - pointer-events: none; - } - - .not-allowed { - cursor: not-allowed; - } } .save-config { diff --git a/public/css/event-rule-config.less b/public/css/event-rule-config.less index 94c41f90..82e09252 100644 --- a/public/css/event-rule-config.less +++ b/public/css/event-rule-config.less @@ -1,6 +1,6 @@ .event-rule-config { display: flex; - align-items: center; + align-items: flex-start; ul { list-style-type: none; margin: 0; @@ -39,11 +39,6 @@ .filter-pipeline { display: inline-flex; - align-self: flex-start; - } - - .filter-pipeline:has(.config-filter .search-controls) { - align-items: baseline; } .add-escalation { @@ -97,7 +92,7 @@ } .horizontal-line { - min-width: 3.5em; + min-width: 3em; } .right-arrow { @@ -275,6 +270,23 @@ } } +.filter-pipeline { + .horizontal-line, + .right-arrow { + width: 78/12em; + } +} + +.right-arrow:has(+ .filter-wrapper .config-filter.empty-filter) { + width: 156/12em +} + +.filter-pipeline:has(.filter-wrapper .config-filter.empty-filter) { + .horizontal-line { + width: 168/12em; + } +} + .config-filter.empty-filter:after { content: 'Filter'; display: block; From 64187b6979175df593c8dadca81cff3facc292ba Mon Sep 17 00:00:00 2001 From: raviks789 Date: Thu, 16 May 2024 17:52:03 +0200 Subject: [PATCH 08/14] Move creation of form submit buttons to EventRuleConfigForm --- .../controllers/EventRuleController.php | 49 +-------------- application/forms/EventRuleConfigForm.php | 60 +++++++++++++++++++ 2 files changed, 62 insertions(+), 47 deletions(-) diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php index fdfa7569..118d23a7 100644 --- a/application/controllers/EventRuleController.php +++ b/application/controllers/EventRuleController.php @@ -10,13 +10,11 @@ use Icinga\Module\Notifications\Forms\EventRuleConfigElements\EventRuleConfigFilter; use Icinga\Module\Notifications\Forms\EventRuleConfigForm; use Icinga\Module\Notifications\Forms\EventRuleForm; -use Icinga\Module\Notifications\Model\Incident; use Icinga\Module\Notifications\Model\Rule; use Icinga\Module\Notifications\Web\Control\SearchBar\ExtraTagSuggestions; use Icinga\Web\Notification; use ipl\Html\Attributes; use ipl\Html\Form; -use ipl\Html\FormElement\SubmitButtonElement; use ipl\Html\Html; use ipl\Html\HtmlElement; use ipl\Stdlib\Filter; @@ -97,41 +95,6 @@ function (EventRuleConfigForm $form) use ($ruleId, $eventRuleConfigValues) { ) ->handleRequest($this->getServerRequest()); - $buttonsWrapper = new HtmlElement('div', Attributes::create(['class' => ['icinga-controls', 'save-config']])); - $eventRuleConfigSubmitButton = new SubmitButtonElement( - 'save', - [ - 'label' => t('Save'), - 'form' => 'event-rule-config-form', - ] - ); - - $deleteButton = new SubmitButtonElement( - 'delete', - [ - 'label' => t('Delete'), - 'form' => 'event-rule-config-form', - 'class' => 'btn-remove', - 'formnovalidate' => true - ] - ); - - $buttonsWrapper->addHtml($eventRuleConfigSubmitButton, $deleteButton); - - if ($ruleId > 0) { - $incidentCount = Incident::on(Database::get()) - ->with('rule') - ->filter(Filter::equal('rule.id', $ruleId)) - ->count(); - - if ($incidentCount) { - $deleteButton->addAttributes([ - 'disabled' => true, - 'title' => t('There are active incidents for this event rule and hence cannot be removed') - ]); - } - } - $eventRuleForm = Html::tag('div', ['class' => 'event-rule-form', 'id' => 'event-rule-form'], [ Html::tag('h2', $eventRuleConfigValues['name']), (new Link( @@ -144,7 +107,7 @@ function (EventRuleConfigForm $form) use ($ruleId, $eventRuleConfigValues) { ]); $this->addControl($eventRuleForm); - $this->addControl($buttonsWrapper); + $this->addControl($eventRuleConfig->createFormSubmitButtons()); $this->addContent($eventRuleConfig); } @@ -331,14 +294,6 @@ public function addAction(): void } } - $eventRuleConfigSubmitButton = (new SubmitButtonElement( - 'save', - [ - 'label' => t('Add Event Rule'), - 'form' => 'event-rule-config-form' - ] - ))->setWrapper(new HtmlElement('div', Attributes::create(['class' => ['icinga-controls', 'save-config']]))); - $eventRuleConfig = new EventRuleConfigForm( $eventRule, Url::fromPath( @@ -377,7 +332,7 @@ public function addAction(): void ]); $this->addControl($eventRuleForm); - $this->addControl($eventRuleConfigSubmitButton); + $this->addControl($eventRuleConfig->createFormSubmitButtons()); $this->addContent($eventRuleConfig); } diff --git a/application/forms/EventRuleConfigForm.php b/application/forms/EventRuleConfigForm.php index 6d7730e8..08f59d97 100644 --- a/application/forms/EventRuleConfigForm.php +++ b/application/forms/EventRuleConfigForm.php @@ -18,6 +18,7 @@ use ipl\Html\Form; use ipl\Html\FormElement\SubmitButtonElement; use ipl\Html\HtmlElement; +use ipl\Html\ValidHtml; use ipl\I18n\Translation; use ipl\Stdlib\Filter; use ipl\Stdlib\Filter\Condition; @@ -88,6 +89,65 @@ public function hasZeroConditionEscalation(): bool return $this->hasZeroConditionEscalation; } + /** + * Create the external submit buttons for the event rule config + * + * @return ValidHtml + */ + public function createFormSubmitButtons(): ValidHtml + { + $buttonsWrapper = new HtmlElement('div', Attributes::create(['class' => ['icinga-controls', 'save-config']])); + $ruleId = $this->config['id']; + + if ($ruleId === '-1') { + return (new SubmitButtonElement( + 'save', + [ + 'label' => $this->translate('Add Event Rule'), + 'form' => 'event-rule-config-form', + ] + ))->setWrapper($buttonsWrapper); + } else { + $saveButton = new SubmitButtonElement( + 'save', + [ + 'label' => $this->translate('Save'), + 'form' => 'event-rule-config-form', + ] + ); + + $deleteButton = new SubmitButtonElement( + 'delete', + [ + 'label' => $this->translate('Delete'), + 'form' => 'event-rule-config-form', + 'class' => 'btn-remove', + 'formnovalidate' => true + ] + ); + + $buttonsWrapper->addHtml($saveButton, $deleteButton); + + if ((int) $ruleId > 0) { + $incidentCount = Incident::on(Database::get()) + ->with('rule') + ->filter(Filter::equal('rule.id', $ruleId)) + ->count(); + + if ($incidentCount) { + $deleteButton->addAttributes([ + 'disabled' => true, + 'title' => $this->translate( + 'There are active incidents for this event rule and hence cannot be removed' + ) + ]); + } + } + + return $buttonsWrapper; + } + } + protected function assemble(): void { $this->addElement($this->createCsrfCounterMeasure(Session::getSession()->getId())); From 6e871e4877b3a0492edee565329023eda896a236 Mon Sep 17 00:00:00 2001 From: raviks789 Date: Fri, 17 May 2024 15:27:44 +0200 Subject: [PATCH 09/14] EventRuleForm: Save new event rule to the database on submission --- .../controllers/EventRuleController.php | 86 +++-------------- application/forms/EventRuleConfigForm.php | 95 +++++++------------ 2 files changed, 51 insertions(+), 130 deletions(-) diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php index 118d23a7..81cf42e6 100644 --- a/application/controllers/EventRuleController.php +++ b/application/controllers/EventRuleController.php @@ -74,7 +74,7 @@ public function indexAction(): void return; } - $form->addOrUpdateRule($ruleId, $diff); + $form->updateRule($ruleId, $diff); Notification::success(sprintf( t('Successfully saved event rule %s'), $eventRuleConfigValues['name'] @@ -274,68 +274,6 @@ public static function createFilterString(Filter\Rule $filters): ?string return $filterStr !== '' ? rawurldecode($filterStr) : null; } - public function addAction(): void - { - $this->addTitleTab(t('Add Event Rule')); - $this->getTabs()->setRefreshUrl(Url::fromRequest()); - - // Add an empty container and set it as the X-Icinga-Container when sending extra updates - // from the modal for filter or event rule - $this->addContent(Html::tag('div', ['class' => 'container', 'id' => 'dummy-event-rule-container'])); - - $this->controls->addAttributes(['class' => 'event-rule-detail']); - $ruleId = $this->params->get('id'); - - if ($this->getRequest()->isPost()) { - if ($this->getRequest()->has('searchbar')) { - $eventRule['object_filter'] = $this->getRequest()->get('searchbar'); - } else { - $eventRule['object_filter'] = null; - } - } - - $eventRuleConfig = new EventRuleConfigForm( - $eventRule, - Url::fromPath( - 'notifications/event-rule/search-editor', - ['id' => $ruleId, 'object_filter' => $eventRule['object_filter']] - ) - ); - - $eventRuleConfig - ->on(Form::ON_SUCCESS, function (EventRuleConfigForm $form) use ($eventRule) { - $eventRuleConfig = array_merge($eventRule, $form->getValues()); - $ruleId = $form->addOrUpdateRule((int) $eventRule['id'], $eventRuleConfig); - Notification::success(sprintf(t('Successfully add event rule %s'), $eventRule['name'])); - - $this->sendExtraUpdates(['#col1']); - $this->redirectNow(Links::eventRule($ruleId)); - }) - ->handleRequest($this->getServerRequest()); - - $eventRuleForm = Html::tag('div', ['class' => 'event-rule-form'], [ - Html::tag('h2', $eventRule['name'] ?? ''), - Html::tag( - 'div', - [ - 'class' => 'not-allowed', - 'title' => $this->translate('Cannot edit event rule until it is saved to database') - ], - (new Link( - new Icon('edit'), - Url::fromPath('notifications/event-rule/edit', [ - 'id' => -1 - ]), - ['class' => ['control-button', 'disabled']] - ))->openInModal() - ) - ]); - - $this->addControl($eventRuleForm); - $this->addControl($eventRuleConfig->createFormSubmitButtons()); - $this->addContent($eventRuleConfig); - } - public function editAction(): void { /** @var string $ruleId */ @@ -353,18 +291,24 @@ public function editAction(): void $eventRuleForm = (new EventRuleForm()) ->populate($config) ->setAction(Url::fromRequest()->getAbsoluteUrl()) - ->on(Form::ON_SUCCESS, function ($form) use ($ruleId, $config, $db) { - $config['name'] = $form->getValue('name'); - $config['is_active'] = $form->getValue('is_active'); - + ->on(Form::ON_SUCCESS, function ($form) use ($ruleId, $db) { if ($ruleId === '-1') { - $redirectUrl = Url::fromPath('notifications/event-rules/add', ['id' => '-1']); + $db->insert('rule', [ + 'name' => $form->getValue('name'), + 'timeperiod_id' => null, + 'object_filter' => null, + 'is_active' => $form->getValue('is_active') + ]); + + $id = $db->lastInsertId(); + $this->getResponse()->setHeader('X-Icinga-Container', 'col2'); - $this->redirectNow($redirectUrl); + $this->sendExtraUpdates(['#col1']); + $this->redirectNow(Links::eventRule($id)); } else { $db->update('rule', [ - 'name' => $config['name'], - 'is_active' => $config['is_active'] ?? 'n' + 'name' => $form->getValue('name'), + 'is_active' => $form->getValue('is_active') ], ['id = ?' => $ruleId]); $this->sendExtraUpdates([ diff --git a/application/forms/EventRuleConfigForm.php b/application/forms/EventRuleConfigForm.php index 08f59d97..2c330512 100644 --- a/application/forms/EventRuleConfigForm.php +++ b/application/forms/EventRuleConfigForm.php @@ -99,53 +99,43 @@ public function createFormSubmitButtons(): ValidHtml $buttonsWrapper = new HtmlElement('div', Attributes::create(['class' => ['icinga-controls', 'save-config']])); $ruleId = $this->config['id']; - if ($ruleId === '-1') { - return (new SubmitButtonElement( - 'save', - [ - 'label' => $this->translate('Add Event Rule'), - 'form' => 'event-rule-config-form', - ] - ))->setWrapper($buttonsWrapper); - } else { - $saveButton = new SubmitButtonElement( - 'save', - [ - 'label' => $this->translate('Save'), - 'form' => 'event-rule-config-form', - ] - ); + $saveButton = new SubmitButtonElement( + 'save', + [ + 'label' => $this->translate('Save'), + 'form' => 'event-rule-config-form', + ] + ); - $deleteButton = new SubmitButtonElement( - 'delete', - [ - 'label' => $this->translate('Delete'), - 'form' => 'event-rule-config-form', - 'class' => 'btn-remove', - 'formnovalidate' => true - ] - ); + $deleteButton = new SubmitButtonElement( + 'delete', + [ + 'label' => $this->translate('Delete'), + 'form' => 'event-rule-config-form', + 'class' => 'btn-remove', + 'formnovalidate' => true + ] + ); - $buttonsWrapper->addHtml($saveButton, $deleteButton); - - if ((int) $ruleId > 0) { - $incidentCount = Incident::on(Database::get()) - ->with('rule') - ->filter(Filter::equal('rule.id', $ruleId)) - ->count(); - - if ($incidentCount) { - $deleteButton->addAttributes([ - 'disabled' => true, - 'title' => $this->translate( - 'There are active incidents for this event rule and hence cannot be removed' - ) - ]); - } - } + $buttonsWrapper->addHtml($saveButton, $deleteButton); + + if ((int) $ruleId > 0) { + $incidentCount = Incident::on(Database::get()) + ->with('rule') + ->filter(Filter::equal('rule.id', $ruleId)) + ->count(); - return $buttonsWrapper; + if ($incidentCount) { + $deleteButton->addAttributes([ + 'disabled' => true, + 'title' => $this->translate( + 'There are active incidents for this event rule and hence cannot be removed' + ) + ]); + } } + + return $buttonsWrapper; } protected function assemble(): void @@ -469,24 +459,11 @@ protected function createRemoveButton(string $prefix): SubmitButtonElement * * @return int */ - public function addOrUpdateRule(int $id, array $config): int + public function updateRule(int $id, array $config): int { $db = Database::get(); $db->beginTransaction(); - if ($id < 0) { - $db->insert('rule', [ - 'name' => $config['name'], - 'timeperiod_id' => $config['timeperiod_id'] ?? null, - 'object_filter' => $config['object_filter'] ?? null, - 'is_active' => $config['is_active'] ?? 'n' - ]); - - $id = $db->lastInsertId(); - } else { - $db->update('rule', [ - 'object_filter' => $config['object_filter'] ?? null, - ], ['id = ?' => $id]); - } + $db->update('rule', ['object_filter' => $config['object_filter'] ?? null], ['id = ?' => $id]); if (! isset($config['rule_escalation'])) { $db->commitTransaction(); @@ -542,7 +519,7 @@ public function addOrUpdateRule(int $id, array $config): int $db->commitTransaction(); - return (int) $id; + return $id; } /** From 62cd26fcc6b0cd3a46fe39ad0f5eb59faa5a9a43 Mon Sep 17 00:00:00 2001 From: raviks789 Date: Tue, 21 May 2024 17:24:10 +0200 Subject: [PATCH 10/14] Make event rule delete action CSRF protected --- application/controllers/EventRuleController.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php index 81cf42e6..a781a9e2 100644 --- a/application/controllers/EventRuleController.php +++ b/application/controllers/EventRuleController.php @@ -86,11 +86,15 @@ public function indexAction(): void ->on( EventRuleConfigForm::ON_DELETE, function (EventRuleConfigForm $form) use ($ruleId, $eventRuleConfigValues) { - $form->removeRule((int) $ruleId); - Notification::success( - sprintf(t('Successfully deleted event rule %s'), $eventRuleConfigValues['name']) - ); - $this->redirectNow('__CLOSE__'); + $csrf = $form->getElement('CSRFToken'); + if ($csrf !== null && $csrf->isValid()) { + $form->removeRule((int) $ruleId); + Notification::success( + sprintf(t('Successfully deleted event rule %s'), $eventRuleConfigValues['name']) + ); + + $this->redirectNow('__CLOSE__'); + } } ) ->handleRequest($this->getServerRequest()); From a5b1d1aaf6f8cd7c0844bbbe636ad3ffe9de9c03 Mon Sep 17 00:00:00 2001 From: raviks789 Date: Tue, 21 May 2024 17:26:01 +0200 Subject: [PATCH 11/14] EventRulesController: Make `getFilter` method private --- .../controllers/EventRulesController.php | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/application/controllers/EventRulesController.php b/application/controllers/EventRulesController.php index efbee30a..530dcc8c 100644 --- a/application/controllers/EventRulesController.php +++ b/application/controllers/EventRulesController.php @@ -84,20 +84,6 @@ public function indexAction(): void $this->getTabs()->activate('event-rules'); } - /** - * Get the filter created from query string parameters - * - * @return Filter\Rule - */ - protected function getFilter(): Filter\Rule - { - if ($this->filter === null) { - $this->filter = QueryString::parse((string) $this->params); - } - - return $this->filter; - } - public function getTabs(): Tabs { if ($this->getRequest()->getActionName() === 'index') { @@ -123,4 +109,18 @@ public function getTabs(): Tabs return parent::getTabs(); } + + /** + * Get the filter created from query string parameters + * + * @return Filter\Rule + */ + private function getFilter(): Filter\Rule + { + if ($this->filter === null) { + $this->filter = QueryString::parse((string) $this->params); + } + + return $this->filter; + } } From e8c6546cf37808c4c4d7fc9948e9e481b327444c Mon Sep 17 00:00:00 2001 From: raviks789 Date: Tue, 4 Jun 2024 11:39:39 +0200 Subject: [PATCH 12/14] Fix: Remove false positive due to not checking object_filter changes --- application/controllers/EventRuleController.php | 5 +++-- application/forms/EventRuleConfigForm.php | 11 ++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php index a781a9e2..d51afa77 100644 --- a/application/controllers/EventRuleController.php +++ b/application/controllers/EventRuleController.php @@ -48,7 +48,7 @@ public function indexAction(): void $ruleId = $this->params->getRequired('id'); $eventRuleConfigValues = $this->fromDb((int) $ruleId); - $filter = &$eventRuleConfigValues['object_filter']; // Assignment by reference to is used as search editor is a + $filter = $eventRuleConfigValues['object_filter']; // Assignment by reference to is used as search editor is a // different form and the config must have the updated // object_filter as soon as the search editor is closed @@ -65,7 +65,8 @@ public function indexAction(): void Url::fromPath( 'notifications/event-rule/search-editor', ['id' => $ruleId, 'object_filter' => $filter] - ) + ), + $filter )) ->populate($eventRuleConfigValues) ->on(Form::ON_SUCCESS, function (EventRuleConfigForm $form) use ($ruleId, $eventRuleConfigValues) { diff --git a/application/forms/EventRuleConfigForm.php b/application/forms/EventRuleConfigForm.php index 2c330512..ee4680e1 100644 --- a/application/forms/EventRuleConfigForm.php +++ b/application/forms/EventRuleConfigForm.php @@ -50,16 +50,20 @@ class EventRuleConfigForm extends Form /** @var bool Whether the config has an escalation with no condition */ protected $hasZeroConditionEscalation = false; + /** @var ?string */ + protected $objectFilter; + /** * Create a new EventRuleConfigForm * * @param array $config * @param Url $searchEditorUrl */ - public function __construct(array $config, Url $searchEditorUrl) + public function __construct(array $config, Url $searchEditorUrl, ?string $objectFilter) { $this->config = $config; $this->searchEditorUrl = $searchEditorUrl; + $this->objectFilter = $objectFilter; } public function hasBeenSubmitted(): bool @@ -179,7 +183,7 @@ protected function assemble(): void $this->getElement('zero-condition-escalation')->setValue($defaultEscalationPrefix); } - $configFilter = new EventRuleConfigFilter($this->searchEditorUrl, $this->config['object_filter']); + $configFilter = new EventRuleConfigFilter($this->searchEditorUrl, $this->objectFilter); $this->registerElement($configFilter); $addEscalationButton = new SubmitButtonElement( @@ -463,7 +467,8 @@ public function updateRule(int $id, array $config): int { $db = Database::get(); $db->beginTransaction(); - $db->update('rule', ['object_filter' => $config['object_filter'] ?? null], ['id = ?' => $id]); + + $db->update('rule', ['object_filter' => $this->objectFilter], ['id = ?' => $id]); if (! isset($config['rule_escalation'])) { $db->commitTransaction(); From 6f5a77a216c2d2a9211d095d8fa1eab0866401cb Mon Sep 17 00:00:00 2001 From: raviks789 Date: Tue, 4 Jun 2024 11:43:39 +0200 Subject: [PATCH 13/14] Fix: Review comments --- .../controllers/EventRuleController.php | 21 +++++-------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/application/controllers/EventRuleController.php b/application/controllers/EventRuleController.php index d51afa77..c47254ab 100644 --- a/application/controllers/EventRuleController.php +++ b/application/controllers/EventRuleController.php @@ -48,9 +48,8 @@ public function indexAction(): void $ruleId = $this->params->getRequired('id'); $eventRuleConfigValues = $this->fromDb((int) $ruleId); - $filter = $eventRuleConfigValues['object_filter']; // Assignment by reference to is used as search editor is a - // different form and the config must have the updated - // object_filter as soon as the search editor is closed + + $filter = $eventRuleConfigValues['object_filter']; if ($this->getRequest()->isPost()) { if ($this->getRequest()->has('searchbar')) { @@ -296,26 +295,16 @@ public function editAction(): void $eventRuleForm = (new EventRuleForm()) ->populate($config) ->setAction(Url::fromRequest()->getAbsoluteUrl()) - ->on(Form::ON_SUCCESS, function ($form) use ($ruleId, $db) { + ->on(Form::ON_SUCCESS, function (EventRuleForm $form) use ($ruleId, $db) { if ($ruleId === '-1') { - $db->insert('rule', [ - 'name' => $form->getValue('name'), - 'timeperiod_id' => null, - 'object_filter' => null, - 'is_active' => $form->getValue('is_active') - ]); - + $db->insert('rule', $form->getValues()); $id = $db->lastInsertId(); $this->getResponse()->setHeader('X-Icinga-Container', 'col2'); $this->sendExtraUpdates(['#col1']); $this->redirectNow(Links::eventRule($id)); } else { - $db->update('rule', [ - 'name' => $form->getValue('name'), - 'is_active' => $form->getValue('is_active') - ], ['id = ?' => $ruleId]); - + $db->update('rule', $form->getValues(), ['id = ?' => $ruleId]); $this->sendExtraUpdates([ '#event-rule-form' => Url::fromPath( 'notifications/event-rule/rule', ['id' => $ruleId] From 6d24f324fcd44d0e3ccacc7c89276ba18365fbc3 Mon Sep 17 00:00:00 2001 From: raviks789 Date: Fri, 7 Jun 2024 17:08:58 +0200 Subject: [PATCH 14/14] EventRuleConfigForm: Consider escalation deletion while checking for changes --- application/forms/EventRuleConfigForm.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/application/forms/EventRuleConfigForm.php b/application/forms/EventRuleConfigForm.php index ee4680e1..8553f719 100644 --- a/application/forms/EventRuleConfigForm.php +++ b/application/forms/EventRuleConfigForm.php @@ -657,6 +657,15 @@ public function getChanges(): array $values = $this->getValues(); $dbValuesToCompare = array_intersect_key($this->config, $values); + if (count($values, COUNT_RECURSIVE) < count($dbValuesToCompare, COUNT_RECURSIVE)) { + // fewer values in the form than in the db, escalation(s) has been removed + if ($values['object_filter'] === $dbValuesToCompare['object_filter']) { + unset($values['object_filter']); + } + + return $values; + } + $checker = static function ($a, $b) use (&$checker) { if (! is_array($a) || ! is_array($b)) { return $a <=> $b;