From: Tom Hughes Date: Tue, 26 Mar 2024 18:32:52 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/4625' X-Git-Tag: live~1168 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/af5d76ecabb8b6a6b35d8df262806367ec4a87cc?hp=ae70bb7689ff968fb3402ea2362659e5a280a4cb Merge remote-tracking branch 'upstream/pull/4625' --- diff --git a/app/controllers/concerns/pagination_methods.rb b/app/controllers/concerns/pagination_methods.rb new file mode 100644 index 000000000..3dc9f52aa --- /dev/null +++ b/app/controllers/concerns/pagination_methods.rb @@ -0,0 +1,27 @@ +module PaginationMethods + extend ActiveSupport::Concern + + private + + ## + # limit selected items to one page, get ids of first item before/after the page + def get_page_items(items, includes: [], limit: 20) + id_column = "#{items.table_name}.id" + page_items = if params[:before] + items.where("#{id_column} < ?", params[:before]).order(:id => :desc) + elsif params[:after] + items.where("#{id_column} > ?", params[:after]).order(:id => :asc) + else + items.order(:id => :desc) + end + + page_items = page_items.limit(limit) + page_items = page_items.includes(includes) + page_items = page_items.sort.reverse + + newer_items_id = page_items.first.id if page_items.count.positive? && items.exists?(["#{id_column} > ?", page_items.first.id]) + older_items_id = page_items.last.id if page_items.count.positive? && items.exists?(["#{id_column} < ?", page_items.last.id]) + + [page_items, newer_items_id, older_items_id] + end +end diff --git a/app/controllers/diary_entries_controller.rb b/app/controllers/diary_entries_controller.rb index 1a3d648f4..8da0842eb 100644 --- a/app/controllers/diary_entries_controller.rb +++ b/app/controllers/diary_entries_controller.rb @@ -1,5 +1,6 @@ class DiaryEntriesController < ApplicationController include UserMethods + include PaginationMethods layout "site", :except => :rss @@ -57,7 +58,7 @@ class DiaryEntriesController < ApplicationController @params = params.permit(:display_name, :friends, :nearby, :language) - @entries, @newer_entries_id, @older_entries_id = get_page_items(entries, [:user, :language]) + @entries, @newer_entries_id, @older_entries_id = get_page_items(entries, :includes => [:user, :language]) end def show @@ -246,7 +247,7 @@ class DiaryEntriesController < ApplicationController @params = params.permit(:display_name, :before, :after) - @comments, @newer_comments_id, @older_comments_id = get_page_items(comments, [:user]) + @comments, @newer_comments_id, @older_comments_id = get_page_items(comments, :includes => [:user]) end private @@ -282,24 +283,4 @@ class DiaryEntriesController < ApplicationController @zoom = 12 end end - - def get_page_items(items, includes) - id_column = "#{items.table_name}.id" - page_items = if params[:before] - items.where("#{id_column} < ?", params[:before]).order(:id => :desc) - elsif params[:after] - items.where("#{id_column} > ?", params[:after]).order(:id => :asc) - else - items.order(:id => :desc) - end - - page_items = page_items.limit(20) - page_items = page_items.includes(includes) - page_items = page_items.sort.reverse - - newer_items_id = page_items.first.id if page_items.count.positive? && items.exists?(["#{id_column} > ?", page_items.first.id]) - older_items_id = page_items.last.id if page_items.count.positive? && items.exists?(["#{id_column} < ?", page_items.last.id]) - - [page_items, newer_items_id, older_items_id] - end end diff --git a/app/controllers/traces_controller.rb b/app/controllers/traces_controller.rb index df6337147..42aea8299 100644 --- a/app/controllers/traces_controller.rb +++ b/app/controllers/traces_controller.rb @@ -1,5 +1,6 @@ class TracesController < ApplicationController include UserMethods + include PaginationMethods layout "site", :except => :georss @@ -60,20 +61,7 @@ class TracesController < ApplicationController @params = params.permit(:display_name, :tag, :before, :after) - @traces = if params[:before] - traces.where("gpx_files.id < ?", params[:before]).order(:id => :desc) - elsif params[:after] - traces.where("gpx_files.id > ?", params[:after]).order(:id => :asc) - else - traces.order(:id => :desc) - end - - @traces = @traces.limit(20) - @traces = @traces.includes(:user, :tags) - @traces = @traces.sort.reverse - - @newer_traces = @traces.count.positive? && traces.exists?(["gpx_files.id > ?", @traces.first.id]) - @older_traces = @traces.count.positive? && traces.exists?(["gpx_files.id < ?", @traces.last.id]) + @traces, @newer_traces_id, @older_traces_id = get_page_items(traces, :includes => [:user, :tags]) # final helper vars for view @target_user = target_user diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index fbf49ecbe..c55a177b4 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -2,6 +2,7 @@ class UsersController < ApplicationController include EmailMethods include SessionMethods include UserMethods + include PaginationMethods layout "site" @@ -29,16 +30,14 @@ class UsersController < ApplicationController redirect_to url_for(:status => params[:status], :ip => params[:ip], :page => params[:page]) else - @params = params.permit(:status, :ip) + @params = params.permit(:status, :ip, :before, :after) - conditions = {} - conditions[:status] = @params[:status] if @params[:status] - conditions[:creation_ip] = @params[:ip] if @params[:ip] + users = User.all + users = users.where(:status => @params[:status]) if @params[:status] + users = users.where(:creation_ip => @params[:ip]) if @params[:ip] - @user_pages, @users = paginate(:users, - :conditions => conditions, - :order => :id, - :per_page => 50) + @users_count = users.count + @users, @newer_users_id, @older_users_id = get_page_items(users, :limit => 50) end end diff --git a/app/views/traces/index.html.erb b/app/views/traces/index.html.erb index 447716a18..5ec9127a8 100644 --- a/app/views/traces/index.html.erb +++ b/app/views/traces/index.html.erb @@ -68,8 +68,8 @@ <%= render "shared/pagination", :newer_key => "traces.trace_paging_nav.newer", :older_key => "traces.trace_paging_nav.older", - :newer_id => @newer_traces && @traces.first.id, - :older_id => @older_traces && @traces.last.id %> + :newer_id => @newer_traces_id, + :older_id => @older_traces_id %> @@ -80,8 +80,8 @@ <%= render "shared/pagination", :newer_key => "traces.trace_paging_nav.newer", :older_key => "traces.trace_paging_nav.older", - :newer_id => @newer_traces && @traces.first.id, - :older_id => @older_traces && @traces.last.id %> + :newer_id => @newer_traces_id, + :older_id => @older_traces_id %> <% else %>

