Add latest changes from gitlab-org/gitlab@13-7-stable-eev13.7.0-rc42

3 files changed, 90 insertions, 99 deletions

diff --git a/spec/services/members/approve_access_request_service_spec.rb b/spec/services/members/approve_access_request_service_spec.rb
index e6a94fdaf84..f7fbac612ee 100644
--- a/spec/services/members/approve_access_request_service_spec.rb
+++ b/spec/services/members/approve_access_request_service_spec.rb
@@ -12,23 +12,23 @@ RSpec.describe Members::ApproveAccessRequestService do
 
   shared_examples 'a service raising ActiveRecord::RecordNotFound' do
     it 'raises ActiveRecord::RecordNotFound' do
-      expect { described_class.new(current_user).execute(access_requester, opts) }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { described_class.new(current_user).execute(access_requester, **opts) }.to raise_error(ActiveRecord::RecordNotFound)
     end
   end
 
   shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do
     it 'raises Gitlab::Access::AccessDeniedError' do
-      expect { described_class.new(current_user).execute(access_requester, opts) }.to raise_error(Gitlab::Access::AccessDeniedError)
+      expect { described_class.new(current_user).execute(access_requester, **opts) }.to raise_error(Gitlab::Access::AccessDeniedError)
     end
   end
 
   shared_examples 'a service approving an access request' do
     it 'succeeds' do
-      expect { described_class.new(current_user).execute(access_requester, opts) }.to change { source.requesters.count }.by(-1)
+      expect { described_class.new(current_user).execute(access_requester, **opts) }.to change { source.requesters.count }.by(-1)
     end
 
     it 'returns a <Source>Member' do
-      member = described_class.new(current_user).execute(access_requester, opts)
+      member = described_class.new(current_user).execute(access_requester, **opts)
 
       expect(member).to be_a "#{source.class}Member".constantize
       expect(member.requested_at).to be_nil
@@ -36,7 +36,7 @@ RSpec.describe Members::ApproveAccessRequestService do
 
     context 'with a custom access level' do
       it 'returns a ProjectMember with the custom access level' do
-        member = described_class.new(current_user, access_level: Gitlab::Access::MAINTAINER).execute(access_requester, opts)
+        member = described_class.new(current_user, access_level: Gitlab::Access::MAINTAINER).execute(access_requester, **opts)
 
         expect(member.access_level).to eq(Gitlab::Access::MAINTAINER)
       end
diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb
index 00b5ff59e48..e8a4a798b20 100644
--- a/spec/services/members/create_service_spec.rb
+++ b/spec/services/members/create_service_spec.rb
@@ -3,59 +3,68 @@
 require 'spec_helper'
 
 RSpec.describe Members::CreateService do
-  let(:project) { create(:project) }
-  let(:user) { create(:user) }
-  let(:project_user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project_user) { create(:user) }
+  let_it_be(:user_ids) { project_user.id.to_s }
+  let_it_be(:access_level) { Gitlab::Access::GUEST }
+  let(:params) { { user_ids: user_ids, access_level: access_level } }
+
+  subject(:execute_service) { described_class.new(user, params).execute(project) }
 
   before do
     project.add_maintainer(user)
+    allow(Namespaces::OnboardingUserAddedWorker).to receive(:idempotent?).and_return(false)
   end
 
-  it 'adds user to members' do
-    params = { user_ids: project_user.id.to_s, access_level: Gitlab::Access::GUEST }
-    result = described_class.new(user, params).execute(project)
-
-    expect(result[:status]).to eq(:success)
-    expect(project.users).to include project_user
+  context 'when passing valid parameters' do
+    it 'adds a user to members' do
+      expect(execute_service[:status]).to eq(:success)
+      expect(project.users).to include project_user
+      expect(Namespaces::OnboardingUserAddedWorker.jobs.last['args'][0]).to eq(project.id)
+    end
   end
 
-  it 'adds no user to members' do
-    params = { user_ids: '', access_level: Gitlab::Access::GUEST }
-    result = described_class.new(user, params).execute(project)
+  context 'when passing no user ids' do
+    let(:user_ids) { '' }
 
-    expect(result[:status]).to eq(:error)
-    expect(result[:message]).to be_present
-    expect(project.users).not_to include project_user
+    it 'does not add a member' do
+      expect(execute_service[:status]).to eq(:error)
+      expect(execute_service[:message]).to be_present
+      expect(project.users).not_to include project_user
+      expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0)
+    end
   end
 
-  it 'limits the number of users to 100' do
-    user_ids = 1.upto(101).to_a.join(',')
-    params = { user_ids: user_ids, access_level: Gitlab::Access::GUEST }
+  context 'when passing many user ids' do
+    let(:user_ids) { 1.upto(101).to_a.join(',') }
 
-    result = described_class.new(user, params).execute(project)
-
-    expect(result[:status]).to eq(:error)
-    expect(result[:message]).to be_present
-    expect(project.users).not_to include project_user
+    it 'limits the number of users to 100' do
+      expect(execute_service[:status]).to eq(:error)
+      expect(execute_service[:message]).to be_present
+      expect(project.users).not_to include project_user
+      expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0)
+    end
   end
 
-  it 'does not add an invalid member' do
-    params = { user_ids: project_user.id.to_s, access_level: -1 }
-    result = described_class.new(user, params).execute(project)
+  context 'when passing an invalid access level' do
+    let(:access_level) { -1 }
 
