Merge branch 'master' into overpass

test/controllers/amf_controller_test.rb

@@ -26,8 +26,13 @@ class AmfControllerTest < ActionController::TestCase
     assert_response :success
     amf_parse_response                                         
     way = amf_result("/1")
-    assert_equal way[0], 0
-    assert_equal way[2], id
+    assert_equal 0, way[0]
+    assert_equal "", way[1]
+    assert_equal id, way[2]
+    assert_equal 1, way[3].length
+    assert_equal 3, way[3][0][2]
+    assert_equal 1, way[5]
+    assert_equal 2, way[6]
   end
 
   def test_getway_invisible
@@ -38,10 +43,64 @@ class AmfControllerTest < ActionController::TestCase
     assert_response :success
     amf_parse_response
     way = amf_result("/1")
-    assert_equal way[0], -4
-    assert_equal way[1], "way"
-    assert_equal way[2], id
-    assert way[3].nil? and way[4].nil?
+    assert_equal -4, way[0], -4
+    assert_equal "way", way[1]
+    assert_equal id, way[2]
+    assert way[3].nil? and way[4].nil? and way[5].nil? and way[6].nil?
+  end
+
+  def test_getway_with_versions
+    # check a way with multiple versions
+    id = current_ways(:way_with_versions).id
+    amf_content "getway", "/1", [id]
+    post :amf_read
+    assert_response :success
+    amf_parse_response                                         
+    way = amf_result("/1")
+    assert_equal 0, way[0]
+    assert_equal "", way[1]
+    assert_equal id, way[2]
+    assert_equal 1, way[3].length
+    assert_equal 15, way[3][0][2]
+    assert_equal 4, way[5]
+    assert_equal 2, way[6]
+  end
+
+  def test_getway_with_duplicate_nodes
+    # check a way with duplicate nodes
+    id = current_ways(:way_with_duplicate_nodes).id
+    amf_content "getway", "/1", [id]
+    post :amf_read
+    assert_response :success
+    amf_parse_response                                         
+    way = amf_result("/1")
+    assert_equal 0, way[0]
+    assert_equal "", way[1]
+    assert_equal id, way[2]
+    assert_equal 2, way[3].length
+    assert_equal 4, way[3][0][2]
+    assert_equal 4, way[3][1][2]
+    assert_equal 1, way[5]
+    assert_equal 2, way[6]
+  end
+
+  def test_getway_with_multiple_nodes
+    # check a way with multiple nodes
+    id = current_ways(:way_with_multiple_nodes).id
+    amf_content "getway", "/1", [id]
+    post :amf_read
+    assert_response :success
+    amf_parse_response                                         
+    way = amf_result("/1")
+    assert_equal 0, way[0]
+    assert_equal "", way[1]
+    assert_equal id, way[2]
+    assert_equal 3, way[3].length
+    assert_equal 4, way[3][0][2]
+    assert_equal 15, way[3][1][2]
+    assert_equal 6, way[3][2][2]
+    assert_equal 2, way[5]
+    assert_equal 2, way[6]
   end
 
   def test_getway_nonexistent
@@ -51,10 +110,10 @@ class AmfControllerTest < ActionController::TestCase
     assert_response :success
     amf_parse_response
     way = amf_result("/1")
-    assert_equal way[0], -4
-    assert_equal way[1], "way"
-    assert_equal way[2], 0
-    assert way[3].nil? and way[4].nil?
+    assert_equal -4, way[0]
+    assert_equal "way", way[1]
+    assert_equal 0, way[2]
+    assert way[3].nil? and way[4].nil? and way[5].nil? and way[6].nil?
   end
 
   def test_whichways

test/controllers/changeset_controller_test.rb

@@ -3,6 +3,7 @@ require 'changeset_controller'
 
 class ChangesetControllerTest < ActionController::TestCase
   api_fixtures
+  fixtures :changeset_comments, :changesets_subscribers
 
   ##
   # test all routes which lead to this controller
@@ -27,6 +28,14 @@ class ChangesetControllerTest < ActionController::TestCase
       { :path => "/api/0.6/changeset/1", :method => :get },
       { :controller => "changeset", :action => "read", :id => "1" }
     )
