From dfc85f089abc60ad1bd481de670413fa509dc36c Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Mon, 2 Mar 2015 00:39:35 +0000 Subject: [PATCH] Test changeset and note comment notification emails --- .rubocop_todo.yml | 2 +- app/models/relation.rb | 3 - test/controllers/changeset_controller_test.rb | 59 ++++++++---- test/controllers/notes_controller_test.rb | 94 ++++++++++++++----- 4 files changed, 111 insertions(+), 47 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 845289b14..025169122 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -37,7 +37,7 @@ Metrics/BlockNesting: # Offense count: 60 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 1523 + Max: 1543 # Offense count: 68 Metrics/CyclomaticComplexity: diff --git a/app/models/relation.rb b/app/models/relation.rb index 9e9e7144f..b19f8aeb1 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -86,13 +86,10 @@ class Relation < ActiveRecord::Base pt.find("member").each do |member| # member_type = - logger.debug "each member" fail OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member["type"] - logger.debug "after raise" # member_ref = member['ref'] # member_role member["role"] ||= "" # Allow the upload to not include this, in which case we default to an empty string. - logger.debug member["role"] relation.add_member(member["type"].classify, member["ref"], member["role"]) end fail OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil? diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb index 7d751a5c7..6a443850f 100644 --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@ -529,7 +529,7 @@ EOF content "" + "" + "" - assert_difference("Changeset.count", 1) do + assert_difference "Changeset.count", 1 do put :create end assert_response :success @@ -1884,10 +1884,35 @@ EOF 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" + assert_difference "ChangesetComment.count", 1 do + assert_no_difference "ActionMailer::Base.deliveries.size" do + post :comment, :id => changesets(:normal_user_closed_change).id, :text => "This is a comment" + end + 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" + end end assert_response :success + + email = ActionMailer::Base.deliveries.first + 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 + + ActionMailer::Base.deliveries.clear end ## @@ -1900,25 +1925,25 @@ EOF basic_authorization(users(:public_user).email, "test") # bad changeset id - assert_no_difference("ChangesetComment.count") do + 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 + 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 + 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 + assert_no_difference "ChangesetComment.count" do post :comment, :id => changesets(:normal_user_closed_change).id, :text => "" end assert_response :bad_request @@ -1930,7 +1955,7 @@ EOF basic_authorization(users(:public_user).email, "test") changeset = changesets(:normal_user_closed_change) - assert_difference("changeset.subscribers.count") do + assert_difference "changeset.subscribers.count", 1 do post :subscribe, :id => changeset.id end assert_response :success @@ -1941,7 +1966,7 @@ EOF def test_subscribe_fail # unauthorized changeset = changesets(:normal_user_closed_change) - assert_no_difference("changeset.subscribers.count") do + assert_no_difference "changeset.subscribers.count" do post :subscribe, :id => changeset.id end assert_response :unauthorized @@ -1949,21 +1974,21 @@ EOF basic_authorization(users(:public_user).email, "test") # bad changeset id - assert_no_difference("changeset.subscribers.count") do + 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 + 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 + assert_no_difference "changeset.subscribers.count" do post :subscribe, :id => changeset.id end assert_response :conflict @@ -1975,7 +2000,7 @@ EOF basic_authorization(users(:public_user).email, "test") changeset = changesets(:normal_user_subscribed_change) - assert_difference("changeset.subscribers.count", -1) do + assert_difference "changeset.subscribers.count", -1 do post :unsubscribe, :id => changeset.id end assert_response :success @@ -1986,7 +2011,7 @@ EOF def test_unsubscribe_fail # unauthorized changeset = changesets(:normal_user_closed_change) - assert_no_difference("changeset.subscribers.count") do + assert_no_difference "changeset.subscribers.count" do post :unsubscribe, :id => changeset.id end assert_response :unauthorized @@ -1994,21 +2019,21 @@ EOF basic_authorization(users(:public_user).email, "test") # bad changeset id - assert_no_difference("changeset.subscribers.count", -1) do + assert_no_difference "changeset.subscribers.count" 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 + assert_no_difference "changeset.subscribers.count" 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 + assert_no_difference "changeset.subscribers.count" do post :unsubscribe, :id => changeset.id end assert_response :not_found diff --git a/test/controllers/notes_controller_test.rb b/test/controllers/notes_controller_test.rb index 438fb7519..96f535e42 100644 --- a/test/controllers/notes_controller_test.rb +++ b/test/controllers/notes_controller_test.rb @@ -122,8 +122,8 @@ class NotesControllerTest < ActionController::TestCase end def test_create_success - assert_difference("Note.count") do - assert_difference("NoteComment.count") do + assert_difference "Note.count", 1 do + assert_difference "NoteComment.count", 1 do post :create, :lat => -1.0, :lon => -1.0, :text => "This is a comment", :format => "json" end end @@ -156,57 +156,57 @@ class NotesControllerTest < ActionController::TestCase end def test_create_fail - assert_no_difference("Note.count") do - assert_no_difference("NoteComment.count") do + assert_no_difference "Note.count" do + assert_no_difference "NoteComment.count" do post :create, :lon => -1.0, :text => "This is a comment" end end assert_response :bad_request - assert_no_difference("Note.count") do - assert_no_difference("NoteComment.count") do + assert_no_difference "Note.count" do + assert_no_difference "NoteComment.count" do post :create, :lat => -1.0, :text => "This is a comment" end end assert_response :bad_request - assert_no_difference("Note.count") do - assert_no_difference("NoteComment.count") do + assert_no_difference "Note.count" do + assert_no_difference "NoteComment.count" do post :create, :lat => -1.0, :lon => -1.0 end end assert_response :bad_request - assert_no_difference("Note.count") do - assert_no_difference("NoteComment.count") do + assert_no_difference "Note.count" do + assert_no_difference "NoteComment.count" do post :create, :lat => -1.0, :lon => -1.0, :text => "" end end assert_response :bad_request - assert_no_difference("Note.count") do - assert_no_difference("NoteComment.count") do + assert_no_difference "Note.count" do + assert_no_difference "NoteComment.count" do post :create, :lat => -100.0, :lon => -1.0, :text => "This is a comment" end end assert_response :bad_request - assert_no_difference("Note.count") do - assert_no_difference("NoteComment.count") do + assert_no_difference "Note.count" do + assert_no_difference "NoteComment.count" do post :create, :lat => -1.0, :lon => -200.0, :text => "This is a comment" end end assert_response :bad_request - assert_no_difference("Note.count") do - assert_no_difference("NoteComment.count") do + assert_no_difference "Note.count" do + assert_no_difference "NoteComment.count" do post :create, :lat => "abc", :lon => -1.0, :text => "This is a comment" end end assert_response :bad_request - assert_no_difference("Note.count") do - assert_no_difference("NoteComment.count") do + assert_no_difference "Note.count" do + assert_no_difference "NoteComment.count" do post :create, :lat => -1.0, :lon => "abc", :text => "This is a comment" end end @@ -214,8 +214,10 @@ class NotesControllerTest < ActionController::TestCase end def test_comment_success - assert_difference("NoteComment.count") do - post :comment, :id => notes(:open_note_with_comment).id, :text => "This is an additional comment", :format => "json" + assert_difference "NoteComment.count", 1 do + assert_no_difference "ActionMailer::Base.deliveries.size" do + post :comment, :id => notes(:open_note_with_comment).id, :text => "This is an additional comment", :format => "json" + end end assert_response :success js = ActiveSupport::JSON.decode(@response.body) @@ -239,35 +241,75 @@ class NotesControllerTest < ActionController::TestCase assert_equal "commented", js["properties"]["comments"].last["action"] assert_equal "This is an additional comment", js["properties"]["comments"].last["text"] assert_nil js["properties"]["comments"].last["user"] + + 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 3, 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_nil js["properties"]["comments"].last["user"] + + email = ActionMailer::Base.deliveries.first + assert_equal 1, email.to.length + assert_equal "[OpenStreetMap] An anonymous user 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] An anonymous user 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 3, 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_nil js["properties"]["comments"].last["user"] + + ActionMailer::Base.deliveries.clear end def test_comment_fail - assert_no_difference("NoteComment.count") do + assert_no_difference "NoteComment.count" do post :comment, :text => "This is an additional comment" end assert_response :bad_request - assert_no_difference("NoteComment.count") do + assert_no_difference "NoteComment.count" do post :comment, :id => notes(:open_note_with_comment).id end assert_response :bad_request - assert_no_difference("NoteComment.count") do + assert_no_difference "NoteComment.count" do post :comment, :id => notes(:open_note_with_comment).id, :text => "" end assert_response :bad_request - assert_no_difference("NoteComment.count") do + assert_no_difference "NoteComment.count" do post :comment, :id => 12345, :text => "This is an additional comment" end assert_response :not_found - assert_no_difference("NoteComment.count") do + assert_no_difference "NoteComment.count" do post :comment, :id => notes(:hidden_note_with_comment).id, :text => "This is an additional comment" end assert_response :gone - assert_no_difference("NoteComment.count") do + assert_no_difference "NoteComment.count" do post :comment, :id => notes(:closed_note_with_comment).id, :text => "This is an additional comment" end assert_response :conflict -- 2.43.2