Merge branch '24309-notifications-for-when-pipelines-are-fixed' into 'master'

Resolve "Notifications for when pipelines are fixed"

Closes #24309

See merge request gitlab-org/gitlab!16951

.haml-lint_todo.yml

@@ -146,6 +146,7 @@ linters:
       - 'app/views/notify/_failed_builds.html.haml'
       - 'app/views/notify/_reassigned_issuable_email.html.haml'
       - 'app/views/notify/_removal_notification.html.haml'
+      - 'app/views/notify/_successful_pipeline.html.haml'
       - 'app/views/notify/autodevops_disabled_email.html.haml'
       - 'app/views/notify/changed_milestone_email.html.haml'
       - 'app/views/notify/import_issues_csv_email.html.haml'
@@ -163,7 +164,6 @@ linters:
       - 'app/views/notify/pages_domain_verification_failed_email.html.haml'
       - 'app/views/notify/pages_domain_verification_succeeded_email.html.haml'
       - 'app/views/notify/pipeline_failed_email.html.haml'
-      - 'app/views/notify/pipeline_success_email.html.haml'
       - 'app/views/notify/project_was_exported_email.html.haml'
       - 'app/views/notify/project_was_moved_email.html.haml'
       - 'app/views/notify/project_was_not_exported_email.html.haml'

app/mailers/emails/pipelines.rb

@@ -10,6 +10,10 @@ module Emails
       pipeline_mail(pipeline, recipients, 'failed')
     end
 
+    def pipeline_fixed_email(pipeline, recipients)
+      pipeline_mail(pipeline, recipients, 'been fixed')
+    end
+
     private
 
     def pipeline_mail(pipeline, recipients, status)

app/mailers/previews/notify_preview.rb

@@ -145,6 +145,10 @@ class NotifyPreview < ActionMailer::Preview
     Notify.pipeline_failed_email(pipeline, pipeline.user.try(:email))
   end
 
+  def pipeline_fixed_email
+    Notify.pipeline_fixed_email(pipeline, pipeline.user.try(:email))
+  end
+
   def autodevops_disabled_email
     Notify.autodevops_disabled_email(pipeline, user.email).message
   end

app/models/ci/pipeline.rb

@@ -63,6 +63,14 @@ module Ci
     has_many :sourced_pipelines, class_name: 'Ci::Sources::Pipeline', foreign_key: :source_pipeline_id
 
     has_one :source_pipeline, class_name: 'Ci::Sources::Pipeline', inverse_of: :pipeline
+
+    has_one :ref_status, ->(pipeline) {
+      # We use .read_attribute to save 1 extra unneeded query to load the :project.
+      unscope(:where)
+        .where(project_id: pipeline.read_attribute(:project_id), ref: pipeline.ref, tag: pipeline.tag)
+      # Sadly :inverse_of is not supported (yet) by Rails for composite PKs.
+    }, class_name: 'Ci::Ref', inverse_of: :pipelines
+
     has_one :chat_data, class_name: 'Ci::PipelineChatData'
 
     has_many :triggered_pipelines, through: :sourced_pipelines, source: :pipeline
@@ -227,7 +235,7 @@ module Ci
 
       after_transition any => [:success, :failed] do |pipeline|
         pipeline.run_after_commit do
-          PipelineNotificationWorker.perform_async(pipeline.id)
+          PipelineUpdateCiRefStatusWorker.perform_async(pipeline.id)
         end
       end
 

app/models/ci/ref.rb

+# frozen_string_literal: true
+
+module Ci
+  class Ref < ApplicationRecord
+    extend Gitlab::Ci::Model
+
+    STATUSES = %w[success failed fixed].freeze
+
+    belongs_to :project
+    belongs_to :last_updated_by_pipeline, foreign_key: :last_updated_by_pipeline_id, class_name: 'Ci::Pipeline'
+    # ActiveRecord doesn't support composite FKs for this reason we have to do the 'unscope(:where)'
+    # hack.
+    has_many :pipelines, ->(ref) {
+      # We use .read_attribute to save 1 extra unneeded query to load the :project.
+      unscope(:where)
+        .where(ref: ref.ref, project_id: ref.read_attribute(:project_id), tag: ref.tag)
+      # Sadly :inverse_of is not supported (yet) by Rails for composite PKs.
+    }, inverse_of: :ref_status
+
+    validates :status, inclusion: { in: STATUSES }
+    validates :last_updated_by_pipeline, presence: true
+  end
+end

app/models/notification_recipient.rb

@@ -52,7 +52,8 @@ class NotificationRecipient
     when :mention
       @type == :mention
     when :participating
-      @custom_action == :failed_pipeline || %i[participating mention].include?(@type)
+      %i[failed_pipeline fixed_pipeline].include?(@custom_action) ||
+        %i[participating mention].include?(@type)
     when :custom
       custom_enabled? || %i[participating mention].include?(@type)
     when :watch
@@ -63,7 +64,13 @@ class NotificationRecipient
   end
 
   def custom_enabled?
-    @custom_action && notification_setting&.event_enabled?(@custom_action)
+    return false unless @custom_action
+    return false unless notification_setting
+
+    notification_setting.event_enabled?(@custom_action) ||
+      # fixed_pipeline is a subset of success_pipeline event
+      (@custom_action == :fixed_pipeline &&
+       notification_setting.event_enabled?(:success_pipeline))
   end
 
   def unsubscribed?

app/models/notification_setting.rb

@@ -44,6 +44,7 @@ class NotificationSetting < ApplicationRecord
     :reassign_merge_request,
     :merge_merge_request,
     :failed_pipeline,
+    :fixed_pipeline,
     :success_pipeline
   ].freeze
 
@@ -76,9 +77,9 @@ class NotificationSetting < ApplicationRecord
     setting
   end
 
