Send notification emails to the "project", and put people in Cc

This fixes email threading in Mail.app, that doesn't like when a thread doesn't have stable recipients. For instance, here is a possible sender-recipient combinations before: From: A To: Me New issue From: B To: Me Reply on new issue From: A To: Me Another reply Mail.app doesn't see B as a participant to the original email thread, and decides to break the thread: it will group all messages from A together, and separately all messages from B. This commit makes the thread look like this: From: A To: gitlab/project Cc: Me New issue From: B To: gitlab/project Cc: Me Reply on new issue From: A To: gitlab/project Cc: Me Another reply Mail.app sees a common recipient, and group the thread correctly.

Send notification emails to the "project", and put people in Cc
This fixes email threading in Mail.app, that doesn't like when a thread doesn't have stable recipients. For instance, here is a possible sender-recipient combinations before: From: A To: Me New issue From: B To: Me Reply on new issue From: A To: Me Another reply Mail.app doesn't see B as a participant to the original email thread, and decides to break the thread: it will group all messages from A together, and separately all messages from B. This commit makes the thread look like this: From: A To: gitlab/project Cc: Me New issue From: B To: gitlab/project Cc: Me Reply on new issue From: A To: gitlab/project Cc: Me Another reply Mail.app sees a common recipient, and group the thread correctly.
Pierre de La Morinerie
1 parent 772f2f1a
Showing 11 changed files with 38 additions and 24 deletions Show diff stats
app/mailers/emails/groups.rb
app/mailers/emails/issues.rb
app/mailers/emails/merge_requests.rb
app/mailers/emails/notes.rb
app/mailers/emails/projects.rb
app/mailers/notify.rb
spec/mailers/notify_spec.rb
spec/services/issues/close_service_spec.rb
spec/services/issues/update_service_spec.rb
spec/services/merge_requests/close_service_spec.rb
spec/services/merge_requests/update_service_spec.rb
@@ -4,7 +4,7 @@ module Emails
       @membership = UsersGroup.find(user_group_id)
       @group = @membership.group
       @target_url = group_url(@group)
