Handle rule filter serialization#469
Conversation
BastianLedererIcinga
force-pushed
the
rule-filter-handling
branch
2 times, most recently
from
e3da66a to
b8a38fc
Compare
BastianLedererIcinga
force-pushed
the
rule-filter-handling
branch
8 times, most recently
from
d5fe2f5 to
3bf7acb
Compare
Introduce a new version of the `SourceHook` interface for v 1.0
`RuleFilterSuggestions` use the relevant functions of the `V2\SourceHook` to provide suggestions when editing event-rule filters
nilmerg
left a comment
nilmerg
left a comment
There was a problem hiding this comment.
Didn't look at the controller or suggestions yet.
| * | ||
| * @return array<string> | ||
| */ | ||
| public function getJsonPaths(Condition $condition): array; |
There was a problem hiding this comment.
This forces integrations to implement a cache of sorts to prevent issuing unnecessary queries in case a database is required. Please pass over all conditions at once.
| * | ||
| * @return bool | ||
| */ | ||
| public function shouldShowRelationFor(string $column): bool; |
There was a problem hiding this comment.
That's for the integration to decide. Remove this.
| $icon = new Icon('share-nodes'); | ||
|
|
||
| foreach (Hook::all('Notifications/v1/Source') as $hook) { | ||
| foreach (Hook::all('Notifications/v2/Source') as $hook) { |
There was a problem hiding this comment.
I think there's a way to get rid of this loop and getSourceType() in the interface. Right now there can only be a single hook anyway, and that's where Hook::first() comes into play.
Please try to incorporate the source type in the name of the hook for v2. In case of icinga, that'd be: Notifications/v2/IcingaSource
The hook can then be loaded this way:
$hook = Hook::first(sprintf('Notifications/v2/%sSource', \ipl\Stdlib\Str::camel($this->type));Please note that this fundamentally changes how sources are configured. This way the available integrations cannot be known beforehand (before an event came in) so the type field must be a free text field. This collides with #450 somewhat, but that's already in progress and based on this anyway.
Once #461 is a thing, the source configuration is mainly for generic sources as well. So it doesn't hurt either.
| */ | ||
| public function getJson(): string | ||
| { | ||
| $result = ['qs' => (new Renderer($this->filter))->render()]; |
There was a problem hiding this comment.
Please use QueryString::render()
| } | ||
|
|
||
| $result['rules'] = $rules; | ||
| return $result; |
There was a problem hiding this comment.
an empty line before return pls, always unless it's the sole statement in a block.
| * | ||
| * @return string | ||
| */ | ||
| protected function preprocessValue(string $value): string |
There was a problem hiding this comment.
asRegularExpression()
toRegularExpression()
createRegularExpression()
🤔
BastianLedererIcinga
force-pushed
the
rule-filter-handling
branch
from
3a0fa1e to
015bfb8
Compare
2 participants
resolve #466
Introduce
RuleSerializerto create Json from a configured filter.Introduce
Icinga\Module\Notifications\Hook\V2which no longer requiresserializeRuleFilter()andgetRuleFilterTargets()as they are now obsolete.Adjust
EventRuleControllerto skip target selection use the newRuleSerializer.require Icinga/ipl-web#376