From: Andy Allan <git@gravitystorm.co.uk>
Date: Wed, 31 Oct 2018 18:56:26 +0000 (+0100)
Subject: Split changeset comment handling into a changeset_comments controller
X-Git-Tag: live~2800^2~5
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/19c2b92fb736e2b424981ce36c8ffd8f75c44d99?hp=a39c645602f9d27b7927500c93890f4d7d79620e

Split changeset comment handling into a changeset_comments controller
---

diff --git a/app/controllers/changeset_comments_controller.rb b/app/controllers/changeset_comments_controller.rb
new file mode 100644
index 000000000..31e152560
--- /dev/null
+++ b/app/controllers/changeset_comments_controller.rb
@@ -0,0 +1,125 @@
+class ChangesetCommentsController < ApplicationController
+  before_action :authorize_web, :only => [:comments_feed]
+  before_action :set_locale, :only => [:comments_feed]
+  before_action :authorize, :only => [:comment, :hide_comment, :unhide_comment]
+  before_action :require_moderator, :only => [:hide_comment, :unhide_comment]
+  before_action :require_allow_write_api, :only => [:comment, :hide_comment, :unhide_comment]
+  before_action :require_public_data, :only => [:comment]
+  before_action :check_api_writable, :only => [:comment, :hide_comment, :unhide_comment]
+  before_action :check_api_readable, :except => [:comment, :comments_feed]
+  before_action(:only => [:comments_feed]) { |c| c.check_database_readable(true) }
+  around_action :api_call_handle_error, :except => [:comments_feed]
+  around_action :api_call_timeout, :except => [:comments_feed]
+  around_action :web_timeout, :only => [:comments_feed]
+
+  ##
+  # Add a comment to a changeset
+  def comment
+    # 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 hide_comment
+    # 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 unhide_comment
+    # 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
+
+  ##
+  # Get a feed of recent changeset comments
+  def comments_feed
+    if params[:id]
+      # Extract the arguments
+      id = params[:id].to_i
+
+      # Find the changeset
+      changeset = Changeset.find(id)
+
+      # Return comments for this changeset only
+      @comments = changeset.comments.includes(:author, :changeset).limit(comments_limit)
+    else
+      # Return comments
+      @comments = ChangesetComment.includes(:author, :changeset).where(:visible => true).order("created_at DESC").limit(comments_limit).preload(:changeset)
+    end
+
+    # Render the result
+    respond_to do |format|
+      format.rss
+    end
+  rescue OSM::APIBadUserInput
+    head :bad_request
+  end
+
+  private
+
+  ##
+  # Get the maximum number of comments to return
+  def comments_limit
+    if params[:limit]
+      if params[:limit].to_i.positive? && params[:limit].to_i <= 10000
+        params[:limit].to_i
+      else
+        raise OSM::APIBadUserInput, "Comments limit must be between 1 and 10000"
+      end
+    else
+      100
+    end
+  end
+end
diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb
index 4ce205fd1..7c9944f63 100644
--- a/app/controllers/changeset_controller.rb
+++ b/app/controllers/changeset_controller.rb
@@ -5,18 +5,17 @@ class ChangesetController < ApplicationController
   require "xml/libxml"
 
   skip_before_action :verify_authenticity_token, :except => [:index]
