Make IssueObserver handle issus, not MailerObserver

6 files changed, 64 insertions, 21 deletions

diff --git a/app/models/issue.rb b/app/models/issue.rb
index fd9db264326..2b4b311da78 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -68,6 +68,10 @@ class Issue < ActiveRecord::Base
   def is_being_closed?
     closed_changed? && closed
   end
+
+  def is_being_reopened?
+    closed_changed? && !closed
+  end
 end
 # == Schema Information
 #
diff --git a/app/models/issue_observer.rb b/app/models/issue_observer.rb
index 461b3eb0dfe..7215dc56332 100644
--- a/app/models/issue_observer.rb
+++ b/app/models/issue_observer.rb
@@ -5,9 +5,10 @@ class IssueObserver < ActiveRecord::Observer
     Notify.new_issue_email(issue.id) if issue.assignee != current_user
   end
 
-  def after_change(issue)
+  def after_update(issue)
     send_reassigned_email(issue) if issue.is_being_reassigned?
     Note.create_status_change_note(issue, current_user, 'closed') if issue.is_being_closed?
+    Note.create_status_change_note(issue, current_user, 'reopened') if issue.is_being_reopened?
   end
 
   def send_reassigned_email(issue)
diff --git a/app/models/mailer_observer.rb b/app/models/mailer_observer.rb
index dbbd7d2f86a..80ce8b365e2 100644
--- a/app/models/mailer_observer.rb
+++ b/app/models/mailer_observer.rb
@@ -3,7 +3,6 @@ class MailerObserver < ActiveRecord::Observer
   cattr_accessor :current_user
 
   def after_create(model)
-    new_issue(model) if model.kind_of?(Issue)
     new_user(model) if model.kind_of?(User)
     new_note(model) if model.kind_of?(Note)
     new_merge_request(model) if model.kind_of?(MergeRequest)
@@ -11,17 +10,10 @@ class MailerObserver < ActiveRecord::Observer
 
   def after_update(model)
     changed_merge_request(model) if model.kind_of?(MergeRequest)
-    changed_issue(model) if model.kind_of?(Issue)
   end
 
   protected
 
-  def new_issue(issue)
-    if issue.assignee != current_user
-      Notify.new_issue_email(issue.id).deliver
-    end
-  end
-
   def new_user(user)
     Notify.new_user_email(user.id, user.password).deliver
   end
@@ -65,12 +57,8 @@ class MailerObserver < ActiveRecord::Observer
     status_notify_and_comment merge_request, :reassigned_merge_request_email
   end
 
-  def changed_issue(issue)
-    status_notify_and_comment issue, :reassigned_issue_email
-  end
-
   # This method used for Issues & Merge Requests
-  # 
+  #
   # It create a comment for Issue or MR if someone close/reopen.
   # It also notify via email if assignee was changed 
   #
diff --git a/config/application.rb b/config/application.rb
index 7bd5703b31a..4531b5ead6c 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -23,7 +23,7 @@ module Gitlab
     # config.plugins = [ :exception_notification, :ssl_requirement, :all ]
 
     # Activate observers that should always be running.
-    config.active_record.observers = :mailer_observer, :activity_observer, :project_observer, :key_observer
+    config.active_record.observers = :mailer_observer, :activity_observer, :project_observer, :key_observer, :issue_observer
 
     # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone.
     # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC.
diff --git a/spec/models/issue_observer_spec.rb b/spec/models/issue_observer_spec.rb
index fec09488d89..42cf81c5fff 100644
--- a/spec/models/issue_observer_spec.rb
+++ b/spec/models/issue_observer_spec.rb
@@ -9,7 +9,12 @@ describe IssueObserver do
 
   subject { IssueObserver.instance }
 
-  context 'when an issue is created' do
+  describe '#after_create' do
+
+    it 'is called when an issue is created' do
+      subject.should_receive(:after_create)
+      Factory.create(:issue, :project => Factory.create(:project))
+    end
 
     it 'sends an email to the assignee' do
       Notify.should_receive(:new_issue_email).with(issue.id)
@@ -25,10 +30,18 @@ describe IssueObserver do
     end
   end
 
-  context 'when an issue is changed' do
+  context '#after_update' do
     before(:each) do
       issue.stub(:is_being_reassigned?).and_return(false)
       issue.stub(:is_being_closed?).and_return(false)
+      issue.stub(:is_being_reopened?).and_return(false)
+    end
+
+    it 'is called when an issue is changed' do
+      changed = Factory.create(:issue, :project => Factory.create(:project))
+      subject.should_receive(:after_update)
+      changed.description = 'I changed'
+      changed.save
     end
 
     context 'a reassigned email' do
@@ -36,14 +49,14 @@ describe IssueObserver do
         issue.should_receive(:is_being_reassigned?).and_return(true)
         subject.should_receive(:send_reassigned_email).with(issue)
 
-        subject.after_change(issue)
+        subject.after_update(issue)
       end
 
       it 'is not sent if the issue is not being reassigned' do
         issue.should_receive(:is_being_reassigned?).and_return(false)
         subject.should_not_receive(:send_reassigned_email)
 
-        subject.after_change(issue)
+        subject.after_update(issue)
       end
     end
 
@@ -52,14 +65,30 @@ describe IssueObserver do
         issue.should_receive(:is_being_closed?).and_return(true)
         Note.should_receive(:create_status_change_note).with(issue, some_user, 'closed')
 
-        subject.after_change(issue)
+        subject.after_update(issue)
       end
 
       it 'is not created if the issue is not being closed' do
         issue.should_receive(:is_being_closed?).and_return(false)
         Note.should_not_receive(:create_status_change_note).with(issue, some_user, 'closed')
 
-        subject.after_change(issue)
+        subject.after_update(issue)
+      end
+    end
+
+    context 'a status "reopened" note' do
+      it 'is created if the issue is being reopened' do
+        issue.should_receive(:is_being_reopened?).and_return(true)
+        Note.should_receive(:create_status_change_note).with(issue, some_user, 'reopened')
+
+        subject.after_update(issue)
+      end
+
+      it 'is not created if the issue is not being reopened' do
+        issue.should_receive(:is_being_reopened?).and_return(false)
+        Note.should_not_receive(:create_status_change_note).with(issue, some_user, 'reopened')
+
+        subject.after_update(issue)
       end
     end
   end
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 9668f0b23f2..fd3af62d74d 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -55,6 +55,26 @@ describe Issue do
     end
   end
 
+
+  describe '#is_being_reopened?' do
+    it 'returns true if the closed attribute has changed and is now false' do
+      issue = Factory.create(:issue,
+                             :closed => true,
+                             :author => Factory(:user),
+                             :assignee => Factory(:user),
+                             :project => Factory.create(:project))
+      issue.closed = false
+      issue.is_being_reopened?.should be_true
+    end
+    it 'returns false if the closed attribute has changed and is now true' do
+      subject.closed = true
+      subject.is_being_reopened?.should be_false
+    end
+    it 'returns false if the closed attribute has not changed' do
+      subject.is_being_reopened?.should be_false
+    end
+  end
+
   describe "plus 1" do
     let(:project) { Factory(:project) }
     subject {
@@ -86,6 +106,7 @@ describe Issue do
       subject.upvotes.should == 2
     end
   end
+
 end
 # == Schema Information
 #