Merge branch 'fj-40407-missing-order-paginate' into 'master'

Added default order to UserFinder

Closes #40407

See merge request gitlab-org/gitlab-ce!15679

app/finders/users_finder.rb

@@ -25,7 +25,7 @@ class UsersFinder
   end
 
   def execute
-    users = User.all
+    users = User.all.order_id_desc
     users = by_username(users)
     users = by_search(users)
     users = by_blocked(users)

app/models/user.rb

@@ -487,8 +487,12 @@ class User < ActiveRecord::Base
   end
 
   def two_factor_u2f_enabled?
+    if u2f_registrations.loaded?
+      u2f_registrations.any?
+    else
       u2f_registrations.exists?
     end
+  end
 
   def namespace_uniq
     # Return early if username already failed the first uniqueness validation

changelogs/unreleased/fj-40407-missing-order-paginate.yml

+---
+title: Added default order to UsersFinder
+merge_request: 15679
+author:
+type: fixed

lib/api/helpers/pagination.rb

@@ -2,6 +2,8 @@ module API
   module Helpers
     module Pagination
       def paginate(relation)
+        relation = add_default_order(relation)
+
         relation.page(params[:page]).per(params[:per_page]).tap do |data|
           add_pagination_headers(data)
         end
@@ -45,6 +47,14 @@ module API
         # Ensure there is in total at least 1 page
         [paginated_data.total_pages, 1].max
       end
+
+      def add_default_order(relation)
+        if relation.is_a?(ActiveRecord::Relation) && relation.order_values.empty?
+          relation = relation.order(:id)
+        end
+
+        relation
+      end
     end
   end
 end

lib/api/users.rb

@@ -76,6 +76,8 @@ module API
         forbidden!("Not authorized to access /api/v4/users") unless authorized
 
         entity = current_user&.admin? ? Entities::UserWithAdmin : Entities::UserBasic
+        users = users.preload(:identities, :u2f_registrations) if entity == Entities::UserWithAdmin
+
         present paginate(users), with: entity
       end
 

spec/lib/api/helpers/pagination_spec.rb

@@ -92,6 +92,27 @@ describe API::Helpers::Pagination do
           subject.paginate(resource)
         end
       end
+
+      context 'if order' do
+        it 'is not present it adds default order(:id) if no order is present' do
+          resource.order_values = []
+
+          paginated_relation = subject.paginate(resource)
+
+          expect(resource.order_values).to be_empty
+          expect(paginated_relation.order_values).to be_present
+          expect(paginated_relation.order_values.first).to be_ascending
+          expect(paginated_relation.order_values.first.expr.name).to eq :id
+        end
+
+        it 'is present it does not add anything' do
+          paginated_relation = subject.paginate(resource.order(created_at: :desc))
+
+          expect(paginated_relation.order_values).to be_present
+          expect(paginated_relation.order_values.first).to be_descending
+          expect(paginated_relation.order_values.first.expr.name).to eq :created_at
+        end
+      end
     end
 
     context 'when resource empty' do