-      mail(to: @membership.user.email,
+      mail(cc: @membership.user.email,
            subject: subject("Access to group was granted"))
     end
   end
@@ -6,7 +6,7 @@ module Emails
       @target_url = project_issue_url(@project, @issue)
       set_message_id("issue_#{issue_id}")
       mail(from: sender(@issue.author_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
  
@@ -17,7 +17,7 @@ module Emails
       @target_url = project_issue_url(@project, @issue)
       set_reference("issue_#{issue_id}")
       mail(from: sender(updated_by_user_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
  
@@ -28,7 +28,7 @@ module Emails
       @target_url = project_issue_url(@project, @issue)
       set_reference("issue_#{issue_id}")
       mail(from: sender(updated_by_user_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
  
@@ -40,7 +40,7 @@ module Emails
       @target_url = project_issue_url(@project, @issue)
       set_reference("issue_#{issue_id}")
       mail(from: sender(updated_by_user_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
   end
@@ -6,7 +6,7 @@ module Emails
       @target_url = project_merge_request_url(@project, @merge_request)
       set_message_id("merge_request_#{merge_request_id}")
       mail(from: sender(@merge_request.author_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
     end
  
@@ -17,7 +17,7 @@ module Emails
       @target_url = project_merge_request_url(@project, @merge_request)
       set_reference("merge_request_#{merge_request_id}")
       mail(from: sender(updated_by_user_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
     end
  
@@ -28,7 +28,7 @@ module Emails
       @target_url = project_merge_request_url(@project, @merge_request)
       set_reference("merge_request_#{merge_request_id}")
       mail(from: sender(updated_by_user_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
     end
  
@@ -38,7 +38,7 @@ module Emails
       @target_url = project_merge_request_url(@project, @merge_request)
       set_reference("merge_request_#{merge_request_id}")
       mail(from: sender(updated_by_user_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
     end
   end
@@ -6,7 +6,7 @@ module Emails
       @project = @note.project
       @target_url = project_commit_url(@project, @commit, anchor: "note_#{@note.id}")
       mail(from: sender(@note.author_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("#{@commit.title} (#{@commit.short_id})"))
     end
  
@@ -17,7 +17,7 @@ module Emails
       @target_url = project_issue_url(@project, @issue, anchor: "note_#{@note.id}")
       set_reference("issue_#{@issue.id}")
       mail(from: sender(@note.author_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
  
@@ -28,7 +28,7 @@ module Emails
       @target_url = project_merge_request_url(@project, @merge_request, anchor: "note_#{@note.id}")
       set_reference("merge_request_#{@merge_request.id}")
       mail(from: sender(@note.author_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
     end
  
@@ -37,7 +37,7 @@ module Emails
       @project = @note.project
       @target_url = project_wall_url(@note.project, anchor: "note_#{@note.id}")
       mail(from: sender(@note.author_id),
-           to: recipient(recipient_id),
+           cc: recipient(recipient_id),
            subject: subject("Note on wall"))
     end
   end
@@ -4,7 +4,7 @@ module Emails
       @users_project = UsersProject.find user_project_id
       @project = @users_project.project
       @target_url = project_url(@project)
-      mail(to: @users_project.user.email,
+      mail(cc: @users_project.user.email,
            subject: subject("Access to project was granted"))
     end
  
@@ -12,7 +12,7 @@ module Emails
       @user = User.find user_id
       @project = Project.find project_id
       @target_url = project_url(@project)
-      mail(to: @user.email,
+      mail(cc: @user.email,
            subject: subject("Project was moved"))
     end
  
@@ -30,7 +30,7 @@ module Emails
       end
  
       mail(from: sender(author_id),
-           to: recipient,
+           cc: recipient,
            subject: subject("New push to repository"))
     end
   end
 class Notify < ActionMailer::Base
+  include ActionDispatch::Routing::PolymorphicRoutes
+
   include Emails::Issues
   include Emails::MergeRequests
   include Emails::Notes
@@ -16,6 +18,7 @@ class Notify &lt; ActionMailer::Base
   default_url_options[:script_name] = Gitlab.config.gitlab.relative_url_root
  
   default from: Proc.new { default_sender_address.format }
+  default to:   Proc.new { project_sender_address.format }
   default reply_to: "noreply@#{Gitlab.config.gitlab.host}"
  
   # Just send email with 2 seconds delay
@@ -32,6 +35,17 @@ class Notify &lt; ActionMailer::Base
     address
   end
  
+  # The default email address to send emails to. Includes the project name if possible.
+  def project_sender_address
+    if @project
+      address = default_sender_address
+      address.display_name = @project.name_with_namespace
+      address
+    else
+      default_sender_address
+    end
+  end
+
   # Return an email address that displays the name of the sender.
   # Only the displayed name changes; the actual email address is always the same.
   def sender(sender_id)
@@ -10,7 +10,7 @@ describe Notify do
  
   shared_examples 'a multiple recipients email' do
     it 'is sent to the given recipient' do
-      should deliver_to recipient.email
+      should cc_to recipient.email
     end
   end
  
@@ -141,7 +141,7 @@ describe Notify do
         end
  
         it 'is sent to the assignee' do
-          should deliver_to assignee.email
+          should cc_to assignee.email
         end
       end
  
@@ -394,7 +394,7 @@ describe Notify do
         end
  
         it 'is sent to the given recipient' do
-          should deliver_to recipient.email
+          should cc_to recipient.email
         end
  
         it 'contains the message from the note' do
@@ -538,7 +538,7 @@ describe Notify do
     end
  
     it 'is sent to recipient' do
-      should deliver_to 'devs@company.name'
+      should cc_to 'devs@company.name'
     end
  
     it 'has the correct subject' do
@@ -22,7 +22,7 @@ describe Issues::CloseService do
  
       it 'should send email to user2 about assign of new issue' do
         email = ActionMailer::Base.deliveries.last
-        email.to.first.should == user2.email
+        email.cc.first.should == user2.email
         email.subject.should include(issue.title)
       end
  
@@ -31,7 +31,7 @@ describe Issues::UpdateService do
  
       it 'should send email to user2 about assign of new issue' do
         email = ActionMailer::Base.deliveries.last
-        email.to.first.should == user2.email
+        email.cc.first.should == user2.email
         email.subject.should include(issue.title)
       end
  
@@ -22,7 +22,7 @@ describe MergeRequests::CloseService do
  
       it 'should send email to user2 about assign of new merge_request' do
         email = ActionMailer::Base.deliveries.last
-        email.to.first.should == user2.email
+        email.cc.first.should == user2.email
         email.subject.should include(merge_request.title)
       end
  
@@ -31,7 +31,7 @@ describe MergeRequests::UpdateService do
  
       it 'should send email to user2 about assign of new merge_request' do
         email = ActionMailer::Base.deliveries.last
-        email.to.first.should == user2.email
+        email.cc.first.should == user2.email
         email.subject.should include(merge_request.title)
       end
...	...	@@ -4,7 +4,7 @@ module Emails
4	4	@membership = UsersGroup.find(user_group_id)
5	5	@group = @membership.group
6	6	@target_url = group_url(@group)
7		- mail(to: @membership.user.email,
	7	+ mail(cc: @membership.user.email,
8	8	subject: subject("Access to group was granted"))
9	9	end
10	10	end
...	...
...	...	@@ -6,7 +6,7 @@ module Emails
6	6	@target_url = project_issue_url(@project, @issue)
7	7	set_message_id("issue_#{issue_id}")
8	8	mail(from: sender(@issue.author_id),
9		- to: recipient(recipient_id),
	9	+ cc: recipient(recipient_id),
10	10	subject: subject("#{@issue.title} (##{@issue.iid})"))
11	11	end
12	12
...	...	@@ -17,7 +17,7 @@ module Emails
17	17	@target_url = project_issue_url(@project, @issue)
18	18	set_reference("issue_#{issue_id}")
19	19	mail(from: sender(updated_by_user_id),
20		- to: recipient(recipient_id),
	20	+ cc: recipient(recipient_id),
21	21	subject: subject("#{@issue.title} (##{@issue.iid})"))
22	22	end
23	23
...	...	@@ -28,7 +28,7 @@ module Emails
28	28	@target_url = project_issue_url(@project, @issue)
29	29	set_reference("issue_#{issue_id}")
30	30	mail(from: sender(updated_by_user_id),
31		- to: recipient(recipient_id),
	31	+ cc: recipient(recipient_id),
32	32	subject: subject("#{@issue.title} (##{@issue.iid})"))
33	33	end
34	34
...	...	@@ -40,7 +40,7 @@ module Emails
40	40	@target_url = project_issue_url(@project, @issue)
41	41	set_reference("issue_#{issue_id}")
42	42	mail(from: sender(updated_by_user_id),
43		- to: recipient(recipient_id),
	43	+ cc: recipient(recipient_id),
44	44	subject: subject("#{@issue.title} (##{@issue.iid})"))
45	45	end
46	46	end
...	...
...	...	@@ -6,7 +6,7 @@ module Emails
6	6	@target_url = project_merge_request_url(@project, @merge_request)
7	7	set_message_id("merge_request_#{merge_request_id}")
8	8	mail(from: sender(@merge_request.author_id),
9		- to: recipient(recipient_id),
	9	+ cc: recipient(recipient_id),
10	10	subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
11	11	end
12	12
...	...	@@ -17,7 +17,7 @@ module Emails
17	17	@target_url = project_merge_request_url(@project, @merge_request)
18	18	set_reference("merge_request_#{merge_request_id}")
19	19	mail(from: sender(updated_by_user_id),
20		- to: recipient(recipient_id),
	20	+ cc: recipient(recipient_id),
21	21	subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
22	22	end
23	23
...	...	@@ -28,7 +28,7 @@ module Emails
28	28	@target_url = project_merge_request_url(@project, @merge_request)
29	29	set_reference("merge_request_#{merge_request_id}")
30	30	mail(from: sender(updated_by_user_id),
31		- to: recipient(recipient_id),
	31	+ cc: recipient(recipient_id),
32	32	subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
33	33	end
34	34
...	...	@@ -38,7 +38,7 @@ module Emails
38	38	@target_url = project_merge_request_url(@project, @merge_request)
39	39	set_reference("merge_request_#{merge_request_id}")
40	40	mail(from: sender(updated_by_user_id),
41		- to: recipient(recipient_id),
	41	+ cc: recipient(recipient_id),
42	42	subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
43	43	end
44	44	end
...	...
...	...	@@ -6,7 +6,7 @@ module Emails
6	6	@project = @note.project
7	7	@target_url = project_commit_url(@project, @commit, anchor: "note_#{@note.id}")
8	8	mail(from: sender(@note.author_id),
9		- to: recipient(recipient_id),
	9	+ cc: recipient(recipient_id),
10	10	subject: subject("#{@commit.title} (#{@commit.short_id})"))
11	11	end
12	12
...	...	@@ -17,7 +17,7 @@ module Emails
17	17	@target_url = project_issue_url(@project, @issue, anchor: "note_#{@note.id}")
18	18	set_reference("issue_#{@issue.id}")
19	19	mail(from: sender(@note.author_id),
20		- to: recipient(recipient_id),
	20	+ cc: recipient(recipient_id),
21	21	subject: subject("#{@issue.title} (##{@issue.iid})"))
22	22	end
23	23
...	...	@@ -28,7 +28,7 @@ module Emails
28	28	@target_url = project_merge_request_url(@project, @merge_request, anchor: "note_#{@note.id}")
29	29	set_reference("merge_request_#{@merge_request.id}")
30	30	mail(from: sender(@note.author_id),
31		- to: recipient(recipient_id),
	31	+ cc: recipient(recipient_id),
32	32	subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
33	33	end
34	34
...	...	@@ -37,7 +37,7 @@ module Emails
37	37	@project = @note.project
38	38	@target_url = project_wall_url(@note.project, anchor: "note_#{@note.id}")
39	39	mail(from: sender(@note.author_id),
40		- to: recipient(recipient_id),
	40	+ cc: recipient(recipient_id),
41	41	subject: subject("Note on wall"))
42	42	end
43	43	end
...	...
...	...	@@ -4,7 +4,7 @@ module Emails
4	4	@users_project = UsersProject.find user_project_id
5	5	@project = @users_project.project
6	6	@target_url = project_url(@project)
7		- mail(to: @users_project.user.email,
	7	+ mail(cc: @users_project.user.email,
8	8	subject: subject("Access to project was granted"))
9	9	end
10	10
...	...	@@ -12,7 +12,7 @@ module Emails
12	12	@user = User.find user_id
13	13	@project = Project.find project_id
14	14	@target_url = project_url(@project)
15		- mail(to: @user.email,
	15	+ mail(cc: @user.email,
16	16	subject: subject("Project was moved"))
17	17	end
18	18
...	...	@@ -30,7 +30,7 @@ module Emails
30	30	end
31	31
32	32	mail(from: sender(author_id),
33		- to: recipient,
	33	+ cc: recipient,
34	34	subject: subject("New push to repository"))
35	35	end
36	36	end
...	...
1	1	class Notify < ActionMailer::Base
	2	+ include ActionDispatch::Routing::PolymorphicRoutes
	3	+
2	4	include Emails::Issues
3	5	include Emails::MergeRequests
4	6	include Emails::Notes
...	...	@@ -16,6 +18,7 @@ class Notify < ActionMailer::Base
16	18	default_url_options[:script_name] = Gitlab.config.gitlab.relative_url_root
17	19
18	20	default from: Proc.new { default_sender_address.format }
	21	+ default to: Proc.new { project_sender_address.format }
19	22	default reply_to: "noreply@#{Gitlab.config.gitlab.host}"
20	23
21	24	# Just send email with 2 seconds delay
...	...	@@ -32,6 +35,17 @@ class Notify < ActionMailer::Base
32	35	address
33	36	end
34	37
	38	+ # The default email address to send emails to. Includes the project name if possible.
	39	+ def project_sender_address
	40	+ if @project
	41	+ address = default_sender_address
	42	+ address.display_name = @project.name_with_namespace
	43	+ address
	44	+ else
	45	+ default_sender_address
	46	+ end
	47	+ end
	48	+
35	49	# Return an email address that displays the name of the sender.
36	50	# Only the displayed name changes; the actual email address is always the same.
37	51	def sender(sender_id)
...	...
...	...	@@ -10,7 +10,7 @@ describe Notify do
10	10
11	11	shared_examples 'a multiple recipients email' do
12	12	it 'is sent to the given recipient' do
13		- should deliver_to recipient.email
	13	+ should cc_to recipient.email
14	14	end
15	15	end
16	16
...	...	@@ -141,7 +141,7 @@ describe Notify do
141	141	end
142	142
143	143	it 'is sent to the assignee' do
144		- should deliver_to assignee.email
	144	+ should cc_to assignee.email
145	145	end
146	146	end
147	147
...	...	@@ -394,7 +394,7 @@ describe Notify do
394	394	end
395	395
396	396	it 'is sent to the given recipient' do
397		- should deliver_to recipient.email
	397	+ should cc_to recipient.email
398	398	end
399	399
400	400	it 'contains the message from the note' do
...	...	@@ -538,7 +538,7 @@ describe Notify do
538	538	end
539	539
540	540	it 'is sent to recipient' do
541		- should deliver_to 'devs@company.name'
	541	+ should cc_to 'devs@company.name'
542	542	end
543	543
544	544	it 'has the correct subject' do
...	...
...	...	@@ -22,7 +22,7 @@ describe Issues::CloseService do
22	22
23	23	it 'should send email to user2 about assign of new issue' do
24	24	email = ActionMailer::Base.deliveries.last
25		- email.to.first.should == user2.email
	25	+ email.cc.first.should == user2.email
26	26	email.subject.should include(issue.title)
27	27	end
28	28
...	...
...	...	@@ -31,7 +31,7 @@ describe Issues::UpdateService do
31	31
32	32	it 'should send email to user2 about assign of new issue' do
33	33	email = ActionMailer::Base.deliveries.last
34		- email.to.first.should == user2.email
	34	+ email.cc.first.should == user2.email
35	35	email.subject.should include(issue.title)
36	36	end
37	37
...	...
...	...	@@ -22,7 +22,7 @@ describe MergeRequests::CloseService do
22	22
23	23	it 'should send email to user2 about assign of new merge_request' do
24	24	email = ActionMailer::Base.deliveries.last
25		- email.to.first.should == user2.email
	25	+ email.cc.first.should == user2.email
26	26	email.subject.should include(merge_request.title)
27	27	end
28	28
...	...