-  # Allow people to receive failed pipeline notifications if they already have
-  # custom notifications enabled, as these are more like mentions than the other
-  # custom settings.
+  # Allow people to receive both failed pipeline/fixed pipeline notifications
+  # if they already have custom notifications enabled,
+  # as these are more like mentions than the other custom settings.
   def failed_pipeline
     bool = super
 
@@ -86,6 +87,13 @@ class NotificationSetting < ApplicationRecord
   end
   alias_method :failed_pipeline?, :failed_pipeline
 
+  def fixed_pipeline
+    bool = super
+
+    bool.nil? || bool
+  end
+  alias_method :fixed_pipeline?, :fixed_pipeline
+
   def event_enabled?(event)
     respond_to?(event) && !!public_send(event) # rubocop:disable GitlabSecurity/PublicSend
   end

app/models/project.rb

@@ -267,6 +267,7 @@ class Project < ApplicationRecord
           class_name: 'Ci::Pipeline',
           inverse_of: :project
   has_many :stages, class_name: 'Ci::Stage', inverse_of: :project
+  has_many :ci_refs, class_name: 'Ci::Ref'
 
   # Ci::Build objects store data on the file system such as artifact files and
   # build traces. Currently there's no efficient way of removing this data in

app/models/project_services/pipelines_email_service.rb

@@ -49,7 +49,7 @@ class PipelinesEmailService < Service
     return unless all_recipients.any?
 
     pipeline_id = data[:object_attributes][:id]
-    PipelineNotificationWorker.new.perform(pipeline_id, all_recipients)
+    PipelineNotificationWorker.new.perform(pipeline_id, recipients: all_recipients)
   end
 
   def can_test?

app/services/ci/update_ci_ref_status_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class UpdateCiRefStatusService
+    include Gitlab::OptimisticLocking
+
+    attr_reader :pipeline
+
+    def initialize(pipeline)
+      @pipeline = pipeline
+    end
+
+    def call
+      save.tap { |success| after_save if success }
+    end
+
+    private
+
+    def save
+      might_insert = ref.new_record?
+
+      begin
+        retry_optimistic_lock(ref) do
+          next false if ref.persisted? &&
+            (ref.last_updated_by_pipeline_id || 0) >= pipeline.id
+
+          ref.update(status: next_status(ref.status, pipeline.status),
+                     last_updated_by_pipeline: pipeline)
+        end
+      rescue ActiveRecord::RecordNotUnique
+        if might_insert
+          @ref = pipeline.reset.ref_status
+          might_insert = false
+          retry
+        else
+          raise
+        end
+      end
+    end
+
+    def next_status(ref_status, pipeline_status)
+      if ref_status == 'failed' && pipeline_status == 'success'
+        'fixed'
+      else
+        pipeline_status
+      end
+    end
+
+    def after_save
+      enqueue_pipeline_notification
+    end
+
+    def enqueue_pipeline_notification
+      PipelineNotificationWorker.perform_async(pipeline.id, ref_status: ref.status)
+    end
+
+    def ref
+      @ref ||= pipeline.ref_status || build_ref
+    end
+
+    def build_ref
+      Ci::Ref.new(ref: pipeline.ref, project: pipeline.project, tag: pipeline.tag)
+    end
+  end
+end

app/services/notification_service.rb

@@ -434,18 +434,19 @@ class NotificationService
     mailer.project_was_not_exported_email(current_user, project, errors).deliver_later
   end
 
-  def pipeline_finished(pipeline, recipients = nil)
+  def pipeline_finished(pipeline, ref_status: nil, recipients: nil)
     # Must always check project configuration since recipients could be a list of emails
     # from the PipelinesEmailService integration.
     return if pipeline.project.emails_disabled?
 
-    email_template = "pipeline_#{pipeline.status}_email"
+    ref_status ||= pipeline.status
+    email_template = "pipeline_#{ref_status}_email"
 
     return unless mailer.respond_to?(email_template)
 
     recipients ||= notifiable_users(
       [pipeline.user], :watch,
-      custom_action: :"#{pipeline.status}_pipeline",
+      custom_action: :"#{ref_status}_pipeline",
       target: pipeline
     ).map do |user|
       user.notification_email_for(pipeline.project.group)

app/views/notify/_successful_pipeline.html.haml