-    expect(result[:status]).to eq(:error)
-    expect(result[:message]).to include("#{project_user.username}: Access level is not included in the list")
-    expect(project.users).not_to include project_user
+    it 'does not add a member' do
+      expect(execute_service[:status]).to eq(:error)
+      expect(execute_service[:message]).to include("#{project_user.username}: Access level is not included in the list")
+      expect(project.users).not_to include project_user
+      expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0)
+    end
   end
 
-  it 'does not add a member with an existing invite' do
-    invited_member = create(:project_member, :invited, project: project)
-
-    params = { user_ids: invited_member.invite_email,
-               access_level: Gitlab::Access::GUEST }
-    result = described_class.new(user, params).execute(project)
+  context 'when passing an existing invite user id' do
+    let(:user_ids) { create(:project_member, :invited, project: project).invite_email }
 
-    expect(result[:status]).to eq(:error)
-    expect(result[:message]).to eq('Invite email has already been taken')
+    it 'does not add a member' do
+      expect(execute_service[:status]).to eq(:error)
+      expect(execute_service[:message]).to eq('Invite email has already been taken')
+      expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0)
+    end
   end
 end
diff --git a/spec/services/members/invitation_reminder_email_service_spec.rb b/spec/services/members/invitation_reminder_email_service_spec.rb
index 88280869476..768a8719d54 100644
--- a/spec/services/members/invitation_reminder_email_service_spec.rb
+++ b/spec/services/members/invitation_reminder_email_service_spec.rb
@@ -9,67 +9,49 @@ RSpec.describe Members::InvitationReminderEmailService do
     let_it_be(:frozen_time) { Date.today.beginning_of_day }
     let_it_be(:invitation) { build(:group_member, :invited, created_at: frozen_time) }
 
-    context 'when the experiment is disabled' do
-      before do
-        allow(Gitlab::Experimentation).to receive(:enabled_for_attribute?).and_return(false)
-        invitation.expires_at = frozen_time + 2.days
-      end
-
-      it 'does not send an invitation' do
-        travel_to(frozen_time + 1.day) do
-          expect(invitation).not_to receive(:send_invitation_reminder)
-
-          subject
-        end
-      end
+    before do
+      invitation.expires_at = frozen_time + expires_at_days.days if expires_at_days
     end
 
-    context 'when the experiment is enabled' do
-      before do
-        allow(Gitlab::Experimentation).to receive(:enabled_for_attribute?).and_return(true)
-        invitation.expires_at = frozen_time + expires_at_days.days if expires_at_days
-      end
-
-      using RSpec::Parameterized::TableSyntax
-
-      where(:expires_at_days, :send_reminder_at_days) do
-        0   | []
-        1   | []
-        2   | [1]
-        3   | [1, 2]
-        4   | [1, 2, 3]
-        5   | [1, 2, 4]
-        6   | [1, 3, 5]
-        7   | [1, 3, 5]
-        8   | [2, 3, 6]
-        9   | [2, 4, 7]
-        10  | [2, 4, 8]
-        11  | [2, 4, 8]
-        12  | [2, 5, 9]
-        13  | [2, 5, 10]
-        14  | [2, 5, 10]
-        15  | [2, 5, 10]
-        nil | [2, 5, 10]
-      end
-
-      with_them do
-        # Create an invitation today with an expiration date from 0 to 10 days in the future or without an expiration date
-        # We chose 10 days here, because we fetch invitations that were created at most 10 days ago.
-        (0..10).each do |day|
-          it 'sends an invitation reminder only on the expected days' do
-            next if day > (expires_at_days || 10) # We don't need to test after the invitation has already expired
-
-            # We are traveling in a loop from today to 10 days from now
-            travel_to(frozen_time + day.days) do
-              # Given an expiration date and the number of days after the creation of the invitation based on the current day in the loop, a reminder may be sent
-              if (reminder_index = send_reminder_at_days.index(day))
-                expect(invitation).to receive(:send_invitation_reminder).with(reminder_index)
-              else
-                expect(invitation).not_to receive(:send_invitation_reminder)
-              end
+    using RSpec::Parameterized::TableSyntax
+
+    where(:expires_at_days, :send_reminder_at_days) do
+      0   | []
+      1   | []
+      2   | [1]
+      3   | [1, 2]
+      4   | [1, 2, 3]
+      5   | [1, 2, 4]
+      6   | [1, 3, 5]
+      7   | [1, 3, 5]
+      8   | [2, 3, 6]
+      9   | [2, 4, 7]
+      10  | [2, 4, 8]
+      11  | [2, 4, 8]
+      12  | [2, 5, 9]
+      13  | [2, 5, 10]
+      14  | [2, 5, 10]
+      15  | [2, 5, 10]
+      nil | [2, 5, 10]
+    end
 
-              subject
+    with_them do
+      # Create an invitation today with an expiration date from 0 to 10 days in the future or without an expiration date
+      # We chose 10 days here, because we fetch invitations that were created at most 10 days ago.
+      (0..10).each do |day|
+        it 'sends an invitation reminder only on the expected days' do
+          next if day > (expires_at_days || 10) # We don't need to test after the invitation has already expired
+
+          # We are traveling in a loop from today to 10 days from now
+          travel_to(frozen_time + day.days) do
+            # Given an expiration date and the number of days after the creation of the invitation based on the current day in the loop, a reminder may be sent
+            if (reminder_index = send_reminder_at_days.index(day))
+              expect(invitation).to receive(:send_invitation_reminder).with(reminder_index)
+            else
+              expect(invitation).not_to receive(:send_invitation_reminder)
             end
+
+            subject
           end
         end
       end