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.
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
Event rule config enhancement #159
base: main
Are you sure you want to change the base?
Conversation
c40476a
to
8e16e7e
Compare
1c9054f
to
7ab548f
Compare
25b4a12
to
163112c
Compare
97018d5
to
4b1380c
Compare
3516123
to
babd436
Compare
3981ff8
to
dbc97ac
Compare
application/forms/EventRuleConfigElements/EscalationCondition.php
Outdated
Show resolved
Hide resolved
application/forms/EventRuleConfigElements/EscalationCondition.php
Outdated
Show resolved
Hide resolved
application/forms/EventRuleConfigElements/EscalationCondition.php
Outdated
Show resolved
Hide resolved
application/forms/EventRuleConfigElements/EscalationCondition.php
Outdated
Show resolved
Hide resolved
application/forms/EventRuleConfigElements/EscalationRecipient.php
Outdated
Show resolved
Hide resolved
application/forms/EventRuleConfigElements/EscalationRecipient.php
Outdated
Show resolved
Hide resolved
04d545c
to
4101ce6
Compare
$buttonsWrapper->add( | ||
[$eventRuleConfigSubmitButton, $discardChangesButton, $deleteButton] | ||
); | ||
$buttonsWrapper->add([$eventRuleConfigSubmitButton, $discardChangesButton, $deleteButton]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use addHtml
wherever possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use add()
here as $discardChangesButton
could be null. But addHtml
could be use in PR #170, as you do not need that button.
@@ -283,7 +281,7 @@ public static function createFilterString(Filter\Rule $filters): string | |||
|
|||
$filterStr = QueryString::render($filters); | |||
|
|||
return ! empty($filterStr) ? $filterStr : ''; | |||
return ! empty($filterStr) ? rawurldecode($filterStr) : ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change required now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to store and display certain special characters like '/' as it is instead of in its url encoded form. This was earlier done in setObjectString
method in EventRuleObjectFilter
.
$config = iterator_to_array( | ||
Rule::on(Database::get()) | ||
->withColumns(['id']) | ||
->filter(Filter::equal('id', $ruleId)) | ||
->first() | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The iterator_to_array
method is not required here, and please fetch only the required columns. You can use Connection::FetchOne()
to get the result as an array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You do not need to mention the columns, as by default only the columns from rule
table will be fetched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but you don't want all rule columns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is true as well
application/forms/EventRuleConfigElements/EscalationCondition.php
Outdated
Show resolved
Hide resolved
application/forms/EventRuleConfigElements/EscalationCondition.php
Outdated
Show resolved
Hide resolved
application/forms/EventRuleConfigElements/EventRuleConfigFilter.php
Outdated
Show resolved
Hide resolved
application/forms/EventRuleConfigElements/EventRuleConfigFilter.php
Outdated
Show resolved
Hide resolved
application/forms/EventRuleConfigElements/EventRuleConfigFilter.php
Outdated
Show resolved
Hide resolved
bfcbbcf
to
6c81474
Compare
513d3a4
to
6243671
Compare
6243671
to
c2dc61d
Compare
Use a single form for event rule configuration.
Blocked by Icinga/icingaweb2#5190