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

Improved alarm callback testing #3196

Merged
merged 4 commits into from Dec 14, 2016

Conversation

Projects
None yet
2 participants
@edmundoa
Member

edmundoa commented Dec 13, 2016

This PR fixes two of the open tasks for alerting #3178:

  • Ensures that testing an EmailAlarmCallback is throwing exceptions when there are errors, that can be displayed to the user when testing a callback
  • Adds a new REST resource, allowing users to test single alarm callbacks

edmundoa added some commits Dec 13, 2016

Do not swallow errors in EmailAlarmCallback
When sending emails for an alert, throw exceptions when there is a
problem, so consumers of the class can react to that situation.

Also check if email configuration is enabled when EmailAlarmCallback has
no recipients, throwing an exception if that is not the case.
Display feedback in test callbacks button
Testing callbacks may take a while, so we need to ensure we give some
feedback about the action in the UI, and also that we support several
concurrent requests.

@edmundoa edmundoa added this to the 2.2.0 milestone Dec 13, 2016

@edmundoa edmundoa referenced this pull request Dec 13, 2016

Closed

Alerting open tasks #3178

8 of 8 tasks complete

@bernd bernd self-assigned this Dec 13, 2016

@bernd

bernd approved these changes Dec 14, 2016

Apart from my question regarding the StreamsStore it LGTM. I also successfully tested it.

_onTestNotification() {
StreamsStore.sendDummyAlert(this.props.alertNotification.stream_id);

This comment has been minimized.

@bernd

bernd Dec 14, 2016

Member

Can the #sendDummyAlert() function in StreamsStore be removed?

This comment has been minimized.

@edmundoa

edmundoa Dec 14, 2016

Member

Yes, it can, at least we don't use it. Will take care of it 👍

This comment has been minimized.

@edmundoa

edmundoa Dec 14, 2016

Member

I was wrong, we use it in some other old components that I didn't clean up yet. As we have another task in #3178 to clean up the code, I would postpone the removal until then.

This comment has been minimized.

@bernd

@bernd bernd merged commit fe76627 into master Dec 14, 2016

4 checks passed

ci-web-linter Jenkins build graylog-pr-linter-check 1141 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@bernd bernd deleted the improved-callback-testing branch Dec 14, 2016

@bernd bernd removed the ready-for-review label Dec 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment