Move the api methods from changeset_comments_controller into the api namespaced controller

This commit is contained in:
Andy Allan 2019-02-24 12:38:09 +01:00
parent 1778fa3d9c
commit 947a41edee
5 changed files with 345 additions and 328 deletions

View file

@ -0,0 +1,85 @@
module Api
class ChangesetCommentsController < ApplicationController
skip_before_action :verify_authenticity_token
before_action :authorize
before_action :api_deny_access_handler
authorize_resource
before_action :require_public_data, :only => [:create]
before_action :check_api_writable
before_action :check_api_readable, :except => [:create]
around_action :api_call_handle_error
around_action :api_call_timeout
##
# Add a comment to a changeset
def create
# Check the arguments are sane
raise OSM::APIBadUserInput, "No id was given" unless params[:id]
raise OSM::APIBadUserInput, "No text was given" if params[:text].blank?
# Extract the arguments
id = params[:id].to_i
body = params[:text]
# Find the changeset and check it is valid
changeset = Changeset.find(id)
raise OSM::APIChangesetNotYetClosedError, changeset if changeset.is_open?
# Add a comment to the changeset
comment = changeset.comments.create(:changeset => changeset,
:body => body,
:author => current_user)
# Notify current subscribers of the new comment
changeset.subscribers.visible.each do |user|
Notifier.changeset_comment_notification(comment, user).deliver_later if current_user != user
end
# Add the commenter to the subscribers if necessary
changeset.subscribers << current_user unless changeset.subscribers.exists?(current_user.id)
# Return a copy of the updated changeset
render :xml => changeset.to_xml.to_s
end
##
# Sets visible flag on comment to false
def destroy
# Check the arguments are sane
raise OSM::APIBadUserInput, "No id was given" unless params[:id]
# Extract the arguments
id = params[:id].to_i
# Find the changeset
comment = ChangesetComment.find(id)
# Hide the comment
comment.update(:visible => false)
# Return a copy of the updated changeset
render :xml => comment.changeset.to_xml.to_s
end
##
# Sets visible flag on comment to true
def restore
# Check the arguments are sane
raise OSM::APIBadUserInput, "No id was given" unless params[:id]
# Extract the arguments
id = params[:id].to_i
# Find the changeset
comment = ChangesetComment.find(id)
# Unhide the comment
comment.update(:visible => true)
# Return a copy of the updated changeset
render :xml => comment.changeset.to_xml.to_s
end
end
end

View file

@ -1,89 +1,11 @@
class ChangesetCommentsController < ApplicationController
skip_before_action :verify_authenticity_token, :except => [:index]
before_action :authorize_web, :only => [:index]
before_action :set_locale, :only => [:index]
before_action :authorize, :only => [:create, :destroy, :restore]
before_action :api_deny_access_handler, :only => [:create, :destroy, :restore]
before_action :authorize_web
before_action :set_locale
authorize_resource
before_action :require_public_data, :only => [:create]
before_action :check_api_writable, :only => [:create, :destroy, :restore]
before_action :check_api_readable, :except => [:create, :index]
before_action(:only => [:index]) { |c| c.check_database_readable(true) }
around_action :api_call_handle_error, :except => [:index]
around_action :api_call_timeout, :except => [:index]
around_action :web_timeout, :only => [:index]
##
# Add a comment to a changeset
def create
# Check the arguments are sane
raise OSM::APIBadUserInput, "No id was given" unless params[:id]
raise OSM::APIBadUserInput, "No text was given" if params[:text].blank?
# Extract the arguments
id = params[:id].to_i
body = params[:text]
# Find the changeset and check it is valid
changeset = Changeset.find(id)
raise OSM::APIChangesetNotYetClosedError, changeset if changeset.is_open?
# Add a comment to the changeset
comment = changeset.comments.create(:changeset => changeset,
:body => body,
:author => current_user)
# Notify current subscribers of the new comment
changeset.subscribers.visible.each do |user|
Notifier.changeset_comment_notification(comment, user).deliver_later if current_user != user
end
# Add the commenter to the subscribers if necessary
changeset.subscribers << current_user unless changeset.subscribers.exists?(current_user.id)
# Return a copy of the updated changeset
render :xml => changeset.to_xml.to_s
end
##
# Sets visible flag on comment to false
def destroy
# Check the arguments are sane
raise OSM::APIBadUserInput, "No id was given" unless params[:id]
# Extract the arguments
id = params[:id].to_i
# Find the changeset
comment = ChangesetComment.find(id)
# Hide the comment
comment.update(:visible => false)
# Return a copy of the updated changeset
render :xml => comment.changeset.to_xml.to_s
end
##
# Sets visible flag on comment to true
def restore
# Check the arguments are sane
raise OSM::APIBadUserInput, "No id was given" unless params[:id]
# Extract the arguments
id = params[:id].to_i
# Find the changeset
comment = ChangesetComment.find(id)
# Unhide the comment
comment.update(:visible => true)
# Return a copy of the updated changeset
render :xml => comment.changeset.to_xml.to_s
end
around_action :web_timeout
##
# Get a feed of recent changeset comments

