Commit e87d67669f678569b140911ab1da44fe63fe8fe6
1 parent
de50bcb5
Exists in
master
and in
1 other branch
Simplify Deploy by moving Mailer calls into observer
Showing
5 changed files
with
32 additions
and
21 deletions
Show diff stats
app/models/deploy.rb
| ... | ... | @@ -12,16 +12,11 @@ class Deploy |
| 12 | 12 | |
| 13 | 13 | embedded_in :app, :inverse_of => :deploys |
| 14 | 14 | |
| 15 | - after_create :deliver_notification, :if => :should_notify? | |
| 16 | 15 | after_create :resolve_app_errs, :if => :should_resolve_app_errs? |
| 17 | 16 | after_create :store_cached_attributes_on_problems |
| 18 | 17 | |
| 19 | 18 | validates_presence_of :username, :environment |
| 20 | 19 | |
| 21 | - def deliver_notification | |
| 22 | - Mailer.deploy_notification(self).deliver | |
| 23 | - end | |
| 24 | - | |
| 25 | 20 | def resolve_app_errs |
| 26 | 21 | app.problems.unresolved.in_env(environment).each {|problem| problem.resolve!} |
| 27 | 22 | end |
| ... | ... | @@ -32,10 +27,6 @@ class Deploy |
| 32 | 27 | |
| 33 | 28 | protected |
| 34 | 29 | |
| 35 | - def should_notify? | |
| 36 | - app.notify_on_deploys? && app.notification_recipients.any? | |
| 37 | - end | |
| 38 | - | |
| 39 | 30 | def should_resolve_app_errs? |
| 40 | 31 | app.resolve_errs_on_deploy? |
| 41 | 32 | end | ... | ... |
config/application.rb
| ... | ... | @@ -51,6 +51,9 @@ module Errbit |
| 51 | 51 | # IssueTracker subclasses use inheritance, so preloading models provides querying consistency in dev mode. |
| 52 | 52 | config.mongoid.preload_models = true |
| 53 | 53 | |
| 54 | + # Set up observers | |
| 55 | + config.mongoid.observers = :deploy_observer | |
| 56 | + | |
| 54 | 57 | # Configure the default encoding used in templates for Ruby 1.9. |
| 55 | 58 | config.encoding = "utf-8" |
| 56 | 59 | ... | ... |
| ... | ... | @@ -0,0 +1,20 @@ |
| 1 | +require 'spec_helper' | |
| 2 | + | |
| 3 | +describe DeployObserver do | |
| 4 | + context 'when a Deploy is saved' do | |
| 5 | + context 'and the app should notify on deploys' do | |
| 6 | + it 'should send an email notification' do | |
| 7 | + Mailer.should_receive(:deploy_notification). | |
| 8 | + and_return(mock('email', :deliver => true)) | |
| 9 | + Fabricate(:deploy, :app => Fabricate(:app_with_watcher, :notify_on_deploys => true)) | |
| 10 | + end | |
| 11 | + end | |
| 12 | + | |
| 13 | + context 'and the app is not set to notify on deploys' do | |
| 14 | + it 'should not send an email notification' do | |
| 15 | + Mailer.should_not_receive(:deploy_notification) | |
| 16 | + Fabricate(:deploy, :app => Fabricate(:app_with_watcher, :notify_on_deploys => false)) | |
| 17 | + end | |
| 18 | + end | |
| 19 | + end | |
| 20 | +end | ... | ... |
spec/models/deploy_spec.rb
| ... | ... | @@ -17,12 +17,6 @@ describe Deploy do |
| 17 | 17 | end |
| 18 | 18 | |
| 19 | 19 | context 'being created' do |
| 20 | - it 'should send an email notification' do | |
| 21 | - Mailer.should_receive(:deploy_notification). | |
| 22 | - and_return(mock('email', :deliver => true)) | |
| 23 | - Fabricate(:deploy, :app => Fabricate(:app_with_watcher, :notify_on_deploys => true)) | |
| 24 | - end | |
| 25 | - | |
| 26 | 20 | context 'when the app has resolve_errs_on_deploy set to false' do |
| 27 | 21 | it 'should not resolve the apps errs' do |
| 28 | 22 | app = Fabricate(:app, :resolve_errs_on_deploy => false) |
| ... | ... | @@ -43,12 +37,6 @@ describe Deploy do |
| 43 | 37 | end |
| 44 | 38 | end |
| 45 | 39 | |
| 46 | - context 'when the app has deploy notifications set to false' do | |
| 47 | - it 'should not send an email notification' do | |
| 48 | - Mailer.should_not_receive(:deploy_notification) | |
| 49 | - Fabricate(:deploy, :app => Fabricate(:app_with_watcher, :notify_on_deploys => false)) | |
| 50 | - end | |
| 51 | - end | |
| 52 | 40 | end |
| 53 | 41 | |
| 54 | 42 | it "should produce a shortened revision with 7 characters" do | ... | ... |