Send default mail when no email destinations found #6165

Merged
merged 9 commits into from Mar 14, 2017

Conversation

Projects
None yet
6 participants
@towster
Contributor

towster commented Mar 10, 2017

When no email destinations are found it simply would log that there were no email destinations.
This change will cause it to send an email to the default mail address if there are no email destinations set.

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

  • Have you signed the Contributors agreement - please do NOT submit a pull request unless you have (signing the agreement in the same pull request is fine). Your commit message for signing the agreement must appear as per the docs.
  • Have you followed our code guidelines?

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Mar 10, 2017

Thank you for submitting a PR @towster! We have found the following @murrant, @laf and @zarya based on the history of these files to review this PR.

Thank you for submitting a PR @towster! We have found the following @murrant, @laf and @zarya based on the history of these files to review this PR.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@murrant

Hmm, this could be unexpected behavior. It might be a good idea to modify the email to explain.

@towster

This comment has been minimized.

Show comment
Hide comment
@towster

towster Mar 11, 2017

Contributor

It does change the "To:" to use the pretty name of "Default". Like this:
To: Default <librenms@work.com>
Is that sufficient?

Contributor

towster commented Mar 11, 2017

It does change the "To:" to use the pretty name of "Default". Like this:
To: Default <librenms@work.com>
Is that sufficient?

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 11, 2017

Member

This is a major change to the way alerts work - this should be it's own toggle switch "Fall back to default contact if no other contacts can be found"

Member

laf commented Mar 11, 2017

This is a major change to the way alerts work - this should be it's own toggle switch "Fall back to default contact if no other contacts can be found"

@laf laf added the Blocker 🚫 label Mar 11, 2017

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 13, 2017

Member

Thanks @towster - It needs also to be in the webui:

html/pages/settings/alerting.inc.php

  • a config entry adding to db, see:

sql-schema/166.sql as an example

Member

laf commented Mar 13, 2017

Thanks @towster - It needs also to be in the webui:

html/pages/settings/alerting.inc.php

  • a config entry adding to db, see:

sql-schema/166.sql as an example

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 14, 2017

Member

trigger ci

Member

murrant commented Mar 14, 2017

trigger ci

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 14, 2017

Member

I've just updated Default to NOC to match other areas - tested though and works fine.

Member

laf commented Mar 14, 2017

I've just updated Default to NOC to match other areas - tested though and works fine.

@laf laf added Alerting 🔔 Schema and removed Blocker 🚫 labels Mar 14, 2017

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Mar 14, 2017

The inspection completed: No new issues

The inspection completed: No new issues

@laf laf merged commit 605b9d2 into librenms:master Mar 14, 2017

2 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment