Add latest changes from gitlab-org/gitlab@master

17 files changed, 81 insertions, 60 deletions

diff --git a/spec/workers/ci/create_cross_project_pipeline_worker_spec.rb b/spec/workers/ci/create_cross_project_pipeline_worker_spec.rb
index 22eab1d20f7..492f5e812ee 100644
--- a/spec/workers/ci/create_cross_project_pipeline_worker_spec.rb
+++ b/spec/workers/ci/create_cross_project_pipeline_worker_spec.rb
@@ -29,7 +29,7 @@ describe Ci::CreateCrossProjectPipelineWorker do
         expect(Ci::CreateCrossProjectPipelineService)
           .not_to receive(:new)
 
-        described_class.new.perform(1234)
+        described_class.new.perform(non_existing_record_id)
       end
     end
   end
diff --git a/spec/workers/ci/pipeline_bridge_status_worker_spec.rb b/spec/workers/ci/pipeline_bridge_status_worker_spec.rb
index d5f95a035fd..a6129b2cf93 100644
--- a/spec/workers/ci/pipeline_bridge_status_worker_spec.rb
+++ b/spec/workers/ci/pipeline_bridge_status_worker_spec.rb
@@ -25,7 +25,7 @@ describe Ci::PipelineBridgeStatusWorker do
     end
 
     context 'when pipeline does not exist' do
-      let(:pipeline_id) { 1234 }
+      let(:pipeline_id) { non_existing_record_id }
 
       it 'does not call the service' do
         expect(Ci::PipelineBridgeStatusService)
diff --git a/spec/workers/cluster_update_app_worker_spec.rb b/spec/workers/cluster_update_app_worker_spec.rb
index d91104334e5..5391c194679 100644
--- a/spec/workers/cluster_update_app_worker_spec.rb
+++ b/spec/workers/cluster_update_app_worker_spec.rb
@@ -52,6 +52,8 @@ describe ClusterUpdateAppWorker do
       let(:lease_key) { "#{described_class.name.underscore}-#{application.id}" }
 
       before do
+        # update_highest_role uses exclusive key too:
+        allow(Gitlab::ExclusiveLease).to receive(:new).and_call_original
         stub_exclusive_lease_taken(lease_key)
       end
 
@@ -62,8 +64,6 @@ describe ClusterUpdateAppWorker do
       end
 
       it 'does not allow same app to be updated concurrently by different project', :aggregate_failures do
-        stub_exclusive_lease("refresh_authorized_projects:#{user.id}")
-        stub_exclusive_lease("update_highest_role:#{user.id}")
         project1 = create(:project, namespace: create(:namespace, owner: user))
 
         expect(Clusters::Applications::PrometheusUpdateService).not_to receive(:new)
@@ -87,8 +87,6 @@ describe ClusterUpdateAppWorker do
         application2 = create(:clusters_applications_prometheus, :installed)
         lease_key2 = "#{described_class.name.underscore}-#{application2.id}"
 
-        stub_exclusive_lease("refresh_authorized_projects:#{user.id}")
-        stub_exclusive_lease("update_highest_role:#{user.id}")
         project2 = create(:project, namespace: create(:namespace, owner: user))
 
         stub_exclusive_lease(lease_key2)
diff --git a/spec/workers/expire_job_cache_worker_spec.rb b/spec/workers/expire_job_cache_worker_spec.rb
index 2776624f14c..804a50e89fe 100644
--- a/spec/workers/expire_job_cache_worker_spec.rb
+++ b/spec/workers/expire_job_cache_worker_spec.rb
@@ -39,7 +39,7 @@ describe ExpireJobCacheWorker do
       it 'does not change the etag store' do
         expect(Gitlab::EtagCaching::Store).not_to receive(:new)
 
-        perform_multiple(9999)
+        perform_multiple(non_existing_record_id)
       end
     end
   end
diff --git a/spec/workers/group_export_worker_spec.rb b/spec/workers/group_export_worker_spec.rb
index 4aa85d2b381..350777df912 100644
--- a/spec/workers/group_export_worker_spec.rb
+++ b/spec/workers/group_export_worker_spec.rb
@@ -21,8 +21,8 @@ describe GroupExportWorker do
       it 'raises an exception when params are invalid' do
         expect_any_instance_of(::Groups::ImportExport::ExportService).not_to receive(:execute)
 
