Merge branch 'vij-delete-billable-error' into 'master'

Return an appropriate error when removing billable members

See merge request gitlab-org/gitlab!60404

ee/app/services/billable_members/destroy_service.rb

 # frozen_string_literal: true
 
-# BillableMembers::DestroyService class
-#
 # 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
 # removed from the group and it's subgroups and projects
@@ -14,6 +12,7 @@ module BillableMembers
     include BaseServiceUtility
 
     InvalidGroupError = Class.new(StandardError)
+    InvalidMemberError = Class.new(StandardError)
 
     def initialize(group, user_id:, current_user:)
       @group = group
@@ -28,7 +27,7 @@ module BillableMembers
       remove_user_from_resources
 
       success
-    rescue InvalidGroupError, Gitlab::Access::AccessDeniedError => e
+    rescue InvalidGroupError, InvalidMemberError, Gitlab::Access::AccessDeniedError => e
       error(e.message)
     end
 
@@ -38,11 +37,15 @@ module BillableMembers
 
     # rubocop: disable CodeReuse/ActiveRecord
     def remove_user_from_resources
+      memberships_found = false
       memberships = ::Member.in_hierarchy(group).where(user_id: user_id)
 
       memberships.find_each do |member|
+        memberships_found = true
         ::Members::DestroyService.new(current_user).execute(member, skip_subresources: true)
       end
+
+      raise InvalidMemberError, 'No member found for the given user_id' unless memberships_found
     end
     # rubocop: enable CodeReuse/ActiveRecord
 

ee/changelogs/unreleased/vij-delete-billable-error.yml

+---
+title: Return an appropriate error message when failing to remove billable members
+merge_request: 60404
+author:
+type: changed

ee/lib/ee/api/members.rb

@@ -112,7 +112,7 @@ module EE
             if result[:status] == :success
               no_content!
             else
-              bad_request!(nil)
+              bad_request!(result[:message])
             end
           end
         end

ee/spec/requests/api/members_spec.rb

@@ -657,6 +657,16 @@ RSpec.describe API::Members do
 
           it_behaves_like 'successful deletion'
         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

ee/spec/services/billable_members/destroy_service_spec.rb

@@ -89,6 +89,18 @@ RSpec.describe BillableMembers::DestroyService do
           expect(multi_project_user.projects).not_to include(project_2)
         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