Commit dd921053c8ef598bad3c7a16331c296d581f0080
1 parent
345f176a
Exists in
master
and in
4 other branches
Rename changed_issue_email to reassigned_issue_email & make resque friendly
#changed_issue_email was really sending emails about issue reassignments. Updated method name to reflect that. Update method to take ids and then perform #finds itself during mailer queue worker kick-off.
Showing
5 changed files
with
28 additions
and
29 deletions
Show diff stats
app/mailers/notify.rb
| @@ -66,12 +66,11 @@ class Notify < ActionMailer::Base | @@ -66,12 +66,11 @@ class Notify < ActionMailer::Base | ||
| 66 | @project = @merge_request.project | 66 | @project = @merge_request.project |
| 67 | mail(:to => @user['email'], :subject => "gitlab | merge request changed | #{@merge_request.title} ") | 67 | mail(:to => @user['email'], :subject => "gitlab | merge request changed | #{@merge_request.title} ") |
| 68 | end | 68 | end |
| 69 | - | ||
| 70 | - def changed_issue_email(user, issue) | ||
| 71 | - @issue = Issue.find(issue['id']) | ||
| 72 | - @user = user | ||
| 73 | - @assignee_was ||= User.find(@issue.assignee_id_was) | ||
| 74 | - @project = @issue.project | ||
| 75 | - mail(:to => @user['email'], :subject => "gitlab | changed issue | #{@issue.title} ") | 69 | + |
| 70 | + def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id) | ||
| 71 | + recipient = User.find(recipient_id) | ||
| 72 | + @issue = Issue.find(issue_id) | ||
| 73 | + @previous_assignee ||= User.find(previous_assignee_id) | ||
| 74 | + mail(:to => recipient.email, :subject => "gitlab | changed issue | #{@issue.title} ") | ||
| 76 | end | 75 | end |
| 77 | end | 76 | end |
app/models/mailer_observer.rb
| @@ -78,8 +78,8 @@ class MailerObserver < ActiveRecord::Observer | @@ -78,8 +78,8 @@ class MailerObserver < ActiveRecord::Observer | ||
| 78 | recipients_ids = issue.assignee_id_was, issue.assignee_id | 78 | recipients_ids = issue.assignee_id_was, issue.assignee_id |
| 79 | recipients_ids.delete current_user.id | 79 | recipients_ids.delete current_user.id |
| 80 | 80 | ||
| 81 | - User.find(recipients_ids).each do |user| | ||
| 82 | - Notify.changed_issue_email(user, issue).deliver | 81 | + recipients_ids.each do |recipient_id| |
| 82 | + Notify.reassigned_issue_email(recipient_id, issue.id, issue.assignee_id_was).deliver | ||
| 83 | end | 83 | end |
| 84 | end | 84 | end |
| 85 | 85 |
app/views/notify/changed_issue_email.html.haml
| @@ -1,16 +0,0 @@ | @@ -1,16 +0,0 @@ | ||
| 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 | - Reassigned Issue | ||
| 8 | - = link_to truncate(@issue.title, :length => 16), project_issue_url(@project, @issue) | ||
| 9 | - %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"} | ||
| 10 | - %tr | ||
| 11 | - %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"} | ||
| 12 | - %td{:style => "padding: 15px 0 15px;", :valign => "top"} | ||
| 13 | - %p{:style => "color:#767676; font-weight: normal; margin: 0; padding: 0; line-height: 20px; font-size: 12px;font-family: Helvetica, Arial, sans-serif; "} | ||
| 14 | - Assignee changed from #{@assignee_was.name} to #{@issue.assignee.name} | ||
| 15 | - %td | ||
| 16 | - |
| @@ -0,0 +1,16 @@ | @@ -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 | + Reassigned Issue | ||
| 8 | + = link_to truncate(@issue.title, :length => 16), project_issue_url(@issue.project, @issue) | ||
| 9 | + %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"} | ||
| 10 | + %tr | ||
| 11 | + %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"} | ||
| 12 | + %td{:style => "padding: 15px 0 15px;", :valign => "top"} | ||
| 13 | + %p{:style => "color:#767676; font-weight: normal; margin: 0; padding: 0; line-height: 20px; font-size: 12px;font-family: Helvetica, Arial, sans-serif; "} | ||
| 14 | + Assignee changed from #{@previous_assignee.name} to #{@issue.assignee_name} | ||
| 15 | + %td | ||
| 16 | + |
spec/mailers/notify_spec.rb
| @@ -47,7 +47,7 @@ describe Notify do | @@ -47,7 +47,7 @@ describe Notify do | ||
| 47 | context 'for a project' do | 47 | context 'for a project' do |
| 48 | describe 'items that are assignable, the email' do | 48 | describe 'items that are assignable, the email' do |
| 49 | let(:assignee) { Factory.create(:user, :email => 'assignee@example.com') } | 49 | let(:assignee) { Factory.create(:user, :email => 'assignee@example.com') } |
| 50 | - let(:old_assignee) { Factory.create(:user, :name => 'Old Assignee Guy') } | 50 | + let(:previous_assignee) { Factory.create(:user, :name => 'Previous Assignee') } |
| 51 | 51 | ||
| 52 | shared_examples 'an assignee email' do | 52 | shared_examples 'an assignee email' do |
| 53 | it 'is sent to the assignee' do | 53 | it 'is sent to the assignee' do |
| @@ -73,9 +73,9 @@ describe Notify do | @@ -73,9 +73,9 @@ describe Notify do | ||
| 73 | end | 73 | end |
| 74 | 74 | ||
| 75 | describe 'that have been reassigned' do | 75 | describe 'that have been reassigned' do |
| 76 | - before(:each) { issue.stub(:assignee_id_was).and_return(old_assignee.id) } | 76 | + before(:each) { issue.stub(:assignee_id_was).and_return(previous_assignee.id) } |
| 77 | 77 | ||
| 78 | - subject { Notify.changed_issue_email(recipient, issue) } | 78 | + subject { Notify.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id) } |
| 79 | 79 | ||
| 80 | it_behaves_like 'a multiple recipients email' | 80 | it_behaves_like 'a multiple recipients email' |
| 81 | 81 | ||
| @@ -84,7 +84,7 @@ describe Notify do | @@ -84,7 +84,7 @@ describe Notify do | ||
| 84 | end | 84 | end |
| 85 | 85 | ||
| 86 | it 'contains the name of the previous assignee' do | 86 | it 'contains the name of the previous assignee' do |
| 87 | - should have_body_text /#{old_assignee.name}/ | 87 | + should have_body_text /#{previous_assignee.name}/ |
| 88 | end | 88 | end |
| 89 | 89 | ||
| 90 | it 'contains the name of the new assignee' do | 90 | it 'contains the name of the new assignee' do |