Merge branch 'show-correct-link-in-mr-pipelines' into 'master'

Show a merge request link in pipeline failure emails

Closes #32325

See merge request gitlab-org/gitlab!17513

app/helpers/gitlab_routing_helper.rb

@@ -58,7 +58,7 @@ module GitlabRoutingHelper
   end
 
   def commits_url(entity, *args)
-    project_commits_url(entity.project, entity.ref, *args)
+    project_commits_url(entity.project, entity.source_ref, *args)
   end
 
   def commit_url(entity, *args)

app/mailers/emails/pipelines.rb

@@ -15,7 +15,7 @@ module Emails
     def pipeline_mail(pipeline, recipients, status)
       @project = pipeline.project
       @pipeline = pipeline
-      @merge_request = pipeline.merge_requests_as_head_pipeline.first
+      @merge_request = pipeline.all_merge_requests.first
       add_headers
 
       # We use bcc here because we don't want to generate this emails for a
@@ -44,7 +44,7 @@ module Emails
       commit = [@pipeline.short_sha]
       commit << "in #{@merge_request.to_reference}" if @merge_request
 
-      subject("Pipeline ##{@pipeline.id} has #{status} for #{@pipeline.ref}", commit.join(' '))
+      subject("Pipeline ##{@pipeline.id} has #{status} for #{@pipeline.source_ref}", commit.join(' '))
     end
   end
 end

app/views/notify/pipeline_failed_email.html.haml

@@ -34,7 +34,7 @@
                     %img{ height: "13", src: image_url('mailers/ci_pipeline_notif_v1/icon-branch-gray.gif'), style: "display:block;", width: "13", alt: "" }/
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
                     %a.muted{ href: commits_url(@pipeline), style: "color:#333333;text-decoration:none;" }
-                      = @pipeline.ref
+                      = @pipeline.source_ref
         %tr
           %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Commit
           %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:400;padding:14px 0;margin:0;color:#333333;width:75%;padding-left:5px;border-top:1px solid #ededed;" }

app/views/notify/pipeline_failed_email.text.erb

 Your pipeline has failed.
 
 Project: <%= @project.name %> ( <%= project_url(@project) %> )
-Branch: <%= @pipeline.ref %> ( <%= commits_url(@pipeline) %> )
+Branch: <%= @pipeline.source_ref %> ( <%= commits_url(@pipeline) %> )
 <% if @merge_request -%>
 Merge Request: <%= @merge_request.to_reference %> ( <%= merge_request_url(@merge_request) %> )
 <% end -%>

app/views/notify/pipeline_success_email.html.haml

@@ -34,7 +34,7 @@
                     %img{ height: "13", src: image_url('mailers/ci_pipeline_notif_v1/icon-branch-gray.gif'), style: "display:block;", width: "13", alt: "" }/
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
                     %a.muted{ href: commits_url(@pipeline), style: "color:#333333;text-decoration:none;" }
-                      = @pipeline.ref
+                      = @pipeline.source_ref
         %tr
           %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Commit
           %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:400;padding:14px 0;margin:0;color:#333333;width:75%;padding-left:5px;border-top:1px solid #ededed;" }

app/views/notify/pipeline_success_email.text.erb

 Your pipeline has passed.
 
 Project: <%= @project.name %> ( <%= project_url(@project) %> )
-Branch: <%= @pipeline.ref %> ( <%= commits_url(@pipeline) %> )
+Branch: <%= @pipeline.source_ref %> ( <%= commits_url(@pipeline) %> )
 <% if @merge_request -%>
 Merge Request: <%= @merge_request.to_reference %> ( <%= merge_request_url(@merge_request) %> )
 <% end -%>

changelogs/unreleased/show-correct-link-in-mr-pipelines.yml

+---
+title: Show the original branch name and link of merge request in pipeline emails
+merge_request: 17513
+author:
+type: fixed

