Download as
Browse Code »

Commit 08a1ac92e9bbb23ca5699ff42592dc91d1f65a81

Authored by Nick Recobra
2 parents b8070297 44600a70
Exists in master and in 1 other branch production

Merge branch 'email_at_notices_per_app' of https://github.com/crossroads/errbit into pull-request-52

Showing 10 changed files with 100 additions and 25 deletions   Show diff stats
app/controllers/apps_controller.rb
  Diff comments   View file @08a1ac9
@@ -2,6 +2,7 @@ class AppsController < ApplicationController @@ -2,6 +2,7 @@ class AppsController < ApplicationController
2 2
3 before_filter :require_admin!, :except => [:index, :show] 3 before_filter :require_admin!, :except => [:index, :show]
4 before_filter :find_app, :except => [:index, :new, :create] 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 def index 7 def index
7 @apps = current_user.admin? ? App.all : current_user.apps.all 8 @apps = current_user.admin? ? App.all : current_user.apps.all
@@ -73,4 +74,21 @@ class AppsController < ApplicationController @@ -73,4 +74,21 @@ class AppsController < ApplicationController
73 # apparently finding by 'watchers.email' and 'id' is broken 74 # apparently finding by 'watchers.email' and 'id' is broken
74 raise(Mongoid::Errors::DocumentNotFound.new(App,@app.id)) unless current_user.admin? || current_user.watching?(@app) 75 raise(Mongoid::Errors::DocumentNotFound.new(App,@app.id)) unless current_user.admin? || current_user.watching?(@app)
75 end 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 end 93 end
  94 +
app/models/app.rb
  Diff comments   View file @08a1ac9
@@ -8,6 +8,7 @@ class App @@ -8,6 +8,7 @@ class App
8 field :resolve_errs_on_deploy, :type => Boolean, :default => false 8 field :resolve_errs_on_deploy, :type => Boolean, :default => false
9 field :notify_on_errs, :type => Boolean, :default => true 9 field :notify_on_errs, :type => Boolean, :default => true
10 field :notify_on_deploys, :type => Boolean, :default => true 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 # Some legacy apps may have sting as key instead of BSON::ObjectID 13 # Some legacy apps may have sting as key instead of BSON::ObjectID
13 identity :type => String 14 identity :type => String
app/models/notice.rb
  Diff comments   View file @08a1ac9
@@ -55,11 +55,11 @@ class Notice @@ -55,11 +55,11 @@ class Notice
55 agent_string = env_vars['HTTP_USER_AGENT'] 55 agent_string = env_vars['HTTP_USER_AGENT']
56 agent_string.blank? ? nil : UserAgent.parse(agent_string) 56 agent_string.blank? ? nil : UserAgent.parse(agent_string)
57 end 57 end
58 - 58 +
59 def self.in_app_backtrace_line? line 59 def self.in_app_backtrace_line? line
60 !!(line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))}) 60 !!(line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))})
61 end 61 end
62 - 62 +
63 def request 63 def request
64 read_attribute(:request) || {} 64 read_attribute(:request) || {}
65 end 65 end
@@ -83,11 +83,11 @@ class Notice @@ -83,11 +83,11 @@ class Notice
83 def cache_last_notice_at 83 def cache_last_notice_at
84 err.update_attributes(:last_notice_at => created_at) 84 err.update_attributes(:last_notice_at => created_at)
85 end 85 end
86 - 86 +
87 protected 87 protected
88 88
89 def should_notify? 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 end 91 end
92 92
93 93
@@ -122,3 +122,4 @@ class Notice @@ -122,3 +122,4 @@ class Notice
122 end 122 end
123 end 123 end
124 end 124 end
  125 +
app/views/apps/_fields.html.haml
  Diff comments   View file @08a1ac9
