From d789d73f07f2a0fa68b55ff1ba2bfc1823a436f4 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu <heinrich@gitlab.com> Date: Wed, 1 Dec 2021 19:44:09 +0800 Subject: [PATCH] Pass current_user when rendering Markdown in email When rendering Markdown in notification emails, we pass in the recipient as current_user in the rendering context so that private references visible to that user are rendered. Without this change, only public references are rendered. This also passes the issuable_reference_expansion_enabled option so that it is rendered similar to how it is in the web UI. Changelog: fixed --- app/mailers/emails/issues.rb | 28 ++++++------- app/mailers/emails/merge_requests.rb | 42 +++++++++---------- app/mailers/emails/notes.rb | 15 +++---- app/mailers/emails/releases.rb | 5 +-- app/views/notify/_note_email.html.haml | 2 +- app/views/notify/issue_due_email.html.haml | 2 +- app/views/notify/new_issue_email.html.haml | 2 +- .../notify/new_merge_request_email.html.haml | 2 +- app/views/notify/new_release_email.html.haml | 2 +- .../service_desk_new_note_email.html.haml | 2 +- ee/app/mailers/ee/emails/issues.rb | 4 +- ee/app/mailers/ee/emails/merge_requests.rb | 6 +-- ee/app/mailers/ee/emails/notes.rb | 2 +- ee/app/mailers/emails/epics.rb | 9 ++-- ...add_merge_request_approver_email.html.haml | 2 +- ee/app/views/notify/new_epic_email.html.haml | 2 +- spec/mailers/notify_spec.rb | 22 ++++++++++ 17 files changed, 86 insertions(+), 63 deletions(-) diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb index 51c4779d8cf6d..bbc4be3b3243d 100644 --- a/app/mailers/emails/issues.rb +++ b/app/mailers/emails/issues.rb @@ -5,18 +5,18 @@ module Issues def new_issue_email(recipient_id, issue_id, reason = nil) setup_issue_mail(issue_id, recipient_id) - mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id, reason)) + mail_new_thread(@issue, issue_thread_options(@issue.author_id, reason)) end def issue_due_email(recipient_id, issue_id, reason = nil) setup_issue_mail(issue_id, recipient_id) - mail_answer_thread(@issue, issue_thread_options(@issue.author_id, recipient_id, reason)) + mail_answer_thread(@issue, issue_thread_options(@issue.author_id, reason)) end def new_mention_in_issue_email(recipient_id, issue_id, updated_by_user_id, reason = nil) setup_issue_mail(issue_id, recipient_id) - mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, reason)) end # rubocop: disable CodeReuse/ActiveRecord @@ -26,7 +26,7 @@ def reassigned_issue_email(recipient_id, issue_id, previous_assignee_ids, update @previous_assignees = [] @previous_assignees = User.where(id: previous_assignee_ids) if previous_assignee_ids.any? - mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, reason)) end # rubocop: enable CodeReuse/ActiveRecord @@ -34,9 +34,8 @@ def closed_issue_email(recipient_id, issue_id, updated_by_user_id, reason: nil, setup_issue_mail(issue_id, recipient_id, closed_via: closed_via) @updated_by = User.find(updated_by_user_id) - @recipient = User.find(recipient_id) - mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, reason)) end def relabeled_issue_email(recipient_id, issue_id, label_names, updated_by_user_id, reason = nil) @@ -44,13 +43,13 @@ def relabeled_issue_email(recipient_id, issue_id, label_names, updated_by_user_i @label_names = label_names @labels_url = project_labels_url(@project) - mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, reason)) end def removed_milestone_issue_email(recipient_id, issue_id, updated_by_user_id, reason = nil) setup_issue_mail(issue_id, recipient_id) - mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, reason)) end def changed_milestone_issue_email(recipient_id, issue_id, milestone, updated_by_user_id, reason = nil) @@ -58,7 +57,7 @@ def changed_milestone_issue_email(recipient_id, issue_id, milestone, updated_by_ @milestone = milestone @milestone_url = milestone_url(@milestone) - mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason).merge({ + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, reason).merge({ template_name: 'changed_milestone_email' })) end @@ -68,7 +67,7 @@ def issue_status_changed_email(recipient_id, issue_id, status, updated_by_user_i @issue_status = status @updated_by = User.find(updated_by_user_id) - mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, reason)) end def issue_moved_email(recipient, issue, new_issue, updated_by_user, reason = nil) @@ -77,7 +76,7 @@ def issue_moved_email(recipient, issue, new_issue, updated_by_user, reason = nil @new_issue = new_issue @new_project = new_issue.project @can_access_project = recipient.can?(:read_project, @new_project) - mail_answer_thread(issue, issue_thread_options(updated_by_user.id, recipient.id, reason)) + mail_answer_thread(issue, issue_thread_options(updated_by_user.id, reason)) end def issue_cloned_email(recipient, issue, new_issue, updated_by_user, reason = nil) @@ -87,7 +86,7 @@ def issue_cloned_email(recipient, issue, new_issue, updated_by_user, reason = ni @issue = issue @new_issue = new_issue @can_access_project = recipient.can?(:read_project, @new_issue.project) - mail_answer_thread(issue, issue_thread_options(updated_by_user.id, recipient.id, reason)) + mail_answer_thread(issue, issue_thread_options(updated_by_user.id, reason)) end def import_issues_csv_email(user_id, project_id, results) @@ -124,14 +123,15 @@ def setup_issue_mail(issue_id, recipient_id, closed_via: nil) @project = @issue.project @target_url = project_issue_url(@project, @issue) @closed_via = closed_via + @recipient = User.find(recipient_id) @sent_notification = SentNotification.record(@issue, recipient_id, reply_key) end - def issue_thread_options(sender_id, recipient_id, reason) + def issue_thread_options(sender_id, reason) { from: sender(sender_id), - to: User.find(recipient_id).notification_email_for(@project.group), + to: @recipient.notification_email_for(@project.group), subject: subject("#{@issue.title} (##{@issue.iid})"), 'X-GitLab-NotificationReason' => reason } diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb index 3dfc8136d452c..d2e710cc329a0 100644 --- a/app/mailers/emails/merge_requests.rb +++ b/app/mailers/emails/merge_requests.rb @@ -5,13 +5,13 @@ module MergeRequests def new_merge_request_email(recipient_id, merge_request_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id, present: true) - mail_new_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, recipient_id, reason)) + mail_new_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, reason)) end def new_mention_in_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id, present: true) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end def push_to_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil, new_commits: [], existing_commits: []) @@ -20,7 +20,7 @@ def push_to_merge_request_email(recipient_id, merge_request_id, updated_by_user_ @existing_commits = existing_commits @updated_by_user = User.find(updated_by_user_id) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end def change_in_merge_request_draft_status_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil) @@ -28,7 +28,7 @@ def change_in_merge_request_draft_status_email(recipient_id, merge_request_id, u @updated_by_user = User.find(updated_by_user_id) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end # rubocop: disable CodeReuse/ActiveRecord @@ -38,7 +38,7 @@ def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assi @previous_assignees = [] @previous_assignees = User.where(id: previous_assignee_ids) if previous_assignee_ids.any? - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end # rubocop: enable CodeReuse/ActiveRecord @@ -49,7 +49,7 @@ def changed_reviewer_of_merge_request_email(recipient_id, merge_request_id, prev @previous_reviewers = [] @previous_reviewers = User.where(id: previous_reviewer_ids) if previous_reviewer_ids.any? - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end # rubocop: enable CodeReuse/ActiveRecord @@ -58,13 +58,13 @@ def relabeled_merge_request_email(recipient_id, merge_request_id, label_names, u @label_names = label_names @labels_url = project_labels_url(@project) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end def removed_milestone_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end def changed_milestone_merge_request_email(recipient_id, merge_request_id, milestone, updated_by_user_id, reason = nil) @@ -72,7 +72,7 @@ def changed_milestone_merge_request_email(recipient_id, merge_request_id, milest @milestone = milestone @milestone_url = milestone_url(@milestone) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason).merge({ + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason).merge({ template_name: 'changed_milestone_email' })) end @@ -81,27 +81,27 @@ def closed_merge_request_email(recipient_id, merge_request_id, updated_by_user_i setup_merge_request_mail(merge_request_id, recipient_id) @updated_by = User.find(updated_by_user_id) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end def merged_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason: nil, closed_via: nil) setup_merge_request_mail(merge_request_id, recipient_id) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end def request_review_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id) @updated_by = User.find(updated_by_user_id) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end def attention_requested_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id) @updated_by = User.find(updated_by_user_id) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end def merge_request_status_email(recipient_id, merge_request_id, status, updated_by_user_id, reason = nil) @@ -109,27 +109,27 @@ def merge_request_status_email(recipient_id, merge_request_id, status, updated_b @mr_status = status @updated_by = User.find(updated_by_user_id) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end def merge_request_unmergeable_email(recipient_id, merge_request_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id) - mail_answer_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, reason)) end def resolved_all_discussions_email(recipient_id, merge_request_id, resolved_by_user_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id) @resolved_by = User.find(resolved_by_user_id) - mail_answer_thread(@merge_request, merge_request_thread_options(resolved_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(resolved_by_user_id, reason)) end def merge_when_pipeline_succeeds_email(recipient_id, merge_request_id, mwps_set_by_user_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id) @mwps_set_by = ::User.find(mwps_set_by_user_id) - mail_answer_thread(@merge_request, merge_request_thread_options(mwps_set_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(mwps_set_by_user_id, reason)) end def merge_requests_csv_email(user, project, csv_data, export_status) @@ -154,19 +154,19 @@ def setup_merge_request_mail(merge_request_id, recipient_id, present: false) @merge_request = MergeRequest.find(merge_request_id) @project = @merge_request.project @target_url = project_merge_request_url(@project, @merge_request) + @recipient = User.find(recipient_id) if present - recipient = User.find(recipient_id) - @mr_presenter = @merge_request.present(current_user: recipient) + @mr_presenter = @merge_request.present(current_user: @recipient) end @sent_notification = SentNotification.record(@merge_request, recipient_id, reply_key) end - def merge_request_thread_options(sender_id, recipient_id, reason = nil) + def merge_request_thread_options(sender_id, reason = nil) { from: sender(sender_id), - to: User.find(recipient_id).notification_email_for(@project.group), + to: @recipient.notification_email_for(@project.group), subject: subject("#{@merge_request.title} (#{@merge_request.to_reference})"), 'X-GitLab-NotificationReason' => reason } diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb index 587c147928632..1e254a32885f7 100644 --- a/app/mailers/emails/notes.rb +++ b/app/mailers/emails/notes.rb @@ -7,7 +7,7 @@ def note_commit_email(recipient_id, note_id, reason = nil) @commit = @note.noteable @target_url = project_commit_url(*note_target_url_options) - mail_answer_note_thread(@commit, @note, note_thread_options(recipient_id, reason)) + mail_answer_note_thread(@commit, @note, note_thread_options(reason)) end def note_issue_email(recipient_id, note_id, reason = nil) @@ -15,7 +15,7 @@ def note_issue_email(recipient_id, note_id, reason = nil) @issue = @note.noteable @target_url = project_issue_url(*note_target_url_options) - mail_answer_note_thread(@issue, @note, note_thread_options(recipient_id, reason)) + mail_answer_note_thread(@issue, @note, note_thread_options(reason)) end def note_merge_request_email(recipient_id, note_id, reason = nil) @@ -23,7 +23,7 @@ def note_merge_request_email(recipient_id, note_id, reason = nil) @merge_request = @note.noteable @target_url = project_merge_request_url(*note_target_url_options) - mail_answer_note_thread(@merge_request, @note, note_thread_options(recipient_id, reason)) + mail_answer_note_thread(@merge_request, @note, note_thread_options(reason)) end def note_snippet_email(recipient_id, note_id, reason = nil) @@ -37,7 +37,7 @@ def note_snippet_email(recipient_id, note_id, reason = nil) @target_url = gitlab_snippet_url(@note.noteable) end - mail_answer_note_thread(@snippet, @note, note_thread_options(recipient_id, reason)) + mail_answer_note_thread(@snippet, @note, note_thread_options(reason)) end def note_design_email(recipient_id, note_id, reason = nil) @@ -49,7 +49,7 @@ def note_design_email(recipient_id, note_id, reason = nil) design.issue, note_target_url_query_params.merge(vueroute: design.filename) ) - mail_answer_note_thread(design, @note, note_thread_options(recipient_id, reason)) + mail_answer_note_thread(design, @note, note_thread_options(reason)) end private @@ -62,10 +62,10 @@ def note_target_url_query_params { anchor: "note_#{@note.id}" } end - def note_thread_options(recipient_id, reason) + def note_thread_options(reason) { from: sender(@note.author_id), - to: User.find(recipient_id).notification_email_for(@project&.group || @group), + to: @recipient.notification_email_for(@project&.group || @group), subject: subject("#{@note.noteable.title} (#{@note.noteable.reference_link_text})"), 'X-GitLab-NotificationReason' => reason } @@ -76,6 +76,7 @@ def setup_note_mail(note_id, recipient_id) @note = note_id.is_a?(Note) ? note_id : Note.find(note_id) @project = @note.project @group = @note.noteable.try(:group) + @recipient = User.find(recipient_id) if (@project || @group) && @note.persisted? @sent_notification = SentNotification.record_note(@note, recipient_id, reply_key) diff --git a/app/mailers/emails/releases.rb b/app/mailers/emails/releases.rb index c9c77ab933364..4875abafe8d24 100644 --- a/app/mailers/emails/releases.rb +++ b/app/mailers/emails/releases.rb @@ -9,11 +9,10 @@ def new_release_email(user_id, release, reason = nil) namespace_id: @project.namespace, project_id: @project ) - - user = User.find(user_id) + @recipient = User.find(user_id) mail( - to: user.notification_email_for(@project.group), + to: @recipient.notification_email_for(@project.group), subject: subject(release_email_subject) ) end diff --git a/app/views/notify/_note_email.html.haml b/app/views/notify/_note_email.html.haml index 2cef6f97d4801..ae9c8554e7302 100644 --- a/app/views/notify/_note_email.html.haml +++ b/app/views/notify/_note_email.html.haml @@ -34,4 +34,4 @@ email: true } %div{ style: note_style } - = markdown(note.note, pipeline: :email, author: note.author) + = markdown(note.note, pipeline: :email, author: note.author, current_user: @recipient, issuable_reference_expansion_enabled: true) diff --git a/app/views/notify/issue_due_email.html.haml b/app/views/notify/issue_due_email.html.haml index adb9da0569435..c9cd9c32b5474 100644 --- a/app/views/notify/issue_due_email.html.haml +++ b/app/views/notify/issue_due_email.html.haml @@ -9,4 +9,4 @@ - if @issue.description %div - = markdown(@issue.description, pipeline: :email, author: @issue.author) + = markdown(@issue.description, pipeline: :email, author: @issue.author, current_user: @recipient, issuable_reference_expansion_enabled: true) diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml index 3219ee3473655..439604a950a7a 100644 --- a/app/views/notify/new_issue_email.html.haml +++ b/app/views/notify/new_issue_email.html.haml @@ -8,4 +8,4 @@ - if @issue.description %div - = markdown(@issue.description, pipeline: :email, author: @issue.author) + = markdown(@issue.description, pipeline: :email, author: @issue.author, current_user: @recipient, issuable_reference_expansion_enabled: true) diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index c8a0a6591a6e4..54fb6573c26fc 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -16,4 +16,4 @@ - if @merge_request.description %div - = markdown(@merge_request.description, pipeline: :email, author: @merge_request.author) + = markdown(@merge_request.description, pipeline: :email, author: @merge_request.author, current_user: @recipient, issuable_reference_expansion_enabled: true) diff --git a/app/views/notify/new_release_email.html.haml b/app/views/notify/new_release_email.html.haml index 9cef4cd85cd40..1cd3a2340c6d1 100644 --- a/app/views/notify/new_release_email.html.haml +++ b/app/views/notify/new_release_email.html.haml @@ -15,4 +15,4 @@ %p %h4= _("Release notes:") - = markdown(@release.description, pipeline: :email, author: @release.author) + = markdown(@release.description, pipeline: :email, author: @release.author, current_user: @recipient) diff --git a/app/views/notify/service_desk_new_note_email.html.haml b/app/views/notify/service_desk_new_note_email.html.haml index 824b4ab712e6e..186bdf133e3d4 100644 --- a/app/views/notify/service_desk_new_note_email.html.haml +++ b/app/views/notify/service_desk_new_note_email.html.haml @@ -2,4 +2,4 @@ %div = _("%{author_link} wrote:").html_safe % { author_link: link_to(@note.author_name, user_url(@note.author)) } %div - = markdown(@note.note, pipeline: :email, author: @note.author) + = markdown(@note.note, pipeline: :email, author: @note.author, issuable_reference_expansion_enabled: true) diff --git a/ee/app/mailers/ee/emails/issues.rb b/ee/app/mailers/ee/emails/issues.rb index c028480d36ba6..af6229e9a49b1 100644 --- a/ee/app/mailers/ee/emails/issues.rb +++ b/ee/app/mailers/ee/emails/issues.rb @@ -6,7 +6,7 @@ module Issues def removed_iteration_issue_email(recipient_id, issue_id, updated_by_user_id, reason = nil) setup_issue_mail(issue_id, recipient_id) - mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, reason)) end def changed_iteration_issue_email(recipient_id, issue_id, iteration, updated_by_user_id, reason = nil) @@ -14,7 +14,7 @@ def changed_iteration_issue_email(recipient_id, issue_id, iteration, updated_by_ @iteration = iteration @iteration_url = iteration_url(@iteration) - mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason).merge({ + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, reason).merge({ template_name: 'changed_iteration_email' })) end diff --git a/ee/app/mailers/ee/emails/merge_requests.rb b/ee/app/mailers/ee/emails/merge_requests.rb index cf76efd7f714c..9ecfa21759b6c 100644 --- a/ee/app/mailers/ee/emails/merge_requests.rb +++ b/ee/app/mailers/ee/emails/merge_requests.rb @@ -7,21 +7,21 @@ def add_merge_request_approver_email(recipient_id, merge_request_id, updated_by_ setup_merge_request_mail(merge_request_id, recipient_id, present: true) @updated_by = ::User.find(updated_by_user_id) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, reason)) end def approved_merge_request_email(recipient_id, merge_request_id, approved_by_user_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id) @approved_by = ::User.find(approved_by_user_id) - mail_answer_thread(@merge_request, merge_request_thread_options(approved_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(approved_by_user_id, reason)) end def unapproved_merge_request_email(recipient_id, merge_request_id, unapproved_by_user_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id) @unapproved_by = ::User.find(unapproved_by_user_id) - mail_answer_thread(@merge_request, merge_request_thread_options(unapproved_by_user_id, recipient_id, reason)) + mail_answer_thread(@merge_request, merge_request_thread_options(unapproved_by_user_id, reason)) end end end diff --git a/ee/app/mailers/ee/emails/notes.rb b/ee/app/mailers/ee/emails/notes.rb index 64e938e100421..5f38fb6c21982 100644 --- a/ee/app/mailers/ee/emails/notes.rb +++ b/ee/app/mailers/ee/emails/notes.rb @@ -9,7 +9,7 @@ def note_epic_email(recipient_id, note_id, reason = nil) @epic = @note.noteable @target_url = group_epic_url(*note_target_url_options) - mail_answer_note_thread(@epic, @note, note_thread_options(recipient_id, reason)) + mail_answer_note_thread(@epic, @note, note_thread_options(reason)) end end end diff --git a/ee/app/mailers/emails/epics.rb b/ee/app/mailers/emails/epics.rb index fd4ed3cf11d7b..292551ed4f92e 100644 --- a/ee/app/mailers/emails/epics.rb +++ b/ee/app/mailers/emails/epics.rb @@ -8,7 +8,7 @@ def new_epic_email(recipient_id, epic_id, reason = nil) setup_epic_mail(recipient_id) - mail_new_thread(@epic, epic_thread_options(@epic.author_id, recipient_id, reason)) + mail_new_thread(@epic, epic_thread_options(@epic.author_id, reason)) end def epic_status_changed_email(recipient_id, epic_id, status, updated_by_user_id, reason = nil) @@ -20,7 +20,7 @@ def epic_status_changed_email(recipient_id, epic_id, status, updated_by_user_id, @status = status @updated_by = User.find(updated_by_user_id) - mail_answer_thread(@epic, epic_thread_options(updated_by_user_id, recipient_id, reason)) + mail_answer_thread(@epic, epic_thread_options(updated_by_user_id, reason)) end private @@ -28,16 +28,17 @@ def epic_status_changed_email(recipient_id, epic_id, status, updated_by_user_id, def setup_epic_mail(recipient_id) @group = @epic.group @target_url = group_epic_url(@epic.group, @epic) + @recipient = User.find(recipient_id) add_group_headers @sent_notification = SentNotification.record(@epic, recipient_id, reply_key) end - def epic_thread_options(sender_id, recipient_id, reason) + def epic_thread_options(sender_id, reason) { from: sender(sender_id), - to: User.find(recipient_id).notification_email_for(@epic.group), + to: @recipient.notification_email_for(@epic.group), subject: subject("#{@epic.title} (#{@epic.to_reference})"), 'X-GitLab-NotificationReason' => reason } diff --git a/ee/app/views/notify/add_merge_request_approver_email.html.haml b/ee/app/views/notify/add_merge_request_approver_email.html.haml index ae761447ea727..3ac6633b4e423 100644 --- a/ee/app/views/notify/add_merge_request_approver_email.html.haml +++ b/ee/app/views/notify/add_merge_request_approver_email.html.haml @@ -15,4 +15,4 @@ - if @merge_request.description %div - = markdown(@merge_request.description, pipeline: :email, author: @merge_request.author) + = markdown(@merge_request.description, pipeline: :email, author: @merge_request.author, current_user: @recipient, issuable_reference_expansion_enabled: true) diff --git a/ee/app/views/notify/new_epic_email.html.haml b/ee/app/views/notify/new_epic_email.html.haml index c037fd6d5dae0..718bd5233fd49 100644 --- a/ee/app/views/notify/new_epic_email.html.haml +++ b/ee/app/views/notify/new_epic_email.html.haml @@ -8,4 +8,4 @@ - if @epic.description %div - = markdown(@epic.description, pipeline: :email, author: @epic.author) + = markdown(@epic.description, pipeline: :email, author: @epic.author, current_user: @recipient, issuable_reference_expansion_enabled: true) diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index a5e3350ec2ec4..098ac21eb18bd 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -613,6 +613,28 @@ def id it 'has References header including the notes and issue of the discussion' do expect(subject.header['References'].message_ids).to include("issue_#{note.noteable.id}@#{host}") end + + context 'with private references accessible to the recipient' do + let_it_be(:private_project) { create(:project, :private) } + let_it_be(:private_issue) { create(:issue, :closed, project: private_project) } + + before_all do + private_project.add_guest(recipient) + + note.update!(note: "#{private_issue.to_reference(full: true)}") + end + + let(:html_part) { subject.body.parts.last.to_s } + + it 'does not redact the reference' do + expect(html_part).to include("data-reference-type=\"issue\"") + expect(html_part).to include("title=\"#{private_issue.title}\"") + end + + it 'renders expanded issue references' do + expect(html_part).to include("#{private_issue.to_reference(full: true)} (closed)") + end + end end end -- GitLab