Add latest changes from gitlab-org/gitlab@13-1-stable-ee

3 files changed, 166 insertions, 25 deletions

diff --git a/spec/services/alert_management/alerts/update_service_spec.rb b/spec/services/alert_management/alerts/update_service_spec.rb
new file mode 100644
index 00000000000..e185e67c5cf
--- /dev/null
+++ b/spec/services/alert_management/alerts/update_service_spec.rb
@@ -0,0 +1,134 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AlertManagement::Alerts::UpdateService do
+  let_it_be(:user_with_permissions) { create(:user) }
+  let_it_be(:user_without_permissions) { create(:user) }
+  let_it_be(:alert, reload: true) { create(:alert_management_alert) }
+  let_it_be(:project) { alert.project }
+
+  let(:current_user) { user_with_permissions }
+  let(:params) { {} }
+
+  let(:service) { described_class.new(alert, current_user, params) }
+
+  before_all do
+    project.add_developer(user_with_permissions)
+  end
+
+  describe '#execute' do
+    subject(:response) { service.execute }
+
+    context 'when the current_user is nil' do
+      let(:current_user) { nil }
+
+      it 'results in an error' do
+        expect(response).to be_error
+        expect(response.message).to eq('You have no permissions')
+      end
+    end
+
+    context 'when user does not have permission to update alerts' do
+      let(:current_user) { user_without_permissions }
+
+      it 'results in an error' do
+        expect(response).to be_error
+        expect(response.message).to eq('You have no permissions')
+      end
+    end
+
+    context 'when no parameters are included' do
+      it 'results in an error' do
+        expect(response).to be_error
+        expect(response.message).to eq('Please provide attributes to update')
+      end
+    end
+
+    context 'when an error occures during update' do
+      let(:params) { { title: nil } }
+
+      it 'results in an error' do
+        expect { response }.not_to change { alert.reload.notes.count }
+        expect(response).to be_error
+        expect(response.message).to eq("Title can't be blank")
+      end
+    end
+
+    context 'when a model attribute is included without assignees' do
+      let(:params) { { title: 'This is an updated alert.' } }
+
+      it 'updates the attribute' do
+        original_title = alert.title
+
+        expect { response }.to change { alert.title }.from(original_title).to(params[:title])
+        expect(response).to be_success
+      end
+
+      it 'skips adding a todo' do
+        expect { response }.not_to change(Todo, :count)
+      end
+    end
+
+    context 'when assignees are included' do
+      let(:params) { { assignees: [user_with_permissions] } }
+
+      after do
+        alert.assignees = []
+      end
+
+      it 'assigns the user' do
+        expect { response }.to change { alert.reload.assignees }.from([]).to(params[:assignees])
+        expect(response).to be_success
+      end
+
+      it 'creates a system note for the assignment' do
+        expect { response }.to change { alert.reload.notes.count }.by(1)
+      end
+
+      it 'adds a todo' do
+        expect { response }.to change { Todo.where(user: user_with_permissions).count }.by(1)
+      end
+
+      context 'when current user is not the assignee' do
+        let(:assignee_user) { create(:user) }
+        let(:params) { { assignees: [assignee_user] } }
+
+        it 'skips adding todo for assignee without permission to read alert' do
+          expect { response }.not_to change(Todo, :count)
+        end
+
+        context 'when assignee has read permission' do
+          before do
+            project.add_developer(assignee_user)
+          end
+
+          it 'adds a todo' do
+            response
+
+            expect(Todo.first.author).to eq(current_user)
+          end
+        end
+
+        context 'when current_user is nil' do
+          let(:current_user) { nil }
+
+          it 'skips adding todo if current_user is nil' do
+            project.add_developer(assignee_user)
+
+            expect { response }.not_to change(Todo, :count)
+          end
+        end
+      end
+
+      context 'with multiple users included' do
+        let(:params) { { assignees: [user_with_permissions, user_without_permissions] } }
+
+        it 'assigns the first permissioned user' do
+          expect { response }.to change { alert.reload.assignees }.from([]).to([user_with_permissions])
+          expect(response).to be_success
+        end
+      end
+    end
+  end
+end
diff --git a/spec/services/alert_management/create_alert_issue_service_spec.rb b/spec/services/alert_management/create_alert_issue_service_spec.rb
index 62afe777165..9bc8b731dc1 100644
--- a/spec/services/alert_management/create_alert_issue_service_spec.rb
+++ b/spec/services/alert_management/create_alert_issue_service_spec.rb
@@ -94,11 +94,7 @@ RSpec.describe AlertManagement::CreateAlertIssueService do
       end
 
       context 'when alert cannot be updated' do