+- title = local_assigns[:title]
+%tr.table-success
+  %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:10px;border-radius:3px;font-size:14px;line-height:1.3;text-align:center;overflow:hidden;color:#ffffff;background-color:#31af64;" }
+    %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;vertical-align:middle;color:#ffffff;text-align:center;padding-right:5px;" }
+            %img{ alt: "✓", height: "13", src: image_url('mailers/ci_pipeline_notif_v1/icon-check-green-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;" }
+            = title
+%tr.spacer
+  %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;height:18px;font-size:18px;line-height:18px;" }
+     
+%tr.section
+  %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:0 15px;border:1px solid #ededed;border-radius:3px;overflow:hidden;" }
+    %table.table-info{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:100%;" }
+      %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: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;" }
+              = namespace_name
+            \/
+            %a.muted{ href: project_url(@project), style: "color:#333333;text-decoration:none;" }
+              = @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: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{ 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.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;" }
+            %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{ height: "13", src: image_url('mailers/ci_pipeline_notif_v1/icon-commit-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{ href: commit_url(@pipeline), style: "color:#3777b0;text-decoration:none;" }
+                      = @pipeline.short_sha
+                    - if @merge_request
+                      in
+                      %a{ href: merge_request_url(@merge_request), style: "color:#3777b0;text-decoration:none;" }
+                        = @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;" } 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
+                  %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_for(commit.author, commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                  %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
+                    - if commit.author
+                      %a.muted{ href: user_url(commit.author), style: "color:#333333;text-decoration:none;" }
+                        = commit.author.name
+                    - 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_for(commit.committer, commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+                    %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 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_for_user(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/
+            %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.total_size
+    - stage_count = @pipeline.stages_count
+    successfully completed
+    #{job_count} #{'job'.pluralize(job_count)}
+    in
+    #{stage_count} #{'stage'.pluralize(stage_count)}.

app/views/notify/_successful_pipeline.text.erb

+<%= local_assigns[:title] %>
+
+Project: <%= @project.name %> ( <%= project_url(@project) %> )
+Branch: <%= @pipeline.source_ref %> ( <%= commits_url(@pipeline) %> )
+<% if @merge_request -%>
+Merge Request: <%= @merge_request.to_reference %> ( <%= merge_request_url(@merge_request) %> )
+<% end -%>
+
+Commit: <%= @pipeline.short_sha %> ( <%= commit_url(@pipeline) %> )
+Commit Message: <%= @pipeline.git_commit_message.truncate(50) %>
+<% commit = @pipeline.commit -%>
+<% if commit.author -%>
+Commit Author: <%= sanitize_name(commit.author.name) %> ( <%= user_url(commit.author) %> )
+<% else -%>
+Commit Author: <%= commit.author_name %>
+<% end -%>
+<% if commit.different_committer? -%>
+<% if commit.committer -%>
+Committed by: <%= sanitize_name(commit.committer.name) %> ( <%= user_url(commit.committer) %> )
+<% else -%>
+Committed by: <%= commit.committer_name %>
+<% end -%>
+<% end -%>
+
+<% job_count = @pipeline.total_size -%>
+<% stage_count = @pipeline.stages_count -%>
+<% if @pipeline.user -%>
+Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by <%= sanitize_name(@pipeline.user.name) %> ( <%= user_url(@pipeline.user) %> )
+<% else -%>
+Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by API
+<% end -%>
+successfully completed <%= job_count %> <%= 'job'.pluralize(job_count) %> in <%= stage_count %> <%= 'stage'.pluralize(stage_count) %>.

app/views/notify/pipeline_fixed_email.html.haml

+= render 'notify/successful_pipeline', title: 'Your pipeline has been fixed!'

app/views/notify/pipeline_fixed_email.text.erb

+<%= render 'notify/successful_pipeline', title: 'Your pipeline has been fixed!' -%>

app/views/notify/pipeline_success_email.text.erb

-Your pipeline has passed.
-
-Project: <%= @project.name %> ( <%= project_url(@project) %> )
-Branch: <%= @pipeline.source_ref %> ( <%= commits_url(@pipeline) %> )
-<% if @merge_request -%>
-Merge Request: <%= @merge_request.to_reference %> ( <%= merge_request_url(@merge_request) %> )
-<% end -%>
-
-Commit: <%= @pipeline.short_sha %> ( <%= commit_url(@pipeline) %> )
-Commit Message: <%= @pipeline.git_commit_message.truncate(50) %>
-<% commit = @pipeline.commit -%>
-<% if commit.author -%>
-Commit Author: <%= sanitize_name(commit.author.name) %> ( <%= user_url(commit.author) %> )
-<% else -%>
-Commit Author: <%= commit.author_name %>
-<% end -%>
-<% if commit.different_committer? -%>
-<% if commit.committer -%>
-Committed by: <%= sanitize_name(commit.committer.name) %> ( <%= user_url(commit.committer) %> )
-<% else -%>
-Committed by: <%= commit.committer_name %>
-<% end -%>
-<% end -%>
-
-<% job_count = @pipeline.total_size -%>
-<% stage_count = @pipeline.stages_count -%>
-<% if @pipeline.user -%>
-Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by <%= sanitize_name(@pipeline.user.name) %> ( <%= user_url(@pipeline.user) %> )
-<% else -%>
-Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by API
-<% end -%>
-successfully completed <%= job_count %> <%= 'job'.pluralize(job_count) %> in <%= stage_count %> <%= 'stage'.pluralize(stage_count) %>.
+<%= render 'notify/successful_pipeline', title: 'Your pipeline has passed.' -%>

app/workers/all_queues.yml

@@ -668,6 +668,13 @@
   :resource_boundary: :cpu
   :weight: 3
   :idempotent: 
+- :name: pipeline_default:pipeline_update_ci_ref_status
+  :feature_category: :continuous_integration
+  :has_external_dependencies: 
+  :latency_sensitive: true
+  :resource_boundary: :cpu
+  :weight: 3
+  :idempotent: 
 - :name: pipeline_hooks:build_hooks
   :feature_category: :continuous_integration
   :has_external_dependencies: 

app/workers/pipeline_notification_worker.rb

@@ -8,12 +8,20 @@ class PipelineNotificationWorker # rubocop:disable Scalability/IdempotentWorker
   worker_resource_boundary :cpu
 
   # rubocop: disable CodeReuse/ActiveRecord
-  def perform(pipeline_id, recipients = nil)
-    pipeline = Ci::Pipeline.find_by(id: pipeline_id)
+  def perform(pipeline_id, args = {})
+    case args
+    when Hash
+      ref_status = args[:ref_status]
+      recipients = args[:recipients]
+    else # TODO: backward compatible interface, can be removed in 12.10
+      recipients = args
+      ref_status = nil
+    end
 
+    pipeline = Ci::Pipeline.find_by(id: pipeline_id)
     return unless pipeline
 
-    NotificationService.new.pipeline_finished(pipeline, recipients)
+    NotificationService.new.pipeline_finished(pipeline, ref_status: ref_status, recipients: recipients)
   end
   # rubocop: enable CodeReuse/ActiveRecord
 end

app/workers/pipeline_update_ci_ref_status_worker.rb

+# frozen_string_literal: true
+
+class PipelineUpdateCiRefStatusWorker # rubocop:disable Scalability/IdempotentWorker
+  include ApplicationWorker
+  include PipelineQueue
+
+  latency_sensitive_worker!
+  worker_resource_boundary :cpu
+
+  def perform(pipeline_id)
+    pipeline = Ci::Pipeline.find_by_id(pipeline_id)
+
+    return unless pipeline
+
+    Ci::UpdateCiRefStatusService.new(pipeline).call
+  end
+end

changelogs/unreleased/24309-notifications-for-when-pipelines-are-fixed.yml

+---
+title: Notifications for when pipelines are fixed
+merge_request: 16951
+author: Jacopo Beschi @jacopo-beschi
+type: added

db/migrate/20191001170300_create_ci_ref.rb

+# frozen_string_literal: true
+
+class CreateCiRef < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    create_table :ci_refs do |t|
+      t.references :project, null: false, index: false, foreign_key: { on_delete: :cascade }, type: :integer
+      t.integer :lock_version, default: 0
+      t.integer :last_updated_by_pipeline_id
+      t.boolean :tag, default: false, null: false
+      t.string :ref, null: false, limit: 255
+      t.string :status, null: false, limit: 255
+      t.foreign_key :ci_pipelines, column: :last_updated_by_pipeline_id, on_delete: :nullify
+      t.index [:project_id, :ref, :tag], unique: true
+      t.index [:last_updated_by_pipeline_id]
+    end
+  end
+end

db/migrate/20191111165017_add_fixed_pipeline_to_notification_settings.rb

+# frozen_string_literal: true
+
+class AddFixedPipelineToNotificationSettings < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column :notification_settings, :fixed_pipeline, :boolean
+  end
+end

db/schema.rb

@@ -878,6 +878,17 @@ ActiveRecord::Schema.define(version: 2020_02_26_162723) do
     t.index ["pipeline_id"], name: "index_ci_pipelines_config_on_pipeline_id"
   end
 
+  create_table "ci_refs", force: :cascade do |t|
+    t.integer "project_id", null: false
+    t.integer "lock_version", default: 0
+    t.integer "last_updated_by_pipeline_id"
+    t.boolean "tag", default: false, null: false
+    t.string "ref", limit: 255, null: false
+    t.string "status", limit: 255, null: false
+    t.index ["last_updated_by_pipeline_id"], name: "index_ci_refs_on_last_updated_by_pipeline_id"
+    t.index ["project_id", "ref", "tag"], name: "index_ci_refs_on_project_id_and_ref_and_tag", unique: true
+  end
+
   create_table "ci_resource_groups", force: :cascade do |t|
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
@@ -2845,6 +2856,7 @@ ActiveRecord::Schema.define(version: 2020_02_26_162723) do
     t.boolean "issue_due"
     t.boolean "new_epic"
     t.string "notification_email"
+    t.boolean "fixed_pipeline"
     t.boolean "new_release"
     t.index ["source_id", "source_type"], name: "index_notification_settings_on_source_id_and_source_type"
     t.index ["user_id", "source_id", "source_type"], name: "index_notifications_on_user_id_and_source_id_and_source_type", unique: true
@@ -4650,6 +4662,8 @@ ActiveRecord::Schema.define(version: 2020_02_26_162723) do
   add_foreign_key "ci_pipelines", "merge_requests", name: "fk_a23be95014", on_delete: :cascade
   add_foreign_key "ci_pipelines", "projects", name: "fk_86635dbd80", on_delete: :cascade
   add_foreign_key "ci_pipelines_config", "ci_pipelines", column: "pipeline_id", on_delete: :cascade
+  add_foreign_key "ci_refs", "ci_pipelines", column: "last_updated_by_pipeline_id", on_delete: :nullify
+  add_foreign_key "ci_refs", "projects", on_delete: :cascade
   add_foreign_key "ci_resource_groups", "projects", name: "fk_774722d144", on_delete: :cascade
   add_foreign_key "ci_resources", "ci_builds", column: "build_id", name: "fk_e169a8e3d5", on_delete: :nullify
   add_foreign_key "ci_resources", "ci_resource_groups", column: "resource_group_id", on_delete: :cascade

doc/api/notification_settings.md

@@ -30,6 +30,7 @@ If the `custom` level is used, specific email events can be controlled. Availabl
 - `reassign_merge_request`
 - `merge_merge_request`
 - `failed_pipeline`
+- `fixed_pipeline`
 - `success_pipeline`
 - `new_epic` **(ULTIMATE)**
 
@@ -83,6 +84,7 @@ curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.
 | `reassign_merge_request` | boolean | no | Enable/disable this notification |
 | `merge_merge_request` | boolean | no | Enable/disable this notification |
 | `failed_pipeline` | boolean | no | Enable/disable this notification |
+| `fixed_pipeline` | boolean | no | Enable/disable this notification |
 | `success_pipeline` | boolean | no | Enable/disable this notification |
 | `new_epic` | boolean | no | Enable/disable this notification ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/6626) in 11.3) **(ULTIMATE)** |
 
@@ -152,6 +154,7 @@ curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.
 | `reassign_merge_request` | boolean | no | Enable/disable this notification |
 | `merge_merge_request` | boolean | no | Enable/disable this notification |
 | `failed_pipeline` | boolean | no | Enable/disable this notification |
+| `fixed_pipeline` | boolean | no | Enable/disable this notification |
 | `success_pipeline` | boolean | no | Enable/disable this notification |
 | `new_epic` | boolean | no | Enable/disable this notification ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/6626) in 11.3) **(ULTIMATE)** |
 
@@ -178,6 +181,7 @@ Example responses:
     "reassign_merge_request": false,
     "merge_merge_request": false,
     "failed_pipeline": false,
+    "fixed_pipeline": false,
     "success_pipeline": false
   }
 }

doc/user/profile/notifications.md

@@ -178,7 +178,8 @@ In most of the below cases, the notification will be sent to:
 | Remove milestone merge request | Subscribers, participants mentioned, and Custom notification level with this event selected |
 | New comment            | The above, plus anyone mentioned by `@username` in the comment, with notification level "Mention" or higher |
 | Failed pipeline        | The author of the pipeline |
-| Successful pipeline    | The author of the pipeline, if they have the custom notification setting for successful pipelines set |
+| Fixed pipeline    | The author of the pipeline |
+| Successful pipeline    | The author of the pipeline, if they have the custom notification setting for successful pipelines set. If the pipeline failed previously, a `Fixed pipeline` message will be sent for the first successful pipeline after the failure, then a `Successful pipeline` message for any further successful pipelines. |
 | New epic **(ULTIMATE)** |        |
 | Close epic **(ULTIMATE)** |      |
 | Reopen epic **(ULTIMATE)** |     |

ee/spec/models/ee/notification_setting_spec.rb

@@ -26,6 +26,7 @@ describe NotificationSetting do
             :reassign_merge_request,
             :merge_merge_request,
             :failed_pipeline,
+            :fixed_pipeline,
             :success_pipeline,
             :new_epic
           ]
@@ -53,6 +54,7 @@ describe NotificationSetting do
             :reassign_merge_request,
             :merge_merge_request,
             :failed_pipeline,
+            :fixed_pipeline,
             :success_pipeline
           ]
         )
