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

[Alerting] Reload the Alerts List when alerts are deleted #73715

Merged
merged 8 commits into from
Aug 5, 2020

Conversation

gmmorris
Copy link
Contributor

@gmmorris gmmorris commented Jul 29, 2020

Summary

Reloads the entire Alerts list when alerts are deleted through the UI.

closes #72372

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@gmmorris gmmorris added Feature:Alerting release_note:fix Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.10.0 v8.0.0 labels Jul 29, 2020
* master:
  [Vega][Inspector] Request panel should show correct names for requests (elastic#73655)
  [Security Solution] Update filter (elastic#73350)
  TSVB Inaccurate Group By (elastic#73683)
  [Vega][Inspect panel] Write tutorials and reference (elastic#73262)
  [ML] Removing node info check for file data viz import (elastic#73717)
  check that pathname has been updated. ignore other parts (elastic#73689)
  [build] rewrite source as transpiled JS later in the process (elastic#73749)
  Fix Snapshot Restore /policies/indices API endpoint on Cloud (elastic#73734)
  skip flaky suite (elastic#69783) (elastic#70043)
  [Security Solution][Exceptions] - Updates exception hooks and viewer (elastic#73588)
  skip failing suite (elastic#58815)
  [Canvas][fatal bug] Fix props confusion in TextStylePicker (elastic#73732)
  [DOCS] Changes level offset of monitoring pages (elastic#73573)
  Added close button to toast notifications by migrating to different API that is more widely used in Kibana and Security solution in particular. (elastic#73662)
  [ML] Transforms/DFA: Change action button size back to 'xs'.
  [Metrics UI] Fix evaluating rate-aggregated alerts when there's no normalized value (elastic#73545)
  [Metrics UI] Fix formatting of values in inventory context.reason (elastic#73155)
  [maps] rename GisMap to MapContainer and convert to TS (elastic#73690)
  [APM] docs: remove watcher documentation  (elastic#73485)
@gmmorris gmmorris marked this pull request as ready for review July 31, 2020 09:03
@gmmorris gmmorris requested a review from a team as a code owner July 31, 2020 09:03
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

Code LGTM. Works as expected

Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

Changes look good so far. The issue is still reproducible when deleting all the items in the last page (need >= 2 pages to reproduce).

@@ -361,11 +362,22 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
});

it('should delete all selection', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Seems like testing for this could be covered in a unit test instead of end to end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can test the implementation of the fix in a unit test, but you can't test the actual behaviour as it relies on hooks and calls being made inside of other components (including EUI).
You could never be sure it's working correctly with just a unit test...

I have a real fear that we're compromising on test quality instead of addressing the scaling issues in our infrastructure. :/

* master: (154 commits)
  [ML] Fix initial plugin's bundle size (elastic#74047)
  [Ingest Manager] prevent crash on unhandled rejection from setupIngestManager (elastic#74300)
  [Logs UI] Correct trial period duration in anomaly splash screen (elastic#74249)
  [Discover] Inline noWhiteSpace function (elastic#74331)
  [DOCS] Add Observability topic (elastic#73041)
  skip flaky suite (elastic#74327)
  [Security Solution][Detections] Fixes Severity Override not matching for Elastic Endpoint Security rule (elastic#74317)
  [Security Solution][Exceptions] - Fixes exceptions builder nested deletion issue and adds unit tests (elastic#74250)
  Fixed Alert details does not update page title and breadcrumb (elastic#74214)
  [src/dev/build] build Kibana Platform bundles from source (elastic#73591)
  [Reporting] Shorten asset path to help CLI FS Watcher (elastic#74185)
  Fix TMS not loaded in legacy maps (elastic#73570)
  [Security Solution] styling for notes' panel (elastic#74274)
  [Security Solution][Tech Debt] cleans up ts-ignore issues and some smaller linter issues  (elastic#74268)
  Make the actions plugin support generics (elastic#71439)
  [Security Solution] Keep original note creator (elastic#74203)
  [CI] Fix xpack kibana build dir in xpack visual regression script
  [CI] Fix baseline_capture job by adding parallel process number back
  [Monitoring] Ensure setup mode works on cloud but only for alerts (elastic#73127)
  [Maps] Custom color ramps should show correctly on the map for mvt layers (elastic#74169)
  ...
* master:
  update docs (elastic#74364)
  [Ingest Node Pipelines] Refactor pipeline simulator code (elastic#72328)
  Add README files for Kibana app plugins (elastic#74277)
  [APM] Average for transaction error rate includes null values (elastic#74345)
  [Ingest Manager] Adjust dataset aggs to use datastream fields instead (elastic#74342)
* master:
  [ML] DF Analytics creation wizard: show link to results (elastic#74025)
@gmmorris
Copy link
Contributor Author

gmmorris commented Aug 5, 2020

Changes look good so far. The issue is still reproducible when deleting all the items in the last page (need >= 2 pages to reproduce).

addressed 👍

Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
triggers_actions_ui 735.7KB +276.0B 735.4KB

History

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

@gmmorris gmmorris merged commit 41e3128 into elastic:master Aug 5, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 5, 2020
)

Reloads the entire Alerts list when alerts are deleted through the UI.
# Conflicts:
#	x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts
gmmorris added a commit that referenced this pull request Aug 5, 2020
…74426)

Reloads the entire Alerts list when alerts are deleted through the UI.
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 6, 2020
* master: (208 commits)
  Observability Overview fix extra basepath prepend for alerting fetch (elastic#74465)
  [Lens] Clean and inline disabling of react-hooks/exhaustive-deps eslint rule (elastic#70010)
  Skip "space with index pattern management disabled" functional test for cloud env (elastic#74073)
  Filter out non-security jobs when collecting Detections telemetry (elastic#74456)
  [Security Solution][Test] Enzyme test for related events button (elastic#74411)
  [SECURITY_SOLUTION] add z-index to get over nav bar (elastic#74427)
  Rename package configs SO to package policies (elastic#74422)
  [DOCS] Add Kibana alerts to Stack Monitoring (elastic#73762)
  skip flaky suite (elastic#71390)
  [ML] DF Analytics: adds functional tests for edit form (elastic#73885)
  Rename agent configs SO to agent policies (elastic#74397)
  [Jenkins] run CI when plugin readmes change (elastic#74388)
  [Metrics UI] Fix validating Metrics Explorer URL (elastic#74311)
  fixing encoding issue with \ for enroll command (elastic#74379)
  [Ingest Manager] Update package registry for testing to f6b01d (elastic#74341)
  Change experimental message for visualizations (elastic#74354)
  [Alerting] Reload the Alerts List when alerts are deleted (elastic#73715)
  [Enterprise Search] Fix/DRY out plugin i18n strings (elastic#74323)
  update empty prompt in analytics list (elastic#74174)
  [Task Manager] Correctly handle `running` tasks when calling RunNow and reduce flakiness in related tests (elastic#73244)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting release_note:fix Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Alerting] Alert list was not refreshed after deleting alerts
5 participants