From: Tom Hughes Date: Sun, 4 Feb 2018 15:09:40 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/1704' X-Git-Tag: live~4133 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/673b58f96f3acfd0281c62a2a7f080825b6fb28b?hp=-c Merge remote-tracking branch 'upstream/pull/1704' --- 673b58f96f3acfd0281c62a2a7f080825b6fb28b diff --combined .rubocop_todo.yml index 3ebd4d35f,d15d23721..2c8d29eb8 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@@ -55,6 -55,11 +55,6 @@@ Lint/InterpolationCheck Exclude: - 'test/controllers/node_controller_test.rb' -# Offense count: 2 -Lint/RescueWithoutErrorClass: - Exclude: - - 'app/helpers/browse_helper.rb' - # Offense count: 2 Lint/ShadowingOuterLocalVariable: Exclude: @@@ -77,7 -82,7 +77,7 @@@ Metrics/BlockNesting # Offense count: 63 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 1796 + Max: 1797 # Offense count: 71 Metrics/CyclomaticComplexity: @@@ -222,8 -227,3 +222,8 @@@ Style/NumericLiterals # SupportedStyles: compact, exploded Style/RaiseArgs: Enabled: false + +# Offense count: 2 +Style/RescueStandardError: + Exclude: + - 'app/helpers/browse_helper.rb' diff --combined app/controllers/changeset_controller.rb index f294d23d5,329cbd79c..0a63d5253 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@@ -7,12 -7,12 +7,12 @@@ class ChangesetController < Application skip_before_action :verify_authenticity_token, :except => [:list] before_action :authorize_web, :only => [:list, :feed, :comments_feed] before_action :set_locale, :only => [:list, :feed, :comments_feed] - before_action :authorize, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment] + 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, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment] - before_action :require_public_data, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe] - before_action :check_api_writable, :only => [:create, :update, :delete, :upload, :include, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment] - before_action :check_api_readable, :except => [:create, :update, :delete, :upload, :download, :query, :list, :feed, :comment, :subscribe, :unsubscribe, :comments_feed] + 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, :list, :feed, :comment, :subscribe, :unsubscribe, :comments_feed] before_action(:only => [:list, :feed, :comments_feed]) { |c| c.check_database_readable(true) } around_action :api_call_handle_error, :except => [:list, :feed, :comments_feed] around_action :api_call_timeout, :except => [:list, :feed, :comments_feed, :upload] @@@ -296,7 -296,9 +296,7 @@@ changesets = changesets.where(:user_id => current_user.nearby) end - if @params[:max_id] - changesets = changesets.where("changesets.id <= ?", @params[:max_id]) - end + changesets = changesets.where("changesets.id <= ?", @params[:max_id]) if @params[:max_id] @edits = changesets.order("changesets.id DESC").limit(20).preload(:user, :changeset_tags, :comments) @@@ -332,7 -334,9 +332,7 @@@ # Notify current subscribers of the new comment changeset.subscribers.visible.each do |user| - if current_user != user - Notifier.changeset_comment_notification(comment, user).deliver_now - end + Notifier.changeset_comment_notification(comment, user).deliver_now if current_user != user end # Add the commenter to the subscribers if necessary @@@ -353,7 -357,6 +353,6 @@@ # Find the changeset and check it is valid changeset = Changeset.find(id) - raise OSM::APIChangesetNotYetClosedError, changeset if changeset.is_open? raise OSM::APIChangesetAlreadySubscribedError, changeset if changeset.subscribers.exists?(current_user.id) # Add the subscriber @@@ -374,7 -377,6 +373,6 @@@ # Find the changeset and check it is valid changeset = Changeset.find(id) - raise OSM::APIChangesetNotYetClosedError, changeset if changeset.is_open? raise OSM::APIChangesetNotSubscribedError, changeset unless changeset.subscribers.exists?(current_user.id) # Remove the subscriber diff --combined test/controllers/changeset_controller_test.rb index 5205714df,329be5be1..cb584759a --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@@ -812,7 -812,9 +812,7 @@@ CHANGESE assert_equal 2, Node.find(new_node_id).tags.size, "new node should have two tags" assert_equal [new_node_id, node.id], Way.find(way.id).nds, "way nodes should match" Relation.find(relation.id).members.each do |type, id, _role| - if type == "node" - assert_equal new_node_id, id, "relation should contain new node" - end + assert_equal new_node_id, id, "relation should contain new node" if type == "node" end end @@@ -2246,6 -2248,13 +2246,13 @@@ CHANGESE post :subscribe, :params => { :id => changeset.id } end assert_response :success + + # not closed changeset + changeset = create(:changeset) + assert_difference "changeset.subscribers.count", 1 do + post :subscribe, :params => { :id => changeset.id } + end + assert_response :success end ## @@@ -2268,13 -2277,6 +2275,6 @@@ end assert_response :not_found - # not closed changeset - changeset = create(:changeset) - assert_no_difference "changeset.subscribers.count" do - post :subscribe, :params => { :id => changeset.id } - end - assert_response :conflict - # trying to subscribe when already subscribed changeset = create(:changeset, :closed) changeset.subscribers.push(user) @@@ -2296,6 -2298,15 +2296,15 @@@ post :unsubscribe, :params => { :id => changeset.id } end assert_response :success + + # not closed changeset + changeset = create(:changeset) + changeset.subscribers.push(user) + + assert_difference "changeset.subscribers.count", -1 do + post :unsubscribe, :params => { :id => changeset.id } + end + assert_response :success end ## @@@ -2316,13 -2327,6 +2325,6 @@@ end assert_response :not_found - # not closed changeset - changeset = create(:changeset) - assert_no_difference "changeset.subscribers.count" do - post :unsubscribe, :params => { :id => changeset.id } - end - assert_response :conflict - # trying to unsubscribe when not subscribed changeset = create(:changeset, :closed) assert_no_difference "changeset.subscribers.count" do