From: Andy Allan Date: Wed, 2 Aug 2023 15:57:26 +0000 (+0100) Subject: Merge pull request #4106 from tomhughes/diary-paging X-Git-Tag: live~1114 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/aceef47cd8f39928d27df43e2fdfee339ddafdc9?hp=-c Merge pull request #4106 from tomhughes/diary-paging Replace page numbers with ID based selection for diary indexes --- aceef47cd8f39928d27df43e2fdfee339ddafdc9 diff --combined app/controllers/diary_entries_controller.rb index 6e7378bf3,f464cacd0..ea9aacb21 --- a/app/controllers/diary_entries_controller.rb +++ b/app/controllers/diary_entries_controller.rb @@@ -17,7 -17,7 +17,7 @@@ class DiaryEntriesController < Applicat if @user @title = t ".user_title", :user => @user.display_name - @entries = @user.diary_entries + entries = @user.diary_entries else render_unknown_user params[:display_name] return @@@ -25,7 -25,7 +25,7 @@@ elsif params[:friends] if current_user @title = t ".title_friends" - @entries = DiaryEntry.where(:user_id => current_user.friends) + entries = DiaryEntry.where(:user_id => current_user.friends) else require_user return @@@ -33,38 -33,44 +33,46 @@@ elsif params[:nearby] if current_user @title = t ".title_nearby" - @entries = DiaryEntry.where(:user_id => current_user.nearby) + entries = DiaryEntry.where(:user_id => current_user.nearby) else require_user return end else - @entries = DiaryEntry.joins(:user).where(:users => { :status => %w[active confirmed] }) + entries = DiaryEntry.joins(:user).where(:users => { :status => %w[active confirmed] }) if params[:language] @title = t ".in_language_title", :language => Language.find(params[:language]).english_name - @entries = @entries.where(:language_code => params[:language]) + entries = entries.where(:language_code => params[:language]) else @title = t ".title" end end + entries = entries.visible unless can? :unhide, DiaryEntry + @params = params.permit(:display_name, :friends, :nearby, :language) - @page = (params[:page] || 1).to_i - @page_size = 20 + @entries = if params[:before] + entries.where("diary_entries.id < ?", params[:before]).order(:id => :desc) + elsif params[:after] + entries.where("diary_entries.id > ?", params[:after]).order(:id => :asc) + else + entries.order(:id => :desc) + end - @entries = @entries.visible unless can? :unhide, DiaryEntry - @entries = @entries.order("created_at DESC") - @entries = @entries.offset((@page - 1) * @page_size) - @entries = @entries.limit(@page_size) + @entries = @entries.limit(20) @entries = @entries.includes(:user, :language) + @entries = @entries.sort.reverse + + @newer_entries = @entries.count.positive? && entries.exists?(["diary_entries.id > ?", @entries.first.id]) + @older_entries = @entries.count.positive? && entries.exists?(["diary_entries.id < ?", @entries.last.id]) end def show - @entry = @user.diary_entries.visible.where(:id => params[:id]).first + entries = @user.diary_entries + entries = entries.visible unless can? :unhide, DiaryEntry + @entry = entries.where(:id => params[:id]).first if @entry @title = t ".title", :user => params[:display_name], :title => @entry.title @comments = can?(:unhidecomment, DiaryEntry) ? @entry.comments : @entry.visible_comments diff --combined test/controllers/diary_entries_controller_test.rb index 8d646c426,2bc6516bc..c9464ffed --- a/test/controllers/diary_entries_controller_test.rb +++ b/test/controllers/diary_entries_controller_test.rb @@@ -564,11 -564,36 +564,36 @@@ class DiaryEntriesControllerTest < Acti get diary_entries_path assert_response :success assert_select "div.diary_post", :count => 20 + assert_select "li.page-item a.page-link", :text => "Older Entries", :count => 1 + assert_select "li.page-item.disabled span.page-link", :text => "Newer Entries", :count => 1 # Try and get the second page - get diary_entries_path(:page => 2) + get css_select("li.page-item a.page-link").first["href"] assert_response :success assert_select "div.diary_post", :count => 20 + assert_select "li.page-item a.page-link", :text => "Older Entries", :count => 1 + assert_select "li.page-item a.page-link", :text => "Newer Entries", :count => 1 + + # Try and get the third page + get css_select("li.page-item a.page-link").first["href"] + assert_response :success + assert_select "div.diary_post", :count => 10 + assert_select "li.page-item.disabled span.page-link", :text => "Older Entries", :count => 1 + assert_select "li.page-item a.page-link", :text => "Newer Entries", :count => 1 + + # Go back to the second page + get css_select("li.page-item a.page-link").last["href"] + assert_response :success + assert_select "div.diary_post", :count => 20 + assert_select "li.page-item a.page-link", :text => "Older Entries", :count => 1 + assert_select "li.page-item a.page-link", :text => "Newer Entries", :count => 1 + + # Go back to the first page + get css_select("li.page-item a.page-link").last["href"] + assert_response :success + assert_select "div.diary_post", :count => 20 + assert_select "li.page-item a.page-link", :text => "Older Entries", :count => 1 + assert_select "li.page-item.disabled span.page-link", :text => "Newer Entries", :count => 1 end def test_rss @@@ -680,26 -705,14 +705,26 @@@ assert_response :not_found # Try an entry by a suspended user - diary_entry_suspended = create(:diary_entry, :user => suspended_user) - get diary_entry_path(:display_name => suspended_user.display_name, :id => diary_entry_suspended) + diary_entry_suspended_user = create(:diary_entry, :user => suspended_user) + get diary_entry_path(:display_name => suspended_user.display_name, :id => diary_entry_suspended_user) assert_response :not_found # Try an entry by a deleted user - diary_entry_deleted = create(:diary_entry, :user => deleted_user) - get diary_entry_path(:display_name => deleted_user.display_name, :id => diary_entry_deleted) + diary_entry_deleted_user = create(:diary_entry, :user => deleted_user) + get diary_entry_path(:display_name => deleted_user.display_name, :id => diary_entry_deleted_user) assert_response :not_found + + # Now try as a moderator + session_for(create(:moderator_user)) + get diary_entry_path(:display_name => user.display_name, :id => diary_entry_deleted) + assert_response :success + assert_template :show + + # Finally try as an administrator + session_for(create(:administrator_user)) + get diary_entry_path(:display_name => user.display_name, :id => diary_entry_deleted) + assert_response :success + assert_template :show end def test_show_hidden_comments @@@ -776,11 -789,8 +801,11 @@@ session_for(create(:moderator_user)) post unhide_diary_entry_path(:display_name => user.display_name, :id => diary_entry) assert_response :redirect - assert_redirected_to :controller => :errors, :action => :forbidden - assert_not DiaryEntry.find(diary_entry.id).visible + assert_redirected_to :action => :index, :display_name => user.display_name + assert DiaryEntry.find(diary_entry.id).visible + + # Reset + diary_entry.reload.update(:visible => true) # Finally try as an administrator session_for(create(:administrator_user)) @@@ -846,11 -856,8 +871,11 @@@ session_for(create(:moderator_user)) post unhide_diary_comment_path(:display_name => user.display_name, :id => diary_entry, :comment => diary_comment) assert_response :redirect - assert_redirected_to :controller => :errors, :action => :forbidden - assert_not DiaryComment.find(diary_comment.id).visible + assert_redirected_to :action => :show, :display_name => user.display_name, :id => diary_entry.id + assert DiaryComment.find(diary_comment.id).visible + + # Reset + diary_comment.reload.update(:visible => true) # Finally try as an administrator session_for(create(:administrator_user))