From: Tom Hughes Date: Wed, 5 Feb 2025 18:26:32 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/5612' X-Git-Tag: live~158 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/4c3fb2c2b56fce7b31ce17d62fbed0c1b1195233?hp=5b1be7a79bff8a431ad701e3ca3448aa6e535c95 Merge remote-tracking branch 'upstream/pull/5612' --- diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index 13a453eb5..c74a4d099 100644 --- a/app/abilities/api_ability.rb +++ b/app/abilities/api_ability.rb @@ -38,7 +38,7 @@ class ApiAbility if user.terms_agreed? can [:create, :update, :upload, :close, :subscribe, :unsubscribe], Changeset if scope?(token, :write_api) can :create, ChangesetComment if scope?(token, :write_api) - can [:create, :update, :delete], [Node, Way, Relation] if scope?(token, :write_api) + can [:create, :update, :destroy], [Node, Way, Relation] if scope?(token, :write_api) end if user.moderator? diff --git a/app/assets/javascripts/osm.js.erb b/app/assets/javascripts/osm.js.erb index 254e00810..749cf5b77 100644 --- a/app/assets/javascripts/osm.js.erb +++ b/app/assets/javascripts/osm.js.erb @@ -4,45 +4,44 @@ //= depend_on key.yml OSM = { -<% if defined?(Settings.matomo) %> - MATOMO: <%= Settings.matomo.to_json %>, -<% end %> - - MAX_REQUEST_AREA: <%= Settings.max_request_area.to_json %>, - SERVER_PROTOCOL: <%= Settings.server_protocol.to_json %>, - SERVER_URL: <%= Settings.server_url.to_json %>, - API_VERSION: <%= Settings.api_version.to_json %>, - STATUS: <%= Settings.status.to_json %>, - MAX_NOTE_REQUEST_AREA: <%= Settings.max_note_request_area.to_json %>, - OVERPASS_URL: <%= Settings.overpass_url.to_json %>, - OVERPASS_CREDENTIALS: <%= Settings.overpass_credentials.to_json %>, - NOMINATIM_URL: <%= Settings.nominatim_url.to_json %>, - GRAPHHOPPER_URL: <%= Settings.graphhopper_url.to_json %>, - FOSSGIS_OSRM_URL: <%= Settings.fossgis_osrm_url.to_json %>, - FOSSGIS_VALHALLA_URL: <%= Settings.fossgis_valhalla_url.to_json %>, - DEFAULT_LOCALE: <%= I18n.default_locale.to_json %>, - -<% if Settings.key?(:thunderforest_key) %> - THUNDERFOREST_KEY: <%= Settings.thunderforest_key.to_json %>, -<% end %> - -<% if Settings.key?(:tracestrack_key) %> - TRACESTRACK_KEY: <%= Settings.tracestrack_key.to_json %>, -<% end %> - - LAYER_DEFINITIONS: <%= YAML.load_file(Rails.root.join("config/layers.yml")).to_json %>, - LAYERS_WITH_MAP_KEY: <%= YAML.load_file(Rails.root.join("config/key.yml")).keys.to_json %>, - - MARKER_GREEN: <%= image_path("marker-green.png").to_json %>, - MARKER_RED: <%= image_path("marker-red.png").to_json %>, - - MARKER_ICON: <%= image_path("leaflet/dist/images/marker-icon.png").to_json %>, - MARKER_ICON_2X: <%= image_path("leaflet/dist/images/marker-icon-2x.png").to_json %>, - MARKER_SHADOW: <%= image_path("leaflet/dist/images/marker-shadow.png").to_json %>, - - NEW_NOTE_MARKER: <%= image_path("new_note_marker.svg").to_json %>, - OPEN_NOTE_MARKER: <%= image_path("open_note_marker.svg").to_json %>, - CLOSED_NOTE_MARKER: <%= image_path("closed_note_marker.svg").to_json %>, + ...<%= + %i[ + matomo + max_request_area + server_protocol + server_url + api_version + status + max_note_request_area + overpass_url + overpass_credentials + nominatim_url + graphhopper_url + fossgis_osrm_url + fossgis_valhalla_url + thunderforest_key + tracestrack_key + ] + .each_with_object({}) do |key, hash| + hash[key.to_s.upcase] = Settings.send(key) if Settings.respond_to?(key) + end.to_json + %>, + + DEFAULT_LOCALE: <%= I18n.default_locale.to_json %>, + + LAYER_DEFINITIONS: <%= YAML.load_file(Rails.root.join("config/layers.yml")).to_json %>, + LAYERS_WITH_MAP_KEY: <%= YAML.load_file(Rails.root.join("config/key.yml")).keys.to_json %>, + + MARKER_GREEN: <%= image_path("marker-green.png").to_json %>, + MARKER_RED: <%= image_path("marker-red.png").to_json %>, + + MARKER_ICON: <%= image_path("leaflet/dist/images/marker-icon.png").to_json %>, + MARKER_ICON_2X: <%= image_path("leaflet/dist/images/marker-icon-2x.png").to_json %>, + MARKER_SHADOW: <%= image_path("leaflet/dist/images/marker-shadow.png").to_json %>, + + NEW_NOTE_MARKER: <%= image_path("new_note_marker.svg").to_json %>, + OPEN_NOTE_MARKER: <%= image_path("open_note_marker.svg").to_json %>, + CLOSED_NOTE_MARKER: <%= image_path("closed_note_marker.svg").to_json %>, apiUrl: function (object) { var apiType = object.type === "note" ? "notes" : object.type; @@ -58,28 +57,12 @@ OSM = { }, params: function (search) { - var params = {}; - - search = (search || window.location.search).replace("?", "").split(/&|;/); - - for (var i = 0; i < search.length; ++i) { - var pair = search[i], - j = pair.indexOf("="), - key = pair.slice(0, j), - val = pair.slice(++j); - - try { - params[key] = decodeURIComponent(val); - } catch (e) { - // Ignore parse exceptions - } - } - - return params; + var query = search || window.location.search; + return Object.fromEntries(new URLSearchParams(query)); }, mapParams: function (search) { - var params = OSM.params(search), mapParams = {}, match; + var params = OSM.params(search), mapParams = {}; if (params.mlon && params.mlat) { mapParams.marker = true; @@ -229,7 +212,7 @@ OSM = { return 6372795 * 2 * Math.asin( Math.sqrt( Math.pow(Math.sin(latdiff / 2), 2) + - Math.cos(lat1) * Math.cos(lat2) * Math.pow(Math.sin(lngdiff / 2), 2) + (Math.cos(lat1) * Math.cos(lat2) * Math.pow(Math.sin(lngdiff / 2), 2)) )); } }; diff --git a/app/assets/javascripts/user.js b/app/assets/javascripts/user.js index 1d167d977..9366115ce 100644 --- a/app/assets/javascripts/user.js +++ b/app/assets/javascripts/user.js @@ -64,7 +64,7 @@ $(document).ready(function () { map.on("click", function (e) { if (!$("#updatehome").is(":checked")) return; - const [lat, lon] = OSM.cropLocation(e.latlng); + const [lat, lon] = OSM.cropLocation(e.latlng, map.getZoom()); $("#home_lat").val(lat); $("#home_lon").val(lon); diff --git a/app/controllers/api/nodes_controller.rb b/app/controllers/api/nodes_controller.rb index 6477271d4..b7165b2fe 100644 --- a/app/controllers/api/nodes_controller.rb +++ b/app/controllers/api/nodes_controller.rb @@ -2,14 +2,14 @@ module Api class NodesController < ApiController - before_action :check_api_writable, :only => [:create, :update, :delete] - before_action :authorize, :only => [:create, :update, :delete] + before_action :check_api_writable, :only => [:create, :update, :destroy] + before_action :authorize, :only => [:create, :update, :destroy] authorize_resource - before_action :require_public_data, :only => [:create, :update, :delete] - before_action :set_request_formats, :except => [:create, :update, :delete] - before_action :check_rate_limit, :only => [:create, :update, :delete] + before_action :require_public_data, :only => [:create, :update, :destroy] + before_action :set_request_formats, :except => [:create, :update, :destroy] + before_action :check_rate_limit, :only => [:create, :update, :destroy] # Dump the details on many nodes whose ids are given in the "nodes" parameter. def index @@ -68,7 +68,7 @@ module Api # Delete a node. Doesn't actually delete it, but retains its history # in a wiki-like way. We therefore treat it like an update, so the delete # method returns the new version number. - def delete + def destroy node = Node.find(params[:id]) new_node = Node.from_xml(request.raw_post) diff --git a/app/controllers/api/relations_controller.rb b/app/controllers/api/relations_controller.rb index ae101f373..f712534f0 100644 --- a/app/controllers/api/relations_controller.rb +++ b/app/controllers/api/relations_controller.rb @@ -1,13 +1,13 @@ module Api class RelationsController < ApiController - before_action :check_api_writable, :only => [:create, :update, :delete] - before_action :authorize, :only => [:create, :update, :delete] + before_action :check_api_writable, :only => [:create, :update, :destroy] + before_action :authorize, :only => [:create, :update, :destroy] authorize_resource - before_action :require_public_data, :only => [:create, :update, :delete] - before_action :set_request_formats, :except => [:create, :update, :delete] - before_action :check_rate_limit, :only => [:create, :update, :delete] + before_action :require_public_data, :only => [:create, :update, :destroy] + before_action :set_request_formats, :except => [:create, :update, :destroy] + before_action :check_rate_limit, :only => [:create, :update, :destroy] def index raise OSM::APIBadUserInput, "The parameter relations is required, and must be of the form relations=id[,id[,id...]]" unless params["relations"] @@ -57,7 +57,7 @@ module Api render :plain => relation.version.to_s end - def delete + def destroy relation = Relation.find(params[:id]) new_relation = Relation.from_xml(request.raw_post) if new_relation && new_relation.id == relation.id diff --git a/app/controllers/api/ways_controller.rb b/app/controllers/api/ways_controller.rb index 632fdb9a6..285ed4604 100644 --- a/app/controllers/api/ways_controller.rb +++ b/app/controllers/api/ways_controller.rb @@ -1,13 +1,13 @@ module Api class WaysController < ApiController - before_action :check_api_writable, :only => [:create, :update, :delete] - before_action :authorize, :only => [:create, :update, :delete] + before_action :check_api_writable, :only => [:create, :update, :destroy] + before_action :authorize, :only => [:create, :update, :destroy] authorize_resource - before_action :require_public_data, :only => [:create, :update, :delete] - before_action :set_request_formats, :except => [:create, :update, :delete] - before_action :check_rate_limit, :only => [:create, :update, :delete] + before_action :require_public_data, :only => [:create, :update, :destroy] + before_action :set_request_formats, :except => [:create, :update, :destroy] + before_action :check_rate_limit, :only => [:create, :update, :destroy] def index raise OSM::APIBadUserInput, "The parameter ways is required, and must be of the form ways=id[,id[,id...]]" unless params["ways"] @@ -60,7 +60,7 @@ module Api end # This is the API call to delete a way - def delete + def destroy way = Way.find(params[:id]) new_way = Way.from_xml(request.raw_post) diff --git a/app/views/issues/_comments.html.erb b/app/views/issues/_comments.html.erb index f828e5a43..d7005d75e 100644 --- a/app/views/issues/_comments.html.erb +++ b/app/views/issues/_comments.html.erb @@ -21,7 +21,8 @@ <%= bootstrap_form_for @new_comment, :url => issue_comments_path(@issue) do |f| %> <%= f.richtext_field :body, :cols => 80, :rows => 20, :hide_label => true %> <%= f.form_group do %> - <%= f.check_box :reassign, { :label => t(".reassign_param"), :id => "reassign", :name => "reassign", :checked => false }, true, false %> + <%= f.check_box :reassign, { :label => @issue.assigned_role == "administrator" ? t(".reassign_to_moderators") : t(".reassign_to_administrators"), + :id => "reassign", :name => "reassign", :checked => false }, true, false %> <% end %> <%= f.primary %> <% end %> diff --git a/app/views/notes/show.html.erb b/app/views/notes/show.html.erb index 3d9b4a9ba..4f20cdd44 100644 --- a/app/views/notes/show.html.erb +++ b/app/views/notes/show.html.erb @@ -5,7 +5,7 @@

