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~747
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/664d02982cbaa8b1223ef03047b6134ff1ffbdac?ds=sidebyside;hp=-c

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

664d02982cbaa8b1223ef03047b6134ff1ffbdac
diff --combined app/controllers/browse_controller.rb
index 82cbe6f98,e842d4872..db291f6eb
--- a/app/controllers/browse_controller.rb
+++ b/app/controllers/browse_controller.rb
@@@ -6,6 -6,7 +6,7 @@@ class BrowseController < ApplicationCon
    before_action -> { check_database_readable(:need_api => true) }
    before_action :require_oauth
    before_action :update_totp, :only => [:query]
+   before_action :require_moderator_for_unredacted_history, :only => [:relation_history, :way_history, :node_history]
    around_action :web_timeout
    authorize_resource :class => false
  
@@@ -57,5 -58,30 +58,11 @@@
      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
+ 
+   def require_moderator_for_unredacted_history
+     deny_access(nil) if params[:show_redactions] && !current_user&.moderator?
+   end
  end
diff --combined config/locales/en.yml
index 6db6e77b4,fcf9f90a8..429786fcc
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@@ -308,9 -308,16 +308,10 @@@ 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"
+     redacted_version: "Redacted Version"
      in_changeset: "Changeset"
      anonymous: "anonymous"
      no_comment: "(no comment)"
@@@ -323,10 -330,29 +324,13 @@@
        other: "%{count} ways"
      download_xml: "Download XML"
      view_history: "View History"
+     view_unredacted_history: "View Unredacted History"
      view_details: "View Details"
+     view_redacted_data: "View Redacted Data"
+     view_redaction_message: "View Redaction Message"
      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}"
@@@ -442,9 -468,6 +446,9 @@@
        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
@@@ -458,28 -481,6 +462,28 @@@
        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 --combined test/controllers/browse_controller_test.rb
