Merge remote-tracking branch 'upstream/pull/2052'
authorTom Hughes <tom@compton.nu>
Thu, 8 Nov 2018 17:44:57 +0000 (17:44 +0000)
committerTom Hughes <tom@compton.nu>
Thu, 8 Nov 2018 17:44:57 +0000 (17:44 +0000)
1  2 
.rubocop_todo.yml
config/locales/en.yml
config/routes.rb
test/controllers/changeset_controller_test.rb

diff --combined .rubocop_todo.yml
index ea957a8caa294444eaf3c715bde9abe80684f1e2,e6ecfdb4aa4304567efbd8fca5bf6e37b66ae793..7232d199b03b52a27137d55691389f75b6413f87
@@@ -48,7 -48,7 +48,7 @@@ Metrics/BlockNesting
  # Offense count: 63
  # Configuration parameters: CountComments.
  Metrics/ClassLength:
 -  Max: 1801
 +  Max: 1627
  
  # Offense count: 72
  Metrics/CyclomaticComplexity:
@@@ -177,7 -177,7 +177,7 @@@ Style/FrozenStringLiteralComment
  # Cop supports --auto-correct.
  Style/IfUnlessModifier:
    Exclude:
-     - 'app/controllers/way_controller.rb'
+     - 'app/controllers/ways_controller.rb'
  
  # Offense count: 70
  # Cop supports --auto-correct.
diff --combined config/locales/en.yml
index 6a496be1a106fc34501037235f49f65db5c23346,d0b574a0966bb134f1ca4c5f2179a9e55a5dec0f..197c6308ccb3385c0250396a5ad791bd8c2cce00
@@@ -242,18 -242,14 +242,18 @@@ 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:
+   diary_entries:
      new:
        title: New Diary Entry
        publish_button: "Publish"
diff --combined config/routes.rb
index 7c3bf0b94a28d1bcb22121c6d9565054e8fb743d,1f25a62c68fe6bcdd870d7c758fa9ef81d50562b..78146606429787f6ec7fd78c74a628ce7b9f0341
@@@ -16,42 -16,42 +16,42 @@@ OpenStreetMap::Application.routes.draw 
      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+/
