Download as
Browse Code »

Commit bb22360d1a7548facff3b72b2f9a0e66c6a55bb7

Authored by Robb Kidd
1 parent 5fe75649
Exists in master and in 4 other branches 6-8-stable, 7-0-stable, 7-0-stable-spb, spb-stable

Make Notify#note_merge_request_email resque friendly 

Update method to take ids and then perform #finds itself during mailer
queue worker kick-off.
Showing 4 changed files with 14 additions and 11 deletions   Show diff stats
app/mailers/notify.rb
  Diff comments   View file @bb22360
@@ -35,13 +35,12 @@ class Notify < ActionMailer::Base @@ -35,13 +35,12 @@ class Notify < ActionMailer::Base
35 @commit = @note.target 35 @commit = @note.target
36 mail(:to => @user['email'], :subject => "gitlab | note for commit | #{@note.project.name} ") 36 mail(:to => @user['email'], :subject => "gitlab | note for commit | #{@note.project.name} ")
37 end 37 end
38 -  
39 - def note_merge_request_email(user, note)  
40 - @user = user  
41 - @note = Note.find(note['id'])  
42 - @project = @note.project 38 +
  39 + def note_merge_request_email(recipient_id, note_id)
  40 + recipient = User.find(recipient_id)
  41 + @note = Note.find(note_id)
43 @merge_request = @note.noteable 42 @merge_request = @note.noteable
44 - mail(:to => @user['email'], :subject => "gitlab | note for merge request | #{@note.project.name} ") 43 + mail(:to => recipient.email, :subject => "gitlab | note for merge request | #{@note.project.name} ")
45 end 44 end
46 45
47 def note_issue_email(user, note) 46 def note_issue_email(user, note)
app/models/mailer_observer.rb
  Diff comments   View file @bb22360
@@ -36,7 +36,7 @@ class MailerObserver < ActiveRecord::Observer @@ -36,7 +36,7 @@ class MailerObserver < ActiveRecord::Observer
36 when "Issue" then 36 when "Issue" then
37 Notify.note_issue_email(u, note).deliver 37 Notify.note_issue_email(u, note).deliver
38 when "MergeRequest" then 38 when "MergeRequest" then
39 - Notify.note_merge_request_email(u, note).deliver 39 + Notify.note_merge_request_email(u.id, note.id).deliver
40 when "Snippet" 40 when "Snippet"
41 true 41 true
42 else 42 else
app/views/notify/note_merge_request_email.html.haml
  Diff comments   View file @bb22360
@@ -5,13 +5,13 @@ @@ -5,13 +5,13 @@
5 %td{:align => "left", :style => "padding: 20px 0 0;"} 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; "} 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 New comment for Merge Request 7 New comment for Merge Request
8 - = link_to truncate(@merge_request.title, :length => 16), project_merge_request_url(@project, @merge_request, :anchor => "note_#{@note.id}") 8 + = link_to truncate(@merge_request.title, :length => 16), project_merge_request_url(@merge_request.project, @merge_request, :anchor => "note_#{@note.id}")
9 %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"} 9 %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
10 %tr 10 %tr
11 %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"} 11 %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
12 %td{:style => "padding: 15px 0 15px;", :valign => "top"} 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; "} 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 - %a{:href => "#", :style => "color: #0eb6ce; text-decoration: none;"} #{@note.author.name} 14 + %a{:href => "#", :style => "color: #0eb6ce; text-decoration: none;"} #{@note.author_name}
15 left next message: 15 left next message:
16 %br 16 %br
17 %table{:border => "0", :cellpadding => "0", :cellspacing => "0", :width => "558"} 17 %table{:border => "0", :cellpadding => "0", :cellspacing => "0", :width => "558"}
spec/mailers/notify_spec.rb
  Diff comments   View file @bb22360
@@ -153,6 +153,10 @@ describe Notify do @@ -153,6 +153,10 @@ describe Notify do
153 let(:note_author) { Factory.create(:user, :name => 'author_name') } 153 let(:note_author) { Factory.create(:user, :name => 'author_name') }
154 let(:note) { Factory.create(:note, :project => project, :author => note_author) } 154 let(:note) { Factory.create(:note, :project => project, :author => note_author) }
155 155
  156 + before :each do
  157 + Note.stub(:find).with(note.id).and_return(note)
  158 + end
  159 +
156 shared_examples 'a note email' do 160 shared_examples 'a note email' do
157 it 'is sent to the given recipient' do 161 it 'is sent to the given recipient' do
158 should deliver_to recipient.email 162 should deliver_to recipient.email
@@ -191,7 +195,7 @@ describe Notify do @@ -191,7 +195,7 @@ describe Notify do
191 end 195 end
192 before(:each) { note.stub(:target).and_return(commit) } 196 before(:each) { note.stub(:target).and_return(commit) }
193 197
194 - subject { Notify.note_commit_email(recipient, note) } 198 + subject { Notify.note_commit_email(recipient.id, note.id) }
195 199
196 it_behaves_like 'a note email' 200 it_behaves_like 'a note email'
197 201
@@ -209,7 +213,7 @@ describe Notify do @@ -209,7 +213,7 @@ describe Notify do
209 let(:note_on_merge_request_url) { project_merge_request_url(project, merge_request, :anchor => "note_#{note.id}") } 213 let(:note_on_merge_request_url) { project_merge_request_url(project, merge_request, :anchor => "note_#{note.id}") }
210 before(:each) { note.stub(:noteable).and_return(merge_request) } 214 before(:each) { note.stub(:noteable).and_return(merge_request) }
211 215
212 - subject { Notify.note_merge_request_email(recipient, note) } 216 + subject { Notify.note_merge_request_email(recipient.id, note.id) }
213 217
214 it_behaves_like 'a note email' 218 it_behaves_like 'a note email'
215 219