From: Tom Hughes <tom@compton.nu>
Date: Fri, 1 Mar 2024 19:23:51 +0000 (+0000)
Subject: Merge remote-tracking branch 'upstream/pull/4324'
X-Git-Tag: live~686
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/664d02982cbaa8b1223ef03047b6134ff1ffbdac?hp=642070f1e5dee82077676aa300ff47fbac11c506

Merge remote-tracking branch 'upstream/pull/4324'
---

diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb
index 5e0e835d4..ef4c0a778 100644
--- a/app/abilities/ability.rb
+++ b/app/abilities/ability.rb
@@ -4,8 +4,7 @@ class Ability
   include CanCan::Ability
 
   def initialize(user)
-    can [:relation, :relation_history, :way, :way_history, :node, :node_history,
-         :changeset, :query], :browse
+    can [:relation, :relation_history, :way, :way_history, :node, :node_history, :query], :browse
     can [:show], OldNode
     can [:show], OldWay
     can [:show], OldRelation
@@ -17,7 +16,7 @@ class Ability
     can [:token, :request_token, :access_token, :test_request], :oauth
 
     if Settings.status != "database_offline"
-      can [:index, :feed], Changeset
+      can [:index, :feed, :show], Changeset
       can :index, ChangesetComment
       can [:confirm, :confirm_resend, :confirm_email], :confirmation
       can [:index, :rss, :show, :comments], DiaryEntry
diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb
index e842d4872..db291f6eb 100644
--- a/app/controllers/browse_controller.rb
+++ b/app/controllers/browse_controller.rb
@@ -58,25 +58,6 @@ class BrowseController < ApplicationController
     render :action => "not_found", :status => :not_found
   end
 
-  def changeset
-    @type = "changeset"
-    @changeset = Changeset.find(params[:id])
-    @comments = if current_user&.moderator?
-                  @changeset.comments.unscope(:where => :visible).includes(:author)
-                else
-                  @changeset.comments.includes(:author)
-                end
-    @node_pages, @nodes = paginate(:old_nodes, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "node_page")
-    @way_pages, @ways = paginate(:old_ways, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "way_page")
-    @relation_pages, @relations = paginate(:old_relations, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "relation_page")
-    if @changeset.user.active? && @changeset.user.data_public?
-      @next_by_user = @changeset.user.changesets.where("id > ?", @changeset.id).reorder(:id => :asc).first
-      @prev_by_user = @changeset.user.changesets.where("id < ?", @changeset.id).reorder(:id => :desc).first
-    end
-  rescue ActiveRecord::RecordNotFound
-    render :action => "not_found", :status => :not_found
-  end
-
   def query; end
 
   private
diff --git a/app/controllers/changesets_controller.rb b/app/controllers/changesets_controller.rb
index 5eb14629a..613a3ee2a 100644
--- a/app/controllers/changesets_controller.rb
+++ b/app/controllers/changesets_controller.rb
@@ -8,7 +8,8 @@ class ChangesetsController < ApplicationController
 
   before_action :authorize_web
   before_action :set_locale
-  before_action -> { check_database_readable(:need_api => true) }, :only => [:index, :feed]
+  before_action -> { check_database_readable(:need_api => true) }, :only => [:index, :feed, :show]
+  before_action :require_oauth, :only => :show
   before_action :check_database_writable, :only => [:subscribe, :unsubscribe]
 
   authorize_resource
@@ -75,6 +76,26 @@ class ChangesetsController < ApplicationController
     index
   end
 
+  def show
+    @type = "changeset"
+    @changeset = Changeset.find(params[:id])
+    @comments = if current_user&.moderator?
+                  @changeset.comments.unscope(:where => :visible).includes(:author)
+                else
+                  @changeset.comments.includes(:author)
+                end
+    @node_pages, @nodes = paginate(:old_nodes, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "node_page")
+    @way_pages, @ways = paginate(:old_ways, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "way_page")
+    @relation_pages, @relations = paginate(:old_relations, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "relation_page")
+    if @changeset.user.active? && @changeset.user.data_public?
+      @next_by_user = @changeset.user.changesets.where("id > ?", @changeset.id).reorder(:id => :asc).first
+      @prev_by_user = @changeset.user.changesets.where("id < ?", @changeset.id).reorder(:id => :desc).first
+    end
+    render :layout => map_layout
+  rescue ActiveRecord::RecordNotFound
+    render :template => "browse/not_found", :status => :not_found, :layout => map_layout
+  end
+
   ##
   # subscribe to a changeset
   def subscribe
diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb
index 67b3c7cf7..53e980fdc 100644
--- a/app/helpers/browse_helper.rb
+++ b/app/helpers/browse_helper.rb
@@ -72,10 +72,10 @@ module BrowseHelper
 
   def type_and_paginated_count(type, pages)
     if pages.page_count == 1
-      t "browse.changeset.#{type}",
+      t ".#{type.pluralize}",
         :count => pages.item_count
     else
-      t "browse.changeset.#{type}_paginated",
+      t ".#{type.pluralize}_paginated",
         :x => pages.current_page.first_item,
         :y => pages.current_page.last_item,
         :count => pages.item_count
diff --git a/app/helpers/changesets_helper.rb b/app/helpers/changesets_helper.rb
index 3a26c2001..dbee08c50 100644
--- a/app/helpers/changesets_helper.rb
+++ b/app/helpers/changesets_helper.rb
@@ -18,15 +18,15 @@ module ChangesetsHelper
     else
       action = :closed
       time = time_ago_in_words(changeset.closed_at, :scope => :"datetime.distance_in_words_ago")
-      title = safe_join([t("browse.created"), ": ", l(changeset.created_at), "&#10;".html_safe, t("browse.closed"), ": ", l(changeset.closed_at)])
+      title = safe_join([t("changesets.show.created", :when => l(changeset.created_at)), "&#10;".html_safe, t("changesets.show.closed", :when => l(changeset.closed_at))])
       datetime = changeset.closed_at.xmlschema
     end
 
     if params.key?(:display_name)
-      t "browse.#{action}_ago_html", :time_ago => tag.time(time, :title => title, :datetime => datetime)
+      t "changesets.show.#{action}_ago_html", :time_ago => tag.time(time, :title => title, :datetime => datetime)
     else
-      t "browse.#{action}_ago_by_html", :time_ago => tag.time(time, :title => title, :datetime => datetime),
-                                        :user => changeset_user_link(changeset)
+      t "changesets.show.#{action}_ago_by_html", :time_ago => tag.time(time, :title => title, :datetime => datetime),
+                                                 :user => changeset_user_link(changeset)
     end
   end
 
diff --git a/app/views/changeset_comments/_comments.rss.builder b/app/views/changeset_comments/_comments.rss.builder
index 59973965f..b1344b488 100644
--- a/app/views/changeset_comments/_comments.rss.builder
+++ b/app/views/changeset_comments/_comments.rss.builder
@@ -2,8 +2,8 @@ comments.each do |comment|
   xml.item do
     xml.title t(".comment", :author => comment.author.display_name, :changeset_id => comment.changeset.id.to_s)
 
-    xml.link url_for(:controller => "browse", :action => "changeset", :id => comment.changeset.id, :anchor => "c#{comment.id}", :only_path => false)
-    xml.guid url_for(:controller => "browse", :action => "changeset", :id => comment.changeset.id, :anchor => "c#{comment.id}", :only_path => false)
+    xml.link changeset_url(comment.changeset, :anchor => "c#{comment.id}")
+    xml.guid changeset_url(comment.changeset, :anchor => "c#{comment.id}")
 
     xml.description do
       xml.cdata! render(:partial => "comment", :object => comment, :formats => [:html])
