Add latest changes from gitlab-org/gitlab@13-5-stable-eev13.5.0-rc42

3 files changed, 252 insertions, 150 deletions

diff --git a/spec/services/system_notes/incident_service_spec.rb b/spec/services/system_notes/incident_service_spec.rb
new file mode 100644
index 00000000000..ab9b9eb2bd4
--- /dev/null
+++ b/spec/services/system_notes/incident_service_spec.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::SystemNotes::IncidentService do
+  let_it_be(:author) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:noteable) { create(:incident, project: project) }
+  let_it_be(:issuable_severity) { create(:issuable_severity, issue: noteable, severity: :medium) }
+
+  describe '#change_incident_severity' do
+    subject(:change_severity) { described_class.new(noteable: noteable, project: project, author: author).change_incident_severity }
+
+    before do
+      allow(Gitlab::AppLogger).to receive(:error).and_call_original
+    end
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'severity' }
+    end
+
+    IssuableSeverity.severities.keys.each do |severity|
+      context "with #{severity} severity" do
+        before do
+          issuable_severity.update!(severity: severity)
+        end
+
+        it 'has the appropriate message' do
+          severity_label = IssuableSeverity::SEVERITY_LABELS.fetch(severity.to_sym)
+
+          expect(change_severity.note).to eq("changed the severity to **#{severity_label}**")
+        end
+      end
+    end
+
+    context 'when severity is invalid' do
+      let(:invalid_severity) { 'invalid-severity' }
+
+      before do
+        allow(noteable).to receive(:severity).and_return(invalid_severity)
+      end
+
+      it 'does not create system note' do
+        expect { change_severity }.not_to change { noteable.notes.count }
+      end
+
+      it 'writes error to logs' do
+        change_severity
+
+        expect(Gitlab::AppLogger).to have_received(:error).with(
+          message: 'Cannot create a system note for severity change',
+          noteable_class: noteable.class.to_s,
+          noteable_id: noteable.id,
+          severity: invalid_severity
+        )
+      end
+    end
+  end
+end
diff --git a/spec/services/system_notes/issuables_service_spec.rb b/spec/services/system_notes/issuables_service_spec.rb
index fec2a711dc2..e78b00fb67a 100644
--- a/spec/services/system_notes/issuables_service_spec.rb
+++ b/spec/services/system_notes/issuables_service_spec.rb
@@ -128,49 +128,76 @@ RSpec.describe ::SystemNotes::IssuablesService do
     end
   end
 
-  describe '#change_status' do
-    subject { service.change_status(status, source) }
+  describe '#change_issuable_reviewers' do
+    subject { service.change_issuable_reviewers([reviewer]) }
 
-    context 'when resource state event tracking is enabled' do
-      let(:status) { 'reopened' }
-      let(:source) { nil }
+    let_it_be(:noteable) { create(:merge_request, :simple, source_project: project) }
+    let_it_be(:reviewer) { create(:user) }
+    let_it_be(:reviewer1) { create(:user) }
+    let_it_be(:reviewer2) { create(:user) }
+    let_it_be(:reviewer3) { create(:user) }
 
-      it 'does not change note count' do
-        expect { subject }.not_to change { Note.count }
-      end
+    it_behaves_like 'a system note' do
+      let(:action) { 'reviewer' }
     end
 
-    context 'with status reopened' do
-      before do
-        stub_feature_flags(track_resource_state_change_events: false)
-      end
+    def build_note(old_reviewers, new_reviewers)
+      noteable.reviewers = new_reviewers
+      service.change_issuable_reviewers(old_reviewers).note
+    end
 
-      let(:status) { 'reopened' }
-      let(:source) { nil }
+    it 'builds a correct phrase when a reviewer is added to a non-assigned merge request' do
+      expect(build_note([], [reviewer1])).to eq "requested review from @#{reviewer1.username}"
+    end
 
-      it_behaves_like 'a note with overridable created_at'
+    it 'builds a correct phrase when reviewer is removed' do
+      expect(build_note([reviewer], [])).to eq "removed review request for @#{reviewer.username}"
+    end
 
