Merge remote-tracking branch 'upstream/pull/2050'

.rubocop_todo.yml

@@ -48,7 +48,7 @@ Metrics/BlockNesting:
 # Offense count: 63
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 1801
+  Max: 1627
 
 # Offense count: 72
 Metrics/CyclomaticComplexity:

app/controllers/changeset_comments_controller.rb

@@ -0,0 +1,125 @@
+class ChangesetCommentsController < ApplicationController
+  before_action :authorize_web, :only => [:index]
+  before_action :set_locale, :only => [:index]
+  before_action :authorize, :only => [:create, :destroy, :restore]
+  before_action :require_moderator, :only => [:destroy, :restore]
+  before_action :require_allow_write_api, :only => [:create, :destroy, :restore]
+  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
+
+  ##
+  # Get a feed of recent changeset comments
+  def index
+    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

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 @@ def feed
     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 @@ def unsubscribe
     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 @@ def conditions_ids(changesets, ids)
   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

app/views/changeset/_comment.html.erb → app/views/changeset_comments/_comment.html.erb

@@ -1,6 +1,6 @@
-<h2><%= t "changeset.rss.comment", :author => comment.author.display_name, 
+<h2><%= t ".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>
+  <div class="changeset-comment-description" style="font-size: smaller; color: #999999"><%= t ".commented_at_by_html", :when => friendly_date(comment.created_at), :user => comment.author.display_name %></div>
   <div class="changeset-comment-text"><%= comment.body %></div>
 </div>

app/views/changeset/_comments.rss.builder → app/views/changeset_comments/_comments.rss.builder

@@ -1,6 +1,6 @@
 comments.each do |comment|
   xml.item do
-    xml.title t("changeset.rss.comment", :author => comment.author.display_name, :changeset_id => comment.changeset.id.to_s)
+    xml.title t(".comment", :author => comment.author.display_name, :changeset_id => comment.changeset.id.to_s)
 
     xml.link url_for(:controller => "browse", :action => "changeset", :id => comment.changeset.id, :anchor => "c#{comment.id}", :only_path => false)
     xml.guid url_for(:controller => "browse", :action => "changeset", :id => comment.changeset.id, :anchor => "c#{comment.id}", :only_path => false)

app/views/changeset/comments_feed.rss.builder → app/views/changeset_comments/index.rss.builder

@@ -2,9 +2,9 @@ xml.rss("version" => "2.0",
         "xmlns:dc" => "http://purl.org/dc/elements/1.1/") do
   xml.channel do
     if @changeset
-      xml.title t("changeset.rss.title_particular", :changeset_id => @changeset.id)
+      xml.title t(".title_particular", :changeset_id => @changeset.id)
     else
-      xml.title t("changeset.rss.title_all")
+      xml.title t(".title_all")
     end
     xml.link url_for(:controller => "site", :action => "index", :only_path => false)
 

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

app/views/changeset_comments/timeout.html.erb

@@ -0,0 +1 @@
+<p><%= t '.sorry' %></p>

config/locales/en.yml

@@ -242,13 +242,17 @@ en:
       load_more: "Load more"
     timeout:
       sorry: "Sorry, the list of changesets you requested took too long to retrieve."
-    rss:
-      title_all: OpenStreetMap changeset discussion
-      title_particular: "OpenStreetMap changeset #%{changeset_id} discussion"
+  changeset_comments:
+    comment:
       comment: "New comment on changeset #%{changeset_id} by %{author}"
-      commented_at_html: "Updated %{when} ago"
       commented_at_by_html: "Updated %{when} ago by %{user}"
-      full: Full discussion
+    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."
   diary_entry:
     new:
       title: New Diary Entry

config/routes.rb

@@ -16,9 +16,9 @@
     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#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+/
 
     put "node/create" => "node#create"
     get "node/:id/ways" => "way#ways_for_node", :id => /\d+/
@@ -116,7 +116,7 @@
   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#index", :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 @@
   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#index", :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]