From c7489578e61bf81af085cbd541dbcf68742061b2 Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Mon, 14 May 2012 14:05:32 -0400 Subject: [PATCH 01/13] Add specs for Notify ActionMailer emails. Covers new user, new issue and wall note emails. Depends on email_spec (https://github.com/bmabey/email-spec/) for friendly matchers. --- Gemfile | 1 + Gemfile.lock | 4 ++ spec/mailers/notify_spec.rb | 82 +++++++++++++++++++++++++++++++++++++ spec/spec_helper.rb | 1 + 4 files changed, 88 insertions(+) create mode 100644 spec/mailers/notify_spec.rb diff --git a/Gemfile b/Gemfile index 1cb5520ad3b..af5b0241397 100644 --- a/Gemfile +++ b/Gemfile @@ -66,4 +66,5 @@ group :test do gem "turn", :require => false gem "simplecov", :require => false gem "shoulda", "3.0.1" + gem 'email_spec' end diff --git a/Gemfile.lock b/Gemfile.lock index 182cf669197..6a18f7d17d6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -114,6 +114,9 @@ GEM warden (~> 1.1) diff-lcs (1.1.3) drapper (0.8.4) + email_spec (1.2.1) + mail (~> 2.2) + rspec (~> 2.0) erubis (2.7.0) escape_utils (0.2.4) eventmachine (0.12.10) @@ -330,6 +333,7 @@ DEPENDENCIES database_cleaner devise (~> 1.5) drapper + email_spec faker foreman git diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb new file mode 100644 index 00000000000..97ac15dc4bc --- /dev/null +++ b/spec/mailers/notify_spec.rb @@ -0,0 +1,82 @@ +require 'spec_helper' + +describe Notify do + include EmailSpec::Helpers + include EmailSpec::Matchers + + before :all do + default_url_options[:host] = 'example.com' + end + + let(:example_email) { 'user@example.com' } + + describe 'new user email' do + let(:example_password) { 'thisismypassword' } + let(:example_site_url) { root_url } + let(:new_user) { Factory.new(:user, :email => example_email, :password => example_password) } + + subject { Notify.new_user_email(new_user, new_user.password) } + + it 'is sent to the new user' do + should deliver_to new_user.email + end + + it 'has the correct subject' do + should have_subject /Account was created for you/ + end + + it 'contains the new user\'s login name' do + should have_body_text /#{new_user.email}/ + end + + it 'contains the new user\'s password' do + should have_body_text /#{new_user.password}/ + end + + it 'includes a link to the site' do + should have_body_text /#{example_site_url}/ + end + end + + describe 'new issue email' do + let(:project) { Factory.create(:project) } + let(:assignee) { Factory.create(:user, :email => example_email) } + let(:issue) { Factory.create(:issue, :assignee => assignee, :project => project ) } + + subject { Notify.new_issue_email(issue) } + + it 'is sent to the assignee' do + should deliver_to assignee.email + end + + it 'has the correct subject' do + should have_subject /New Issue was created/ + end + + it 'contains a link to the new issue' do + should have_body_text /#{project_issue_url project, issue}/ + end + end + + describe 'note wall email' do + let(:project) { Factory.create(:project) } + let(:recipient) { Factory.create(:user, :email => example_email) } + let(:author) { Factory.create(:user) } + let(:note) { Factory.create(:note, :project => project, :author => author) } + let(:note_url) { wall_project_url(project, :anchor => "note_#{note.id}") } + + subject { Notify.note_wall_email(recipient, note) } + + it 'is sent to the given recipient' do + should deliver_to recipient.email + end + + it 'has the correct subject' do + should have_subject /#{project.name}/ + end + + it 'contains a link to the wall note' do + should have_body_text /#{note_url}/ + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f24496ec92c..18b7854d102 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -11,6 +11,7 @@ require 'capybara/dsl' require 'webmock/rspec' require 'factories' require 'monkeypatch' +require 'email_spec' # Requires supporting ruby files with custom matchers and macros, etc, # in spec/support/ and its subdirectories. From 06b45acb8f2491234811a06f01b94af634d58b61 Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Mon, 14 May 2012 18:03:30 -0400 Subject: [PATCH 02/13] Add specs for all of the emails. --- spec/mailers/notify_spec.rb | 227 ++++++++++++++++++++++++++++++------ 1 file changed, 194 insertions(+), 33 deletions(-) diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 97ac15dc4bc..b8d880c029a 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -8,12 +8,18 @@ describe Notify do default_url_options[:host] = 'example.com' end - let(:example_email) { 'user@example.com' } + let(:recipient) { Factory.create(:user, :email => 'recipient@example.com') } + let(:project) { Factory.create(:project) } - describe 'new user email' do - let(:example_password) { 'thisismypassword' } + shared_examples 'a multiple recipients email' do + it 'is sent to the given recipient' do + should deliver_to recipient.email + end + end + + describe 'for new users, the email' do let(:example_site_url) { root_url } - let(:new_user) { Factory.new(:user, :email => example_email, :password => example_password) } + let(:new_user) { Factory.new(:user, :email => 'newguy@example.com', :password => 'new_password') } subject { Notify.new_user_email(new_user, new_user.password) } @@ -38,45 +44,200 @@ describe Notify do end end - describe 'new issue email' do - let(:project) { Factory.create(:project) } - let(:assignee) { Factory.create(:user, :email => example_email) } - let(:issue) { Factory.create(:issue, :assignee => assignee, :project => project ) } + 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') } - subject { Notify.new_issue_email(issue) } + shared_examples 'an assignee email' do + it 'is sent to the assignee' do + should deliver_to assignee.email + end + end - it 'is sent to the assignee' do - should deliver_to assignee.email + context 'for issues' do + let(:issue) { Factory.create(:issue, :assignee => assignee, :project => project ) } + + describe 'that are new' do + subject { Notify.new_issue_email(issue) } + + it_behaves_like 'an assignee email' + + it 'has the correct subject' do + should have_subject /New Issue was created/ + end + + it 'contains a link to the new issue' do + should have_body_text /#{project_issue_url project, issue}/ + end + end + + describe 'that have been reassigned' do + before(:each) { issue.stub(:assignee_id_was).and_return(old_assignee.id) } + + subject { Notify.changed_issue_email(recipient, issue) } + + it_behaves_like 'a multiple recipients email' + + it 'has the correct subject' do + should have_subject /changed issue/ + end + + it 'contains the name of the previous assignee' do + should have_body_text /#{old_assignee.name}/ + end + + it 'contains the name of the new assignee' do + should have_body_text /#{assignee.name}/ + end + + it 'contains a link to the issue' do + should have_body_text /#{project_issue_url project, issue}/ + end + end + end + + context 'for merge requests' do + let(:merge_request) { Factory.create(:merge_request, :assignee => assignee, :project => project) } + + describe 'that are new' do + subject { Notify.new_merge_request_email(merge_request) } + + it_behaves_like 'an assignee email' + + it 'has the correct subject' do + should have_subject /new merge request/ + end + + it 'contains a link to the new merge request' do + should have_body_text /#{project_merge_request_url(project, merge_request)}/ + end + + it 'contains the source branch for the merge request' do + should have_body_text /#{merge_request.source_branch}/ + end + + it 'contains the target branch for the merge request' do + should have_body_text /#{merge_request.target_branch}/ + end + end + + describe 'that are reassigned' do + before(:each) { merge_request.stub(:assignee_id_was).and_return(old_assignee.id) } + + subject { Notify.changed_merge_request_email(recipient, merge_request) } + + it_behaves_like 'a multiple recipients email' + + it 'has the correct subject' do + should have_subject /merge request changed/ + end + + it 'contains the name of the previous assignee' do + should have_body_text /#{old_assignee.name}/ + end + + it 'contains the name of the new assignee' do + should have_body_text /#{assignee.name}/ + end + + it 'contains a link to the merge request' do + should have_body_text /#{project_merge_request_url project, merge_request}/ + end + + end + end end - it 'has the correct subject' do - should have_subject /New Issue was created/ - end + context 'items that are noteable, the email for a note' do + let(:note_author) { Factory.create(:user, :name => 'author_name') } + let(:note) { Factory.create(:note, :project => project, :author => note_author) } - it 'contains a link to the new issue' do - should have_body_text /#{project_issue_url project, issue}/ - end - end + shared_examples 'a note email' do + it 'is sent to the given recipient' do + should deliver_to recipient.email + end - describe 'note wall email' do - let(:project) { Factory.create(:project) } - let(:recipient) { Factory.create(:user, :email => example_email) } - let(:author) { Factory.create(:user) } - let(:note) { Factory.create(:note, :project => project, :author => author) } - let(:note_url) { wall_project_url(project, :anchor => "note_#{note.id}") } + it 'contains the name of the note\'s author' do + should have_body_text /#{note_author.name}/ + end - subject { Notify.note_wall_email(recipient, note) } + it 'contains the message from the note' do + should have_body_text /#{note.note}/ + end + end - it 'is sent to the given recipient' do - should deliver_to recipient.email - end + describe 'on a project wall' do + let(:note_on_the_wall_url) { wall_project_url(project, :anchor => "note_#{note.id}") } - it 'has the correct subject' do - should have_subject /#{project.name}/ - end + subject { Notify.note_wall_email(recipient, note) } - it 'contains a link to the wall note' do - should have_body_text /#{note_url}/ + it_behaves_like 'a note email' + + it 'has the correct subject' do + should have_subject /#{project.name}/ + end + + it 'contains a link to the wall note' do + should have_body_text /#{note_on_the_wall_url}/ + end + end + + describe 'on a commit' do + let(:commit) do + mock(:commit).tap do |commit| + commit.stub(:id).and_return('faux_sha_1') + end + end + before(:each) { note.stub(:target).and_return(commit) } + + subject { Notify.note_commit_email(recipient, note) } + + it_behaves_like 'a note email' + + it 'has the correct subject' do + should have_subject /note for commit/ + end + + it 'contains a link to the commit' do + should have_body_text /faux_sha_1/ + end + end + + describe 'on a merge request' do + let(:merge_request) { Factory.create(:merge_request, :project => project) } + let(:note_on_merge_request_url) { project_merge_request_url(project, merge_request, :anchor => "note_#{note.id}") } + before(:each) { note.stub(:noteable).and_return(merge_request) } + + subject { Notify.note_merge_request_email(recipient, note) } + + it_behaves_like 'a note email' + + it 'has the correct subject' do + should have_subject /note for merge request/ + end + + it 'contains a link to the merge request note' do + should have_body_text /#{note_on_merge_request_url}/ + end + end + + describe 'on an issue' do + let(:issue) { Factory.create(:issue, :project => project) } + let(:note_on_issue_url) { project_issue_url(project, issue, :anchor => "note_#{note.id}") } + before(:each) { note.stub(:noteable).and_return(issue) } + subject { Notify.note_issue_email(recipient, note) } + + it_behaves_like 'a note email' + + it 'has the correct subject' do + should have_subject /note for issue #{issue.id}/ + end + + it 'contains a link to the issue note' do + should have_body_text /#{note_on_issue_url}/ + end + end end end end From 345f176a7458ec1f99a2911988f0c4c0cb4d2704 Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Mon, 14 May 2012 23:07:36 -0400 Subject: [PATCH 03/13] Update new_user_email to take id for User and perform find itself. --- app/mailers/notify.rb | 6 +++--- app/models/mailer_observer.rb | 2 +- spec/mailers/notify_spec.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 967be900e07..d702a78ea47 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -7,10 +7,10 @@ class Notify < ActionMailer::Base default from: EMAIL_OPTS["from"] - def new_user_email(user, password) - @user = user + def new_user_email(user_id, password) + @user = User.find(user_id) @password = password - mail(:to => @user['email'], :subject => "gitlab | Account was created for you") + mail(:to => @user.email, :subject => "gitlab | Account was created for you") end def new_issue_email(issue) diff --git a/app/models/mailer_observer.rb b/app/models/mailer_observer.rb index f84cbdead59..940ad1da0b6 100644 --- a/app/models/mailer_observer.rb +++ b/app/models/mailer_observer.rb @@ -23,7 +23,7 @@ class MailerObserver < ActiveRecord::Observer end def new_user(user) - Notify.new_user_email(user, user.password).deliver + Notify.new_user_email(user.id, user.password).deliver end def new_note(note) diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index b8d880c029a..102db485a70 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -19,9 +19,9 @@ describe Notify do describe 'for new users, the email' do let(:example_site_url) { root_url } - let(:new_user) { Factory.new(:user, :email => 'newguy@example.com', :password => 'new_password') } + let(:new_user) { Factory.create(:user, :email => 'newguy@example.com') } - subject { Notify.new_user_email(new_user, new_user.password) } + subject { Notify.new_user_email(new_user.id, new_user.password) } it 'is sent to the new user' do should deliver_to new_user.email From dd921053c8ef598bad3c7a16331c296d581f0080 Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Mon, 14 May 2012 23:27:52 -0400 Subject: [PATCH 04/13] 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 d702a78ea47..875e83e5331 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 940ad1da0b6..573e98ef8dd 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 fe046e408a1..43579b274d3 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 102db485a70..8c896085226 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 From 5fe75649b3e278562b241abadb7d5a62146cff34 Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Mon, 14 May 2012 23:42:47 -0400 Subject: [PATCH 05/13] Rename changed_mr_email to reassigned_mr_email & make resque friendly #changed_merge_request_email was really sending emails about merge request 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 ++++++------- ...aml => reassigned_merge_request_email.html.haml} | 4 ++-- spec/mailers/notify_spec.rb | 6 +++--- 3 files changed, 11 insertions(+), 12 deletions(-) rename app/views/notify/{changed_merge_request_email.html.haml => reassigned_merge_request_email.html.haml} (87%) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 875e83e5331..5e8af49738b 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -58,13 +58,12 @@ class Notify < ActionMailer::Base @project = @merge_request.project mail(:to => @user.email, :subject => "gitlab | new merge request | #{@merge_request.title} ") end - - def changed_merge_request_email(user, merge_request) - @user = user - @merge_request = MergeRequest.find(merge_request.id) - @assignee_was ||= User.find(@merge_request.assignee_id_was) - @project = @merge_request.project - mail(:to => @user['email'], :subject => "gitlab | merge request changed | #{@merge_request.title} ") + + def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id) + recipient = User.find(recipient_id) + @merge_request = MergeRequest.find(merge_request_id) + @previous_assignee ||= User.find(previous_assignee_id) + mail(:to => recipient.email, :subject => "gitlab | merge request changed | #{@merge_request.title} ") end def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id) diff --git a/app/views/notify/changed_merge_request_email.html.haml b/app/views/notify/reassigned_merge_request_email.html.haml similarity index 87% rename from app/views/notify/changed_merge_request_email.html.haml rename to app/views/notify/reassigned_merge_request_email.html.haml index 403d51232a9..30b1f4fec42 100644 --- a/app/views/notify/changed_merge_request_email.html.haml +++ b/app/views/notify/reassigned_merge_request_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 Merge Request - = link_to truncate(@merge_request.title, :length => 16), project_merge_request_url(@project, @merge_request) + = link_to truncate(@merge_request.title, :length => 16), project_merge_request_url(@merge_request.project, @merge_request) %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 #{@merge_request.assignee.name} + Assignee changed from #{@previous_assignee.name} to #{@merge_request.assignee_name} %td diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 8c896085226..7211f121239 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -123,9 +123,9 @@ describe Notify do end describe 'that are reassigned' do - before(:each) { merge_request.stub(:assignee_id_was).and_return(old_assignee.id) } + before(:each) { merge_request.stub(:assignee_id_was).and_return(previous_assignee.id) } - subject { Notify.changed_merge_request_email(recipient, merge_request) } + subject { Notify.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id) } it_behaves_like 'a multiple recipients email' @@ -134,7 +134,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 From bb22360d1a7548facff3b72b2f9a0e66c6a55bb7 Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Tue, 15 May 2012 18:48:00 -0400 Subject: [PATCH 06/13] Make Notify#note_merge_request_email resque friendly Update method to take ids and then perform #finds itself during mailer queue worker kick-off. --- app/mailers/notify.rb | 11 +++++------ app/models/mailer_observer.rb | 2 +- app/views/notify/note_merge_request_email.html.haml | 4 ++-- spec/mailers/notify_spec.rb | 8 ++++++-- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 5e8af49738b..dc6ef154aac 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -35,13 +35,12 @@ class Notify < ActionMailer::Base @commit = @note.target mail(:to => @user['email'], :subject => "gitlab | note for commit | #{@note.project.name} ") end - - def note_merge_request_email(user, note) - @user = user - @note = Note.find(note['id']) - @project = @note.project + + def note_merge_request_email(recipient_id, note_id) + recipient = User.find(recipient_id) + @note = Note.find(note_id) @merge_request = @note.noteable - mail(:to => @user['email'], :subject => "gitlab | note for merge request | #{@note.project.name} ") + mail(:to => recipient.email, :subject => "gitlab | note for merge request | #{@note.project.name} ") end def note_issue_email(user, note) diff --git a/app/models/mailer_observer.rb b/app/models/mailer_observer.rb index 573e98ef8dd..73920b8fea6 100644 --- a/app/models/mailer_observer.rb +++ b/app/models/mailer_observer.rb @@ -36,7 +36,7 @@ class MailerObserver < ActiveRecord::Observer when "Issue" then Notify.note_issue_email(u, note).deliver when "MergeRequest" then - Notify.note_merge_request_email(u, note).deliver + Notify.note_merge_request_email(u.id, note.id).deliver when "Snippet" true else diff --git a/app/views/notify/note_merge_request_email.html.haml b/app/views/notify/note_merge_request_email.html.haml index e2dfec35b7f..9c2284a6457 100644 --- a/app/views/notify/note_merge_request_email.html.haml +++ b/app/views/notify/note_merge_request_email.html.haml @@ -5,13 +5,13 @@ %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; "} New comment for Merge Request - = link_to truncate(@merge_request.title, :length => 16), project_merge_request_url(@project, @merge_request, :anchor => "note_#{@note.id}") + = link_to truncate(@merge_request.title, :length => 16), project_merge_request_url(@merge_request.project, @merge_request, :anchor => "note_#{@note.id}") %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; "} - %a{:href => "#", :style => "color: #0eb6ce; text-decoration: none;"} #{@note.author.name} + %a{:href => "#", :style => "color: #0eb6ce; text-decoration: none;"} #{@note.author_name} left next message: %br %table{:border => "0", :cellpadding => "0", :cellspacing => "0", :width => "558"} diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 7211f121239..052840967eb 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -153,6 +153,10 @@ describe Notify do let(:note_author) { Factory.create(:user, :name => 'author_name') } let(:note) { Factory.create(:note, :project => project, :author => note_author) } + before :each do + Note.stub(:find).with(note.id).and_return(note) + end + shared_examples 'a note email' do it 'is sent to the given recipient' do should deliver_to recipient.email @@ -191,7 +195,7 @@ describe Notify do end before(:each) { note.stub(:target).and_return(commit) } - subject { Notify.note_commit_email(recipient, note) } + subject { Notify.note_commit_email(recipient.id, note.id) } it_behaves_like 'a note email' @@ -209,7 +213,7 @@ describe Notify do let(:note_on_merge_request_url) { project_merge_request_url(project, merge_request, :anchor => "note_#{note.id}") } before(:each) { note.stub(:noteable).and_return(merge_request) } - subject { Notify.note_merge_request_email(recipient, note) } + subject { Notify.note_merge_request_email(recipient.id, note.id) } it_behaves_like 'a note email' From 435fd8f0874450f2da480fa72b0f014d3f1fe271 Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Tue, 15 May 2012 18:50:36 -0400 Subject: [PATCH 07/13] Make Notify#note_issue_email resque friendly Update method to take ids and then perform #finds itself during mailer queue worker kick-off. --- app/mailers/notify.rb | 11 +++++------ app/views/notify/note_issue_email.html.haml | 4 ++-- spec/mailers/notify_spec.rb | 3 ++- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index dc6ef154aac..328825dfdab 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -43,14 +43,13 @@ class Notify < ActionMailer::Base mail(:to => recipient.email, :subject => "gitlab | note for merge request | #{@note.project.name} ") end - def note_issue_email(user, note) - @user = user - @note = Note.find(note['id']) - @project = @note.project + def note_issue_email(recipient_id, note_id) + recipient = User.find(recipient_id) + @note = Note.find(note_id) @issue = @note.noteable - mail(:to => @user['email'], :subject => "gitlab | note for issue #{@issue.id} | #{@note.project.name} ") + mail(:to => recipient.email, :subject => "gitlab | note for issue #{@issue.id} | #{@note.project.name} ") end - + def new_merge_request_email(merge_request) @merge_request = MergeRequest.find(merge_request['id']) @user = @merge_request.assignee diff --git a/app/views/notify/note_issue_email.html.haml b/app/views/notify/note_issue_email.html.haml index eb2cbc0c33d..17d58bdec73 100644 --- a/app/views/notify/note_issue_email.html.haml +++ b/app/views/notify/note_issue_email.html.haml @@ -5,7 +5,7 @@ %td{:align => "left", :style => "padding: 20px 0 0;"} %h2{:style => "color:#646464 !important; font-weight: bold; margin: 0; padding: 0; line-height: 26px; font-size: 18px; font-family: Helvetica, Arial, sans-serif; "} New comment - - = link_to project_issue_url(@project, @issue, :anchor => "note_#{@note.id}") do + = link_to project_issue_url(@issue.project, @issue, :anchor => "note_#{@note.id}") do = "Issue ##{@issue.id.to_s}" = truncate(@issue.title, :length => 35) %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"} @@ -13,7 +13,7 @@ %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; "} - %a{:href => "#", :style => "color: #0eb6ce; text-decoration: none;"} #{@note.author.name} + %a{:href => "#", :style => "color: #0eb6ce; text-decoration: none;"} #{@note.author_name} left next message: %br %table{:border => "0", :cellpadding => "0", :cellspacing => "0", :width => "558"} diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 052840967eb..e9c87b9b2e9 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -230,7 +230,8 @@ describe Notify do let(:issue) { Factory.create(:issue, :project => project) } let(:note_on_issue_url) { project_issue_url(project, issue, :anchor => "note_#{note.id}") } before(:each) { note.stub(:noteable).and_return(issue) } - subject { Notify.note_issue_email(recipient, note) } + + subject { Notify.note_issue_email(recipient.id, note.id) } it_behaves_like 'a note email' From 0a9a2c2a0b75ba617611382b6335bf2b7fc68b9f Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Tue, 15 May 2012 19:20:15 -0400 Subject: [PATCH 08/13] Make Notify#note_commit_email resque friendly Update method to take ids and then perform #finds itself during mailer queue worker kick-off. Also, the faux SHA1 cannot have underscores or it will not match the commit pattern defined in the routes. --- app/mailers/notify.rb | 9 ++++----- app/views/notify/note_commit_email.html.haml | 4 ++-- spec/mailers/notify_spec.rb | 5 +++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 328825dfdab..1fa1a2b8ebc 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -28,12 +28,11 @@ class Notify < ActionMailer::Base mail(:to => @user['email'], :subject => "gitlab | #{@note.project.name} ") end - def note_commit_email(user, note) - @user = user - @note = Note.find(note['id']) - @project = @note.project + def note_commit_email(recipient_id, note_id) + recipient = User.find(recipient_id) + @note = Note.find(note_id) @commit = @note.target - mail(:to => @user['email'], :subject => "gitlab | note for commit | #{@note.project.name} ") + mail(:to => recipient.email, :subject => "gitlab | note for commit | #{@note.project.name} ") end def note_merge_request_email(recipient_id, note_id) diff --git a/app/views/notify/note_commit_email.html.haml b/app/views/notify/note_commit_email.html.haml index aee8fe6c5da..c834f1ddfd7 100644 --- a/app/views/notify/note_commit_email.html.haml +++ b/app/views/notify/note_commit_email.html.haml @@ -5,13 +5,13 @@ %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; "} New comment for commit - = link_to truncate(@commit.id.to_s, :length => 16), project_commit_url(@project, :id => @commit.id, :anchor => "note_#{@note.id}") + = link_to truncate(@commit.id.to_s, :length => 16), project_commit_url(@commit.project, :id => @commit.id, :anchor => "note_#{@note.id}") %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; "} - %a{:href => "#", :style => "color: #0eb6ce; text-decoration: none;"} #{@note.author.name} + %a{:href => "#", :style => "color: #0eb6ce; text-decoration: none;"} #{@note.author_name} left next message: %br %table{:border => "0", :cellpadding => "0", :cellspacing => "0", :width => "558"} diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index e9c87b9b2e9..7bf42671033 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -190,7 +190,8 @@ describe Notify do describe 'on a commit' do let(:commit) do mock(:commit).tap do |commit| - commit.stub(:id).and_return('faux_sha_1') + commit.stub(:id).and_return('fauxsha1') + commit.stub(:project).and_return(project) end end before(:each) { note.stub(:target).and_return(commit) } @@ -204,7 +205,7 @@ describe Notify do end it 'contains a link to the commit' do - should have_body_text /faux_sha_1/ + should have_body_text /fauxsha1/ end end From 41c00a20a97e7ee3764822249d4bedc538c515aa Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Tue, 15 May 2012 19:21:12 -0400 Subject: [PATCH 09/13] Make Notify#note_wall_email resque friendly Update method to take ids and then perform #finds itself during mailer queue worker kick-off. --- app/mailers/notify.rb | 9 ++++----- app/views/notify/note_wall_email.html.haml | 4 ++-- spec/mailers/notify_spec.rb | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 1fa1a2b8ebc..bb46ba04f08 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -21,11 +21,10 @@ class Notify < ActionMailer::Base mail(:to => @user.email, :subject => "gitlab | New Issue was created") end - def note_wall_email(user, note) - @user = user - @note = Note.find(note['id']) - @project = @note.project - mail(:to => @user['email'], :subject => "gitlab | #{@note.project.name} ") + def note_wall_email(recipient_id, note_id) + recipient = User.find(recipient_id) + @note = Note.find(note_id) + mail(:to => recipient.email, :subject => "gitlab | #{@note.project.name} ") end def note_commit_email(recipient_id, note_id) diff --git a/app/views/notify/note_wall_email.html.haml b/app/views/notify/note_wall_email.html.haml index da18f64f937..f62e1f917cd 100644 --- a/app/views/notify/note_wall_email.html.haml +++ b/app/views/notify/note_wall_email.html.haml @@ -5,13 +5,13 @@ %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; "} New message on - = link_to "Project Wall", wall_project_url(@project, :anchor => "note_#{@note.id}") + = link_to "Project Wall", wall_project_url(@note.project, :anchor => "note_#{@note.id}") %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; "} - %a{:href => "#", :style => "color: #0eb6ce; text-decoration: none;"} #{@note.author.name} + %a{:href => "#", :style => "color: #0eb6ce; text-decoration: none;"} #{@note.author_name} left next message: %br %table{:border => "0", :cellpadding => "0", :cellspacing => "0", :width => "558"} diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 7bf42671033..60c8c8ab9a0 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -174,7 +174,7 @@ describe Notify do describe 'on a project wall' do let(:note_on_the_wall_url) { wall_project_url(project, :anchor => "note_#{note.id}") } - subject { Notify.note_wall_email(recipient, note) } + subject { Notify.note_wall_email(recipient.id, note.id) } it_behaves_like 'a note email' From 2d124d9496ea2bca71a25a29a4301cc718b5001c Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Tue, 15 May 2012 19:25:21 -0400 Subject: [PATCH 10/13] Add delegate for project's name on Note. --- app/mailers/notify.rb | 8 ++++---- app/models/note.rb | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index bb46ba04f08..a4ffd97fb7c 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -24,28 +24,28 @@ class Notify < ActionMailer::Base def note_wall_email(recipient_id, note_id) recipient = User.find(recipient_id) @note = Note.find(note_id) - mail(:to => recipient.email, :subject => "gitlab | #{@note.project.name} ") + mail(:to => recipient.email, :subject => "gitlab | #{@note.project_name} ") end def note_commit_email(recipient_id, note_id) recipient = User.find(recipient_id) @note = Note.find(note_id) @commit = @note.target - mail(:to => recipient.email, :subject => "gitlab | note for commit | #{@note.project.name} ") + mail(:to => recipient.email, :subject => "gitlab | note for commit | #{@note.project_name} ") end def note_merge_request_email(recipient_id, note_id) recipient = User.find(recipient_id) @note = Note.find(note_id) @merge_request = @note.noteable - mail(:to => recipient.email, :subject => "gitlab | note for merge request | #{@note.project.name} ") + mail(:to => recipient.email, :subject => "gitlab | note for merge request | #{@note.project_name} ") end def note_issue_email(recipient_id, note_id) recipient = User.find(recipient_id) @note = Note.find(note_id) @issue = @note.noteable - mail(:to => recipient.email, :subject => "gitlab | note for issue #{@issue.id} | #{@note.project.name} ") + mail(:to => recipient.email, :subject => "gitlab | note for issue #{@issue.id} | #{@note.project_name} ") end def new_merge_request_email(merge_request) diff --git a/app/models/note.rb b/app/models/note.rb index cee726ea0e5..c655eb807eb 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -7,6 +7,10 @@ class Note < ActiveRecord::Base belongs_to :author, :class_name => "User" + delegate :name, + :to => :project, + :prefix => true + delegate :name, :email, :to => :author, From 39061af9f8f19d114b48f79a66c22116a52e57be Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Tue, 15 May 2012 19:36:48 -0400 Subject: [PATCH 11/13] Make Notify#new_issue_email resque friendly. --- app/mailers/notify.rb | 9 +++------ app/views/notify/new_issue_email.html.haml | 2 +- spec/mailers/notify_spec.rb | 2 +- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index a4ffd97fb7c..b776878b03b 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -13,12 +13,9 @@ class Notify < ActionMailer::Base mail(:to => @user.email, :subject => "gitlab | Account was created for you") end - def new_issue_email(issue) - @issue = Issue.find(issue['id']) - @user = @issue.assignee - @project = @issue.project - - mail(:to => @user.email, :subject => "gitlab | New Issue was created") + def new_issue_email(issue_id) + @issue = Issue.find(issue_id) + mail(:to => @issue.assignee_email, :subject => "gitlab | New Issue was created") end def note_wall_email(recipient_id, note_id) diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml index 64c5aa611eb..dd6f50c0686 100644 --- a/app/views/notify/new_issue_email.html.haml +++ b/app/views/notify/new_issue_email.html.haml @@ -10,7 +10,7 @@ %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"} %td{:align => "left", :style => "padding: 20px 0 0;"} %h2{:style => "color:#646464 !important; font-weight: bold; margin: 0; padding: 0; line-height: 26px; font-size: 18px; font-family: Helvetica, Arial, sans-serif; "} - = link_to project_issue_url(@project, @issue), :title => @issue.title do + = link_to project_issue_url(@issue.project, @issue), :title => @issue.title do = "Issue ##{@issue.id.to_s}" = truncate(@issue.title, :length => 45) %br diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 60c8c8ab9a0..40a51437908 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -59,7 +59,7 @@ describe Notify do let(:issue) { Factory.create(:issue, :assignee => assignee, :project => project ) } describe 'that are new' do - subject { Notify.new_issue_email(issue) } + subject { Notify.new_issue_email(issue.id) } it_behaves_like 'an assignee email' From e660043d22bd686f6799a46b506a4c2ad53e46fa Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Tue, 15 May 2012 19:41:37 -0400 Subject: [PATCH 12/13] Make Notify#new_merge_request_email resque friendly. --- app/mailers/notify.rb | 8 +++----- app/views/notify/new_merge_request_email.html.haml | 6 ++---- spec/mailers/notify_spec.rb | 2 +- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index b776878b03b..05fd17b511a 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -45,11 +45,9 @@ class Notify < ActionMailer::Base mail(:to => recipient.email, :subject => "gitlab | note for issue #{@issue.id} | #{@note.project_name} ") end - def new_merge_request_email(merge_request) - @merge_request = MergeRequest.find(merge_request['id']) - @user = @merge_request.assignee - @project = @merge_request.project - mail(:to => @user.email, :subject => "gitlab | new merge request | #{@merge_request.title} ") + def new_merge_request_email(merge_request_id) + @merge_request = MergeRequest.find(merge_request_id) + mail(:to => @merge_request.assignee_email, :subject => "gitlab | new merge request | #{@merge_request.title} ") end def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id) diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 57c35db50cb..f7ec01e84b3 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -5,16 +5,14 @@ %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; "} New Merge Request - = link_to truncate(@merge_request.title, :length => 16), project_merge_request_url(@project, @merge_request) + = link_to truncate(@merge_request.title, :length => 16), project_merge_request_url(@merge_request.project, @merge_request) %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; "} Branches: #{@merge_request.source_branch} → #{@merge_request.target_branch} - %p{:style => "color:#767676; font-weight: normal; margin: 0; padding: 0; line-height: 20px; font-size: 12px;font-family: Helvetica, Arial, sans-serif; "} - Asignee: #{@merge_request.author.name} → #{@merge_request.assignee.name} - + Asignee: #{@merge_request.author_name} → #{@merge_request.assignee_name} %td diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 40a51437908..065094e964e 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -101,7 +101,7 @@ describe Notify do let(:merge_request) { Factory.create(:merge_request, :assignee => assignee, :project => project) } describe 'that are new' do - subject { Notify.new_merge_request_email(merge_request) } + subject { Notify.new_merge_request_email(merge_request.id) } it_behaves_like 'an assignee email' From 991d23e216cc4a847ee5724967ea69f40bb16fe8 Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Tue, 15 May 2012 22:27:35 -0400 Subject: [PATCH 13/13] Change calls to Notify methods to send IDs instead of objects. --- app/models/mailer_observer.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/mailer_observer.rb b/app/models/mailer_observer.rb index 73920b8fea6..e581ae804e8 100644 --- a/app/models/mailer_observer.rb +++ b/app/models/mailer_observer.rb @@ -18,7 +18,7 @@ class MailerObserver < ActiveRecord::Observer def new_issue(issue) if issue.assignee != current_user - Notify.new_issue_email(issue).deliver + Notify.new_issue_email(issue.id).deliver end end @@ -32,26 +32,26 @@ class MailerObserver < ActiveRecord::Observer note.project.users.reject { |u| u.id == current_user.id } .each do |u| case note.noteable_type when "Commit" then - Notify.note_commit_email(u, note).deliver + Notify.note_commit_email(u.id, note.id).deliver when "Issue" then - Notify.note_issue_email(u, note).deliver + Notify.note_issue_email(u.id, note.id).deliver when "MergeRequest" then Notify.note_merge_request_email(u.id, note.id).deliver when "Snippet" true else - Notify.note_wall_email(u, note).deliver + Notify.note_wall_email(u.id, note.id).deliver end end # Notify only author of resource elsif note.notify_author - Notify.note_commit_email(note.commit_author, note).deliver + Notify.note_commit_email(note.commit_author.id, note.id).deliver end end def new_merge_request(merge_request) if merge_request.assignee != current_user - Notify.new_merge_request_email(merge_request).deliver + Notify.new_merge_request_email(merge_request.id).deliver end end @@ -61,7 +61,7 @@ class MailerObserver < ActiveRecord::Observer recipients_ids.delete current_user.id User.find(recipients_ids).each do |user| - Notify.changed_merge_request_email(user, merge_request).deliver + Notify.reassigned_merge_request_email(user.id, merge_request.id, merge_request.assignee_id_was).deliver end end