1 files changed, 267 insertions, 89 deletions

diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb
index f919d6d1516..eb8fae03c39 100644
--- a/spec/services/members/update_service_spec.rb
+++ b/spec/services/members/update_service_spec.rb
@@ -3,18 +3,34 @@
 require 'spec_helper'
 
 RSpec.describe Members::UpdateService do
-  let(:project) { create(:project, :public) }
-  let(:group) { create(:group, :public) }
-  let(:current_user) { create(:user) }
-  let(:member_user) { create(:user) }
-  let(:permission) { :update }
-  let(:member) { source.members_and_requesters.find_by!(user_id: member_user.id) }
-  let(:access_level) { Gitlab::Access::MAINTAINER }
-  let(:params) do
-    { access_level: access_level }
+  let_it_be(:project) { create(:project, :public) }
+  let_it_be(:group) { create(:group, :public) }
+  let_it_be(:current_user) { create(:user) }
+  let_it_be(:member_user1) { create(:user) }
+  let_it_be(:member_user2) { create(:user) }
+  let_it_be(:member_users) { [member_user1, member_user2] }
+  let_it_be(:permission) { :update }
+  let_it_be(:access_level) { Gitlab::Access::MAINTAINER }
+  let(:members) { source.members_and_requesters.where(user_id: member_users).to_a }
+  let(:update_service) { described_class.new(current_user, params) }
+  let(:params) { { access_level: access_level } }
+  let(:updated_members) do
+    result = subject
+    Array.wrap(result[:members] || result[:member])
   end
 
-  subject { described_class.new(current_user, params).execute(member, permission: permission) }
+  before do
+    member_users.first.tap do |member_user|
+      expires_at = 10.days.from_now
+      project.add_member(member_user, Gitlab::Access::DEVELOPER, expires_at: expires_at)
+      group.add_member(member_user, Gitlab::Access::DEVELOPER, expires_at: expires_at)
+    end
+
+    member_users[1..].each do |member_user|
+      project.add_developer(member_user)
+      group.add_developer(member_user)
+    end
+  end
 
   shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do
     it 'raises Gitlab::Access::AccessDeniedError' do
@@ -23,164 +39,326 @@ RSpec.describe Members::UpdateService do
     end
   end
 
-  shared_examples 'a service updating a member' do
-    it 'updates the member' do
-      expect(TodosDestroyer::EntityLeaveWorker).not_to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name)
+  shared_examples 'current user cannot update the given members' do
+    it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
+      let_it_be(:source) { project }
+    end
+
+    it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
+      let_it_be(:source) { group }
+    end
+  end
+
+  shared_examples 'returns error status when params are invalid' do
+    let_it_be(:params) { { expires_at: 2.days.ago } }
 
-      updated_member = subject.fetch(:member)
+    specify do
+      expect(subject[:status]).to eq(:error)
+    end
+  end
 
-      expect(updated_member).to be_valid
-      expect(updated_member.access_level).to eq(access_level)
+  shared_examples 'a service updating members' do
+    it 'updates the members' do
+      new_access_levels = updated_members.map(&:access_level)
+
+      expect(updated_members).not_to be_empty
+      expect(updated_members).to all(be_valid)
+      expect(new_access_levels).to all(be access_level)
     end
 
     it 'returns success status' do
-      result = subject.fetch(:status)
+      expect(subject.fetch(:status)).to eq(:success)
+    end
 
-      expect(result).to eq(:success)
+    it 'invokes after_execute with correct args' do
+      members.each do |member|
+        expect(update_service).to receive(:after_execute).with(
+          action: permission,
+          old_access_level: member.human_access,
+          old_expiry: member.expires_at,
+          member: member
+        )
+      end
+
+      subject
     end
 
-    context 'when member is downgraded to guest' do
-      shared_examples 'schedules to delete confidential todos' do
-        it do
-          expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name).once
+    it 'authorization update callback is triggered' do
+      expect(members).to all(receive(:refresh_member_authorized_projects).once)
 
-          updated_member = subject.fetch(:member)
+      subject
+    end
+
+    it 'does not enqueues todos for deletion' do
+      members.each do |member|
+        expect(TodosDestroyer::EntityLeaveWorker)
+          .not_to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name)
+      end
 
-          expect(updated_member).to be_valid
-          expect(updated_member.access_level).to eq(Gitlab::Access::GUEST)
+      subject
+    end
+
+    context 'when members are downgraded to guest' do
+      shared_examples 'schedules to delete confidential todos' do
+        it do
+          members.each do |member|
+            expect(TodosDestroyer::EntityLeaveWorker)
+              .to receive(:perform_in)
+                    .with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name).once
+          end
+
+          new_access_levels = updated_members.map(&:access_level)
+          expect(updated_members).to all(be_valid)
+          expect(new_access_levels).to all(be Gitlab::Access::GUEST)
         end
       end
 
       context 'with Gitlab::Access::GUEST level as a string' do