<%= t(".description") %>

- <%= h(note_description(@note.author, @note.description).to_html) %> + <%= note_description(@note.author, @note.description).to_html %>
diff --git a/config/eslint.js b/config/eslint.js index c9ef72e3e..cb421a992 100644 --- a/config/eslint.js +++ b/config/eslint.js @@ -1,9 +1,11 @@ const globals = require("globals"); const js = require("@eslint/js"); +const erb = require("eslint-plugin-erb"); const stylisticJs = require("@stylistic/eslint-plugin-js"); module.exports = [ js.configs.recommended, + erb.configs.recommended, { plugins: { "@stylistic": stylisticJs @@ -19,11 +21,19 @@ module.exports = [ L: "readonly", OSM: "writable", Matomo: "readonly", - Qs: "readonly", Turbo: "readonly", updateLinks: "readonly" } }, + linterOptions: { + // The "unused disable directive" is set to "warn" by default. + // For the ERB plugin to work correctly, you must disable + // this directive to avoid issues described here + // https://github.com/eslint/eslint/discussions/18114 + // If you're using the CLI, you might also use the following flag: + // --report-unused-disable-directives-severity=off + reportUnusedDisableDirectives: "off" + }, rules: { "@stylistic/array-bracket-newline": ["error", "consistent"], "@stylistic/array-bracket-spacing": "error", diff --git a/config/initializers/sanitize.rb b/config/initializers/sanitize.rb index c9d6a5dba..f8f2b784d 100644 --- a/config/initializers/sanitize.rb +++ b/config/initializers/sanitize.rb @@ -2,6 +2,10 @@ Sanitize::Config::OSM = Sanitize::Config.merge( Sanitize::Config::RELAXED, :elements => Sanitize::Config::RELAXED[:elements] - %w[div style], :remove_contents => %w[script style], + :attributes => Sanitize::Config.merge( + Sanitize::Config::RELAXED[:attributes], + "img" => Sanitize::Config::RELAXED[:attributes]["img"] + ["loading"] + ), :transformers => lambda do |env| style = env[:node]["style"] || "" @@ -24,5 +28,7 @@ Sanitize::Config::OSM = Sanitize::Config.merge( env[:node]["rel"] = rel.split.select { |r| r == "me" }.append("nofollow", "noopener", "noreferrer").sort.join(" ") end + + env[:node]["loading"] = "lazy" if env[:node_name] == "img" end ) diff --git a/config/locales/en.yml b/config/locales/en.yml index 40b4bfa14..25794cc9f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1551,7 +1551,8 @@ en: reopened: Issue status has been set to 'Open' comments: comment_from_html: "Comment from %{user_link} on %{comment_created_at}" - reassign_param: Reassign Issue? + reassign_to_moderators: Reassign Issue to Moderators + reassign_to_administrators: Reassign Issue to Administrators reports: reported_by_html: "Reported as %{category} by %{user} on %{updated_at}" helper: diff --git a/config/routes.rb b/config/routes.rb index 19dfc9a77..b562ca9f4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -30,41 +30,38 @@ OpenStreetMap::Application.routes.draw do post "changeset/comment/:id/hide" => "changeset_comments#destroy", :as => :changeset_comment_hide, :id => /\d+/ post "changeset/comment/:id/unhide" => "changeset_comments#restore", :as => :changeset_comment_unhide, :id => /\d+/ - put "node/create" => "nodes#create" get "node/:id/ways" => "ways#ways_for_node", :as => :node_ways, :id => /\d+/ get "node/:id/relations" => "relations#relations_for_node", :as => :node_relations, :id => /\d+/ get "node/:id/history" => "old_nodes#history", :as => :api_node_history, :id => /\d+/ post "node/:id/:version/redact" => "old_nodes#redact", :as => :node_version_redact, :version => /\d+/, :id => /\d+/ get "node/:id/:version" => "old_nodes#show", :as => :api_old_node, :id => /\d+/, :version => /\d+/ - get "node/:id" => "nodes#show", :as => :api_node, :id => /\d+/ - put "node/:id" => "nodes#update", :id => /\d+/ - delete "node/:id" => "nodes#delete", :id => /\d+/ - get "nodes" => "nodes#index" - put "way/create" => "ways#create" get "way/:id/history" => "old_ways#history", :as => :api_way_history, :id => /\d+/ get "way/:id/full" => "ways#full", :as => :way_full, :id => /\d+/ get "way/:id/relations" => "relations#relations_for_way", :as => :way_relations, :id => /\d+/ post "way/:id/:version/redact" => "old_ways#redact", :as => :way_version_redact, :version => /\d+/, :id => /\d+/ get "way/:id/:version" => "old_ways#show", :as => :api_old_way, :id => /\d+/, :version => /\d+/ - get "way/:id" => "ways#show", :as => :api_way, :id => /\d+/ - put "way/:id" => "ways#update", :id => /\d+/ - delete "way/:id" => "ways#delete", :id => /\d+/ - get "ways" => "ways#index" - put "relation/create" => "relations#create" get "relation/:id/relations" => "relations#relations_for_relation", :as => :relation_relations, :id => /\d+/ get "relation/:id/history" => "old_relations#history", :as => :api_relation_history, :id => /\d+/ get "relation/:id/full" => "relations#full", :as => :relation_full, :id => /\d+/ post "relation/:id/:version/redact" => "old_relations#redact", :as => :relation_version_redact, :version => /\d+/, :id => /\d+/ get "relation/:id/:version" => "old_relations#show", :as => :api_old_relation, :id => /\d+/, :version => /\d+/ - get "relation/:id" => "relations#show", :as => :api_relation, :id => /\d+/ - put "relation/:id" => "relations#update", :id => /\d+/ - delete "relation/:id" => "relations#delete", :id => /\d+/ - get "relations" => "relations#index" end namespace :api, :path => "api/0.6" do + resources :nodes, :only => [:index, :create] + resources :nodes, :path => "node", :id => /\d+/, :only => [:show, :update, :destroy] + put "node/create" => "nodes#create", :as => nil + + resources :ways, :only => [:index, :create] + resources :ways, :path => "way", :id => /\d+/, :only => [:show, :update, :destroy] + put "way/create" => "ways#create", :as => nil + + resources :relations, :only => [:index, :create] + resources :relations, :path => "relation", :id => /\d+/, :only => [:show, :update, :destroy] + put "relation/create" => "relations#create", :as => nil + resource :map, :only => :show resources :tracepoints, :path => "trackpoints", :only => :index diff --git a/lib/tasks/eslint.rake b/lib/tasks/eslint.rake index 790ef150d..27b550e40 100644 --- a/lib/tasks/eslint.rake +++ b/lib/tasks/eslint.rake @@ -10,7 +10,7 @@ end def js_files Rails.application.assets.each_file.select do |file| - file.ends_with?(".js") && !file.match?(%r{/(gems|vendor|i18n|node_modules)/}) + (file.ends_with?(".js") || file.ends_with?(".js.erb")) && !file.match?(%r{/(gems|vendor|i18n|node_modules)/}) end end diff --git a/package.json b/package.json index 1b331ec96..371d01aff 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,7 @@ }, "devDependencies": { "eslint": "^9.0.0", + "eslint-plugin-erb": "^2.1.0", "@stylistic/eslint-plugin-js": "^3.0.0", "eslint-formatter-compact": "^8.40.0" } diff --git a/test/controllers/api/changesets_controller_test.rb b/test/controllers/api/changesets_controller_test.rb index 5c0e7c9c2..39b1f3cf8 100644 --- a/test/controllers/api/changesets_controller_test.rb +++ b/test/controllers/api/changesets_controller_test.rb @@ -2130,7 +2130,7 @@ module Api # add a single node to it with_controller(NodesController.new) do xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header assert_response :success, "Couldn't create node." end @@ -2145,7 +2145,7 @@ module Api # add another node to it with_controller(NodesController.new) do xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header assert_response :success, "Couldn't create second node." end @@ -2515,7 +2515,7 @@ module Api with_controller(NodesController.new) do # create a new node xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header assert_response :success, "can't create a new node" node_id = @response.body.to_i diff --git a/test/controllers/api/nodes_controller_test.rb b/test/controllers/api/nodes_controller_test.rb index 9896c34a5..523498216 100644 --- a/test/controllers/api/nodes_controller_test.rb +++ b/test/controllers/api/nodes_controller_test.rb @@ -6,7 +6,15 @@ module Api # test all routes which lead to this controller def test_routes assert_routing( - { :path => "/api/0.6/node/create", :method => :put }, + { :path => "/api/0.6/nodes", :method => :get }, + { :controller => "api/nodes", :action => "index" } + ) + assert_routing( + { :path => "/api/0.6/nodes.json", :method => :get }, + { :controller => "api/nodes", :action => "index", :format => "json" } + ) + assert_routing( + { :path => "/api/0.6/nodes", :method => :post }, { :controller => "api/nodes", :action => "create" } ) assert_routing( @@ -23,18 +31,62 @@ module Api ) assert_routing( { :path => "/api/0.6/node/1", :method => :delete }, - { :controller => "api/nodes", :action => "delete", :id => "1" } + { :controller => "api/nodes", :action => "destroy", :id => "1" } ) - assert_routing( - { :path => "/api/0.6/nodes", :method => :get }, - { :controller => "api/nodes", :action => "index" } - ) - assert_routing( - { :path => "/api/0.6/nodes.json", :method => :get }, - { :controller => "api/nodes", :action => "index", :format => "json" } + + assert_recognizes( + { :controller => "api/nodes", :action => "create" }, + { :path => "/api/0.6/node/create", :method => :put } ) end + ## + # test fetching multiple nodes + def test_index + node1 = create(:node) + node2 = create(:node, :deleted) + node3 = create(:node) + node4 = create(:node, :with_history, :version => 2) + node5 = create(:node, :deleted, :with_history, :version => 2) + + # check error when no parameter provided + get api_nodes_path + assert_response :bad_request + + # check error when no parameter value provided + get api_nodes_path(:nodes => "") + assert_response :bad_request + + # test a working call + get api_nodes_path(:nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id}") + assert_response :success + assert_select "osm" do + assert_select "node", :count => 5 + assert_select "node[id='#{node1.id}'][visible='true']", :count => 1 + assert_select "node[id='#{node2.id}'][visible='false']", :count => 1 + assert_select "node[id='#{node3.id}'][visible='true']", :count => 1 + assert_select "node[id='#{node4.id}'][visible='true']", :count => 1 + assert_select "node[id='#{node5.id}'][visible='false']", :count => 1 + end + + # test a working call with json format + get api_nodes_path(:nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id}", :format => "json") + + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 5, js["elements"].count + assert_equal 5, (js["elements"].count { |a| a["type"] == "node" }) + assert_equal 1, (js["elements"].count { |a| a["id"] == node1.id && a["visible"].nil? }) + assert_equal 1, (js["elements"].count { |a| a["id"] == node2.id && a["visible"] == false }) + assert_equal 1, (js["elements"].count { |a| a["id"] == node3.id && a["visible"].nil? }) + assert_equal 1, (js["elements"].count { |a| a["id"] == node4.id && a["visible"].nil? }) + assert_equal 1, (js["elements"].count { |a| a["id"] == node5.id && a["visible"] == false }) + + # check error when a non-existent node is included + get api_nodes_path(:nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id},0") + assert_response :not_found + end + def test_create private_user = create(:user, :data_public => false) private_changeset = create(:changeset, :user => private_user) @@ -49,7 +101,7 @@ module Api # create a minimal xml file xml = "" assert_difference("OldNode.count", 0) do - put node_create_path, :params => xml + post api_nodes_path, :params => xml end # hope for unauthorized assert_response :unauthorized, "node upload did not return unauthorized status" @@ -60,7 +112,7 @@ module Api # create a minimal xml file xml = "" assert_difference("Node.count", 0) do - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header end # hope for success assert_require_public_data "node create did not return forbidden status" @@ -70,7 +122,7 @@ module Api # create a minimal xml file xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header # hope for success assert_response :success, "node upload did not return success status" @@ -98,14 +150,14 @@ module Api # test that the upload is rejected when xml is valid, but osm doc isn't xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header assert_response :bad_request, "node upload did not return bad_request status" assert_equal "Cannot parse valid node from xml string . XML doesn't contain an osm/node element.", @response.body # test that the upload is rejected when no lat is supplied # create a minimal xml file xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header # hope for success assert_response :bad_request, "node upload did not return bad_request status" assert_equal "Cannot parse valid node from xml string . lat missing", @response.body @@ -113,7 +165,7 @@ module Api # test that the upload is rejected when no lon is supplied # create a minimal xml file xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header # hope for success assert_response :bad_request, "node upload did not return bad_request status" assert_equal "Cannot parse valid node from xml string . lon missing", @response.body @@ -121,7 +173,7 @@ module Api # test that the upload is rejected when lat is non-numeric # create a minimal xml file xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header # hope for success assert_response :bad_request, "node upload did not return bad_request status" assert_equal "Cannot parse valid node from xml string . lat not a number", @response.body @@ -129,30 +181,36 @@ module Api # test that the upload is rejected when lon is non-numeric # create a minimal xml file xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header # hope for success assert_response :bad_request, "node upload did not return bad_request status" assert_equal "Cannot parse valid node from xml string . lon not a number", @response.body # test that the upload is rejected when we have a tag which is too long xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header assert_response :bad_request, "node upload did not return bad_request status" assert_match(/ v: is too long \(maximum is 255 characters\) /, @response.body) end - def test_show - # check that a visible node is returned properly - get api_node_path(create(:node)) - assert_response :success + def test_show_not_found + get api_node_path(0) + assert_response :not_found + end - # check that an deleted node is not returned + def test_show_deleted get api_node_path(create(:node, :deleted)) assert_response :gone + end - # check chat a non-existent node is not returned - get api_node_path(0) - assert_response :not_found + def test_show + node = create(:node, :timestamp => "2021-02-03T00:00:00Z") + + get api_node_path(node) + + assert_response :success + assert_not_nil @response.header["Last-Modified"] + assert_equal "2021-02-03T00:00:00Z", Time.parse(@response.header["Last-Modified"]).utc.xmlschema end # Ensure the lat/lon is formatted as a decimal e.g. not 4.0e-05 @@ -166,7 +224,7 @@ module Api # this tests deletion restrictions - basic deletion is tested in the unit # tests for node! - def test_delete + def test_destroy private_user = create(:user, :data_public => false) private_user_changeset = create(:changeset, :user => private_user) private_user_closed_changeset = create(:changeset, :closed, :user => private_user) @@ -424,53 +482,6 @@ module Api assert_response :success, "a valid update request failed" end - ## - # test fetching multiple nodes - def test_index - node1 = create(:node) - node2 = create(:node, :deleted) - node3 = create(:node) - node4 = create(:node, :with_history, :version => 2) - node5 = create(:node, :deleted, :with_history, :version => 2) - - # check error when no parameter provided - get nodes_path - assert_response :bad_request - - # check error when no parameter value provided - get nodes_path(:nodes => "") - assert_response :bad_request - - # test a working call - get nodes_path(:nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id}") - assert_response :success - assert_select "osm" do - assert_select "node", :count => 5 - assert_select "node[id='#{node1.id}'][visible='true']", :count => 1 - assert_select "node[id='#{node2.id}'][visible='false']", :count => 1 - assert_select "node[id='#{node3.id}'][visible='true']", :count => 1 - assert_select "node[id='#{node4.id}'][visible='true']", :count => 1 - assert_select "node[id='#{node5.id}'][visible='false']", :count => 1 - end - - # test a working call with json format - get nodes_path(:nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id}", :format => "json") - - js = ActiveSupport::JSON.decode(@response.body) - assert_not_nil js - assert_equal 5, js["elements"].count - assert_equal 5, (js["elements"].count { |a| a["type"] == "node" }) - assert_equal 1, (js["elements"].count { |a| a["id"] == node1.id && a["visible"].nil? }) - assert_equal 1, (js["elements"].count { |a| a["id"] == node2.id && a["visible"] == false }) - assert_equal 1, (js["elements"].count { |a| a["id"] == node3.id && a["visible"].nil? }) - assert_equal 1, (js["elements"].count { |a| a["id"] == node4.id && a["visible"].nil? }) - assert_equal 1, (js["elements"].count { |a| a["id"] == node5.id && a["visible"] == false }) - - # check error when a non-existent node is included - get nodes_path(:nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id},0") - assert_response :not_found - end - ## # test adding tags to a node def test_duplicate_tags @@ -510,7 +521,7 @@ module Api xml = "" \ "" \ "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header assert_require_public_data "Shouldn't be able to create with non-public user" ## Then try with the public data user @@ -521,7 +532,7 @@ module Api xml = "" \ "" \ "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header assert_response :success nodeid = @response.body @@ -556,7 +567,7 @@ module Api # try creating a node xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header assert_response :success, "node create did not return success status" # get the id of the node we created @@ -574,7 +585,7 @@ module Api # try creating a node, which should be rate limited xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header assert_response :too_many_requests, "node create did not hit rate limit" end @@ -603,7 +614,7 @@ module Api # try creating a node xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header assert_response :success, "node create did not return success status" # get the id of the node we created @@ -621,7 +632,7 @@ module Api # try creating a node, which should be rate limited xml = "" - put node_create_path, :params => xml, :headers => auth_header + post api_nodes_path, :params => xml, :headers => auth_header assert_response :too_many_requests, "node create did not hit rate limit" end diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb index 5fb62d29f..34953f7b7 100644 --- a/test/controllers/api/relations_controller_test.rb +++ b/test/controllers/api/relations_controller_test.rb @@ -6,16 +6,16 @@ module Api # test all routes which lead to this controller def test_routes assert_routing( - { :path => "/api/0.6/relation/create", :method => :put }, - { :controller => "api/relations", :action => "create" } + { :path => "/api/0.6/relations", :method => :get }, + { :controller => "api/relations", :action => "index" } ) assert_routing( - { :path => "/api/0.6/relation/1/full", :method => :get }, - { :controller => "api/relations", :action => "full", :id => "1" } + { :path => "/api/0.6/relations.json", :method => :get }, + { :controller => "api/relations", :action => "index", :format => "json" } ) assert_routing( - { :path => "/api/0.6/relation/1/full.json", :method => :get }, - { :controller => "api/relations", :action => "full", :id => "1", :format => "json" } + { :path => "/api/0.6/relations", :method => :post }, + { :controller => "api/relations", :action => "create" } ) assert_routing( { :path => "/api/0.6/relation/1", :method => :get }, @@ -26,20 +26,20 @@ module Api { :controller => "api/relations", :action => "show", :id => "1", :format => "json" } ) assert_routing( - { :path => "/api/0.6/relation/1", :method => :put }, - { :controller => "api/relations", :action => "update", :id => "1" } + { :path => "/api/0.6/relation/1/full", :method => :get }, + { :controller => "api/relations", :action => "full", :id => "1" } ) assert_routing( - { :path => "/api/0.6/relation/1", :method => :delete }, - { :controller => "api/relations", :action => "delete", :id => "1" } + { :path => "/api/0.6/relation/1/full.json", :method => :get }, + { :controller => "api/relations", :action => "full", :id => "1", :format => "json" } ) assert_routing( - { :path => "/api/0.6/relations", :method => :get }, - { :controller => "api/relations", :action => "index" } + { :path => "/api/0.6/relation/1", :method => :put }, + { :controller => "api/relations", :action => "update", :id => "1" } ) assert_routing( - { :path => "/api/0.6/relations.json", :method => :get }, - { :controller => "api/relations", :action => "index", :format => "json" } + { :path => "/api/0.6/relation/1", :method => :delete }, + { :controller => "api/relations", :action => "destroy", :id => "1" } ) assert_routing( @@ -66,26 +66,144 @@ module Api { :path => "/api/0.6/relation/1/relations.json", :method => :get }, { :controller => "api/relations", :action => "relations_for_relation", :id => "1", :format => "json" } ) + + assert_recognizes( + { :controller => "api/relations", :action => "create" }, + { :path => "/api/0.6/relation/create", :method => :put } + ) + end + + ## + # test fetching multiple relations + def test_index + relation1 = create(:relation) + relation2 = create(:relation, :deleted) + relation3 = create(:relation, :with_history, :version => 2) + relation4 = create(:relation, :with_history, :version => 2) + relation4.old_relations.find_by(:version => 1).redact!(create(:redaction)) + + # check error when no parameter provided + get api_relations_path + assert_response :bad_request + + # check error when no parameter value provided + get api_relations_path(:relations => "") + assert_response :bad_request + + # test a working call + get api_relations_path(:relations => "#{relation1.id},#{relation2.id},#{relation3.id},#{relation4.id}") + assert_response :success + assert_select "osm" do + assert_select "relation", :count => 4 + assert_select "relation[id='#{relation1.id}'][visible='true']", :count => 1 + assert_select "relation[id='#{relation2.id}'][visible='false']", :count => 1 + assert_select "relation[id='#{relation3.id}'][visible='true']", :count => 1 + assert_select "relation[id='#{relation4.id}'][visible='true']", :count => 1 + end + + # test a working call with json format + get api_relations_path(:relations => "#{relation1.id},#{relation2.id},#{relation3.id},#{relation4.id}", :format => "json") + + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 4, js["elements"].count + assert_equal 4, (js["elements"].count { |a| a["type"] == "relation" }) + assert_equal 1, (js["elements"].count { |a| a["id"] == relation1.id && a["visible"].nil? }) + assert_equal 1, (js["elements"].count { |a| a["id"] == relation2.id && a["visible"] == false }) + assert_equal 1, (js["elements"].count { |a| a["id"] == relation3.id && a["visible"].nil? }) + assert_equal 1, (js["elements"].count { |a| a["id"] == relation4.id && a["visible"].nil? }) + + # check error when a non-existent relation is included + get api_relations_path(:relations => "#{relation1.id},#{relation2.id},#{relation3.id},#{relation4.id},0") + assert_response :not_found end # ------------------------------------- # Test showing relations. # ------------------------------------- - def test_show - # check that a visible relation is returned properly - get api_relation_path(create(:relation)) - assert_response :success + def test_show_not_found + get api_relation_path(0) + assert_response :not_found + end - # check that an invisible relation is not returned + def test_show_deleted get api_relation_path(create(:relation, :deleted)) assert_response :gone + end - # check chat a non-existent relation is not returned - get api_relation_path(0) + def test_show + relation = create(:relation, :timestamp => "2021-02-03T00:00:00Z") + node = create(:node, :timestamp => "2021-04-05T00:00:00Z") + create(:relation_member, :relation => relation, :member => node) + + get api_relation_path(relation) + + assert_response :success + assert_not_nil @response.header["Last-Modified"] + assert_equal "2021-02-03T00:00:00Z", Time.parse(@response.header["Last-Modified"]).utc.xmlschema + assert_dom "node", :count => 0 + assert_dom "relation", :count => 1 do + assert_dom "> @id", :text => relation.id.to_s + end + end + + def test_full_not_found + get relation_full_path(999999) assert_response :not_found end + def test_full_deleted + get relation_full_path(create(:relation, :deleted)) + assert_response :gone + end + + def test_full_empty + relation = create(:relation) + + get relation_full_path(relation) + + assert_response :success + assert_dom "relation", :count => 1 do + assert_dom "> @id", :text => relation.id.to_s + end + end + + def test_full_with_node_member + relation = create(:relation) + node = create(:node) + create(:relation_member, :relation => relation, :member => node) + + get relation_full_path(relation) + + assert_response :success + assert_dom "node", :count => 1 do + assert_dom "> @id", :text => node.id.to_s + end + assert_dom "relation", :count => 1 do + assert_dom "> @id", :text => relation.id.to_s + end + end + + def test_full_with_way_member + relation = create(:relation) + way = create(:way_with_nodes) + create(:relation_member, :relation => relation, :member => way) + + get relation_full_path(relation) + + assert_response :success + assert_dom "node", :count => 1 do + assert_dom "> @id", :text => way.nodes[0].id.to_s + end + assert_dom "way", :count => 1 do + assert_dom "> @id", :text => way.id.to_s + end + assert_dom "relation", :count => 1 do + assert_dom "> @id", :text => relation.id.to_s + end + end + ## # check that all relations containing a particular node, and no extra # relations, are returned from the relations_for_node call. @@ -151,64 +269,6 @@ module Api [relation_with_relation, second_relation]) end - def test_full - # check the "full" mode - get relation_full_path(:id => 999999) - assert_response :not_found - - get relation_full_path(:id => create(:relation, :deleted).id) - assert_response :gone - - get relation_full_path(:id => create(:relation).id) - assert_response :success - # FIXME: check whether this contains the stuff we want! - end - - ## - # test fetching multiple relations - def test_index - relation1 = create(:relation) - relation2 = create(:relation, :deleted) - relation3 = create(:relation, :with_history, :version => 2) - relation4 = create(:relation, :with_history, :version => 2) - relation4.old_relations.find_by(:version => 1).redact!(create(:redaction)) - - # check error when no parameter provided - get relations_path - assert_response :bad_request - - # check error when no parameter value provided - get relations_path(:relations => "") - assert_response :bad_request - - # test a working call - get relations_path(:relations => "#{relation1.id},#{relation2.id},#{relation3.id},#{relation4.id}") - assert_response :success - assert_select "osm" do - assert_select "relation", :count => 4 - assert_select "relation[id='#{relation1.id}'][visible='true']", :count => 1 - assert_select "relation[id='#{relation2.id}'][visible='false']", :count => 1 - assert_select "relation[id='#{relation3.id}'][visible='true']", :count => 1 - assert_select "relation[id='#{relation4.id}'][visible='true']", :count => 1 - end - - # test a working call with json format - get relations_path(:relations => "#{relation1.id},#{relation2.id},#{relation3.id},#{relation4.id}", :format => "json") - - js = ActiveSupport::JSON.decode(@response.body) - assert_not_nil js - assert_equal 4, js["elements"].count - assert_equal 4, (js["elements"].count { |a| a["type"] == "relation" }) - assert_equal 1, (js["elements"].count { |a| a["id"] == relation1.id && a["visible"].nil? }) - assert_equal 1, (js["elements"].count { |a| a["id"] == relation2.id && a["visible"] == false }) - assert_equal 1, (js["elements"].count { |a| a["id"] == relation3.id && a["visible"].nil? }) - assert_equal 1, (js["elements"].count { |a| a["id"] == relation4.id && a["visible"].nil? }) - - # check error when a non-existent relation is included - get relations_path(:relations => "#{relation1.id},#{relation2.id},#{relation3.id},#{relation4.id},0") - assert_response :not_found - end - # ------------------------------------- # Test simple relation creation. # ------------------------------------- @@ -225,7 +285,7 @@ module Api # create an relation without members xml = "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header # hope for forbidden, due to user assert_response :forbidden, "relation upload should have failed with forbidden" @@ -236,7 +296,7 @@ module Api xml = "" \ "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header # hope for forbidden due to user assert_response :forbidden, "relation upload did not return forbidden status" @@ -246,7 +306,7 @@ module Api # need a role attribute to be included xml = "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header # hope for forbidden due to user assert_response :forbidden, "relation upload did not return forbidden status" @@ -257,7 +317,7 @@ module Api "" \ "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header # hope for forbidden, due to user assert_response :forbidden, "relation upload did not return success status" @@ -267,7 +327,7 @@ module Api # create an relation without members xml = "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header # hope for success assert_response :success, "relation upload did not return success status" @@ -295,7 +355,7 @@ module Api xml = "" \ "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header # hope for success assert_response :success, "relation upload did not return success status" @@ -323,7 +383,7 @@ module Api # need a role attribute to be included xml = "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header # hope for success assert_response :success, "relation upload did not return success status" @@ -352,7 +412,7 @@ module Api "" \ "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header # hope for success assert_response :success, "relation upload did not return success status" @@ -472,7 +532,7 @@ module Api xml = "" \ "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header # expect failure assert_response :precondition_failed, "relation upload with invalid node did not return 'precondition failed'" @@ -493,7 +553,7 @@ module Api xml = "" \ "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header # expect failure assert_response :bad_request assert_match(/Cannot parse valid relation from xml string/, @response.body) @@ -504,7 +564,7 @@ module Api # Test deleting relations. # ------------------------------------- - def test_delete + def test_destroy private_user = create(:user, :data_public => false) private_user_closed_changeset = create(:changeset, :closed, :user => private_user) user = create(:user) @@ -757,7 +817,7 @@ module Api OSM doc = XML::Parser.string(doc_str).parse - put relation_create_path, :params => doc.to_s, :headers => auth_header + post api_relations_path, :params => doc.to_s, :headers => auth_header assert_response :success, "can't create a relation: #{@response.body}" relation_id = @response.body.to_i @@ -818,13 +878,13 @@ module Api ## First try with the private user auth_header = bearer_authorization_header private_user - put relation_create_path, :params => doc.to_s, :headers => auth_header + post api_relations_path, :params => doc.to_s, :headers => auth_header assert_response :forbidden ## Now try with the public user auth_header = bearer_authorization_header user - put relation_create_path, :params => doc.to_s, :headers => auth_header + post api_relations_path, :params => doc.to_s, :headers => auth_header assert_response :success, "can't create a relation: #{@response.body}" relation_id = @response.body.to_i @@ -857,7 +917,7 @@ module Api doc = XML::Parser.string(doc_str).parse auth_header = bearer_authorization_header user - put relation_create_path, :params => doc.to_s, :headers => auth_header + post api_relations_path, :params => doc.to_s, :headers => auth_header assert_response :success, "can't create a relation: #{@response.body}" relation_id = @response.body.to_i @@ -929,7 +989,7 @@ module Api "" \ "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header assert_response :success, "relation create did not return success status" # get the id of the relation we created @@ -953,7 +1013,7 @@ module Api "" \ "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header assert_response :too_many_requests, "relation create did not hit rate limit" end @@ -989,7 +1049,7 @@ module Api "" \ "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header assert_response :success, "relation create did not return success status" # get the id of the relation we created @@ -1013,7 +1073,7 @@ module Api "" \ "" \ "" - put relation_create_path, :params => xml, :headers => auth_header + post api_relations_path, :params => xml, :headers => auth_header assert_response :too_many_requests, "relation create did not hit rate limit" end diff --git a/test/controllers/api/ways_controller_test.rb b/test/controllers/api/ways_controller_test.rb index 6aa4bdfa5..2ff5e6f29 100644 --- a/test/controllers/api/ways_controller_test.rb +++ b/test/controllers/api/ways_controller_test.rb @@ -6,16 +6,16 @@ module Api # test all routes which lead to this controller def test_routes assert_routing( - { :path => "/api/0.6/way/create", :method => :put }, - { :controller => "api/ways", :action => "create" } + { :path => "/api/0.6/ways", :method => :get }, + { :controller => "api/ways", :action => "index" } ) assert_routing( - { :path => "/api/0.6/way/1/full", :method => :get }, - { :controller => "api/ways", :action => "full", :id => "1" } + { :path => "/api/0.6/ways.json", :method => :get }, + { :controller => "api/ways", :action => "index", :format => "json" } ) assert_routing( - { :path => "/api/0.6/way/1/full.json", :method => :get }, - { :controller => "api/ways", :action => "full", :id => "1", :format => "json" } + { :path => "/api/0.6/ways", :method => :post }, + { :controller => "api/ways", :action => "create" } ) assert_routing( { :path => "/api/0.6/way/1", :method => :get }, @@ -26,67 +26,26 @@ module Api { :controller => "api/ways", :action => "show", :id => "1", :format => "json" } ) assert_routing( - { :path => "/api/0.6/way/1", :method => :put }, - { :controller => "api/ways", :action => "update", :id => "1" } + { :path => "/api/0.6/way/1/full", :method => :get }, + { :controller => "api/ways", :action => "full", :id => "1" } ) assert_routing( - { :path => "/api/0.6/way/1", :method => :delete }, - { :controller => "api/ways", :action => "delete", :id => "1" } + { :path => "/api/0.6/way/1/full.json", :method => :get }, + { :controller => "api/ways", :action => "full", :id => "1", :format => "json" } ) assert_routing( - { :path => "/api/0.6/ways", :method => :get }, - { :controller => "api/ways", :action => "index" } + { :path => "/api/0.6/way/1", :method => :put }, + { :controller => "api/ways", :action => "update", :id => "1" } ) assert_routing( - { :path => "/api/0.6/ways.json", :method => :get }, - { :controller => "api/ways", :action => "index", :format => "json" } + { :path => "/api/0.6/way/1", :method => :delete }, + { :controller => "api/ways", :action => "destroy", :id => "1" } ) - end - - # ------------------------------------- - # Test showing ways. - # ------------------------------------- - - def test_show - # check that a visible way is returned properly - get api_way_path(create(:way)) - assert_response :success - - # check that an invisible way is not returned - get api_way_path(create(:way, :deleted)) - assert_response :gone - - # check chat a non-existent way is not returned - get api_way_path(0) - assert_response :not_found - end - - ## - # check the "full" mode - def test_full - way = create(:way_with_nodes, :nodes_count => 3) - - get way_full_path(way) - - assert_response :success - - # Check the way is correctly returned - assert_select "osm way[id='#{way.id}'][version='1'][visible='true']", 1 - - # check that each node in the way appears once in the output as a - # reference and as the node element. - way.nodes.each do |n| - assert_select "osm way nd[ref='#{n.id}']", 1 - assert_select "osm node[id='#{n.id}'][version='1'][lat='#{format('%.7f', :lat => n.lat)}'][lon='#{format('%.7f', :lon => n.lon)}']", 1 - end - end - - def test_full_deleted - way = create(:way, :deleted) - - get way_full_path(way) - assert_response :gone + assert_recognizes( + { :controller => "api/ways", :action => "create" }, + { :path => "/api/0.6/way/create", :method => :put } + ) end ## @@ -98,15 +57,15 @@ module Api way4 = create(:way) # check error when no parameter provided - get ways_path + get api_ways_path assert_response :bad_request # check error when no parameter value provided - get ways_path(:ways => "") + get api_ways_path(:ways => "") assert_response :bad_request # test a working call - get ways_path(:ways => "#{way1.id},#{way2.id},#{way3.id},#{way4.id}") + get api_ways_path(:ways => "#{way1.id},#{way2.id},#{way3.id},#{way4.id}") assert_response :success assert_select "osm" do assert_select "way", :count => 4 @@ -117,7 +76,7 @@ module Api end # test a working call with json format - get ways_path(:ways => "#{way1.id},#{way2.id},#{way3.id},#{way4.id}", :format => "json") + get api_ways_path(:ways => "#{way1.id},#{way2.id},#{way3.id},#{way4.id}", :format => "json") js = ActiveSupport::JSON.decode(@response.body) assert_not_nil js @@ -129,10 +88,80 @@ module Api assert_equal 1, (js["elements"].count { |a| a["id"] == way4.id && a["visible"].nil? }) # check error when a non-existent way is included - get ways_path(:ways => "#{way1.id},#{way2.id},#{way3.id},#{way4.id},0") + get api_ways_path(:ways => "#{way1.id},#{way2.id},#{way3.id},#{way4.id},0") + assert_response :not_found + end + + # ------------------------------------- + # Test showing ways. + # ------------------------------------- + + def test_show_not_found + get api_way_path(0) assert_response :not_found end + def test_show_deleted + get api_way_path(create(:way, :deleted)) + assert_response :gone + end + + def test_show + way = create(:way, :timestamp => "2021-02-03T00:00:00Z") + node = create(:node, :timestamp => "2021-04-05T00:00:00Z") + create(:way_node, :way => way, :node => node) + + get api_way_path(way) + + assert_response :success + assert_not_nil @response.header["Last-Modified"] + assert_equal "2021-02-03T00:00:00Z", Time.parse(@response.header["Last-Modified"]).utc.xmlschema + end + + def test_show_json + way = create(:way_with_nodes, :nodes_count => 3) + + get api_way_path(way, :format => "json") + + assert_response :success + + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 1, js["elements"].count + js_ways = js["elements"].filter { |e| e["type"] == "way" } + assert_equal 1, js_ways.count + assert_equal way.id, js_ways[0]["id"] + assert_equal 1, js_ways[0]["version"] + end + + ## + # check the "full" mode + def test_full + way = create(:way_with_nodes, :nodes_count => 3) + + get way_full_path(way) + + assert_response :success + + # Check the way is correctly returned + assert_select "osm way[id='#{way.id}'][version='1'][visible='true']", 1 + + # check that each node in the way appears once in the output as a + # reference and as the node element. + way.nodes.each do |n| + assert_select "osm way nd[ref='#{n.id}']", 1 + assert_select "osm node[id='#{n.id}'][version='1'][lat='#{format('%.7f', :lat => n.lat)}'][lon='#{format('%.7f', :lon => n.lon)}']", 1 + end + end + + def test_full_deleted + way = create(:way, :deleted) + + get way_full_path(way) + + assert_response :gone + end + # ------------------------------------- # Test simple way creation. # ------------------------------------- @@ -155,7 +184,7 @@ module Api xml = "" \ "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header # hope for failure assert_response :forbidden, "way upload did not return forbidden status" @@ -170,7 +199,7 @@ module Api xml = "" \ "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header # hope for success assert_response :success, "way upload did not return success status" @@ -213,7 +242,7 @@ module Api # create a way with non-existing node xml = "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header # expect failure assert_response :forbidden, "way upload with invalid node using a private user did not return 'forbidden'" @@ -221,7 +250,7 @@ module Api # create a way with no nodes xml = "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header # expect failure assert_response :forbidden, "way upload with no node using a private userdid not return 'forbidden'" @@ -229,7 +258,7 @@ module Api # create a way inside a closed changeset xml = "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header # expect failure assert_response :forbidden, "way upload to closed changeset with a private user did not return 'forbidden'" @@ -241,7 +270,7 @@ module Api # create a way with non-existing node xml = "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header # expect failure assert_response :precondition_failed, "way upload with invalid node did not return 'precondition failed'" @@ -250,7 +279,7 @@ module Api # create a way with no nodes xml = "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header # expect failure assert_response :precondition_failed, "way upload with no node did not return 'precondition failed'" @@ -259,7 +288,7 @@ module Api # create a way inside a closed changeset xml = "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header # expect failure assert_response :conflict, "way upload to closed changeset did not return 'conflict'" @@ -269,7 +298,7 @@ module Api "" \ "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header # expect failure assert_response :bad_request, "way upload to with too long tag did not return 'bad_request'" @@ -279,7 +308,7 @@ module Api # Test deleting ways. # ------------------------------------- - def test_delete + def test_destroy private_user = create(:user, :data_public => false) private_open_changeset = create(:changeset, :user => private_user) private_closed_changeset = create(:changeset, :closed, :user => private_user) @@ -696,7 +725,7 @@ module Api way_str << "" # try and upload it - put way_create_path, :params => way_str, :headers => auth_header + post api_ways_path, :params => way_str, :headers => auth_header assert_response :forbidden, "adding new duplicate tags to a way with a non-public user should fail with 'forbidden'" @@ -711,7 +740,7 @@ module Api way_str << "" # try and upload it - put way_create_path, :params => way_str, :headers => auth_header + post api_ways_path, :params => way_str, :headers => auth_header assert_response :bad_request, "adding new duplicate tags to a way should fail with 'bad request'" assert_equal "Element way/ has duplicate tags with key addr:housenumber", @response.body @@ -775,7 +804,7 @@ module Api xml = "" \ "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header assert_response :success, "way create did not return success status" # get the id of the way we created @@ -797,7 +826,7 @@ module Api xml = "" \ "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header assert_response :too_many_requests, "way create did not hit rate limit" end @@ -832,7 +861,7 @@ module Api xml = "" \ "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header assert_response :success, "way create did not return success status" # get the id of the way we created @@ -854,7 +883,7 @@ module Api xml = "" \ "" \ "" - put way_create_path, :params => xml, :headers => auth_header + post api_ways_path, :params => xml, :headers => auth_header assert_response :too_many_requests, "way create did not hit rate limit" end diff --git a/test/system/issues_test.rb b/test/system/issues_test.rb index a961ea80f..e26ae89ac 100644 --- a/test/system/issues_test.rb +++ b/test/system/issues_test.rb @@ -114,15 +114,15 @@ class IssuesTest < ApplicationSystemTestCase assert_equal("test comment", issue.comments.first.body) end - def test_reassign_issue - issue = create(:issue) - assert_equal "administrator", issue.assigned_role + def test_reassign_issue_to_moderators + issue = create(:issue, :assigned_role => "administrator") sign_in_as(create(:administrator_user)) visit issue_path(issue) + assert_unchecked_field "Reassign Issue to Moderators" fill_in :issue_comment_body, :with => "reassigning to moderators" - check :reassign + check "Reassign Issue to Moderators" click_on "Add Comment" assert_content "and the issue was reassigned" @@ -132,6 +132,24 @@ class IssuesTest < ApplicationSystemTestCase assert_equal "moderator", issue.assigned_role end + def test_reassign_issue_to_administrators + issue = create(:issue, :assigned_role => "moderator") + sign_in_as(create(:moderator_user)) + + visit issue_path(issue) + + assert_unchecked_field "Reassign Issue to Administrators" + fill_in :issue_comment_body, :with => "reassigning to administrators" + check "Reassign Issue to Administrators" + click_on "Add Comment" + + assert_content "and the issue was reassigned" + assert_current_path issues_path(:status => "open") + + issue.reload + assert_equal "administrator", issue.assigned_role + end + def test_reassign_issue_as_super_user issue = create(:issue) sign_in_as(create(:super_user)) @@ -139,7 +157,7 @@ class IssuesTest < ApplicationSystemTestCase visit issue_path(issue) fill_in :issue_comment_body, :with => "reassigning to moderators" - check :reassign + check "Reassign Issue to Moderators" click_on "Add Comment" assert_content "and the issue was reassigned" diff --git a/yarn.lock b/yarn.lock index fc144d734..c15c97e29 100644 --- a/yarn.lock +++ b/yarn.lock @@ -225,6 +225,11 @@ eslint-formatter-compact@^8.40.0: resolved "https://registry.yarnpkg.com/eslint-formatter-compact/-/eslint-formatter-compact-8.40.0.tgz#d7455b2d75fd70e8c0e7a98a5e189f168e9dfe2d" integrity sha512-cwGUs113TgmTQXecx5kfRjB7m0y2wkDLSadPTE2pK6M/wO4N8PjmUaoWOFNCP9MHgsiZwgqd5bZFnDCnszC56Q== +eslint-plugin-erb@^2.1.0: + version "2.1.1" + resolved "https://registry.yarnpkg.com/eslint-plugin-erb/-/eslint-plugin-erb-2.1.1.tgz#8a0a6c2bcaf3a8573c381b595969145aff93cfc6" + integrity sha512-AhznaVwRpQqR8NADjN4SZnKNbaIdAbGxTjCg6cj3UhwGyQOUJ6kXwhYrl1LYrGDNx7Ouyd8xuEG7wepFZyPgFw== + eslint-scope@^8.2.0: version "8.2.0" resolved "https://registry.yarnpkg.com/eslint-scope/-/eslint-scope-8.2.0.tgz#377aa6f1cb5dc7592cfd0b7f892fd0cf352ce442"