Added observers to watch model objects for mail delivery instead of calling Mailer.

* Added an IssueObserver to watch when Issues are created
* Added a JournalObserver to watch when Journals are created (Issue updates)
* Added a NewsObserver for News items.
* Added a DocumentObserver for Document notifications.
* Setup IssuesController#new to use the IssueObserver.
* Setup IssuesController#edit to use the IssueObserver.
* Setup IssuesController#bulk_edit to use the JournalObserver.
* Removed the Mailer call in Changeset#scan_commit_for_issue_ids, the
  JournalObserver will handle it.
* Removed Mailer calls in MailHandler in favor of the Observers.

  #2659

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@2637 e93f8b46-1217-0410-a6f0-8f06a7374b81
This commit is contained in:
Eric Davis 2009-03-28 00:38:57 +00:00
parent 3557e767e0
commit b4be8849c0
19 changed files with 206 additions and 18 deletions

View File

@ -48,7 +48,6 @@ class DocumentsController < ApplicationController
if request.post? and @document.save if request.post? and @document.save
attach_files(@document, params[:attachments]) attach_files(@document, params[:attachments])
flash[:notice] = l(:notice_successful_create) flash[:notice] = l(:notice_successful_create)
Mailer.deliver_document_added(@document) if Setting.notified_events.include?('document_added')
redirect_to :action => 'index', :project_id => @project redirect_to :action => 'index', :project_id => @project
end end
end end

View File

@ -146,7 +146,6 @@ class IssuesController < ApplicationController
if @issue.save if @issue.save
attach_files(@issue, params[:attachments]) attach_files(@issue, params[:attachments])
flash[:notice] = l(:notice_successful_create) flash[:notice] = l(:notice_successful_create)
Mailer.deliver_issue_add(@issue) if Setting.notified_events.include?('issue_added')
call_hook(:controller_issues_new_after_save, { :params => params, :issue => @issue}) call_hook(:controller_issues_new_after_save, { :params => params, :issue => @issue})
redirect_to(params[:continue] ? { :action => 'new', :tracker_id => @issue.tracker } : redirect_to(params[:continue] ? { :action => 'new', :tracker_id => @issue.tracker } :
{ :action => 'show', :id => @issue }) { :action => 'show', :id => @issue })
@ -193,7 +192,6 @@ class IssuesController < ApplicationController
if !journal.new_record? if !journal.new_record?
# Only send notification if something was actually changed # Only send notification if something was actually changed
flash[:notice] = l(:notice_successful_update) flash[:notice] = l(:notice_successful_update)
Mailer.deliver_issue_edit(journal) if Setting.notified_events.include?('issue_updated')
end end
call_hook(:controller_issues_edit_after_save, { :params => params, :issue => @issue, :time_entry => @time_entry, :journal => journal}) call_hook(:controller_issues_edit_after_save, { :params => params, :issue => @issue, :time_entry => @time_entry, :journal => journal})
redirect_to(params[:back_to] || {:action => 'show', :id => @issue}) redirect_to(params[:back_to] || {:action => 'show', :id => @issue})
@ -247,10 +245,7 @@ class IssuesController < ApplicationController
issue.custom_field_values = custom_field_values if custom_field_values && !custom_field_values.empty? issue.custom_field_values = custom_field_values if custom_field_values && !custom_field_values.empty?
call_hook(:controller_issues_bulk_edit_before_save, { :params => params, :issue => issue }) call_hook(:controller_issues_bulk_edit_before_save, { :params => params, :issue => issue })
# Don't save any change to the issue if the user is not authorized to apply the requested status # Don't save any change to the issue if the user is not authorized to apply the requested status
if (status.nil? || (issue.status.new_status_allowed_to?(status, current_role, issue.tracker) && issue.status = status)) && issue.save unless (status.nil? || (issue.status.new_status_allowed_to?(status, current_role, issue.tracker) && issue.status = status)) && issue.save
# Send notification for each issue (if changed)
Mailer.deliver_issue_edit(journal) if journal.details.any? && Setting.notified_events.include?('issue_updated')
else
# Keep unsaved issue ids to display them in flash error # Keep unsaved issue ids to display them in flash error
unsaved_issue_ids << issue.id unsaved_issue_ids << issue.id
end end