-     get "node/:id/relations" => "relation#relations_for_node", :id => /\d+/
-     get "node/:id/history" => "old_node#history", :id => /\d+/
-     post "node/:id/:version/redact" => "old_node#redact", :version => /\d+/, :id => /\d+/
-     get "node/:id/:version" => "old_node#version", :id => /\d+/, :version => /\d+/
-     get "node/:id" => "node#read", :id => /\d+/
-     put "node/:id" => "node#update", :id => /\d+/
-     delete "node/:id" => "node#delete", :id => /\d+/
-     get "nodes" => "node#nodes"
-     put "way/create" => "way#create"
-     get "way/:id/history" => "old_way#history", :id => /\d+/
-     get "way/:id/full" => "way#full", :id => /\d+/
-     get "way/:id/relations" => "relation#relations_for_way", :id => /\d+/
-     post "way/:id/:version/redact" => "old_way#redact", :version => /\d+/, :id => /\d+/
-     get "way/:id/:version" => "old_way#version", :id => /\d+/, :version => /\d+/
-     get "way/:id" => "way#read", :id => /\d+/
-     put "way/:id" => "way#update", :id => /\d+/
-     delete "way/:id" => "way#delete", :id => /\d+/
-     get "ways" => "way#ways"
-     put "relation/create" => "relation#create"
-     get "relation/:id/relations" => "relation#relations_for_relation", :id => /\d+/
-     get "relation/:id/history" => "old_relation#history", :id => /\d+/
-     get "relation/:id/full" => "relation#full", :id => /\d+/
-     post "relation/:id/:version/redact" => "old_relation#redact", :version => /\d+/, :id => /\d+/
-     get "relation/:id/:version" => "old_relation#version", :id => /\d+/, :version => /\d+/
-     get "relation/:id" => "relation#read", :id => /\d+/
-     put "relation/:id" => "relation#update", :id => /\d+/
-     delete "relation/:id" => "relation#delete", :id => /\d+/
-     get "relations" => "relation#relations"
+     put "node/create" => "nodes#create"
+     get "node/:id/ways" => "ways#ways_for_node", :id => /\d+/
+     get "node/:id/relations" => "relations#relations_for_node", :id => /\d+/
+     get "node/:id/history" => "old_nodes#history", :id => /\d+/
+     post "node/:id/:version/redact" => "old_nodes#redact", :version => /\d+/, :id => /\d+/
+     get "node/:id/:version" => "old_nodes#version", :id => /\d+/, :version => /\d+/
+     get "node/:id" => "nodes#read", :id => /\d+/
+     put "node/:id" => "nodes#update", :id => /\d+/
+     delete "node/:id" => "nodes#delete", :id => /\d+/
+     get "nodes" => "nodes#nodes"
+     put "way/create" => "ways#create"
+     get "way/:id/history" => "old_ways#history", :id => /\d+/
+     get "way/:id/full" => "ways#full", :id => /\d+/
+     get "way/:id/relations" => "relations#relations_for_way", :id => /\d+/
+     post "way/:id/:version/redact" => "old_ways#redact", :version => /\d+/, :id => /\d+/
+     get "way/:id/:version" => "old_ways#version", :id => /\d+/, :version => /\d+/
+     get "way/:id" => "ways#read", :id => /\d+/
+     put "way/:id" => "ways#update", :id => /\d+/
+     delete "way/:id" => "ways#delete", :id => /\d+/
+     get "ways" => "ways#ways"
+     put "relation/create" => "relations#create"
+     get "relation/:id/relations" => "relations#relations_for_relation", :id => /\d+/
+     get "relation/:id/history" => "old_relations#history", :id => /\d+/
+     get "relation/:id/full" => "relations#full", :id => /\d+/
+     post "relation/:id/:version/redact" => "old_relations#redact", :version => /\d+/, :id => /\d+/
+     get "relation/:id/:version" => "old_relations#version", :id => /\d+/, :version => /\d+/
+     get "relation/:id" => "relations#read", :id => /\d+/
+     put "relation/:id" => "relations#update", :id => /\d+/
+     delete "relation/:id" => "relations#delete", :id => /\d+/
+     get "relations" => "relations#relations"
  
      get "map" => "api#map"
  
    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"
    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]
    post "/trace/:id/delete" => "traces#delete", :id => /\d+/
  
    # diary pages