-        before do
-          # invalidate alert
-          too_many_hosts = Array.new(AlertManagement::Alert::HOSTS_MAX_LENGTH + 1) { |_| 'host' }
-          alert.update_columns(hosts: too_many_hosts)
-        end
+        let(:alert) { create(:alert_management_alert, :with_validation_errors, :triggered, project: project, payload: payload) }
 
         it 'responds with error' do
           expect(execute).to be_error
@@ -122,17 +118,6 @@ RSpec.describe AlertManagement::CreateAlertIssueService do
           expect(execute.message).to eq(_('An issue already exists'))
         end
       end
-
-      context 'when alert_management_create_alert_issue feature flag is disabled' do
-        before do
-          stub_feature_flags(alert_management_create_alert_issue: false)
-        end
-
-        it 'responds with error' do
-          expect(execute).to be_error
-          expect(execute.message).to eq(_('You have no permissions'))
-        end
-      end
     end
 
     context 'when a user is not allowed to create an issue' do
diff --git a/spec/services/alert_management/process_prometheus_alert_service_spec.rb b/spec/services/alert_management/process_prometheus_alert_service_spec.rb
index 73f9f103902..5b4da5e9077 100644
--- a/spec/services/alert_management/process_prometheus_alert_service_spec.rb
+++ b/spec/services/alert_management/process_prometheus_alert_service_spec.rb
@@ -5,8 +5,12 @@ require 'spec_helper'
 RSpec.describe AlertManagement::ProcessPrometheusAlertService do
   let_it_be(:project) { create(:project) }
 
+  before do
+    allow(ProjectServiceWorker).to receive(:perform_async)
+  end
+
   describe '#execute' do
-    subject { described_class.new(project, nil, payload).execute }
+    subject(:execute) { described_class.new(project, nil, payload).execute }
 
     context 'when alert payload is valid' do
       let(:parsed_alert) { Gitlab::Alerting::Alert.new(project: project, payload: payload) }
@@ -37,12 +41,22 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
         context 'when alert with the same fingerprint already exists' do
           let!(:alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: parsed_alert.gitlab_fingerprint) }
 
+          it 'increases alert events count' do
+            expect { execute }.to change { alert.reload.events }.by(1)
+          end
+
           context 'when status can be changed' do
             it 'changes status to triggered' do
-              expect { subject }.to change { alert.reload.triggered? }.to(true)
+              expect { execute }.to change { alert.reload.triggered? }.to(true)
             end
           end
 
+          it 'does not executes the alert service hooks' do
+            expect(alert).not_to receive(:execute_services)
+
+            subject
+          end
+
           context 'when status change did not succeed' do
             before do
               allow(AlertManagement::Alert).to receive(:for_fingerprint).and_return([alert])
@@ -56,7 +70,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
                 alert_id: alert.id
               )
 
-              subject
+              execute
             end
           end
 
@@ -66,7 +80,15 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
         context 'when alert does not exist' do
           context 'when alert can be created' do
             it 'creates a new alert' do
-              expect { subject }.to change { AlertManagement::Alert.where(project: project).count }.by(1)
+              expect { execute }.to change { AlertManagement::Alert.where(project: project).count }.by(1)
+            end
+
+            it 'executes the alert service hooks' do
+              slack_service = create(:service, type: 'SlackService', project: project, alert_events: true, active: true)
+
+              subject
+
+              expect(ProjectServiceWorker).to have_received(:perform_async).with(slack_service.id, an_instance_of(Hash))
             end
           end
 
@@ -85,7 +107,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
                 alert_errors: { hosts: ['hosts array is over 255 chars'] }
               )
 
-              subject
+              execute
             end
           end
 
@@ -99,7 +121,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
 
         context 'when status can be changed' do
           it 'resolves an existing alert' do
-            expect { subject }.to change { alert.reload.resolved? }.to(true)
+            expect { execute }.to change { alert.reload.resolved? }.to(true)
           end
         end
 
@@ -116,7 +138,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
               alert_id: alert.id
             )
 
-            subject
+            execute
           end
         end
 
@@ -128,8 +150,8 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
       let(:payload) { {} }
 
       it 'responds with bad_request' do
-        expect(subject).to be_error
-        expect(subject.http_status).to eq(:bad_request)
+        expect(execute).to be_error
+        expect(execute.http_status).to eq(:bad_request)
       end
     end
   end