Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename api old element actions and path helpers #4557

Merged
merged 2 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions app/abilities/api_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ def initialize(user)
can [:index, :show], Node
can [:index, :show, :full, :ways_for_node], Way
can [:index, :show, :full, :relations_for_node, :relations_for_way, :relations_for_relation], Relation
can [:history, :version], OldNode
can [:history, :version], OldWay
can [:history, :version], OldRelation
can [:history, :show], [OldNode, OldWay, OldRelation]
can [:show], UserBlock

if user&.active?
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/old_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class OldController < ApiController

before_action :check_api_readable
before_action :check_api_writable, :only => [:redact]
before_action :setup_user_auth, :only => [:history, :version]
before_action :setup_user_auth, :only => [:history, :show]
before_action :authorize, :only => [:redact]

authorize_resource
Expand Down Expand Up @@ -38,7 +38,7 @@ def history
end
end

def version
def show
if @old_element.redacted? && !show_redactions?
head :forbidden

Expand Down
2 changes: 1 addition & 1 deletion app/views/browse/_version_actions.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<%= link_to t("browse.view_details"), :controller => :browse, :action => @type %>
<% if !@feature.redacted? %>
&middot;
<%= link_to t("browse.download_xml"), :controller => "api/old_#{@type.pluralize}", :action => :version %>
<%= link_to t("browse.download_xml"), :controller => "api/old_#{@type.pluralize}", :action => :show %>
<% elsif current_user&.moderator? %>
&middot;
<% if !params[:show_redactions] %>
Expand Down
6 changes: 3 additions & 3 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
get "node/:id/relations" => "api/relations#relations_for_node", :as => :node_relations, :id => /\d+/
get "node/:id/history" => "api/old_nodes#history", :as => :api_node_history, :id => /\d+/
post "node/:id/:version/redact" => "api/old_nodes#redact", :as => :node_version_redact, :version => /\d+/, :id => /\d+/
get "node/:id/:version" => "api/old_nodes#version", :as => :node_version, :id => /\d+/, :version => /\d+/
get "node/:id/:version" => "api/old_nodes#show", :as => :api_old_node, :id => /\d+/, :version => /\d+/
get "node/:id" => "api/nodes#show", :as => :api_node, :id => /\d+/
put "node/:id" => "api/nodes#update", :id => /\d+/
delete "node/:id" => "api/nodes#delete", :id => /\d+/
Expand All @@ -46,7 +46,7 @@
get "way/:id/full" => "api/ways#full", :as => :way_full, :id => /\d+/
get "way/:id/relations" => "api/relations#relations_for_way", :as => :way_relations, :id => /\d+/
post "way/:id/:version/redact" => "api/old_ways#redact", :as => :way_version_redact, :version => /\d+/, :id => /\d+/
get "way/:id/:version" => "api/old_ways#version", :as => :way_version, :id => /\d+/, :version => /\d+/
get "way/:id/:version" => "api/old_ways#show", :as => :api_old_way, :id => /\d+/, :version => /\d+/
get "way/:id" => "api/ways#show", :as => :api_way, :id => /\d+/
put "way/:id" => "api/ways#update", :id => /\d+/
delete "way/:id" => "api/ways#delete", :id => /\d+/
Expand All @@ -57,7 +57,7 @@
get "relation/:id/history" => "api/old_relations#history", :as => :api_relation_history, :id => /\d+/
get "relation/:id/full" => "api/relations#full", :as => :relation_full, :id => /\d+/
post "relation/:id/:version/redact" => "api/old_relations#redact", :as => :relation_version_redact, :version => /\d+/, :id => /\d+/
get "relation/:id/:version" => "api/old_relations#version", :as => :relation_version, :id => /\d+/, :version => /\d+/
get "relation/:id/:version" => "api/old_relations#show", :as => :api_old_relation, :id => /\d+/, :version => /\d+/
get "relation/:id" => "api/relations#show", :as => :api_relation, :id => /\d+/
put "relation/:id" => "api/relations#update", :id => /\d+/
delete "relation/:id" => "api/relations#delete", :id => /\d+/
Expand Down
26 changes: 13 additions & 13 deletions test/controllers/api/old_nodes_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ def test_routes
)
assert_routing(
{ :path => "/api/0.6/node/1/2", :method => :get },
{ :controller => "api/old_nodes", :action => "version", :id => "1", :version => "2" }
{ :controller => "api/old_nodes", :action => "show", :id => "1", :version => "2" }
)
assert_routing(
{ :path => "/api/0.6/node/1/history.json", :method => :get },
{ :controller => "api/old_nodes", :action => "history", :id => "1", :format => "json" }
)
assert_routing(
{ :path => "/api/0.6/node/1/2.json", :method => :get },
{ :controller => "api/old_nodes", :action => "version", :id => "1", :version => "2", :format => "json" }
{ :controller => "api/old_nodes", :action => "show", :id => "1", :version => "2", :format => "json" }
)
assert_routing(
{ :path => "/api/0.6/node/1/2/redact", :method => :post },
Expand Down Expand Up @@ -142,7 +142,7 @@ def test_version

# check all the versions
versions.each_key do |key|
get node_version_path(:id => nodeid, :version => key.to_i)
get api_old_node_path(:id => nodeid, :version => key.to_i)

assert_response :success,
"couldn't get version #{key.to_i} of node #{nodeid}"
Expand Down Expand Up @@ -283,12 +283,12 @@ def test_version_redacted
node_v1 = node.old_nodes.find_by(:version => 1)
node_v1.redact!(create(:redaction))

get node_version_path(:id => node_v1.node_id, :version => node_v1.version)
get api_old_node_path(:id => node_v1.node_id, :version => node_v1.version)
assert_response :forbidden, "Redacted node shouldn't be visible via the version API."

# not even to a logged-in user
auth_header = basic_authorization_header create(:user).email, "test"
get node_version_path(:id => node_v1.node_id, :version => node_v1.version), :headers => auth_header
get api_old_node_path(:id => node_v1.node_id, :version => node_v1.version), :headers => auth_header
assert_response :forbidden, "Redacted node shouldn't be visible via the version API, even when logged in."
end

Expand Down Expand Up @@ -325,9 +325,9 @@ def test_redact_node_moderator

# check moderator can still see the redacted data, when passing
# the appropriate flag
get node_version_path(:id => node_v3.node_id, :version => node_v3.version), :headers => auth_header
get api_old_node_path(:id => node_v3.node_id, :version => node_v3.version), :headers => auth_header
assert_response :forbidden, "After redaction, node should be gone for moderator, when flag not passed."
get node_version_path(:id => node_v3.node_id, :version => node_v3.version), :params => { :show_redactions => "true" }, :headers => auth_header
get api_old_node_path(:id => node_v3.node_id, :version => node_v3.version), :params => { :show_redactions => "true" }, :headers => auth_header
assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."

# and when accessed via history
Expand Down Expand Up @@ -355,7 +355,7 @@ def test_redact_node_is_redacted
auth_header = basic_authorization_header create(:user).email, "test"

# check can't see the redacted data
get node_version_path(:id => node_v3.node_id, :version => node_v3.version), :headers => auth_header
get api_old_node_path(:id => node_v3.node_id, :version => node_v3.version), :headers => auth_header
assert_response :forbidden, "Redacted node shouldn't be visible via the version API."

# and when accessed via history
Expand Down Expand Up @@ -408,7 +408,7 @@ def test_unredact_node_moderator

# check moderator can now see the redacted data, when not
# passing the aspecial flag
get node_version_path(:id => node_v1.node_id, :version => node_v1.version), :headers => auth_header
get api_old_node_path(:id => node_v1.node_id, :version => node_v1.version), :headers => auth_header
assert_response :success, "After unredaction, node should not be gone for moderator."

# and when accessed via history
Expand All @@ -420,7 +420,7 @@ def test_unredact_node_moderator
auth_header = basic_authorization_header create(:user).email, "test"

# check normal user can now see the redacted data
get node_version_path(:id => node_v1.node_id, :version => node_v1.version), :headers => auth_header
get api_old_node_path(:id => node_v1.node_id, :version => node_v1.version), :headers => auth_header
assert_response :success, "After unredaction, node should be visible to normal users."

# and when accessed via history
Expand All @@ -446,7 +446,7 @@ def do_redact_redactable_node(headers = {})
end

def do_redact_node(node, redaction, headers = {})
get node_version_path(:id => node.node_id, :version => node.version), :headers => headers
get api_old_node_path(:id => node.node_id, :version => node.version), :headers => headers
assert_response :success, "should be able to get version #{node.version} of node #{node.node_id}."

# now redact it
Expand All @@ -463,7 +463,7 @@ def check_current_version(node_id)
assert_not_nil current_node, "getting node #{node_id} returned nil"

# get the "old" version of the node from the old_node interface
get node_version_path(:id => node_id, :version => current_node.version)
get api_old_node_path(:id => node_id, :version => current_node.version)
assert_response :success, "cant get old node #{node_id}, v#{current_node.version}"
old_node = Node.from_xml(@response.body)

Expand All @@ -472,7 +472,7 @@ def check_current_version(node_id)
end

def check_not_found_id_version(id, version)
get node_version_path(:id => id, :version => version)
get api_old_node_path(:id => id, :version => version)
assert_response :not_found
rescue ActionController::UrlGenerationError => e
assert_match(/No route matches/, e.to_s)
Expand Down
22 changes: 11 additions & 11 deletions test/controllers/api/old_relations_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ def test_routes
)
assert_routing(
{ :path => "/api/0.6/relation/1/2", :method => :get },
{ :controller => "api/old_relations", :action => "version", :id => "1", :version => "2" }
{ :controller => "api/old_relations", :action => "show", :id => "1", :version => "2" }
)
assert_routing(
{ :path => "/api/0.6/relation/1/history.json", :method => :get },
{ :controller => "api/old_relations", :action => "history", :id => "1", :format => "json" }
)
assert_routing(
{ :path => "/api/0.6/relation/1/2.json", :method => :get },
{ :controller => "api/old_relations", :action => "version", :id => "1", :version => "2", :format => "json" }
{ :controller => "api/old_relations", :action => "show", :id => "1", :version => "2", :format => "json" }
)
assert_routing(
{ :path => "/api/0.6/relation/1/2/redact", :method => :post },
Expand Down Expand Up @@ -122,12 +122,12 @@ def test_version_redacted
relation_v1 = relation.old_relations.find_by(:version => 1)
relation_v1.redact!(create(:redaction))

get relation_version_path(:id => relation_v1.relation_id, :version => relation_v1.version)
get api_old_relation_path(:id => relation_v1.relation_id, :version => relation_v1.version)
assert_response :forbidden, "Redacted relation shouldn't be visible via the version API."

# not even to a logged-in user
auth_header = basic_authorization_header create(:user).email, "test"
get relation_version_path(:id => relation_v1.relation_id, :version => relation_v1.version), :headers => auth_header
get api_old_relation_path(:id => relation_v1.relation_id, :version => relation_v1.version), :headers => auth_header
assert_response :forbidden, "Redacted relation shouldn't be visible via the version API, even when logged in."
end

Expand All @@ -145,7 +145,7 @@ def test_history_redacted

# not even to a logged-in user
auth_header = basic_authorization_header create(:user).email, "test"
get relation_version_path(:id => relation_v1.relation_id, :version => relation_v1.version), :headers => auth_header
get api_old_relation_path(:id => relation_v1.relation_id, :version => relation_v1.version), :headers => auth_header
get api_relation_history_path(:id => relation_v1.relation_id), :headers => auth_header
assert_response :success, "Redaction shouldn't have stopped history working."
assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 0,
Expand All @@ -166,9 +166,9 @@ def test_redact_relation_moderator

# check moderator can still see the redacted data, when passing
# the appropriate flag
get relation_version_path(:id => relation_v3.relation_id, :version => relation_v3.version), :headers => auth_header
get api_old_relation_path(:id => relation_v3.relation_id, :version => relation_v3.version), :headers => auth_header
assert_response :forbidden, "After redaction, relation should be gone for moderator, when flag not passed."
get relation_version_path(:id => relation_v3.relation_id, :version => relation_v3.version), :params => { :show_redactions => "true" }, :headers => auth_header
get api_old_relation_path(:id => relation_v3.relation_id, :version => relation_v3.version), :params => { :show_redactions => "true" }, :headers => auth_header
assert_response :success, "After redaction, relation should not be gone for moderator, when flag passed."

# and when accessed via history
Expand Down Expand Up @@ -197,7 +197,7 @@ def test_redact_relation_is_redacted
auth_header = basic_authorization_header create(:user).email, "test"

# check can't see the redacted data
get relation_version_path(:id => relation_v3.relation_id, :version => relation_v3.version), :headers => auth_header
get api_old_relation_path(:id => relation_v3.relation_id, :version => relation_v3.version), :headers => auth_header
assert_response :forbidden, "Redacted relation shouldn't be visible via the version API."

# and when accessed via history
Expand Down Expand Up @@ -248,7 +248,7 @@ def test_unredact_relation_moderator

# check moderator can still see the redacted data, without passing
# the appropriate flag
get relation_version_path(:id => relation_v1.relation_id, :version => relation_v1.version), :headers => auth_header
get api_old_relation_path(:id => relation_v1.relation_id, :version => relation_v1.version), :headers => auth_header
assert_response :success, "After unredaction, relation should not be gone for moderator."

# and when accessed via history
Expand All @@ -260,7 +260,7 @@ def test_unredact_relation_moderator
auth_header = basic_authorization_header create(:user).email, "test"

# check normal user can now see the redacted data
get relation_version_path(:id => relation_v1.relation_id, :version => relation_v1.version), :headers => auth_header
get api_old_relation_path(:id => relation_v1.relation_id, :version => relation_v1.version), :headers => auth_header
assert_response :success, "After redaction, node should not be gone for normal user."

# and when accessed via history
Expand Down Expand Up @@ -329,7 +329,7 @@ def do_redact_redactable_relation(headers = {})
end

def do_redact_relation(relation, redaction, headers = {})
get relation_version_path(:id => relation.relation_id, :version => relation.version)
get api_old_relation_path(:id => relation.relation_id, :version => relation.version)
assert_response :success, "should be able to get version #{relation.version} of relation #{relation.relation_id}."

# now redact it
Expand Down
Loading