@@ -79,6 +81,7 @@ describe NotificationSetting do
             :reassign_merge_request,
             :merge_merge_request,
             :failed_pipeline,
+            :fixed_pipeline,
             :success_pipeline,
             :new_epic
           ]

locale/gitlab.pot

@@ -13149,6 +13149,9 @@ msgstr ""
 msgid "NotificationEvent|Failed pipeline"
 msgstr ""
 
+msgid "NotificationEvent|Fixed pipeline"
+msgstr ""
+
 msgid "NotificationEvent|Merge merge request"
 msgstr ""
 

locale/unfound_translations.rb

@@ -14,4 +14,5 @@ N_('NotificationEvent|Close merge request')
 N_('NotificationEvent|Reassign merge request')
 N_('NotificationEvent|Merge merge request')
 N_('NotificationEvent|Failed pipeline')
+N_('NotificationEvent|Fixed pipeline')
 N_('NotificationEvent|New release')

spec/factories/ci/ref.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :ci_ref, class: 'Ci::Ref' do
+    ref { 'master' }
+    status { :success }
+    tag { false }
+    project
+
+    before(:create) do |ref, evaluator|
+      next if ref.pipelines.exists?
+
+      ref.update!(last_updated_by_pipeline: create(:ci_pipeline, project: evaluator.project, ref: evaluator.ref, tag: evaluator.tag, status: evaluator.status))
+    end
+  end
+end

