From: Anton Khorev Date: Sun, 16 Mar 2025 00:18:23 +0000 (+0300) Subject: Add 'after' parameter to history pages X-Git-Tag: live~33^2~1 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/8a65f551a71e92052107805c03ad687697d375e3 Add 'after' parameter to history pages --- diff --git a/app/assets/javascripts/index/history.js b/app/assets/javascripts/index/history.js index dddd84354..4264e5dce 100644 --- a/app/assets/javascripts/index/history.js +++ b/app/assets/javascripts/index/history.js @@ -47,12 +47,24 @@ OSM.History = function (map) { $("#sidebar_content .changesets").html(html); } - function displayMoreChangesets(html) { - $("#sidebar_content .changeset_more").replaceWith(html); - const oldList = $("#sidebar_content .changesets ol").first(); - const newList = oldList.next("ol"); - newList.children().appendTo(oldList); - newList.remove(); + function displayMoreChangesets(div, html) { + const oldList = $("#sidebar_content .changesets ol"); + + div.replaceWith(html); + + const prevNewList = oldList.prevAll("ol"); + if (prevNewList.length) { + prevNewList.next(".changeset_more").remove(); + prevNewList.children().prependTo(oldList); + prevNewList.remove(); + } + + const nextNewList = oldList.nextAll("ol"); + if (nextNewList.length) { + nextNewList.prev(".changeset_more").remove(); + nextNewList.children().appendTo(oldList); + nextNewList.remove(); + } } function update() { @@ -71,6 +83,9 @@ OSM.History = function (map) { if (params.has("before")) { data.set("before", params.get("before")); } + if (params.has("after")) { + data.set("after", params.get("after")); + } fetch(location.pathname + "?" + data) .then(response => response.text()) @@ -90,7 +105,7 @@ OSM.History = function (map) { div.find(".loader").show(); $.get($(this).attr("href"), function (html) { - displayMoreChangesets(html); + displayMoreChangesets(div, html); updateMap(); }); } diff --git a/app/controllers/changesets_controller.rb b/app/controllers/changesets_controller.rb index abdc23f04..5e79f4da6 100644 --- a/app/controllers/changesets_controller.rb +++ b/app/controllers/changesets_controller.rb @@ -2,6 +2,7 @@ class ChangesetsController < ApplicationController include UserMethods + include PaginationMethods layout "site" @@ -18,11 +19,12 @@ class ChangesetsController < ApplicationController # list non-empty changesets in reverse chronological order def index param! :before, Integer, :min => 1 + param! :after, Integer, :min => 1 - @params = params.permit(:display_name, :bbox, :friends, :nearby, :before, :list) + @params = params.permit(:display_name, :bbox, :friends, :nearby, :before, :after, :list) - if request.format == :atom && @params[:before] - redirect_to url_for(@params.merge(:before => nil)), :status => :moved_permanently + if request.format == :atom && (@params[:before] || @params[:after]) + redirect_to url_for(@params.merge(:before => nil, :after => nil)), :status => :moved_permanently return end @@ -59,9 +61,7 @@ class ChangesetsController < ApplicationController changesets = changesets.where(:user => current_user.nearby) end - changesets = changesets.where(:changesets => { :id => ...@params[:before] }) if @params[:before] - - @changesets = changesets.order("changesets.id DESC").limit(20).preload(:user, :changeset_tags, :comments) + @changesets, @newer_changesets_id, @older_changesets_id = get_page_items(changesets, :includes => [:user, :changeset_tags, :comments]) render :action => :index, :layout => false end diff --git a/app/views/changesets/index.html.erb b/app/views/changesets/index.html.erb index 85c5f0bcb..3f25cf134 100644 --- a/app/views/changesets/index.html.erb +++ b/app/views/changesets/index.html.erb @@ -1,17 +1,17 @@ -<% if @changesets.present? %> -
    - <%= render @changesets %> -
-<% if @changesets.size == 20 -%> -
- <%= link_to t(".load_more"), url_for(@params.merge(:before => @changesets.last.id)), :class => "btn btn-primary" %> +<% if @newer_changesets_id %> +
+ <%= link_to t(".load_more"), url_for(@params.merge(:before => nil, :after => @newer_changesets_id)), :class => "btn btn-primary" %>
-<% end -%> +<% end %> +<% if @changesets.present? %> +
    + <%= render @changesets %> +
<% elsif params[:bbox] %>

<%= params[:before] ? t(".no_more_area") : t(".empty_area") %>

<% elsif params[:display_name] %> @@ -19,3 +19,13 @@ <% else %>

<%= params[:before] ? t(".no_more") : t(".empty") %>

<% end %> +<% if @older_changesets_id -%> +
+ <%= link_to t(".load_more"), url_for(@params.merge(:before => @older_changesets_id, :after => nil)), :class => "btn btn-primary" %> +
+ +
+
+<% end -%> diff --git a/test/controllers/changesets_controller_test.rb b/test/controllers/changesets_controller_test.rb index 4274f7c31..452084951 100644 --- a/test/controllers/changesets_controller_test.rb +++ b/test/controllers/changesets_controller_test.rb @@ -82,6 +82,9 @@ class ChangesetsControllerTest < ActionDispatch::IntegrationTest %w[-1 0 fred].each do |id| get history_path(:format => "html", :list => "1", :before => id) assert_redirected_to :controller => :errors, :action => :bad_request + + get history_path(:format => "html", :list => "1", :after => id) + assert_redirected_to :controller => :errors, :action => :bad_request end end @@ -238,6 +241,23 @@ class ChangesetsControllerTest < ActionDispatch::IntegrationTest check_index_result [changeset1] end + def test_index_after_id + changeset1 = create(:changeset, :num_changes => 1) + changeset2 = create(:changeset, :num_changes => 1) + + get history_path(:format => "html", :after => changeset1.id), :xhr => true + assert_response :success + assert_template "history" + assert_template :layout => "xhr" + assert_select "h2", :text => "Changesets", :count => 1 + + get history_path(:format => "html", :list => "1", :after => changeset1.id), :xhr => true + assert_response :success + assert_template "index" + + check_index_result [changeset2] + end + ## # Check that a list with a next page link works def test_index_more @@ -416,6 +436,11 @@ class ChangesetsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to :action => :feed end + def test_feed_after + get history_feed_path(:format => "atom", :after => 100) + assert_redirected_to :action => :feed + end + private ## diff --git a/test/system/history_test.rb b/test/system/history_test.rb index 4f2114f25..7d730355a 100644 --- a/test/system/history_test.rb +++ b/test/system/history_test.rb @@ -81,6 +81,34 @@ class HistoryTest < ApplicationSystemTestCase end end + test "user history starts after specified changeset" do + user = create(:user) + changeset0 = create(:changeset) + changeset1 = create_visible_changeset(user, "1st-changeset-in-history") + changeset2 = create_visible_changeset(user, "2nd-changeset-in-history") + + visit "#{user_path user}/history?after=#{changeset2.id}" + + within_sidebar do + assert_no_link "1st-changeset-in-history" + assert_no_link "2nd-changeset-in-history" + end + + visit "#{user_path user}/history?after=#{changeset1.id}" + + within_sidebar do + assert_no_link "1st-changeset-in-history" + assert_link "2nd-changeset-in-history" + end + + visit "#{user_path user}/history?after=#{changeset0.id}" + + within_sidebar do + assert_link "1st-changeset-in-history" + assert_link "2nd-changeset-in-history" + end + end + private def create_visible_changeset(user, comment)