Tests!

.rubocop_todo.yml

@@ -28,7 +28,7 @@ Lint/ParenthesesAsGroupedExpression:
 
 # Offense count: 542
 Metrics/AbcSize:
-  Max: 194
+  Max: 270
 
 # Offense count: 12
 Metrics/BlockNesting:
@@ -37,7 +37,7 @@ Metrics/BlockNesting:
 # Offense count: 60
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 1543
+  Max: 1552
 
 # Offense count: 68
 Metrics/CyclomaticComplexity:

app/controllers/application_controller.rb

@@ -389,12 +389,6 @@ class ApplicationController < ActionController::Base
     render :action => "timeout"
   end
 
-  ##
-  # is the requestor logged in?
-  def logged_in?
-    !@user.nil?
-  end
-
   ##
   # ensure that there is a "this_user" instance variable
   def lookup_this_user

test/controllers/changeset_controller_test.rb

@@ -1891,15 +1891,6 @@ EOF
     end
     assert_response :success
 
-    assert_difference "ChangesetComment.count", 1 do
-      assert_no_difference "ActionMailer::Base.deliveries.size" do
-        post :comment, :id => changesets(:normal_user_subscribed_change).id, :text => "This is a comment"
-      end
-    end
-    assert_response :success
-
-    basic_authorization(users(:second_public_user).email, "test")
-
     assert_difference "ChangesetComment.count", 1 do
       assert_difference "ActionMailer::Base.deliveries.size", 1 do
         post :comment, :id => changesets(:normal_user_subscribed_change).id, :text => "This is a comment"
@@ -1909,6 +1900,27 @@ EOF
 
     email = ActionMailer::Base.deliveries.first
     assert_equal 1, email.to.length
+    assert_equal "[OpenStreetMap] test2 has commented on one of your changesets", email.subject
+    assert_equal "test@openstreetmap.org", email.to.first
+
+    ActionMailer::Base.deliveries.clear
+
+    basic_authorization(users(:second_public_user).email, "test")
+
+    assert_difference "ChangesetComment.count", 1 do
+      assert_difference "ActionMailer::Base.deliveries.size", 2 do
+        post :comment, :id => changesets(:normal_user_subscribed_change).id, :text => "This is a comment"
+      end
+    end
+    assert_response :success
+
+    email = ActionMailer::Base.deliveries.first
+    assert_equal 1, email.to.length
+    assert_equal "[OpenStreetMap] pulibc_test2 has commented on one of your changesets", email.subject
+    assert_equal "test@openstreetmap.org", email.to.first
+
+    email = ActionMailer::Base.deliveries.second
+    assert_equal 1, email.to.length
     assert_equal "[OpenStreetMap] pulibc_test2 has commented on a changeset you are interested in", email.subject
     assert_equal "test@example.com", email.to.first
 

test/controllers/diary_entry_controller_test.rb

@@ -147,6 +147,32 @@ class DiaryEntryControllerTest < ActionController::TestCase
     assert_equal new_language_code, entry.language_code
   end
 
+  def test_new_spammy
+    # Generate some spammy content
+    spammy_title = "Spam Spam Spam Spam Spam"
+    spammy_body = 1.upto(50).map { |n| "http://example.com/spam#{n}" }.join(" ")
+
+    # Try creating a spammy diary entry
+    assert_difference "DiaryEntry.count", 1 do
+      post :new, { :commit => "save",
+                   :diary_entry => { :title => spammy_title, :body => spammy_body, :language_code => "en" } },
+           { :user => users(:normal_user).id }
+    end
+    assert_response :redirect
+    assert_redirected_to :action => :list, :display_name => users(:normal_user).display_name
+    entry = DiaryEntry.order(:id).last
+    assert_equal users(:normal_user).id, entry.user_id
+    assert_equal spammy_title, entry.title
+    assert_equal spammy_body, entry.body
+    assert_equal "en", entry.language_code
+    assert_equal "suspended", User.find(users(:normal_user).id).status
+
+    # Follow the redirect
+    get :list, { :display_name => users(:normal_user).display_name }, { :user => users(:normal_user).id }
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :suspended
+  end
+
   def test_edit
     entry = diary_entries(:normal_user_entry_1)
 
@@ -287,7 +313,7 @@ class DiaryEntryControllerTest < ActionController::TestCase
     assert_match /New comment/, email.text_part.decoded
     assert_match /New comment/, email.html_part.decoded
     ActionMailer::Base.deliveries.clear
