From 55ee5a481d1f6c52edb1eb45c1e6e67c650fedd6 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Wed, 30 Apr 2025 05:50:23 +0300 Subject: [PATCH] Create api changeset close resource and controller --- app/abilities/api_ability.rb | 2 +- .../api/changesets/closes_controller.rb | 31 +++++++++++++++++++ app/controllers/api/changesets_controller.rb | 22 ++----------- config/routes.rb | 2 +- .../api/changesets_controller_test.rb | 18 +++++------ 5 files changed, 45 insertions(+), 30 deletions(-) create mode 100644 app/controllers/api/changesets/closes_controller.rb diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index b0bd2578f..14d332ed0 100644 --- a/app/abilities/api_ability.rb +++ b/app/abilities/api_ability.rb @@ -34,7 +34,7 @@ class ApiAbility can :read, :active_user_blocks_list if scopes.include?("read_prefs") if user.terms_agreed? - can [:create, :update, :upload, :close], Changeset if scopes.include?("write_map") + can [:create, :update, :upload], Changeset if scopes.include?("write_map") can [:create, :destroy], ChangesetSubscription if scopes.include?("write_map") can :create, ChangesetComment if scopes.include?("write_changeset_comments") can [:create, :update, :destroy], [Node, Way, Relation] if scopes.include?("write_map") diff --git a/app/controllers/api/changesets/closes_controller.rb b/app/controllers/api/changesets/closes_controller.rb new file mode 100644 index 000000000..e3a6f9319 --- /dev/null +++ b/app/controllers/api/changesets/closes_controller.rb @@ -0,0 +1,31 @@ +module Api + module Changesets + class ClosesController < ApiController + before_action :check_api_writable + before_action :authorize + + authorize_resource :class => Changeset + + before_action :require_public_data + + # Helper methods for checking consistency + include ConsistencyValidations + + ## + # marks a changeset as closed. this may be called multiple times + # on the same changeset, so is idempotent. + def update + changeset = Changeset.find(params[:changeset_id]) + check_changeset_consistency(changeset, current_user) + + # to close the changeset, we'll just set its closed_at time to + # now. this might not be enough if there are concurrency issues, + # but we'll have to wait and see. + changeset.set_closed_time_now + + changeset.save! + head :ok + end + end + end +end diff --git a/app/controllers/api/changesets_controller.rb b/app/controllers/api/changesets_controller.rb index 891e2175b..1a60fa99b 100644 --- a/app/controllers/api/changesets_controller.rb +++ b/app/controllers/api/changesets_controller.rb @@ -6,12 +6,12 @@ module Api before_action :check_api_writable, :only => [:create, :update, :upload] before_action :setup_user_auth, :only => [:show] - before_action :authorize, :only => [:create, :update, :upload, :close] + before_action :authorize, :only => [:create, :update, :upload] authorize_resource - before_action :require_public_data, :only => [:create, :update, :upload, :close] - before_action :set_request_formats, :except => [:create, :close, :upload] + before_action :require_public_data, :only => [:create, :update, :upload] + before_action :set_request_formats, :except => [:create, :upload] skip_around_action :api_call_timeout, :only => [:upload] @@ -87,22 +87,6 @@ module Api render :plain => cs.id.to_s end - ## - # marks a changeset as closed. this may be called multiple times - # on the same changeset, so is idempotent. - def close - changeset = Changeset.find(params[:id]) - check_changeset_consistency(changeset, current_user) - - # to close the changeset, we'll just set its closed_at time to - # now. this might not be enough if there are concurrency issues, - # but we'll have to wait and see. - changeset.set_closed_time_now - - changeset.save! - head :ok - end - ## # Upload a diff in a single transaction. # diff --git a/config/routes.rb b/config/routes.rb index 93aa2f9fd..3aef845c4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,12 +18,12 @@ OpenStreetMap::Application.routes.draw do get "permissions" => "permissions#show" post "changeset/:id/upload" => "changesets#upload", :as => :changeset_upload, :id => /\d+/ - put "changeset/:id/close" => "changesets#close", :as => :changeset_close, :id => /\d+/ end namespace :api, :path => "api/0.6" do resources :changesets, :only => [:index, :create] resources :changesets, :path => "changeset", :id => /\d+/, :only => [:show, :update] do + resource :close, :module => :changesets, :only => :update resource :download, :module => :changesets, :only => :show resource :subscription, :controller => :changeset_subscriptions, :only => [:create, :destroy] resources :changeset_comments, :path => "comment", :only => :create diff --git a/test/controllers/api/changesets_controller_test.rb b/test/controllers/api/changesets_controller_test.rb index c20b9bddc..719afded3 100644 --- a/test/controllers/api/changesets_controller_test.rb +++ b/test/controllers/api/changesets_controller_test.rb @@ -35,7 +35,7 @@ module Api ) assert_routing( { :path => "/api/0.6/changeset/1/close", :method => :put }, - { :controller => "api/changesets", :action => "close", :id => "1" } + { :controller => "api/changesets/closes", :action => "update", :changeset_id => "1" } ) assert_recognizes( @@ -660,19 +660,19 @@ module Api changeset = create(:changeset, :user => user) ## Try without authentication - put changeset_close_path(changeset) + put api_changeset_close_path(changeset) assert_response :unauthorized ## Try using the non-public user auth_header = bearer_authorization_header private_user - put changeset_close_path(private_changeset), :headers => auth_header + put api_changeset_close_path(private_changeset), :headers => auth_header assert_require_public_data ## The try with the public user auth_header = bearer_authorization_header user cs_id = changeset.id - put changeset_close_path(cs_id), :headers => auth_header + put api_changeset_close_path(cs_id), :headers => auth_header assert_response :success # test that it really is closed now @@ -689,7 +689,7 @@ module Api auth_header = bearer_authorization_header user - put changeset_close_path(changeset), :headers => auth_header + put api_changeset_close_path(changeset), :headers => auth_header assert_response :conflict assert_equal "The user doesn't own that changeset", @response.body end @@ -702,11 +702,11 @@ module Api auth_header = bearer_authorization_header user - get changeset_close_path(changeset), :headers => auth_header + get api_changeset_close_path(changeset), :headers => auth_header assert_response :not_found assert_template "rescues/routing_error" - post changeset_close_path(changeset), :headers => auth_header + post api_changeset_close_path(changeset), :headers => auth_header assert_response :not_found assert_template "rescues/routing_error" end @@ -718,7 +718,7 @@ module Api # First try to do it with no auth cs_ids.each do |id| - put changeset_close_path(id) + put api_changeset_close_path(id) assert_response :unauthorized, "Shouldn't be able close the non-existant changeset #{id}, when not authorized" rescue ActionController::UrlGenerationError => e assert_match(/No route matches/, e.to_s) @@ -727,7 +727,7 @@ module Api # Now try with auth auth_header = bearer_authorization_header cs_ids.each do |id| - put changeset_close_path(id), :headers => auth_header + put api_changeset_close_path(id), :headers => auth_header assert_response :not_found, "The changeset #{id} doesn't exist, so can't be closed" rescue ActionController::UrlGenerationError => e assert_match(/No route matches/, e.to_s) -- 2.39.5