-        let(:params) { { access_level: Gitlab::Access::GUEST.to_s } }
+        let_it_be(:params) { { access_level: Gitlab::Access::GUEST.to_s } }
 
         it_behaves_like 'schedules to delete confidential todos'
       end
 
       context 'with Gitlab::Access::GUEST level as an integer' do
-        let(:params) { { access_level: Gitlab::Access::GUEST } }
+        let_it_be(:params) { { access_level: Gitlab::Access::GUEST } }
 
         it_behaves_like 'schedules to delete confidential todos'
       end
     end
 
     context 'when access_level is invalid' do
-      let(:params) { { access_level: 'invalid' } }
+      let_it_be(:params) { { access_level: 'invalid' } }
 
       it 'raises an error' do
-        expect { described_class.new(current_user, params) }.to raise_error(ArgumentError, 'invalid value for Integer(): "invalid"')
+        expect { described_class.new(current_user, params) }
+          .to raise_error(ArgumentError, 'invalid value for Integer(): "invalid"')
       end
     end
 
-    context 'when member is not valid' do
-      let(:params) { { expires_at: 2.days.ago } }
+    context 'when members update results in no change' do
+      let(:params) { { access_level: members.first.access_level } }
 
-      it 'returns error status' do
-        result = subject
+      it 'does not invoke update! and post_update' do
+        expect(update_service).not_to receive(:save!)
+        expect(update_service).not_to receive(:post_update)
 
-        expect(result[:status]).to eq(:error)
+        expect(subject[:status]).to eq(:success)
       end
-    end
-  end
-
-  before do
-    project.add_developer(member_user)
-    group.add_developer(member_user)
-  end
 
-  context 'when current user cannot update the given member' do
-    it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
-      let(:source) { project }
-    end
+      it 'authorization update callback is not triggered' do
+        members.each { |member| expect(member).not_to receive(:refresh_member_authorized_projects) }
 
-    it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
-      let(:source) { group }
+        subject
+      end
     end
   end
 
-  context 'when current user can update the given member' do
+  shared_examples 'updating a project' do
+    let_it_be(:group_project) { create(:project, group: create(:group)) }
+    let_it_be(:source) { group_project }
+
     before do
-      project.add_maintainer(current_user)
-      group.add_owner(current_user)
+      member_users.each { |member_user| group_project.add_developer(member_user) }
     end
 
-    it_behaves_like 'a service updating a member' do
-      let(:source) { project }
-    end
+    context 'as a project maintainer' do
+      before do
+        group_project.add_maintainer(current_user)
+      end
 
-    it_behaves_like 'a service updating a member' do
-      let(:source) { group }
-    end
-  end
+      it_behaves_like 'a service updating members'
 
-  context 'in a project' do
-    let_it_be(:group_project) { create(:project, group: create(:group)) }
+      context 'when member update results in an error' do
+        it_behaves_like 'a service returning an error'
+      end
 
-    let(:source) { group_project }
+      context 'and updating members to OWNER' do
+        it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
+          let_it_be(:access_level) { Gitlab::Access::OWNER }
+        end
+      end
 
-    context 'a project maintainer' do
-      before do
-        group_project.add_maintainer(current_user)
+      context 'and updating themselves to OWNER' do
+        let(:members) { source.members_and_requesters.find_by!(user_id: current_user.id) }
+
+        it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
+          let_it_be(:access_level) { Gitlab::Access::OWNER }
+        end
       end
 
-      context 'cannot update a member to OWNER' do
+      context 'and downgrading members from OWNER' do
         before do
-          group_project.add_developer(member_user)
+          member_users.each { |member_user| group_project.add_owner(member_user) }
         end
 
         it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
-          let(:access_level) { Gitlab::Access::OWNER }
+          let_it_be(:access_level) { Gitlab::Access::MAINTAINER }
         end
       end
+    end
 
-      context 'cannot update themselves to OWNER' do
-        let(:member) { source.members_and_requesters.find_by!(user_id: current_user.id) }
+    context 'when current_user is considered an owner in the project via inheritance' do
+      before do
+        group_project.group.add_owner(current_user)
+      end
 
+      context 'and can update members to OWNER' do
         before do
-          group_project.add_developer(member_user)
+          member_users.each { |member_user| group_project.add_developer(member_user) }
         end
 
-        it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
-          let(:access_level) { Gitlab::Access::OWNER }
+        it_behaves_like 'a service updating members' do
+          let_it_be(:access_level) { Gitlab::Access::OWNER }
         end
       end
 