-  before_action :authorize_web, :only => [:index, :feed, :comments_feed]
-  before_action :set_locale, :only => [:index, :feed, :comments_feed]
-  before_action :authorize, :only => [:create, :update, :upload, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
-  before_action :require_moderator, :only => [:hide_comment, :unhide_comment]
-  before_action :require_allow_write_api, :only => [:create, :update, :upload, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
-  before_action :require_public_data, :only => [:create, :update, :upload, :close, :comment, :subscribe, :unsubscribe]
-  before_action :check_api_writable, :only => [:create, :update, :upload, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
-  before_action :check_api_readable, :except => [:create, :update, :upload, :download, :query, :index, :feed, :comment, :subscribe, :unsubscribe, :comments_feed]
-  before_action(:only => [:index, :feed, :comments_feed]) { |c| c.check_database_readable(true) }
-  around_action :api_call_handle_error, :except => [:index, :feed, :comments_feed]
-  around_action :api_call_timeout, :except => [:index, :feed, :comments_feed, :upload]
-  around_action :web_timeout, :only => [:index, :feed, :comments_feed]
+  before_action :authorize_web, :only => [:index, :feed]
+  before_action :set_locale, :only => [:index, :feed]
+  before_action :authorize, :only => [:create, :update, :upload, :close, :subscribe, :unsubscribe]
+  before_action :require_allow_write_api, :only => [:create, :update, :upload, :close, :subscribe, :unsubscribe]
+  before_action :require_public_data, :only => [:create, :update, :upload, :close, :subscribe, :unsubscribe]
+  before_action :check_api_writable, :only => [:create, :update, :upload, :subscribe, :unsubscribe]
+  before_action :check_api_readable, :except => [:create, :update, :upload, :download, :query, :index, :feed, :subscribe, :unsubscribe]
+  before_action(:only => [:index, :feed]) { |c| c.check_database_readable(true) }
+  around_action :api_call_handle_error, :except => [:index, :feed]
+  around_action :api_call_timeout, :except => [:index, :feed, :upload]
+  around_action :web_timeout, :only => [:index, :feed]
 
   # Helper methods for checking consistency
   include ConsistencyValidations
@@ -310,38 +309,6 @@ class ChangesetController < ApplicationController
     index
   end
 
-  ##
-  # Add a comment to a changeset
-  def comment
-    # 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
-
   ##
   # Adds a subscriber to the changeset
   def subscribe
@@ -382,69 +349,6 @@ class ChangesetController < ApplicationController
     render :xml => changeset.to_xml.to_s
   end
 
-  ##
-  # Sets visible flag on comment to false
-  def hide_comment
-    # 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 unhide_comment
-    # 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
-
-  ##
-  # Get a feed of recent changeset comments
-  def comments_feed
-    if params[:id]
-      # Extract the arguments
-      id = params[:id].to_i
-
-      # Find the changeset
-      changeset = Changeset.find(id)
-
-      # Return comments for this changeset only
-      @comments = changeset.comments.includes(:author, :changeset).limit(comments_limit)
-    else
-      # Return comments
-      @comments = ChangesetComment.includes(:author, :changeset).where(:visible => true).order("created_at DESC").limit(comments_limit).preload(:changeset)
-    end
-
-    # Render the result
-    respond_to do |format|
-      format.rss
-    end
-  rescue OSM::APIBadUserInput
-    head :bad_request
-  end
-
   private
 
   #------------------------------------------------------------
@@ -577,18 +481,4 @@ class ChangesetController < ApplicationController
   def conditions_nonempty(changesets)
     changesets.where("num_changes > 0")
   end
-
-  ##
-  # Get the maximum number of comments to return
-  def comments_limit
-    if params[:limit]
-      if params[:limit].to_i.positive? && params[:limit].to_i <= 10000
-        params[:limit].to_i
-      else
-        raise OSM::APIBadUserInput, "Comments limit must be between 1 and 10000"
-      end
-    else
-      100
-    end
-  end
 end
diff --git a/app/views/changeset/_comment.html.erb b/app/views/changeset_comments/_comment.html.erb
similarity index 97%
rename from app/views/changeset/_comment.html.erb
rename to app/views/changeset_comments/_comment.html.erb
index dfd91167b..34b15ff88 100644
--- a/app/views/changeset/_comment.html.erb
+++ b/app/views/changeset_comments/_comment.html.erb
@@ -1,4 +1,4 @@
-<h2><%= t "changeset.rss.comment", :author => comment.author.display_name, 
+<h2><%= t "changeset.rss.comment", :author => comment.author.display_name,
   :changeset_id => comment.changeset.id.to_s %></h2>
 <div class="changeset-comment" style="margin-top: 5px">
   <div class="changeset-comment-description" style="font-size: smaller; color: #999999"><%= t "changeset.rss.commented_at_by_html", :when => friendly_date(comment.created_at), :user => comment.author.display_name %></div>
diff --git a/app/views/changeset/_comments.rss.builder b/app/views/changeset_comments/_comments.rss.builder
similarity index 100%
rename from app/views/changeset/_comments.rss.builder
rename to app/views/changeset_comments/_comments.rss.builder
diff --git a/app/views/changeset/comments_feed.rss.builder b/app/views/changeset_comments/comments_feed.rss.builder
similarity index 100%
rename from app/views/changeset/comments_feed.rss.builder
rename to app/views/changeset_comments/comments_feed.rss.builder
diff --git a/app/views/changeset_comments/timeout.atom.builder b/app/views/changeset_comments/timeout.atom.builder
new file mode 100644
index 000000000..b5eeeed4a
--- /dev/null
+++ b/app/views/changeset_comments/timeout.atom.builder
@@ -0,0 +1,12 @@
+atom_feed(:language => I18n.locale, :schema_date => 2009,
+          :id => url_for(params.merge(:only_path => false)),
+          :root_url => url_for(params.merge(:only_path => false, :format => nil)),
+          "xmlns:georss" => "http://www.georss.org/georss") do |feed|
+  feed.title @title
+
+  feed.subtitle :type => "xhtml" do |xhtml|
+    xhtml.p do |p|
+      p << t(".sorry")
+    end
+  end
+end
diff --git a/app/views/changeset_comments/timeout.html.erb b/app/views/changeset_comments/timeout.html.erb
new file mode 100644
index 000000000..84432bc8d
--- /dev/null
+++ b/app/views/changeset_comments/timeout.html.erb
@@ -0,0 +1 @@
+<p><%= t '.sorry' %></p>
diff --git a/config/routes.rb b/config/routes.rb
index 7d9c8dbec..38ce38aa2 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -16,9 +16,9 @@ OpenStreetMap::Application.routes.draw do
     put "changeset/:id" => "changeset#update", :id => /\d+/
     put "changeset/:id/close" => "changeset#close", :id => /\d+/
     get "changesets" => "changeset#query"
-    post "changeset/:id/comment" => "changeset#comment", :as => :changeset_comment, :id => /\d+/
-    post "changeset/comment/:id/hide" => "changeset#hide_comment", :as => :changeset_comment_hide, :id => /\d+/
-    post "changeset/comment/:id/unhide" => "changeset#unhide_comment", :as => :changeset_comment_unhide, :id => /\d+/
+    post "changeset/:id/comment" => "changeset_comments#comment", :as => :changeset_comment, :id => /\d+/
+    post "changeset/comment/:id/hide" => "changeset_comments#hide_comment", :as => :changeset_comment_hide, :id => /\d+/
+    post "changeset/comment/:id/unhide" => "changeset_comments#unhide_comment", :as => :changeset_comment_unhide, :id => /\d+/
 
     put "node/create" => "node#create"
     get "node/:id/ways" => "way#ways_for_node", :id => /\d+/
@@ -116,7 +116,7 @@ OpenStreetMap::Application.routes.draw do
   get "/relation/:id" => "browse#relation", :id => /\d+/, :as => :relation
   get "/relation/:id/history" => "browse#relation_history", :id => /\d+/
   get "/changeset/:id" => "browse#changeset", :as => :changeset, :id => /\d+/
-  get "/changeset/:id/comments/feed" => "changeset#comments_feed", :as => :changeset_comments_feed, :id => /\d*/, :defaults => { :format => "rss" }
+  get "/changeset/:id/comments/feed" => "changeset_comments#comments_feed", :as => :changeset_comments_feed, :id => /\d*/, :defaults => { :format => "rss" }
   get "/note/:id" => "browse#note", :id => /\d+/, :as => "browse_note"
   get "/note/new" => "browse#new_note"
   get "/user/:display_name/history" => "changeset#index"
@@ -152,7 +152,7 @@ OpenStreetMap::Application.routes.draw do
   get "/about" => "site#about"
   get "/history" => "changeset#index"
   get "/history/feed" => "changeset#feed", :defaults => { :format => :atom }
-  get "/history/comments/feed" => "changeset#comments_feed", :as => :changesets_comments_feed, :defaults => { :format => "rss" }
+  get "/history/comments/feed" => "changeset_comments#comments_feed", :as => :changesets_comments_feed, :defaults => { :format => "rss" }
   get "/export" => "site#export"
   match "/login" => "users#login", :via => [:get, :post]
   match "/logout" => "users#logout", :via => [:get, :post]
diff --git a/test/controllers/changeset_comments_controller_test.rb b/test/controllers/changeset_comments_controller_test.rb
new file mode 100644
index 000000000..215f45957
--- /dev/null
+++ b/test/controllers/changeset_comments_controller_test.rb
@@ -0,0 +1,251 @@
+require "test_helper"
+
+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 => "comment", :id => "1" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/changeset/comment/1/hide", :method => :post },
+      { :controller => "changeset_comments", :action => "hide_comment", :id => "1" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/changeset/comment/1/unhide", :method => :post },
+      { :controller => "changeset_comments", :action => "unhide_comment", :id => "1" }
+    )
+    assert_routing(
+      { :path => "/changeset/1/comments/feed", :method => :get },
+      { :controller => "changeset_comments", :action => "comments_feed", :id => "1", :format => "rss" }
+    )
+    assert_routing(
+      { :path => "/history/comments/feed", :method => :get },
+      { :controller => "changeset_comments", :action => "comments_feed", :format => "rss" }
+    )
+  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 :comment, :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 :comment, :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 :comment, :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 :comment, :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 :comment, :params => { :id => 999111, :text => "This is a comment" }
+    end
+    assert_response :not_found
+
+    # not closed changeset
+    assert_no_difference "ChangesetComment.count" do
+      post :comment, :params => { :id => create(:changeset).id, :text => "This is a comment" }
+    end
+    assert_response :conflict
+
+    # no text
+    assert_no_difference "ChangesetComment.count" do
+      post :comment, :params => { :id => create(:changeset, :closed).id }
+    end
+    assert_response :bad_request
+
+    # empty text
+    assert_no_difference "ChangesetComment.count" do
+      post :comment, :params => { :id => create(:changeset, :closed).id, :text => "" }
+    end
+    assert_response :bad_request
+  end
+
+  ##
+  # test hide comment fail
+  def test_hide_comment_fail
+    # unauthorized
+    comment = create(:changeset_comment)
+    assert_equal true, comment.visible
+
+    post :hide_comment, :params => { :id => comment.id }
+    assert_response :unauthorized
+    assert_equal true, comment.reload.visible
+
+    basic_authorization create(:user).email, "test"
+
+    # not a moderator
+    post :hide_comment, :params => { :id => comment.id }
+    assert_response :forbidden
+    assert_equal true, comment.reload.visible
+
+    basic_authorization create(:moderator_user).email, "test"
+
+    # bad comment id
+    post :hide_comment, :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 :hide_comment, :params => { :id => comment.id }
+    assert_response :success
+    assert_equal false, comment.reload.visible
+  end
+
+  ##
+  # test unhide comment fail
+  def test_unhide_comment_fail
+    # unauthorized
+    comment = create(:changeset_comment, :visible => false)
+    assert_equal false, comment.visible
+
+    post :unhide_comment, :params => { :id => comment.id }
+    assert_response :unauthorized
+    assert_equal false, comment.reload.visible
+
+    basic_authorization create(:user).email, "test"
+
+    # not a moderator
+    post :unhide_comment, :params => { :id => comment.id }
+    assert_response :forbidden
+    assert_equal false, comment.reload.visible
+
+    basic_authorization create(:moderator_user).email, "test"
+
+    # bad comment id
+    post :unhide_comment, :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 :unhide_comment, :params => { :id => comment.id }
+    assert_response :success
+    assert_equal true, comment.reload.visible
+  end
+
+  ##
+  # test comments feed
+  def test_comments_feed
+    changeset = create(:changeset, :closed)
+    create_list(:changeset_comment, 3, :changeset => changeset)
+
+    get :comments_feed, :params => { :format => "rss" }
+    assert_response :success
+    assert_equal "application/rss+xml", @response.content_type
+    assert_select "rss", :count => 1 do
+      assert_select "channel", :count => 1 do
+        assert_select "item", :count => 3
+      end
+    end
+
+    get :comments_feed, :params => { :format => "rss", :limit => 2 }
+    assert_response :success
+    assert_equal "application/rss+xml", @response.content_type
+    assert_select "rss", :count => 1 do
+      assert_select "channel", :count => 1 do
+        assert_select "item", :count => 2
+      end
+    end
+
+    get :comments_feed, :params => { :id => changeset.id, :format => "rss" }
+    assert_response :success
+    assert_equal "application/rss+xml", @response.content_type
+    assert_select "rss", :count => 1 do
+      assert_select "channel", :count => 1 do
+        assert_select "item", :count => 3
+      end
+    end
+  end
+
+  ##
+  # test comments feed
+  def test_comments_feed_bad_limit
+    get :comments_feed, :params => { :format => "rss", :limit => 0 }
+    assert_response :bad_request
+
+    get :comments_feed, :params => { :format => "rss", :limit => 100001 }
+    assert_response :bad_request
+  end
+end
diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb
index fdb689978..72cf68ad9 100644
--- a/test/controllers/changeset_controller_test.rb
+++ b/test/controllers/changeset_controller_test.rb
@@ -41,26 +41,10 @@ class ChangesetControllerTest < ActionController::TestCase
       { :path => "/api/0.6/changeset/1/close", :method => :put },
       { :controller => "changeset", :action => "close", :id => "1" }
     )
-    assert_routing(
-      { :path => "/api/0.6/changeset/1/comment", :method => :post },
-      { :controller => "changeset", :action => "comment", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/changeset/comment/1/hide", :method => :post },
-      { :controller => "changeset", :action => "hide_comment", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/changeset/comment/1/unhide", :method => :post },
-      { :controller => "changeset", :action => "unhide_comment", :id => "1" }
-    )
     assert_routing(
       { :path => "/api/0.6/changesets", :method => :get },
       { :controller => "changeset", :action => "query" }
     )
-    assert_routing(
-      { :path => "/changeset/1/comments/feed", :method => :get },
-      { :controller => "changeset", :action => "comments_feed", :id => "1", :format => "rss" }
-    )
     assert_routing(
       { :path => "/user/name/history", :method => :get },
       { :controller => "changeset", :action => "index", :display_name => "name" }
@@ -85,10 +69,6 @@ class ChangesetControllerTest < ActionController::TestCase
       { :path => "/history/feed", :method => :get },
       { :controller => "changeset", :action => "feed", :format => :atom }
     )
-    assert_routing(
-      { :path => "/history/comments/feed", :method => :get },
-      { :controller => "changeset", :action => "comments_feed", :format => "rss" }
-    )
   end
 
   # -----------------------
@@ -2139,107 +2119,6 @@ CHANGESET
     assert_select "osmChange node[id='#{node.id}'][version='1']", 0
   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 :comment, :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 :comment, :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 :comment, :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 :comment, :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 :comment, :params => { :id => 999111, :text => "This is a comment" }
-    end
-    assert_response :not_found
-
-    # not closed changeset
-    assert_no_difference "ChangesetComment.count" do
-      post :comment, :params => { :id => create(:changeset).id, :text => "This is a comment" }
-    end
-    assert_response :conflict
-
-    # no text
-    assert_no_difference "ChangesetComment.count" do
-      post :comment, :params => { :id => create(:changeset, :closed).id }
-    end
-    assert_response :bad_request
-
-    # empty text
-    assert_no_difference "ChangesetComment.count" do
-      post :comment, :params => { :id => create(:changeset, :closed).id, :text => "" }
-    end
-    assert_response :bad_request
-  end
-
   ##
   # test subscribe success
   def test_subscribe_success
@@ -2337,128 +2216,6 @@ CHANGESET
     assert_response :not_found
   end
 
-  ##
-  # test hide comment fail
-  def test_hide_comment_fail
-    # unauthorized
-    comment = create(:changeset_comment)
-    assert_equal true, comment.visible
-
-    post :hide_comment, :params => { :id => comment.id }
-    assert_response :unauthorized
-    assert_equal true, comment.reload.visible
-
-    basic_authorization create(:user).email, "test"
-
-    # not a moderator
-    post :hide_comment, :params => { :id => comment.id }
-    assert_response :forbidden
-    assert_equal true, comment.reload.visible
-
-    basic_authorization create(:moderator_user).email, "test"
-
-    # bad comment id
-    post :hide_comment, :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 :hide_comment, :params => { :id => comment.id }
-    assert_response :success
-    assert_equal false, comment.reload.visible
-  end
-
-  ##
-  # test unhide comment fail
-  def test_unhide_comment_fail
-    # unauthorized
-    comment = create(:changeset_comment, :visible => false)
-    assert_equal false, comment.visible
-
-    post :unhide_comment, :params => { :id => comment.id }
-    assert_response :unauthorized
-    assert_equal false, comment.reload.visible
-
-    basic_authorization create(:user).email, "test"
-
-    # not a moderator
-    post :unhide_comment, :params => { :id => comment.id }
-    assert_response :forbidden
-    assert_equal false, comment.reload.visible
-
-    basic_authorization create(:moderator_user).email, "test"
-
-    # bad comment id
-    post :unhide_comment, :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 :unhide_comment, :params => { :id => comment.id }
-    assert_response :success
-    assert_equal true, comment.reload.visible
-  end
-
-  ##
-  # test comments feed
-  def test_comments_feed
-    changeset = create(:changeset, :closed)
-    create_list(:changeset_comment, 3, :changeset => changeset)
-
-    get :comments_feed, :params => { :format => "rss" }
-    assert_response :success
-    assert_equal "application/rss+xml", @response.content_type
-    assert_select "rss", :count => 1 do
-      assert_select "channel", :count => 1 do
-        assert_select "item", :count => 3
-      end
-    end
-
-    get :comments_feed, :params => { :format => "rss", :limit => 2 }
-    assert_response :success
-    assert_equal "application/rss+xml", @response.content_type
-    assert_select "rss", :count => 1 do
-      assert_select "channel", :count => 1 do
-        assert_select "item", :count => 2
-      end
-    end
-
-    get :comments_feed, :params => { :id => changeset.id, :format => "rss" }
-    assert_response :success
-    assert_equal "application/rss+xml", @response.content_type
-    assert_select "rss", :count => 1 do
-      assert_select "channel", :count => 1 do
-        assert_select "item", :count => 3
-      end
-    end
-  end
-
-  ##
-  # test comments feed
-  def test_comments_feed_bad_limit
-    get :comments_feed, :params => { :format => "rss", :limit => 0 }
-    assert_response :bad_request
-
-    get :comments_feed, :params => { :format => "rss", :limit => 100001 }
-    assert_response :bad_request
-  end
-
   private
 
   ##