-      it_behaves_like 'a system note' do
-        let(:action) { 'opened' }
-      end
+    it 'builds a correct phrase when reviewers changed' do
+      expect(build_note([reviewer1], [reviewer2])).to(
+        eq("requested review from @#{reviewer2.username} and removed review request for @#{reviewer1.username}")
+      )
     end
 
-    context 'with a source' do
-      before do
-        stub_feature_flags(track_resource_state_change_events: false)
-      end
+    it 'builds a correct phrase when three reviewers removed and one added' do
+      expect(build_note([reviewer, reviewer1, reviewer2], [reviewer3])).to(
+        eq("requested review from @#{reviewer3.username} and removed review request for @#{reviewer.username}, @#{reviewer1.username}, and @#{reviewer2.username}")
+      )
+    end
 
-      let(:status) { 'opened' }
-      let(:source) { double('commit', gfm_reference: 'commit 123456') }
+    it 'builds a correct phrase when one reviewer is changed from a set' do
+      expect(build_note([reviewer, reviewer1], [reviewer, reviewer2])).to(
+        eq("requested review from @#{reviewer2.username} and removed review request for @#{reviewer1.username}")
+      )
+    end
 
-      it_behaves_like 'a note with overridable created_at'
+    it 'builds a correct phrase when one reviewer removed from a set' do
+      expect(build_note([reviewer, reviewer1, reviewer2], [reviewer, reviewer1])).to(
+        eq( "removed review request for @#{reviewer2.username}")
+      )
+    end
 
-      it 'sets the note text' do
-        expect(subject.note).to eq "#{status} via commit 123456"
+    it 'builds a correct phrase when the locale is different' do
+      Gitlab::I18n.with_locale('pt-BR') do
+        expect(build_note([reviewer, reviewer1, reviewer2], [reviewer3])).to(
+          eq("requested review from @#{reviewer3.username} and removed review request for @#{reviewer.username}, @#{reviewer1.username}, and @#{reviewer2.username}")
+        )
       end
     end
   end
 
+  describe '#change_status' do
+    subject { service.change_status(status, source) }
+
+    let(:status) { 'reopened' }
+    let(:source) { nil }
+
+    it 'creates a resource state event' do
+      expect { subject }.to change { ResourceStateEvent.count }.by(1)
+    end
+  end
+
   describe '#change_title' do
     let(:noteable) { create(:issue, project: project, title: 'Lorem ipsum') }
 
@@ -636,67 +663,26 @@ RSpec.describe ::SystemNotes::IssuablesService do
   describe '#close_after_error_tracking_resolve' do
     subject { service.close_after_error_tracking_resolve }
 
-    context 'when state tracking is enabled' do
-      before do
-        stub_feature_flags(track_resource_state_change_events: true)
-      end
-
-      it 'creates the expected state event' do
-        subject
-
-        event = ResourceStateEvent.last
-
-        expect(event.close_after_error_tracking_resolve).to eq(true)
-        expect(event.state).to eq('closed')
-      end
-    end
+    it 'creates the expected state event' do
+      subject
 
-    context 'when state tracking is disabled' do
-      before do
-        stub_feature_flags(track_resource_state_change_events: false)
-      end
+      event = ResourceStateEvent.last
 
-      it_behaves_like 'a system note' do
-        let(:action) { 'closed' }
-      end
-
-      it 'creates the expected system note' do
-        expect(subject.note)
-          .to eq('resolved the corresponding error and closed the issue.')
-      end
+      expect(event.close_after_error_tracking_resolve).to eq(true)
+      expect(event.state).to eq('closed')
     end
   end
 
   describe '#auto_resolve_prometheus_alert' do
     subject { service.auto_resolve_prometheus_alert }
 
-    context 'when state tracking is enabled' do
-      before do
-        stub_feature_flags(track_resource_state_change_events: true)
-      end
+    it 'creates the expected state event' do
+      subject
 
-      it 'creates the expected state event' do
-        subject
+      event = ResourceStateEvent.last
 