spec/helpers/notifications_helper_spec.rb

@@ -21,6 +21,7 @@ describe NotificationsHelper do
   describe '#notification_event_name' do
     it { expect(notification_event_name(:success_pipeline)).to match('Successful pipeline') }
     it { expect(notification_event_name(:failed_pipeline)).to match('Failed pipeline') }
+    it { expect(notification_event_name(:fixed_pipeline)).to match('Fixed pipeline') }
   end
 
   describe '#notification_icon_level' do

spec/lib/gitlab/import_export/all_models.yml

@@ -192,6 +192,7 @@ ci_pipelines:
 - environments
 - chat_data
 - source_pipeline
+- ref_status
 - source_bridge
 - source_job
 - sourced_pipelines
@@ -359,6 +360,7 @@ project:
 - ci_pipelines
 - all_pipelines
 - stages
+- ci_refs
 - builds
 - runner_projects
 - runners

spec/mailers/emails/pipelines_spec.rb

@@ -106,4 +106,17 @@ describe Emails::Pipelines do
       let(:status_text) { 'Your pipeline has failed.' }
     end
   end
+
+  describe '#pipeline_fixed_email' do
+    subject { Notify.pipeline_fixed_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) { 'been fixed' }
+      let(:status_text) { 'Your pipeline has been fixed!' }
+    end
+  end
 end

spec/models/ci/ref_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::Ref do
+  it { is_expected.to belong_to(:project) }
+  it { is_expected.to belong_to(:last_updated_by_pipeline) }
+
+  it { is_expected.to validate_inclusion_of(:status).in_array(%w[success failed fixed]) }
+  it { is_expected.to validate_presence_of(:last_updated_by_pipeline) }
+end

spec/models/notification_recipient_spec.rb

@@ -176,8 +176,20 @@ describe NotificationRecipient do
             )
           end
 
-          before do
-            notification_setting.update!(failed_pipeline: true)
+          it 'returns true' do
+            expect(recipient.suitable_notification_level?).to eq true
+          end
+        end
+
+        context "when action is fixed_pipeline" do
+          let(:recipient) do
+            described_class.new(
+              user,
+              :watch,
+              custom_action: :fixed_pipeline,
+              target: target,
+              project: project
+            )
           end
 
           it 'returns true' do
@@ -185,7 +197,7 @@ describe NotificationRecipient do
           end
         end
 
-        context "when action is not failed_pipeline" do
+        context "when action is not fixed_pipeline or failed_pipeline" do
           let(:recipient) do
             described_class.new(
               user,
@@ -196,10 +208,6 @@ describe NotificationRecipient do
             )
           end
 
-          before do
-            notification_setting.update!(success_pipeline: true)
-          end
-
           it 'returns false' do
             expect(recipient.suitable_notification_level?).to eq false
           end