View File

@ -45,7 +45,6 @@ class NewsController < ApplicationController
@news.attributes = params[:news] @news.attributes = params[:news]
if @news.save if @news.save
flash[:notice] = l(:notice_successful_create) flash[:notice] = l(:notice_successful_create)
Mailer.deliver_news_added(@news) if Setting.notified_events.include?('news_added')
redirect_to :controller => 'news', :action => 'index', :project_id => @project redirect_to :controller => 'news', :action => 'index', :project_id => @project
end end
end end

View File

@ -113,7 +113,6 @@ class Changeset < ActiveRecord::Base
issue.status = fix_status issue.status = fix_status
issue.done_ratio = done_ratio if done_ratio issue.done_ratio = done_ratio if done_ratio
issue.save issue.save
Mailer.deliver_issue_edit(journal) if Setting.notified_events.include?('issue_updated')
end end
end end
referenced_issues += target_issues referenced_issues += target_issues

View File

@ -0,0 +1,22 @@
# redMine - project management software
# Copyright (C) 2006-2007 Jean-Philippe Lang
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
class DocumentObserver < ActiveRecord::Observer
def after_create(document)
Mailer.deliver_document_added(document) if Setting.notified_events.include?('document_added')
end
end

View File

@ -0,0 +1,22 @@
# redMine - project management software
# Copyright (C) 2006-2007 Jean-Philippe Lang
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
class IssueObserver < ActiveRecord::Observer
def after_create(issue)
Mailer.deliver_issue_add(issue) if Setting.notified_events.include?('issue_added')
end
end

View File

@ -0,0 +1,22 @@
# redMine - project management software
# Copyright (C) 2006-2007 Jean-Philippe Lang
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
class JournalObserver < ActiveRecord::Observer
def after_create(journal)
Mailer.deliver_issue_edit(journal) if Setting.notified_events.include?('issue_updated')
end
end

View File

@ -110,13 +110,11 @@ class MailHandler < ActionMailer::Base
end end
h h
end end
# add To and Cc as watchers before saving so the watchers can reply to Redmine
add_watchers(issue)
issue.save! issue.save!
add_attachments(issue) add_attachments(issue)
logger.info "MailHandler: issue ##{issue.id} created by #{user}" if logger && logger.info logger.info "MailHandler: issue ##{issue.id} created by #{user}" if logger && logger.info
# add To and Cc as watchers
add_watchers(issue)
# send notification after adding watchers so that they can reply to Redmine
Mailer.deliver_issue_add(issue) if Setting.notified_events.include?('issue_added')
issue issue
end end
@ -148,7 +146,6 @@ class MailHandler < ActionMailer::Base
end end
issue.save! issue.save!
logger.info "MailHandler: issue ##{issue.id} updated by #{user}" if logger && logger.info logger.info "MailHandler: issue ##{issue.id} updated by #{user}" if logger && logger.info
Mailer.deliver_issue_edit(journal) if Setting.notified_events.include?('issue_updated')
journal journal
end end

View File

@ -0,0 +1,22 @@
# redMine - project management software
# Copyright (C) 2006-2007 Jean-Philippe Lang
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
class NewsObserver < ActiveRecord::Observer
def after_create(news)
Mailer.deliver_news_added(news) if Setting.notified_events.include?('news_added')
end
end

View File

@ -36,7 +36,7 @@ Rails::Initializer.run do |config|
# Activate observers that should always be running # Activate observers that should always be running
# config.active_record.observers = :cacher, :garbage_collector # config.active_record.observers = :cacher, :garbage_collector
config.active_record.observers = :message_observer config.active_record.observers = :message_observer, :issue_observer, :journal_observer, :news_observer, :document_observer
# Make Active Record use UTC-base instead of local time # Make Active Record use UTC-base instead of local time
# config.active_record.default_timezone = :utc # config.active_record.default_timezone = :utc