-        expect { subject.perform(1234, group.id, {}) }.to raise_exception(ActiveRecord::RecordNotFound)
-        expect { subject.perform(user.id, 1234, {}) }.to raise_exception(ActiveRecord::RecordNotFound)
+        expect { subject.perform(non_existing_record_id, group.id, {}) }.to raise_exception(ActiveRecord::RecordNotFound)
+        expect { subject.perform(user.id, non_existing_record_id, {}) }.to raise_exception(ActiveRecord::RecordNotFound)
       end
     end
   end
diff --git a/spec/workers/group_import_worker_spec.rb b/spec/workers/group_import_worker_spec.rb
index 0783ac4df4e..641aa45c9b0 100644
--- a/spec/workers/group_import_worker_spec.rb
+++ b/spec/workers/group_import_worker_spec.rb
@@ -21,8 +21,8 @@ describe GroupImportWorker do
       it 'raises an exception when params are invalid' do
         expect_any_instance_of(::Groups::ImportExport::ImportService).not_to receive(:execute)
 
-        expect { subject.perform(1234, group.id) }.to raise_exception(ActiveRecord::RecordNotFound)
-        expect { subject.perform(user.id, 1234) }.to raise_exception(ActiveRecord::RecordNotFound)
+        expect { subject.perform(non_existing_record_id, group.id) }.to raise_exception(ActiveRecord::RecordNotFound)
+        expect { subject.perform(user.id, non_existing_record_id) }.to raise_exception(ActiveRecord::RecordNotFound)
       end
     end
   end
diff --git a/spec/workers/incident_management/process_prometheus_alert_worker_spec.rb b/spec/workers/incident_management/process_prometheus_alert_worker_spec.rb
index 19ef2635882..5fbc39cad4e 100644
--- a/spec/workers/incident_management/process_prometheus_alert_worker_spec.rb
+++ b/spec/workers/incident_management/process_prometheus_alert_worker_spec.rb
@@ -60,7 +60,7 @@ describe IncidentManagement::ProcessPrometheusAlertWorker do
     end
 
     context 'when project could not be found' do
-      let(:non_existing_project_id) { (Project.maximum(:id) || 0) + 1 }
+      let(:non_existing_project_id) { non_existing_record_id }
 
       it 'does not create an issue' do
         expect { subject.perform(non_existing_project_id, alert_params) }
@@ -75,7 +75,7 @@ describe IncidentManagement::ProcessPrometheusAlertWorker do
 
     context 'when event could not be found' do
       before do
-        alert_params[:labels][:gitlab_alert_id] = (PrometheusAlertEvent.maximum(:id) || 0) + 1
+        alert_params[:labels][:gitlab_alert_id] = non_existing_record_id
       end
 
       it 'does not create an issue' do
diff --git a/spec/workers/namespaceless_project_destroy_worker_spec.rb b/spec/workers/namespaceless_project_destroy_worker_spec.rb
index e6686328291..b97a44c714d 100644
--- a/spec/workers/namespaceless_project_destroy_worker_spec.rb
+++ b/spec/workers/namespaceless_project_destroy_worker_spec.rb
@@ -82,7 +82,7 @@ describe NamespacelessProjectDestroyWorker do
 
     context 'project has non-existing namespace' do
       let!(:project) do
-        project = build(:project, namespace_id: Namespace.maximum(:id).to_i.succ)
+        project = build(:project, namespace_id: non_existing_record_id)
         project.save(validate: false)
         project
       end
diff --git a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb
index e7da346df46..2f4c7f8bc07 100644
--- a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb
+++ b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb
@@ -50,7 +50,7 @@ describe Namespaces::ScheduleAggregationWorker, '#perform', :clean_gitlab_redis_
     it 'logs the error' do
       expect(Gitlab::ErrorTracking).to receive(:track_exception).once
 
-      worker.perform(12345)
+      worker.perform(non_existing_record_id)
     end
   end
 
diff --git a/spec/workers/new_issue_worker_spec.rb b/spec/workers/new_issue_worker_spec.rb
index 88a75ce5b70..1584e9d5302 100644
--- a/spec/workers/new_issue_worker_spec.rb
+++ b/spec/workers/new_issue_worker_spec.rb
@@ -30,9 +30,11 @@ describe NewIssueWorker do
       end
 
       it 'logs an error' do
+        issue = create(:issue)
+
         expect(Rails.logger).to receive(:error).with('NewIssueWorker: couldn\'t find User with ID=99, skipping job')
 