index e74345f6a,312caca51..fcdd7c752
--- a/test/controllers/browse_controller_test.rb
+++ b/test/controllers/browse_controller_test.rb
@@@ -28,6 -28,10 +28,6 @@@ class BrowseControllerTest < ActionDisp
        { :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" }
@@@ -36,7 -40,7 +36,7 @@@
  
    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
@@@ -47,7 -51,7 +47,7 @@@
  
    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
@@@ -55,7 -59,7 +55,7 @@@
  
    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
@@@ -63,7 -67,7 +63,7 @@@
  
    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
@@@ -74,7 -78,7 +74,7 @@@
  
    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
@@@ -82,7 -86,7 +82,7 @@@
  
    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
@@@ -90,7 -94,7 +90,7 @@@
  
    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
@@@ -101,7 -105,7 +101,7 @@@
  
    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
@@@ -109,7 -113,7 +109,7 @@@
  
    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
@@@ -118,12 -122,35 +118,12 @@@
  
    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.
    #
@@@ -166,6 -193,21 +166,21 @@@
      assert_select ".browse-section.browse-node .longitude", 0
    end
  
+   def test_redacted_node_unredacted_history
+     session_for(create(:moderator_user))
+     node = create(:node, :with_history, :deleted, :version => 2)
+     node_v1 = node.old_nodes.find_by(:version => 1)
+     node_v1.redact!(create(:redaction))
+ 
+     get node_history_path(:id => node, :params => { :show_redactions => true })
+     assert_response :success
+     assert_template "browse/history"
+ 
+     assert_select ".browse-section", 2
+     assert_select ".browse-section.browse-redacted", 0
+     assert_select ".browse-section.browse-node", 2
+   end
+ 
    def test_redacted_way_history
      way = create(:way, :with_history, :version => 4)
      way_v1 = way.old_ways.find_by(:version => 1)
@@@ -184,6 -226,23 +199,23 @@@
      assert_select ".browse-section.browse-way", 2
    end
  
+   def test_redacted_way_unredacted_history
+     session_for(create(:moderator_user))
+     way = create(:way, :with_history, :version => 4)
+     way_v1 = way.old_ways.find_by(:version => 1)
+     way_v1.redact!(create(:redaction))
+     way_v3 = way.old_ways.find_by(:version => 3)
+     way_v3.redact!(create(:redaction))
+ 
+     get way_history_path(:id => way, :params => { :show_redactions => true })
+     assert_response :success
+     assert_template "browse/history"
+ 
+     assert_select ".browse-section", 4
+     assert_select ".browse-section.browse-redacted", 0
+     assert_select ".browse-section.browse-way", 4
+   end
+ 
    def test_redacted_relation_history
      relation = create(:relation, :with_history, :version => 4)
      relation_v1 = relation.old_relations.find_by(:version => 1)
@@@ -202,9 -261,145 +234,107 @@@
      assert_select ".browse-section.browse-relation", 2
    end
  
+   def test_redacted_relation_unredacted_history
+     session_for(create(:moderator_user))
+     relation = create(:relation, :with_history, :version => 4)
+     relation_v1 = relation.old_relations.find_by(:version => 1)
+     relation_v1.redact!(create(:redaction))
+     relation_v3 = relation.old_relations.find_by(:version => 3)
+     relation_v3.redact!(create(:redaction))
+ 
+     get relation_history_path(:id => relation, :params => { :show_redactions => true })
+     assert_response :success
+     assert_template "browse/history"
+ 
+     assert_select ".browse-section", 4
+     assert_select ".browse-section.browse-redacted", 0
+     assert_select ".browse-section.browse-relation", 4
+   end
+ 
    def test_query
      get query_path
      assert_response :success
      assert_template "browse/query"
    end
+ 
+   def test_anonymous_user_feature_page_secondary_actions
+     node = create(:node, :with_history)
+     get node_path(:id => node)
+     assert_response :success
+     assert_select ".secondary-actions a", :text => "View Details", :count => 0
+     assert_select ".secondary-actions a", :text => "View History", :count => 1
+     assert_select ".secondary-actions a", :text => "View Unredacted History", :count => 0
+   end
+ 
+   def test_regular_user_feature_page_secondary_actions
+     session_for(create(:user))
+     node = create(:node, :with_history)
+     get node_path(:id => node)
+     assert_response :success
+     assert_select ".secondary-actions a", :text => "View Details", :count => 0
+     assert_select ".secondary-actions a", :text => "View History", :count => 1
+     assert_select ".secondary-actions a", :text => "View Unredacted History", :count => 0
+   end
+ 
+   def test_moderator_user_feature_page_secondary_actions
+     session_for(create(:moderator_user))
+     node = create(:node, :with_history)
+     get node_path(:id => node)
+     assert_response :success
+     assert_select ".secondary-actions a", :text => "View Details", :count => 0
+     assert_select ".secondary-actions a", :text => "View History", :count => 1
+     assert_select ".secondary-actions a", :text => "View Unredacted History", :count => 1
+   end
+ 
+   def test_anonymous_user_history_page_secondary_actions
+     node = create(:node, :with_history)
+     get node_history_path(:id => node)
+     assert_response :success
+     assert_select ".secondary-actions a", :text => "View Details", :count => 1
+     assert_select ".secondary-actions a", :text => "View History", :count => 0
+     assert_select ".secondary-actions a", :text => "View Unredacted History", :count => 0
+   end
+ 
+   def test_regular_user_history_page_secondary_actions
+     session_for(create(:user))
+     node = create(:node, :with_history)
+     get node_history_path(:id => node)
+     assert_response :success
+     assert_select ".secondary-actions a", :text => "View Details", :count => 1
+     assert_select ".secondary-actions a", :text => "View History", :count => 0
+     assert_select ".secondary-actions a", :text => "View Unredacted History", :count => 0
+   end
+ 
+   def test_moderator_user_history_page_secondary_actions
+     session_for(create(:moderator_user))
+     node = create(:node, :with_history)
+     get node_history_path(:id => node)
+     assert_response :success
+     assert_select ".secondary-actions a", :text => "View Details", :count => 1
+     assert_select ".secondary-actions a", :text => "View History", :count => 0
+     assert_select ".secondary-actions a", :text => "View Unredacted History", :count => 1
+   end
+ 
+   def test_anonymous_user_unredacted_history_page_secondary_actions
+     node = create(:node, :with_history)
+     get node_history_path(:id => node, :params => { :show_redactions => true })
+     assert_response :redirect
+   end
+ 
+   def test_regular_user_unredacted_history_page_secondary_actions
+     session_for(create(:user))
+     node = create(:node, :with_history)
+     get node_history_path(:id => node, :params => { :show_redactions => true })
+     assert_response :redirect
+   end
+ 
+   def test_moderator_user_unredacted_history_page_secondary_actions
+     session_for(create(:moderator_user))
+     node = create(:node, :with_history)
+     get node_history_path(:id => node, :params => { :show_redactions => true })
+     assert_response :success
+     assert_select ".secondary-actions a", :text => "View Details", :count => 1
+     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