View File

@ -66,6 +66,8 @@ class DocumentsControllerTest < Test::Unit::TestCase
end end
def test_new_with_one_attachment def test_new_with_one_attachment
ActionMailer::Base.deliveries.clear
Setting.notified_events << 'document_added'
@request.session[:user_id] = 2 @request.session[:user_id] = 2
set_tmp_attachments_directory set_tmp_attachments_directory
@ -82,6 +84,7 @@ class DocumentsControllerTest < Test::Unit::TestCase
assert_equal Enumeration.find(2), document.category assert_equal Enumeration.find(2), document.category
assert_equal 1, document.attachments.size assert_equal 1, document.attachments.size
assert_equal 'testfile.txt', document.attachments.first.filename assert_equal 'testfile.txt', document.attachments.first.filename
assert_equal 1, ActionMailer::Base.deliveries.size
end end
def test_edit_routing def test_edit_routing

View File

@ -503,6 +503,21 @@ class IssuesControllerTest < Test::Unit::TestCase
assert [mail.bcc, mail.cc].flatten.include?(User.find(3).mail) assert [mail.bcc, mail.cc].flatten.include?(User.find(3).mail)
end end
def test_post_new_should_send_a_notification
ActionMailer::Base.deliveries.clear
@request.session[:user_id] = 2
post :new, :project_id => 1,
:issue => {:tracker_id => 3,
:subject => 'This is the test_new issue',
:description => 'This is the description',
:priority_id => 5,
:estimated_hours => '',
:custom_field_values => {'2' => 'Value for field 2'}}
assert_redirected_to :action => 'show'
assert_equal 1, ActionMailer::Base.deliveries.size
end
def test_post_should_preserve_fields_values_on_validation_failure def test_post_should_preserve_fields_values_on_validation_failure
@request.session[:user_id] = 2 @request.session[:user_id] = 2
post :new, :project_id => 1, post :new, :project_id => 1,
@ -760,6 +775,20 @@ class IssuesControllerTest < Test::Unit::TestCase
# No email should be sent # No email should be sent
assert ActionMailer::Base.deliveries.empty? assert ActionMailer::Base.deliveries.empty?
end end
def test_post_edit_should_send_a_notification
@request.session[:user_id] = 2
ActionMailer::Base.deliveries.clear
issue = Issue.find(1)
old_subject = issue.subject
new_subject = 'Subject modified by IssuesControllerTest#test_post_edit'
post :edit, :id => 1, :issue => {:subject => new_subject,
:priority_id => '6',
:category_id => '1' # no change
}
assert_equal 1, ActionMailer::Base.deliveries.size
end
def test_post_edit_with_invalid_spent_time def test_post_edit_with_invalid_spent_time
@request.session[:user_id] = 2 @request.session[:user_id] = 2
@ -797,6 +826,22 @@ class IssuesControllerTest < Test::Unit::TestCase
assert_equal 1, journal.details.size assert_equal 1, journal.details.size
end end
def test_bullk_edit_should_send_a_notification
@request.session[:user_id] = 2
ActionMailer::Base.deliveries.clear
post(:bulk_edit,
{
:ids => [1, 2],
:priority_id => 7,
:assigned_to_id => '',
:custom_field_values => {'2' => ''},
:notes => 'Bulk editing'
})
assert_response 302
assert_equal 2, ActionMailer::Base.deliveries.size
end
def test_bulk_edit_custom_field def test_bulk_edit_custom_field
@request.session[:user_id] = 2 @request.session[:user_id] = 2
# update issues priority # update issues priority

View File