View file

@ -18,9 +18,9 @@ OpenStreetMap::Application.routes.draw do
put "changeset/:id" => "api/changesets#update", :id => /\d+/
put "changeset/:id/close" => "api/changesets#close", :id => /\d+/
get "changesets" => "api/changesets#query"
post "changeset/:id/comment" => "changeset_comments#create", :as => :changeset_comment, :id => /\d+/
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+/
post "changeset/:id/comment" => "api/changeset_comments#create", :as => :changeset_comment, :id => /\d+/
post "changeset/comment/:id/hide" => "api/changeset_comments#destroy", :as => :changeset_comment_hide, :id => /\d+/
post "changeset/comment/:id/unhide" => "api/changeset_comments#restore", :as => :changeset_comment_unhide, :id => /\d+/
put "node/create" => "nodes#create"
get "node/:id/ways" => "ways#ways_for_node", :id => /\d+/

View file

@ -0,0 +1,254 @@
require "test_helper"
module Api
class ChangesetCommentsControllerTest < ActionController::TestCase
##
# test all routes which lead to this controller
def test_routes
assert_routing(
{ :path => "/api/0.6/changeset/1/comment", :method => :post },
{ :controller => "api/changeset_comments", :action => "create", :id => "1" }
)
assert_routing(
{ :path => "/api/0.6/changeset/comment/1/hide", :method => :post },
{ :controller => "api/changeset_comments", :action => "destroy", :id => "1" }
)
assert_routing(
{ :path => "/api/0.6/changeset/comment/1/unhide", :method => :post },
{ :controller => "api/changeset_comments", :action => "restore", :id => "1" }
)
end
##
# create comment success
def test_create_comment_success
user = create(:user)
user2 = create(:user)
private_user = create(:user, :data_public => false)
suspended_user = create(:user, :suspended)
deleted_user = create(:user, :deleted)
private_user_closed_changeset = create(:changeset, :closed, :user => private_user)
basic_authorization user.email, "test"
assert_difference "ChangesetComment.count", 1 do
assert_no_difference "ActionMailer::Base.deliveries.size" do
perform_enqueued_jobs do
post :create, :params => { :id => private_user_closed_changeset.id, :text => "This is a comment" }
end
end
end
assert_response :success
changeset = create(:changeset, :closed, :user => private_user)
changeset.subscribers.push(private_user)
changeset.subscribers.push(user)
changeset.subscribers.push(suspended_user)
changeset.subscribers.push(deleted_user)
assert_difference "ChangesetComment.count", 1 do
assert_difference "ActionMailer::Base.deliveries.size", 1 do
perform_enqueued_jobs do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
end
end
assert_response :success
email = ActionMailer::Base.deliveries.first
assert_equal 1, email.to.length
assert_equal "[OpenStreetMap] #{user.display_name} has commented on one of your changesets", email.subject
assert_equal private_user.email, email.to.first
ActionMailer::Base.deliveries.clear
basic_authorization user2.email, "test"
assert_difference "ChangesetComment.count", 1 do
assert_difference "ActionMailer::Base.deliveries.size", 2 do
perform_enqueued_jobs do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
end
end
assert_response :success
email = ActionMailer::Base.deliveries.find { |e| e.to.first == private_user.email }
assert_not_nil email
assert_equal 1, email.to.length
assert_equal "[OpenStreetMap] #{user2.display_name} has commented on one of your changesets", email.subject
email = ActionMailer::Base.deliveries.find { |e| e.to.first == user.email }
assert_not_nil email
assert_equal 1, email.to.length
assert_equal "[OpenStreetMap] #{user2.display_name} has commented on a changeset you are interested in", email.subject
ActionMailer::Base.deliveries.clear
end
##
# create comment fail
def test_create_comment_fail
# unauthorized
post :create, :params => { :id => create(:changeset, :closed).id, :text => "This is a comment" }
assert_response :unauthorized
basic_authorization create(:user).email, "test"
# bad changeset id
assert_no_difference "ChangesetComment.count" do
post :create, :params => { :id => 999111, :text => "This is a comment" }
end
assert_response :not_found
# not closed changeset
assert_no_difference "ChangesetComment.count" do
post :create, :params => { :id => create(:changeset).id, :text => "This is a comment" }
end
assert_response :conflict
# no text
assert_no_difference "ChangesetComment.count" do
post :create, :params => { :id => create(:changeset, :closed).id }
end
assert_response :bad_request
# empty text
assert_no_difference "ChangesetComment.count" do
post :create, :params => { :id => create(:changeset, :closed).id, :text => "" }
end
assert_response :bad_request
end
##
# test hide comment fail
def test_destroy_comment_fail
# unauthorized
comment = create(:changeset_comment)
assert_equal true, comment.visible
post :destroy, :params => { :id => comment.id }
assert_response :unauthorized
assert_equal true, comment.reload.visible
basic_authorization create(:user).email, "test"
# not a moderator
post :destroy, :params => { :id => comment.id }
assert_response :forbidden
assert_equal true, comment.reload.visible
basic_authorization create(:moderator_user).email, "test"
# bad comment id
post :destroy, :params => { :id => 999111 }
assert_response :not_found
assert_equal true, comment.reload.visible
end
##
# test hide comment succes
def test_hide_comment_success
comment = create(:changeset_comment)
assert_equal true, comment.visible
basic_authorization create(:moderator_user).email, "test"
post :destroy, :params => { :id => comment.id }
assert_response :success
assert_equal false, comment.reload.visible
end
##
# test unhide comment fail
def test_restore_comment_fail
# unauthorized
comment = create(:changeset_comment, :visible => false)
assert_equal false, comment.visible
post :restore, :params => { :id => comment.id }
assert_response :unauthorized
assert_equal false, comment.reload.visible
basic_authorization create(:user).email, "test"
# not a moderator
post :restore, :params => { :id => comment.id }
assert_response :forbidden
assert_equal false, comment.reload.visible
basic_authorization create(:moderator_user).email, "test"
# bad comment id
post :restore, :params => { :id => 999111 }
assert_response :not_found
assert_equal false, comment.reload.visible
end
##
# test unhide comment succes
def test_unhide_comment_success
comment = create(:changeset_comment, :visible => false)
assert_equal false, comment.visible
basic_authorization create(:moderator_user).email, "test"
post :restore, :params => { :id => comment.id }
assert_response :success
assert_equal true, comment.reload.visible
end
# This test ensures that token capabilities behave correctly for a method that
# requires the terms to have been agreed.
# (This would be better as an integration or system testcase, since the changeset_comment
# create method is simply a stand-in for any method that requires terms agreement.
# But writing oauth tests is hard, and so it's easier to put in a controller test.)
def test_api_write_and_terms_agreed_via_token
user = create(:user, :terms_agreed => nil)
token = create(:access_token, :user => user, :allow_write_api => true)
changeset = create(:changeset, :closed)
# Hack together an oauth request - an alternative would be to sign the request properly
@request.env["oauth.version"] = 1
@request.env["oauth.strategies"] = [:token]
@request.env["oauth.token"] = token
assert_difference "ChangesetComment.count", 0 do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
assert_response :forbidden
# Try again, after agreement with the terms
user.terms_agreed = Time.now
user.save!
assert_difference "ChangesetComment.count", 1 do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
assert_response :success
end
# This test does the same as above, but with basic auth, to similarly test that the
# abilities take into account terms agreement too.
def test_api_write_and_terms_agreed_via_basic_auth
user = create(:user, :terms_agreed => nil)
changeset = create(:changeset, :closed)
basic_authorization user.email, "test"
assert_difference "ChangesetComment.count", 0 do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
assert_response :forbidden
# Try again, after agreement with the terms
user.terms_agreed = Time.now
user.save!
assert_difference "ChangesetComment.count", 1 do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
assert_response :success
end
end
end