spec/mailers/emails/pipelines_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'email_spec'
+
+describe Emails::Pipelines do
+  include EmailSpec::Matchers
+
+  set(:project) { create(:project, :repository) }
+
+  shared_examples_for 'correct pipeline information' do
+    it 'has a correct information' do
+      expect(subject)
+          .to have_subject "#{project.name} | Pipeline ##{pipeline.id} has " \
+                           "#{status} for #{pipeline.source_ref} | " \
+                           "#{pipeline.short_sha}".to_s
+
+      expect(subject).to have_body_text pipeline.source_ref
+      expect(subject).to have_body_text status_text
+    end
+
+    context 'when pipeline for merge requests' do
+      let(:pipeline) { merge_request.all_pipelines.first }
+
+      let(:merge_request) do
+        create(:merge_request, :with_detached_merge_request_pipeline,
+          source_project: project,
+          target_project: project)
+      end
+
+      it 'has a correct information with merge request link' do
+        expect(subject)
+          .to have_subject "#{project.name} | Pipeline ##{pipeline.id} has " \
+                           "#{status} for #{pipeline.source_ref} | " \
+                           "#{pipeline.short_sha} in !#{merge_request.iid}".to_s
+
+        expect(subject).to have_body_text merge_request.to_reference
+        expect(subject).to have_body_text pipeline.source_ref
+        expect(subject).not_to have_body_text pipeline.ref
+      end
+    end
+  end
+
+  describe '#pipeline_success_email' do
+    subject { Notify.pipeline_success_email(pipeline, pipeline.user.try(:email)) }
+
+    let(:pipeline) { create(:ci_pipeline, project: project, ref: ref, sha: sha) }
+    let(:ref) { 'master' }
+    let(:sha) { project.commit(ref).sha }
+
+    it_behaves_like 'correct pipeline information' do
+      let(:status) { 'succeeded' }
+      let(:status_text) { 'Your pipeline has passed.' }
+    end
+  end
+
+  describe '#pipeline_failed_email' do
+    subject { Notify.pipeline_failed_email(pipeline, pipeline.user.try(:email)) }
+
+    let(:pipeline) { create(:ci_pipeline, project: project, ref: ref, sha: sha) }
+    let(:ref) { 'master' }
+    let(:sha) { project.commit(ref).sha }
+
+    it_behaves_like 'correct pipeline information' do
+      let(:status) { 'failed' }
+      let(:status_text) { 'Your pipeline has failed.' }
+    end
+  end
+end

spec/support/shared_examples/ci/pipeline_email_examples.rb

+# frozen_string_literal: true
+
+shared_examples_for 'correct pipeline information for pipelines for merge requests' do
+  context 'when pipeline for merge request' do
+    let(:pipeline) { merge_request.all_pipelines.first }
+
+    let(:merge_request) do
+      create(:merge_request, :with_detached_merge_request_pipeline,
+        source_project: project,
+        target_project: project)
+    end
+
+    it 'renders a source ref of the pipeline' do
+      render
+
+      expect(rendered).to have_content pipeline.source_ref
+      expect(rendered).not_to have_content pipeline.ref
+    end
+  end
+end

spec/views/notify/pipeline_failed_email.html.haml_spec.rb

@@ -33,6 +33,8 @@ describe 'notify/pipeline_failed_email.html.haml' do
       expect(rendered).to have_content "##{pipeline.id}"
       expect(rendered).to have_content pipeline.user.name
     end
+
+    it_behaves_like 'correct pipeline information for pipelines for merge requests'
   end
 
   context 'pipeline without user' do

spec/views/notify/pipeline_failed_email.text.erb_spec.rb

@@ -36,4 +36,6 @@ describe 'notify/pipeline_failed_email.text.erb' do
     expect(rendered).to have_content(pipeline.user.name)
     expect(rendered).to have_content("/-/jobs/#{job.id}/raw")
   end
+
+  it_behaves_like 'correct pipeline information for pipelines for merge requests'
 end

spec/views/notify/pipeline_success_email.html.haml_spec.rb

@@ -33,6 +33,8 @@ describe 'notify/pipeline_success_email.html.haml' do
       expect(rendered).to have_content "##{pipeline.id}"
       expect(rendered).to have_content pipeline.user.name
     end
+
+    it_behaves_like 'correct pipeline information for pipelines for merge requests'
   end
 
   context 'pipeline without user' do

spec/views/notify/pipeline_success_email.text.erb_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'notify/pipeline_success_email.text.erb' do
+  let(:user) { create(:user, developer_projects: [project]) }
+  let(:project) { create(:project, :repository) }
+  let(:merge_request) { create(:merge_request, :simple, source_project: project) }
+
+  let(:pipeline) do
+    create(:ci_pipeline,
+           :success,
+           project: project,
+           user: user,
+           ref: project.default_branch,
+           sha: project.commit.sha)
+  end
+
+  before do
+    assign(:project, project)
+    assign(:pipeline, pipeline)
+    assign(:merge_request, merge_request)
+  end
+
+  it_behaves_like 'correct pipeline information for pipelines for merge requests'
+end