Applied maintainer reviewer recommendation

Reorganized code based on review.

app/models/members/group_member.rb

@@ -22,7 +22,6 @@ class GroupMember < Member
   scope :of_ldap_type, -> { where(ldap: true) }
   scope :count_users_by_group_id, -> { group(:source_id).count }
   scope :with_user, -> (user) { where(user: user) }
-  scope :get_user_id, -> { pluck(:user_id) }
 
   after_create :update_two_factor_requirement, unless: :invite?
   after_destroy :update_two_factor_requirement, unless: :invite?

ee/app/models/ee/group.rb

@@ -8,7 +8,6 @@ module EE
   module Group
     extend ActiveSupport::Concern
     extend ::Gitlab::Utils::Override
-    include ::SortingHelper
 
     prepended do
       include TokenAuthenticatable
@@ -303,16 +302,16 @@ module EE
       if [actual_plan_name, requested_hosted_plan].include?(::Plan::GOLD)
         strong_memoize(:gold_billed_user_ids) do
           (billed_group_members.non_guests.distinct.pluck(:user_id) +
-           billed_project_members.non_guests.distinct.pluck(:user_id) +
-           billed_shared_non_guests_group_members.non_guests.distinct.pluck(:user_id) +
-           billed_invited_non_guests_group_to_project_members.non_guests.distinct.pluck(:user_id)).to_set
+          billed_project_members.non_guests.distinct.pluck(:user_id) +
+          billed_shared_non_guests_group_members.non_guests.distinct.pluck(:user_id) +
+          billed_invited_non_guests_group_to_project_members.non_guests.distinct.pluck(:user_id)).to_set
         end
       else
         strong_memoize(:non_gold_billed_user_ids) do
           (billed_group_members.distinct.pluck(:user_id) +
-           billed_project_members.distinct.pluck(:user_id) +
-           billed_shared_group_members.distinct.pluck(:user_id) +
-           billed_invited_group_to_project_members.distinct.pluck(:user_id)).to_set
+          billed_project_members.distinct.pluck(:user_id) +
+          billed_shared_group_members.distinct.pluck(:user_id) +
+          billed_invited_group_to_project_members.distinct.pluck(:user_id)).to_set
         end
       end
     end
@@ -449,13 +448,6 @@ module EE
       )
     end
 
-    def billed_users_for(search_term, order_by)
-      users = ::User.id_in(billed_user_ids)
-      users = users.search(search_term) if search_term
-
-      users.sort_by_attribute(order_by || sort_value_name)
-    end
-
     private
 
     def custom_project_templates_group_allowed

ee/lib/ee/api/helpers/members_helpers.rb

@@ -7,6 +7,14 @@ module EE
         extend ActiveSupport::Concern
         extend ::Gitlab::Utils::Override
 
+        class << self
+          include ::SortingHelper
+
+          def member_sort_options
+            member_sort_options_hash.keys
+          end
+        end
+
         prepended do
           params :optional_filter_params_ee do
             optional :with_saml_identity, type: Grape::API::Boolean, desc: "List only members with linked SAML identity"
@@ -76,12 +84,11 @@ module EE
           ).for_member(member).security_event
         end
 
-        class << self
-          include ::SortingHelper
+        def billed_users_for(group, search_term, order_by)
+          users = ::User.id_in(group.billed_user_ids)
+          users = users.search(search_term) if search_term
 
-          def member_sort_options
-            member_sort_options_hash.keys
-          end
+          users.sort_by_attribute(order_by || 'name_asc')
         end
       end
     end

ee/lib/ee/api/members.rb

@@ -59,7 +59,7 @@ module EE
             bad_request!(nil) unless ::Ability.allowed?(current_user, :admin_group_member, group)
 
             sorting = params[:sort] || 'id_asc'
-            users = paginate(group.billed_users_for(params[:search], sorting))
+            users = paginate(billed_users_for(group, params[:search], sorting))
 
             present users, with: ::API::Entities::UserBasic, current_user: current_user
           end

ee/spec/lib/ee/api/helpers/members_helpers_spec.rb

