Download as
Browse Code »

Commit 44600a70dba245821081c7ff3de78b6f41b7d8c8

Authored by Nathan Broadbent
1 parent 49468ce4
Exists in master and in 1 other branch production

Configure "email_at_notices" independently for each app. We have a combination o… 

…f both low and high traffic sites, so this feature would be useful so that we can stay on top of errors in some cases, and avoid getting spammed in others.
Showing 10 changed files with 103 additions and 26 deletions   Show diff stats
app/controllers/apps_controller.rb
  Diff comments   View file @44600a7
... ... @@ -2,6 +2,7 @@ class AppsController < ApplicationController
2 2  
3 3 before_filter :require_admin!, :except => [:index, :show]
4 4 before_filter :find_app, :except => [:index, :new, :create]
  5 + before_filter :parse_email_at_notices_or_set_default, :only => [:create, :update]
5 6  
6 7 def index
7 8 @apps = current_user.admin? ? App.all : current_user.apps.all
... ... @@ -73,4 +74,21 @@ class AppsController < ApplicationController
73 74 # apparently finding by 'watchers.email' and 'id' is broken
74 75 raise(Mongoid::Errors::DocumentNotFound.new(App,@app.id)) unless current_user.admin? || current_user.watching?(@app)
75 76 end
  77 +
  78 + # email_at_notices is edited as a string, and stored as an array.
  79 + def parse_email_at_notices_or_set_default
  80 + if params[:app] && val = params[:app][:email_at_notices]
  81 + # Sanitize negative values, split on comma,
  82 + # strip, parse as integer, remove all '0's.
  83 + # If empty, set as default and show an error message.
  84 + email_at_notices = val.gsub(/-\d+/,"").split(",").map{|v| v.strip.to_i }.reject{|v| v == 0}
  85 + if email_at_notices.any?
  86 + params[:app][:email_at_notices] = email_at_notices
  87 + else
  88 + default_array = params[:app][:email_at_notices] = Errbit::Config.default_email_at_notices
  89 + flash[:error] = "Couldn't parse your notification frequency. Value was reset to default (#{default_array.join(', ')})."
  90 + end
  91 + end
  92 + end
76 93 end
  94 +
... ...
app/models/app.rb
  Diff comments   View file @44600a7
... ... @@ -8,6 +8,7 @@ class App
8 8 field :resolve_errs_on_deploy, :type => Boolean, :default => false
9 9 field :notify_on_errs, :type => Boolean, :default => true
10 10 field :notify_on_deploys, :type => Boolean, :default => true
  11 + field :email_at_notices, :type => Array, :default => Errbit::Config.default_email_at_notices
11 12  
12 13 # Some legacy apps may have sting as key instead of BSON::ObjectID
13 14 identity :type => String
... ... @@ -68,7 +69,7 @@ class App
68 69 def github_url_to_file(file)
69 70 "#{self.github_url}/blob/master#{file}"
70 71 end
71   -
  72 +
72 73 def issue_tracker_configured?
73 74 issue_tracker && !issue_tracker.project_id.blank?
74 75 end
... ... @@ -96,3 +97,4 @@ class App
96 97 end
97 98  
98 99 end
  100 +
... ...
app/models/notice.rb
  Diff comments   View file @44600a7
... ... @@ -55,11 +55,11 @@ class Notice
55 55 agent_string = env_vars['HTTP_USER_AGENT']
56 56 agent_string.blank? ? nil : UserAgent.parse(agent_string)
57 57 end
58   -
  58 +
59 59 def self.in_app_backtrace_line? line
60 60 !!(line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))})
61 61 end
62   -
  62 +
63 63 def request
64 64 read_attribute(:request) || {}
65 65 end
... ... @@ -83,11 +83,11 @@ class Notice
83 83 def cache_last_notice_at
84 84 err.update_attributes(:last_notice_at => created_at)
85 85 end
86   -
  86 +
87 87 protected
88 88  
89 89 def should_notify?
90   - err.app.notify_on_errs? && Errbit::Config.email_at_notices.include?(err.notices.count) && err.app.watchers.any?
  90 + err.app.notify_on_errs? && err.app.email_at_notices.include?(err.notices.count) && err.app.watchers.any?
91 91 end
92 92  
93 93  
... ... @@ -122,3 +122,4 @@ class Notice
122 122 end
123 123 end
124 124 end
  125 +
... ...
app/views/apps/_fields.html.haml
  Diff comments   View file @44600a7
