Commit cb517cecf4d82b698dd2b4da4bfc0a342c13c55e
1 parent
1135aa99
Exists in
master
and in
1 other branch
Give the option to either send notifications to all users, or to a configured list of watchers.
Adding users one by one could be a bit tedious for large teams.
Showing
4 changed files
with
43 additions
and
20 deletions
Show diff stats
app/mailers/mailer.rb
| 1 | 1 | class Mailer < ActionMailer::Base |
| 2 | 2 | default :from => Errbit::Config.email_from |
| 3 | - | |
| 3 | + | |
| 4 | 4 | def err_notification(notice) |
| 5 | 5 | @notice = notice |
| 6 | 6 | @app = notice.err.app |
| 7 | - | |
| 7 | + | |
| 8 | 8 | mail({ |
| 9 | - :to => @app.watchers.map(&:address), | |
| 9 | + :to => @app.notification_recipients, | |
| 10 | 10 | :subject => "[#{@app.name}][#{@notice.err.environment}] #{@notice.err.message}" |
| 11 | 11 | }) |
| 12 | 12 | end |
| 13 | - | |
| 13 | + | |
| 14 | 14 | def deploy_notification(deploy) |
| 15 | 15 | @deploy = deploy |
| 16 | 16 | @app = deploy.app |
| 17 | - | |
| 17 | + | |
| 18 | 18 | mail({ |
| 19 | - :to => @app.watchers.map(&:address), | |
| 19 | + :to => @app.notification_recipients, | |
| 20 | 20 | :subject => "[#{@app.name}] Deployed to #{@deploy.environment} by #{@deploy.username}" |
| 21 | 21 | }) |
| 22 | 22 | end |
| 23 | - | |
| 23 | + | |
| 24 | 24 | end |
| 25 | + | ... | ... |
app/models/app.rb
| ... | ... | @@ -6,6 +6,7 @@ class App |
| 6 | 6 | field :api_key |
| 7 | 7 | field :github_url |
| 8 | 8 | field :resolve_errs_on_deploy, :type => Boolean, :default => false |
| 9 | + field :notify_all_users, :type => Boolean, :default => false | |
| 9 | 10 | field :notify_on_errs, :type => Boolean, :default => true |
| 10 | 11 | field :notify_on_deploys, :type => Boolean, :default => true |
| 11 | 12 | field :email_at_notices, :type => Array, :default => Errbit::Config.email_at_notices |
| ... | ... | @@ -74,6 +75,10 @@ class App |
| 74 | 75 | issue_tracker && issue_tracker.issue_tracker_type != "none" && !issue_tracker.project_id.blank? |
| 75 | 76 | end |
| 76 | 77 | |
| 78 | + def notification_recipients | |
| 79 | + notify_all_users ? User.all.map(&:email) : watchers.map(&:address) | |
| 80 | + end | |
| 81 | + | |
| 77 | 82 | protected |
| 78 | 83 | |
| 79 | 84 | def generate_api_key | ... | ... |
app/views/apps/_fields.html.haml
| ... | ... | @@ -23,11 +23,11 @@ |
| 23 | 23 | = f.label :notify_on_deploys, 'Notify on deploys' |
| 24 | 24 | |
| 25 | 25 | %div.checkbox |
| 26 | - = f.check_box :resolve_errs_on_deploy | |
| 27 | - = f.label :resolve_errs_on_deploy, 'Resolve errs on deploy' | |
| 26 | + = f.check_box :notify_all_users, 'data-hide-when-checked' => '.watchers.nested-wrapper' | |
| 27 | + = f.label :notify_all_users, 'Send notifications to all users' | |
| 28 | 28 | |
| 29 | 29 | |
| 30 | -%fieldset.nested-wrapper | |
| 30 | +%fieldset.watchers.nested-wrapper{:style => f.object.notify_all_users ? 'display: none;' : ''} | |
| 31 | 31 | %legend Watchers |
| 32 | 32 | = f.fields_for :watchers do |w| |
| 33 | 33 | %div.watcher.nested |
| ... | ... | @@ -41,6 +41,10 @@ |
| 41 | 41 | %div.watcher_params.email{:class => w.object.email.present? ? 'chosen' : nil} |
| 42 | 42 | = w.text_field :email |
| 43 | 43 | |
| 44 | +%div.checkbox | |
| 45 | + = f.check_box :resolve_errs_on_deploy | |
| 46 | + = f.label :resolve_errs_on_deploy, 'Resolve errs on deploy' | |
| 47 | + | |
| 44 | 48 | %fieldset |
| 45 | 49 | %legend Issue tracker |
| 46 | 50 | = f.fields_for :issue_tracker do |w| | ... | ... |
spec/models/app_spec.rb
| 1 | 1 | require 'spec_helper' |
| 2 | 2 | |
| 3 | 3 | describe App do |
| 4 | - | |
| 4 | + | |
| 5 | 5 | context 'validations' do |
| 6 | 6 | it 'requires a name' do |
| 7 | 7 | app = Factory.build(:app, :name => nil) |
| 8 | 8 | app.should_not be_valid |
| 9 | 9 | app.errors[:name].should include("can't be blank") |
| 10 | 10 | end |
| 11 | - | |
| 11 | + | |
| 12 | 12 | it 'requires unique names' do |
| 13 | 13 | Factory(:app, :name => 'Errbit') |
| 14 | 14 | app = Factory.build(:app, :name => 'Errbit') |
| 15 | 15 | app.should_not be_valid |
| 16 | 16 | app.errors[:name].should include('is already taken') |
| 17 | 17 | end |
| 18 | - | |
| 18 | + | |
| 19 | 19 | it 'requires unique api_keys' do |
| 20 | 20 | Factory(:app, :api_key => 'APIKEY') |
| 21 | 21 | app = Factory.build(:app, :api_key => 'APIKEY') |
| ... | ... | @@ -23,7 +23,7 @@ describe App do |
| 23 | 23 | app.errors[:api_key].should include('is already taken') |
| 24 | 24 | end |
| 25 | 25 | end |
| 26 | - | |
| 26 | + | |
| 27 | 27 | context 'being created' do |
| 28 | 28 | it 'generates a new api-key' do |
| 29 | 29 | app = Factory.build(:app) |
| ... | ... | @@ -31,18 +31,18 @@ describe App do |
| 31 | 31 | app.save |
| 32 | 32 | app.api_key.should_not be_nil |
| 33 | 33 | end |
| 34 | - | |
| 34 | + | |
| 35 | 35 | it 'generates a correct api-key' do |
| 36 | 36 | app = Factory(:app) |
| 37 | 37 | app.api_key.should match(/^[a-f0-9]{32}$/) |
| 38 | 38 | end |
| 39 | - | |
| 39 | + | |
| 40 | 40 | it 'is fine with blank github urls' do |
| 41 | 41 | app = Factory.build(:app, :github_url => "") |
| 42 | 42 | app.save |
| 43 | 43 | app.github_url.should == "" |
| 44 | 44 | end |
| 45 | - | |
| 45 | + | |
| 46 | 46 | it 'does not touch https github urls' do |
| 47 | 47 | app = Factory.build(:app, :github_url => "https://github.com/jdpace/errbit") |
| 48 | 48 | app.save |
| ... | ... | @@ -67,14 +67,14 @@ describe App do |
| 67 | 67 | app.github_url.should == "https://github.com/jdpace/errbit" |
| 68 | 68 | end |
| 69 | 69 | end |
| 70 | - | |
| 70 | + | |
| 71 | 71 | context '#github_url_to_file' do |
| 72 | 72 | it 'resolves to full path to file' do |
| 73 | 73 | app = Factory(:app, :github_url => "https://github.com/jdpace/errbit") |
| 74 | 74 | app.github_url_to_file('/path/to/file').should == "https://github.com/jdpace/errbit/blob/master/path/to/file" |
| 75 | 75 | end |
| 76 | 76 | end |
| 77 | - | |
| 77 | + | |
| 78 | 78 | context '#github_url?' do |
| 79 | 79 | it 'is true when there is a github_url' do |
| 80 | 80 | app = Factory(:app, :github_url => "https://github.com/jdpace/errbit") |
| ... | ... | @@ -86,5 +86,18 @@ describe App do |
| 86 | 86 | app.github_url?.should be_false |
| 87 | 87 | end |
| 88 | 88 | end |
| 89 | - | |
| 89 | + | |
| 90 | + context "notification recipients" do | |
| 91 | + it "should send notices to either all users, or the configured watchers" do | |
| 92 | + @app = Factory(:app) | |
| 93 | + 3.times { Factory(:user) } | |
| 94 | + 5.times { Factory(:watcher, :app => @app) } | |
| 95 | + @app.notify_all_users = true | |
| 96 | + @app.notification_recipients.size.should == 3 | |
| 97 | + @app.notify_all_users = false | |
| 98 | + @app.notification_recipients.size.should == 5 | |
| 99 | + end | |
| 100 | + end | |
| 101 | + | |
| 90 | 102 | end |
| 103 | + | ... | ... |