Commit 00ec81eacb881fbe0223183737d9c95b801ab01c
1 parent
2416e3cb
Exists in
master
and in
4 other branches
Update IssueObserver to send reassigned emails when an issue is reassigned.
Showing
2 changed files
with
44 additions
and
34 deletions
Show diff stats
app/models/issue_observer.rb
... | ... | @@ -6,12 +6,15 @@ class IssueObserver < ActiveRecord::Observer |
6 | 6 | end |
7 | 7 | |
8 | 8 | def after_change(issue) |
9 | - if issue.assignee_id_changed? | |
10 | - recipient_ids = [issue.assignee_id, issue.assignee_id_was].keep_if {|id| id != current_user.id } | |
9 | + send_reassigned_email(issue) if issue.is_being_reassigned? | |
10 | + end | |
11 | + | |
12 | + def send_reassigned_email(issue) | |
13 | + recipient_ids = [issue.assignee_id, issue.assignee_id_was].keep_if {|id| id != current_user.id } | |
11 | 14 | |
12 | - recipient_ids.each do |recipient_id| | |
13 | - Notify.reassigned_issue_email(recipient_id, issue.id, issue.assignee_id_was) | |
14 | - end | |
15 | + recipient_ids.each do |recipient_id| | |
16 | + Notify.reassigned_issue_email(recipient_id, issue.id, issue.assignee_id_was) | |
15 | 17 | end |
16 | 18 | end |
19 | + | |
17 | 20 | end | ... | ... |
spec/models/issue_observer_spec.rb
1 | 1 | require 'spec_helper' |
2 | 2 | |
3 | 3 | describe IssueObserver do |
4 | - let(:some_user) { Factory.new(:user, :id => 1) } | |
5 | - let(:assignee) { Factory.new(:user, :id => 2) } | |
6 | - let(:issue) { Factory.new(:issue, :id => 42, :assignee => assignee) } | |
4 | + let(:some_user) { double(:user, :id => 1) } | |
5 | + let(:assignee) { double(:user, :id => 2) } | |
6 | + let(:issue) { double(:issue, :id => 42, :assignee => assignee) } | |
7 | 7 | |
8 | 8 | before(:each) { subject.stub(:current_user).and_return(some_user) } |
9 | 9 | |
... | ... | @@ -25,42 +25,49 @@ describe IssueObserver do |
25 | 25 | end |
26 | 26 | end |
27 | 27 | |
28 | - context 'when an issue is modified' do | |
29 | - it 'but not reassigned, does not send a reassigned email' do | |
30 | - issue.stub(:assignee_id_changed?).and_return(false) | |
31 | - Notify.should_not_receive(:reassigned_issue_email) | |
28 | + context 'when an issue is changed' do | |
29 | + it 'sends a reassigned email, if the issue is being reassigned' do | |
30 | + issue.should_receive(:is_being_reassigned?).and_return(true) | |
31 | + subject.should_receive(:send_reassigned_email).with(issue) | |
32 | 32 | |
33 | 33 | subject.after_change(issue) |
34 | 34 | end |
35 | 35 | |
36 | - context 'and is reassigned' do | |
37 | - let(:previous_assignee) { Factory.new(:user, :id => 3) } | |
36 | + it 'does not send a reassigned email, if the issue was not reassigned' do | |
37 | + issue.should_receive(:is_being_reassigned?).and_return(false) | |
38 | + subject.should_not_receive(:send_reassigned_email) | |
38 | 39 | |
39 | - before(:each) do | |
40 | - issue.stub(:assignee_id_changed?).and_return(true) | |
41 | - issue.stub(:assignee_id_was).and_return(previous_assignee.id) | |
42 | - end | |
40 | + subject.after_change(issue) | |
41 | + end | |
42 | + end | |
43 | 43 | |
44 | - it 'sends a reassigned email to the previous and current assignees' do | |
45 | - Notify.should_receive(:reassigned_issue_email).with(assignee.id, issue.id, previous_assignee.id) | |
46 | - Notify.should_receive(:reassigned_issue_email).with(previous_assignee.id, issue.id, previous_assignee.id) | |
44 | + describe '#send_reassigned_email' do | |
45 | + let(:previous_assignee) { double(:user, :id => 3) } | |
47 | 46 | |
48 | - subject.after_change(issue) | |
49 | - end | |
47 | + before(:each) do | |
48 | + issue.stub(:assignee_id).and_return(assignee.id) | |
49 | + issue.stub(:assignee_id_was).and_return(previous_assignee.id) | |
50 | + end | |
51 | + | |
52 | + it 'sends a reassigned email to the previous and current assignees' do | |
53 | + Notify.should_receive(:reassigned_issue_email).with(assignee.id, issue.id, previous_assignee.id) | |
54 | + Notify.should_receive(:reassigned_issue_email).with(previous_assignee.id, issue.id, previous_assignee.id) | |
50 | 55 | |
51 | - context 'does not send an email to the user who made the reassignment' do | |
52 | - it 'if the user is the assignee' do | |
53 | - subject.stub(:current_user).and_return(assignee) | |
54 | - Notify.should_not_receive(:reassigned_issue_email).with(assignee.id, issue.id, previous_assignee.id) | |
56 | + subject.send_reassigned_email(issue) | |
57 | + end | |
55 | 58 | |
56 | - subject.after_change(issue) | |
57 | - end | |
58 | - it 'if the user is the previous assignee' do | |
59 | - subject.stub(:current_user).and_return(previous_assignee) | |
60 | - Notify.should_not_receive(:reassigned_issue_email).with(previous_assignee.id, issue.id, previous_assignee.id) | |
59 | + context 'does not send an email to the user who made the reassignment' do | |
60 | + it 'if the user is the assignee' do | |
61 | + subject.stub(:current_user).and_return(assignee) | |
62 | + Notify.should_not_receive(:reassigned_issue_email).with(assignee.id, issue.id, previous_assignee.id) | |
63 | + | |
64 | + subject.send_reassigned_email(issue) | |
65 | + end | |
66 | + it 'if the user is the previous assignee' do | |
67 | + subject.stub(:current_user).and_return(previous_assignee) | |
68 | + Notify.should_not_receive(:reassigned_issue_email).with(previous_assignee.id, issue.id, previous_assignee.id) | |
61 | 69 | |
62 | - subject.after_change(issue) | |
63 | - end | |
70 | + subject.send_reassigned_email(issue) | |
64 | 71 | end |
65 | 72 | end |
66 | 73 | end | ... | ... |