[Uptime] Filters in create alert flyout#64753
Conversation
|
Pinging @elastic/uptime (Team:uptime) |
justinkambic
left a comment
There was a problem hiding this comment.
I had a few minor comments about code. In addition there were a few functionality-related things I wanted to voice.
First - in a case where the user uses the statusFilter on the Overview page to filter by up, and then uses the search field in the AddAlert flyout to select a monitor whose status is down (or the reverse), our new monitor counter will show 0 monitors.
Another thing to consider is how we want the page to behave when other filtering criteria changes the values we display for filtering. For instance, I chose a couple of ports to filter on, then updated the search text, and now I seem to be unable to deselect one of the ports:
Lastly, I'm having some difficulty getting the Create alert button at the bottom of the flyout to enable. This might be related to some validation issue signaling the flyout to not allow the user to submit.
x-pack/plugins/uptime/public/components/overview/alerts/add_filter_btn.tsx
Outdated
Show resolved
Hide resolved
...gins/uptime/public/components/overview/alerts/monitor_expressions/time_expression_select.tsx
Outdated
Show resolved
Hide resolved
...gins/uptime/public/components/overview/alerts/monitor_expressions/time_expression_select.tsx
Outdated
Show resolved
Hide resolved
...gins/uptime/public/components/overview/alerts/monitor_expressions/time_expression_select.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/uptime/public/components/overview/filter_group/filter_popover.tsx
Show resolved
Hide resolved
x-pack/plugins/uptime/public/lib/alert_types/monitor_status_title.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/uptime/public/lib/alert_types/monitor_status_title.tsx
Outdated
Show resolved
Hide resolved
justinkambic
left a comment
There was a problem hiding this comment.
Ok, this LGTM now! The functional concerns I mentioned have been resolved, and there's a follow-up PR #65049 to address the problem with filters/status filter clashing.
|
@shahzad31 I hope you don't mind, I noticed there were a couple conflicts in this PR so I pushed a commit resolving them 🤞 |
|
@shahzad31 I also pushed a tiny commit fixing the casing of the word |
andrewvc
left a comment
There was a problem hiding this comment.
@shahzad31 I'm LGTM on this if my last two commits look good to you
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |


Summary
Fixes: elastic/uptime#163
Displayed currently selected filter in create alert and added ability to change filters