comments: allow swapping votes.

Allow users to upvote a comment, then downvote it, without raising an
error or having to manually remove the upvote first. The upvote is
automatically removed and replaced by the downvote.

Changes to the /comment_votes API:

* `POST /comment_votes` and `DELETE /comment_votes` now return a comment
  vote instead of a comment.
* The `score` param in `POST /comment_votes` is now 1 or -1, not
  `up` or `down.`

test/functional/comment_votes_controller_test.rb

@@ -46,43 +46,83 @@ class CommentVotesControllerTest < ActionDispatch::IntegrationTest
       end
     end
 
-    context "#create.json" do
-      should "create a vote" do
+    context "create action" do
+      setup do
+        @user = create(:user)
+        @comment = create(:comment)
+      end
+
+      should "not allow anonymous users to vote" do
+        post comment_comment_votes_path(comment_id: @comment.id, score: "1"), xhr: true
+        assert_response 403
+      end
+
+      should "allow Members to vote" do
+        post_auth comment_comment_votes_path(comment_id: @comment.id, score: "1"), @user, xhr: true
+        assert_response :success
+      end
+
+      should "create a upvote" do
         assert_difference("CommentVote.count", 1) do
-          post_auth comment_comment_votes_path(comment_id: @comment.id, score: "down"), @user, as: :json
-          assert_response :success
-
-          assert_equal(@comment.id, response.parsed_body["id"])
-          assert_equal(-1, response.parsed_body["score"])
+          post_auth comment_comment_votes_path(comment_id: @comment.id, score: "1"), @user, xhr: true
         end
+
+        assert_response :success
+        assert_equal(1, @comment.reload.score)
       end
 
-      should "fail silently on errors" do
-        create(:comment_vote, user: @user, comment: @comment, score: -1)
-        assert_difference("CommentVote.count", 0) do
-          post_auth comment_comment_votes_path(comment_id: @comment.id, score: "down", format: "json"), @user
-          assert_response 422
-
-          comment = JSON.parse(@response.body)
-          assert_equal(false, comment["success"])
-          assert_equal("Validation failed: You have already voted for this comment", comment["message"])
-        end
-      end
-    end
-
-    context "#create.js" do
-      should "create a vote" do
+      should "create a downvote" do
         assert_difference("CommentVote.count", 1) do
-          post_auth comment_comment_votes_path(comment_id: @comment.id, format: "json", score: "down"), @user
-          assert_response :success
+          post_auth comment_comment_votes_path(comment_id: @comment.id, score: "-1"), @user, xhr: true
         end
+
+        assert_response :success
+        assert_equal(-1, @comment.reload.score)
       end
 
-      should "fail on errors" do
-        create(:comment_vote, user: @user, comment: @comment, score: -1)
+      should "ignore duplicate votes" do
+        vote = create(:comment_vote, comment: @comment, user: @user, score: 1)
+        assert_equal(1, vote.comment.reload.score)
+
         assert_difference("CommentVote.count", 0) do
-          post_auth comment_comment_votes_path(comment_id: @comment.id, :score => "down", format: "js"), @user
-          assert_response 422
+          post_auth comment_comment_votes_path(comment_id: @comment.id, score: "1"), @user, xhr: true
+        end
+
+        assert_response :success
+        assert_equal(1, @comment.reload.score)
+      end
+
+      should "automatically undo existing votes" do
+        create(:comment_vote, comment: @comment, user: @user, score: -1)
+        assert_equal(-1, @comment.reload.score)
+
+        assert_difference("CommentVote.count", 0) do
+          post_auth comment_comment_votes_path(comment_id: @comment.id, score: "1"), @user, xhr: true
+        end
+
+        assert_response :success
+        assert_equal(1, @comment.reload.score)
+      end
+
+      should "not allow voting on deleted comments" do
+        @comment.update!(is_deleted: true)
+
+        assert_difference("CommentVote.count", 0) do
+          post_auth comment_comment_votes_path(comment_id: @comment.id, score: "1"), @user, xhr: true
+        end
+
+        assert_response 403
+        assert_equal(0, @comment.reload.score)
+      end
+
+      should "not update the comment's updated_at or updater_id" do
+        assert_no_difference(["@comment.updater_id", "@comment.reload.updated_at"]) do
+          assert_difference("CommentVote.count", 1) do
+            post_auth comment_comment_votes_path(comment_id: @comment.id, score: "1"), @user, xhr: true
+
+            assert_response :success
+            assert_equal(1, @comment.reload.score)
+          end
         end
       end
     end
@@ -92,8 +132,8 @@ class CommentVotesControllerTest < ActionDispatch::IntegrationTest
         @vote = create(:comment_vote, user: @user)
 
         assert_difference("CommentVote.count", -1) do
-          delete_auth comment_comment_votes_path(@vote.comment), @user
-          assert_redirected_to @vote.comment
+          delete_auth comment_comment_votes_path(@vote.comment), @user, xhr: true
+          assert_response :success
         end
       end
 
