Commit ed954ebae24b5f7308e6f5e7250320342f3d9a41
Exists in
master
and in
4 other branches
Merge pull request #1326 from AlexDenisov/issue_status_changed_notifications
Issue status changed notifications
Showing
5 changed files
with
119 additions
and
9 deletions
Show diff stats
app/mailers/notify.rb
| ... | ... | @@ -83,6 +83,14 @@ class Notify < ActionMailer::Base |
| 83 | 83 | subject: subject("access to project was granted")) |
| 84 | 84 | end |
| 85 | 85 | |
| 86 | + def issue_status_changed_email(recipient_id, issue_id, status, updated_by_user_id) | |
| 87 | + @issue = Issue.find issue_id | |
| 88 | + @issue_status = status | |
| 89 | + @updated_by = User.find updated_by_user_id | |
| 90 | + mail(to: recipient(recipient_id), | |
| 91 | + subject: subject("changed issue ##{@issue.id}", @issue.title)) | |
| 92 | + end | |
| 93 | + | |
| 86 | 94 | private |
| 87 | 95 | |
| 88 | 96 | # Look up a User by their ID and return their email address | ... | ... |
app/observers/issue_observer.rb
| ... | ... | @@ -9,8 +9,16 @@ class IssueObserver < ActiveRecord::Observer |
| 9 | 9 | |
| 10 | 10 | def after_update(issue) |
| 11 | 11 | send_reassigned_email(issue) if issue.is_being_reassigned? |
| 12 | - Note.create_status_change_note(issue, current_user, 'closed') if issue.is_being_closed? | |
| 13 | - Note.create_status_change_note(issue, current_user, 'reopened') if issue.is_being_reopened? | |
| 12 | + | |
| 13 | + status = nil | |
| 14 | + status = 'closed' if issue.is_being_closed? | |
| 15 | + status = 'reopened' if issue.is_being_reopened? | |
| 16 | + if status | |
| 17 | + Note.create_status_change_note(issue, current_user, status) | |
| 18 | + [issue.author, issue.assignee].compact.each do |recipient| | |
| 19 | + Notify.issue_status_changed_email(recipient.id, issue.id, status, current_user) | |
| 20 | + end | |
| 21 | + end | |
| 14 | 22 | end |
| 15 | 23 | |
| 16 | 24 | protected | ... | ... |
| ... | ... | @@ -0,0 +1,16 @@ |
| 1 | +%td.content{align: "left", style: "font-family: Helvetica, Arial, sans-serif; padding: 20px 0 0;", valign: "top", width: "600"} | |
| 2 | + %table{border: "0", cellpadding: "0", cellspacing: "0", style: "color: #717171; font: normal 11px Helvetica, Arial, sans-serif; margin: 0; padding: 0;", width: "600"} | |
| 3 | + %tr | |
| 4 | + %td{style: "font-size: 1px; line-height: 1px;", width: "21"} | |
| 5 | + %td{align: "left", style: "padding: 20px 0 0;"} | |
| 6 | + %h2{style: "color:#646464; font-weight: bold; margin: 0; padding: 0; line-height: 26px; font-size: 18px; font-family: Helvetica, Arial, sans-serif; "} | |
| 7 | + = "Issue was #{@issue_status} by #{@updated_by.name}" | |
| 8 | + %td{style: "font-size: 1px; line-height: 1px;", width: "21"} | |
| 9 | + %tr | |
| 10 | + %td{style: "font-size: 1px; line-height: 1px;", width: "21"} | |
| 11 | + %td{align: "left", style: "padding: 20px 0 0;"} | |
| 12 | + %h2{style: "color:#646464 !important; font-weight: bold; margin: 0; padding: 0; line-height: 26px; font-size: 18px; font-family: Helvetica, Arial, sans-serif; "} | |
| 13 | + = "Issue ##{@issue.id}" | |
| 14 | + = link_to_gfm truncate(@issue.title, length: 45), project_issue_url(@issue.project, @issue), title: @issue.title | |
| 15 | + %br | |
| 16 | + | ... | ... |
spec/mailers/notify_spec.rb
| ... | ... | @@ -91,6 +91,29 @@ describe Notify do |
| 91 | 91 | should have_body_text /#{project_issue_path project, issue}/ |
| 92 | 92 | end |
| 93 | 93 | end |
| 94 | + | |
| 95 | + describe 'status changed' do | |
| 96 | + let(:current_user) { Factory.create :user, email: "current@email.com" } | |
| 97 | + let(:status) { 'closed' } | |
| 98 | + subject { Notify.issue_status_changed_email(recipient.id, issue.id, status, current_user) } | |
| 99 | + | |
| 100 | + it 'has the correct subject' do | |
| 101 | + should have_subject /changed issue ##{issue.id} \| #{issue.title}/i | |
| 102 | + end | |
| 103 | + | |
| 104 | + it 'contains the new status' do | |
| 105 | + should have_body_text /#{status}/i | |
| 106 | + end | |
| 107 | + | |
| 108 | + it 'contains the user name' do | |
| 109 | + should have_body_text /#{current_user.name}/i | |
| 110 | + end | |
| 111 | + | |
| 112 | + it 'contains a link to the issue' do | |
| 113 | + should have_body_text /#{project_issue_path project, issue}/ | |
| 114 | + end | |
| 115 | + end | |
| 116 | + | |
| 94 | 117 | end |
| 95 | 118 | |
| 96 | 119 | context 'for merge requests' do | ... | ... |
spec/observers/issue_observer_spec.rb
| ... | ... | @@ -3,7 +3,8 @@ require 'spec_helper' |
| 3 | 3 | describe IssueObserver do |
| 4 | 4 | let(:some_user) { double(:user, id: 1) } |
| 5 | 5 | let(:assignee) { double(:user, id: 2) } |
| 6 | - let(:issue) { double(:issue, id: 42, assignee: assignee) } | |
| 6 | + let(:author) { double(:user, id: 3) } | |
| 7 | + let(:issue) { double(:issue, id: 42, assignee: assignee, author: author) } | |
| 7 | 8 | |
| 8 | 9 | before(:each) { subject.stub(:current_user).and_return(some_user) } |
| 9 | 10 | |
| ... | ... | @@ -67,36 +68,90 @@ describe IssueObserver do |
| 67 | 68 | end |
| 68 | 69 | end |
| 69 | 70 | |
| 70 | - context 'a status "closed" note' do | |
| 71 | - it 'is created if the issue is being closed' do | |
| 71 | + context 'a status "closed"' do | |
| 72 | + it 'note is created if the issue is being closed' do | |
| 72 | 73 | issue.should_receive(:is_being_closed?).and_return(true) |
| 73 | 74 | Note.should_receive(:create_status_change_note).with(issue, some_user, 'closed') |
| 74 | 75 | |
| 75 | 76 | subject.after_update(issue) |
| 76 | 77 | end |
| 77 | 78 | |
| 78 | - it 'is not created if the issue is not being closed' do | |
| 79 | + it 'note is not created if the issue is not being closed' do | |
| 79 | 80 | issue.should_receive(:is_being_closed?).and_return(false) |
| 80 | 81 | Note.should_not_receive(:create_status_change_note).with(issue, some_user, 'closed') |
| 81 | 82 | |
| 82 | 83 | subject.after_update(issue) |
| 83 | 84 | end |
| 85 | + | |
| 86 | + it 'notification is delivered if the issue being closed' do | |
| 87 | + issue.stub(:is_being_closed?).and_return(true) | |
| 88 | + Notify.should_receive(:issue_status_changed_email).twice | |
| 89 | + Note.should_receive(:create_status_change_note).with(issue, some_user, 'closed') | |
| 90 | + | |
| 91 | + subject.after_update(issue) | |
| 92 | + end | |
| 93 | + | |
| 94 | + it 'notification is not delivered if the issue not being closed' do | |
| 95 | + issue.stub(:is_being_closed?).and_return(false) | |
| 96 | + Notify.should_not_receive(:issue_status_changed_email) | |
| 97 | + Note.should_not_receive(:create_status_change_note).with(issue, some_user, 'closed') | |
| 98 | + | |
| 99 | + subject.after_update(issue) | |
| 100 | + end | |
| 101 | + | |
| 102 | + it 'notification is delivered only to author if the issue being closed' do | |
| 103 | + issue_without_assignee = double(:issue, id: 42, author: author, assignee: nil) | |
| 104 | + issue_without_assignee.stub(:is_being_reassigned?).and_return(false) | |
| 105 | + issue_without_assignee.stub(:is_being_closed?).and_return(true) | |
| 106 | + issue_without_assignee.stub(:is_being_reopened?).and_return(false) | |
| 107 | + Notify.should_receive(:issue_status_changed_email).once | |
| 108 | + Note.should_receive(:create_status_change_note).with(issue_without_assignee, some_user, 'closed') | |
| 109 | + | |
| 110 | + subject.after_update(issue_without_assignee) | |
| 111 | + end | |
| 84 | 112 | end |
| 85 | 113 | |
| 86 | - context 'a status "reopened" note' do | |
| 87 | - it 'is created if the issue is being reopened' do | |
| 114 | + context 'a status "reopened"' do | |
| 115 | + it 'note is created if the issue is being reopened' do | |
| 88 | 116 | issue.should_receive(:is_being_reopened?).and_return(true) |
| 89 | 117 | Note.should_receive(:create_status_change_note).with(issue, some_user, 'reopened') |
| 90 | 118 | |
| 91 | 119 | subject.after_update(issue) |
| 92 | 120 | end |
| 93 | 121 | |
| 94 | - it 'is not created if the issue is not being reopened' do | |
| 122 | + it 'note is not created if the issue is not being reopened' do | |
| 95 | 123 | issue.should_receive(:is_being_reopened?).and_return(false) |
| 96 | 124 | Note.should_not_receive(:create_status_change_note).with(issue, some_user, 'reopened') |
| 97 | 125 | |
| 98 | 126 | subject.after_update(issue) |
| 99 | 127 | end |
| 128 | + | |
| 129 | + it 'notification is delivered if the issue being reopened' do | |
| 130 | + issue.stub(:is_being_reopened?).and_return(true) | |
| 131 | + Notify.should_receive(:issue_status_changed_email).twice | |
| 132 | + Note.should_receive(:create_status_change_note).with(issue, some_user, 'reopened') | |
| 133 | + | |
| 134 | + subject.after_update(issue) | |
| 135 | + end | |
| 136 | + | |
| 137 | + it 'notification is not delivered if the issue not being reopened' do | |
| 138 | + issue.stub(:is_being_reopened?).and_return(false) | |
| 139 | + Notify.should_not_receive(:issue_status_changed_email) | |
| 140 | + Note.should_not_receive(:create_status_change_note).with(issue, some_user, 'reopened') | |
| 141 | + | |
| 142 | + subject.after_update(issue) | |
| 143 | + end | |
| 144 | + | |
| 145 | + it 'notification is delivered only to author if the issue being reopened' do | |
| 146 | + issue_without_assignee = double(:issue, id: 42, author: author, assignee: nil) | |
| 147 | + issue_without_assignee.stub(:is_being_reassigned?).and_return(false) | |
| 148 | + issue_without_assignee.stub(:is_being_closed?).and_return(false) | |
| 149 | + issue_without_assignee.stub(:is_being_reopened?).and_return(true) | |
| 150 | + Notify.should_receive(:issue_status_changed_email).once | |
| 151 | + Note.should_receive(:create_status_change_note).with(issue_without_assignee, some_user, 'reopened') | |
| 152 | + | |
| 153 | + subject.after_update(issue_without_assignee) | |
| 154 | + end | |
| 100 | 155 | end |
| 101 | 156 | end |
| 102 | 157 | ... | ... |