Merge branch 'tc-fix-pipeline-recipient' into 'master'

Clearly show who triggered the pipeline in pipeline successful/failed email

Closes #28755

See merge request !10283

app/views/notify/pipeline_failed_email.text.erb

@@ -14,9 +14,21 @@ Commit Author: <%= commit.author.name %> ( <%= user_url(commit.author) %> )
 <% else -%>
 Commit Author: <%= commit.author_name %>
 <% end -%>
+<% if commit.different_committer? -%>
+<% if commit.committer -%>
+Committed by: <%= commit.committer.name %> ( <%= user_url(commit.committer) %> )
+<% else -%>
+Committed by: <%= commit.committer_name %>
+<% end -%>
+<% end -%>
 
+<% if @pipeline.user -%>
+Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by <%= @pipeline.user.name %> ( <%= user_url(@pipeline.user) %> )
+<% else -%>
+Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by API
+<% end -%>
 <% failed = @pipeline.statuses.latest.failed -%>
-Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) had <%= failed.size %> failed <%= 'build'.pluralize(failed.size) %>.
+had <%= failed.size %> failed <%= 'build'.pluralize(failed.size) %>.
 
 <% failed.each do |build| -%>
 <%= render "notify/links/#{build.to_partial_path}", pipeline: @pipeline, build: build %>

app/views/notify/pipeline_success_email.html.haml

@@ -16,7 +16,7 @@
       %tbody
         %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;" } Project
-          %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;color:#333333;font-weight:400;width:75%;padding-left:5px;" }
+          %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:500;padding:14px 0;margin:0;color:#333333;width:75%;padding-left:5px;" }
             - namespace_name = @project.group ? @project.group.name : @project.namespace.owner.name
             - namespace_url = @project.group ? group_url(@project.group) : user_url(@project.namespace.owner)
             %a.muted{ href: namespace_url, style: "color:#333333;text-decoration:none;" }
@@ -26,7 +26,7 @@
               = @project.name
         %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;" } Branch
-          %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;color:#333333;font-weight:400;width:75%;padding-left:5px;border-top:1px solid #ededed;" }
+          %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:500;padding:14px 0;margin:0;color:#333333;width:75%;padding-left:5px;border-top:1px solid #ededed;" }
             %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" }
               %tbody
                 %tr
@@ -37,7 +37,7 @@
                       = @pipeline.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:300;padding:14px 0;margin:0;color:#333333;font-weight:400;width:75%;padding-left:5px;border-top:1px solid #ededed;" }
+          %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;" }
             %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" }
               %tbody
                 %tr
@@ -52,13 +52,13 @@
                         = @merge_request.to_reference
             .commit{ style: "color:#5c5c5c;font-weight:300;" }
               = @pipeline.git_commit_message.truncate(50)
+        - commit = @pipeline.commit
         %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;" } Author
-          %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;color:#333333;font-weight:400;width:75%;padding-left:5px;border-top:1px solid #ededed;" }
+          %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 Author
+          %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:500;padding:14px 0;margin:0;color:#333333;width:75%;padding-left:5px;border-top:1px solid #ededed;" }
             %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" }
               %tbody
                 %tr
