Merge branch...

Merge branch '223127-pipeline-success-emails-looks-too-much-like-the-pipeline-failure-message-2' into 'master'

Resolve "Pipeline success emails looks too much like the pipeline failure message"

See merge request gitlab-org/gitlab!50405

app/mailers/emails/pipelines.rb

@@ -3,15 +3,15 @@
 module Emails
   module Pipelines
     def pipeline_success_email(pipeline, recipients)
-      pipeline_mail(pipeline, recipients, 'succeeded')
+      pipeline_mail(pipeline, recipients, 'Succesful')
     end
 
     def pipeline_failed_email(pipeline, recipients)
-      pipeline_mail(pipeline, recipients, 'failed')
+      pipeline_mail(pipeline, recipients, 'Failed')
     end
 
     def pipeline_fixed_email(pipeline, recipients)
-      pipeline_mail(pipeline, recipients, 'been fixed')
+      pipeline_mail(pipeline, recipients, 'Fixed')
     end
 
     private
@@ -50,10 +50,13 @@ module Emails
     end
 
     def pipeline_subject(status)
-      commit = [@pipeline.short_sha]
-      commit << "in #{@merge_request.to_reference}" if @merge_request
+      subject = []
 
-      subject("Pipeline ##{@pipeline.id} has #{status} for #{@pipeline.source_ref}", commit.join(' '))
+      subject << "#{status} pipeline for #{@pipeline.source_ref}"
+      subject << @project.name if @project
+      subject << @pipeline.short_sha
+
+      subject.join(' | ')
     end
   end
 end

app/views/notify/pipeline_failed_email.html.haml

@@ -6,7 +6,7 @@
           %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;vertical-align:middle;color:#ffffff;text-align:center;padding-right:5px;line-height:1;" }
             %img{ alt: "✖", height: "13", src: image_url('mailers/ci_pipeline_notif_v1/icon-x-red-inverted.gif'), style: "display:block;", width: "13" }/
           %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;vertical-align:middle;color:#ffffff;text-align:center;" }
-            Your pipeline has failed.
+            Pipeline ##{@pipeline.id} has failed!
 %tr.spacer
   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;height:18px;font-size:18px;line-height:18px;" }
      

app/views/notify/pipeline_failed_email.text.erb

-Your pipeline has failed.
+Pipeline #<%= @pipeline.id %> has failed!
 
 Project: <%= @project.name %> ( <%= project_url(@project) %> )
 Branch: <%= @pipeline.source_ref %> ( <%= commits_url(@pipeline) %> )

app/views/notify/pipeline_fixed_email.html.haml

-= render 'notify/successful_pipeline', title: 'Your pipeline has been fixed!'
+= render 'notify/successful_pipeline', title: "Pipeline has been fixed and ##{@pipeline.id} has passed!"

app/views/notify/pipeline_fixed_email.text.erb

-<%= render 'notify/successful_pipeline', title: 'Your pipeline has been fixed!' -%>
+<%= render 'notify/successful_pipeline', title: "Pipeline has been fixed and ##{@pipeline.id} has passed!" -%>

app/views/notify/pipeline_success_email.html.haml

-= render 'notify/successful_pipeline', title: 'Your pipeline has passed.'
+= render 'notify/successful_pipeline', title: "Pipeline ##{@pipeline.id} has passed!"

app/views/notify/pipeline_success_email.text.erb

-<%= render 'notify/successful_pipeline', title: 'Your pipeline has passed.' -%>
+<%= render 'notify/successful_pipeline', title: "Pipeline ##{@pipeline.id} has passed!" -%>

changelogs/unreleased/pipeline-status-conciseness.yml

+---
+title: More concise pipeline notification emails.
+merge_request: 50405
+author:
+type: changed

spec/mailers/emails/pipelines_spec.rb

@@ -11,8 +11,8 @@ RSpec.describe Emails::Pipelines do
   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} | " \
+          .to have_subject "#{status} pipeline for #{pipeline.source_ref} | " \
+                           "#{project.name} | " \
                            "#{pipeline.short_sha}".to_s
 
       expect(subject).to have_body_text pipeline.source_ref
@@ -29,8 +29,8 @@ RSpec.describe Emails::Pipelines do
 
       it 'has correct information that there is no merge request link' do
         expect(subject)
