[8.0][RAC] 117686 replace alert workflow status in alerts view #118723

fkanout · 2021-11-16T16:03:41Z

Summary

This PR fixes #118078, fixes #117686 by:

Replace the "Workflow status" EuiButtonGroup to show "Alert status" instead. i.e. "Open / Acknowledged / Closed", we'll have "Show all / Active / Recovered" instead.

Hide "Mark as Acknowledged", "Mark as Closed", and "Mark as Open" from the action list in the alerts table.
~~Hide the alerts' leading checkboxes that provide bulk actions with "Mark as Acknowledged", "Mark as Closed", and "Mark as Open".~~
This PR can't fulfill "hide the alerts' leading checkboxes" requirements, as there is a bug in the T Grid. (please check this issue for more details).
Keep sync the selected alert filter buttons with the rest of the Alerts page (see the expected behavior section).

Expected behavior:

To ease the PR test for the reviewer, here are the use cases that could be tested:

Change the alert filter where the KQL bar is empty

Change the alert filter where the KQL bar has already one manually-added query

Keep the alert buttons filter in sync even with a cold start of the Alerts page (from browser URL bar)

Keep in sync, when adding manually a query first, then add an alert filter.

Keep in sync, when adding an alert filter first, then manually add a query

Detect the filter alert when writing the alert query manually in the KQL bar

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

…ow status

…lumns props

fkanout · 2021-11-19T14:18:57Z

@elasticmachine merge upstream

…ts-view

fkanout · 2021-11-22T09:50:44Z

@elasticmachine merge upstream

kibanamachine · 2021-11-22T09:50:45Z

merge conflict between base and head

…ts-view

mgiota · 2021-11-24T17:46:54Z

x-pack/plugins/observability/public/pages/alerts/alerts_table_t_grid.tsx