<%= t ".empty_title" %>

<%= t ".empty_upload_html", :upload_link => link_to(t(".upload_new"), new_trace_path), diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index 12d5ae7af..e8031900f 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -10,6 +10,19 @@ <% unless @users.empty? %> <%= form_tag do %> +

+
+ <%= render "shared/pagination", + :newer_key => "users.index.newer", + :older_key => "users.index.older", + :newer_id => @newer_users_id, + :older_id => @older_users_id %> +
+
+ <%= t ".found_users", :count => @users_count %> +
+
+ <%= hidden_field_tag :status, params[:status] if params[:status] %> <%= hidden_field_tag :ip, params[:ip] if params[:ip] %> <%= hidden_field_tag :page, params[:page] if params[:page] %> @@ -17,15 +30,6 @@
- <%= t ".showing", - :page => @user_pages.current_page.number, - :first_item => @user_pages.current_page.first_item, - :last_item => @user_pages.current_page.last_item, - :items => @user_pages.item_count, - :count => @user_pages.current_page.last_item - @user_pages.current_page.first_item + 1 %> - <% if @user_pages.page_count > 1 %> - | <%= raw pagination_links_each(@user_pages, {}) { |n| link_to n, @params.merge(:page => n) } %> - <% end %> <%= check_box_tag "user_all", "1", false %> @@ -35,6 +39,19 @@ <%= render @users %>
+
+
+ <%= render "shared/pagination", + :newer_key => "users.index.newer", + :older_key => "users.index.older", + :newer_id => @newer_users_id, + :older_id => @older_users_id %> +
+
+ <%= t ".found_users", :count => @users_count %> +
+
+
<%= submit_tag t(".confirm"), :name => "confirm", :class => "btn btn-primary" %> <%= submit_tag t(".hide"), :name => "hide", :class => "btn btn-primary" %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 9af89978e..ec5e54442 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2832,9 +2832,11 @@ en: index: title: Users heading: Users - showing: - one: Page %{page} (%{first_item} of %{items}) - other: Page %{page} (%{first_item}-%{last_item} of %{items}) + older: "Older Users" + newer: "Newer Users" + found_users: + one: "%{count} user found" + other: "%{count} users found" summary_html: "%{name} created from %{ip_address} on %{date}" summary_no_ip_html: "%{name} created on %{date}" confirm: Confirm Selected Users diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 4ff9e76f6..62bb34279 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -644,23 +644,44 @@ class UsersControllerTest < ActionDispatch::IntegrationTest # 100 examples, an administrator, and a granter for the admin. assert_equal 102, User.count + next_path = users_path - get users_path + get next_path assert_response :success assert_template :index assert_select "table#user_list tbody tr", :count => 50 + check_no_page_link "Newer Users" + next_path = check_page_link "Older Users" - get users_path, :params => { :page => 2 } + get next_path assert_response :success assert_template :index assert_select "table#user_list tbody tr", :count => 50 + check_page_link "Newer Users" + next_path = check_page_link "Older Users" - get users_path, :params => { :page => 3 } + get next_path assert_response :success assert_template :index assert_select "table#user_list tbody tr", :count => 2 + check_page_link "Newer Users" + check_no_page_link "Older Users" + end + + private + + def check_no_page_link(name) + assert_select "a.page-link", { :text => /#{Regexp.quote(name)}/, :count => 0 }, "unexpected #{name} page link" end + def check_page_link(name) + assert_select "a.page-link", { :text => /#{Regexp.quote(name)}/ }, "missing #{name} page link" do |buttons| + return buttons.first.attributes["href"].value + end + end + + public + def test_index_post_confirm inactive_user = create(:user, :pending) suspended_user = create(:user, :suspended)