-        worker.perform(create(:issue).id, 99)
+        worker.perform(issue.id, 99)
       end
     end
 
diff --git a/spec/workers/new_merge_request_worker_spec.rb b/spec/workers/new_merge_request_worker_spec.rb
index d078ddd07d9..fe22226903f 100644
--- a/spec/workers/new_merge_request_worker_spec.rb
+++ b/spec/workers/new_merge_request_worker_spec.rb
@@ -15,9 +15,11 @@ describe NewMergeRequestWorker do
       end
 
       it 'logs an error' do
+        user = create(:user)
+
         expect(Rails.logger).to receive(:error).with('NewMergeRequestWorker: couldn\'t find MergeRequest with ID=99, skipping job')
 
-        worker.perform(99, create(:user).id)
+        worker.perform(99, user.id)
       end
     end
 
@@ -30,9 +32,11 @@ describe NewMergeRequestWorker do
       end
 
       it 'logs an error' do
+        merge_request = create(:merge_request)
+
         expect(Rails.logger).to receive(:error).with('NewMergeRequestWorker: couldn\'t find User with ID=99, skipping job')
 
-        worker.perform(create(:merge_request).id, 99)
+        worker.perform(merge_request.id, 99)
       end
     end
 
diff --git a/spec/workers/new_note_worker_spec.rb b/spec/workers/new_note_worker_spec.rb
index ae62237960a..387a0958d6b 100644
--- a/spec/workers/new_note_worker_spec.rb
+++ b/spec/workers/new_note_worker_spec.rb
@@ -24,11 +24,11 @@ describe NewNoteWorker do
   end
 
   context 'when Note not found' do
-    let(:unexistent_note_id) { 999 }
+    let(:unexistent_note_id) { non_existing_record_id }
 
     it 'logs NewNoteWorker process skipping' do
       expect(Rails.logger).to receive(:error)
-        .with("NewNoteWorker: couldn't find note with ID=999, skipping job")
+        .with("NewNoteWorker: couldn't find note with ID=#{unexistent_note_id}, skipping job")
 
       described_class.new.perform(unexistent_note_id)
     end
diff --git a/spec/workers/pipeline_notification_worker_spec.rb b/spec/workers/pipeline_notification_worker_spec.rb
index 5defd3d5bd7..f2cc2b56236 100644
--- a/spec/workers/pipeline_notification_worker_spec.rb
+++ b/spec/workers/pipeline_notification_worker_spec.rb
@@ -18,7 +18,7 @@ describe PipelineNotificationWorker, :mailer do
     it 'does nothing when the pipeline does not exist' do
       expect(NotificationService).not_to receive(:new)
 
-      subject.perform(Ci::Pipeline.maximum(:id).to_i.succ)
+      subject.perform(non_existing_record_id)
     end
   end
 end
diff --git a/spec/workers/project_export_worker_spec.rb b/spec/workers/project_export_worker_spec.rb
index d0d52e0df2d..373e7f32530 100644
--- a/spec/workers/project_export_worker_spec.rb
+++ b/spec/workers/project_export_worker_spec.rb
@@ -57,11 +57,11 @@ describe ProjectExportWorker do
       end
 
       it 'does not raise error when project cannot be found' do
-        expect { subject.perform(user.id, -234, {}) }.not_to raise_error
+        expect { subject.perform(user.id, non_existing_record_id, {}) }.not_to raise_error
       end
 
       it 'does not raise error when user cannot be found' do
-        expect { subject.perform(-863, project.id, {}) }.not_to raise_error
+        expect { subject.perform(non_existing_record_id, project.id, {}) }.not_to raise_error
       end
     end
   end
diff --git a/spec/workers/remote_mirror_notification_worker_spec.rb b/spec/workers/remote_mirror_notification_worker_spec.rb
index c9321fd7c56..f0fb46f84d9 100644
--- a/spec/workers/remote_mirror_notification_worker_spec.rb
+++ b/spec/workers/remote_mirror_notification_worker_spec.rb
@@ -26,7 +26,7 @@ describe RemoteMirrorNotificationWorker, :mailer do
     it 'does nothing when the mirror does not exist' do
       expect(NotificationService).not_to receive(:new)
 
-      subject.perform(RemoteMirror.maximum(:id).to_i.succ)
+      subject.perform(non_existing_record_id)
     end
 
     it 'does nothing when a notification has already been sent' do
