From dd921053c8ef598bad3c7a16331c296d581f0080 Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Mon, 14 May 2012 23:27:52 -0400 Subject: [PATCH] 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. --- app/mailers/notify.rb | 13 ++++++------- app/models/mailer_observer.rb | 4 ++-- ...l.html.haml => reassigned_issue_email.html.haml} | 4 ++-- spec/mailers/notify_spec.rb | 8 ++++---- 4 files changed, 14 insertions(+), 15 deletions(-) rename app/views/notify/{changed_issue_email.html.haml => reassigned_issue_email.html.haml} (90%) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index d702a78e..875e83e5 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -66,12 +66,11 @@ class Notify < ActionMailer::Base @project = @merge_request.project mail(:to => @user['email'], :subject => "gitlab | merge request changed | #{@merge_request.title} ") end - - def changed_issue_email(user, issue) - @issue = Issue.find(issue['id']) - @user = user - @assignee_was ||= User.find(@issue.assignee_id_was) - @project = @issue.project - mail(:to => @user['email'], :subject => "gitlab | changed issue | #{@issue.title} ") + + def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id) + recipient = User.find(recipient_id) + @issue = Issue.find(issue_id) + @previous_assignee ||= User.find(previous_assignee_id) + mail(:to => recipient.email, :subject => "gitlab | changed issue | #{@issue.title} ") end end diff --git a/app/models/mailer_observer.rb b/app/models/mailer_observer.rb index 940ad1da..573e98ef 100644 --- a/app/models/mailer_observer.rb +++ b/app/models/mailer_observer.rb @@ -78,8 +78,8 @@ class MailerObserver < ActiveRecord::Observer recipients_ids = issue.assignee_id_was, issue.assignee_id recipients_ids.delete current_user.id - User.find(recipients_ids).each do |user| - Notify.changed_issue_email(user, issue).deliver + recipients_ids.each do |recipient_id| + Notify.reassigned_issue_email(recipient_id, issue.id, issue.assignee_id_was).deliver end end diff --git a/app/views/notify/changed_issue_email.html.haml b/app/views/notify/reassigned_issue_email.html.haml similarity index 90% rename from app/views/notify/changed_issue_email.html.haml rename to app/views/notify/reassigned_issue_email.html.haml index fe046e40..43579b27 100644 --- a/app/views/notify/changed_issue_email.html.haml +++ b/app/views/notify/reassigned_issue_email.html.haml @@ -5,12 +5,12 @@ %td{:align => "left", :style => "padding: 20px 0 0;"} %h2{:style => "color:#646464; font-weight: bold; margin: 0; padding: 0; line-height: 26px; font-size: 18px; font-family: Helvetica, Arial, sans-serif; "} Reassigned Issue - = link_to truncate(@issue.title, :length => 16), project_issue_url(@project, @issue) + = link_to truncate(@issue.title, :length => 16), project_issue_url(@issue.project, @issue) %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"} %tr %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"} %td{:style => "padding: 15px 0 15px;", :valign => "top"} %p{:style => "color:#767676; font-weight: normal; margin: 0; padding: 0; line-height: 20px; font-size: 12px;font-family: Helvetica, Arial, sans-serif; "} - Assignee changed from #{@assignee_was.name} to #{@issue.assignee.name} + Assignee changed from #{@previous_assignee.name} to #{@issue.assignee_name} %td diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 102db485..8c896085 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -47,7 +47,7 @@ describe Notify do context 'for a project' do describe 'items that are assignable, the email' do let(:assignee) { Factory.create(:user, :email => 'assignee@example.com') } - let(:old_assignee) { Factory.create(:user, :name => 'Old Assignee Guy') } + let(:previous_assignee) { Factory.create(:user, :name => 'Previous Assignee') } shared_examples 'an assignee email' do it 'is sent to the assignee' do @@ -73,9 +73,9 @@ describe Notify do end describe 'that have been reassigned' do - before(:each) { issue.stub(:assignee_id_was).and_return(old_assignee.id) } + before(:each) { issue.stub(:assignee_id_was).and_return(previous_assignee.id) } - subject { Notify.changed_issue_email(recipient, issue) } + subject { Notify.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id) } it_behaves_like 'a multiple recipients email' @@ -84,7 +84,7 @@ describe Notify do end it 'contains the name of the previous assignee' do - should have_body_text /#{old_assignee.name}/ + should have_body_text /#{previous_assignee.name}/ end it 'contains the name of the new assignee' do