From b96115bf7b061d9b3c755fb0bc8b3b06bf803f95 Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Wed, 30 Mar 2011 10:40:00 -0700 Subject: [PATCH] [#310 LSS#5727] Add checkbox option to determine if issue emails should be sent --- app/controllers/issues_controller.rb | 4 +- app/helpers/issues_helper.rb | 10 +++++ app/models/issue_observer.rb | 20 ++++++++- app/models/journal_observer.rb | 18 +++++++- app/views/issues/_edit.rhtml | 2 + app/views/issues/bulk_edit.rhtml | 1 + app/views/issues/new.rhtml | 1 + config/locales/en.yml | 1 + test/functional/issues_controller_test.rb | 51 +++++++++++++++++++++++ test/unit/issue_test.rb | 9 ++++ test/unit/journal_test.rb | 10 +++++ 11 files changed, 124 insertions(+), 3 deletions(-) diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 051f8910..55c42a60 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -132,6 +132,7 @@ class IssuesController < ApplicationController def create call_hook(:controller_issues_new_before_save, { :params => params, :issue => @issue }) + IssueObserver.instance.send_notification = params[:send_notification] == '0' ? false : true if @issue.save attachments = Attachment.attach_files(@issue, params[:attachments]) render_attachment_warning_if_needed(@issue) @@ -166,7 +167,7 @@ class IssuesController < ApplicationController def update update_issue_from_params - + JournalObserver.instance.send_notification = params[:send_notification] == '0' ? false : true if @issue.save_issue_with_child_records(params, @time_entry) render_attachment_warning_if_needed(@issue) flash[:notice] = l(:notice_successful_update) unless @issue.current_journal.new_record? @@ -206,6 +207,7 @@ class IssuesController < ApplicationController journal = issue.init_journal(User.current, params[:notes]) issue.safe_attributes = attributes call_hook(:controller_issues_bulk_edit_before_save, { :params => params, :issue => issue }) + JournalObserver.instance.send_notification = params[:send_notification] == '0' ? false : true unless issue.save # Keep unsaved issue ids to display them in flash error unsaved_issue_ids << issue.id diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index ce23668e..6faadf2a 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -260,4 +260,14 @@ module IssuesHelper end export end + + def send_notification_option + content_tag(:p, + content_tag(:label, + l(:label_notify_member_plural)) + + hidden_field_tag('send_notification', '0') + + check_box_tag('send_notification', '1', true)) + + + end end diff --git a/app/models/issue_observer.rb b/app/models/issue_observer.rb index bdb5c1d4..2a563a5a 100644 --- a/app/models/issue_observer.rb +++ b/app/models/issue_observer.rb @@ -16,7 +16,25 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. class IssueObserver < ActiveRecord::Observer + attr_accessor :send_notification + def after_create(issue) - Mailer.deliver_issue_add(issue) if Setting.notified_events.include?('issue_added') + if self.send_notification + Mailer.deliver_issue_add(issue) if Setting.notified_events.include?('issue_added') + end + clear_notification + end + + # Wrap send_notification so it defaults to true, when it's nil + def send_notification + return true if @send_notification.nil? + return @send_notification + end + + private + + # Need to clear the notification setting after each usage otherwise it might be cached + def clear_notification + @send_notification = true end end diff --git a/app/models/journal_observer.rb b/app/models/journal_observer.rb index db7115cd..b006ad64 100644 --- a/app/models/journal_observer.rb +++ b/app/models/journal_observer.rb @@ -16,12 +16,28 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. class JournalObserver < ActiveRecord::Observer + attr_accessor :send_notification + def after_create(journal) if Setting.notified_events.include?('issue_updated') || (Setting.notified_events.include?('issue_note_added') && journal.notes.present?) || (Setting.notified_events.include?('issue_status_updated') && journal.new_status.present?) || (Setting.notified_events.include?('issue_priority_updated') && journal.new_value_for('priority_id').present?) - Mailer.deliver_issue_edit(journal) + Mailer.deliver_issue_edit(journal) if self.send_notification end + clear_notification + end + + # Wrap send_notification so it defaults to true, when it's nil + def send_notification + return true if @send_notification.nil? + return @send_notification + end + + private + + # Need to clear the notification setting after each usage otherwise it might be cached + def clear_notification + @send_notification = true end end diff --git a/app/views/issues/_edit.rhtml b/app/views/issues/_edit.rhtml index 6d74751f..6c9bc3d7 100644 --- a/app/views/issues/_edit.rhtml +++ b/app/views/issues/_edit.rhtml @@ -43,6 +43,8 @@ <%= render :partial => 'attachments/form' %> + + <%= send_notification_option %> <%= f.hidden_field :lock_version %> diff --git a/app/views/issues/bulk_edit.rhtml b/app/views/issues/bulk_edit.rhtml index b26e47c8..d7621e05 100644 --- a/app/views/issues/bulk_edit.rhtml +++ b/app/views/issues/bulk_edit.rhtml @@ -76,6 +76,7 @@
<%= l(:field_notes) %> <%= text_area_tag 'notes', @notes, :cols => 60, :rows => 10, :class => 'wiki-edit' %> <%= wikitoolbar_for 'notes' %> +<%= send_notification_option %>
diff --git a/app/views/issues/new.rhtml b/app/views/issues/new.rhtml index 867590fa..b9d6f414 100644 --- a/app/views/issues/new.rhtml +++ b/app/views/issues/new.rhtml @@ -5,6 +5,7 @@ <%= error_messages_for 'issue' %>
<%= render :partial => 'issues/form', :locals => {:f => f} %> + <%= send_notification_option %>
<%= submit_tag l(:button_create) %> <%= submit_tag l(:button_create_and_continue), :name => 'continue' %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 382273f5..edb0b41c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -800,6 +800,7 @@ en: label_cvs_module: Module label_bazaar_path: Root directory label_filesystem_path: Root directory + label_notify_member_plural: Email issue updates button_login: Login button_submit: Submit diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index b21b28e6..0edcf9c0 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -430,6 +430,22 @@ class IssuesControllerTest < ActionController::TestCase assert_equal 'Value for field 2', v.value end + def test_post_new_should_not_send_a_notification_if_send_notification_is_off + ActionMailer::Base.deliveries.clear + @request.session[:user_id] = 2 + post :create, :project_id => 1, + :send_notification => 0, + :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 :controller => 'issues', :action => 'show', :id => Issue.last.id + + assert_equal 0, ActionMailer::Base.deliveries.size + end + def test_post_create_without_start_date @request.session[:user_id] = 2 assert_difference 'Issue.count' do @@ -1001,6 +1017,22 @@ class IssuesControllerTest < ActionController::TestCase } assert_equal 1, ActionMailer::Base.deliveries.size end + + def test_put_update_should_not_send_a_notification_if_send_notification_is_off + @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' + + put :update, :id => 1, + :send_notification => 0, + :issue => {:subject => new_subject, + :priority_id => '6', + :category_id => '1' # no change + } + assert_equal 0, ActionMailer::Base.deliveries.size + end def test_put_update_with_invalid_spent_time @request.session[:user_id] = 2 @@ -1115,6 +1147,25 @@ class IssuesControllerTest < ActionController::TestCase assert_equal 1, journal.details.size end + def test_bullk_update_should_not_send_a_notification_if_send_notification_is_off + @request.session[:user_id] = 2 + ActionMailer::Base.deliveries.clear + post(:bulk_update, + { + :ids => [1, 2], + :issue => { + :priority_id => 7, + :assigned_to_id => '', + :custom_field_values => {'2' => ''} + }, + :notes => 'Bulk editing', + :send_notification => '0' + }) + + assert_response 302 + assert_equal 0, ActionMailer::Base.deliveries.size + end + def test_bulk_update_on_different_projects @request.session[:user_id] = 2 # update issues priority diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index d284c746..700548ca 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -832,4 +832,13 @@ class IssueTest < ActiveSupport::TestCase end end + + def test_create_should_not_send_email_notification_if_told_not_to + ActionMailer::Base.deliveries.clear + issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => IssuePriority.first, :subject => 'test_create', :estimated_hours => '1:30') + IssueObserver.instance.send_notification = false + + assert issue.save + assert_equal 0, ActionMailer::Base.deliveries.size + end end diff --git a/test/unit/journal_test.rb b/test/unit/journal_test.rb index 67e719df..fd2e6fdc 100644 --- a/test/unit/journal_test.rb +++ b/test/unit/journal_test.rb @@ -47,4 +47,14 @@ class JournalTest < ActiveSupport::TestCase assert_equal 1, ActionMailer::Base.deliveries.size end + def test_create_should_not_send_email_notification_if_told_not_to + ActionMailer::Base.deliveries.clear + issue = Issue.find(:first) + user = User.find(:first) + journal = issue.init_journal(user, issue) + JournalObserver.instance.send_notification = false + + assert journal.save + assert_equal 0, ActionMailer::Base.deliveries.size + end end