Update IssueObserver to send reassigned emails when an issue is reassigned.

2 files changed, 44 insertions, 34 deletions

diff --git a/app/models/issue_observer.rb b/app/models/issue_observer.rb
index e18195aa281..38c4fde91a9 100644
--- a/app/models/issue_observer.rb
+++ b/app/models/issue_observer.rb
@@ -6,12 +6,15 @@ class IssueObserver < ActiveRecord::Observer
   end
 
   def after_change(issue)
-    if issue.assignee_id_changed?
-      recipient_ids = [issue.assignee_id, issue.assignee_id_was].keep_if {|id| id != current_user.id }
+    send_reassigned_email(issue) if issue.is_being_reassigned?
+  end
+
+  def send_reassigned_email(issue)
+    recipient_ids = [issue.assignee_id, issue.assignee_id_was].keep_if {|id| id != current_user.id }
 
-      recipient_ids.each do |recipient_id|
-        Notify.reassigned_issue_email(recipient_id, issue.id, issue.assignee_id_was)
-      end
+    recipient_ids.each do |recipient_id|
+      Notify.reassigned_issue_email(recipient_id, issue.id, issue.assignee_id_was)
     end
   end
+
 end
diff --git a/spec/models/issue_observer_spec.rb b/spec/models/issue_observer_spec.rb
index 166e03ce916..7de9a0ae628 100644
--- a/spec/models/issue_observer_spec.rb
+++ b/spec/models/issue_observer_spec.rb
@@ -1,9 +1,9 @@
 require 'spec_helper'
 
 describe IssueObserver do
-  let(:some_user) { Factory.new(:user, :id => 1) }
-  let(:assignee) { Factory.new(:user, :id => 2) }
-  let(:issue)    { Factory.new(:issue, :id => 42, :assignee => assignee) }
+  let(:some_user) { double(:user, :id => 1) }
+  let(:assignee) { double(:user, :id => 2) }
+  let(:issue)    { double(:issue, :id => 42, :assignee => assignee) }
 
   before(:each) { subject.stub(:current_user).and_return(some_user) }
 
@@ -25,42 +25,49 @@ describe IssueObserver do
     end
   end
 
-  context 'when an issue is modified' do
-    it 'but not reassigned, does not send a reassigned email' do
-      issue.stub(:assignee_id_changed?).and_return(false)
-      Notify.should_not_receive(:reassigned_issue_email)
+  context 'when an issue is changed' do
+    it 'sends a reassigned email, if the issue is being reassigned' do
+      issue.should_receive(:is_being_reassigned?).and_return(true)
+      subject.should_receive(:send_reassigned_email).with(issue)
 
       subject.after_change(issue)
     end
 
-    context 'and is reassigned' do
-      let(:previous_assignee) { Factory.new(:user, :id => 3) }
+    it 'does not send a reassigned email, if the issue was not reassigned' do
+      issue.should_receive(:is_being_reassigned?).and_return(false)
+      subject.should_not_receive(:send_reassigned_email)
 
-      before(:each) do
-        issue.stub(:assignee_id_changed?).and_return(true)
-        issue.stub(:assignee_id_was).and_return(previous_assignee.id)
-      end
+      subject.after_change(issue)
+    end
+  end
 
-      it 'sends a reassigned email to the previous and current assignees' do
-        Notify.should_receive(:reassigned_issue_email).with(assignee.id, issue.id, previous_assignee.id)
-        Notify.should_receive(:reassigned_issue_email).with(previous_assignee.id, issue.id, previous_assignee.id)
+  describe '#send_reassigned_email' do
+    let(:previous_assignee) { double(:user, :id => 3) }
 
-        subject.after_change(issue)
-      end
+    before(:each) do
+      issue.stub(:assignee_id).and_return(assignee.id)
+      issue.stub(:assignee_id_was).and_return(previous_assignee.id)
+    end
+
+    it 'sends a reassigned email to the previous and current assignees' do
+      Notify.should_receive(:reassigned_issue_email).with(assignee.id, issue.id, previous_assignee.id)
+      Notify.should_receive(:reassigned_issue_email).with(previous_assignee.id, issue.id, previous_assignee.id)
 
-      context 'does not send an email to the user who made the reassignment' do
-        it 'if the user is the assignee' do
-          subject.stub(:current_user).and_return(assignee)
-          Notify.should_not_receive(:reassigned_issue_email).with(assignee.id, issue.id, previous_assignee.id)
+      subject.send_reassigned_email(issue)
+    end
 
-          subject.after_change(issue)
-        end
-        it 'if the user is the previous assignee' do
-          subject.stub(:current_user).and_return(previous_assignee)
-          Notify.should_not_receive(:reassigned_issue_email).with(previous_assignee.id, issue.id, previous_assignee.id)
+    context 'does not send an email to the user who made the reassignment' do
+      it 'if the user is the assignee' do
+        subject.stub(:current_user).and_return(assignee)
+        Notify.should_not_receive(:reassigned_issue_email).with(assignee.id, issue.id, previous_assignee.id)
+
+        subject.send_reassigned_email(issue)
+      end
+      it 'if the user is the previous assignee' do
+        subject.stub(:current_user).and_return(previous_assignee)
+        Notify.should_not_receive(:reassigned_issue_email).with(previous_assignee.id, issue.id, previous_assignee.id)
 
-          subject.after_change(issue)
-        end
+        subject.send_reassigned_email(issue)
       end
     end
   end