diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb
index f597124357f74ba453cc7248fc67fa603ec1a623..c3f743ced5bb1968bf9448507fadb7a9d5ae8af0 100644
--- a/app/controllers/concerns/membership_actions.rb
+++ b/app/controllers/concerns/membership_actions.rb
@@ -133,10 +133,6 @@ def plain_source_type
raise NotImplementedError
end
- def source
- raise NotImplementedError
- end
-
def members_and_requesters
membershipable.members_and_requesters
end
@@ -146,7 +142,7 @@ def requesters
end
def update_params
- params.require(root_params_key).permit(:access_level, :expires_at).merge({ source: source })
+ params.require(root_params_key).permit(:access_level, :expires_at)
end
def requested_relations(inherited_permissions = :with_inherited_permissions)
diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb
index 47c1f07acd6b3531ec0e93866a4f7e11fc4d1149..2f8efedbc268c7a335b5e7cbc46af2e9de9113f1 100644
--- a/app/controllers/groups/group_members_controller.rb
+++ b/app/controllers/groups/group_members_controller.rb
@@ -115,10 +115,6 @@ def source_type
_("group")
end
- def source
- group
- end
-
def members_page_url
polymorphic_url([group, :members])
end
diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index b0bfce96f6ce850924fb348d1688dfdd02d7d8d3..168ae402898de1507fa2ae3396a7167413d0f103 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -58,10 +58,6 @@ def source_type
_("project")
end
- def source
- project
- end
-
def members_page_url
project_project_members_path(project)
end
diff --git a/app/graphql/mutations/members/bulk_update_base.rb b/app/graphql/mutations/members/bulk_update_base.rb
index 0a54597f6c638525ee7e3a7f000699cec6f35ce7..fe24e6b48a6035f30a4927bffa2404f638fa8715 100644
--- a/app/graphql/mutations/members/bulk_update_base.rb
+++ b/app/graphql/mutations/members/bulk_update_base.rb
@@ -26,10 +26,8 @@ class BulkUpdateBase < BaseMutation
INVALID_MEMBERS_ERROR = 'Only access level of direct members can be updated.'
def resolve(**args)
- source = authorized_find!(source_id: args[source_id_param_name])
-
result = ::Members::UpdateService
- .new(current_user, args.except(:user_ids, source_id_param_name).merge({ source: source }))
+ .new(current_user, args.except(:user_ids, source_id_param_name))
.execute(@updatable_members)
present_result(result)
diff --git a/app/services/members/update_service.rb b/app/services/members/update_service.rb
index a13a96910846d6b4bf704c9549ddcc11d49d80de..e126f062be14dc55d6c2d2194c7a3b1461af5ccc 100644
--- a/app/services/members/update_service.rb
+++ b/app/services/members/update_service.rb
@@ -2,17 +2,9 @@
module Members
class UpdateService < Members::BaseService
- def initialize(*args)
- super
-
- @source = params[:source]
- end
-
# @param members [Member, Array<Member>]
# returns the updated member(s)
def execute(members, permission: :update)
- validate_source_type!
-
members = Array.wrap(members)
old_access_level_expiry_map = members.to_h do |member|
@@ -32,12 +24,6 @@ def execute(members, permission: :update)
private
- attr_reader :source
-
- def validate_source_type!
- raise "Unknown source type: #{source.class}!" unless source.is_a?(Group) || source.is_a?(Project)
- end
-
def update_members(members, permission)
# `filter_map` avoids the `post_update` call for the member that resulted in no change
Member.transaction do
diff --git a/ee/app/controllers/ee/groups/group_members_controller.rb b/ee/app/controllers/ee/groups/group_members_controller.rb
index 93917a7fa0a46055feed95cd8a3b4f65e8c000a8..6b46743fc371426b9645a6bbf65463bd80d22ab3 100644
--- a/ee/app/controllers/ee/groups/group_members_controller.rb
+++ b/ee/app/controllers/ee/groups/group_members_controller.rb
@@ -141,7 +141,7 @@ def authorize_ban_group_member!
end
def override_params
- params.require(:group_member).permit(:override).merge({ source: source })
+ params.require(:group_member).permit(:override)
end
override :membershipable_members
diff --git a/ee/lib/ee/api/members.rb b/ee/lib/ee/api/members.rb
index a16d815227f2173691a0ab5426ee798567a8fcd4..d19c71139f59043f6b97d1a920e346e8bc87973b 100644
--- a/ee/lib/ee/api/members.rb
+++ b/ee/lib/ee/api/members.rb
@@ -17,11 +17,10 @@ module Members
requires :user_id, type: Integer, desc: 'The user ID of the member'
end
post ":id/members/:user_id/override", feature_category: :user_management do
- group = find_group!(params[:id])
member = find_member(params)
result = ::Members::UpdateService
- .new(current_user, { override: true, source: group })
+ .new(current_user, { override: true })
.execute(member, permission: :override)
updated_member = result[:members].first
@@ -40,11 +39,10 @@ module Members
requires :user_id, type: Integer, desc: 'The user ID of the member'
end
delete ":id/members/:user_id/override", feature_category: :user_management do
- group = find_group!(params[:id])
member = find_member(params)
result = ::Members::UpdateService
- .new(current_user, { override: false, source: group })
+ .new(current_user, { override: false })
.execute(member, permission: :override)
updated_member = result[:members].first
diff --git a/ee/spec/services/ee/members/update_service_spec.rb b/ee/spec/services/ee/members/update_service_spec.rb
index 889a640338ccc8bfeec2ed84f50a8a5cfe3747d4..8ca6c7723839664daa4c786cd131f0d177a4ce13 100644
--- a/ee/spec/services/ee/members/update_service_spec.rb
+++ b/ee/spec/services/ee/members/update_service_spec.rb
@@ -31,8 +31,7 @@
let(:current_user) { user }
let(:member) { group_member }
- let(:source) { group }
- let(:params) { { access_level: new_access_level, expires_at: new_expiration, source: source } }
+ let(:params) { { access_level: new_access_level, expires_at: new_expiration } }
let(:service) { described_class.new(current_user, params) }
@@ -178,7 +177,6 @@
end
it_behaves_like 'does not log an audit event' do
- let(:source) { project }
let(:member) { project_member }
end
end
@@ -213,7 +211,7 @@
let_it_be(:member_role_guest) { create(:member_role, :guest, namespace: group) }
let_it_be(:member_role_reporter) { create(:member_role, :reporter, namespace: group) }
- let(:params) { { member_role_id: target_member_role&.id, source: source } }
+ let(:params) { { member_role_id: target_member_role&.id } }
before do
stub_licensed_features(custom_roles: true)
@@ -279,9 +277,7 @@
end
context 'when invalid access_level is provided' do
- let(:params) do
- { member_role_id: target_member_role&.id, access_level: GroupMember::DEVELOPER, source: source }
- end
+ let(:params) { { member_role_id: target_member_role&.id, access_level: GroupMember::DEVELOPER } }
it 'returns error' do
expect(update_member[:status]).to eq(:error)
@@ -311,7 +307,7 @@
create(:member_role, namespace: root_ancestor, admin_group_member: true)
end
- let(:params) { { access_level: role, source: source } }
+ let(:params) { { access_level: role } }
shared_examples 'updating members using custom permission' do
let_it_be(:member, reload: true) do
diff --git a/lib/api/invitations.rb b/lib/api/invitations.rb
index 10eedacf5516655dda81dd7fb434ae48ea58eb75..c10c7ff59e5ea5ca55abea3f6c853775cb15acad 100644
--- a/lib/api/invitations.rb
+++ b/lib/api/invitations.rb
@@ -101,7 +101,7 @@ class Invitations < ::API::Base
bad_request! unless update_params.any?
result = ::Members::UpdateService
- .new(current_user, update_params.merge({ source: source }))
+ .new(current_user, update_params)
.execute(invite)
updated_member = result[:members].first
diff --git a/lib/api/members.rb b/lib/api/members.rb
index e56c8f548230be706019f8fdfdf083a115aa0dd4..a72b8958038818aa0d9254c79651f26985d57ddc 100644
--- a/lib/api/members.rb
+++ b/lib/api/members.rb
@@ -149,7 +149,7 @@ class Members < ::API::Base
authorize_update_source_member!(source_type, member)
result = ::Members::UpdateService
- .new(current_user, declared_params(include_missing: false).merge({ source: source }))
+ .new(current_user, declared_params(include_missing: false))
.execute(member)
present_put_membership_response(result)
diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb
index 5a86b6706bd9171b0ae9225aacb3c8cf55e1d75a..d75e6da0fcf3318db6c9d4c7a3e83f70b9a4d91c 100644
--- a/spec/requests/api/invitations_spec.rb
+++ b/spec/requests/api/invitations_spec.rb
@@ -661,15 +661,13 @@ def invite_api(source, user, email)
end
end
- describe 'PUT /groups/:id/invitations' do
- let(:source) { group }
-
+ shared_examples 'PUT /:source_type/:id/invitations/:email' do |source_type|
def update_api(source, user, email)
api("/#{source.model_name.plural}/#{source.id}/invitations/#{email}", user)
end
- context "with :source_type == 'groups'" do
- let!(:invite) { invite_member_by_email(source, 'group', developer.email, maintainer) }
+ context "with :source_type == #{source_type.pluralize}" do
+ let!(:invite) { invite_member_by_email(source, source_type, developer.email, maintainer) }
it_behaves_like 'a 404 response when source is private' do
let(:route) do
@@ -738,7 +736,7 @@ def update_api(source, user, email)
end
context 'updating access expiry date' do
- subject(:put_request) do
+ subject do
put update_api(source, maintainer, invite.invite_email), params: { expires_at: expires_at }
end
@@ -746,7 +744,7 @@ def update_api(source, user, email)
let(:expires_at) { 2.days.ago.to_date }
it 'does not update the member' do
- put_request
+ subject
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']).to eq({ 'expires_at' => ['cannot be a date in the past'] })
@@ -757,7 +755,7 @@ def update_api(source, user, email)
let(:expires_at) { 2.days.from_now.to_date }
it 'updates the member' do
- put_request
+ subject
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['expires_at']).to eq(expires_at.to_s)
@@ -766,4 +764,10 @@ def update_api(source, user, email)
end
end
end
+
+ describe 'PUT /groups/:id/invitations' do
+ it_behaves_like 'PUT /:source_type/:id/invitations/:email', 'group' do
+ let(:source) { group }
+ end
+ end
end
diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb
index 1382095e84afc66c5fb6a6c5f616cbc861a6b0f2..0ebe6503a9d572c10ec74e0ab08ee9feea598ac4 100644
--- a/spec/services/members/update_service_spec.rb
+++ b/spec/services/members/update_service_spec.rb
@@ -13,7 +13,7 @@
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, source: source } }
+ let(:params) { { access_level: access_level } }
let(:updated_members) { subject[:members] }
before do
@@ -47,7 +47,7 @@
end
shared_examples 'returns error status when params are invalid' do
- let_it_be(:params) { { expires_at: 2.days.ago, source: source } }
+ let_it_be(:params) { { expires_at: 2.days.ago } }
specify do
expect(subject[:status]).to eq(:error)
@@ -111,20 +111,20 @@
end
context 'with Gitlab::Access::GUEST level as a string' do
- let_it_be(:params) { { access_level: Gitlab::Access::GUEST.to_s, source: source } }
+ 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_it_be(:params) { { access_level: Gitlab::Access::GUEST, source: source } }
+ 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_it_be(:params) { { access_level: 'invalid', source: source } }
+ let_it_be(:params) { { access_level: 'invalid' } }
it 'raises an error' do
expect { described_class.new(current_user, params) }
@@ -133,7 +133,7 @@
end
context 'when members update results in no change' do
- let(:params) { { access_level: members.first.access_level, source: source } }
+ let(:params) { { access_level: members.first.access_level } }
it 'does not invoke update! and post_update' do
expect(update_service).not_to receive(:save!)
@@ -221,7 +221,7 @@
end
context 'when project members expiration date is updated with expiry_notified_at' do
- let_it_be(:params) { { expires_at: 20.days.from_now, source: source } }
+ let_it_be(:params) { { expires_at: 20.days.from_now } }
before do
group_project.group.add_owner(current_user)
@@ -254,7 +254,7 @@
end
context 'when group members expiration date is updated' do
- let_it_be(:params) { { expires_at: 20.days.from_now, source: source } }
+ let_it_be(:params) { { expires_at: 20.days.from_now } }
let(:notification_service) { instance_double(NotificationService) }
before do
@@ -271,7 +271,7 @@
end
context 'when group members expiration date is updated with expiry_notified_at' do
- let_it_be(:params) { { expires_at: 20.days.from_now, source: source } }
+ let_it_be(:params) { { expires_at: 20.days.from_now } }
before do
members.each do |member|
@@ -321,20 +321,11 @@
end
end
- context 'when passing an invalid source' do
- let_it_be(:source) { Object.new }
-
- it 'raises a RuntimeError' do
- expect { update_service.execute([]) }.to raise_error(RuntimeError, 'Unknown source type: Object!')
- 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_it_be(:source) { group }
let(:members) { create(:group_member, group: group) }
before do