@@ -4,21 +4,34 @@ @@ -4,21 +4,34 @@
4 = f.label :name 4 = f.label :name
5 = f.text_field :name 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 %div 7 %div
12 = f.label :github_url 8 = f.label :github_url
13 = f.text_field :github_url 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 %div.checkbox 31 %div.checkbox
16 = f.check_box :resolve_errs_on_deploy 32 = f.check_box :resolve_errs_on_deploy
17 = f.label :resolve_errs_on_deploy, 'Resolve errs on deploy' 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 %fieldset.nested-wrapper 36 %fieldset.nested-wrapper
24 %legend Watchers 37 %legend Watchers
config/config.example.yml
  Diff comments   View file @08a1ac9
@@ -14,11 +14,13 @@ host: errbit.example.com @@ -14,11 +14,13 @@ host: errbit.example.com
14 # will be sent from. 14 # will be sent from.
15 email_from: errbit@example.com 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 # [1,3,7] = 1st, 3rd, and 7th occurence triggers 18 # [1,3,7] = 1st, 3rd, and 7th occurence triggers
19 # an email notification. 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 # resolving errors. 24 # resolving errors.
24 confirm_resolve_err: true 25 confirm_resolve_err: true
  26 +
config/initializers/_load_config.rb
  Diff comments   View file @08a1ac9