... ... @@ -4,21 +4,34 @@
4 4 = f.label :name
5 5 = f.text_field :name
6 6  
7   -%div.checkbox
8   - = f.check_box :notify_on_errs
9   - = f.label :notify_on_errs, 'Notify on errors'
10   -
11 7 %div
12 8 = f.label :github_url
13 9 = f.text_field :github_url
14 10  
  11 +%fieldset
  12 + %legend Notifications
  13 + %div.checkbox
  14 + = f.check_box :notify_on_errs
  15 + = f.label :notify_on_errs, 'Notify on errors'
  16 + %div.email_at_notices_nested{:style => f.object.notify_on_errs ? '' : 'display: none;'}
  17 + .field-helpertext Send a notification every
  18 + -# Edit the email_at_notices array as a CSV string
  19 + = f.text_field :email_at_notices, :value => f.object.email_at_notices.join(", ")
  20 + .field-helpertext times an error occurs. (CSV)
  21 + %div.checkbox
  22 + = f.check_box :notify_on_deploys
  23 + = f.label :notify_on_deploys, 'Notify on deploys'
  24 +
  25 +:javascript
  26 + $('#app_notify_on_errs').change(function(){
  27 + var el = $('.email_at_notices_nested');
  28 + this.checked ? el.show() : el.hide();
  29 + });
  30 +
15 31 %div.checkbox
16 32 = f.check_box :resolve_errs_on_deploy
17 33 = f.label :resolve_errs_on_deploy, 'Resolve errs on deploy'
18 34  
19   -%div.checkbox
20   - = f.check_box :notify_on_deploys
21   - = f.label :notify_on_deploys, 'Notify on deploys'
22 35  
23 36 %fieldset.nested-wrapper
24 37 %legend Watchers
... ... @@ -75,3 +88,4 @@
75 88 = w.text_field :username, :placeholder => 'Username/Email for your account'
76 89 = w.label :password, 'account password'
77 90 = w.password_field :password, :placeholder => 'Password for your account'
  91 +
... ...
config/config.example.yml
  Diff comments   View file @44600a7
... ... @@ -14,11 +14,13 @@ host: errbit.example.com
14 14 # will be sent from.
15 15 email_from: errbit@example.com
16 16  
17   -# Configure when emails are sent for an error.
  17 +# Configure the default value for when emails are sent for an error.
18 18 # [1,3,7] = 1st, 3rd, and 7th occurence triggers
19 19 # an email notification.
20   -email_at_notices: [1, 10, 100]
  20 +# Each app can be configured individually.
  21 +default_email_at_notices: [1, 10, 100]
21 22  
22   -# Set to false to suppress confirmation when
  23 +# Set to false to suppress confirmation when
23 24 # resolving errors.
24 25 confirm_resolve_err: true
  26 +
... ...
config/initializers/_load_config.rb
  Diff comments   View file @44600a7
... ... @@ -4,7 +4,7 @@ if ENV['HEROKU']
4 4 Errbit::Config = OpenStruct.new
5 5 Errbit::Config.host = ENV['ERRBIT_HOST']
6 6 Errbit::Config.email_from = ENV['ERRBIT_EMAIL_FROM']
7   - Errbit::Config.email_at_notices = [1,3,10] #ENV['ERRBIT_EMAIL_AT_NOTICES']
  7 + Errbit::Config.default_email_at_notices = [1,3,10] #ENV['ERRBIT_EMAIL_AT_NOTICES']
8 8 Errbit::Application.config.action_mailer.smtp_settings = {
9 9 :address => "smtp.sendgrid.net",
10 10 :port => "25",
... ... @@ -25,4 +25,5 @@ end
25 25 # Set config specific values
26 26 (Errbit::Application.config.action_mailer.default_url_options ||= {}).tap do |default|
27 27 default.merge! :host => Errbit::Config.host if default[:host].blank?
28   -end
29 28 \ No newline at end of file
  29 +end
  30 +
... ...
public/stylesheets/application.css
  Diff comments   View file @44600a7
... ... @@ -350,6 +350,17 @@ form .error-messages ul {
350 350 list-style-type: square;
351 351 }
352 352  
  353 +form .field-helpertext {
  354 + display: inline;
  355 + text-transform: uppercase;
  356 +}
  357 +
  358 +form input#app_email_at_notices {
  359 + width: 130px;
  360 + margin: 0 5px;
  361 +}
  362 +
  363 +