-                  - commit = @pipeline.commit
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
                     %img.avatar{ height: "24", src: avatar_icon(commit.author || commit.author_email, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
                   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
@@ -68,17 +68,50 @@
                     - else
                       %span
                         = commit.author_name
+        - if commit.different_committer?
+          %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;" } Committed by
+            %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:500;padding:14px 0;margin:0;color:#333333;width:75%;padding-left:5px;border-top:1px solid #ededed;" }
+              %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" }
+                %tbody
+                  %tr
+                    %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
+                      %img.avatar{ height: "24", src: avatar_icon(commit.committer || commit.committer_email, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
+                    %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
+                      - if commit.committer
+                        %a.muted{ href: user_url(commit.committer), style: "color:#333333;text-decoration:none;" }
+                          = commit.committer.name
+                      - else
+                        %span
+                          = commit.committer_name
+
 %tr.spacer
   %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;height:18px;font-size:18px;line-height:18px;" }
      
 %tr.success-message
-  %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;color:#333333;font-size:15px;font-weight:400;line-height:1.4;padding:15px 5px;text-align:center;" }
-    - build_count = @pipeline.statuses.latest.size
+  %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;color:#333333;font-size:15px;font-weight:400;line-height:1.4;padding:15px 5px 0 5px;text-align:center;" }
+    %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;margin:0 auto;" }
+      %tbody
+        %tr
+          %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;font-weight:500;line-height:1.4;vertical-align:baseline;" }
+            Pipeline
+            %a{ href: pipeline_url(@pipeline), style: "color:#3777b0;text-decoration:none;" }
+              = "\##{@pipeline.id}"
+            triggered by
+          - if @pipeline.user
+            %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;padding-left:5px", width: "24" }
+              %img.avatar{ height: "24", src: avatar_icon(@pipeline.user, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
+            %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;font-weight:500;line-height:1.4;vertical-align:baseline;" }
+              %a.muted{ href: user_url(@pipeline.user), style: "color:#333333;text-decoration:none;" }
+                = @pipeline.user.name
+          - else
+            %td{ style: "font-family:'Menlo','Liberation Mono','Consolas','DejaVu Sans Mono','Ubuntu Mono','Courier New','andale mono','lucida console',monospace;font-size:14px;line-height:1.4;vertical-align:baseline;padding:0 5px;" }
+              API
+%tr
+  %td{ colspan: 2, style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;color:#333333;font-size:15px;font-weight:300;line-height:1.4;padding:15px 5px;text-align:center;" }
+    - job_count = @pipeline.statuses.latest.size
     - stage_count = @pipeline.stages_count
-    Pipeline
-    %a{ href: pipeline_url(@pipeline), style: "color:#3777b0;text-decoration:none;" }
-      = "\##{@pipeline.id}"
     successfully completed
-    #{build_count} #{'build'.pluralize(build_count)}
+    #{job_count} #{'job'.pluralize(job_count)}
     in
     #{stage_count} #{'stage'.pluralize(stage_count)}.

app/views/notify/pipeline_success_email.text.erb

@@ -14,7 +14,19 @@ Commit Author: <%= commit.author.name %> ( <%= user_url(commit.author) %> )
 <% else -%>
 Commit Author: <%= commit.author_name %>
 <% end -%>
+<% if commit.different_committer? -%>
+<% if commit.committer -%>
+Committed by: <%= commit.committer.name %> ( <%= user_url(commit.committer) %> )
+<% else -%>
+Committed by: <%= commit.committer_name %>
+<% end -%>
+<% end -%>
 
 <% build_count = @pipeline.statuses.latest.size -%>
 <% stage_count = @pipeline.stages_count -%>
-Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) successfully completed <%= build_count %> <%= 'build'.pluralize(build_count) %> in <%= stage_count %> <%= 'stage'.pluralize(stage_count) %>.
+<% if @pipeline.user -%>
+Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by <%= @pipeline.user.name %> ( <%= user_url(@pipeline.user) %> )
+<% else -%>
+Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by API
+<% end -%>
+successfully completed <%= build_count %> <%= 'build'.pluralize(build_count) %> in <%= stage_count %> <%= 'stage'.pluralize(stage_count) %>.

changelogs/unreleased/tc-fix-pipeline-recipient.yml

+---
+title: Clearly show who triggered the pipeline in email
+merge_request: 10283
+author:

spec/mailers/previews/notify_preview.rb

+class NotifyPreview < ActionMailer::Preview
+  def pipeline_success_email
+    pipeline = Ci::Pipeline.last
+    Notify.pipeline_success_email(pipeline, pipeline.user.try(:email))
+  end
+
+  def pipeline_failed_email
+    pipeline = Ci::Pipeline.last
+    Notify.pipeline_failed_email(pipeline, pipeline.user.try(:email))
+  end
+end

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

+require 'spec_helper'
+
+describe 'notify/pipeline_failed_email.html.haml' do
+  include Devise::Test::ControllerHelpers
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:merge_request) { create(:merge_request, :simple, source_project: project) }
+
+  let(:pipeline) do
+    create(:ci_pipeline,
+           project: project,
+           user: user,
+           ref: project.default_branch,
+           sha: project.commit.sha,
+           status: :success)
+  end
+
+  before do
+    assign(:project, project)
+    assign(:pipeline, pipeline)
+    assign(:merge_request, merge_request)
+  end
+
+  context 'pipeline with user' do
+    it 'renders the email correctly' do
+      render
+
+      expect(rendered).to have_content "Your pipeline has failed"
+      expect(rendered).to have_content pipeline.project.name
+      expect(rendered).to have_content pipeline.git_commit_message.truncate(50)
+      expect(rendered).to have_content pipeline.commit.author_name
+      expect(rendered).to have_content "##{pipeline.id}"
+      expect(rendered).to have_content pipeline.user.name
+    end
+  end
+
+  context 'pipeline without user' do
+    before do
+      pipeline.update_attribute(:user, nil)
+    end
+
+    it 'renders the email correctly' do
+      render
+
+      expect(rendered).to have_content "Your pipeline has failed"
+      expect(rendered).to have_content pipeline.project.name
+      expect(rendered).to have_content pipeline.git_commit_message.truncate(50)
+      expect(rendered).to have_content pipeline.commit.author_name
+      expect(rendered).to have_content "##{pipeline.id}"
+      expect(rendered).to have_content "by API"
+    end
+  end
+end

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

+require 'spec_helper'
+
+describe 'notify/pipeline_success_email.html.haml' do
+  include Devise::Test::ControllerHelpers
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:merge_request) { create(:merge_request, :simple, source_project: project) }
+
+  let(:pipeline) do
+    create(:ci_pipeline,
+           project: project,
+           user: user,
+           ref: project.default_branch,
+           sha: project.commit.sha,
+           status: :success)
+  end
+
+  before do
+    assign(:project, project)
+    assign(:pipeline, pipeline)
+    assign(:merge_request, merge_request)
+  end
+
+  context 'pipeline with user' do
+    it 'renders the email correctly' do
+      render
+
+      expect(rendered).to have_content "Your pipeline has passed"
+      expect(rendered).to have_content pipeline.project.name
+      expect(rendered).to have_content pipeline.git_commit_message.truncate(50)
+      expect(rendered).to have_content pipeline.commit.author_name
+      expect(rendered).to have_content "##{pipeline.id}"
+      expect(rendered).to have_content pipeline.user.name
+    end
+  end
+
+  context 'pipeline without user' do
+    before do
+      pipeline.update_attribute(:user, nil)
+    end
+
+    it 'renders the email correctly' do
+      render
+
+      expect(rendered).to have_content "Your pipeline has passed"
+      expect(rendered).to have_content pipeline.project.name
+      expect(rendered).to have_content pipeline.git_commit_message.truncate(50)
+      expect(rendered).to have_content pipeline.commit.author_name
+      expect(rendered).to have_content "##{pipeline.id}"
+      expect(rendered).to have_content "by API"
+    end
+  end
+end