@ -112,6 +112,9 @@ class NewsControllerTest < Test::Unit::TestCase
end end
def test_post_new def test_post_new
ActionMailer::Base.deliveries.clear
Setting.notified_events << 'news_added'
@request.session[:user_id] = 2 @request.session[:user_id] = 2
post :new, :project_id => 1, :news => { :title => 'NewsControllerTest', post :new, :project_id => 1, :news => { :title => 'NewsControllerTest',
:description => 'This is the description', :description => 'This is the description',
@ -123,6 +126,7 @@ class NewsControllerTest < Test::Unit::TestCase
assert_equal 'This is the description', news.description assert_equal 'This is the description', news.description
assert_equal User.find(2), news.author assert_equal User.find(2), news.author
assert_equal Project.find(1), news.project assert_equal Project.find(1), news.project
assert_equal 1, ActionMailer::Base.deliveries.size
end end
def test_edit_routing def test_edit_routing

View File

@ -24,6 +24,7 @@ class ChangesetTest < Test::Unit::TestCase
end end
def test_ref_keywords_any def test_ref_keywords_any
ActionMailer::Base.deliveries.clear
Setting.commit_fix_status_id = IssueStatus.find(:first, :conditions => ["is_closed = ?", true]).id Setting.commit_fix_status_id = IssueStatus.find(:first, :conditions => ["is_closed = ?", true]).id
Setting.commit_fix_done_ratio = '90' Setting.commit_fix_done_ratio = '90'
Setting.commit_ref_keywords = '*' Setting.commit_ref_keywords = '*'
@ -38,6 +39,7 @@ class ChangesetTest < Test::Unit::TestCase
fixed = Issue.find(1) fixed = Issue.find(1)
assert fixed.closed? assert fixed.closed?
assert_equal 90, fixed.done_ratio assert_equal 90, fixed.done_ratio
assert_equal 1, ActionMailer::Base.deliveries.size
end end
def test_ref_keywords_any_line_start def test_ref_keywords_any_line_start

View File

@ -25,6 +25,15 @@ class DocumentTest < Test::Unit::TestCase
assert doc.save assert doc.save
end end
def test_create_should_send_email_notification
ActionMailer::Base.deliveries.clear
Setting.notified_events << 'document_added'
doc = Document.new(:project => Project.find(1), :title => 'New document', :category => Enumeration.find_by_name('User documentation'))
assert doc.save
assert_equal 1, ActionMailer::Base.deliveries.size
end
def test_create_with_default_category def test_create_with_default_category
# Sets a default category # Sets a default category
e = Enumeration.find_by_name('Technical documentation') e = Enumeration.find_by_name('Technical documentation')

View File

@ -241,4 +241,12 @@ class IssueTest < Test::Unit::TestCase
assert !Issue.new(:due_date => nil).overdue? assert !Issue.new(:due_date => nil).overdue?
assert !Issue.new(:due_date => 1.day.ago.to_date, :status => IssueStatus.find(:first, :conditions => {:is_closed => true})).overdue? assert !Issue.new(:due_date => 1.day.ago.to_date, :status => IssueStatus.find(:first, :conditions => {:is_closed => true})).overdue?
end end
def test_create_should_send_email_notification
ActionMailer::Base.deliveries.clear
issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => Enumeration.priorities.first, :subject => 'test_create', :estimated_hours => '1:30')
assert issue.save
assert_equal 1, ActionMailer::Base.deliveries.size
end
end end

View File

@ -36,4 +36,15 @@ class JournalTest < Test::Unit::TestCase
assert_kind_of IssueStatus, status assert_kind_of IssueStatus, status
assert_equal 2, status.id assert_equal 2, status.id
end end
def test_create_should_send_email_notification
ActionMailer::Base.deliveries.clear
issue = Issue.find(:first)
user = User.find(:first)
journal = issue.init_journal(user, issue)
assert journal.save
assert_equal 1, ActionMailer::Base.deliveries.size
end
end end

View File

@ -133,6 +133,14 @@ class MailHandlerTest < Test::Unit::TestCase
Role.anonymous.add_permission!(:add_issues) Role.anonymous.add_permission!(:add_issues)
assert_equal false, submit_email('ticket_without_from_header.eml') assert_equal false, submit_email('ticket_without_from_header.eml')
end end
def test_add_issue_should_send_email_notification
ActionMailer::Base.deliveries.clear
# This email contains: 'Project: onlinestore'
issue = submit_email('ticket_on_given_project.eml')
assert issue.is_a?(Issue)
assert_equal 1, ActionMailer::Base.deliveries.size
end
def test_add_issue_note def test_add_issue_note
journal = submit_email('ticket_reply.eml') journal = submit_email('ticket_reply.eml')
@ -152,6 +160,13 @@ class MailHandlerTest < Test::Unit::TestCase
assert_match /This is reply/, journal.notes assert_match /This is reply/, journal.notes
assert_equal IssueStatus.find_by_name("Resolved"), issue.status assert_equal IssueStatus.find_by_name("Resolved"), issue.status
end end
def test_add_issue_note_should_send_email_notification
ActionMailer::Base.deliveries.clear
journal = submit_email('ticket_reply.eml')
assert journal.is_a?(Journal)
assert_equal 1, ActionMailer::Base.deliveries.size
end
def test_reply_to_a_message def test_reply_to_a_message
m = submit_email('message_reply.eml') m = submit_email('message_reply.eml')

View File

@ -20,9 +20,23 @@ require File.dirname(__FILE__) + '/../test_helper'
class NewsTest < Test::Unit::TestCase class NewsTest < Test::Unit::TestCase
fixtures :projects, :users, :roles, :members, :enabled_modules, :news fixtures :projects, :users, :roles, :members, :enabled_modules, :news
def valid_news
{ :title => 'Test news', :description => 'Lorem ipsum etc', :author => User.find(:first) }
end
def setup def setup
end end
def test_create_should_send_email_notification
ActionMailer::Base.deliveries.clear
Setting.notified_events << 'news_added'
news = Project.find(:first).news.new(valid_news)
assert news.save
assert_equal 1, ActionMailer::Base.deliveries.size
end
def test_should_include_news_for_projects_with_news_enabled def test_should_include_news_for_projects_with_news_enabled
project = projects(:projects_001) project = projects(:projects_001)
assert project.enabled_modules.any?{ |em| em.name == 'news' } assert project.enabled_modules.any?{ |em| em.name == 'news' }
@ -37,7 +51,7 @@ class NewsTest < Test::Unit::TestCase
assert ! project.enabled_modules.any?{ |em| em.name == 'news' } assert ! project.enabled_modules.any?{ |em| em.name == 'news' }
# Add a piece of news to the project # Add a piece of news to the project
news = project.news.create(:title => 'Test news', :description => 'This should not be returned by News.latest') news = project.news.create(valid_news)
# News.latest should not return that new piece of news # News.latest should not return that new piece of news
assert News.latest.include?(news) == false assert News.latest.include?(news) == false
@ -50,14 +64,14 @@ class NewsTest < Test::Unit::TestCase
def test_should_limit_the_amount_of_returned_news def test_should_limit_the_amount_of_returned_news
# Make sure we have a bunch of news stories # Make sure we have a bunch of news stories
10.times { projects(:projects_001).news.create(:title => 'Test news', :description => 'Lorem ipsum etc') } 10.times { projects(:projects_001).news.create(valid_news) }
assert_equal 2, News.latest(users(:users_002), 2).size assert_equal 2, News.latest(users(:users_002), 2).size
assert_equal 6, News.latest(users(:users_002), 6).size assert_equal 6, News.latest(users(:users_002), 6).size
end end
def test_should_return_5_news_stories_by_default def test_should_return_5_news_stories_by_default
# Make sure we have a bunch of news stories # Make sure we have a bunch of news stories
10.times { projects(:projects_001).news.create(:title => 'Test news', :description => 'Lorem ipsum etc') } 10.times { projects(:projects_001).news.create(valid_news) }
assert_equal 5, News.latest(users(:users_004)).size assert_equal 5, News.latest(users(:users_004)).size
end end
end end