Test changeset and note comment notification emails

.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:

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?

test/controllers/changeset_controller_test.rb

@@ -529,7 +529,7 @@ EOF
       content "<osm><changeset>" +
         "<tag k='created_by' v='osm test suite checking changesets'/>" +
         "</changeset></osm>"
-      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
+    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

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 "Note.count", 1 do
-      assert_difference("NoteComment.count") 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 "Note.count" do
-      assert_no_difference("NoteComment.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 "Note.count" do
-      assert_no_difference("NoteComment.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 "Note.count" do
-      assert_no_difference("NoteComment.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 "Note.count" do
-      assert_no_difference("NoteComment.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 "Note.count" do
-      assert_no_difference("NoteComment.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 "Note.count" do
-      assert_no_difference("NoteComment.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 "Note.count" do
-      assert_no_difference("NoteComment.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 "Note.count" do
-      assert_no_difference("NoteComment.count") do
+      assert_no_difference "NoteComment.count" do
         post :create, :lat => -1.0, :lon => "abc", :text => "This is a comment"
       end
     end
@@ -214,9 +214,11 @@ class NotesControllerTest < ActionController::TestCase
   end
 
   def test_comment_success
-    assert_difference("NoteComment.count") do
+    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)
     assert_not_nil js
@@ -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