Commit 972ee565 authored by Vijay Hawoldar's avatar Vijay Hawoldar

Add error message when removing billable members

When the removal of a billable member via the API fails, we now
return a user friendly error message.

Changelog: changed
parent 9fe3efd6
# frozen_string_literal: true # frozen_string_literal: true
# BillableMembers::DestroyService class
#
# Used to find and remove all billable Member records (GroupMember or ProjectMember) # Used to find and remove all billable Member records (GroupMember or ProjectMember)
# within a group's hierarchy for the given user_id, so that a billable member can be completely # within a group's hierarchy for the given user_id, so that a billable member can be completely
# removed from the group and it's subgroups and projects # removed from the group and it's subgroups and projects
...@@ -14,6 +12,7 @@ module BillableMembers ...@@ -14,6 +12,7 @@ module BillableMembers
include BaseServiceUtility include BaseServiceUtility
InvalidGroupError = Class.new(StandardError) InvalidGroupError = Class.new(StandardError)
InvalidMemberError = Class.new(StandardError)
def initialize(group, user_id:, current_user:) def initialize(group, user_id:, current_user:)
@group = group @group = group
...@@ -28,7 +27,7 @@ module BillableMembers ...@@ -28,7 +27,7 @@ module BillableMembers
remove_user_from_resources remove_user_from_resources
success success
rescue InvalidGroupError, Gitlab::Access::AccessDeniedError => e rescue InvalidGroupError, InvalidMemberError, Gitlab::Access::AccessDeniedError => e
error(e.message) error(e.message)
end end
...@@ -38,11 +37,15 @@ module BillableMembers ...@@ -38,11 +37,15 @@ module BillableMembers
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def remove_user_from_resources def remove_user_from_resources
memberships_found = false
memberships = ::Member.in_hierarchy(group).where(user_id: user_id) memberships = ::Member.in_hierarchy(group).where(user_id: user_id)
memberships.find_each do |member| memberships.find_each do |member|
memberships_found = true
::Members::DestroyService.new(current_user).execute(member, skip_subresources: true) ::Members::DestroyService.new(current_user).execute(member, skip_subresources: true)
end end
raise InvalidMemberError, 'No member found for the given user_id' unless memberships_found
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
---
title: Return an appropriate error message when failing to remove billable members
merge_request: 60404
author:
type: changed
...@@ -112,7 +112,7 @@ module EE ...@@ -112,7 +112,7 @@ module EE
if result[:status] == :success if result[:status] == :success
no_content! no_content!
else else
bad_request!(nil) bad_request!(result[:message])
end end
end end
end end
......
...@@ -657,6 +657,16 @@ RSpec.describe API::Members do ...@@ -657,6 +657,16 @@ RSpec.describe API::Members do
it_behaves_like 'successful deletion' it_behaves_like 'successful deletion'
end end
context 'with a user that is not a member' do
it 'returns a relevant error message' do
user = create(:user)
delete api("/groups/#{group.id}/billable_members/#{user.id}", owner)
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']).to eq '400 Bad request - No member found for the given user_id'
end
end
end end
end end
end end
......
...@@ -89,6 +89,18 @@ RSpec.describe BillableMembers::DestroyService do ...@@ -89,6 +89,18 @@ RSpec.describe BillableMembers::DestroyService do
expect(multi_project_user.projects).not_to include(project_2) expect(multi_project_user.projects).not_to include(project_2)
end end
end end
context 'when the user has no Member record' do
let(:non_member) { create(:user) }
let(:user_id) { non_member.id }
it 'returns an appropriate error' do
result = execute
expect(result[:status]).to eq :error
expect(result[:message]).to eq 'No member found for the given user_id'
end
end
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment