Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Event rule config enhancement #159

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Event rule config enhancement #159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2b0614e
Build a single form for the event rule configuration
raviks789 Dec 18, 2023
8bf6fdd
Remove unnecessary type hints
raviks789 Apr 11, 2024
c0658ca
Remove unused files
raviks789 Mar 4, 2024
166cd53
PHPStan: Update baseline
raviks789 Mar 27, 2024
1aa3eef
EventRuleConfigForm: Modify the HTML structure of filter wrapper and …
raviks789 May 15, 2024
c00a2f1
Fix alignment issue for select elements in safari
raviks789 May 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
279 changes: 164 additions & 115 deletions application/controllers/EventRuleController.php
View file
Open in desktop

Large diffs are not rendered by default.

163 changes: 82 additions & 81 deletions application/controllers/EventRulesController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@

use Icinga\Module\Notifications\Common\Database;
use Icinga\Module\Notifications\Common\Links;
use Icinga\Module\Notifications\Forms\SaveEventRuleForm;
use Icinga\Module\Notifications\Forms\EventRuleConfigForm;
use Icinga\Module\Notifications\Model\Rule;
use Icinga\Module\Notifications\Web\Control\SearchBar\ObjectSuggestions;
use Icinga\Module\Notifications\Widget\EventRuleConfig;
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;
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\LimitControl;
use ipl\Web\Control\SearchEditor;
use ipl\Web\Control\SortControl;
use ipl\Web\Filter\QueryString;
use ipl\Web\Url;
use ipl\Web\Widget\ButtonLink;
Expand All @@ -45,6 +45,7 @@ public function init()
public function indexAction(): void
{
$eventRules = Rule::on(Database::get());
$this->sessionNamespace->delete('-1');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I reload the left column while in the right a new rule is about to be created, this clears all my changes made so far.

The session storage should only be reset if absolutely necessary, which is when a new one is being set up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that, as I just saw that the New Event Rule button used to have a parameter with such functionality, you should use a separate action for new event rules. This then should clear the session before writing anything to it upon submit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/And now I noticed that this very action is already the addAction here, but that's used to set up a new rule. i.e. It's not just the endpoint for the very first modal, it's a detail.

I think that there doesn't need to be a distinction between event-rule?id=-1 and event-rule?id!=-1. The event-rule/index route should be able to handle new rules (=-1) and existing ones (!=-1). There's not much difference between them, only a few buttons and event handlers. By transferring the responsibility to create buttons to the form, this is already not part of the action anymore. So that only leaves the events and those are just not emitted in case of a new rule, so it doesn't really hurt that they are defined.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in the end:

event-rules/add is the endpoint for the the New Event Rule button, this resets the session upon submit
event-rule/index is the endpoint to configure new and existing rules
event-rule/edit is the endpoint to edit new and existing rules


$limitControl = $this->createLimitControl();
$paginationControl = $this->createPaginationControl($eventRules);
Expand Down Expand Up @@ -84,7 +85,7 @@ public function indexAction(): void
$this->addContent(
(new ButtonLink(
t('New Event Rule'),
Url::fromPath('notifications/event-rule/edit', ['id' => -1, 'clearCache' => true]),
Url::fromPath('notifications/event-rule/edit', ['id' => -1]),
'plus'
))->openInModal()
->addAttributes(['class' => 'new-event-rule'])
Expand All @@ -103,22 +104,49 @@ public function indexAction(): void
public function addAction(): void
{
$this->addTitleTab(t('Add Event Rule'));
$this->getTabs()->setRefreshUrl(Url::fromPath('notifications/event-rules/add'));
$this->getTabs()->setRefreshUrl(Url::fromPath('notifications/event-rules/add', ['id' => '-1']));

$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->params->has('use_cache') || $this->getServerRequest()->getMethod() !== 'GET') {
$cache = $this->sessionNamespace->get(-1, []);
} else {
$this->sessionNamespace->delete(-1);

$cache = [];
}
$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(
$config,
Url::fromPath(
'notifications/event-rules/search-editor',
['id' => $ruleId]
)
);

$eventRuleConfig = new EventRuleConfig(Url::fromPath('notifications/event-rules/add-search-editor'), $cache);
$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);
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', $eventRuleConfig->getConfig()['name'] ?? ''),
Html::tag('h2', $config['name'] ?? ''),
sukhwinder33445 marked this conversation as resolved.
Show resolved Hide resolved
(new Link(
new Icon('edit'),
Url::fromPath('notifications/event-rule/edit', [
Expand All @@ -128,86 +156,37 @@ public function addAction(): void
))->openInModal()
]);

$saveForm = (new SaveEventRuleForm())
->on(SaveEventRuleForm::ON_SUCCESS, function ($saveForm) use ($eventRuleConfig) {
if (! $eventRuleConfig->isValid()) {
$eventRuleConfig->addAttributes(['class' => 'invalid']);
return;
}

$id = $saveForm->addRule($this->sessionNamespace->get(-1));

Notification::success($this->translate('Successfully added rule.'));
$this->sendExtraUpdates(['#col1']);
$this->redirectNow(Links::eventRule($id));
})->handleRequest($this->getServerRequest());

$eventRuleConfig->on(EventRuleConfig::ON_CHANGE, function ($eventRuleConfig) {
$this->sessionNamespace->set(-1, $eventRuleConfig->getConfig());

$this->redirectNow(Url::fromPath('notifications/event-rules/add', ['use_cache' => true]));
});

foreach ($eventRuleConfig->getForms() as $f) {
$f->handleRequest($this->getServerRequest());

if (! $f->hasBeenSent()) {
// Force validation of populated values in case we display an unsaved rule
$f->validatePartial();
}
}

$eventRuleFormAndSave = Html::tag('div', ['class' => 'event-rule-and-save-forms']);
$eventRuleFormAndSave->add([
$eventRuleForm,
$saveForm
]);

$this->addControl($eventRuleFormAndSave);
$this->addControl($eventRuleForm);
$this->addControl($eventRuleConfigSubmitButton);
$this->addContent($eventRuleConfig);
}