@@ -101,8 +141,8 @@ class CommentVotesControllerTest < ActionDispatch::IntegrationTest
         @vote = create(:comment_vote)
 
         assert_difference("CommentVote.count", 0) do
-          delete_auth comment_comment_votes_path(@vote.comment), @user
-          assert_response 422
+          delete_auth comment_comment_votes_path(@vote.comment), @user, xhr: true
+          assert_response 404
         end
       end
     end

test/functional/moderator/dashboards_controller_test.rb

@@ -86,9 +86,7 @@ module Moderator
             end
 
             @users.each do |user|
-              CurrentUser.as(user) do
-                @comment.vote!(-1)
-              end
+              create(:comment_vote, score: -1, comment: @comment, user: user)
             end
           end
 

test/unit/comment_test.rb

@@ -143,55 +143,6 @@ class CommentTest < ActiveSupport::TestCase
         end
       end
 
-      should "not record the user id of the voter" do
-        user = FactoryBot.create(:user)
-        post = FactoryBot.create(:post)
-        c1 = FactoryBot.create(:comment, :post => post)
-        CurrentUser.scoped(user, "127.0.0.1") do
-          c1.vote!("up")
-          c1.reload
-          assert_not_equal(user.id, c1.updater_id)
-        end
-      end
-
-      should "not allow duplicate votes" do
-        user = FactoryBot.create(:user)
-        post = FactoryBot.create(:post)
-        c1 = FactoryBot.create(:comment, :post => post)
-
-        assert_nothing_raised { c1.vote!("down") }
-        exception = assert_raises(ActiveRecord::RecordInvalid) { c1.vote!("down") }
-        assert_equal("Validation failed: You have already voted for this comment", exception.message)
-        assert_equal(1, CommentVote.count)
-        assert_equal(-1, CommentVote.last.score)
-
-        c2 = FactoryBot.create(:comment, :post => post)
-        assert_nothing_raised { c2.vote!("down") }
-        assert_equal(2, CommentVote.count)
-      end
-
-      should "not allow upvotes by the creator" do
-        user = FactoryBot.create(:user)
-        post = FactoryBot.create(:post)
-        c1 = create(:comment, post: post, creator: CurrentUser.user)
-
-        exception = assert_raises(ActiveRecord::RecordInvalid) { c1.vote!("up") }
-        assert_equal("Validation failed: You cannot upvote your own comments", exception.message)
-      end
-
-      should "allow undoing of votes" do
-        user = FactoryBot.create(:user)
-        post = FactoryBot.create(:post)
-        comment = FactoryBot.create(:comment, :post => post)
-        CurrentUser.scoped(user, "127.0.0.1") do
-          comment.vote!("up")
-          comment.unvote!
-          comment.reload
-          assert_equal(0, comment.score)
-          assert_nothing_raised {comment.vote!("down")}
-        end
-      end
-
       should "be searchable" do
         c1 = FactoryBot.create(:comment, :body => "aaa bbb ccc")
         c2 = FactoryBot.create(:comment, :body => "aaa ddd")

test/unit/comment_vote_test.rb

@@ -0,0 +1,57 @@
+require 'test_helper'
+
+class CommentVoteTest < ActiveSupport::TestCase
+  context "A CommentVote" do
+    setup do
+      @user = create(:user)
+      @comment = as(@user) { create(:comment) }
+    end
+
+    context "during validation" do
+      subject { build(:comment_vote, comment: as(@user) { create(:comment) }) }
+
+      should validate_uniqueness_of(:user_id).scoped_to(:comment_id).with_message("have already voted for this comment")
+      should validate_inclusion_of(:score).in_array([-1, 1]).with_message("must be 1 or -1")
+    end
+
+    context "creating" do
+      context "an upvote" do
+        should "increment the comment's score" do
+          vote = create(:comment_vote, comment: @comment, score: 1)
+
+          assert_equal(1, @comment.reload.score)
+        end
+      end
+
+      context "a downvote" do
+        should "decrement the comment's score" do
+          vote = create(:comment_vote, comment: @comment, score: -1)
+
+          assert_equal(-1, @comment.reload.score)
+        end
+      end
+    end
+
+    context "destroying" do
+      context "an upvote" do
+        should "decrement the comment's score" do
+          vote = create(:comment_vote, comment: @comment, score: 1)
+          assert_equal(1, @comment.reload.score)
+
+          vote.destroy
+          assert_equal(0, @comment.reload.score)
+        end
+      end
+
+      context "a downvote" do
+        should "increment the comment's score" do
+          vote = create(:comment_vote, comment: @comment, score: -1)
+          assert_equal(-1, @comment.reload.score)
+
+          vote.destroy
+          assert_equal(0, @comment.reload.score)
+        end
+      end
+    end
+  end
+end