-    comment = DiaryComment.find(5)
+    comment = DiaryComment.order(:id).last
     assert_equal entry.id, comment.diary_entry_id
     assert_equal users(:public_user).id, comment.user_id
     assert_equal "New comment", comment.body
@@ -296,13 +322,51 @@ class DiaryEntryControllerTest < ActionController::TestCase
     get :view, :display_name => entry.user.display_name, :id => entry.id
     assert_response :success
     assert_select ".diary-comment", :count => 1 do
-      assert_select "#comment5", :count => 1 do
+      assert_select "#comment#{comment.id}", :count => 1 do
         assert_select "a[href='/user/#{users(:public_user).display_name}']", :text => users(:public_user).display_name, :count => 1
       end
       assert_select ".richtext", :text => /New comment/, :count => 1
     end
   end
 
+  def test_comment_spammy
+    # Find the entry to comment on
+    entry = diary_entries(:normal_user_entry_1)
+
+    # Generate some spammy content
+    spammy_text = 1.upto(50).map { |n| "http://example.com/spam#{n}" }.join(" ")
+
+    # Try creating a spammy comment
+    assert_difference "ActionMailer::Base.deliveries.size", 1 do
+      assert_difference "DiaryComment.count", 1 do
+        post :comment, { :display_name => entry.user.display_name, :id => entry.id, :diary_comment => { :body => spammy_text } }, { :user => users(:public_user).id }
+      end
+    end
+    assert_response :redirect
+    assert_redirected_to :action => :view, :display_name => entry.user.display_name, :id => entry.id
+    email = ActionMailer::Base.deliveries.first
+    assert_equal [users(:normal_user).email], email.to
+    assert_equal "[OpenStreetMap] #{users(:public_user).display_name} commented on your diary entry", email.subject
+    assert_match %r{http://example.com/spam}, email.text_part.decoded
+    assert_match %r{http://example.com/spam}, email.html_part.decoded
+    ActionMailer::Base.deliveries.clear
+    comment = DiaryComment.order(:id).last
+    assert_equal entry.id, comment.diary_entry_id
+    assert_equal users(:public_user).id, comment.user_id
+    assert_equal spammy_text, comment.body
+    assert_equal "suspended", User.find(users(:public_user).id).status
+
+    # Follow the redirect
+    get :list, { :display_name => users(:normal_user).display_name }, { :user => users(:public_user).id }
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :suspended
+
+    # Now view the diary entry, and check the new comment is not present
+    get :view, :display_name => entry.user.display_name, :id => entry.id
+    assert_response :success
+    assert_select ".diary-comment", :count => 0
+  end
+
   def test_list_all
     # Try a list of all diary entries
     get :list

test/controllers/notes_controller_test.rb

@@ -281,6 +281,48 @@ class NotesControllerTest < ActionController::TestCase
     assert_nil js["properties"]["comments"].last["user"]
 
     ActionMailer::Base.deliveries.clear
+
+    basic_authorization(users(:public_user).email, "test")
+
+    assert_difference "NoteComment.count", 1 do
+      assert_difference "ActionMailer::Base.deliveries.size", 2 do
+        post :comment, :id => notes(:note_with_comments_by_users).id, :text => "This is an additional comment", :format => "json"
+      end
+    end
+    assert_response :success
+    js = ActiveSupport::JSON.decode(@response.body)
+    assert_not_nil js
+    assert_equal "Feature", js["type"]
+    assert_equal notes(:note_with_comments_by_users).id, js["properties"]["id"]
+    assert_equal "open", js["properties"]["status"]
+    assert_equal 4, js["properties"]["comments"].count
+    assert_equal "commented", js["properties"]["comments"].last["action"]
+    assert_equal "This is an additional comment", js["properties"]["comments"].last["text"]
+    assert_equal "test2", js["properties"]["comments"].last["user"]
+
+    email = ActionMailer::Base.deliveries.first
+    assert_equal 1, email.to.length
+    assert_equal "[OpenStreetMap] test2 has commented on one of your notes", email.subject
+    assert_equal "test@openstreetmap.org", email.to.first
+
+    email = ActionMailer::Base.deliveries.second
+    assert_equal 1, email.to.length
+    assert_equal "[OpenStreetMap] test2 has commented on a note you are interested in", email.subject
+    assert_equal "public@OpenStreetMap.org", email.to.first
+
+    get :show, :id => notes(:note_with_comments_by_users).id, :format => "json"
+    assert_response :success
+    js = ActiveSupport::JSON.decode(@response.body)
+    assert_not_nil js
+    assert_equal "Feature", js["type"]
+    assert_equal notes(:note_with_comments_by_users).id, js["properties"]["id"]
+    assert_equal "open", js["properties"]["status"]
+    assert_equal 4, js["properties"]["comments"].count
+    assert_equal "commented", js["properties"]["comments"].last["action"]
+    assert_equal "This is an additional comment", js["properties"]["comments"].last["text"]
+    assert_equal "test2", js["properties"]["comments"].last["user"]
+
+    ActionMailer::Base.deliveries.clear
   end
 
   def test_comment_fail

test/controllers/site_controller_test.rb

@@ -199,6 +199,28 @@ class SiteControllerTest < ActionController::TestCase
     assert_template "index"
   end
 
+  # Test the right editor gets used when the URL has an override
+  def test_edit_with_override
+    get :edit, { :editor => "id" }, { :user => users(:public_user).id }
+    assert_response :success
+    assert_template "edit"
+    assert_template :partial => "_id", :count => 1
+
+    get :edit, { :editor => "potlatch2" }, { :user => users(:public_user).id }
+    assert_response :success
+    assert_template "edit"
+    assert_template :partial => "_potlatch2", :count => 1
+
+    get :edit, { :editor => "potlatch" }, { :user => users(:public_user).id }
+    assert_response :success
+    assert_template "edit"
+    assert_template :partial => "_potlatch", :count => 1
+
+    get :edit, { :editor => "remote" }, { :user => users(:public_user).id }
+    assert_response :success
+    assert_template "index"
+  end
+
   # Test editing a specific node
   def test_edit_with_node
     user = users(:public_user)

test/fixtures/changesets_subscribers.yml

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

test/fixtures/users.yml

@@ -63,7 +63,6 @@ second_public_user:
   home_zoom: 12
   terms_agreed: "2010-01-01 11:22:33"
   terms_seen: true
-  languages: en
 
 moderator_user:
   id: 5

test/integration/locale_test.rb

@@ -0,0 +1,37 @@
+require "test_helper"
+
+class LocaleTest < ActionDispatch::IntegrationTest
+  fixtures :users
+
+  def test_defaulting
+    user = users(:second_public_user)
+
+    post_via_redirect "/login", :username => user.email, :password => "test"
+
+    get "/diary/new", {}
+    assert_equal [], User.find(user.id).languages
+    assert_select "html[lang=?]", "en"
+
+    get "/diary/new", {},  { "HTTP_ACCEPT_LANGUAGE" => "fr, en" }
+    assert_equal %w(fr en), User.find(user.id).languages
+    assert_select "html[lang=?]", "fr"
+  end
+
+  def test_override
+    user = users(:german_user)
+
+    get "/diary"
+    assert_select "html[lang=?]", "en"
+
+    get "/diary", :locale => "es"
+    assert_select "html[lang=?]", "es"
+
+    post_via_redirect "/login", :username => user.email, :password => "test"
+
+    get "/diary"
+    assert_select "html[lang=?]", "de"
+
+    get "/diary", :locale => "fr"
+    assert_select "html[lang=?]", "fr"
+  end
+end

test/integration/oauth_test.rb

@@ -49,6 +49,9 @@ class OAuthTest < ActionDispatch::IntegrationTest
     signed_get "/api/0.6/user/preferences", :consumer => client, :token => token
     assert_response :success
 
+    signed_get "/api/0.6/gpx/2", :consumer => client, :token => token
+    assert_response :forbidden
+
     post "/oauth/revoke", :token => token.token
     assert_redirected_to oauth_clients_url(token.user.display_name)
     token = OauthToken.find_by_token(token.token)
@@ -94,6 +97,9 @@ class OAuthTest < ActionDispatch::IntegrationTest
     signed_get "/api/0.6/gpx/2", :consumer => client, :token => token
     assert_response :success
 
+    signed_get "/api/0.6/user/details", :consumer => client, :token => token
+    assert_response :forbidden
+
     post "/oauth/revoke", :token => token.token
     assert_redirected_to oauth_clients_url(token.user.display_name)
     token = OauthToken.find_by_token(token.token)
@@ -146,6 +152,9 @@ class OAuthTest < ActionDispatch::IntegrationTest
     signed_get "/api/0.6/user/preferences", :consumer => client, :token => token
     assert_response :success
 
+    signed_get "/api/0.6/gpx/2", :consumer => client, :token => token
+    assert_response :forbidden
+
     post "/oauth/revoke", :token => token.token
     assert_redirected_to oauth_clients_url(token.user.display_name)
     token = OauthToken.find_by_token(token.token)
@@ -204,6 +213,9 @@ class OAuthTest < ActionDispatch::IntegrationTest
     signed_get "/api/0.6/user/preferences", :consumer => client, :token => token
     assert_response :success
 
+    signed_get "/api/0.6/gpx/2", :consumer => client, :token => token
+    assert_response :forbidden
+
     post "/oauth/revoke", :token => token.token
     assert_redirected_to oauth_clients_url(token.user.display_name)
     token = OauthToken.find_by_token(token.token)
@@ -255,6 +267,9 @@ class OAuthTest < ActionDispatch::IntegrationTest
     signed_get "/api/0.6/gpx/2", :consumer => client, :token => token
     assert_response :success
 
+    signed_get "/api/0.6/user/details", :consumer => client, :token => token
+    assert_response :forbidden
+
     post "/oauth/revoke", :token => token.token
     assert_redirected_to oauth_clients_url(token.user.display_name)
     token = OauthToken.find_by_token(token.token)
@@ -315,6 +330,9 @@ class OAuthTest < ActionDispatch::IntegrationTest
     signed_get "/api/0.6/user/preferences", :consumer => client, :token => token
     assert_response :success
 
+    signed_get "/api/0.6/gpx/2", :consumer => client, :token => token
+    assert_response :forbidden
+
     post "/oauth/revoke", :token => token.token
     assert_redirected_to oauth_clients_url(token.user.display_name)
     token = OauthToken.find_by_token(token.token)

test/integration/user_terms_seen_test.rb

@@ -64,6 +64,8 @@ class UserTermsSeenTest < ActionDispatch::IntegrationTest
       # back to the terms page.
       get "/traces/mine"
       assert_redirected_to "controller" => "user", "action" => "terms", :referer => "/traces/mine"
+      get "/traces/mine", :referer => "/test"
+      assert_redirected_to "controller" => "user", "action" => "terms", :referer => "/test"
     end
   end
 

test/models/trace_test.rb

@@ -7,11 +7,17 @@ class TraceTest < ActiveSupport::TestCase
   def setup
     @gpx_trace_dir = Object.send("remove_const", "GPX_TRACE_DIR")
     Object.const_set("GPX_TRACE_DIR", File.dirname(__FILE__) + "/../traces")
+
+    @gpx_image_dir = Object.send("remove_const", "GPX_IMAGE_DIR")
+    Object.const_set("GPX_IMAGE_DIR", File.dirname(__FILE__) + "/../traces")
   end
 
   def teardown
     Object.send("remove_const", "GPX_TRACE_DIR")
     Object.const_set("GPX_TRACE_DIR", @gpx_trace_dir)
+
+    Object.send("remove_const", "GPX_IMAGE_DIR")
+    Object.const_set("GPX_IMAGE_DIR", @gpx_image_dir)
   end
 
   def test_trace_count
@@ -142,6 +148,28 @@ class TraceTest < ActiveSupport::TestCase
     assert_equal "848caa72f2f456d1bd6a0fdf228aa1b9", md5sum(gpx_files(:tar_bzip_trace_file).xml_file)
   end
 
+  def test_large_picture
+    picture = gpx_files(:public_trace_file).large_picture
+    trace = Trace.create
+
+    trace.large_picture = picture
+    assert_equal "7c841749e084ee4a5d13f12cd3bef456", md5sum(File.new(trace.large_picture_name))
+    assert_equal picture, trace.large_picture
+
+    trace.destroy
+  end
+
+  def test_icon_picture
+    picture = gpx_files(:public_trace_file).icon_picture
+    trace = Trace.create
+
+    trace.icon_picture = picture
+    assert_equal "b47baf22ed0e85d77e808694fad0ee27", md5sum(File.new(trace.icon_picture_name))
+    assert_equal picture, trace.icon_picture
+
+    trace.destroy
+  end
+
   private
 
   def check_query(query, traces)