Applied suggestion from reviewer

Did some refactoring in Group class.

ee/app/models/ee/group.rb

@@ -299,23 +299,43 @@ module EE
     # We are plucking the user_ids from the "Members" table in an array and
     # converting the array of user_ids to a Set which will have unique user_ids.
     def billed_user_ids(requested_hosted_plan = nil)
+      billed_user_members.pluck(:user_id).to_set
+    end
+
+    def billed_user_members(requested_hosted_plan = nil)
       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
+        strong_memoize(:gold_billed_users) do
+          gold_billed_members
         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
+        strong_memoize(:non_gold_billed_users) do
+          non_gold_billed_members
         end
       end
     end
 
+    def non_gold_billed_members
+      ::Member.from_union(
+        [
+          billed_group_members,
+          billed_project_members,
+          billed_shared_group_members,
+          billed_invited_group_to_project_members
+        ]
+      ).distinct
+    end
+
+    def gold_billed_members
+      Member.from_union(
+        [
+          billed_group_members.non_guests,
+          billed_project_members.non_guests,
+          billed_shared_non_guests_group_members.non_guests,
+          billed_invited_non_guests_group_to_project_members.non_guests
+        ]
+      ).distinct
+    end
+
     override :supports_events?
     def supports_events?
       feature_available?(:epics)
@@ -448,17 +468,11 @@ module EE
       )
     end
 
-    def billed_user_ids_for(search_term, order_by)
-      if search_term.present?
-        ::GroupMember
-          .with_user(billed_user_ids)
-          .search(search_term)
-          .sort_by_attribute(::GroupMember.sorting_for(order_by))
-          .get_user_id
-          .uniq
-      else
-        billed_user_ids.sort
-      end
+    def billed_users_for(search_term, order_by)
+      users = ::User.id_in(billed_user_members.pluck(:user_id).uniq)
+      users = users.search(search_term) if search_term
+
+      users.sort_by_attribute(::GroupMember.sorting_for(order_by))
     end
 
     private

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

@@ -76,19 +76,6 @@ module EE
           ).for_member(member).security_event
         end
 
-        def paginate_billable_from_user_ids(user_ids, params = {})
-          paginated = paginate(::Kaminari.paginate_array(user_ids))
-
-          users_as_hash = ::User
-            .id_in(paginated)
-            .sort_by_attribute(::GroupMember.sorting_for(params[:sort]))
-            .index_by(&:id)
-
-          # map! ensures same paginatable array is manipulated
-          # instead of creating a new non-paginatable array
-          paginated.map! { |user_id| users_as_hash[user_id] }
-        end
-
         class << self
           include ::SortingHelper
 

ee/lib/ee/api/members.rb

@@ -58,7 +58,7 @@ module EE
             bad_request!(nil) if group.subgroup?
             bad_request!(nil) unless ::Ability.allowed?(current_user, :admin_group_member, group)
 
-            users = paginate_billable_from_user_ids(group.billed_user_ids_for(params[:search], params[:sort]), params)
+            users = paginate(group.billed_users_for(params[:search], params[:sort]))
 
             present users, with: ::API::Entities::UserBasic, current_user: current_user
           end

ee/spec/models/group_spec.rb

@@ -1224,47 +1224,50 @@ RSpec.describe Group do
     end
   end
 
-  describe '#billed_user_ids_for' do
+  describe '#billed_users_for' do
     let_it_be(:group) { create(:group) }
-    let_it_be(:gm_1) { create(:group_member, group: group, user: create(:user, name: 'Maria Gomez')) }
-    let_it_be(:gm_2) { create(:group_member, group: group, user: create(:user, name: 'John Smith')) }
-    let_it_be(:gm_3) { create(:group_member, group: group, user: create(:user, name: 'John Doe')) }
-    let_it_be(:gm_4) { create(:group_member, group: group, user: create(:user, name: 'Sophie Dupont')) }
+    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_user_ids_for(search_term, order_by) }
+    subject { group.billed_users_for(search_term, order_by) }
 
     context 'when a search parameter is present' do
       let(:search_term) { 'John' }
-      let(:order_by) { sort }
 
       context 'when a sorting parameter is provided (eg name descending)' do
-        let(:sort) { 'name_desc' }
+        let(:order_by) { 'name_desc' }
 
         it 'sorts results accordingly' do
-          expect(subject).to eq([gm_2, gm_3].map(&:user_id))
+          expect(subject).to eq([john_smith, john_doe].map(&:user))
         end
       end
 
       context 'when a sorting parameter is not provided' do
-        let(:sort) { nil }
+        let(:order_by) { nil }
 
         it 'sorts results by name ascending' do
-          expect(subject).to eq([gm_3, gm_2].map(&:user_id))
+          expect(subject).to eq([john_doe, john_smith].map(&:user))
         end
       end
     end
 
     context 'when a search parameter is not present' do
-      it 'returns the expected group member user ids' do
-        expect(subject).to eq([gm_1, gm_2, gm_3, gm_4].map(&:user_id))
+      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
 
-      it 'returns user array in asc order' do
-        allow(group).to receive(:billed_user_ids).and_return([gm_3, gm_2, gm_4, gm_1].map(&:user_id))
+      context 'and when a sorting parameter is provided (eg name descending)' do
+        let(:order_by) { 'name_desc' }
 
-        expect(subject).to eq([gm_1, gm_2, gm_3, gm_4].map(&:user_id))
+        it 'sorts results accordingly' do
+          expect(subject).to eq([sophie, maria, john_smith, john_doe].map(&:user))
+        end
       end
     end
   end