@@ -4,7 +4,7 @@ if ENV['HEROKU'] @@ -4,7 +4,7 @@ if ENV['HEROKU']
4 Errbit::Config = OpenStruct.new 4 Errbit::Config = OpenStruct.new
5 Errbit::Config.host = ENV['ERRBIT_HOST'] 5 Errbit::Config.host = ENV['ERRBIT_HOST']
6 Errbit::Config.email_from = ENV['ERRBIT_EMAIL_FROM'] 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 Errbit::Application.config.action_mailer.smtp_settings = { 8 Errbit::Application.config.action_mailer.smtp_settings = {
9 :address => "smtp.sendgrid.net", 9 :address => "smtp.sendgrid.net",
10 :port => "25", 10 :port => "25",
@@ -25,4 +25,5 @@ end @@ -25,4 +25,5 @@ end
25 # Set config specific values 25 # Set config specific values
26 (Errbit::Application.config.action_mailer.default_url_options ||= {}).tap do |default| 26 (Errbit::Application.config.action_mailer.default_url_options ||= {}).tap do |default|
27 default.merge! :host => Errbit::Config.host if default[:host].blank? 27 default.merge! :host => Errbit::Config.host if default[:host].blank?
28 -end  
29 \ No newline at end of file 28 \ No newline at end of file
  29 +end
  30 +
public/stylesheets/application.css
  Diff comments   View file @08a1ac9
@@ -350,6 +350,17 @@ form .error-messages ul { @@ -350,6 +350,17 @@ form .error-messages ul {
350 list-style-type: square; 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 /* Tables */ 364 /* Tables */
354 table { 365 table {
355 width: 100%; 366 width: 100%;
@@ -683,3 +694,4 @@ span.click_span { @@ -683,3 +694,4 @@ span.click_span {
683 #deploys_div, #repository_div, #watchers_div { 694 #deploys_div, #repository_div, #watchers_div {
684 display: none; 695 display: none;
685 } 696 }
  697 +
spec/controllers/apps_controller_spec.rb
  Diff comments   View file @08a1ac9
@@ -241,6 +241,27 @@ describe AppsController do @@ -241,6 +241,27 @@ describe AppsController do
241 end 241 end
242 end 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 context "setting up issue tracker", :cur => true do 265 context "setting up issue tracker", :cur => true do
245 context "unknown tracker type" do 266 context "unknown tracker type" do
246 before(:each) do 267 before(:each) do
@@ -386,3 +407,4 @@ describe AppsController do @@ -386,3 +407,4 @@ describe AppsController do
386 end 407 end
387 408
388 end 409 end
  410 +
spec/factories/app_factories.rb
  Diff comments   View file @08a1ac9
1 Factory.define(:app) do |p| 1 Factory.define(:app) do |p|
2 p.name { Factory.next :app_name } 2 p.name { Factory.next :app_name }
  3 + p.email_at_notices { [1, 10, 100] }
3 end 4 end
4 5
5 Factory.define(:app_with_watcher, :parent => :app) do |p| 6 Factory.define(:app_with_watcher, :parent => :app) do |p|
@@ -26,3 +27,4 @@ Factory.define(:deploy) do |d| @@ -26,3 +27,4 @@ Factory.define(:deploy) do |d|
26 d.environment 'production' 27 d.environment 'production'
27 d.revision ActiveSupport::SecureRandom.hex(10) 28 d.revision ActiveSupport::SecureRandom.hex(10)
28 end 29 end
  30 +
spec/models/notice_spec.rb
  Diff comments   View file @08a1ac9
@@ -21,15 +21,15 @@ describe Notice do @@ -21,15 +21,15 @@ describe Notice do
21 notice.errors[:notifier].should include("can't be blank") 21 notice.errors[:notifier].should include("can't be blank")
22 end 22 end
23 end 23 end
24 - 24 +
25 context '.in_app_backtrace_line?' do 25 context '.in_app_backtrace_line?' do
26 let(:backtrace) do [{ 26 let(:backtrace) do [{
27 'number' => rand(999), 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 'method' => ActiveSupport.methods.shuffle.first 29 'method' => ActiveSupport.methods.shuffle.first
30 }, { 30 }, {
31 'number' => rand(999), 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 'method' => ActiveSupport.methods.shuffle.first 33 'method' => ActiveSupport.methods.shuffle.first
34 }, { 34 }, {
35 'number' => rand(999), 35 'number' => rand(999),
@@ -50,7 +50,7 @@ describe Notice do @@ -50,7 +50,7 @@ describe Notice do
50 Notice.in_app_backtrace_line?(backtrace[2]).should == true 50 Notice.in_app_backtrace_line?(backtrace[2]).should == true
51 end 51 end
52 end 52 end
53 - 53 +
54 context '#from_xml' do 54 context '#from_xml' do
55 before do 55 before do
56 @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read 56 @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read
@@ -159,20 +159,22 @@ describe Notice do @@ -159,20 +159,22 @@ describe Notice do
159 notice.user_agent.browser.should == 'Chrome' 159 notice.user_agent.browser.should == 'Chrome'
160 notice.user_agent.version.to_s.should =~ /^10\.0/ 160 notice.user_agent.version.to_s.should =~ /^10\.0/
161 end 161 end
162 - 162 +
163 it "should be nil if HTTP_USER_AGENT is blank" do 163 it "should be nil if HTTP_USER_AGENT is blank" do
164 notice = Factory.build(:notice) 164 notice = Factory.build(:notice)
165 notice.user_agent.should == nil 165 notice.user_agent.should == nil
166 end 166 end
167 end 167 end
168 -  
169 - describe "email notifications" do 168 +
  169 + describe "email notifications (configured individually for each app)" do
  170 + custom_thresholds = [2, 4, 8, 16, 32, 64]
  171 +
170 before do 172 before do
171 - @app = Factory(:app_with_watcher) 173 + @app = Factory(:app_with_watcher, :email_at_notices => custom_thresholds)
172 @err = Factory(:err, :app => @app) 174 @err = Factory(:err, :app => @app)
173 end 175 end
174 176
175 - Errbit::Config.email_at_notices.each do |threshold| 177 + custom_thresholds.each do |threshold|
176 it "sends an email notification after #{threshold} notice(s)" do 178 it "sends an email notification after #{threshold} notice(s)" do
177 @err.notices.stub(:count).and_return(threshold) 179 @err.notices.stub(:count).and_return(threshold)
178 Mailer.should_receive(:err_notification). 180 Mailer.should_receive(:err_notification).
@@ -183,3 +185,4 @@ describe Notice do @@ -183,3 +185,4 @@ describe Notice do
183 end 185 end
184 186
185 end 187 end
  188 +