Skip to content
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

[RAM] Update Rule Status - UI Changes #144466

Merged
merged 58 commits into from
Nov 15, 2022

Conversation

JiaweiWu
Copy link
Contributor

@JiaweiWu JiaweiWu commented Nov 2, 2022

Summary

Parent issue for updating rule status: #136039
Frontend issue: #145191

Backend PR: #140882

Updates the rules list and rules details page to support the new consolidated statuses. With E2E and unit testing.

Rules list:

  • Table cell values
  • Last response filter
  • Table cell filtering
  • Status aggregations

Rule details:

  • Rule status summary
  • KPI headers renaming
  • Event log cells renaming

dashdash

rule_details_consolidate

Checklist

@JiaweiWu JiaweiWu added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) ci:cloud-deploy Create or update a Cloud deployment labels Nov 2, 2022
@JiaweiWu JiaweiWu mentioned this pull request Nov 2, 2022
2 tasks
@EricDavisX
Copy link
Contributor

EricDavisX commented Nov 3, 2022

Initial tests don't reveal anything yet. Just confirming some tests are underway. :)

@XavierM

This comment was marked as resolved.

@@ -465,7 +490,7 @@ export const RulesList = ({
setShowErrors((prevValue) => {
if (!prevValue) {
const rulesToExpand = rulesState.data.reduce((acc, ruleItem) => {
if (ruleItem.executionStatus.status === 'error') {
if (ruleItem.lastRun?.outcome === RuleLastRunOutcomeValues[2]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check the feature flag and have two conditions instead of one or just a OR

});
await refreshAlertsList();
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
await testSubjects.click('ruleExecutionStatusFilterButton');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JiaweiWu and I talked more about it and we think we should add a new config file to allow this test working when the feature flag is off. We will create an issue to add back this test until the feature flag exists

@JiaweiWu
Copy link
Contributor Author

JiaweiWu commented Nov 15, 2022

While testing the experimental flags I noticed we have a small bug in our experimental features getters. I will create an issue and work on that immediately after FF (#145300)

@JiaweiWu
Copy link
Contributor Author

@elasticmachine merge upstream

@JiaweiWu JiaweiWu enabled auto-merge (squash) November 15, 2022 21:37
Copy link
Contributor

@XavierM XavierM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, so glad to see this PR getting merged! Thanks a lot for your help on this one!

@JiaweiWu JiaweiWu removed the ci:cloud-deploy Create or update a Cloud deployment label Nov 15, 2022
@JiaweiWu
Copy link
Contributor Author

@elasticmachine merge upstream

@JiaweiWu
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
triggersActionsUi 461 466 +5

Async chunks

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

id before after diff
triggersActionsUi 665.6KB 647.9KB -17.8KB

Page load bundle

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

id before after diff
triggersActionsUi 104.0KB 114.1KB +10.1KB
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 108 113 +5
securitySolution 441 447 +6
total +19

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 67 73 +6
osquery 109 115 +6
securitySolution 518 524 +6
total +20

History

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

@JiaweiWu JiaweiWu merged commit 256e1f8 into elastic:main Nov 15, 2022
jcger pushed a commit that referenced this pull request Nov 16, 2022
## Summary
In the PR that updated and consolidated rule status
(#144466). The new helper that was
added had references to translations, which increased the
`triggers_actions_ui` bundle size by quite a bit.

This PR refactors the helper so we're passing in the translations. It
seems like a good compromise to allow some logic reuse but without the
massive bundle increase.

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.6 candidate backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants