diff options
author | Francisco Javier López <fjlopez@gitlab.com> | 2017-12-04 09:49:53 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-12-04 09:49:53 +0000 |
commit | 7c2b7296d4e623ba4eaa19cf03405ee7f2ae1ca9 (patch) | |
tree | 80784f83c52313797b8cf7e78ccf8db66f0880d8 | |
parent | c997c95dfca2c3d30f8c7b8ef5f0fd95c1356226 (diff) | |
download | gitlab-ce-7c2b7296d4e623ba4eaa19cf03405ee7f2ae1ca9.tar.gz |
Added default order to UserFinder
-rw-r--r-- | app/finders/users_finder.rb | 2 | ||||
-rw-r--r-- | app/models/user.rb | 6 | ||||
-rw-r--r-- | changelogs/unreleased/fj-40407-missing-order-paginate.yml | 5 | ||||
-rw-r--r-- | lib/api/helpers/pagination.rb | 10 | ||||
-rw-r--r-- | lib/api/users.rb | 2 | ||||
-rw-r--r-- | spec/lib/api/helpers/pagination_spec.rb | 21 |
6 files changed, 44 insertions, 2 deletions
diff --git a/app/finders/users_finder.rb b/app/finders/users_finder.rb index 1a7e97004fb..edde8022ec9 100644 --- a/app/finders/users_finder.rb +++ b/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) diff --git a/app/models/user.rb b/app/models/user.rb index 14941fd7f98..5e7fe01c825 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -487,7 +487,11 @@ class User < ActiveRecord::Base end def two_factor_u2f_enabled? - u2f_registrations.exists? + if u2f_registrations.loaded? + u2f_registrations.any? + else + u2f_registrations.exists? + end end def namespace_uniq diff --git a/changelogs/unreleased/fj-40407-missing-order-paginate.yml b/changelogs/unreleased/fj-40407-missing-order-paginate.yml new file mode 100644 index 00000000000..27471dc2c52 --- /dev/null +++ b/changelogs/unreleased/fj-40407-missing-order-paginate.yml @@ -0,0 +1,5 @@ +--- +title: Added default order to UsersFinder +merge_request: 15679 +author: +type: fixed diff --git a/lib/api/helpers/pagination.rb b/lib/api/helpers/pagination.rb index 95108292aac..bb70370ba77 100644 --- a/lib/api/helpers/pagination.rb +++ b/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 diff --git a/lib/api/users.rb b/lib/api/users.rb index 0cd89b1bcf8..e5de31ad51b 100644 --- a/lib/api/users.rb +++ b/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 diff --git a/spec/lib/api/helpers/pagination_spec.rb b/spec/lib/api/helpers/pagination_spec.rb index 59deca7757b..a547988d631 100644 --- a/spec/lib/api/helpers/pagination_spec.rb +++ b/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 |