Merge branch 'ticket/unstable/310-skip-issue-mail-notifications' into unstable

Conflicts: config/locales/en.yml test/unit/journal_test.rb

Merge branch 'ticket/unstable/310-skip-issue-mail-notifications' into unstable
Conflicts: config/locales/en.yml test/unit/journal_test.rb
614bad89 · Eric Davis · d02d8e7e · b96115bf · 614bad89 · 614bad89
Commit 614bad89 authored Apr 29, 2011 by Eric Davis
11 changed files
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -121,6 +121,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)
@@ -155,7 +156,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?
@@ -195,6 +196,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

--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -289,4 +289,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
--- 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)
+    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
--- 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
--- a/app/views/issues/_edit.rhtml
+++ b/app/views/issues/_edit.rhtml
@@ -43,6 +43,8 @@
      <%= render :partial => 'attachments/form' %>
      </div>
    </fieldset>
+
+    <%= send_notification_option %>
    </div>
    
    <%= f.hidden_field :lock_version %>

--- a/app/views/issues/bulk_edit.rhtml
+++ b/app/views/issues/bulk_edit.rhtml
@@ -84,6 +84,7 @@
 <fieldset><legend><%= l(:field_notes) %></legend>
 <%= text_area_tag 'notes', @notes, :cols => 60, :rows => 10, :class => 'wiki-edit' %>
 <%= wikitoolbar_for 'notes' %>
+<%= send_notification_option %>
 </fieldset>
 </div>


--- a/app/views/issues/new.rhtml
+++ b/app/views/issues/new.rhtml
@@ -5,6 +5,7 @@
    <%= error_messages_for 'issue' %>
    <div class="box">
    <%= render :partial => 'issues/form', :locals => {:f => f} %>
+    <%= send_notification_option %>
    </div>
    <%= submit_tag l(:button_create) %>
    <%= submit_tag l(:button_create_and_continue), :name => 'continue' %>

--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -808,6 +808,7 @@ en:
  label_filesystem_path: Root directory
  label_additional_workflow_transitions_for_assignee: Additional transitions allowed when the user is the assignee
  label_additional_workflow_transitions_for_author: Additional transitions allowed when the user is the author
+  label_notify_member_plural: Email issue updates

  button_login: Login
  button_submit: Submit

--- 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
@@ -1002,6 +1018,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_hours_only
    @request.session[:user_id] = 2
    notes = 'Note added by IssuesControllerTest#test_post_edit_with_invalid_spent_time'
@@ -1139,6 +1171,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

--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -894,4 +894,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
--- a/test/unit/journal_test.rb
+++ b/test/unit/journal_test.rb
@@ -87,4 +87,15 @@ class JournalTest < ActiveSupport::TestCase
    # Admin should see issues on private projects that he does not belong to
    assert journals.detect {|journal| !journal.issue.project.is_public?}
  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