Fix a few things after the initial improvment to Members::DestroyService
Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
parent
3158f57dba
commit
c8b1311934
9 changed files with 30 additions and 15 deletions
|
@ -15,16 +15,17 @@ module MembershipActions
|
|||
end
|
||||
|
||||
def leave
|
||||
Members::DestroyService.new(membershipable, current_user, user_id: current_user.id).execute(:all)
|
||||
member = Members::DestroyService.new(membershipable, current_user, user_id: current_user.id).
|
||||
execute(:all)
|
||||
|
||||
source_type = membershipable.class.to_s.humanize(capitalize: false)
|
||||
notice =
|
||||
if @member.request?
|
||||
if member.request?
|
||||
"Your access request to the #{source_type} has been withdrawn."
|
||||
else
|
||||
"You left the \"#{membershipable.human_name}\" #{source_type}."
|
||||
end
|
||||
redirect_path = @member.request? ? @member.source : [:dashboard, membershipable.class.to_s.tableize]
|
||||
redirect_path = member.request? ? member.source : [:dashboard, membershipable.class.to_s.tableize]
|
||||
|
||||
redirect_to redirect_path, notice: notice
|
||||
end
|
||||
|
|
|
@ -40,7 +40,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
|
|||
end
|
||||
|
||||
def destroy
|
||||
Members::DestroyService.new(@group, current_user, user_id: params[:id]).execute(:all)
|
||||
Members::DestroyService.new(@group, current_user, id: params[:id]).execute(:all)
|
||||
|
||||
respond_to do |format|
|
||||
format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' }
|
||||
|
|
|
@ -55,7 +55,8 @@ class Projects::ProjectMembersController < Projects::ApplicationController
|
|||
end
|
||||
|
||||
def destroy
|
||||
Members::DestroyService.new(@project, current_user, user_id: params[:id]).execute(:all)
|
||||
Members::DestroyService.new(@project, current_user, params).
|
||||
execute(:all)
|
||||
|
||||
respond_to do |format|
|
||||
format.html do
|
||||
|
|
|
@ -14,6 +14,8 @@ module Members
|
|||
if member.request? && member.user != user
|
||||
notification_service.decline_access_request(member)
|
||||
end
|
||||
|
||||
member
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -15,7 +15,7 @@ module Members
|
|||
def execute(scope = :members)
|
||||
raise "scope :#{scope} is not allowed!" unless ALLOWED_SCOPES.include?(scope)
|
||||
|
||||
member = find_member(scope)
|
||||
member = find_member!(scope)
|
||||
|
||||
raise Gitlab::Access::AccessDeniedError if cannot_destroy_member?(member)
|
||||
|
||||
|
@ -24,13 +24,14 @@ module Members
|
|||
|
||||
private
|
||||
|
||||
def find_member(scope)
|
||||
def find_member!(scope)
|
||||
condition = params[:user_id] ? { user_id: params[:user_id] } : { id: params[:id] }
|
||||
case scope
|
||||
when :all
|
||||
source.members.find_by(user_id: params[:user_id]) ||
|
||||
source.requesters.find_by!(user_id: params[:user_id])
|
||||
source.members.find_by(condition) ||
|
||||
source.requesters.find_by!(condition)
|
||||
else
|
||||
source.public_send(scope).find_by!(user_id: params[:user_id])
|
||||
source.public_send(scope).find_by!(condition)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -75,7 +75,8 @@ module API
|
|||
required_attributes! [:user_id]
|
||||
source = find_source(source_type, params[:id])
|
||||
|
||||
::Members::DestroyService.new(source, current_user, declared(params)).execute(:requesters)
|
||||
::Members::DestroyService.new(source, current_user, params).
|
||||
execute(:requesters)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -87,10 +87,10 @@ describe Groups::GroupMembersController do
|
|||
context 'when member is not found' do
|
||||
before { sign_in(user) }
|
||||
|
||||
it 'returns 403' do
|
||||
it 'returns 404' do
|
||||
delete :leave, group_id: group
|
||||
|
||||
expect(response).to have_http_status(403)
|
||||
expect(response).to have_http_status(404)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -135,11 +135,11 @@ describe Projects::ProjectMembersController do
|
|||
context 'when member is not found' do
|
||||
before { sign_in(user) }
|
||||
|
||||
it 'returns 403' do
|
||||
it 'returns 404' do
|
||||
delete :leave, namespace_id: project.namespace,
|
||||
project_id: project
|
||||
|
||||
expect(response).to have_http_status(403)
|
||||
expect(response).to have_http_status(404)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -98,6 +98,15 @@ describe Members::DestroyService, services: true do
|
|||
it_behaves_like 'a service destroying a member' do
|
||||
let(:source) { group }
|
||||
end
|
||||
|
||||
context 'when given a :id' do
|
||||
let(:params) { { id: project.members.find_by!(user_id: user.id).id } }
|
||||
|
||||
it 'destroys the member' do
|
||||
expect { described_class.new(project, user, params).execute }.
|
||||
to change { project.members.count }.by(-1)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue