From: Tom Hughes Date: Thu, 8 Nov 2018 17:44:57 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/2052' X-Git-Tag: live~3846 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/70d6880e10881dfd4b68f51cf16609a9f8aaff24?ds=sidebyside;hp=-c Merge remote-tracking branch 'upstream/pull/2052' --- 70d6880e10881dfd4b68f51cf16609a9f8aaff24 diff --combined .rubocop_todo.yml index ea957a8ca,e6ecfdb4a..7232d199b --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@@ -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 6a496be1a,d0b574a09..197c6308c --- a/config/locales/en.yml +++ b/config/locales/en.yml @@@ -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 7c3bf0b94,1f25a62c6..781466064 --- a/config/routes.rb +++ b/config/routes.rb @@@ -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" @@@ -116,7 -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 +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] @@@ -214,24 -214,24 +214,24 @@@ 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" diff --combined test/controllers/changeset_controller_test.rb index 72cf68ad9,472ebb259..7dc479cad --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@@ -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" } @@@ -69,6 -85,10 +69,6 @@@ { :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 "" put :create assert_response :success, "Couldn't create node." @@@ -1523,7 -1543,7 +1523,7 @@@ 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 "" put :create assert_response :success, "Couldn't create second node." @@@ -1538,7 -1558,7 +1538,7 @@@ 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." @@@ -1818,7 -1838,7 +1818,7 @@@ changeset.num_changes = Changeset::MAX_ELEMENTS - offset changeset.save! - with_controller(NodeController.new) do + with_controller(NodesController.new) do # create a new node content "" put :create @@@ -2119,6 -2139,107 +2119,6 @@@ 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 @@@ -2216,6 -2337,128 +2216,6 @@@ 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 ##