diff --git a/app/views/browse/_paging_nav.html.erb b/app/views/changesets/_paging_nav.html.erb
similarity index 100%
rename from app/views/browse/_paging_nav.html.erb
rename to app/views/changesets/_paging_nav.html.erb
diff --git a/app/views/changesets/index.atom.builder b/app/views/changesets/index.atom.builder
index 521dffe5f..c8ffe1a81 100644
--- a/app/views/changesets/index.atom.builder
+++ b/app/views/changesets/index.atom.builder
@@ -41,16 +41,16 @@ atom_feed(:language => I18n.locale, :schema_date => 2009,
         xhtml.style "th { text-align: left } tr { vertical-align: top }"
         xhtml.table do |table|
           table.tr do |tr|
-            tr.th t("browse.created")
+            tr.th t(".feed.created")
             tr.td l(changeset.created_at)
           end
           table.tr do |tr|
-            tr.th t("browse.closed")
+            tr.th t(".feed.closed")
             tr.td l(changeset.closed_at)
           end
           if changeset.user.data_public?
             table.tr do |tr|
-              tr.th t("browse.changeset.belongs_to")
+              tr.th t(".feed.belongs_to")
               tr.td do |td|
                 td.a changeset.user.display_name, :href => user_url(changeset.user, :only_path => false)
               end
diff --git a/app/views/browse/changeset.html.erb b/app/views/changesets/show.html.erb
similarity index 98%
rename from app/views/browse/changeset.html.erb
rename to app/views/changesets/show.html.erb
index db6b9c966..57a3fdb56 100644
--- a/app/views/browse/changeset.html.erb
+++ b/app/views/changesets/show.html.erb
@@ -8,7 +8,7 @@
   </p>
   <p class="details"><%= changeset_details(@changeset) %></p>
 
-  <%= render :partial => "tag_details", :object => @changeset.tags.except("comment") %>
+  <%= render :partial => "browse/tag_details", :object => @changeset.tags.except("comment") %>
 
   <div class="row">
     <div class="col">
diff --git a/app/views/dashboards/_contact.html.erb b/app/views/dashboards/_contact.html.erb
index 7e78a6895..b789f721b 100644
--- a/app/views/dashboards/_contact.html.erb
+++ b/app/views/dashboards/_contact.html.erb
@@ -25,9 +25,7 @@
       <% if changeset %>
         <%= t(".latest_edit_html", :ago => friendly_date_ago(changeset.created_at)) %>
         <% comment = changeset.tags["comment"].to_s == "" ? t("browse.no_comment") : changeset.tags["comment"] %>
-        <q><%= link_to(comment,
-                       { :controller => "browse", :action => "changeset", :id => changeset.id },
-                       { :title => t("changesets.changeset.view_changeset_details") }) %></q>
+        <q><%= link_to comment, changeset_path(changeset), :title => t("changesets.changeset.view_changeset_details") %></q>
       <% else %>
        <%= t "changesets.changeset.no_edits" %>
       <% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index fcf9f90a8..429786fcc 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -308,12 +308,6 @@ en:
     destroy:
       success: "Account Deleted."
   browse:
-    created: "Created"
-    closed: "Closed"
-    created_ago_html: "Created %{time_ago}"
-    closed_ago_html: "Closed %{time_ago}"
-    created_ago_by_html: "Created %{time_ago} by %{user}"
-    closed_ago_by_html: "Closed %{time_ago} by %{user}"
     deleted_ago_by_html: "Deleted %{time_ago} by %{user}"
     edited_ago_by_html: "Edited %{time_ago} by %{user}"
     version: "Version"
@@ -337,22 +331,6 @@ en:
     location: "Location:"
     common_details:
       coordinates_html: "%{latitude}, %{longitude}"
-    changeset:
-      title: "Changeset: %{id}"
-      belongs_to: "Author"
-      node: "Nodes (%{count})"
-      node_paginated: "Nodes (%{x}-%{y} of %{count})"
-      way: "Ways (%{count})"
-      way_paginated: "Ways (%{x}-%{y} of %{count})"
-      relation: "Relations (%{count})"
-      relation_paginated: "Relations (%{x}-%{y} of %{count})"
-      hidden_comment_by_html: "Hidden comment from %{user} %{time_ago}"
-      comment_by_html: "Comment from %{user} %{time_ago}"
-      changesetxml: "Changeset XML"
-      osmchangexml: "osmChange XML"
-      join_discussion: "Log in to join the discussion"
-      discussion: Discussion
-      still_open: "Changeset still open - discussion will open once the changeset is closed."
     node:
       title_html: "Node: %{name}"
       history_title_html: "Node History: %{name}"
@@ -468,6 +446,9 @@ en:
       feed:
         title: "Changeset %{id}"
         title_comment: "Changeset %{id} - %{comment}"
+        created: "Created"
+        closed: "Closed"
+        belongs_to: "Author"
     subscribe:
       heading: Subscribe to the following changeset discussion?
       button: Subscribe to discussion
@@ -481,6 +462,28 @@ en:
       title: "No such changeset"
       heading: "No entry with the id: %{id}"
       body: "Sorry, there is no changeset with the id %{id}. Please check your spelling, or maybe the link you clicked is wrong."
+    show:
+      title: "Changeset: %{id}"
+      created: "Created: %{when}"
+      closed: "Closed: %{when}"
+      created_ago_html: "Created %{time_ago}"
+      closed_ago_html: "Closed %{time_ago}"
+      created_ago_by_html: "Created %{time_ago} by %{user}"
+      closed_ago_by_html: "Closed %{time_ago} by %{user}"
+      discussion: Discussion
+      join_discussion: "Log in to join the discussion"
+      still_open: "Changeset still open - discussion will open once the changeset is closed."
+      comment_by_html: "Comment from %{user} %{time_ago}"
+      hidden_comment_by_html: "Hidden comment from %{user} %{time_ago}"
+      changesetxml: "Changeset XML"
+      osmchangexml: "osmChange XML"
+    paging_nav:
+      nodes: "Nodes (%{count})"
+      nodes_paginated: "Nodes (%{x}-%{y} of %{count})"
+      ways: "Ways (%{count})"
+      ways_paginated: "Ways (%{x}-%{y} of %{count})"
+      relations: "Relations (%{count})"
+      relations_paginated: "Relations (%{x}-%{y} of %{count})"
     timeout:
       sorry: "Sorry, the list of changesets you requested took too long to retrieve."
   changeset_comments:
diff --git a/config/routes.rb b/config/routes.rb
index af5730d21..98184e0e1 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -118,7 +118,7 @@ OpenStreetMap::Application.routes.draw do
   get "/relation/:id" => "browse#relation", :id => /\d+/, :as => :relation
   get "/relation/:id/history" => "browse#relation_history", :id => /\d+/, :as => :relation_history
   resources :old_relations, :path => "/relation/:id/history", :id => /\d+/, :version => /\d+/, :param => :version, :only => :show
-  get "/changeset/:id" => "browse#changeset", :as => :changeset, :id => /\d+/
+  resources :changesets, :path => "changeset", :id => /\d+/, :only => :show
   get "/changeset/:id/comments/feed" => "changeset_comments#index", :as => :changeset_comments_feed, :id => /\d*/, :defaults => { :format => "rss" }
   resources :notes, :path => "note", :only => [:show, :new]
 
diff --git a/test/controllers/browse_controller_test.rb b/test/controllers/browse_controller_test.rb
index 312caca51..fcdd7c752 100644
--- a/test/controllers/browse_controller_test.rb
+++ b/test/controllers/browse_controller_test.rb
@@ -28,10 +28,6 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
       { :path => "/relation/1/history", :method => :get },
       { :controller => "browse", :action => "relation_history", :id => "1" }
     )