@@ -330,7 +320,7 @@ const FIELDS_WITHOUT_CELL_ACTIONS = [
 ];

 export function AlertsTableTGrid(props: AlertsTableTGridProps) {
-  const { indexNames, rangeFrom, rangeTo, kuery, workflowStatus, setRefetch, addToQuery } = props;
+  const { indexNames, rangeFrom, rangeTo, kuery, workflowStatus, setRefetch } = props;


@fkanout We should delete workflowStatus from here. There are probably a few more places that workflowStatus might need to be deleted as well. This is not needed, since we are not filtering by workflowStatus anymore in the UI. That's why we still see workflowStatus:open in the urlbar when we first load the page. This is not needed anymore

Same as here:
#118723 (comment)

Removing workflowStatus It will blocked by TGridStandaloneCompProps requires filterStatus. Because the value of filterStatus is workflowStatus

@fkanout filterStatus at the moment has value workflowStatus, but nothing prevents us by changing it back to alert status. This change was introduced as part of this PR #109817, so we could pair together to manually revert some workflows status leftover that is not needed

Let's get this PR merged for now. I summarized later on in a comment what we discussed during our sync up.

mgiota · 2021-11-24T20:55:36Z

@fkanout Great work on that PR and thanks a lot for the detailed use cases! I like how you synced the search bar with the url bar. I still see workflowStatus:open in the urlbar though. Was it left on purpose?

I understand that you left a few parts in the code on purpose, in case we bring back workflow status filtering. Let's quickly sync up tomorrow regarding what things can stay and what could be removed (for example workflowStatus from the urlbar).

mgiota · 2021-11-24T20:58:07Z

@ersin-erdal I totally agree. Could you maybe create a ticket to track down this code improvement you suggested? We could add the label technical-debt or refactoring in this ticket.

mgiota · 2021-11-24T21:13:43Z

Btw FYI here's the PR that replaced status filtering with workflow status filtering https://github.com/elastic/kibana/pull/109817/files. And now we are replacing it again. Maybe this is a hint that we should have an AlertsFilter container as @ersin-erdal suggested

fkanout · 2021-11-25T17:28:26Z

@elasticmachine merge upstream

…ts-view

fkanout · 2021-11-29T08:57:03Z

@elasticmachine merge upstream

…ts-view

kibana-ci · 2021-11-29T10:13:03Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`observability`	330	328	-2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`observability`	352.9KB	351.8KB	-1.1KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`observability`	80.2KB	80.0KB	-139.0B

History

💛 Build #9376 was flaky 8ada6ae
💛 Build #9302 was flaky 029bbea
💚 Build #9111 succeeded 2a0774f
💚 Build #8796 succeeded 5b55600
💔 Build #8747 failed 3ade606
💚 Build #8701 succeeded 963b34e

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @fkanout

mgiota · 2021-11-29T11:47:43Z

@fkanout As discussed we can merge this PR. I summarize what we discussed:

We agreed to get this PR merged as it is, so that we don't block @ersin-erdal with his PR.
we agreed to discuss into today's team meeting how we deal with situations where product requires us to keep some code in our codebase that could possibly be reused in the future, but on the other hand makes the code harder to read
based on team's feedback as well, we might need to follow up with another PR to clean up the code

fkanout · 2021-11-29T16:57:39Z

#118723 (comment)

After team discussion, a separate issue will be created to clean up the workflow code.

kibanamachine · 2021-12-01T12:04:59Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 118723 or prevent reminders by adding the backport:skip label.

…ic#118723) * Add AlertStatus types * Add alert status filter component * Remove Filter in action from the t grid table * Update group buttons to applied Alert status filter instead of Workflow status * Keep the Alert status button in sync when typing and first page load * Fix data test object name and translation keys label * Add possibility to hide the bulk actions * Update how hide the bulk actions * Fix showCheckboxes hardcoded "true". Instead use the leadingControlColumns props * Hide the leading checkboxes in the T Grid with the bulk actions * Update showCheckboxes to false * Fix test as the leading checkboxes are hidden * Update tests * Get back disabledCellActions as it's required by T Grid * Update tests to skip test related to Workflow action buttons * Skip workflow tests * Revert fix showCheckboxes * Remove unused imports * Revert the o11y tests as the checkBoxes fix is reverted * Reactive the tests effected by checkBoxes * Skip alert workflow status * [Code review] use predefined types * Remove unused prop * Use the alert-data index name in the RegEx * Detect * in KQL as "show al"l alert filter Co-authored-by: Kibana Machine <[email protected]>

…) (#120198) * Add AlertStatus types * Add alert status filter component * Remove Filter in action from the t grid table * Update group buttons to applied Alert status filter instead of Workflow status * Keep the Alert status button in sync when typing and first page load * Fix data test object name and translation keys label * Add possibility to hide the bulk actions * Update how hide the bulk actions * Fix showCheckboxes hardcoded "true". Instead use the leadingControlColumns props * Hide the leading checkboxes in the T Grid with the bulk actions * Update showCheckboxes to false * Fix test as the leading checkboxes are hidden * Update tests * Get back disabledCellActions as it's required by T Grid * Update tests to skip test related to Workflow action buttons * Skip workflow tests * Revert fix showCheckboxes * Remove unused imports * Revert the o11y tests as the checkBoxes fix is reverted * Reactive the tests effected by checkBoxes * Skip alert workflow status * [Code review] use predefined types * Remove unused prop * Use the alert-data index name in the RegEx * Detect * in KQL as "show al"l alert filter Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

…ic#118723) * Add AlertStatus types * Add alert status filter component * Remove Filter in action from the t grid table * Update group buttons to applied Alert status filter instead of Workflow status * Keep the Alert status button in sync when typing and first page load * Fix data test object name and translation keys label * Add possibility to hide the bulk actions * Update how hide the bulk actions * Fix showCheckboxes hardcoded "true". Instead use the leadingControlColumns props * Hide the leading checkboxes in the T Grid with the bulk actions * Update showCheckboxes to false * Fix test as the leading checkboxes are hidden * Update tests * Get back disabledCellActions as it's required by T Grid * Update tests to skip test related to Workflow action buttons * Skip workflow tests * Revert fix showCheckboxes * Remove unused imports * Revert the o11y tests as the checkBoxes fix is reverted * Reactive the tests effected by checkBoxes * Skip alert workflow status * [Code review] use predefined types * Remove unused prop * Use the alert-data index name in the RegEx * Detect * in KQL as "show al"l alert filter Co-authored-by: Kibana Machine <[email protected]>

fkanout added v8.0.0 Theme: rac label obsolete Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" labels Nov 16, 2021

fkanout force-pushed the 117686-replace-alert-workflow-status-in-alerts-view branch 3 times, most recently from a8a8e8f to 553dc0c Compare November 17, 2021 14:56

fkanout added 5 commits November 17, 2021 16:00

Add AlertStatus types

69091b4

Add alert status filter component

38f1829

Remove Filter in action from the t grid table

f4cb043

Update group buttons to applied Alert status filter instead of Workfl…

7880781

…ow status

Keep the Alert status button in sync when typing and first page load

b17d8db

fkanout force-pushed the 117686-replace-alert-workflow-status-in-alerts-view branch from 553dc0c to b17d8db Compare November 17, 2021 15:01

fkanout added 6 commits November 17, 2021 17:11

Fix data test object name and translation keys label

ec3be8c

Add possibility to hide the bulk actions

391945b

Update how hide the bulk actions

3918b8d

Fix showCheckboxes hardcoded "true". Instead use the leadingControlCo…

2fb0b01

…lumns props

Hide the leading checkboxes in the T Grid with the bulk actions

b4f8df4

Update showCheckboxes to false

80132a3

fkanout mentioned this pull request Nov 19, 2021

Replace alert workflow status (open/acknowledged/closed) and filter with alert status filter (active/recovered) in Alerts View #117686

Closed

fkanout force-pushed the 117686-replace-alert-workflow-status-in-alerts-view branch from 2c609ef to 80132a3 Compare November 19, 2021 13:43

kibanamachine and others added 3 commits November 19, 2021 09:19

Merge branch 'main' into 117686-replace-alert-workflow-status-in-aler…

9b8ef26

…ts-view

Fix test as the leading checkboxes are hidden

67bf04d

Update tests

1b82d80

fkanout added 4 commits November 22, 2021 13:33

Merge branch 'main' into 117686-replace-alert-workflow-status-in-aler…

edca181

…ts-view

Get back disabledCellActions as it's required by T Grid

338da2d

Update tests to skip test related to Workflow action buttons

b1562d6

Skip workflow tests

3e6c8c6

[Code review] use predefined types

8c8f114

mgiota requested changes Nov 24, 2021

View reviewed changes

Remove unused prop

2a0774f

fkanout added 2 commits November 25, 2021 14:26

Use the alert-data index name in the RegEx

6ebbb68

Detect * in KQL as "show al"l alert filter

029bbea

Merge branch 'main' into 117686-replace-alert-workflow-status-in-aler…

8ada6ae

…ts-view

Merge branch 'main' into 117686-replace-alert-workflow-status-in-aler…

01caf34

…ts-view

mgiota self-requested a review November 29, 2021 11:47

mgiota approved these changes Nov 29, 2021

View reviewed changes

fkanout merged commit ee3cb46 into elastic:main Nov 29, 2021

fkanout mentioned this pull request Nov 30, 2021

[RAC] Show all alerts, not only "open" workflow status, and remove Workflow filter related logic and clean up the code #119946

Closed

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 1, 2021

fkanout mentioned this pull request Dec 2, 2021

[8.0] [8.0][RAC] 117686 replace alert workflow status in alerts view (#118723) #120198

Merged

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 2, 2021

fkanout added the v8.1.0 label Dec 2, 2021

mgiota mentioned this pull request Apr 21, 2022

[Infra Monitoring]: add rac to feature privileges for logs and metrics #130762

Merged

elasticmachine unassigned fkanout Oct 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[8.0][RAC] 117686 replace alert workflow status in alerts view #118723

[8.0][RAC] 117686 replace alert workflow status in alerts view #118723

fkanout commented Nov 16, 2021 •

edited

Loading

fkanout commented Nov 19, 2021

fkanout commented Nov 22, 2021

kibanamachine commented Nov 22, 2021

mgiota Nov 24, 2021

fkanout Nov 25, 2021

fkanout Nov 25, 2021

mgiota Nov 29, 2021 •

edited

Loading

mgiota commented Nov 24, 2021

mgiota commented Nov 24, 2021 •

edited

Loading

mgiota commented Nov 24, 2021

fkanout commented Nov 25, 2021

fkanout commented Nov 29, 2021

kibana-ci commented Nov 29, 2021

mgiota commented Nov 29, 2021 •

edited

Loading

fkanout commented Nov 29, 2021 •

edited

Loading

kibanamachine commented Dec 1, 2021

[8.0][RAC] 117686 replace alert workflow status in alerts view #118723

[8.0][RAC] 117686 replace alert workflow status in alerts view #118723

Conversation

fkanout commented Nov 16, 2021 • edited Loading

Summary

Expected behavior:

Checklist

Risk Matrix

For maintainers

fkanout commented Nov 19, 2021

fkanout commented Nov 22, 2021

kibanamachine commented Nov 22, 2021

mgiota Nov 24, 2021

Choose a reason for hiding this comment

fkanout Nov 25, 2021

Choose a reason for hiding this comment

fkanout Nov 25, 2021

Choose a reason for hiding this comment

mgiota Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

mgiota commented Nov 24, 2021

mgiota commented Nov 24, 2021 • edited Loading

mgiota commented Nov 24, 2021

fkanout commented Nov 25, 2021

fkanout commented Nov 29, 2021

kibana-ci commented Nov 29, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

mgiota commented Nov 29, 2021 • edited Loading

fkanout commented Nov 29, 2021 • edited Loading

kibanamachine commented Dec 1, 2021

fkanout commented Nov 16, 2021 •

edited

Loading

mgiota Nov 29, 2021 •

edited

Loading

mgiota commented Nov 24, 2021 •

edited

Loading

mgiota commented Nov 29, 2021 •

edited

Loading

fkanout commented Nov 29, 2021 •

edited

Loading