Add latest changes from gitlab-org/gitlab@master

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/lib/gitlab/background_migration/mentions_migration_shared_examples.rb

@@ -65,9 +65,16 @@ shared_examples 'resource notes mentions migration' do |migration_class, resourc
 end
 shared_examples 'schedules resource mentions migration' do |resource_class, is_for_notes|
+  before do
+    stub_const("#{described_class.name}::BATCH_SIZE", 1)
+  end
+
   it 'schedules background migrations' do
     Sidekiq::Testing.fake! do
       Timecop.freeze do
+        resource_count = is_for_notes ? Note.count : resource_class.count
+        expect(resource_count).to eq 5
+
         migrate!
         migration = described_class::MIGRATION

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)
-      subject.perform(pipeline.id)
+      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, 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