Event rule config enhancement

[raviks789]

Use a single form for event rule configuration.

Blocked by Icinga/icingaweb2#5190

[nilmerg]

next: controllers, fieldsets, css

[nilmerg]

next: controllers, css

[nilmerg]

Finito. For now.

I didn't look at CSS, will do this at the very end.

Since the controllers will receive a rather large rework, I didn't look that detailed over them as in the previous reviews. Will do this once they're adjusted.

[sukhwinder33445]

Removing an escalation condition writes '' (empty string) (rule_escalation.condition='') instead of null to the database.

[nilmerg]

Should be done now. Did change nearly everything again, because the most of what was previously there was not usable and the plethora of other open pull requests referring to this one were either obsolete due to it anyway or now integrated here.

Requires Icinga/ipl-html#151

@raviks789 Please have a look and test it.

The tests fail because they require: Icinga/ipl-web#307

[sukhwinder33445]

This is only a quick review. I will test the UI tomorrow.
Some new classes are missing PHPDoc and method return types.

[sukhwinder33445]

As discussed offline, I would suggest to change the order of the buttons to as follows:

This would also provide in some way protection to prevent accidentally clicking on the Delete button instead of Save.

[nilmerg]

It now requires Icinga/ipl-html#153

[sukhwinder33445]

Class RightArrow is not used anywhere and can be removed.

The class FlowLine can also be removed, as the CSS classes added by this class apply exactly the same rules. The getVerticalLine() method is not used. Instead of applying class right-arrow or horizontal-line, you can add class line connector or connector-line. Instead of the right-arrow and horizontal-line class, you can add the something like connector or connector-line class.

[sukhwinder33445]

I'm not sure if this is the expected behavior. It is possible that there are character strings that contain operator symbols. these should then remain as they are and not be separated into column and value.

Before	After

Customvars allows it too.

[nilmerg]

Fixed. Though, now the shown filter shows escaped chars again. I'm not sure whether it's necessary to avoid this here. The object filters will change anyway and when they do, I doubt that such a simple text input will remain. So let's keep that for now.

[sukhwinder33445]

Fixed. Though, now the shown filter shows escaped chars again. I'm not sure whether it's necessary to avoid this here. The object filters will change anyway and when they do, I doubt that such a simple text input will remain. So let's keep that for now.

Now the database entry has the operator as a suffix, is that OK?

[nilmerg]

In case the daemon accepts it, sure :P

[sukhwinder33445]

@oxzi, that should be fine as discussed offline, right?

[oxzi]

In case the daemon accepts it, sure :P

Well, based on the architectural change described in the "Custom variable support in Notifications" document, the event rules will not be evaluated in the Icinga Notifications daemon anymore, but will be moved to the source.

Implementation started in Icinga/icinga-notifications#324 and Icinga/icingadb#998.

Thus, I am a bit puzzled how this PR works with the upcoming changes, especially the intermediate representation mentioned in the docs.

@oxzi, that should be fine as discussed offline, right?

I was unable to review +2,762 −2,829 changes in 34 files, but at least having a suffix is something which works for us 🙃

[nilmerg]

😅