+    assert_routing(
+      { :path => "/api/0.6/changeset/1/subscribe", :method => :post },
+      { :controller => "changeset", :action => "subscribe", :id => "1" }
+    )
+    assert_routing(
+      { :path => "/api/0.6/changeset/1/unsubscribe", :method => :post },
+      { :controller => "changeset", :action => "unsubscribe", :id => "1" }
+    )
     assert_routing(
       { :path => "/api/0.6/changeset/1", :method => :put },
       { :controller => "changeset", :action => "update", :id => "1" }
@@ -35,10 +44,26 @@ class ChangesetControllerTest < ActionController::TestCase
       { :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 => "list", :display_name => "name" }
@@ -63,6 +88,10 @@ class ChangesetControllerTest < ActionController::TestCase
       { :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
 
   # -----------------------
@@ -100,6 +129,9 @@ class ChangesetControllerTest < ActionController::TestCase
       # must be number of seconds...
       assert_equal 3600, duration.round, "initial idle timeout should be an hour (#{cs.created_at} -> #{cs.closed_at})"
     end
+
+    # checks if uploader was subscribed
+    assert_equal 1, cs.subscribers.length
   end
 
   def test_create_invalid
@@ -154,6 +186,14 @@ class ChangesetControllerTest < ActionController::TestCase
 
     assert_select "osm[version=#{API_VERSION}][generator=\"OpenStreetMap server\"]", 1
     assert_select "osm>changeset[id=#{changeset_id}]", 1
+    assert_select "osm>changeset>discussion", 0
+
+    get :read, :id => changeset_id, :include_discussion => true
+    assert_response :success, "cannot get first changeset with comments"
+
+    assert_select "osm[version=#{API_VERSION}][generator=\"OpenStreetMap server\"]", 1
+    assert_select "osm>changeset[id=#{changeset_id}]", 1
+    assert_select "osm>changeset>discussion", 1
   end
 
   ##
@@ -1502,11 +1542,11 @@ EOF
     basic_authorization "test@openstreetmap.org", "test"
     get :query, :user => users(:normal_user).id
     assert_response :success, "can't get changesets by user ID"
-    assert_changesets [1,3,6]
+    assert_changesets [1,3,6,8]
 
     get :query, :display_name => users(:normal_user).display_name
     assert_response :success, "can't get changesets by user name"
-    assert_changesets [1,3,6]
+    assert_changesets [1,3,6,8]
 
     # check that the correct error is given when we provide both UID and name
     get :query, :user => users(:normal_user).id, :display_name => users(:normal_user).display_name
@@ -1534,11 +1574,11 @@ EOF
 
     get :query, :closed => 'true'
     assert_response :success, "can't get changesets by closed-ness"
-    assert_changesets [3,5,6,7]
+    assert_changesets [3,5,6,7,8]
 
     get :query, :closed => 'true', :user => users(:normal_user).id
     assert_response :success, "can't get changesets by closed-ness and user"
-    assert_changesets [3,6]
+    assert_changesets [3,6,8]
 
     get :query, :closed => 'true', :user => users(:public_user).id
     assert_response :success, "can't get changesets by closed-ness and user"
@@ -1840,6 +1880,239 @@ EOF
     assert_select "osmChange node[id=17][version=1]", 0
   end
 
+  ##
+  # create comment success
+  def test_create_comment_success
+    basic_authorization(users(:public_user).email, 'test')
+
+    assert_difference('ChangesetComment.count') do
+      post :comment, { :id => changesets(:normal_user_closed_change).id, :text => 'This is a comment' }
+    end
+    assert_response :success
+  end
+
+  ##
+  # create comment fail
+  def test_create_comment_fail
+    # unauthorized
+    post :comment, { :id => changesets(:normal_user_closed_change).id, :text => 'This is a comment' }
+    assert_response :unauthorized
+
+    basic_authorization(users(:public_user).email, 'test')
+
+    # bad changeset id
+    assert_no_difference('ChangesetComment.count') do
+      post :comment, { :id => 999111, :text => 'This is a comment' }
+    end
+    assert_response :not_found
+
+    # not closed changeset
+    assert_no_difference('ChangesetComment.count') do
+      post :comment, { :id => changesets(:normal_user_first_change).id, :text => 'This is a comment' }
+    end
+    assert_response :conflict
+
+    # no text
+    assert_no_difference('ChangesetComment.count') do
+      post :comment, { :id => changesets(:normal_user_closed_change).id }
+    end
+    assert_response :bad_request
+
+    # empty text
+    assert_no_difference('ChangesetComment.count') do
+      post :comment, { :id => changesets(:normal_user_closed_change).id, :text => '' }
+    end
+    assert_response :bad_request    
+  end
+
+  ##
+  # test subscribe success
+  def test_subscribe_success
+    basic_authorization(users(:public_user).email, 'test')
+    changeset = changesets(:normal_user_closed_change)
+
+    assert_difference('changeset.subscribers.count') do
+      post :subscribe, { :id => changeset.id }
+    end
+    assert_response :success
+  end
+
+  ##
+  # test subscribe fail
+  def test_subscribe_fail
+    # unauthorized
+    changeset = changesets(:normal_user_closed_change)
+    assert_no_difference('changeset.subscribers.count') do
+      post :subscribe, { :id => changeset.id }
+    end
+    assert_response :unauthorized
+
+    basic_authorization(users(:public_user).email, 'test')
+
+    # bad changeset id
+    assert_no_difference('changeset.subscribers.count') do
+      post :subscribe, { :id => 999111 }
+    end
+    assert_response :not_found
+
+    # not closed changeset
+    changeset = changesets(:normal_user_first_change)
+    assert_no_difference('changeset.subscribers.count') do
+      post :subscribe, { :id => changeset.id }
+    end
+    assert_response :conflict
+
+    # trying to subscribe when already subscribed
+    changeset = changesets(:normal_user_subscribed_change)
+    assert_no_difference('changeset.subscribers.count') do
+      post :subscribe, { :id => changeset.id }
+    end
+    assert_response :conflict
+  end
+
+  ##
+  # test unsubscribe success
+  def test_unsubscribe_success
+    basic_authorization(users(:public_user).email, 'test')
+    changeset = changesets(:normal_user_subscribed_change)
+
+    assert_difference('changeset.subscribers.count', -1) do
+      post :unsubscribe, { :id => changeset.id }
+    end
+    assert_response :success
+  end
+
+  ##
+  # test unsubscribe fail
+  def test_unsubscribe_fail
+    # unauthorized
+    changeset = changesets(:normal_user_closed_change)
+    assert_no_difference('changeset.subscribers.count') do
+      post :unsubscribe, { :id => changeset.id }
+    end
+    assert_response :unauthorized
+
+    basic_authorization(users(:public_user).email, 'test')
+
+    # bad changeset id
+    assert_no_difference('changeset.subscribers.count', -1) do
+      post :unsubscribe, { :id => 999111 }
+    end
+    assert_response :not_found
+
+    # not closed changeset
+    changeset = changesets(:normal_user_first_change)
+    assert_no_difference('changeset.subscribers.count', -1) do
+      post :unsubscribe, { :id => changeset.id }
+    end
+    assert_response :conflict
+
+    # trying to unsubscribe when not subscribed
+    changeset = changesets(:normal_user_closed_change)
+    assert_no_difference('changeset.subscribers.count') do
+      post :unsubscribe, { :id => changeset.id }
+    end
+    assert_response :not_found
+  end
+
+  ##
+  # test hide comment fail
+  def test_hide_comment_fail
+    # unauthorized
+    comment = changeset_comments(:normal_comment_1)
+    assert('comment.visible') do
+      post :hide_comment, { :id => comment.id }
+      assert_response :unauthorized
+    end
+
+    basic_authorization(users(:public_user).email, 'test')
+
+    # not a moderator
+    assert('comment.visible') do
+      post :hide_comment, { :id => comment.id }
+      assert_response :forbidden
+    end
+
+    basic_authorization(users(:moderator_user).email, 'test')
+
+    # bad comment id
+    post :hide_comment, { :id => 999111 }
+    assert_response :not_found
+  end
+
+  ##
+  # test hide comment succes
+  def test_hide_comment_success
+    comment = changeset_comments(:normal_comment_1)
+
+    basic_authorization(users(:moderator_user).email, 'test')
+
+    assert('!comment.visible') do
+      post :hide_comment, { :id => comment.id }
+    end
+    assert_response :success
+  end
+
+  ##
+  # test unhide comment fail
+  def test_unhide_comment_fail
+    # unauthorized
+    comment = changeset_comments(:normal_comment_1)
+    assert('comment.visible') do
+      post :unhide_comment, { :id => comment.id }
+      assert_response :unauthorized
+    end
+    
+    basic_authorization(users(:public_user).email, 'test')
+
+    # not a moderator
+    assert('comment.visible') do
+      post :unhide_comment, { :id => comment.id }
+      assert_response :forbidden
+    end
+
+    basic_authorization(users(:moderator_user).email, 'test')
+
+    # bad comment id
+    post :unhide_comment, { :id => 999111 }
+    assert_response :not_found
+  end
+
+  ##
+  # test unhide comment succes
+  def test_unhide_comment_success
+    comment = changeset_comments(:normal_comment_1)
+
+    basic_authorization(users(:moderator_user).email, 'test')
+
+    assert('!comment.visible') do
+      post :unhide_comment, { :id => comment.id }
+    end
+    assert_response :success
+  end
+
+  ##
+  # test comments feed
+  def test_comments_feed
+    get :comments_feed, {: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, { :id => changesets(:normal_user_closed_change), :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
+
   #------------------------------------------------------------
   # utility functions
   #------------------------------------------------------------
@@ -1886,5 +2159,4 @@ EOF
     xml.find("//osm/way").first[name] = value.to_s
     return xml
   end
-
 end

test/controllers/site_controller_test.rb

@@ -119,7 +119,7 @@ class SiteControllerTest < ActionController::TestCase
 
     get :permalink, :code => 'wBz3--', :layers => 'T'
     assert_response :redirect
-    assert_redirected_to :controller => :site, :action => :index, :layers => 'T', :anchor => 'map=3/4.8779296875/3.955078125'
+    assert_redirected_to :controller => :site, :action => :index, :anchor => 'map=3/4.8779296875/3.955078125&layers=T'
 
     get :permalink, :code => 'wBz3--', :node => 1
     assert_response :redirect

test/controllers/trace_controller_test.rb

@@ -163,10 +163,10 @@ class TraceControllerTest < ActionController::TestCase
   # Check that the list of changesets is displayed
   def test_list
     get :list
-    check_trace_list Trace.public
+    check_trace_list Trace.visible_to_all
 
     get :list, :tag => "London"
-    check_trace_list Trace.tagged("London").public
+    check_trace_list Trace.tagged("London").visible_to_all
   end
 
   # Check that I can get mine
@@ -188,15 +188,15 @@ class TraceControllerTest < ActionController::TestCase
   def test_list_user
     # Test a user with no traces
     get :list, :display_name => users(:second_public_user).display_name
-    check_trace_list users(:second_public_user).traces.public
+    check_trace_list users(:second_public_user).traces.visible_to_all
 
     # Test a user with some traces - should see only public ones
     get :list, :display_name => users(:public_user).display_name
-    check_trace_list users(:public_user).traces.public
+    check_trace_list users(:public_user).traces.visible_to_all
 
     # Should still see only public ones when authenticated as another user
     get :list, {:display_name => users(:public_user).display_name}, {:user => users(:normal_user).id}
-    check_trace_list users(:public_user).traces.public
+    check_trace_list users(:public_user).traces.visible_to_all
 
     # Should see all traces when authenticated as the target user
     get :list, {:display_name => users(:public_user).display_name}, {:user => users(:public_user).id}
@@ -210,16 +210,16 @@ class TraceControllerTest < ActionController::TestCase
   # Check that the rss loads
   def test_rss
     get :georss, :format => :rss
-    check_trace_feed Trace.public
+    check_trace_feed Trace.visible_to_all
 
     get :georss, :tag => "London", :format => :rss
-    check_trace_feed Trace.tagged("London").public
+    check_trace_feed Trace.tagged("London").visible_to_all
 
     get :georss, :display_name => users(:public_user).display_name, :format => :rss
-    check_trace_feed users(:public_user).traces.public
+    check_trace_feed users(:public_user).traces.visible_to_all
 
     get :georss, :display_name => users(:public_user).display_name, :tag => "Birmingham", :format => :rss
-    check_trace_feed users(:public_user).traces.tagged("Birmingham").public
+    check_trace_feed users(:public_user).traces.tagged("Birmingham").visible_to_all
   end
 
   # Test viewing a trace

test/controllers/user_blocks_controller_test.rb

@@ -223,9 +223,10 @@ class UserBlocksControllerTest < ActionController::TestCase
         :user_block_period => "12",
         :user_block => { :needs_view => false, :reason => "Vandalism" }
     end
-    assert_redirected_to user_block_path(:id => 4)
+    id = UserBlock.order(:id).ids.last
+    assert_redirected_to user_block_path(:id => id)
     assert_equal "Created a block on user #{users(:unblocked_user).display_name}.", flash[:notice]
-    b = UserBlock.find(4)
+    b = UserBlock.find(id)
     assert_in_delta Time.now, b.created_at, 1
     assert_in_delta Time.now, b.updated_at, 1
     assert_in_delta Time.now + 12.hour, b.ends_at, 1

test/controllers/user_controller_test.rb

@@ -222,8 +222,8 @@ class UserControllerTest < ActionController::TestCase
   def test_user_create_success
     user = new_user
 
-    assert_difference('User.count') do
-      assert_difference('ActionMailer::Base.deliveries.size') do
+    assert_difference('User.count', 1) do
+      assert_difference('ActionMailer::Base.deliveries.size', 1) do
         post :save, {}, {:new_user => user}
       end
     end
@@ -303,11 +303,17 @@ class UserControllerTest < ActionController::TestCase
   def test_user_save_referer_params
     user = new_user
 
-    post :save, {}, {:new_user => user,
-                     :referer => '/edit?editor=id#map=1/2/3'}
+    assert_difference('User.count', 1) do
+      assert_difference('ActionMailer::Base.deliveries.size', 1) do
+        post :save, {}, {:new_user => user,
+                         :referer => '/edit?editor=id#map=1/2/3'}
+        end
+    end
 
     assert_equal welcome_path(:editor => 'id', :zoom => 1, :lat => 2, :lon => 3),
                  user.tokens.order("id DESC").first.referer
+
+    ActionMailer::Base.deliveries.clear
   end
 
   def test_user_confirm_expired_token
@@ -370,7 +376,10 @@ class UserControllerTest < ActionController::TestCase
     assert_response :redirect
     assert_redirected_to :action => :login
     assert_match /^Sorry you lost it/, flash[:notice]
-    assert_equal users(:normal_user).email, ActionMailer::Base.deliveries.last.to[0]
+    email = ActionMailer::Base.deliveries.first
+    assert_equal 1, email.to.count
+    assert_equal users(:normal_user).email, email.to.first
+    ActionMailer::Base.deliveries.clear
 
     # Test resetting using an address that matches a different user
     # that has the same address in a different case
@@ -380,7 +389,10 @@ class UserControllerTest < ActionController::TestCase
     assert_response :redirect
     assert_redirected_to :action => :login
     assert_match /^Sorry you lost it/, flash[:notice]
-    assert_equal users(:uppercase_user).email, ActionMailer::Base.deliveries.last.to[0]
+    email = ActionMailer::Base.deliveries.first
+    assert_equal 1, email.to.count
+    assert_equal users(:uppercase_user).email, email.to.first
+    ActionMailer::Base.deliveries.clear
 
     # Test resetting using an address that is a case insensitive match
     # for more than one user but not an exact match for either
@@ -399,7 +411,10 @@ class UserControllerTest < ActionController::TestCase
     assert_response :redirect
     assert_redirected_to :action => :login
     assert_match /^Sorry you lost it/, flash[:notice]
-    assert_equal users(:public_user).email, ActionMailer::Base.deliveries.last.to[0]
+    email = ActionMailer::Base.deliveries.first
+    assert_equal 1, email.to.count
+    assert_equal users(:public_user).email, email.to.first
+    ActionMailer::Base.deliveries.clear
 
     # Test resetting using an address that matches a user that has the
     # same (case insensitively unique) address in a different case
@@ -409,7 +424,10 @@ class UserControllerTest < ActionController::TestCase
     assert_response :redirect
     assert_redirected_to :action => :login
     assert_match /^Sorry you lost it/, flash[:notice]
-    assert_equal users(:public_user).email, ActionMailer::Base.deliveries.last.to[0]
+    email = ActionMailer::Base.deliveries.first
+    assert_equal 1, email.to.count
+    assert_equal users(:public_user).email, email.to.first
+    ActionMailer::Base.deliveries.clear
   end
 
   def test_reset_password
@@ -503,7 +521,9 @@ class UserControllerTest < ActionController::TestCase
 
     # Changing email to one that exists should fail
     user.new_email = users(:public_user).email
-    post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+    assert_no_difference('ActionMailer::Base.deliveries.size') do
+      post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+    end
     assert_response :success
     assert_template :account
     assert_select ".notice", false
@@ -512,7 +532,9 @@ class UserControllerTest < ActionController::TestCase
 
     # Changing email to one that exists should fail, regardless of case
     user.new_email = users(:public_user).email.upcase
-    post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+    assert_no_difference('ActionMailer::Base.deliveries.size') do
+      post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+    end
     assert_response :success
     assert_template :account
     assert_select ".notice", false
@@ -521,12 +543,18 @@ class UserControllerTest < ActionController::TestCase
 
     # Changing email to one that doesn't exist should work
     user.new_email = "new_tester@example.com"
-    post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+    assert_difference('ActionMailer::Base.deliveries.size', 1) do
+      post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+    end
     assert_response :success
     assert_template :account
     assert_select "div#errorExplanation", false
     assert_select ".notice", /^User information updated successfully/
     assert_select "form#accountForm > fieldset > div.form-row > input#user_new_email[value=?]", user.new_email
+    email = ActionMailer::Base.deliveries.first
+    assert_equal 1, email.to.count
+    assert_equal user.new_email, email.to.first
+    ActionMailer::Base.deliveries.clear
   end
   
   # Check that the user account page will display and contains some relevant
@@ -744,13 +772,21 @@ class UserControllerTest < ActionController::TestCase
     assert_nil Friend.where(:user_id => user.id, :friend_user_id => friend.id).first
 
     # When logged in a POST should add the friendship
-    post :make_friend, {:display_name => friend.display_name}, {"user" => user}
+    assert_difference('ActionMailer::Base.deliveries.size', 1) do
+      post :make_friend, {:display_name => friend.display_name}, {"user" => user}
+    end
     assert_redirected_to user_path(:display_name => friend.display_name)
     assert_match /is now your friend/, flash[:notice]
     assert Friend.where(:user_id => user.id, :friend_user_id => friend.id).first
+    email = ActionMailer::Base.deliveries.first
+    assert_equal 1, email.to.count
+    assert_equal friend.email, email.to.first
+    ActionMailer::Base.deliveries.clear
 
     # A second POST should report that the friendship already exists
-    post :make_friend, {:display_name => friend.display_name}, {"user" => user}
+    assert_no_difference('ActionMailer::Base.deliveries.size') do
+      post :make_friend, {:display_name => friend.display_name}, {"user" => user}
+    end
     assert_redirected_to user_path(:display_name => friend.display_name)
     assert_match /You are already friends with/, flash[:warning]
     assert Friend.where(:user_id => user.id, :friend_user_id => friend.id).first

test/controllers/user_preference_controller_test.rb

@@ -92,10 +92,10 @@ class UserPreferenceControllerTest < ActionController::TestCase
       put :update
     end
     assert_response :unauthorized, "should be authenticated"
-    assert_equal "value", UserPreference.find(1, "key").v
-    assert_equal "some_value", UserPreference.find(1, "some_key").v
+    assert_equal "value", UserPreference.find([1, "key"]).v
+    assert_equal "some_value", UserPreference.find([1, "some_key"]).v
     assert_raises ActiveRecord::RecordNotFound do
-      UserPreference.find(1, "new_key")
+      UserPreference.find([1, "new_key"])
     end
 
     # authenticate as a user with preferences
@@ -109,10 +109,10 @@ class UserPreferenceControllerTest < ActionController::TestCase
     assert_response :success
     assert_equal "text/plain", @response.content_type
     assert_equal "", @response.body
-    assert_equal "new_value", UserPreference.find(1, "key").v
-    assert_equal "value", UserPreference.find(1, "new_key").v
+    assert_equal "new_value", UserPreference.find([1, "key"]).v
+    assert_equal "value", UserPreference.find([1, "new_key"]).v
     assert_raises ActiveRecord::RecordNotFound do
-      UserPreference.find(1, "some_key")
+      UserPreference.find([1, "some_key"])
     end
 
     # try a put with duplicate keys
@@ -123,7 +123,7 @@ class UserPreferenceControllerTest < ActionController::TestCase
     assert_response :bad_request
     assert_equal "text/plain", @response.content_type
     assert_equal "Duplicate preferences with key key", @response.body
-    assert_equal "new_value", UserPreference.find(1, "key").v
+    assert_equal "new_value", UserPreference.find([1, "key"]).v
 
     # try a put with invalid content
     assert_no_difference "UserPreference.count" do
@@ -143,7 +143,7 @@ class UserPreferenceControllerTest < ActionController::TestCase
     end
     assert_response :unauthorized, "should be authenticated"
     assert_raises ActiveRecord::RecordNotFound do
-      UserPreference.find(1, "new_key")
+      UserPreference.find([1, "new_key"])
     end
 
     # authenticate as a user with preferences
@@ -157,7 +157,7 @@ class UserPreferenceControllerTest < ActionController::TestCase
     assert_response :success
     assert_equal "text/plain", @response.content_type
     assert_equal "", @response.body
-    assert_equal "new_value", UserPreference.find(1, "new_key").v
+    assert_equal "new_value", UserPreference.find([1, "new_key"]).v
 
     # try changing the value of a preference
     assert_no_difference "UserPreference.count" do
@@ -167,7 +167,7 @@ class UserPreferenceControllerTest < ActionController::TestCase
     assert_response :success
     assert_equal "text/plain", @response.content_type
     assert_equal "", @response.body
-    assert_equal "newer_value", UserPreference.find(1, "new_key").v
+    assert_equal "newer_value", UserPreference.find([1, "new_key"]).v
   end
 
   ##
@@ -178,7 +178,7 @@ class UserPreferenceControllerTest < ActionController::TestCase
       delete :delete_one, :preference_key => "key"
     end
     assert_response :unauthorized, "should be authenticated"
-    assert_equal "value", UserPreference.find(1, "key").v
+    assert_equal "value", UserPreference.find([1, "key"]).v
 
     # authenticate as a user with preferences
     basic_authorization("test@openstreetmap.org", "test")
@@ -191,7 +191,7 @@ class UserPreferenceControllerTest < ActionController::TestCase
     assert_equal "text/plain", @response.content_type
     assert_equal "", @response.body
     assert_raises ActiveRecord::RecordNotFound do
-      UserPreference.find(1, "key")
+      UserPreference.find([1, "key"])
     end
 
     # try the delete again for the same key
@@ -200,7 +200,7 @@ class UserPreferenceControllerTest < ActionController::TestCase
     end
     assert_response :not_found
     assert_raises ActiveRecord::RecordNotFound do
-      UserPreference.find(1, "key")
+      UserPreference.find([1, "key"])
     end
   end
 end

test/fixtures/changeset_comments.yml

@@ -0,0 +1,31 @@
+normal_comment_1:
+  id: 1
+  changeset_id: 3
+  created_at: 2007-01-01 00:00:00
+  author_id: 1
+  body: 'A comment from a logged-in user'
+  visible: true
+
+normal_comment_2:
+  id: 2
+  changeset_id: 3
+  created_at: 2007-02-01 00:00:00
+  author_id: 4
+  body: 'A comment from another logged-in user'
+  visible: true
+
+normal_comment_3:
+  id: 4
+  changeset_id: 3
+  created_at: 2007-02-01 00:00:00
+  author_id: 4
+  body: 'A comment from another logged-in user'
+  visible: true
+
+hidden_comment:
+  id: 3
+  changeset_id: 3
+  created_at: 2007-02-01 00:00:00
+  author_id: 4
+  body: 'A non-visible comment'
+  visible: false

test/fixtures/changesets.yml

@@ -69,3 +69,9 @@ too_many_elements_changeset:
   max_lat: <%= 4 * SCALE %>
   num_changes: <%= Changeset::MAX_ELEMENTS + 1 %>
 
+normal_user_subscribed_change:
+  id: 8
+  user_id: 1
+  created_at: "2007-01-01 00:00:00"
+  closed_at: "2007-01-02 00:00:00"
+  num_changes: 0

test/fixtures/changesets_subscribers.yml

@@ -0,0 +1,3 @@
+t1:
+  changeset_id: 8
+  subscriber_id: 2

test/fixtures/note_comments.yml

@@ -2,6 +2,7 @@ t1:
   id: 1
   note_id: 1
   visible: true
+  event: opened
   created_at: 2007-01-01 00:00:00
   author_ip: '192.168.1.1'
   body: 'This is the initial description of the note 1'
@@ -10,6 +11,7 @@ t2:
   id: 2
   note_id: 2
   visible: true
+  event: opened
   created_at: 2007-01-01 00:00:00
   author_ip: '192.168.1.1'
   body: 'This is the initial description of the note 2'
@@ -18,6 +20,7 @@ t3:
   id: 3
   note_id: 2
   visible: true
+  event: opened
   created_at: 2007-02-01 00:00:00
   author_ip: '192.168.1.1'
   body: 'This is an additional comment for note 2'
@@ -26,6 +29,7 @@ t4:
   id: 4
   note_id: 3
   visible: true
+  event: opened
   created_at: 2007-01-01 00:00:00
   author_ip: '192.168.1.1'
   body: 'This is the initial comment for note 3'
@@ -34,6 +38,7 @@ t5:
   id: 5
   note_id: 4
   visible: true
+  event: opened
   created_at: 2007-01-01 00:00:00
   author_ip: '192.168.1.1'
   body: 'Spam for note 4'
@@ -42,6 +47,7 @@ t6:
   id: 6
   note_id: 5
   visible: true
+  event: opened
   created_at: 2007-01-01 00:00:00
   author_ip: '192.168.1.1'
   body: 'Valid comment for note 5'
@@ -50,6 +56,7 @@ t7:
   id: 7
   note_id: 5
   visible: false
+  event: commented
   created_at: 2007-02-01 00:00:00
   author_ip: '192.168.1.1'
   body: 'Spam for note 5'
@@ -58,6 +65,7 @@ t8:
   id: 8
   note_id: 5
   visible: true
+  event: commented
   created_at: 2007-02-01 00:00:00
   author_ip: '192.168.1.1'
   body: 'Another valid comment for note 5'
@@ -66,8 +74,8 @@ t9:
   id: 9
   note_id: 6
   visible: true
-  created_at: 2007-01-01 00:00:00
   event: opened
+  created_at: 2007-01-01 00:00:00
   author_id: 1
   body: 'This is a note with from a logged-in user'
 
@@ -75,8 +83,8 @@ t10:
   id: 10
   note_id: 6
   visible: true
-  created_at: 2007-02-01 00:00:00
   event: commented
+  created_at: 2007-02-01 00:00:00
   author_id: 4
   body: 'A comment from another logged-in user'
 

test/helpers/application_helper_test.rb

@@ -7,6 +7,10 @@ class ApplicationHelperTest < ActionView::TestCase
     I18n.locale = "en"
   end
 
+  def setup
+    I18n.locale = "en"
+  end
+
   def test_linkify
     %w(http://example.com/test ftp://example.com/test https://example.com/test).each do |link|
       text = "Test #{link} is made into a link"

test/helpers/browse_helper_test.rb

@@ -12,6 +12,10 @@ class BrowseHelperTest < ActionView::TestCase
     I18n.locale = "en"
   end
 
+  def teardown
+    I18n.locale = "en"
+  end
+
   def test_printable_name
     assert_equal "17", printable_name(current_nodes(:redacted_node))
     assert_equal "<bdi>Test Node</bdi> (<bdi>18</bdi>)", printable_name(current_nodes(:node_with_name))
@@ -132,6 +136,33 @@ class BrowseHelperTest < ActionView::TestCase
     assert_equal "http://wiki.openstreetmap.org/wiki/Tag:highway=primary?uselang=tr", link
   end
 
+  def test_wikidata_link
+    link = wikidata_link("foo", "Test")
+    assert_nil link
+
+    link = wikidata_link("wikidata", "http://www.wikidata.org/wiki/Q1")
+    assert_nil link
+
+    link = wikidata_link("wikidata", "en:Q1")
+    assert_nil link
+
+    link = wikidata_link("wikidata", "1")
+    assert_nil link
+
+    link = wikidata_link("wikidata", "Q0123")
+    assert_nil link
+
+    link = wikidata_link("wikidata", "Q42")
+    assert_equal "//www.wikidata.org/wiki/Q42?uselang=en", link[:url]
+    assert_equal "Q42", link[:title]
+
+    I18n.locale = "zh-CN"
+
+    link = wikidata_link("wikidata", "Q1234")
+    assert_equal "//www.wikidata.org/wiki/Q1234?uselang=zh-CN", link[:url]
+    assert_equal "Q1234", link[:title]
+  end
+
   def test_wikipedia_link
     link = wikipedia_link("wikipedia", "http://en.wikipedia.org/wiki/Full%20URL")
     assert_nil link

test/helpers/note_helper_test.rb

@@ -7,7 +7,7 @@ class NoteHelperTest < ActionView::TestCase
   fixtures :users
 
   def test_note_event
-    date = Time.new(2014, 3, 5, 21, 37, 45)
+    date = Time.new(2014, 3, 5, 21, 37, 45, "+00:00")
 
     assert_match /^Created by anonymous <abbr title='Wed, 05 Mar 2014 21:37:45 \+0000'><span title=" 5 March 2014 at 21:37">.*<\/span> ago<\/abbr>$/, note_event("open", date, nil)
     assert_match /^Resolved by <a href="\/user\/test2">test2<\/a> <abbr title='Wed, 05 Mar 2014 21:37:45 \+0000'><span title=" 5 March 2014 at 21:37">.*<\/span> ago<\/abbr>$/, note_event("closed", date, users(:public_user))

test/helpers/title_helper_test.rb

@@ -9,5 +9,9 @@ class TitleHelperTest < ActionView::TestCase
     set_title("Test Title")
     assert_equal "OpenStreetMap | Test Title", response.header["X-Page-Title"]
     assert_equal "Test Title", @title
+
+    set_title("Test & Title")
+    assert_equal "OpenStreetMap | Test & Title", response.header["X-Page-Title"]
+    assert_equal "Test & Title", @title
   end
 end

test/integration/short_links_test.rb

@@ -25,9 +25,9 @@ class ShortLinksTest < ActionDispatch::IntegrationTest
 
     # test with layers and a marker
     get '/go/' + short_link + "?m&layers=B000FTF"
-    assert_redirected_to :controller => 'site', :action => 'index', :mlat => lat.to_s, :mlon => lon.to_s, :layers => "B000FTF", :anchor => anchor
+    assert_redirected_to :controller => 'site', :action => 'index', :mlat => lat.to_s, :mlon => lon.to_s, :anchor => "#{anchor}&layers=B000FTF"
     get '/go/' + short_link + "?layers=B000FTF&m"
-    assert_redirected_to :controller => 'site', :action => 'index', :mlat => lat.to_s, :mlon => lon.to_s, :layers => "B000FTF", :anchor => anchor
+    assert_redirected_to :controller => 'site', :action => 'index', :mlat => lat.to_s, :mlon => lon.to_s, :anchor => "#{anchor}&layers=B000FTF"
 
     # test with some random query parameters we haven't even implemented yet
     get '/go/' + short_link + "?foobar=yes"

test/integration/user_changeset_comments_test.rb

@@ -0,0 +1,49 @@
+require 'test_helper'
+
+class UserChangesetCommentsTest < ActionDispatch::IntegrationTest
+  fixtures :users, :changesets, :changeset_comments
+
+  # Test 'log in to comment' message for nonlogged in user
+  def test_log_in_message
+    get "/changeset/#{changesets(:normal_user_closed_change).id}"
+    assert_response :success
+    
+    assert_select "div#content" do
+      assert_select "div#sidebar" do
+        assert_select "div#sidebar_content" do
+          assert_select "div.browse-section" do
+            assert_select "div.notice.hide_if_logged_in"
+          end
+        end
+      end
+    end
+  end
+
+  # Test if the form is shown
+  def test_displaying_form
+    get_via_redirect '/login'
+    # We should now be at the login page
+    assert_response :success
+    assert_template 'user/login'
+    # We can now login
+    post  '/login', {'username' => "test@openstreetmap.org", 'password' => "test"}
+    assert_response :redirect
+
+    get "/changeset/#{changesets(:normal_user_closed_change).id}"
+    
+    assert_response :success
+    assert_template 'browse/changeset'
+
+    assert_select "div#content" do
+      assert_select "div#sidebar" do
+        assert_select "div#sidebar_content" do
+          assert_select "div.browse-section" do
+            assert_select "form[action='#']" do
+              assert_select "textarea[name=text]"
+            end
+          end
+        end
+      end
+    end
+  end
+end

test/models/changeset_comment_test.rb

@@ -0,0 +1,41 @@
+require 'test_helper'
+
+class ChangesetCommentTest < ActiveSupport::TestCase
+  fixtures :changesets, :changeset_comments
+
+  def test_changeset_comment_count
+    assert_equal 4, ChangesetComment.count
+  end
+
+  # validations
+  def test_does_not_accept_invalid_author
+    comment = changeset_comments(:normal_comment_1)
+    
+    comment.author = nil
+    assert !comment.valid?
+
+    comment.author_id = 999111
+    assert !comment.valid?
+  end
+
+  def test_does_not_accept_invalid_changeset
+    comment = changeset_comments(:normal_comment_1)
+
+    comment.changeset = nil
+    assert !comment.valid?
+
+    comment.changeset_id = 999111
+    assert !comment.valid?
+  end
+
+  def test_does_not_accept_empty_visible
+    comment = changeset_comments(:normal_comment_1)
+
+    comment.visible = nil
+    assert !comment.valid?
+  end
+
+  def test_comments_of_changeset_count
+    assert_equal 3, Changeset.find(changesets(:normal_user_closed_change)).comments.count
+  end
+end

test/models/changeset_test.rb

@@ -4,7 +4,7 @@ class ChangesetTest < ActiveSupport::TestCase
   api_fixtures
 
   def test_changeset_count
-    assert_equal 7, Changeset.count
+    assert_equal 8, Changeset.count
   end
 
   def test_from_xml_no_text

test/models/node_test.rb

@@ -331,4 +331,31 @@ class NodeTest < ActiveSupport::TestCase
     assert_equal "added in node version 3", tags["testing"]
     assert_equal "modified in node version 4", tags["testing two"]
   end
+
+  def test_containing_relation_members
+    node = current_nodes(:node_used_by_relationship)
+    crm = Node.find(node.id).containing_relation_members.order(:relation_id)
+#    assert_equal 3, crm.size
+    assert_equal 1, crm.first.relation_id
+    assert_equal "Node", crm.first.member_type
+    assert_equal node.id, crm.first.member_id
+    assert_equal 1, crm.first.relation.id
+    assert_equal 2, crm.second.relation_id
+    assert_equal "Node", crm.second.member_type
+    assert_equal node.id, crm.second.member_id
+    assert_equal 2, crm.second.relation.id
+    assert_equal 3, crm.third.relation_id
+    assert_equal "Node", crm.third.member_type
+    assert_equal node.id, crm.third.member_id
+    assert_equal 3, crm.third.relation.id
+  end
+
+  def test_containing_relations
+    node = current_nodes(:node_used_by_relationship)
+    cr = Node.find(node.id).containing_relations.order(:id)
+    assert_equal 3, cr.size
+    assert_equal 1, cr.first.id
+    assert_equal 2, cr.second.id
+    assert_equal 3, cr.third.id
+  end
 end

test/models/note_comment_test.rb

@@ -0,0 +1,42 @@
+# -*- coding: utf-8 -*-
+require 'test_helper'
+
+class NoteCommentTest < ActiveSupport::TestCase
+  fixtures :users, :notes, :note_comments
+
+  def test_event_valid
+    ok = [ "opened", "closed", "reopened", "commented", "hidden" ]
+    bad = [ "expropriated", "fubared" ]
+
+    ok.each do |event|
+      note_comment = note_comments(:t1)
+      note_comment.event = event
+      assert note_comment.valid?, "#{event} is invalid, when it should be"
+    end
+
+    bad.each do |event|
+      note_comment = note_comments(:t1)
+      note_comment.event = event
+      assert !note_comment.valid?, "#{event} is valid when it shouldn't be"
+    end
+  end
+
+  def test_body_valid
+    ok = [ "Name", "vergrößern", "foo\x0abar",
+           "ルシステムにも対応します", "輕觸搖晃的遊戲", ]
+    bad = [ "foo\x00bar", "foo\x08bar", "foo\x1fbar", "foo\x7fbar",
+            "foo\ufffebar", "foo\uffffbar" ]
+
+    ok.each do |body|
+      note_comment = note_comments(:t1)
+      note_comment.body = body
+      assert note_comment.valid?, "#{body} is invalid, when it should be"
+    end
+
+    bad.each do |body|
+      note_comment = note_comments(:t1)
+      note_comment.body = body
+      assert !note_comment.valid?, "#{body} is valid when it shouldn't be"
+    end
+  end
+end

test/models/note_test.rb

@@ -0,0 +1,62 @@
+# -*- coding: utf-8 -*-
+require 'test_helper'
+
+class NoteTest < ActiveSupport::TestCase
+  fixtures :users, :notes, :note_comments
+
+  def test_status_valid
+    ok = [ "open", "closed", "hidden" ]
+    bad = [ "expropriated", "fubared" ]
+
+    ok.each do |status|
+      note = notes(:open_note)
+      note.status = status
+      assert note.valid?, "#{status} is invalid, when it should be"
+    end
+
+    bad.each do |status|
+      note = notes(:open_note)
+      note.status = status
+      assert !note.valid?, "#{status} is valid when it shouldn't be"
+    end
+  end
+
+  def test_close
+    note = notes(:open_note)
+    assert_equal "open", note.status
+    assert_nil note.closed_at
+    note.close
+    assert_equal "closed", note.status
+    assert_not_nil note.closed_at
+  end
+
+  def test_close
+    note = notes(:closed_note_with_comment)
+    assert_equal "closed", note.status
+    assert_not_nil note.closed_at
+    note.reopen
+    assert_equal "open", note.status
+    assert_nil note.closed_at
+  end
+
+  def test_visible?
+    assert_equal true, notes(:open_note).visible?
+    assert_equal true, notes(:note_with_hidden_comment).visible?
+    assert_equal false, notes(:hidden_note_with_comment).visible?
+  end
+
+  def test_closed?
+    assert_equal true, notes(:closed_note_with_comment).closed?
+    assert_equal false, notes(:open_note).closed?
+  end
+
+  def test_author
+    assert_nil notes(:open_note).author
+    assert_equal users(:normal_user), notes(:note_with_comments_by_users).author
+  end
+
+  def test_author_ip
+    assert_equal IPAddr.new("192.168.1.1"), notes(:open_note).author_ip
+    assert_nil notes(:note_with_comments_by_users).author_ip
+  end
+end

test/models/relation_test.rb

@@ -144,4 +144,21 @@ class RelationTest < ActiveSupport::TestCase
     assert_equal "added in relation version 3", tags["testing"]
     assert_equal "modified in relation version 4", tags["testing two"]
   end
+
+  def test_containing_relation_members
+    relation = current_relations(:used_relation)
+    crm = Relation.find(relation.id).containing_relation_members.order(:relation_id)
+#    assert_equal 1, crm.size
+    assert_equal 1, crm.first.relation_id
+    assert_equal "Relation", crm.first.member_type
+    assert_equal relation.id, crm.first.member_id
+    assert_equal 1, crm.first.relation.id
+  end
+
+  def test_containing_relations
+    relation = current_relations(:used_relation)
+    cr = Relation.find(relation.id).containing_relations.order(:id)
+    assert_equal 1, cr.size
+    assert_equal 1, cr.first.id
+  end
 end

test/models/trace_test.rb

@@ -27,8 +27,8 @@ class TraceTest < ActiveSupport::TestCase
     check_query(Trace.visible_to(3), [:public_trace_file, :identifiable_trace_file])
   end
 
-  def test_public
-    check_query(Trace.public, [:public_trace_file, :identifiable_trace_file, :deleted_trace_file])
+  def test_visible_to_all
+    check_query(Trace.visible_to_all, [:public_trace_file, :identifiable_trace_file, :deleted_trace_file])
   end
 
   def test_tagged

test/models/user_test.rb

@@ -86,7 +86,10 @@ class UserTest < ActiveSupport::TestCase
     # These need to be 3 chars in length, otherwise the length test above
     # should be used.
     bad = [ "<hr/>", "test@example.com", "s/f", "aa/", "aa;", "aa.",
-            "aa,", "aa?", "/;.,?", "も対応します/", "#ping" ]
+            "aa,", "aa?", "/;.,?", "も対応します/", "#ping",
+            "foo\x1fbar", "foo\x7fbar", "foo\ufffebar", "foo\uffffbar",
+            "new", "terms", "save", "confirm", "confirm-email",
+            "go_public", "reset-password", "forgot-password", "suspended" ]
     ok.each do |display_name|
       user = users(:normal_user)
       user.display_name = display_name
@@ -97,7 +100,6 @@ class UserTest < ActiveSupport::TestCase
       user = users(:normal_user)
       user.display_name = display_name
       assert !user.valid?, "#{display_name} is valid when it shouldn't be"
-      assert user.errors[:display_name].include?("is invalid")
     end
   end
   
@@ -179,10 +181,10 @@ class UserTest < ActiveSupport::TestCase
     end
   end
 
-  def test_public
-    assert_equal 16, User.public.count
+  def test_identifiable
+    assert_equal 16, User.identifiable.count
     assert_raise ActiveRecord::RecordNotFound do
-      User.public.find(users(:normal_user).id)
+      User.identifiable.find(users(:normal_user).id)
     end
   end
 

test/models/way_test.rb

@@ -180,4 +180,21 @@ class WayTest < ActiveSupport::TestCase
     assert_equal "added in way version 3", tags["testing"]
     assert_equal "modified in way version 4", tags["testing two"]
   end
+
+  def test_containing_relation_members
+    way = current_ways(:used_way)
+    crm = Way.find(way.id).containing_relation_members.order(:relation_id)
+#    assert_equal 1, crm.size
+    assert_equal 1, crm.first.relation_id
+    assert_equal "Way", crm.first.member_type
+    assert_equal way.id, crm.first.member_id
+    assert_equal 1, crm.first.relation.id
+  end
+
+  def test_containing_relations
+    way = current_ways(:used_way)
+    cr = Way.find(way.id).containing_relations.order(:id)
+    assert_equal 1, cr.size
+    assert_equal 1, cr.first.id
+  end
 end