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

diff --combined .rubocop_todo.yml
index 3ebd4d35f686bb0f4252f2689524b342ea3406ba,d15d23721cb8dc10537bd1289bd4796a824d1f1b..2c8d29eb887fac048d55a61cfcdb3d34e5138fa3
--- 1/.rubocop_todo.yml
--- 2/.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 f294d23d59026c31b931c04c9f9b97d8bdfb849f,329cbd79c07fe3b9d21a278c5438d8116acfe7ce..0a63d525373aa33892c08a243895aa90e3b57598
--- 1/app/controllers/changeset_controller.rb
--- 2/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 5205714df9171ca72232ce460438b4fe72a0c3db,329be5be1fd14cc9916426c8992c421d7b4f40fd..cb584759ae4ebf478a73129cafc33afed8843f2b
--- 1/test/controllers/changeset_controller_test.rb
--- 2/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