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

Edit alert flyout #58964

Merged
merged 17 commits into from
Mar 4, 2020
Merged

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented Feb 29, 2020

Summary

Resolve #51545

Checklist

Delete any items that are not applicable to this PR.

@YulNaumenko YulNaumenko added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Feb 29, 2020
@YulNaumenko YulNaumenko requested a review from a team as a code owner February 29, 2020 01:29
@YulNaumenko YulNaumenko self-assigned this Feb 29, 2020
@elasticmachine
Copy link
Contributor

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

…alert_flyout

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
Copy link
Contributor

@gmmorris gmmorris left a comment

Choose a reason for hiding this comment

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

I haven't been able to get the UI to work for me yet, so not approving, but I can't see anything worrying, so happy to approve once I get to test the UI. :)

Comment on lines 78 to 81
// TODO: uncomment variables test when server API will be ready
// await testSubjects.click('slackAddVariableButton');
// const variableMenuButton = await testSubjects.find('variableMenuButton-0');
// await variableMenuButton.click();
Copy link
Contributor

Choose a reason for hiding this comment

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

Has this been forgotten or is it still in progress? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In progress, we have an issue for this #58529

…alert_flyout

# Conflicts:
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/expression.tsx
#	x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts
…alert_flyout

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
…alert_flyout

# Conflicts:
#	x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.test.tsx
Copy link
Contributor

@gmmorris gmmorris left a comment

Choose a reason for hiding this comment

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

Talked with @YulNaumenko about a couple of points and a UI bug - all seems to already be known and documented in other issues, so approving this. 👍

Copy link
Member

@pmuellr pmuellr 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

I loaded up the PR, was able to edit an alert \o/

A few issues; have to modify the interval number to save (but also on create), and an alert saved with an interval 1s comes up in the edit form as 1 minute instead. We can open these as separate issues. Let's ship this thing!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@YulNaumenko YulNaumenko merged commit daf6226 into elastic:master Mar 4, 2020
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Mar 4, 2020
* Implemented edit alert functionality

* Added unit tests

* Added functional test for edit alert

* Fixed failed tests

* Fixed edit api

* Fixed due to comments

* Fixed functional test

* Fixed tests

* Fixed add alert

* Small type fix

* Fixed jest test

* Fixed type check

* Fixed bugs with interval and throttle + index threshold expression
YulNaumenko added a commit that referenced this pull request Mar 5, 2020
* Edit alert flyout (#58964)

* Implemented edit alert functionality

* Added unit tests

* Added functional test for edit alert

* Fixed failed tests

* Fixed edit api

* Fixed due to comments

* Fixed functional test

* Fixed tests

* Fixed add alert

* Small type fix

* Fixed jest test

* Fixed type check

* Fixed bugs with interval and throttle + index threshold expression

* Removed before all from alert jest tests

* Fixed type check
jloleysens added a commit to jloleysens/kibana that referenced this pull request Mar 5, 2020
…re/files-and-filetree

* 'master' of github.com:elastic/kibana: (254 commits)
  Convert discover_page to ts, remove redundunt methods (elastic#59312)
  [Fix for Vis Editor] Revert setting time field to empty string when it's undefined (elastic#58873)
  Delete legacy search endpoint (elastic#59341)
  [Uptime] Improve duration chart (elastic#58404)
  [Snapshot & Restore] NP migration (elastic#59109)
  [ML] Add support for date_nanos time field in anomaly job wizard (elastic#59017)
  Revert "Makes alerting and actions optional properties for interface RequestH… (elastic#59264)"
  Change remote_clusters ID to remoteClusters (elastic#59246)
  Makes alerting and actions optional properties for interface RequestH… (elastic#59264)
  Clean up date histogram agg type. (elastic#58805)
  [ML] Management: fix license unsubscribe (elastic#59365)
  Remove documentation for server.cors settings (elastic#59096)
  Edit alert flyout (elastic#58964)
  [SIEM] Fix rule delete/duplicate actions (elastic#59306)
  move mouse to close obstructing tooltip (elastic#59214)
  Reset page after deleting (elastic#59310)
  Make sure phrases input filter triggers autosuggestons (elastic#59299)
  Add loading count source for http requests (elastic#59245)
  Revert "[ML] Transforms: Deprecate custom KibanaContext. (elastic#59133)"
  Expose metrics service to public API (elastic#59294)
  ...

# Conflicts:
#	src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.tsx
spong pushed a commit that referenced this pull request Mar 24, 2020
## Summary

Allow defining notifications that will trigger whenever the rule created new signals.

Requires:
- #58395
- #58964
- #60832


![Screenshot 2020-03-02 at 10 19 18](https://user-images.githubusercontent.com/5188868/75662390-4fe8bf00-5c6f-11ea-943f-591367348b91.png)

![Screenshot 2020-03-02 at 10 13 00](https://user-images.githubusercontent.com/5188868/75662421-5e36db00-5c6f-11ea-9317-d158cddf4344.png)


### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials
- [ ] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)
- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)
- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
spong pushed a commit to spong/kibana that referenced this pull request Mar 24, 2020
## Summary

Allow defining notifications that will trigger whenever the rule created new signals.

Requires:
- elastic#58395
- elastic#58964
- elastic#60832


![Screenshot 2020-03-02 at 10 19 18](https://user-images.githubusercontent.com/5188868/75662390-4fe8bf00-5c6f-11ea-943f-591367348b91.png)

![Screenshot 2020-03-02 at 10 13 00](https://user-images.githubusercontent.com/5188868/75662421-5e36db00-5c6f-11ea-9317-d158cddf4344.png)


### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials
- [ ] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)
- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)
- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
spong added a commit that referenced this pull request Mar 25, 2020
## Summary

Allow defining notifications that will trigger whenever the rule created new signals.

Requires:
- #58395
- #58964
- #60832


![Screenshot 2020-03-02 at 10 19 18](https://user-images.githubusercontent.com/5188868/75662390-4fe8bf00-5c6f-11ea-943f-591367348b91.png)

![Screenshot 2020-03-02 at 10 13 00](https://user-images.githubusercontent.com/5188868/75662421-5e36db00-5c6f-11ea-9317-d158cddf4344.png)


### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials
- [ ] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)
- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)
- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)

Co-authored-by: patrykkopycinski <patryk.kopycinski@elastic.co>
@mikecote mikecote added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting release_note:enhancement Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UI] Edit alert
6 participants