-    assert_routing(
-      { :path => "/changeset/1", :method => :get },
-      { :controller => "browse", :action => "changeset", :id => "1" }
-    )
     assert_routing(
       { :path => "/query", :method => :get },
       { :controller => "browse", :action => "query" }
@@ -40,7 +36,7 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
 
   def test_read_relation
     relation = create(:relation)
-    browse_check :relation_path, relation.id, "browse/feature"
+    sidebar_browse_check :relation_path, relation.id, "browse/feature"
     assert_select "h4", /^Version/ do
       assert_select "a[href='#{old_relation_path relation, 1}']", :text => "1", :count => 1
     end
@@ -51,7 +47,7 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
 
   def test_multiple_version_relation_links
     relation = create(:relation, :with_history, :version => 2)
-    browse_check :relation_path, relation.id, "browse/feature"
+    sidebar_browse_check :relation_path, relation.id, "browse/feature"
     assert_select ".secondary-actions a[href='#{relation_history_path relation}']", :count => 1
     assert_select ".secondary-actions a[href='#{old_relation_path relation, 1}']", :count => 1
     assert_select ".secondary-actions a[href='#{old_relation_path relation, 2}']", :count => 1
@@ -59,7 +55,7 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
 
   def test_read_relation_history
     relation = create(:relation, :with_history)
-    browse_check :relation_history_path, relation.id, "browse/history"
+    sidebar_browse_check :relation_history_path, relation.id, "browse/history"
     assert_select "h4", /^Version/ do
       assert_select "a[href='#{old_relation_path relation, 1}']", :text => "1", :count => 1
     end
@@ -67,7 +63,7 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
 
   def test_read_way
     way = create(:way)
-    browse_check :way_path, way.id, "browse/feature"
+    sidebar_browse_check :way_path, way.id, "browse/feature"
     assert_select "h4", /^Version/ do
       assert_select "a[href='#{old_way_path way, 1}']", :text => "1", :count => 1
     end
@@ -78,7 +74,7 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
 
   def test_multiple_version_way_links
     way = create(:way, :with_history, :version => 2)
-    browse_check :way_path, way.id, "browse/feature"
+    sidebar_browse_check :way_path, way.id, "browse/feature"
     assert_select ".secondary-actions a[href='#{way_history_path way}']", :count => 1
     assert_select ".secondary-actions a[href='#{old_way_path way, 1}']", :count => 1
     assert_select ".secondary-actions a[href='#{old_way_path way, 2}']", :count => 1
@@ -86,7 +82,7 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
 
   def test_read_way_history
     way = create(:way, :with_history)
-    browse_check :way_history_path, way.id, "browse/history"
+    sidebar_browse_check :way_history_path, way.id, "browse/history"
     assert_select "h4", /^Version/ do
       assert_select "a[href='#{old_way_path way, 1}']", :text => "1", :count => 1
     end
@@ -94,7 +90,7 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
 
   def test_read_node
     node = create(:node)
-    browse_check :node_path, node.id, "browse/feature"
+    sidebar_browse_check :node_path, node.id, "browse/feature"
     assert_select "h4", /^Version/ do
       assert_select "a[href='#{old_node_path node, 1}']", :text => "1", :count => 1
     end
@@ -105,7 +101,7 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
 
   def test_multiple_version_node_links
     node = create(:node, :with_history, :version => 2)
-    browse_check :node_path, node.id, "browse/feature"
+    sidebar_browse_check :node_path, node.id, "browse/feature"
     assert_select ".secondary-actions a[href='#{node_history_path node}']", :count => 1
     assert_select ".secondary-actions a[href='#{old_node_path node, 1}']", :count => 1
     assert_select ".secondary-actions a[href='#{old_node_path node, 2}']", :count => 1
@@ -113,7 +109,7 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
 
   def test_read_deleted_node
     node = create(:node, :visible => false)
-    browse_check :node_path, node.id, "browse/feature"
+    sidebar_browse_check :node_path, node.id, "browse/feature"
     assert_select "h4", /^Version/ do
       assert_select "a[href='#{old_node_path node, 1}']", :text => "1", :count => 1
     end
@@ -122,35 +118,12 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
 
   def test_read_node_history
     node = create(:node, :with_history)
-    browse_check :node_history_path, node.id, "browse/history"
+    sidebar_browse_check :node_history_path, node.id, "browse/history"
     assert_select "h4", /^Version/ do
       assert_select "a[href='#{old_node_path node, 1}']", :text => "1", :count => 1
     end
   end
 
-  def test_read_changeset
-    user = create(:user)
-    changeset = create(:changeset, :user => user)
-    create(:changeset, :user => user)
-    browse_check :changeset_path, changeset.id, "browse/changeset"
-  end
-
-  def test_read_private_changeset
-    user = create(:user)
-    changeset = create(:changeset, :user => create(:user, :data_public => false))
-    create(:changeset, :user => user)
-    browse_check :changeset_path, changeset.id, "browse/changeset"
-  end
-
-  def test_read_changeset_element_links
-    changeset = create(:changeset)
-    node = create(:node, :with_history, :changeset => changeset)
-
-    browse_check :changeset_path, changeset.id, "browse/changeset"
-    assert_dom "a[href='#{node_path node}']", :count => 1
-    assert_dom "a[href='#{old_node_path node, 1}']", :count => 1
-  end
-
   ##
   #  Methods to check redaction.
   #
@@ -364,42 +337,4 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
     assert_select ".secondary-actions a", :text => "View History", :count => 1
     assert_select ".secondary-actions a", :text => "View Unredacted History", :count => 0
   end
-
-  private
-
-  # This is a convenience method for most of the above checks
-  # First we check that when we don't have an id, it will correctly return a 404
-  # then we check that we get the correct 404 when a non-existant id is passed
-  # then we check that it will get a successful response, when we do pass an id
-  def browse_check(path, id, template)
-    path_method = method(path)
-
-    assert_raise ActionController::UrlGenerationError do
-      get path_method.call
-    end
-
-    assert_raise ActionController::UrlGenerationError do
-      get path_method.call(:id => -10) # we won't have an id that's negative
-    end
-
-    get path_method.call(:id => 0)
-    assert_response :not_found
-    assert_template "browse/not_found"
-    assert_template :layout => "map"
-
-    get path_method.call(:id => 0), :xhr => true
-    assert_response :not_found
-    assert_template "browse/not_found"
-    assert_template :layout => "xhr"
-
-    get path_method.call(:id => id)
-    assert_response :success
-    assert_template template
-    assert_template :layout => "map"
-
-    get path_method.call(:id => id), :xhr => true
-    assert_response :success
-    assert_template template
-    assert_template :layout => "xhr"
-  end
 end
diff --git a/test/controllers/changesets_controller_test.rb b/test/controllers/changesets_controller_test.rb
index a0747a0cd..ce16b1406 100644
--- a/test/controllers/changesets_controller_test.rb
+++ b/test/controllers/changesets_controller_test.rb
@@ -4,6 +4,10 @@ class ChangesetsControllerTest < ActionDispatch::IntegrationTest
   ##
   # test all routes which lead to this controller
   def test_routes
+    assert_routing(
+      { :path => "/changeset/1", :method => :get },
+      { :controller => "changesets", :action => "show", :id => "1" }
+    )
     assert_routing(
       { :path => "/user/name/history", :method => :get },
       { :controller => "changesets", :action => "index", :display_name => "name" }
@@ -121,8 +125,9 @@ class ChangesetsControllerTest < ActionDispatch::IntegrationTest
   # Checks the display of the user changesets listing
   def test_index_user
     user = create(:user)
-    create(:changeset, :user => user)
-    create(:changeset, :closed, :user => user)
+    create(:changeset, :user => user, :num_changes => 1)
+    create(:changeset, :closed, :user => user, :num_changes => 1)
+    user.reload
 
     get history_path(:format => "html", :display_name => user.display_name)
     assert_response :success
@@ -252,6 +257,67 @@ class ChangesetsControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
   end
 
+  def test_show
+    changeset = create(:changeset)
+    create(:changeset_tag, :changeset => changeset, :k => "comment", :v => "tested-changeset-comment")
+    commenting_user = create(:user)
+    changeset_comment = create(:changeset_comment, :changeset => changeset, :author => commenting_user, :body => "Unwanted comment")
+
+    sidebar_browse_check :changeset_path, changeset.id, "changesets/show"
+    assert_dom "h2", :text => "Changeset: #{changeset.id}"
+    assert_dom "p", :text => "tested-changeset-comment"
+    assert_dom "li#c#{changeset_comment.id}" do
+      assert_dom "> small", :text => /^Comment from #{commenting_user.display_name}/
+    end
+  end
+
+  def test_show_closed_changeset
+    changeset = create(:changeset, :closed)
+
+    sidebar_browse_check :changeset_path, changeset.id, "changesets/show"
+  end
+
+  def test_show_private_changeset
+    user = create(:user)
+    changeset = create(:changeset, :user => create(:user, :data_public => false))
+    create(:changeset, :user => user)
+
+    sidebar_browse_check :changeset_path, changeset.id, "changesets/show"
+  end
+
+  def test_show_element_links
+    changeset = create(:changeset)
+    node = create(:node, :with_history, :changeset => changeset)
+    way = create(:way, :with_history, :changeset => changeset)
+    relation = create(:relation, :with_history, :changeset => changeset)
+
+    sidebar_browse_check :changeset_path, changeset.id, "changesets/show"
+    assert_dom "a[href='#{node_path node}']", :count => 1
+    assert_dom "a[href='#{old_node_path node, 1}']", :count => 1
+    assert_dom "a[href='#{way_path way}']", :count => 1
+    assert_dom "a[href='#{old_way_path way, 1}']", :count => 1
+    assert_dom "a[href='#{relation_path relation}']", :count => 1
+    assert_dom "a[href='#{old_relation_path relation, 1}']", :count => 1
+  end
+
+  def test_show_paginated_element_links
+    page_size = 20
+    changeset = create(:changeset)
+    nodes = create_list(:node, page_size + 1, :with_history, :changeset => changeset)
+    ways = create_list(:way, page_size + 1, :with_history, :changeset => changeset)
+    relations = create_list(:relation, page_size + 1, :with_history, :changeset => changeset)
+
+    sidebar_browse_check :changeset_path, changeset.id, "changesets/show"
+    page_size.times do |i|
+      assert_dom "a[href='#{node_path nodes[i]}']", :count => 1
+      assert_dom "a[href='#{old_node_path nodes[i], 1}']", :count => 1
+      assert_dom "a[href='#{way_path ways[i]}']", :count => 1
+      assert_dom "a[href='#{old_way_path ways[i], 1}']", :count => 1
+      assert_dom "a[href='#{relation_path relations[i]}']", :count => 1
+      assert_dom "a[href='#{old_relation_path relations[i], 1}']", :count => 1
+    end
+  end
+
   ##
   # This should display the last 20 non-empty changesets
   def test_feed
diff --git a/test/controllers/site_controller_test.rb b/test/controllers/site_controller_test.rb
index cc155155f..eff9ce093 100644
--- a/test/controllers/site_controller_test.rb
+++ b/test/controllers/site_controller_test.rb
@@ -147,7 +147,7 @@ class SiteControllerTest < ActionDispatch::IntegrationTest
 
     get permalink_path(:code => "wBz3--", :changeset => 4)
     assert_response :redirect
-    assert_redirected_to :controller => :browse, :action => :changeset, :id => 4, :anchor => "map=3/4.8779296875/3.955078125"
+    assert_redirected_to changeset_path(:id => 4, :anchor => "map=3/4.8779296875/3.955078125")
   end
 
   # Test the key page
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 60edf6e0c..1d04f57da 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -383,5 +383,41 @@ module ActiveSupport
     ensure
       unfreeze_time
     end
+
+    # This is a convenience method for checks of resources rendered in a map view sidebar
+    # First we check that when we don't have an id, it will correctly return a 404
+    # then we check that we get the correct 404 when a non-existant id is passed
+    # then we check that it will get a successful response, when we do pass an id
+    def sidebar_browse_check(path, id, template)
+      path_method = method(path)
+
+      assert_raise ActionController::UrlGenerationError do
+        get path_method.call
+      end
+
+      assert_raise ActionController::UrlGenerationError do
+        get path_method.call(:id => -10) # we won't have an id that's negative
+      end
+
+      get path_method.call(:id => 0)
+      assert_response :not_found
+      assert_template "browse/not_found"
+      assert_template :layout => "map"
+
+      get path_method.call(:id => 0), :xhr => true
+      assert_response :not_found
+      assert_template "browse/not_found"
+      assert_template :layout => "xhr"
+
+      get path_method.call(:id => id)
+      assert_response :success
+      assert_template template
+      assert_template :layout => "map"
+
+      get path_method.call(:id => id), :xhr => true
+      assert_response :success
+      assert_template template
+      assert_template :layout => "xhr"
+    end
   end
 end