-            .to have_subject "#{project.name} | Pipeline ##{pipeline.id} has " \
-                             "#{status} for #{pipeline.source_ref} | " \
+            .to have_subject "#{status} pipeline for #{pipeline.source_ref} | " \
+                             "#{project.name} | " \
                              "#{pipeline.short_sha}".to_s
 
         expect(subject).to have_body_text pipeline.source_ref
@@ -49,9 +49,9 @@ RSpec.describe Emails::Pipelines do
 
       it 'has correct information that there is a 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
+          .to have_subject "#{status} pipeline for #{pipeline.source_ref} | " \
+                           "#{project.name} | " \
+                           "#{pipeline.short_sha}".to_s
 
         expect(subject).to have_body_text merge_request.to_reference
         expect(subject).to have_body_text pipeline.source_ref
@@ -71,9 +71,9 @@ RSpec.describe Emails::Pipelines do
 
       it 'has correct information that there is a 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
+          .to have_subject "#{status} pipeline for #{pipeline.source_ref} | " \
+                           "#{project.name} | " \
+                           "#{pipeline.short_sha}".to_s
 
         expect(subject).to have_body_text merge_request.to_reference
         expect(subject).to have_body_text pipeline.source_ref
@@ -89,8 +89,8 @@ RSpec.describe Emails::Pipelines do
     let(:sha) { project.commit(ref).sha }
 
     it_behaves_like 'correct pipeline information' do
-      let(:status) { 'succeeded' }
-      let(:status_text) { 'Your pipeline has passed.' }
+      let(:status) { 'Succesful' }
+      let(:status_text) { "Pipeline ##{pipeline.id} has passed!" }
     end
   end
 
@@ -102,8 +102,8 @@ RSpec.describe Emails::Pipelines do
     let(:sha) { project.commit(ref).sha }
 
     it_behaves_like 'correct pipeline information' do
-      let(:status) { 'failed' }
-      let(:status_text) { 'Your pipeline has failed.' }
+      let(:status) { 'Failed' }
+      let(:status_text) { "Pipeline ##{pipeline.id} has failed!" }
     end
   end
 
@@ -115,8 +115,8 @@ RSpec.describe Emails::Pipelines do
     let(:sha) { project.commit(ref).sha }
 
     it_behaves_like 'correct pipeline information' do
-      let(:status) { 'been fixed' }
-      let(:status_text) { 'Your pipeline has been fixed!' }
+      let(:status) { 'Fixed' }
+      let(:status_text) { "Pipeline has been fixed and ##{pipeline.id} has passed!" }
     end
   end
 end

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

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'notify/pipeline_failed_email.html.haml' do
   it_behaves_like 'pipeline status changes email' do
-    let(:title) { 'Your pipeline has failed' }
+    let(:title) { "Pipeline ##{pipeline.id} has failed!" }
     let(:status) { :failed }
   end
 end

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

@@ -27,7 +27,7 @@ RSpec.describe 'notify/pipeline_failed_email.text.erb' do
     it 'renders the email correctly' do
       render
 
-      expect(rendered).to have_content('Your pipeline has failed')
+      expect(rendered).to have_content("Pipeline ##{pipeline.id} has failed!")
       expect(rendered).to have_content(pipeline.project.name)
       expect(rendered).to have_content(pipeline.git_commit_message.truncate(50).gsub(/\s+/, ' '))
       expect(rendered).to have_content(pipeline.commit.author_name)

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

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'notify/pipeline_fixed_email.html.haml' do
   it_behaves_like 'pipeline status changes email' do
-    let(:title) { 'Your pipeline has been fixed!' }
+    let(:title) { "Pipeline has been fixed and ##{pipeline.id} has passed!" }
     let(:status) { :success }
   end
 end

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

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'notify/pipeline_fixed_email.text.erb' do
   it_behaves_like 'pipeline status changes email' do
-    let(:title) { 'Your pipeline has been fixed!' }
+    let(:title) { "Pipeline has been fixed and ##{pipeline.id} has passed!" }
     let(:status) { :success }
   end
 end

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

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'notify/pipeline_success_email.html.haml' do
   it_behaves_like 'pipeline status changes email' do
-    let(:title) { 'Your pipeline has passed' }
+    let(:title) { "Pipeline ##{pipeline.id} has passed!" }
     let(:status) { :success }
   end
 end

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

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'notify/pipeline_success_email.text.erb' do
   it_behaves_like 'pipeline status changes email' do
-    let(:title) { 'Your pipeline has passed' }
+    let(:title) { "Pipeline ##{pipeline.id} has passed!" }
     let(:status) { :success }
   end
 end