View file

@ -4,18 +4,6 @@ class ChangesetCommentsControllerTest < ActionController::TestCase
##
# test all routes which lead to this controller
def test_routes
assert_routing(
{ :path => "/api/0.6/changeset/1/comment", :method => :post },
{ :controller => "changeset_comments", :action => "create", :id => "1" }
)
assert_routing(
{ :path => "/api/0.6/changeset/comment/1/hide", :method => :post },
{ :controller => "changeset_comments", :action => "destroy", :id => "1" }
)
assert_routing(
{ :path => "/api/0.6/changeset/comment/1/unhide", :method => :post },
{ :controller => "changeset_comments", :action => "restore", :id => "1" }
)
assert_routing(
{ :path => "/changeset/1/comments/feed", :method => :get },
{ :controller => "changeset_comments", :action => "index", :id => "1", :format => "rss" }
@ -26,185 +14,6 @@ class ChangesetCommentsControllerTest < ActionController::TestCase
)
end
##
# create comment success
def test_create_comment_success
user = create(:user)
user2 = create(:user)
private_user = create(:user, :data_public => false)
suspended_user = create(:user, :suspended)
deleted_user = create(:user, :deleted)
private_user_closed_changeset = create(:changeset, :closed, :user => private_user)
basic_authorization user.email, "test"
assert_difference "ChangesetComment.count", 1 do
assert_no_difference "ActionMailer::Base.deliveries.size" do
perform_enqueued_jobs do
post :create, :params => { :id => private_user_closed_changeset.id, :text => "This is a comment" }
end
end
end
assert_response :success
changeset = create(:changeset, :closed, :user => private_user)
changeset.subscribers.push(private_user)
changeset.subscribers.push(user)
changeset.subscribers.push(suspended_user)
changeset.subscribers.push(deleted_user)
assert_difference "ChangesetComment.count", 1 do
assert_difference "ActionMailer::Base.deliveries.size", 1 do
perform_enqueued_jobs do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
end
end
assert_response :success
email = ActionMailer::Base.deliveries.first
assert_equal 1, email.to.length
assert_equal "[OpenStreetMap] #{user.display_name} has commented on one of your changesets", email.subject
assert_equal private_user.email, email.to.first
ActionMailer::Base.deliveries.clear
basic_authorization user2.email, "test"
assert_difference "ChangesetComment.count", 1 do
assert_difference "ActionMailer::Base.deliveries.size", 2 do
perform_enqueued_jobs do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
end
end
assert_response :success
email = ActionMailer::Base.deliveries.find { |e| e.to.first == private_user.email }
assert_not_nil email
assert_equal 1, email.to.length
assert_equal "[OpenStreetMap] #{user2.display_name} has commented on one of your changesets", email.subject
email = ActionMailer::Base.deliveries.find { |e| e.to.first == user.email }
assert_not_nil email
assert_equal 1, email.to.length
assert_equal "[OpenStreetMap] #{user2.display_name} has commented on a changeset you are interested in", email.subject
ActionMailer::Base.deliveries.clear
end
##
# create comment fail
def test_create_comment_fail
# unauthorized
post :create, :params => { :id => create(:changeset, :closed).id, :text => "This is a comment" }
assert_response :unauthorized
basic_authorization create(:user).email, "test"
# bad changeset id
assert_no_difference "ChangesetComment.count" do
post :create, :params => { :id => 999111, :text => "This is a comment" }
end
assert_response :not_found
# not closed changeset
assert_no_difference "ChangesetComment.count" do
post :create, :params => { :id => create(:changeset).id, :text => "This is a comment" }
end
assert_response :conflict
# no text
assert_no_difference "ChangesetComment.count" do
post :create, :params => { :id => create(:changeset, :closed).id }
end
assert_response :bad_request
# empty text
assert_no_difference "ChangesetComment.count" do
post :create, :params => { :id => create(:changeset, :closed).id, :text => "" }
end
assert_response :bad_request
end
##
# test hide comment fail
def test_destroy_comment_fail
# unauthorized
comment = create(:changeset_comment)
assert_equal true, comment.visible
post :destroy, :params => { :id => comment.id }
assert_response :unauthorized
assert_equal true, comment.reload.visible
basic_authorization create(:user).email, "test"
# not a moderator
post :destroy, :params => { :id => comment.id }
assert_response :forbidden
assert_equal true, comment.reload.visible
basic_authorization create(:moderator_user).email, "test"
# bad comment id
post :destroy, :params => { :id => 999111 }
assert_response :not_found
assert_equal true, comment.reload.visible
end
##
# test hide comment succes
def test_hide_comment_success
comment = create(:changeset_comment)
assert_equal true, comment.visible
basic_authorization create(:moderator_user).email, "test"
post :destroy, :params => { :id => comment.id }
assert_response :success
assert_equal false, comment.reload.visible
end
##
# test unhide comment fail
def test_restore_comment_fail
# unauthorized
comment = create(:changeset_comment, :visible => false)
assert_equal false, comment.visible
post :restore, :params => { :id => comment.id }
assert_response :unauthorized
assert_equal false, comment.reload.visible
basic_authorization create(:user).email, "test"
# not a moderator
post :restore, :params => { :id => comment.id }
assert_response :forbidden
assert_equal false, comment.reload.visible
basic_authorization create(:moderator_user).email, "test"
# bad comment id
post :restore, :params => { :id => 999111 }
assert_response :not_found
assert_equal false, comment.reload.visible
end
##
# test unhide comment succes
def test_unhide_comment_success
comment = create(:changeset_comment, :visible => false)
assert_equal false, comment.visible
basic_authorization create(:moderator_user).email, "test"
post :restore, :params => { :id => comment.id }
assert_response :success
assert_equal true, comment.reload.visible
end
##
# test comments feed
def test_feed
@ -248,57 +57,4 @@ class ChangesetCommentsControllerTest < ActionController::TestCase
get :index, :params => { :format => "rss", :limit => 100001 }
assert_response :bad_request
end
# This test ensures that token capabilities behave correctly for a method that
# requires the terms to have been agreed.
# (This would be better as an integration or system testcase, since the changeset_comment
# create method is simply a stand-in for any method that requires terms agreement.
# But writing oauth tests is hard, and so it's easier to put in a controller test.)
def test_api_write_and_terms_agreed_via_token
user = create(:user, :terms_agreed => nil)
token = create(:access_token, :user => user, :allow_write_api => true)
changeset = create(:changeset, :closed)
# Hack together an oauth request - an alternative would be to sign the request properly
@request.env["oauth.version"] = 1
@request.env["oauth.strategies"] = [:token]
@request.env["oauth.token"] = token
assert_difference "ChangesetComment.count", 0 do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
assert_response :forbidden
# Try again, after agreement with the terms
user.terms_agreed = Time.now
user.save!
assert_difference "ChangesetComment.count", 1 do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
assert_response :success
end
# This test does the same as above, but with basic auth, to similarly test that the
# abilities take into account terms agreement too.
def test_api_write_and_terms_agreed_via_basic_auth
user = create(:user, :terms_agreed => nil)
changeset = create(:changeset, :closed)
basic_authorization user.email, "test"
assert_difference "ChangesetComment.count", 0 do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
assert_response :forbidden
# Try again, after agreement with the terms
user.terms_agreed = Time.now
user.save!
assert_difference "ChangesetComment.count", 1 do
post :create, :params => { :id => changeset.id, :text => "This is a comment" }
end
assert_response :success
end
end