@@ -33,4 +33,52 @@ RSpec.describe EE::API::Helpers::MembersHelpers do
       let(:member) { create(:project_member, project: source, user: create(:user)) }
     end
   end
+
+  describe '#billed_users_for' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:maria) { create(:group_member, group: group, user: create(:user, name: 'Maria Gomez')) }
+    let_it_be(:john_smith) { create(:group_member, group: group, user: create(:user, name: 'John Smith')) }
+    let_it_be(:john_doe) { create(:group_member, group: group, user: create(:user, name: 'John Doe')) }
+    let_it_be(:sophie) { create(:group_member, group: group, user: create(:user, name: 'Sophie Dupont')) }
+    let(:search_term) { nil }
+    let(:order_by) { nil }
+
+    subject { members_helpers.billed_users_for(group, search_term, order_by) }
+
+    context 'when a search parameter is present' do
+      let(:search_term) { 'John' }
+
+      context 'when a sorting parameter is provided (eg name descending)' do
+        let(:order_by) { 'name_desc' }
+
+        it 'sorts results accordingly' do
+          expect(subject).to eq([john_smith, john_doe].map(&:user))
+        end
+      end
+
+      context 'when a sorting parameter is not provided' do
+        let(:order_by) { nil }
+
+        it 'sorts results by name ascending' do
+          expect(subject).to eq([john_doe, john_smith].map(&:user))
+        end
+      end
+    end
+
+    context 'when a search parameter is not present' do
+      it 'returns expected users in name asc order' do
+        allow(group).to receive(:billed_user_members).and_return([john_doe, john_smith, sophie, maria])
+
+        expect(subject).to eq([john_doe, john_smith, maria, sophie].map(&:user))
+      end
+
+      context 'and when a sorting parameter is provided (eg name descending)' do
+        let(:order_by) { 'name_desc' }
+
+        it 'sorts results accordingly' do
+          expect(subject).to eq([sophie, maria, john_smith, john_doe].map(&:user))
+        end
+      end
+    end
+  end
 end

ee/spec/models/group_spec.rb

@@ -1223,52 +1223,4 @@ RSpec.describe Group do
       end
     end
   end
-
-  describe '#billed_users_for' do
-    let_it_be(:group) { create(:group) }
-    let_it_be(:maria) { create(:group_member, group: group, user: create(:user, name: 'Maria Gomez')) }
-    let_it_be(:john_smith) { create(:group_member, group: group, user: create(:user, name: 'John Smith')) }
-    let_it_be(:john_doe) { create(:group_member, group: group, user: create(:user, name: 'John Doe')) }
-    let_it_be(:sophie) { create(:group_member, group: group, user: create(:user, name: 'Sophie Dupont')) }
-    let(:search_term) { nil }
-    let(:order_by) { nil }
-
-    subject { group.billed_users_for(search_term, order_by) }
-
-    context 'when a search parameter is present' do
-      let(:search_term) { 'John' }
-
-      context 'when a sorting parameter is provided (eg name descending)' do
-        let(:order_by) { 'name_desc' }
-
-        it 'sorts results accordingly' do
-          expect(subject).to eq([john_smith, john_doe].map(&:user))
-        end
-      end
-
-      context 'when a sorting parameter is not provided' do
-        let(:order_by) { nil }
-
-        it 'sorts results by name ascending' do
-          expect(subject).to eq([john_doe, john_smith].map(&:user))
-        end
-      end
-    end
-
-    context 'when a search parameter is not present' do
-      it 'returns expected users in name asc order' do
-        allow(group).to receive(:billed_user_members).and_return([john_doe, john_smith, sophie, maria])
-
-        expect(subject).to eq([john_doe, john_smith, maria, sophie].map(&:user))
-      end
-
-      context 'and when a sorting parameter is provided (eg name descending)' do
-        let(:order_by) { 'name_desc' }
-
-        it 'sorts results accordingly' do
-          expect(subject).to eq([sophie, maria, john_smith, john_doe].map(&:user))
-        end
-      end
-    end
-  end
 end