diff --git a/spec/workers/update_highest_role_worker_spec.rb b/spec/workers/update_highest_role_worker_spec.rb
index cb112ebe07e..1e378a5a61e 100644
--- a/spec/workers/update_highest_role_worker_spec.rb
+++ b/spec/workers/update_highest_role_worker_spec.rb
@@ -8,56 +8,73 @@ describe UpdateHighestRoleWorker, :clean_gitlab_redis_shared_state do
   let(:worker) { described_class.new }
 
   describe '#perform' do
-    let(:active_scope_attributes) do
-      {
-        state: 'active',
-        ghost: false,
-        user_type: nil
-      }
+    context 'when user is not found' do
+      it 'does not update or deletes any highest role', :aggregate_failures do
+        expect { worker.perform(-1) }.not_to change(UserHighestRole, :count)
+      end
     end
-    let(:user) { create(:user, attributes) }
-
-    subject { worker.perform(user.id) }
 
     context 'when user is found' do
-      let(:attributes) { active_scope_attributes }
+      let(:active_attributes) do
+        {
+          state: 'active',
+          ghost: false,
+          user_type: nil
+        }
+      end
+      let(:user) { create(:user, active_attributes) }
 
-      it 'updates the highest role for the user' do
-        user_highest_role = create(:user_highest_role, user: user)
-        create(:group_member, :developer, user: user)
+      subject { worker.perform(user.id) }
 
-        expect { subject }
-          .to change { user_highest_role.reload.highest_access_level }
-          .from(nil)
-          .to(Gitlab::Access::DEVELOPER)
-      end
-    end
+      context 'when user is active and not internal' do
+        context 'when user highest role exists' do
+          it 'updates the highest role for the user' do
+            user_highest_role = create(:user_highest_role, user: user)
+            create(:group_member, :developer, user: user)
 
-    context 'when user is not found' do
-      shared_examples 'no update' do
-        it 'does not update any highest role' do
-          expect(Users::UpdateHighestMemberRoleService).not_to receive(:new)
+            expect { subject }
+              .to change { user_highest_role.reload.highest_access_level }
+              .from(nil)
+              .to(Gitlab::Access::DEVELOPER)
+          end
+        end
+
+        context 'when user highest role does not exist' do
+          it 'creates the highest role for the user' do
+            create(:group_member, :developer, user: user)
 
-          worker.perform(user.id)
+            expect { subject }.to change { UserHighestRole.count }.by(1)
+          end
         end
       end
 
-      context 'when user is blocked' do
-        let(:attributes) { active_scope_attributes.merge(state: 'blocked') }
+      context 'when user is either inactive or internal' do
+        using RSpec::Parameterized::TableSyntax
 
-        it_behaves_like 'no update'
-      end
+        where(:additional_attributes) do
+          [
+            { state: 'blocked' },
+            { ghost: true },
+            { user_type: :alert_bot }
+          ]
+        end
 
-      context 'when user is a ghost' do
-        let(:attributes) { active_scope_attributes.merge(ghost: true) }
+        with_them do
+          it 'deletes highest role' do
+            user = create(:user, active_attributes.merge(additional_attributes))
+            create(:user_highest_role, user: user)
 
-        it_behaves_like 'no update'
-      end
+            expect { worker.perform(user.id) }.to change { UserHighestRole.count }.from(1).to(0)
+          end
+        end
 
-      context 'when user has a user type' do
-        let(:attributes) { active_scope_attributes.merge(user_type: :alert_bot) }
+        context 'when user highest role does not exist' do
+          it 'does not delete a highest role' do
+            user = create(:user, state: 'blocked')
 
-        it_behaves_like 'no update'
+            expect { worker.perform(user.id) }.not_to change(UserHighestRole, :count)
+          end
+        end
       end
     end
   end
diff --git a/spec/workers/upload_checksum_worker_spec.rb b/spec/workers/upload_checksum_worker_spec.rb
index ad054f0ff72..44e9e4f048d 100644
--- a/spec/workers/upload_checksum_worker_spec.rb
+++ b/spec/workers/upload_checksum_worker_spec.rb
@@ -8,7 +8,7 @@ describe UploadChecksumWorker do
 
     context 'without a valid record' do
       it 'rescues ActiveRecord::RecordNotFound' do
-        expect { subject.perform(999_999) }.not_to raise_error
+        expect { subject.perform(non_existing_record_id) }.not_to raise_error
       end
     end