353 364 /* Tables */
354 365 table {
355 366 width: 100%;
... ... @@ -683,3 +694,4 @@ span.click_span {
683 694 #deploys_div, #repository_div, #watchers_div {
684 695 display: none;
685 696 }
  697 +
... ...
spec/controllers/apps_controller_spec.rb
  Diff comments   View file @44600a7
... ... @@ -241,6 +241,27 @@ describe AppsController do
241 241 end
242 242 end
243 243  
  244 + context "changing email_at_notices" do
  245 + it "should parse legal csv values" do
  246 + put :update, :id => @app.id, :app => { :email_at_notices => '1, 4, 7,8, 10' }
  247 + @app.reload
  248 + @app.email_at_notices.should == [1, 4, 7, 8, 10]
  249 + end
  250 + context "failed parsing of CSV" do
  251 + it "should set the default value" do
  252 + @app = Factory(:app, :email_at_notices => [1, 2, 3, 4])
  253 + put :update, :id => @app.id, :app => { :email_at_notices => 'asdf, -1,0,foobar,gd00,0,abc' }
  254 + @app.reload
  255 + @app.email_at_notices.should == Errbit::Config.default_email_at_notices
  256 + end
  257 +
  258 + it "should display a message" do
  259 + put :update, :id => @app.id, :app => { :email_at_notices => 'qwertyuiop' }
  260 + request.flash[:error].should match(/Couldn't parse/)
  261 + end
  262 + end
  263 + end
  264 +
244 265 context "setting up issue tracker", :cur => true do
245 266 context "unknown tracker type" do
246 267 before(:each) do
... ... @@ -386,3 +407,4 @@ describe AppsController do
386 407 end
387 408  
388 409 end
  410 +
... ...
spec/factories/app_factories.rb
  Diff comments   View file @44600a7
1 1 Factory.define(:app) do |p|
2 2 p.name { Factory.next :app_name }
  3 + p.email_at_notices { [1, 10, 100] }
3 4 end
4 5  
5 6 Factory.define(:app_with_watcher, :parent => :app) do |p|
... ... @@ -26,3 +27,4 @@ Factory.define(:deploy) do |d|
26 27 d.environment 'production'
27 28 d.revision ActiveSupport::SecureRandom.hex(10)
28 29 end
  30 +
... ...
spec/models/notice_spec.rb
  Diff comments   View file @44600a7
... ... @@ -21,15 +21,15 @@ describe Notice do
21 21 notice.errors[:notifier].should include("can't be blank")
22 22 end
23 23 end
24   -
  24 +
25 25 context '.in_app_backtrace_line?' do
26 26 let(:backtrace) do [{
27 27 'number' => rand(999),
28   - 'file' => '[GEM_ROOT]/gems/actionpack-3.0.4/lib/action_controller/metal/rescue.rb',
  28 + 'file' => '[GEM_ROOT]/gems/actionpack-3.0.4/lib/action_controller/metal/rescue.rb',
29 29 'method' => ActiveSupport.methods.shuffle.first
30 30 }, {
31 31 'number' => rand(999),
32   - 'file' => '[PROJECT_ROOT]/vendor/plugins/seamless_database_pool/lib/seamless_database_pool/controller_filter.rb',
  32 + 'file' => '[PROJECT_ROOT]/vendor/plugins/seamless_database_pool/lib/seamless_database_pool/controller_filter.rb',
33 33 'method' => ActiveSupport.methods.shuffle.first
34 34 }, {
35 35 'number' => rand(999),
... ... @@ -50,7 +50,7 @@ describe Notice do
50 50 Notice.in_app_backtrace_line?(backtrace[2]).should == true
51 51 end
52 52 end
53   -
  53 +
54 54 context '#from_xml' do
55 55 before do
56 56 @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read
... ... @@ -154,20 +154,22 @@ describe Notice do
154 154 notice.user_agent.browser.should == 'Chrome'
155 155 notice.user_agent.version.to_s.should =~ /^10\.0/
156 156 end
157   -
  157 +
158 158 it "should be nil if HTTP_USER_AGENT is blank" do
159 159 notice = Factory.build(:notice)
160 160 notice.user_agent.should == nil
161 161 end
162 162 end
163   -
164   - describe "email notifications" do
  163 +
  164 + describe "email notifications (configured individually for each app)" do
  165 + custom_thresholds = [2, 4, 8, 16, 32, 64]
  166 +
165 167 before do
166   - @app = Factory(:app_with_watcher)
  168 + @app = Factory(:app_with_watcher, :email_at_notices => custom_thresholds)
167 169 @err = Factory(:err, :app => @app)
168 170 end
169 171  
170   - Errbit::Config.email_at_notices.each do |threshold|
  172 + custom_thresholds.each do |threshold|
171 173 it "sends an email notification after #{threshold} notice(s)" do
172 174 @err.notices.stub(:count).and_return(threshold)
173 175 Mailer.should_receive(:err_notification).
... ... @@ -178,3 +180,4 @@ describe Notice do
178 180 end
179 181  
180 182 end
  183 +
... ...