-   match "/diary/new" => "diary_entry#new", :via => [:get, :post]
-   get "/diary/friends" => "diary_entry#index", :friends => true, :as => "friend_diaries"
-   get "/diary/nearby" => "diary_entry#index", :nearby => true, :as => "nearby_diaries"
-   get "/user/:display_name/diary/rss" => "diary_entry#rss", :defaults => { :format => :rss }
-   get "/diary/:language/rss" => "diary_entry#rss", :defaults => { :format => :rss }
-   get "/diary/rss" => "diary_entry#rss", :defaults => { :format => :rss }
-   get "/user/:display_name/diary/comments/:page" => "diary_entry#comments", :page => /[1-9][0-9]*/
-   get "/user/:display_name/diary/comments/" => "diary_entry#comments"
-   get "/user/:display_name/diary" => "diary_entry#index"
-   get "/diary/:language" => "diary_entry#index"
-   get "/diary" => "diary_entry#index"
-   get "/user/:display_name/diary/:id" => "diary_entry#show", :id => /\d+/, :as => :diary_entry
-   post "/user/:display_name/diary/:id/newcomment" => "diary_entry#comment", :id => /\d+/
-   match "/user/:display_name/diary/:id/edit" => "diary_entry#edit", :via => [:get, :post], :id => /\d+/
-   post "/user/:display_name/diary/:id/hide" => "diary_entry#hide", :id => /\d+/, :as => :hide_diary_entry
-   post "/user/:display_name/diary/:id/hidecomment/:comment" => "diary_entry#hidecomment", :id => /\d+/, :comment => /\d+/, :as => :hide_diary_comment
-   post "/user/:display_name/diary/:id/subscribe" => "diary_entry#subscribe", :as => :diary_entry_subscribe, :id => /\d+/
-   post "/user/:display_name/diary/:id/unsubscribe" => "diary_entry#unsubscribe", :as => :diary_entry_unsubscribe, :id => /\d+/
+   match "/diary/new" => "diary_entries#new", :via => [:get, :post]
+   get "/diary/friends" => "diary_entries#index", :friends => true, :as => "friend_diaries"
+   get "/diary/nearby" => "diary_entries#index", :nearby => true, :as => "nearby_diaries"
+   get "/user/:display_name/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss }
+   get "/diary/:language/rss" => "diary_entries#rss", :defaults => { :format => :rss }
+   get "/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss }
+   get "/user/:display_name/diary/comments/:page" => "diary_entries#comments", :page => /[1-9][0-9]*/
+   get "/user/:display_name/diary/comments/" => "diary_entries#comments"
+   get "/user/:display_name/diary" => "diary_entries#index"
+   get "/diary/:language" => "diary_entries#index"
+   get "/diary" => "diary_entries#index"
+   get "/user/:display_name/diary/:id" => "diary_entries#show", :id => /\d+/, :as => :diary_entry
+   post "/user/:display_name/diary/:id/newcomment" => "diary_entries#comment", :id => /\d+/
+   match "/user/:display_name/diary/:id/edit" => "diary_entries#edit", :via => [:get, :post], :id => /\d+/
+   post "/user/:display_name/diary/:id/hide" => "diary_entries#hide", :id => /\d+/, :as => :hide_diary_entry
+   post "/user/:display_name/diary/:id/hidecomment/:comment" => "diary_entries#hidecomment", :id => /\d+/, :comment => /\d+/, :as => :hide_diary_comment
+   post "/user/:display_name/diary/:id/subscribe" => "diary_entries#subscribe", :as => :diary_entry_subscribe, :id => /\d+/
+   post "/user/:display_name/diary/:id/unsubscribe" => "diary_entries#unsubscribe", :as => :diary_entry_unsubscribe, :id => /\d+/
  
    # user pages
    get "/user/:display_name" => "users#show", :as => "user"
index 72cf68ad9f3f81cbfb8becea297a9ce9e94dba23,472ebb25942c70e689cdc12c99a0bf4588bf42ef..7dc479cad825a96965d1a17ff5b50fc726b03a7e
@@@ -41,10 -41,26 +41,10 @@@ class ChangesetControllerTest < ActionC
        { :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" }
        { :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
  
    # -----------------------
@@@ -1508,7 -1528,7 +1508,7 @@@ CHANGESE
      changeset_id = @response.body.to_i
  
      # add a single node to it
-     with_controller(NodeController.new) do
+     with_controller(NodesController.new) do
        content "<osm><node lon='1' lat='2' changeset='#{changeset_id}'/></osm>"
        put :create
        assert_response :success, "Couldn't create node."
      assert_select "osm>changeset[max_lat='2.0000000']", 1
  
      # add another node to it
-     with_controller(NodeController.new) do
+     with_controller(NodesController.new) do
        content "<osm><node lon='2' lat='1' changeset='#{changeset_id}'/></osm>"
        put :create
        assert_response :success, "Couldn't create second node."
      assert_select "osm>changeset[max_lat='2.0000000']", 1
  
      # add (delete) a way to it, which contains a point at (3,3)
-     with_controller(WayController.new) do
+     with_controller(WaysController.new) do
        content update_changeset(way.to_xml, changeset_id)
        put :delete, :params => { :id => way.id }
        assert_response :success, "Couldn't delete a way."
      changeset.num_changes = Changeset::MAX_ELEMENTS - offset
      changeset.save!
  
-     with_controller(NodeController.new) do
+     with_controller(NodesController.new) do
        # create a new node
        content "<osm><node changeset='#{cs_id}' lat='0.0' lon='0.0'/></osm>"
        put :create
      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
      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
  
    ##