-        event = ResourceStateEvent.last
-
-        expect(event.close_auto_resolve_prometheus_alert).to eq(true)
-        expect(event.state).to eq('closed')
-      end
-    end
-
-    context 'when state tracking is disabled' do
-      before do
-        stub_feature_flags(track_resource_state_change_events: false)
-      end
-
-      it_behaves_like 'a system note' do
-        let(:action) { 'closed' }
-      end
-
-      it 'creates the expected system note' do
-        expect(subject.note).to eq('automatically closed this issue because the alert resolved.')
-      end
+      expect(event.close_auto_resolve_prometheus_alert).to eq(true)
+      expect(event.state).to eq('closed')
     end
   end
 end
diff --git a/spec/services/system_notes/time_tracking_service_spec.rb b/spec/services/system_notes/time_tracking_service_spec.rb
index f671e66cdcd..ec126cb5447 100644
--- a/spec/services/system_notes/time_tracking_service_spec.rb
+++ b/spec/services/system_notes/time_tracking_service_spec.rb
@@ -6,124 +6,181 @@ RSpec.describe ::SystemNotes::TimeTrackingService do
   let_it_be(:author)   { create(:user) }
   let_it_be(:project)  { create(:project, :repository) }
 
-  let(:noteable) { create(:issue, project: project) }
-
   describe '#change_due_date' do
     subject { described_class.new(noteable: noteable, project: project, author: author).change_due_date(due_date) }
 
     let(:due_date) { Date.today }
 
-    it_behaves_like 'a note with overridable created_at'
+    context 'when noteable is an issue' do
+      let_it_be(:noteable) { create(:issue, project: project) }
 
-    it_behaves_like 'a system note' do
-      let(:action) { 'due_date' }
-    end
+      it_behaves_like 'a note with overridable created_at'
+
+      it_behaves_like 'a system note' do
+        let(:action) { 'due_date' }
+      end
 
-    context 'when due date added' do
-      it 'sets the note text' do
-        expect(subject.note).to eq "changed due date to #{due_date.to_s(:long)}"
+      context 'when due date added' do
+        it 'sets the note text' do
+          expect(subject.note).to eq "changed due date to #{due_date.to_s(:long)}"
+        end
+      end
+
+      context 'when due date removed' do
+        let(:due_date) { nil }
+
+        it 'sets the note text' do
+          expect(subject.note).to eq 'removed due date'
+        end
+      end
+
+      it 'tracks the issue event in usage ping' do
+        expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_due_date_changed_action).with(author: author)
+
+        subject
       end
     end
 
-    context 'when due date removed' do
-      let(:due_date) { nil }
+    context 'when noteable is a merge request' do
+      let_it_be(:noteable) { create(:merge_request, source_project: project) }
+
+      it 'does not track the issue event in usage ping' do
+        expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_due_date_changed_action).with(author: author)
 
-      it 'sets the note text' do
-        expect(subject.note).to eq 'removed due date'
+        subject
       end
     end
   end
 
-  describe '.change_time_estimate' do
+  describe '#change_time_estimate' do
     subject { described_class.new(noteable: noteable, project: project, author: author).change_time_estimate }
 
-    it_behaves_like 'a system note' do
-      let(:action) { 'time_tracking' }
-    end
-
-    context 'with a time estimate' do
-      it 'sets the note text' do
-        noteable.update_attribute(:time_estimate, 277200)
+    context 'when noteable is an issue' do
+      let_it_be(:noteable, reload: true) { create(:issue, project: project) }
 
-        expect(subject.note).to eq "changed time estimate to 1w 4d 5h"
+      it_behaves_like 'a system note' do
+        let(:action) { 'time_tracking' }
       end
 
-      context 'when time_tracking_limit_to_hours setting is true' do
-        before do
-          stub_application_setting(time_tracking_limit_to_hours: true)
-        end
-
+      context 'with a time estimate' do
         it 'sets the note text' do
           noteable.update_attribute(:time_estimate, 277200)
 
-          expect(subject.note).to eq "changed time estimate to 77h"
+          expect(subject.note).to eq "changed time estimate to 1w 4d 5h"
+        end
+
+        context 'when time_tracking_limit_to_hours setting is true' do
+          before do
+            stub_application_setting(time_tracking_limit_to_hours: true)
+          end
+
+          it 'sets the note text' do
+            noteable.update_attribute(:time_estimate, 277200)
+
+            expect(subject.note).to eq "changed time estimate to 77h"
+          end
         end
       end
