Merge pull request #4587 from AntonKhorev/changeset-comment-routes

Move changeset comments feed to resourceful routes

app/controllers/feeds/changeset_comments_controller.rb

@@ -0,0 +1,52 @@
+module Feeds
+  class ChangesetCommentsController < ApplicationController
+    before_action :authorize_web
+    before_action :set_locale
+
+    authorize_resource
+
+    before_action -> { check_database_readable(:need_api => true) }
+    around_action :web_timeout
+
+    ##
+    # Get a feed of recent changeset comments
+    def index
+      if params[:changeset_id]
+        # Extract the arguments
+        changeset_id = params[:changeset_id].to_i
+
+        # Find the changeset
+        changeset = Changeset.find(changeset_id)
+
+        # Return comments for this changeset only
+        @comments = changeset.comments.includes(:author, :changeset).reverse_order.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
+end

app/views/changeset_comments/index.rss.builder → app/views/feeds/changeset_comments/index.rss.builder

@@ -6,7 +6,7 @@ xml.rss("version" => "2.0",
     else
       xml.title t(".title_all")
     end
-    xml.link url_for(:controller => "site", :action => "index", :only_path => false)
+    xml.link root_url
 
     xml << render(:partial => "comment", :collection => @comments)
   end

config/locales/en.yml

@@ -246,6 +246,18 @@ en:
       entry:
         comment: Comment
         full: Full note
+  feeds:
+    changeset_comments:
+      comment:
+        comment: "New comment on changeset #%{changeset_id} by %{author}"
+        commented_at_by_html: "Updated %{when} by %{user}"
+      comments:
+        comment: "New comment on changeset #%{changeset_id} by %{author}"
+      index:
+        title_all: OpenStreetMap changeset discussion
+        title_particular: "OpenStreetMap changeset #%{changeset_id} discussion"
+      timeout:
+        sorry: "Sorry, the list of changeset comments you requested took too long to retrieve."
   account:
     deletions:
       show:
@@ -490,17 +502,6 @@ en:
       relations_paginated: "Relations (%{x}-%{y} of %{count})"
     timeout:
       sorry: "Sorry, the list of changesets you requested took too long to retrieve."
-  changeset_comments:
-    comment:
-      comment: "New comment on changeset #%{changeset_id} by %{author}"
-      commented_at_by_html: "Updated %{when} by %{user}"
-    comments:
-      comment: "New comment on changeset #%{changeset_id} by %{author}"
-    index:
-      title_all: OpenStreetMap changeset discussion
-      title_particular: "OpenStreetMap changeset #%{changeset_id} discussion"
-    timeout:
-      sorry: "Sorry, the list of changeset comments you requested took too long to retrieve."
   dashboards:
     contact:
       km away: "%{count}km away"

config/routes.rb

@@ -125,8 +125,11 @@
   resources :old_relations, :path => "/relation/:id/history", :id => /\d+/, :version => /\d+/, :param => :version, :only => :show
   resources :changesets, :path => "changeset", :id => /\d+/, :only => :show do
     match :subscribe, :unsubscribe, :on => :member, :via => [:get, :post]
+
+    namespace :feeds, :path => "" do
+      resources :changeset_comments, :path => "comments/feed", :only => :index, :defaults => { :format => "rss" }
+    end
   end
-  get "/changeset/:id/comments/feed" => "changeset_comments#index", :as => :changeset_comments_feed, :id => /\d*/, :defaults => { :format => "rss" }
   resources :notes, :path => "note", :id => /\d+/, :only => [:show, :new]
 
   get "/user/:display_name/history" => "changesets#index"
@@ -164,7 +167,9 @@
   get "/communities" => "site#communities"
   get "/history" => "changesets#index"
   get "/history/feed" => "changesets#feed", :defaults => { :format => :atom }
-  get "/history/comments/feed" => "changeset_comments#index", :as => :changesets_comments_feed, :defaults => { :format => "rss" }
+  namespace :feeds, :path => "" do
+    resources :changeset_comments, :path => "/history/comments/feed", :only => :index, :defaults => { :format => "rss" }
+  end
   get "/export" => "site#export"
   get "/login" => "sessions#new"
   post "/login" => "sessions#create"

test/controllers/feeds/changeset_comments_controller_test.rb

@@ -0,0 +1,72 @@
+require "test_helper"
+
+module Feeds
+  class ChangesetCommentsControllerTest < ActionDispatch::IntegrationTest
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/changeset/1/comments/feed", :method => :get },
+        { :controller => "feeds/changeset_comments", :action => "index", :changeset_id => "1", :format => "rss" }
+      )
+      assert_routing(
+        { :path => "/history/comments/feed", :method => :get },
+        { :controller => "feeds/changeset_comments", :action => "index", :format => "rss" }
+      )
+    end
+
+    ##
+    # test comments feed
+    def test_feed
+      changeset = create(:changeset, :closed)
+      create_list(:changeset_comment, 3, :changeset => changeset)
+
+      get feeds_changeset_comments_path(:format => "rss")
+      assert_response :success
+      assert_equal "application/rss+xml", @response.media_type
+      assert_select "rss", :count => 1 do
+        assert_select "channel", :count => 1 do
+          assert_select "item", :count => 3
+        end
+      end
+
+      get feeds_changeset_comments_path(:format => "rss", :limit => 2)
+      assert_response :success
+      assert_equal "application/rss+xml", @response.media_type
+      assert_select "rss", :count => 1 do
+        assert_select "channel", :count => 1 do
+          assert_select "item", :count => 2
+        end
+      end
+
+      get changeset_feeds_changeset_comments_path(changeset, :format => "rss")
+      assert_response :success
+      assert_equal "application/rss+xml", @response.media_type
+      last_comment_id = -1
+      assert_select "rss", :count => 1 do
+        assert_select "channel", :count => 1 do
+          assert_select "item", :count => 3 do |items|
+            items.each do |item|
+              assert_select item, "link", :count => 1 do |link|
+                match = assert_match(/^#{changeset_url changeset}#c(\d+)$/, link.text)
+                comment_id = match[1].to_i
+                assert_operator comment_id, "<", last_comment_id if last_comment_id != -1
+                last_comment_id = comment_id
+              end
+            end
+          end
+        end
+      end
+    end
+
+    ##
+    # test comments feed
+    def test_feed_bad_limit
+      get feeds_changeset_comments_path(:format => "rss", :limit => 0)
+      assert_response :bad_request
+
+      get feeds_changeset_comments_path(:format => "rss", :limit => 100001)
+      assert_response :bad_request
+    end
+  end
+end