public function completeAction(): void
{
$suggestions = new ObjectSuggestions();
$suggestions->setModel(Rule::class);
$suggestions->forRequest($this->getServerRequest());
$this->getDocument()->add($suggestions);
}

public function searchEditorAction(): void
{
$editor = $this->createSearchEditor(
Rule::on(Database::get()),
[
LimitControl::DEFAULT_LIMIT_PARAM,
SortControl::DEFAULT_SORT_PARAM,
]
);

$this->getDocument()->add($editor);
$this->setTitle($this->translate('Adjust Filter'));
}
$ruleId = $this->params->shiftRequired('id');
$eventRule = $this->sessionNamespace->get($ruleId);

public function addSearchEditorAction(): void
{
$cache = $this->sessionNamespace->get(-1);
if ($eventRule === null) {
$eventRule = ['id' => '-1'];
}

$editor = EventRuleConfig::createSearchEditor()
->setQueryString($cache['object_filter'] ?? '');
$editor = new SearchEditor();

$editor->on(SearchEditor::ON_SUCCESS, function (SearchEditor $form) {
$cache = $this->sessionNamespace->get(-1);
$cache['object_filter'] = EventRuleConfig::createFilterString($form->getFilter());
$objectFilter = $eventRule['object_filter'] ?? '';
$editor->setQueryString($objectFilter)
->setAction(Url::fromRequest()->getAbsoluteUrl())
->setSuggestionUrl(Links::ruleFilterSuggestionUrl($ruleId));

$this->sessionNamespace->set(-1, $cache);
$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-rules/add',
['use_cache' => true]
['id' => $ruleId]
)
);
});
Expand All @@ -218,6 +197,28 @@ public function addSearchEditorAction(): void
$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
*
Expand Down
42 changes: 0 additions & 42 deletions application/forms/AddEscalationForm.php
View file
Open in desktop

This file was deleted.

42 changes: 0 additions & 42 deletions application/forms/AddFilterForm.php
View file
Open in desktop

This file was deleted.

Loading
Loading