@@ -309,6 +317,26 @@ describe NotificationRecipient do
             expect(recipient.suitable_notification_level?).to eq false
           end
         end
+
+        context 'when custom_action is fixed_pipeline and success_pipeline event is enabled' do
+          let(:recipient) do
+            described_class.new(
+              user,
+              :watch,
+              custom_action: :fixed_pipeline,
+              target: target,
+              project: project
+            )
+          end
+
+          before do
+            notification_setting.update!(success_pipeline: true)
+          end
+
+          it 'returns true' do
+            expect(recipient.suitable_notification_level?).to eq true
+          end
+        end
       end
     end
 

spec/models/notification_setting_spec.rb

@@ -110,7 +110,8 @@ RSpec.describe NotificationSetting do
         :reassign_merge_request,
         :merge_merge_request,
         :failed_pipeline,
-        :success_pipeline
+        :success_pipeline,
+        :fixed_pipeline
       )
     end
 

spec/models/project_spec.rb

@@ -72,6 +72,7 @@ describe Project do
     it { is_expected.to have_one(:project_setting) }
     it { is_expected.to have_many(:commit_statuses) }
     it { is_expected.to have_many(:ci_pipelines) }
+    it { is_expected.to have_many(:ci_refs) }
     it { is_expected.to have_many(:builds) }
     it { is_expected.to have_many(:build_trace_section_names)}
     it { is_expected.to have_many(:runner_projects) }

spec/services/ci/update_ci_ref_status_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::UpdateCiRefStatusService do
+  describe '#call' do
+    subject { described_class.new(pipeline) }
+
+    shared_examples 'creates ci_ref' do
+      it 'creates a ci_ref with the pipeline attributes' do
+        expect do
+          expect(subject.call).to eq(true)
+        end.to change { Ci::Ref.count }.by(1)
+
+        created_ref = pipeline.reload.ref_status
+        %w[ref tag project status].each do |attr|
+          expect(created_ref[attr]).to eq(pipeline[attr])
+        end
+      end
+
+      it 'calls PipelineNotificationWorker pasing the ref_status' do
+        expect(PipelineNotificationWorker).to receive(:perform_async).with(pipeline.id, ref_status: pipeline.status)
+
+        subject.call
+      end
+    end
+
+    shared_examples 'updates ci_ref' do
+      where(:ref_status, :pipeline_status, :next_status) do
+        [
+          %w[failed success fixed],
+          %w[failed failed failed],
+          %w[success success success],
+          %w[success failed failed]
+        ]
+      end
+
+      with_them do
+        let(:ci_ref) { create(:ci_ref, status: ref_status) }
+        let(:pipeline) { create(:ci_pipeline, status: pipeline_status, project: ci_ref.project, ref: ci_ref.ref) }
+
+        it 'sets ci_ref.status to next_status' do
+          expect do
+            expect(subject.call).to eq(true)
+            expect(ci_ref.reload.status).to eq(next_status)
+          end.not_to change { Ci::Ref.count }
+        end
+
+        it 'calls PipelineNotificationWorker pasing the ref_status' do
+          expect(PipelineNotificationWorker).to receive(:perform_async).with(pipeline.id, ref_status: next_status)
+
+          subject.call
+        end
+      end
+    end
+
+    shared_examples 'does a noop' do
+      it "doesn't change ci_ref" do
+        expect do
+          expect do
+            expect(subject.call).to eq(false)
+          end.not_to change { ci_ref.reload.status }
+        end.not_to change { Ci::Ref.count }
+      end
+
+      it "doesn't call PipelineNotificationWorker" do
+        expect(PipelineNotificationWorker).not_to receive(:perform_async)
+
+        subject.call
+      end
+    end
+
+    context "ci_ref doesn't exists" do
+      let(:pipeline) { create(:ci_pipeline, :success, ref: 'new-ref') }
+
+      it_behaves_like 'creates ci_ref'
+
+      context 'when an ActiveRecord::RecordNotUnique validation is raised' do
+        let(:ci_ref) { create(:ci_ref, status: 'failed') }
+        let(:pipeline) { create(:ci_pipeline, status: :success, project: ci_ref.project, ref: ci_ref.ref) }
+
+        it 'reloads the ci_ref and retries once' do
+          subject.instance_variable_set("@ref", subject.send(:build_ref))
+
+          expect do
+            expect(subject.call).to eq(true)
+          end.not_to change { Ci::Ref.count }
+          expect(ci_ref.reload.status).to eq('fixed')
+        end
+
+        it 'raises error on multiple retries' do
+          allow_any_instance_of(Ci::Ref).to receive(:update)
+            .and_raise(ActiveRecord::RecordNotUnique)
+
+          expect { subject.call }.to raise_error(ActiveRecord::RecordNotUnique)
+        end
+      end
+    end
+
+    context 'ci_ref exists' do
+      let!(:ci_ref) { create(:ci_ref, status: 'failed') }
+      let(:pipeline) { ci_ref.pipelines.first }
+
+      it_behaves_like 'updates ci_ref'
+
+      context 'pipeline status is invalid' do
+        let!(:pipeline) { create(:ci_pipeline, :running, project: ci_ref.project, ref: ci_ref.ref, tag: ci_ref.tag) }
+
+        it_behaves_like 'does a noop'
+      end
+
+      context 'newer pipeline finished' do
+        let(:newer_pipeline) { create(:ci_pipeline, :success, project: ci_ref.project, ref: ci_ref.ref, tag: ci_ref.tag) }
+
+        before do
+          ci_ref.update!(last_updated_by_pipeline: newer_pipeline)
+        end
+
+        it_behaves_like 'does a noop'
+      end
+
+      context 'ref is stale' do
+        let(:pipeline1) { create(:ci_pipeline, :success, project: ci_ref.project, ref: ci_ref.ref, tag: ci_ref.tag) }
+        let(:pipeline2) { create(:ci_pipeline, :success, project: ci_ref.project, ref: ci_ref.ref, tag: ci_ref.tag) }
+
+        it 'reloads the ref and retry' do
+          service1 = described_class.new(pipeline1)
+          service2 = described_class.new(pipeline2)
+
+          service2.send(:ref)
+          service1.call
+          expect(ci_ref.reload.status).to eq('fixed')
+          expect do
+            expect(service2.call).to eq(true)
+            # We expect 'success' in this case rather than 'fixed' because
+            # the ref is correctly reloaded on stale error.
+            expect(ci_ref.reload.status).to eq('success')
+          end.not_to change { Ci::Ref.count }
+        end
+
+        it 'aborts when a newer pipeline finished' do
+          service1 = described_class.new(pipeline1)
+          service2 = described_class.new(pipeline2)
+
+          service2.call
+          expect do
+            expect(service1.call).to eq(false)
+            expect(ci_ref.reload.status).to eq('fixed')
+          end.not_to change { Ci::Ref.count }
+        end
+      end
+
+      context 'ref exists as both tag/branch and tag' do
+        let(:pipeline) { create(:ci_pipeline, :failed, project: ci_ref.project, ref: ci_ref.ref, tag: true) }
+        let!(:branch_pipeline) { create(:ci_pipeline, :success, project: ci_ref.project, ref: ci_ref.ref, tag: false) }
+
+        it_behaves_like 'creates ci_ref'
+      end
+    end
+  end
+end

