Commit 780d868fec936327cbdf688cf6e2571b07d780c9
1 parent
c1ff1fde
Exists in
spb-stable
and in
2 other branches
Revert "Merge branch 'fix-email-threading' into 'master'"
This reverts commit 97fd990ecde387290be269ef7daafa5761f94af6, reversing changes made to f451a697e0c018359e6d8ff3aaba4eb0484c4bee. Conflicts: app/mailers/emails/notes.rb app/mailers/emails/projects.rb
Showing
11 changed files
with
80 additions
and
142 deletions
Show diff stats
app/mailers/emails/groups.rb
| @@ -4,7 +4,7 @@ module Emails | @@ -4,7 +4,7 @@ module Emails | ||
| 4 | @membership = UsersGroup.find(user_group_id) | 4 | @membership = UsersGroup.find(user_group_id) |
| 5 | @group = @membership.group | 5 | @group = @membership.group |
| 6 | @target_url = group_url(@group) | 6 | @target_url = group_url(@group) |
| 7 | - mail(cc: @membership.user.email, | 7 | + mail(to: @membership.user.email, |
| 8 | subject: subject("Access to group was granted")) | 8 | subject: subject("Access to group was granted")) |
| 9 | end | 9 | end |
| 10 | end | 10 | end |
app/mailers/emails/issues.rb
| @@ -4,10 +4,10 @@ module Emails | @@ -4,10 +4,10 @@ module Emails | ||
| 4 | @issue = Issue.find(issue_id) | 4 | @issue = Issue.find(issue_id) |
| 5 | @project = @issue.project | 5 | @project = @issue.project |
| 6 | @target_url = project_issue_url(@project, @issue) | 6 | @target_url = project_issue_url(@project, @issue) |
| 7 | - mail_new_thread(@issue, | ||
| 8 | - from: sender(@issue.author_id), | ||
| 9 | - cc: recipient(recipient_id), | ||
| 10 | - subject: subject("#{@issue.title} (##{@issue.iid})")) | 7 | + set_message_id("issue_#{issue_id}") |
| 8 | + mail(from: sender(@issue.author_id), | ||
| 9 | + to: recipient(recipient_id), | ||
| 10 | + subject: subject("#{@issue.title} (##{@issue.iid})")) | ||
| 11 | end | 11 | end |
| 12 | 12 | ||
| 13 | def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id, updated_by_user_id) | 13 | def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id, updated_by_user_id) |
| @@ -15,10 +15,10 @@ module Emails | @@ -15,10 +15,10 @@ module Emails | ||
| 15 | @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id | 15 | @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id |
| 16 | @project = @issue.project | 16 | @project = @issue.project |
| 17 | @target_url = project_issue_url(@project, @issue) | 17 | @target_url = project_issue_url(@project, @issue) |
| 18 | - mail_answer_thread(@issue, | ||
| 19 | - from: sender(updated_by_user_id), | ||
| 20 | - cc: recipient(recipient_id), | ||
| 21 | - subject: subject("#{@issue.title} (##{@issue.iid})")) | 18 | + set_reference("issue_#{issue_id}") |
| 19 | + mail(from: sender(updated_by_user_id), | ||
| 20 | + to: recipient(recipient_id), | ||
| 21 | + subject: subject("#{@issue.title} (##{@issue.iid})")) | ||
| 22 | end | 22 | end |
| 23 | 23 | ||
| 24 | def closed_issue_email(recipient_id, issue_id, updated_by_user_id) | 24 | def closed_issue_email(recipient_id, issue_id, updated_by_user_id) |
| @@ -26,10 +26,10 @@ module Emails | @@ -26,10 +26,10 @@ module Emails | ||
| 26 | @project = @issue.project | 26 | @project = @issue.project |
| 27 | @updated_by = User.find updated_by_user_id | 27 | @updated_by = User.find updated_by_user_id |
| 28 | @target_url = project_issue_url(@project, @issue) | 28 | @target_url = project_issue_url(@project, @issue) |
| 29 | - mail_answer_thread(@issue, | ||
| 30 | - from: sender(updated_by_user_id), | ||
| 31 | - cc: recipient(recipient_id), | ||
| 32 | - subject: subject("#{@issue.title} (##{@issue.iid})")) | 29 | + set_reference("issue_#{issue_id}") |
| 30 | + mail(from: sender(updated_by_user_id), | ||
| 31 | + to: recipient(recipient_id), | ||
| 32 | + subject: subject("#{@issue.title} (##{@issue.iid})")) | ||
| 33 | end | 33 | end |
| 34 | 34 | ||
| 35 | def issue_status_changed_email(recipient_id, issue_id, status, updated_by_user_id) | 35 | def issue_status_changed_email(recipient_id, issue_id, status, updated_by_user_id) |
| @@ -38,10 +38,10 @@ module Emails | @@ -38,10 +38,10 @@ module Emails | ||
| 38 | @project = @issue.project | 38 | @project = @issue.project |
| 39 | @updated_by = User.find updated_by_user_id | 39 | @updated_by = User.find updated_by_user_id |
| 40 | @target_url = project_issue_url(@project, @issue) | 40 | @target_url = project_issue_url(@project, @issue) |
| 41 | - mail_answer_thread(@issue, | ||
| 42 | - from: sender(updated_by_user_id), | ||
| 43 | - cc: recipient(recipient_id), | ||
| 44 | - subject: subject("#{@issue.title} (##{@issue.iid})")) | 41 | + set_reference("issue_#{issue_id}") |
| 42 | + mail(from: sender(updated_by_user_id), | ||
| 43 | + to: recipient(recipient_id), | ||
| 44 | + subject: subject("#{@issue.title} (##{@issue.iid})")) | ||
| 45 | end | 45 | end |
| 46 | end | 46 | end |
| 47 | end | 47 | end |
app/mailers/emails/merge_requests.rb
| @@ -4,10 +4,10 @@ module Emails | @@ -4,10 +4,10 @@ module Emails | ||
| 4 | @merge_request = MergeRequest.find(merge_request_id) | 4 | @merge_request = MergeRequest.find(merge_request_id) |
| 5 | @project = @merge_request.project | 5 | @project = @merge_request.project |
| 6 | @target_url = project_merge_request_url(@project, @merge_request) | 6 | @target_url = project_merge_request_url(@project, @merge_request) |
| 7 | - mail_new_thread(@merge_request, | ||
| 8 | - from: sender(@merge_request.author_id), | ||
| 9 | - cc: recipient(recipient_id), | ||
| 10 | - subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) | 7 | + set_message_id("merge_request_#{merge_request_id}") |
| 8 | + mail(from: sender(@merge_request.author_id), | ||
| 9 | + to: recipient(recipient_id), | ||
| 10 | + subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) | ||
| 11 | end | 11 | end |
| 12 | 12 | ||
| 13 | def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id) | 13 | def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id) |
| @@ -15,10 +15,10 @@ module Emails | @@ -15,10 +15,10 @@ module Emails | ||
| 15 | @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id | 15 | @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id |
| 16 | @project = @merge_request.project | 16 | @project = @merge_request.project |
| 17 | @target_url = project_merge_request_url(@project, @merge_request) | 17 | @target_url = project_merge_request_url(@project, @merge_request) |
| 18 | - mail_answer_thread(@merge_request, | ||
| 19 | - from: sender(updated_by_user_id), | ||
| 20 | - cc: recipient(recipient_id), | ||
| 21 | - subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) | 18 | + set_reference("merge_request_#{merge_request_id}") |
| 19 | + mail(from: sender(updated_by_user_id), | ||
| 20 | + to: recipient(recipient_id), | ||
| 21 | + subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) | ||
| 22 | end | 22 | end |
| 23 | 23 | ||
| 24 | def closed_merge_request_email(recipient_id, merge_request_id, updated_by_user_id) | 24 | def closed_merge_request_email(recipient_id, merge_request_id, updated_by_user_id) |
| @@ -26,20 +26,20 @@ module Emails | @@ -26,20 +26,20 @@ module Emails | ||
| 26 | @updated_by = User.find updated_by_user_id | 26 | @updated_by = User.find updated_by_user_id |
| 27 | @project = @merge_request.project | 27 | @project = @merge_request.project |
| 28 | @target_url = project_merge_request_url(@project, @merge_request) | 28 | @target_url = project_merge_request_url(@project, @merge_request) |
| 29 | - mail_answer_thread(@merge_request, | ||
| 30 | - from: sender(updated_by_user_id), | ||
| 31 | - cc: recipient(recipient_id), | ||
| 32 | - subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) | 29 | + set_reference("merge_request_#{merge_request_id}") |
| 30 | + mail(from: sender(updated_by_user_id), | ||
| 31 | + to: recipient(recipient_id), | ||
| 32 | + subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) | ||
| 33 | end | 33 | end |
| 34 | 34 | ||
| 35 | def merged_merge_request_email(recipient_id, merge_request_id, updated_by_user_id) | 35 | def merged_merge_request_email(recipient_id, merge_request_id, updated_by_user_id) |
| 36 | @merge_request = MergeRequest.find(merge_request_id) | 36 | @merge_request = MergeRequest.find(merge_request_id) |
| 37 | @project = @merge_request.project | 37 | @project = @merge_request.project |
| 38 | @target_url = project_merge_request_url(@project, @merge_request) | 38 | @target_url = project_merge_request_url(@project, @merge_request) |
| 39 | - mail_answer_thread(@merge_request, | ||
| 40 | - from: sender(updated_by_user_id), | ||
| 41 | - cc: recipient(recipient_id), | ||
| 42 | - subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) | 39 | + set_reference("merge_request_#{merge_request_id}") |
| 40 | + mail(from: sender(updated_by_user_id), | ||
| 41 | + to: recipient(recipient_id), | ||
| 42 | + subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) | ||
| 43 | end | 43 | end |
| 44 | end | 44 | end |
| 45 | 45 |
app/mailers/emails/notes.rb
| @@ -5,10 +5,9 @@ module Emails | @@ -5,10 +5,9 @@ module Emails | ||
| 5 | @commit = @note.noteable | 5 | @commit = @note.noteable |
| 6 | @project = @note.project | 6 | @project = @note.project |
| 7 | @target_url = project_commit_url(@project, @commit, anchor: "note_#{@note.id}") | 7 | @target_url = project_commit_url(@project, @commit, anchor: "note_#{@note.id}") |
| 8 | - mail_answer_thread(@commit, | ||
| 9 | - from: sender(@note.author_id), | ||
| 10 | - cc: recipient(recipient_id), | ||
| 11 | - subject: subject("#{@commit.title} (#{@commit.short_id})")) | 8 | + mail(from: sender(@note.author_id), |
| 9 | + to: recipient(recipient_id), | ||
| 10 | + subject: subject("#{@commit.title} (#{@commit.short_id})")) | ||
| 12 | end | 11 | end |
| 13 | 12 | ||
| 14 | def note_issue_email(recipient_id, note_id) | 13 | def note_issue_email(recipient_id, note_id) |
| @@ -16,10 +15,10 @@ module Emails | @@ -16,10 +15,10 @@ module Emails | ||
| 16 | @issue = @note.noteable | 15 | @issue = @note.noteable |
| 17 | @project = @note.project | 16 | @project = @note.project |
| 18 | @target_url = project_issue_url(@project, @issue, anchor: "note_#{@note.id}") | 17 | @target_url = project_issue_url(@project, @issue, anchor: "note_#{@note.id}") |
| 19 | - mail_answer_thread(@issue, | ||
| 20 | - from: sender(@note.author_id), | ||
| 21 | - cc: recipient(recipient_id), | ||
| 22 | - subject: subject("#{@issue.title} (##{@issue.iid})")) | 18 | + set_reference("issue_#{@issue.id}") |
| 19 | + mail(from: sender(@note.author_id), | ||
| 20 | + to: recipient(recipient_id), | ||
| 21 | + subject: subject("#{@issue.title} (##{@issue.iid})")) | ||
| 23 | end | 22 | end |
| 24 | 23 | ||
| 25 | def note_merge_request_email(recipient_id, note_id) | 24 | def note_merge_request_email(recipient_id, note_id) |
| @@ -27,10 +26,10 @@ module Emails | @@ -27,10 +26,10 @@ module Emails | ||
| 27 | @merge_request = @note.noteable | 26 | @merge_request = @note.noteable |
| 28 | @project = @note.project | 27 | @project = @note.project |
| 29 | @target_url = project_merge_request_url(@project, @merge_request, anchor: "note_#{@note.id}") | 28 | @target_url = project_merge_request_url(@project, @merge_request, anchor: "note_#{@note.id}") |
| 30 | - mail_answer_thread(@merge_request, | ||
| 31 | - from: sender(@note.author_id), | ||
| 32 | - cc: recipient(recipient_id), | ||
| 33 | - subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) | 29 | + set_reference("merge_request_#{@merge_request.id}") |
| 30 | + mail(from: sender(@note.author_id), | ||
| 31 | + to: recipient(recipient_id), | ||
| 32 | + subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) | ||
| 34 | end | 33 | end |
| 35 | end | 34 | end |
| 36 | end | 35 | end |
app/mailers/emails/projects.rb
| @@ -4,7 +4,7 @@ module Emails | @@ -4,7 +4,7 @@ module Emails | ||
| 4 | @users_project = UsersProject.find user_project_id | 4 | @users_project = UsersProject.find user_project_id |
| 5 | @project = @users_project.project | 5 | @project = @users_project.project |
| 6 | @target_url = project_url(@project) | 6 | @target_url = project_url(@project) |
| 7 | - mail(cc: @users_project.user.email, | 7 | + mail(to: @users_project.user.email, |
| 8 | subject: subject("Access to project was granted")) | 8 | subject: subject("Access to project was granted")) |
| 9 | end | 9 | end |
| 10 | 10 | ||
| @@ -12,7 +12,7 @@ module Emails | @@ -12,7 +12,7 @@ module Emails | ||
| 12 | @user = User.find user_id | 12 | @user = User.find user_id |
| 13 | @project = Project.find project_id | 13 | @project = Project.find project_id |
| 14 | @target_url = project_url(@project) | 14 | @target_url = project_url(@project) |
| 15 | - mail(cc: @user.email, | 15 | + mail(to: @user.email, |
| 16 | subject: subject("Project was moved")) | 16 | subject: subject("Project was moved")) |
| 17 | end | 17 | end |
| 18 | 18 | ||
| @@ -32,7 +32,7 @@ module Emails | @@ -32,7 +32,7 @@ module Emails | ||
| 32 | end | 32 | end |
| 33 | 33 | ||
| 34 | mail(from: sender(author_id), | 34 | mail(from: sender(author_id), |
| 35 | - cc: recipient, | 35 | + to: recipient, |
| 36 | subject: subject(@subject)) | 36 | subject: subject(@subject)) |
| 37 | end | 37 | end |
| 38 | end | 38 | end |
app/mailers/notify.rb
| 1 | class Notify < ActionMailer::Base | 1 | class Notify < ActionMailer::Base |
| 2 | - include ActionDispatch::Routing::PolymorphicRoutes | ||
| 3 | - | ||
| 4 | include Emails::Issues | 2 | include Emails::Issues |
| 5 | include Emails::MergeRequests | 3 | include Emails::MergeRequests |
| 6 | include Emails::Notes | 4 | include Emails::Notes |
| @@ -18,7 +16,6 @@ class Notify < ActionMailer::Base | @@ -18,7 +16,6 @@ class Notify < ActionMailer::Base | ||
| 18 | default_url_options[:script_name] = Gitlab.config.gitlab.relative_url_root | 16 | default_url_options[:script_name] = Gitlab.config.gitlab.relative_url_root |
| 19 | 17 | ||
| 20 | default from: Proc.new { default_sender_address.format } | 18 | default from: Proc.new { default_sender_address.format } |
| 21 | - default to: Proc.new { project_sender_address.format } | ||
| 22 | default reply_to: "noreply@#{Gitlab.config.gitlab.host}" | 19 | default reply_to: "noreply@#{Gitlab.config.gitlab.host}" |
| 23 | 20 | ||
| 24 | # Just send email with 2 seconds delay | 21 | # Just send email with 2 seconds delay |
| @@ -35,17 +32,6 @@ class Notify < ActionMailer::Base | @@ -35,17 +32,6 @@ class Notify < ActionMailer::Base | ||
| 35 | address | 32 | address |
| 36 | end | 33 | end |
| 37 | 34 | ||
| 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 | - | ||
| 49 | # Return an email address that displays the name of the sender. | 35 | # Return an email address that displays the name of the sender. |
| 50 | # Only the displayed name changes; the actual email address is always the same. | 36 | # Only the displayed name changes; the actual email address is always the same. |
| 51 | def sender(sender_id) | 37 | def sender(sender_id) |
| @@ -67,6 +53,14 @@ class Notify < ActionMailer::Base | @@ -67,6 +53,14 @@ class Notify < ActionMailer::Base | ||
| 67 | end | 53 | end |
| 68 | end | 54 | end |
| 69 | 55 | ||
| 56 | + # Set the Message-ID header field | ||
| 57 | + # | ||
| 58 | + # local_part - The local part of the message ID | ||
| 59 | + # | ||
| 60 | + def set_message_id(local_part) | ||
| 61 | + headers["Message-ID"] = "<#{local_part}@#{Gitlab.config.gitlab.host}>" | ||
| 62 | + end | ||
| 63 | + | ||
| 70 | # Set the References header field | 64 | # Set the References header field |
| 71 | # | 65 | # |
| 72 | # local_part - The local part of the referenced message ID | 66 | # local_part - The local part of the referenced message ID |
| @@ -99,48 +93,4 @@ class Notify < ActionMailer::Base | @@ -99,48 +93,4 @@ class Notify < ActionMailer::Base | ||
| 99 | subject << extra.join(' | ') if extra.present? | 93 | subject << extra.join(' | ') if extra.present? |
| 100 | subject | 94 | subject |
| 101 | end | 95 | end |
| 102 | - | ||
| 103 | - # Return a string suitable for inclusion in the 'Message-Id' mail header. | ||
| 104 | - # | ||
| 105 | - # The message-id is generated from the unique URL to a model object. | ||
| 106 | - def message_id(model) | ||
| 107 | - model_name = model.class.model_name.singular_route_key | ||
| 108 | - "<#{model_name}_#{model.id}@#{Gitlab.config.gitlab.host}>" | ||
| 109 | - end | ||
| 110 | - | ||
| 111 | - # Send an email that starts a new conversation thread, | ||
| 112 | - # with headers suitable for grouping by thread in email clients. | ||
| 113 | - # | ||
| 114 | - # See: mail_answer_thread | ||
| 115 | - def mail_new_thread(model, headers = {}, &block) | ||
| 116 | - raise ArgumentError, '"To:" header will be overwritten; use "Cc:" or "Bcc:"' unless headers[:to].nil? | ||
| 117 | - headers[:to] = project_sender_address.format | ||
| 118 | - | ||
| 119 | - headers['Message-ID'] = message_id(model) | ||
| 120 | - | ||
| 121 | - mail(headers, &block) | ||
| 122 | - end | ||
| 123 | - | ||
| 124 | - # Send an email that responds to an existing conversation thread, | ||
| 125 | - # with headers suitable for grouping by thread in email clients. | ||
| 126 | - # | ||
| 127 | - # For grouping emails by thread, email clients heuristics require the answers to: | ||
| 128 | - # | ||
| 129 | - # * have a subject that begin by 'Re: ' | ||
| 130 | - # * have a 'In-Reply-To' or 'References' header that references the original 'Message-ID' | ||
| 131 | - # * have stable 'From' and 'To' headers between messages of the same thread | ||
| 132 | - # | ||
| 133 | - def mail_answer_thread(model, headers = {}, &block) | ||
| 134 | - raise ArgumentError, '"To:" header will be overwritten; use "Cc:" or "Bcc:"' unless headers[:to].nil? | ||
| 135 | - headers[:to] = project_sender_address.format | ||
| 136 | - | ||
| 137 | - headers['In-Reply-To'] = message_id(model) | ||
| 138 | - headers['References'] = message_id(model) | ||
| 139 | - | ||
| 140 | - if (headers[:subject]) | ||
| 141 | - headers[:subject].prepend('Re: ') | ||
| 142 | - end | ||
| 143 | - | ||
| 144 | - mail(headers, &block) | ||
| 145 | - end | ||
| 146 | end | 96 | end |
spec/mailers/notify_spec.rb
| @@ -10,7 +10,7 @@ describe Notify do | @@ -10,7 +10,7 @@ describe Notify do | ||
| 10 | 10 | ||
| 11 | shared_examples 'a multiple recipients email' do | 11 | shared_examples 'a multiple recipients email' do |
| 12 | it 'is sent to the given recipient' do | 12 | it 'is sent to the given recipient' do |
| 13 | - should cc_to recipient.email | 13 | + should deliver_to recipient.email |
| 14 | end | 14 | end |
| 15 | end | 15 | end |
| 16 | 16 | ||
| @@ -22,23 +22,6 @@ describe Notify do | @@ -22,23 +22,6 @@ describe Notify do | ||
| 22 | end | 22 | end |
| 23 | end | 23 | end |
| 24 | 24 | ||
| 25 | - shared_examples 'an email starting a new thread' do |message_id_prefix| | ||
| 26 | - it 'has a discussion identifier' do | ||
| 27 | - should have_header 'Message-ID', /<#{message_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/ | ||
| 28 | - end | ||
| 29 | - end | ||
| 30 | - | ||
| 31 | - shared_examples 'an answer to an existing thread' do |thread_id_prefix| | ||
| 32 | - it 'has a subject that begins with Re: ' do | ||
| 33 | - should have_subject /^Re: / | ||
| 34 | - end | ||
| 35 | - | ||
| 36 | - it 'has headers that reference an existing thread' do | ||
| 37 | - should have_header 'References', /<#{thread_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/ | ||
| 38 | - should have_header 'In-Reply-To', /<#{thread_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/ | ||
| 39 | - end | ||
| 40 | - end | ||
| 41 | - | ||
| 42 | describe 'for new users, the email' do | 25 | describe 'for new users, the email' do |
| 43 | let(:example_site_path) { root_path } | 26 | let(:example_site_path) { root_path } |
| 44 | let(:new_user) { create(:user, email: 'newguy@example.com', created_by_id: 1) } | 27 | let(:new_user) { create(:user, email: 'newguy@example.com', created_by_id: 1) } |
| @@ -158,7 +141,7 @@ describe Notify do | @@ -158,7 +141,7 @@ describe Notify do | ||
| 158 | end | 141 | end |
| 159 | 142 | ||
| 160 | it 'is sent to the assignee' do | 143 | it 'is sent to the assignee' do |
| 161 | - should cc_to assignee.email | 144 | + should deliver_to assignee.email |
| 162 | end | 145 | end |
| 163 | end | 146 | end |
| 164 | 147 | ||
| @@ -170,7 +153,6 @@ describe Notify do | @@ -170,7 +153,6 @@ describe Notify do | ||
| 170 | subject { Notify.new_issue_email(issue.assignee_id, issue.id) } | 153 | subject { Notify.new_issue_email(issue.assignee_id, issue.id) } |
| 171 | 154 | ||
| 172 | it_behaves_like 'an assignee email' | 155 | it_behaves_like 'an assignee email' |
| 173 | - it_behaves_like 'an email starting a new thread', 'issue' | ||
| 174 | 156 | ||
| 175 | it 'has the correct subject' do | 157 | it 'has the correct subject' do |
| 176 | should have_subject /#{project.name} \| #{issue.title} \(##{issue.iid}\)/ | 158 | should have_subject /#{project.name} \| #{issue.title} \(##{issue.iid}\)/ |
| @@ -179,6 +161,10 @@ describe Notify do | @@ -179,6 +161,10 @@ describe Notify do | ||
| 179 | it 'contains a link to the new issue' do | 161 | it 'contains a link to the new issue' do |
| 180 | should have_body_text /#{project_issue_path project, issue}/ | 162 | should have_body_text /#{project_issue_path project, issue}/ |
| 181 | end | 163 | end |
| 164 | + | ||
| 165 | + it 'has the correct message-id set' do | ||
| 166 | + should have_header 'Message-ID', "<issue_#{issue.id}@#{Gitlab.config.gitlab.host}>" | ||
| 167 | + end | ||
| 182 | end | 168 | end |
| 183 | 169 | ||
| 184 | describe 'that are new with a description' do | 170 | describe 'that are new with a description' do |
| @@ -193,7 +179,6 @@ describe Notify do | @@ -193,7 +179,6 @@ describe Notify do | ||
| 193 | subject { Notify.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id, current_user) } | 179 | subject { Notify.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id, current_user) } |
| 194 | 180 | ||
| 195 | it_behaves_like 'a multiple recipients email' | 181 | it_behaves_like 'a multiple recipients email' |
| 196 | - it_behaves_like 'an answer to an existing thread', 'issue' | ||
| 197 | 182 | ||
| 198 | it 'is sent as the author' do | 183 | it 'is sent as the author' do |
| 199 | sender = subject.header[:from].addrs[0] | 184 | sender = subject.header[:from].addrs[0] |
| @@ -216,14 +201,16 @@ describe Notify do | @@ -216,14 +201,16 @@ describe Notify do | ||
| 216 | it 'contains a link to the issue' do | 201 | it 'contains a link to the issue' do |
| 217 | should have_body_text /#{project_issue_path project, issue}/ | 202 | should have_body_text /#{project_issue_path project, issue}/ |
| 218 | end | 203 | end |
| 204 | + | ||
| 205 | + it 'has the correct reference set' do | ||
| 206 | + should have_header 'References', "<issue_#{issue.id}@#{Gitlab.config.gitlab.host}>" | ||
| 207 | + end | ||
| 219 | end | 208 | end |
| 220 | 209 | ||
| 221 | describe 'status changed' do | 210 | describe 'status changed' do |
| 222 | let(:status) { 'closed' } | 211 | let(:status) { 'closed' } |
| 223 | subject { Notify.issue_status_changed_email(recipient.id, issue.id, status, current_user) } | 212 | subject { Notify.issue_status_changed_email(recipient.id, issue.id, status, current_user) } |
| 224 | 213 | ||
| 225 | - it_behaves_like 'an answer to an existing thread', 'issue' | ||
| 226 | - | ||
| 227 | it 'is sent as the author' do | 214 | it 'is sent as the author' do |
| 228 | sender = subject.header[:from].addrs[0] | 215 | sender = subject.header[:from].addrs[0] |
| 229 | sender.display_name.should eq(current_user.name) | 216 | sender.display_name.should eq(current_user.name) |
| @@ -245,6 +232,10 @@ describe Notify do | @@ -245,6 +232,10 @@ describe Notify do | ||
| 245 | it 'contains a link to the issue' do | 232 | it 'contains a link to the issue' do |
| 246 | should have_body_text /#{project_issue_path project, issue}/ | 233 | should have_body_text /#{project_issue_path project, issue}/ |
| 247 | end | 234 | end |
| 235 | + | ||
| 236 | + it 'has the correct reference set' do | ||
| 237 | + should have_header 'References', "<issue_#{issue.id}@#{Gitlab.config.gitlab.host}>" | ||
| 238 | + end | ||
| 248 | end | 239 | end |
| 249 | 240 | ||
| 250 | end | 241 | end |
| @@ -258,7 +249,6 @@ describe Notify do | @@ -258,7 +249,6 @@ describe Notify do | ||
| 258 | subject { Notify.new_merge_request_email(merge_request.assignee_id, merge_request.id) } | 249 | subject { Notify.new_merge_request_email(merge_request.assignee_id, merge_request.id) } |
| 259 | 250 | ||
| 260 | it_behaves_like 'an assignee email' | 251 | it_behaves_like 'an assignee email' |
| 261 | - it_behaves_like 'an email starting a new thread', 'merge_request' | ||
| 262 | 252 | ||
| 263 | it 'has the correct subject' do | 253 | it 'has the correct subject' do |
| 264 | should have_subject /#{merge_request.title} \(##{merge_request.iid}\)/ | 254 | should have_subject /#{merge_request.title} \(##{merge_request.iid}\)/ |
| @@ -293,7 +283,6 @@ describe Notify do | @@ -293,7 +283,6 @@ describe Notify do | ||
| 293 | subject { Notify.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) } | 283 | subject { Notify.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) } |
| 294 | 284 | ||
| 295 | it_behaves_like 'a multiple recipients email' | 285 | it_behaves_like 'a multiple recipients email' |
| 296 | - it_behaves_like 'an answer to an existing thread', 'merge_request' | ||
| 297 | 286 | ||
| 298 | it 'is sent as the author' do | 287 | it 'is sent as the author' do |
| 299 | sender = subject.header[:from].addrs[0] | 288 | sender = subject.header[:from].addrs[0] |
| @@ -322,7 +311,6 @@ describe Notify do | @@ -322,7 +311,6 @@ describe Notify do | ||
| 322 | subject { Notify.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) } | 311 | subject { Notify.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) } |
| 323 | 312 | ||
| 324 | it_behaves_like 'a multiple recipients email' | 313 | it_behaves_like 'a multiple recipients email' |
| 325 | - it_behaves_like 'an answer to an existing thread', 'merge_request' | ||
| 326 | 314 | ||
| 327 | it 'is sent as the merge author' do | 315 | it 'is sent as the merge author' do |
| 328 | sender = subject.header[:from].addrs[0] | 316 | sender = subject.header[:from].addrs[0] |
| @@ -341,6 +329,10 @@ describe Notify do | @@ -341,6 +329,10 @@ describe Notify do | ||
| 341 | it 'contains a link to the merge request' do | 329 | it 'contains a link to the merge request' do |
| 342 | should have_body_text /#{project_merge_request_path project, merge_request}/ | 330 | should have_body_text /#{project_merge_request_path project, merge_request}/ |
| 343 | end | 331 | end |
| 332 | + | ||
| 333 | + it 'has the correct reference set' do | ||
| 334 | + should have_header 'References', "<merge_request_#{merge_request.id}@#{Gitlab.config.gitlab.host}>" | ||
| 335 | + end | ||
| 344 | end | 336 | end |
| 345 | end | 337 | end |
| 346 | end | 338 | end |
| @@ -402,7 +394,7 @@ describe Notify do | @@ -402,7 +394,7 @@ describe Notify do | ||
| 402 | end | 394 | end |
| 403 | 395 | ||
| 404 | it 'is sent to the given recipient' do | 396 | it 'is sent to the given recipient' do |
| 405 | - should cc_to recipient.email | 397 | + should deliver_to recipient.email |
| 406 | end | 398 | end |
| 407 | 399 | ||
| 408 | it 'contains the message from the note' do | 400 | it 'contains the message from the note' do |
| @@ -418,7 +410,6 @@ describe Notify do | @@ -418,7 +410,6 @@ describe Notify do | ||
| 418 | subject { Notify.note_commit_email(recipient.id, note.id) } | 410 | subject { Notify.note_commit_email(recipient.id, note.id) } |
| 419 | 411 | ||
| 420 | it_behaves_like 'a note email' | 412 | it_behaves_like 'a note email' |
| 421 | - it_behaves_like 'an answer to an existing thread', 'commits' | ||
| 422 | 413 | ||
| 423 | it 'has the correct subject' do | 414 | it 'has the correct subject' do |
| 424 | should have_subject /#{commit.title} \(#{commit.short_id}\)/ | 415 | should have_subject /#{commit.title} \(#{commit.short_id}\)/ |
| @@ -437,7 +428,6 @@ describe Notify do | @@ -437,7 +428,6 @@ describe Notify do | ||
| 437 | subject { Notify.note_merge_request_email(recipient.id, note.id) } | 428 | subject { Notify.note_merge_request_email(recipient.id, note.id) } |
| 438 | 429 | ||
| 439 | it_behaves_like 'a note email' | 430 | it_behaves_like 'a note email' |
| 440 | - it_behaves_like 'an answer to an existing thread', 'merge_request' | ||
| 441 | 431 | ||
| 442 | it 'has the correct subject' do | 432 | it 'has the correct subject' do |
| 443 | should have_subject /#{merge_request.title} \(##{merge_request.iid}\)/ | 433 | should have_subject /#{merge_request.title} \(##{merge_request.iid}\)/ |
| @@ -456,7 +446,6 @@ describe Notify do | @@ -456,7 +446,6 @@ describe Notify do | ||
| 456 | subject { Notify.note_issue_email(recipient.id, note.id) } | 446 | subject { Notify.note_issue_email(recipient.id, note.id) } |
| 457 | 447 | ||
| 458 | it_behaves_like 'a note email' | 448 | it_behaves_like 'a note email' |
| 459 | - it_behaves_like 'an answer to an existing thread', 'issue' | ||
| 460 | 449 | ||
| 461 | it 'has the correct subject' do | 450 | it 'has the correct subject' do |
| 462 | should have_subject /#{issue.title} \(##{issue.iid}\)/ | 451 | should have_subject /#{issue.title} \(##{issue.iid}\)/ |
| @@ -533,7 +522,7 @@ describe Notify do | @@ -533,7 +522,7 @@ describe Notify do | ||
| 533 | end | 522 | end |
| 534 | 523 | ||
| 535 | it 'is sent to recipient' do | 524 | it 'is sent to recipient' do |
| 536 | - should cc_to 'devs@company.name' | 525 | + should deliver_to 'devs@company.name' |
| 537 | end | 526 | end |
| 538 | 527 | ||
| 539 | it 'has the correct subject' do | 528 | it 'has the correct subject' do |
| @@ -569,7 +558,7 @@ describe Notify do | @@ -569,7 +558,7 @@ describe Notify do | ||
| 569 | end | 558 | end |
| 570 | 559 | ||
| 571 | it 'is sent to recipient' do | 560 | it 'is sent to recipient' do |
| 572 | - should cc_to 'devs@company.name' | 561 | + should deliver_to 'devs@company.name' |
| 573 | end | 562 | end |
| 574 | 563 | ||
| 575 | it 'has the correct subject' do | 564 | it 'has the correct subject' do |
spec/services/issues/close_service_spec.rb
| @@ -22,7 +22,7 @@ describe Issues::CloseService do | @@ -22,7 +22,7 @@ describe Issues::CloseService do | ||
| 22 | 22 | ||
| 23 | it 'should send email to user2 about assign of new issue' do | 23 | it 'should send email to user2 about assign of new issue' do |
| 24 | email = ActionMailer::Base.deliveries.last | 24 | email = ActionMailer::Base.deliveries.last |
| 25 | - email.cc.first.should == user2.email | 25 | + email.to.first.should == user2.email |
| 26 | email.subject.should include(issue.title) | 26 | email.subject.should include(issue.title) |
| 27 | end | 27 | end |
| 28 | 28 |
spec/services/issues/update_service_spec.rb
| @@ -31,7 +31,7 @@ describe Issues::UpdateService do | @@ -31,7 +31,7 @@ describe Issues::UpdateService do | ||
| 31 | 31 | ||
| 32 | it 'should send email to user2 about assign of new issue' do | 32 | it 'should send email to user2 about assign of new issue' do |
| 33 | email = ActionMailer::Base.deliveries.last | 33 | email = ActionMailer::Base.deliveries.last |
| 34 | - email.cc.first.should == user2.email | 34 | + email.to.first.should == user2.email |
| 35 | email.subject.should include(issue.title) | 35 | email.subject.should include(issue.title) |
| 36 | end | 36 | end |
| 37 | 37 |
spec/services/merge_requests/close_service_spec.rb
| @@ -22,7 +22,7 @@ describe MergeRequests::CloseService do | @@ -22,7 +22,7 @@ describe MergeRequests::CloseService do | ||
| 22 | 22 | ||
| 23 | it 'should send email to user2 about assign of new merge_request' do | 23 | it 'should send email to user2 about assign of new merge_request' do |
| 24 | email = ActionMailer::Base.deliveries.last | 24 | email = ActionMailer::Base.deliveries.last |
| 25 | - email.cc.first.should == user2.email | 25 | + email.to.first.should == user2.email |
| 26 | email.subject.should include(merge_request.title) | 26 | email.subject.should include(merge_request.title) |
| 27 | end | 27 | end |
| 28 | 28 |
spec/services/merge_requests/update_service_spec.rb
| @@ -31,7 +31,7 @@ describe MergeRequests::UpdateService do | @@ -31,7 +31,7 @@ describe MergeRequests::UpdateService do | ||
| 31 | 31 | ||
| 32 | it 'should send email to user2 about assign of new merge_request' do | 32 | it 'should send email to user2 about assign of new merge_request' do |
| 33 | email = ActionMailer::Base.deliveries.last | 33 | email = ActionMailer::Base.deliveries.last |
| 34 | - email.cc.first.should == user2.email | 34 | + email.to.first.should == user2.email |
| 35 | email.subject.should include(merge_request.title) | 35 | email.subject.should include(merge_request.title) |
| 36 | end | 36 | end |
| 37 | 37 |