-    end
 
-    context 'without a time estimate' do
-      it 'sets the note text' do
-        expect(subject.note).to eq "removed time estimate"
+      context 'without a time estimate' do
+        it 'sets the note text' do
+          expect(subject.note).to eq "removed time estimate"
+        end
       end
-    end
-  end
 
-  describe '.change_time_spent' do
-    # We need a custom noteable in order to the shared examples to be green.
-    let(:noteable) do
-      mr = create(:merge_request, source_project: project)
-      mr.spend_time(duration: 360000, user_id: author.id)
-      mr.save!
-      mr
-    end
+      it 'tracks the issue event in usage ping' do
+        expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_time_estimate_changed_action).with(author: author)
 
-    subject do
-      described_class.new(noteable: noteable, project: project, author: author).change_time_spent
+        subject
+      end
     end
 
-    it_behaves_like 'a system note' do
-      let(:action) { 'time_tracking' }
-    end
+    context 'when noteable is a merge request' do
+      let_it_be(:noteable) { create(:merge_request, source_project: project) }
 
-    context 'when time was added' do
-      it 'sets the note text' do
-        spend_time!(277200)
+      it 'does not track the issue event in usage ping' do
+        expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_time_estimate_changed_action).with(author: author)
 
-        expect(subject.note).to eq "added 1w 4d 5h of time spent"
+        subject
       end
     end
+  end
 
-    context 'when time was subtracted' do
-      it 'sets the note text' do
-        spend_time!(-277200)
+  describe '#change_time_spent' do
+    subject { described_class.new(noteable: noteable, project: project, author: author).change_time_spent }
 
-        expect(subject.note).to eq "subtracted 1w 4d 5h of time spent"
-      end
-    end
+    context 'when noteable is an issue' do
+      let_it_be(:noteable, reload: true) { create(:issue, project: project) }
 
-    context 'when time was removed' do
-      it 'sets the note text' do
-        spend_time!(:reset)
+      it_behaves_like 'a system note' do
+        let(:action) { 'time_tracking' }
 
-        expect(subject.note).to eq "removed time spent"
+        before do
+          spend_time!(277200)
+        end
       end
-    end
 
-    context 'when time_tracking_limit_to_hours setting is true' do
-      before do
-        stub_application_setting(time_tracking_limit_to_hours: true)
+      context 'when time was added' do
+        it 'sets the note text' do
+          spend_time!(277200)
+
+          expect(subject.note).to eq "added 1w 4d 5h of time spent"
+        end
+
+        context 'when time was subtracted' do
+          it 'sets the note text' do
+            spend_time!(360000)
+            spend_time!(-277200)
+
+            expect(subject.note).to eq "subtracted 1w 4d 5h of time spent"
+          end
+        end
+
+        context 'when time was removed' do
+          it 'sets the note text' do
+            spend_time!(:reset)
+
+            expect(subject.note).to eq "removed time spent"
+          end
+        end
+
+        context 'when time_tracking_limit_to_hours setting is true' do
+          before do
+            stub_application_setting(time_tracking_limit_to_hours: true)
+          end
+
+          it 'sets the note text' do
+            spend_time!(277200)
+
+            expect(subject.note).to eq "added 77h of time spent"
+          end
+        end
+
+        it 'tracks the issue event in usage ping' do
+          expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_time_spent_changed_action).with(author: author)
+
+          spend_time!(277200)
+
+          subject
+        end
       end
 
-      it 'sets the note text' do
-        spend_time!(277200)
+      context 'when noteable is a merge request' do
+        let_it_be(:noteable) { create(:merge_request, source_project: project) }
+
+        it 'does not track the issue event in usage ping' do
+          expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_time_estimate_changed_action).with(author: author)
+
+          spend_time!(277200)
 
-        expect(subject.note).to eq "added 77h of time spent"
+          subject
+        end
       end
-    end
 
-    def spend_time!(seconds)
-      noteable.spend_time(duration: seconds, user_id: author.id)
-      noteable.save!
+      def spend_time!(seconds)
+        noteable.spend_time(duration: seconds, user_id: author.id)
+        noteable.save!
+      end
     end
   end
 end