-      context 'cannot downgrade a member from OWNER' do
+      context 'and can downgrade members from OWNER' do
         before do
-          group_project.add_owner(member_user)
+          member_users.each { |member_user| group_project.add_owner(member_user) }
         end
 
-        it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
-          let(:access_level) { Gitlab::Access::MAINTAINER }
+        it_behaves_like 'a service updating members' do
+          let_it_be(:access_level) { Gitlab::Access::MAINTAINER }
         end
       end
     end
+  end
+
+  shared_examples 'updating a group' do
+    let_it_be(:source) { group }
+
+    before do
+      group.add_owner(current_user)
+    end
+
+    it_behaves_like 'a service updating members'
+
+    context 'when member update results in an error' do
+      it_behaves_like 'a service returning an error'
+    end
+
+    context 'when group members expiration date is updated' do
+      let_it_be(:params) { { expires_at: 20.days.from_now } }
+      let(:notification_service) { instance_double(NotificationService) }
 
-    context 'owners' do
       before do
-        # so that `current_user` is considered an `OWNER` in the project via inheritance.
-        group_project.group.add_owner(current_user)
+        allow(NotificationService).to receive(:new).and_return(notification_service)
       end
 
-      context 'can update a member to OWNER' do
-        before do
-          group_project.add_developer(member_user)
+      it 'emails the users that their group membership expiry has changed' do
+        members.each do |member|
+          expect(notification_service).to receive(:updated_group_member_expiration).with(member)
         end
 
-        it_behaves_like 'a service updating a member' do
-          let(:access_level) { Gitlab::Access::OWNER }
-        end
+        subject
       end
+    end
+  end
 
-      context 'can downgrade a member from OWNER' do
-        before do
-          group_project.add_owner(member_user)
+  context 'when :bulk_update_membership_roles feature flag is disabled' do
+    let(:member) { source.members_and_requesters.find_by!(user_id: member_user1.id) }
+    let(:members) { [member] }
+
+    subject { update_service.execute(member, permission: permission) }
+
+    shared_examples 'a service returning an error' do
+      before do
+        allow(member).to receive(:save) do
+          member.errors.add(:user_id)
+          member.errors.add(:access_level)
         end
+          .and_return(false)
+      end
+
+      it_behaves_like 'returns error status when params are invalid'
+
+      it 'returns the error' do
+        response = subject
+
+        expect(response[:status]).to eq(:error)
+        expect(response[:message]).to eq('User is invalid and Access level is invalid')
+      end
+    end
+
+    before do
+      stub_feature_flags(bulk_update_membership_roles: false)
+    end
+
+    it_behaves_like 'current user cannot update the given members'
+    it_behaves_like 'updating a project'
+    it_behaves_like 'updating a group'
+  end
+
+  subject { update_service.execute(members, permission: permission) }
+
+  shared_examples 'a service returning an error' do
+    it_behaves_like 'returns error status when params are invalid'
 
-        it_behaves_like 'a service updating a member' do
-          let(:access_level) { Gitlab::Access::MAINTAINER }
+    context 'when a member update results in invalid record' do
+      let(:member2) { members.second }
+
+      before do
+        allow(member2).to receive(:save!) do
+          member2.errors.add(:user_id)
+          member2.errors.add(:access_level)
+        end.and_raise(ActiveRecord::RecordInvalid)
+      end
+
+      it 'returns the error' do
+        response = subject
+
+        expect(response[:status]).to eq(:error)
+        expect(response[:message]).to eq('User is invalid and Access level is invalid')
+      end
+
+      it 'rollbacks back the entire update' do
+        old_access_levels = members.pluck(:access_level)
+
+        subject
+
+        expect(members.each(&:reset).pluck(:access_level)).to eq(old_access_levels)
+      end
+    end
+  end
+
+  it_behaves_like 'current user cannot update the given members'
+  it_behaves_like 'updating a project'
+  it_behaves_like 'updating a group'
+
+  context 'with a single member' do
+    let(:member) { create(:group_member, group: group) }
+    let(:members) { member }
+
+    before do
+      group.add_owner(current_user)
+    end
+
+    it 'returns the correct response' do
+      expect(subject[:member]).to eq(member)
+    end
+  end
+
+  context 'when current user is an admin', :enable_admin_mode do
+    let_it_be(:current_user) { create(:admin) }
+    let_it_be(:source) { group }
+
+    context 'when all owners are being downgraded' do
+      before do
+        member_users.each { |member_user| group.add_owner(member_user) }
+      end
+
+      it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError'
+    end
+
+    context 'when all blocked owners are being downgraded' do
+      before do
+        member_users.each do |member_user|
+          group.add_owner(member_user)
+          member_user.block
         end
       end
+
+      it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError'
     end
   end
 end