spec/services/notification_service_spec.rb

@@ -2315,6 +2315,7 @@ describe NotificationService, :mailer do
         user = create_user_with_notification(:custom, 'custom_enabled')
         update_custom_notification(:success_pipeline, user, resource: project)
         update_custom_notification(:failed_pipeline, user, resource: project)
+        update_custom_notification(:fixed_pipeline, user, resource: project)
         user
       end
 
@@ -2322,6 +2323,7 @@ describe NotificationService, :mailer do
         user = create_user_with_notification(:custom, 'custom_disabled')
         update_custom_notification(:success_pipeline, user, resource: project, value: false)
         update_custom_notification(:failed_pipeline, user, resource: project, value: false)
+        update_custom_notification(:fixed_pipeline, user, resource: project, value: false)
         user
       end
 
@@ -2514,6 +2516,85 @@ describe NotificationService, :mailer do
           end
         end
       end
+
+      context 'with a fixed pipeline' do
+        let(:ref_status) { 'fixed' }
+
+        context 'when the creator has no custom notification set' do
+          let(:pipeline) { create_pipeline(u_member, :success) }
+
+          it 'emails only the creator' do
+            notification.pipeline_finished(pipeline, ref_status: ref_status)
+
+            should_only_email(u_member, kind: :bcc)
+          end
+
+          it_behaves_like 'project emails are disabled' do
+            let(:notification_target)  { pipeline }
+            let(:notification_trigger) { notification.pipeline_finished(pipeline, ref_status: ref_status) }
+          end
+
+          context 'when the creator has group notification email set' do
+            let(:group_notification_email) { 'user+group@example.com' }
+
+            before do
+              group = create(:group)
+
+              project.update(group: group)
+              create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email)
+            end
+
+            it 'sends to group notification email' do
+              notification.pipeline_finished(pipeline, ref_status: ref_status)
+
+              expect(email_recipients(kind: :bcc).first).to eq(group_notification_email)
+            end
+          end
+        end
+
+        context 'when the creator has watch set' do
+          before do
+            pipeline = create_pipeline(u_watcher, :success)
+            notification.pipeline_finished(pipeline, ref_status: ref_status)
+          end
+
+          it 'emails only the creator' do
+            should_only_email(u_watcher, kind: :bcc)
+          end
+        end
+
+        context 'when the creator has custom notifications, but without any set' do
+          before do
+            pipeline = create_pipeline(u_custom_notification_unset, :success)
+            notification.pipeline_finished(pipeline, ref_status: ref_status)
+          end
+
+          it 'emails only the creator' do
+            should_only_email(u_custom_notification_unset, kind: :bcc)
+          end
+        end
+
+        context 'when the creator has custom notifications disabled' do
+          before do
+            pipeline = create_pipeline(u_custom_notification_disabled, :success)
+            notification.pipeline_finished(pipeline, ref_status: ref_status)
+          end
+
+          it 'notifies nobody' do
+            should_not_email_anyone
+          end
+        end
+
+        context 'when the creator has custom notifications set' do
+          it 'emails only the creator' do
+            pipeline = create_pipeline(u_custom_notification_enabled, :success)
+
+            notification.pipeline_finished(pipeline, ref_status: ref_status)
+
+            should_only_email(u_custom_notification_enabled, kind: :bcc)
+          end
+        end
+      end
     end
   end
 

spec/support/shared_examples/views/pipeline_status_changes_email.rb

+# frozen_string_literal: true
+
+shared_examples 'pipeline status changes email' do
+  include Devise::Test::ControllerHelpers
+
+  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,
+           project: project,
+           user: user,
+           ref: project.default_branch,
+           sha: project.commit.sha,
+           status: status)
+  end
+
+  before do
+    assign(:project, project)
+    assign(:pipeline, pipeline)
+    assign(:merge_request, merge_request)
+  end
+
+  shared_examples_for 'renders the pipeline status changes email correctly' do
+    context 'pipeline with user' do
+      it 'renders the email correctly' do
+        render
+
+        expect(rendered).to have_content title
+        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
+        expect(rendered).to have_content "##{pipeline.id}"
+        expect(rendered).to have_content pipeline.user.name
+
+        if status == :failed
+          expect(rendered).to have_content build.name
+        end
+      end
+
+      it_behaves_like 'correct pipeline information for pipelines for merge requests'
+    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 title
+        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
+        expect(rendered).to have_content "##{pipeline.id}"
+        expect(rendered).to have_content "by API"
+
+        if status == :failed
+          expect(rendered).to have_content build.name
+        end
+      end
+    end
+  end
+
+  context 'when the pipeline contains a failed job' do
+    let!(:build) { create(:ci_build, status: status, pipeline: pipeline, project: pipeline.project) }
+
+    it_behaves_like 'renders the pipeline status changes email correctly'
+  end
+
+  context 'when the latest failed job is a bridge job' do
+    let!(:build) { create(:ci_bridge, status: status, pipeline: pipeline, project: pipeline.project) }
+
+    it_behaves_like 'renders the pipeline status changes email correctly'
+  end
+end

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

@@ -3,72 +3,8 @@
 require 'spec_helper'
 
 describe 'notify/pipeline_failed_email.html.haml' do
-  include Devise::Test::ControllerHelpers
-
-  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,
-           project: project,
-           user: user,
-           ref: project.default_branch,
-           sha: project.commit.sha,
-           status: :failed)
-  end
-
-  before do
-    assign(:project, project)
-    assign(:pipeline, pipeline)
-    assign(:merge_request, merge_request)
-  end
-
-  shared_examples_for 'renders the pipeline failed email correctly' do
-    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).gsub(/\s+/, ' ')
-        expect(rendered).to have_content pipeline.commit.author_name
-        expect(rendered).to have_content "##{pipeline.id}"
-        expect(rendered).to have_content pipeline.user.name
-        expect(rendered).to have_content build.name
-      end
-
-      it_behaves_like 'correct pipeline information for pipelines for merge requests'
-    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).gsub(/\s+/, ' ')
-        expect(rendered).to have_content pipeline.commit.author_name
-        expect(rendered).to have_content "##{pipeline.id}"
-        expect(rendered).to have_content "by API"
-        expect(rendered).to have_content build.name
-      end
-    end
-  end
-
-  context 'when the pipeline contains a failed job' do
-    let!(:build) { create(:ci_build, :failed, pipeline: pipeline, project: pipeline.project) }
-
-    it_behaves_like 'renders the pipeline failed email correctly'
-  end
-
-  context 'when the latest failed job is a bridge job' do
-    let!(:build) { create(:ci_bridge, status: :failed, pipeline: pipeline, project: pipeline.project) }
-
-    it_behaves_like 'renders the pipeline failed email correctly'
+  it_behaves_like 'pipeline status changes email' do
+    let(:title) { 'Your pipeline has failed' }
+    let(:status) { :failed }
   end
 end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'notify/pipeline_fixed_email.html.haml' do
+  it_behaves_like 'pipeline status changes email' do
+    let(:title) { 'Your pipeline has been fixed!' }
+    let(:status) { :success }
+  end
+end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'notify/pipeline_fixed_email.text.erb' do
+  it_behaves_like 'pipeline status changes email' do
+    let(:title) { 'Your pipeline has been fixed!' }
+    let(:status) { :success }
+  end
+end

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

@@ -3,56 +3,8 @@
 require 'spec_helper'
 
 describe 'notify/pipeline_success_email.html.haml' do
-  include Devise::Test::ControllerHelpers
-
-  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,
-           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).gsub(/\s+/, ' ')
-      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
-
-    it_behaves_like 'correct pipeline information for pipelines for merge requests'
-  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).gsub(/\s+/, ' ')
-      expect(rendered).to have_content pipeline.commit.author_name
-      expect(rendered).to have_content "##{pipeline.id}"
-      expect(rendered).to have_content "by API"
-    end
+  it_behaves_like 'pipeline status changes email' do
+    let(:title) { 'Your pipeline has passed' }
+    let(:status) { :success }
   end
 end

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

@@ -3,24 +3,8 @@
 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)
+  it_behaves_like 'pipeline status changes email' do
+    let(:title) { 'Your pipeline has passed' }
+    let(:status) { :success }
   end
-
-  it_behaves_like 'correct pipeline information for pipelines for merge requests'
 end

spec/workers/pipeline_notification_worker_spec.rb

@@ -3,13 +3,16 @@
 require 'spec_helper'
 
 describe PipelineNotificationWorker, :mailer do
-  let(:pipeline) { create(:ci_pipeline) }
+  let_it_be(:pipeline) { create(:ci_pipeline) }
 
   describe '#execute' do
     it 'calls NotificationService#pipeline_finished when the pipeline exists' do
-      expect(NotificationService).to receive_message_chain(:new, :pipeline_finished)
+      notification_service_double = double
+      expect(notification_service_double).to receive(:pipeline_finished)
+        .with(pipeline, ref_status: 'success', recipients: ['test@gitlab.com'])
+      expect(NotificationService).to receive(:new).and_return(notification_service_double)
 
-      subject.perform(pipeline.id)
+      subject.perform(pipeline.id, ref_status: 'success', recipients: ['test@gitlab.com'])
     end
 
     it 'does nothing when the pipeline does not exist' do

spec/workers/pipeline_update_ci_ref_status_worker_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe PipelineUpdateCiRefStatusWorker do
+  let(:worker) { described_class.new }
+  let(:pipeline) { create(:ci_pipeline) }
+
+  describe '#perform' do
+    it 'updates the ci_ref status' do
+      expect(Ci::UpdateCiRefStatusService).to receive(:new)
+        .with(pipeline)
+        .and_return(double(call: true))
+
+      worker.perform(pipeline.id)
+    end
+  end
+end