comments: allow votes to be soft deleted.

Make it so that when a user removes their own vote, the vote is soft
deleted (the is_deleted flag is set) instead of hard deleted.

Changes:

* Add is_deleted flag to comment votes.
* Relax uniqueness constraint so you can have multiple deleted votes on
  the same comment. You can still only have one active vote on the comment.
* Add `soft_delete` method to Deletable concern.

app/components/comment_component.rb

@@ -28,12 +28,12 @@ class CommentComponent < ApplicationComponent
 
   def upvoted?
     return false if current_user.is_anonymous?
-    comment.votes.select(&:is_positive?).map(&:user_id).include?(current_user.id)
+    comment.votes.active.select(&:is_positive?).map(&:user_id).include?(current_user.id)
   end
 
   def downvoted?
     return false if current_user.is_anonymous?
-    comment.votes.select(&:is_negative?).map(&:user_id).include?(current_user.id)
+    comment.votes.active.select(&:is_negative?).map(&:user_id).include?(current_user.id)
   end
 
   def reported?

app/controllers/comment_votes_controller.rb

@@ -13,7 +13,11 @@ class CommentVotesController < ApplicationController
 
     @comment.with_lock do
       @comment_vote = authorize CommentVote.new(comment: @comment, score: params[:score], user: CurrentUser.user)
-      CommentVote.where(comment: @comment, user: CurrentUser.user).destroy_all
+
+      CommentVote.active.where(comment: @comment, user: CurrentUser.user).each do |vote|
+        vote.soft_delete!(updater: CurrentUser.user)
+      end
+
       @comment_vote.save
     end
 
@@ -22,8 +26,9 @@ class CommentVotesController < ApplicationController
   end
 
   def destroy
-    @comment_vote = authorize CommentVote.find_by!(comment_id: params[:comment_id], user: CurrentUser.user)
+    # XXX should find by comment vote id.
-    @comment_vote.destroy
+    @comment_vote = authorize CommentVote.active.find_by!(comment_id: params[:comment_id], user: CurrentUser.user)
+    @comment_vote.soft_delete(updater: CurrentUser.user)
 
     respond_with(@comment_vote)
   end

app/logical/concerns/deletable.rb

@@ -6,6 +6,14 @@ module Deletable
       scope :active, -> { where(is_deleted: false) }
       scope :deleted, -> { where(is_deleted: true) }
       scope :undeleted, -> { where(is_deleted: false) }
+
+      define_method(:soft_delete) do |**options|
+        update(is_deleted: true, **options)
+      end
+
+      define_method(:soft_delete!) do |**options|
+        update!(is_deleted: true, **options)
+      end
     end
   end
 end

app/models/comment_vote.rb

@@ -1,12 +1,16 @@
 class CommentVote < ApplicationRecord
+  attr_accessor :updater
+
   belongs_to :comment
   belongs_to :user
 
-  validates :user_id, uniqueness: { scope: :comment_id, message: "have already voted for this comment" }
+  validate :validate_vote_is_unique, if: :is_deleted_changed?
   validates :score, inclusion: { in: [-1, 1], message: "must be 1 or -1" }
 
-  after_create :update_score_after_create
+  before_create :update_score_on_create
-  after_destroy :update_score_after_destroy
+  before_save :update_score_on_delete_or_undelete, if: -> { !new_record? && is_deleted_changed? }
+
+  deletable
 
   def self.visible(user)
     if user.is_moderator?
@@ -19,7 +23,7 @@ class CommentVote < ApplicationRecord
   end
 
   def self.search(params)
-    q = search_attributes(params, :id, :created_at, :updated_at, :score, :comment, :user)
+    q = search_attributes(params, :id, :created_at, :updated_at, :score, :is_deleted, :comment, :user)
     q.apply_default_order(params)
   end
 
@@ -31,15 +35,34 @@ class CommentVote < ApplicationRecord
     score == -1
   end
 
-  def update_score_after_create
+  # allow duplicate deleted votes but not duplicate active votes
+  def validate_vote_is_unique
+    if !is_deleted? && CommentVote.active.where.not(id: id).exists?(comment_id: comment_id, user_id: user_id)
+      errors.add(:user, "have already voted for this comment")
+    end
+  end
+
+  def update_score_on_create
     comment.with_lock do
       comment.update_columns(score: comment.score + score)
     end
   end
 
-  def update_score_after_destroy
+  def update_score_on_delete_or_undelete
     comment.with_lock do
-      comment.update_columns(score: comment.score - score)
+      if is_deleted_changed?(from: false, to: true)
+        comment.update_columns(score: comment.score - score)
+
+        if updater != user
+          ModAction.log("#{updater.name} deleted comment vote ##{id} on comment ##{comment_id}", :comment_vote_delete, updater)
+        end
+      else
+        comment.update_columns(score: comment.score + score)
+
+        if updater != user
+          ModAction.log("#{updater.name} undeleted comment vote ##{id} on comment ##{comment_id}", :comment_vote_undelete, updater)
+        end
+      end
     end
   end
 

app/models/mod_action.rb

@@ -37,6 +37,8 @@ class ModAction < ApplicationRecord
     artist_unban: 185,
     comment_update: 81,
     comment_delete: 82,
+    comment_vote_delete: 92,
+    comment_vote_undelete: 93,
     forum_topic_delete: 202,
     forum_topic_undelete: 203,
     forum_topic_lock: 206,

db/migrate/20210330003356_add_is_deleted_to_comment_votes.rb

@@ -0,0 +1,11 @@
+class AddIsDeletedToCommentVotes < ActiveRecord::Migration[6.1]
+  def change
+    add_column :comment_votes, :is_deleted, :boolean, default: false, null: :false
+    change_column_null :comment_votes, :is_deleted, false
+
+    add_index :comment_votes, :is_deleted, where: "is_deleted = TRUE"
+
+    remove_index :comment_votes, [:user_id, :comment_id], unique: true
+    add_index :comment_votes, [:user_id, :comment_id], unique: true, where: "is_deleted = FALSE"
+  end
+end

db/structure.sql

@@ -764,7 +764,8 @@ CREATE TABLE public.comment_votes (
     user_id integer NOT NULL,
     score integer NOT NULL,
     created_at timestamp without time zone NOT NULL,
-    updated_at timestamp without time zone NOT NULL
+    updated_at timestamp without time zone NOT NULL,
+    is_deleted boolean DEFAULT false NOT NULL
 );
 
 
@@ -5107,6 +5108,13 @@ CREATE INDEX index_comment_votes_on_comment_id ON public.comment_votes USING btr
 CREATE INDEX index_comment_votes_on_created_at ON public.comment_votes USING btree (created_at);
 
 
+--
+-- Name: index_comment_votes_on_is_deleted; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_comment_votes_on_is_deleted ON public.comment_votes USING btree (is_deleted) WHERE (is_deleted = true);
+
+
 --
 -- Name: index_comment_votes_on_user_id; Type: INDEX; Schema: public; Owner: -
 --
@@ -5118,7 +5126,7 @@ CREATE INDEX index_comment_votes_on_user_id ON public.comment_votes USING btree
 -- Name: index_comment_votes_on_user_id_and_comment_id; Type: INDEX; Schema: public; Owner: -
 --
 
-CREATE UNIQUE INDEX index_comment_votes_on_user_id_and_comment_id ON public.comment_votes USING btree (user_id, comment_id);
+CREATE UNIQUE INDEX index_comment_votes_on_user_id_and_comment_id ON public.comment_votes USING btree (user_id, comment_id) WHERE (is_deleted = false);
 
 
 --
@@ -8005,6 +8013,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20210214095121'),
 ('20210214101614'),
 ('20210303195217'),
-('20210310221248');
+('20210310221248'),
+('20210330003356');
 
 

test/components/comment_component_test.rb

@@ -92,5 +92,16 @@ class CommentComponentTest < ViewComponent::TestCase
         end
       end
     end
+
+    context "for a comment with a deleted vote" do
+      should "not treat the vote as active" do
+        @user = create(:user)
+        @vote = create(:comment_vote, user: @user, comment: @comment, is_deleted: true, score: 1)
+        render_comment(@comment, current_user: @user)
+
+        assert_css("article.comment[data-is-upvoted=false]")
+        assert_css("article.comment .comment-upvote-link.inactive-link")
+      end
+    end
   end
 end

test/factories/comment_vote.rb

@@ -3,5 +3,6 @@ FactoryBot.define do
     comment
     user
     score {1}
+    is_deleted { false }
   end
 end

test/functional/comment_votes_controller_test.rb

@@ -91,24 +91,28 @@ class CommentVotesControllerTest < ActionDispatch::IntegrationTest
         vote = create(:comment_vote, comment: @comment, user: @user, score: 1)
         assert_equal(1, vote.comment.reload.score)
 
-        assert_difference("CommentVote.count", 0) do
+        assert_difference("CommentVote.count", 1) 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)
+        assert_equal(1, @comment.votes.active.count)
+        assert_equal(1, @comment.votes.deleted.count)
       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
+        assert_difference("CommentVote.count", 1) 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)
+        assert_equal(1, @comment.votes.active.count)
+        assert_equal(1, @comment.votes.deleted.count)
       end
 
       should "not allow voting on deleted comments" do
@@ -138,9 +142,10 @@ class CommentVotesControllerTest < ActionDispatch::IntegrationTest
       should "allow users to remove their own comment votes" do
         @vote = create(:comment_vote, user: @user)
 
-        assert_difference("CommentVote.count", -1) do
+        assert_difference("CommentVote.count", 0) do
           delete_auth comment_comment_votes_path(@vote.comment), @user, xhr: true
           assert_response :success
+          assert_equal(true, @vote.reload.is_deleted?)
         end
       end
 
@@ -150,6 +155,22 @@ class CommentVotesControllerTest < ActionDispatch::IntegrationTest
         assert_difference("CommentVote.count", 0) do
           delete_auth comment_comment_votes_path(@vote.comment), @user, xhr: true
           assert_response 404
+          assert_equal(false, @vote.reload.is_deleted?)
+        end
+      end
+
+      context "deleting a vote on a comment that already has deleted votes" do
+        setup do
+          create(:comment_vote, comment: @comment, user: @user, score: 1, is_deleted: true)
+          create(:comment_vote, comment: @comment, user: @user, score: -1, is_deleted: true)
+        end
+
+        should "delete the current active vote" do
+          @vote = create(:comment_vote, comment: @comment, user: @user)
+          delete_auth comment_comment_votes_path(@vote.comment), @user, xhr: true
+
+          assert_response :success
+          assert_equal(true, @vote.reload.is_deleted?)
         end
       end
     end

test/unit/comment_vote_test.rb

@@ -10,11 +10,10 @@ class CommentVoteTest < ActiveSupport::TestCase
     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
 
-    should "not allow creating duplicate votes" do
+    should "not allow creating duplicate active votes" do
       v1 = create(:comment_vote, comment: @comment, user: @user)
       v2 = build(:comment_vote, comment: @comment, user: @user)
 
@@ -23,6 +22,16 @@ class CommentVoteTest < ActiveSupport::TestCase
       end
     end
 
+    should "allow creating duplicate deleted votes" do
+      v1 = create(:comment_vote, comment: @comment, user: @user)
+      v2 = create(:comment_vote, comment: @comment, user: @user, is_deleted: true)
+      v3 = create(:comment_vote, comment: @comment, user: @user, is_deleted: true)
+
+      assert_equal(true, v1.valid?)
+      assert_equal(true, v2.valid?)
+      assert_equal(true, v3.valid?)
+    end
+
     context "creating" do
       context "an upvote" do
         should "increment the comment's score" do
@@ -41,14 +50,15 @@ class CommentVoteTest < ActiveSupport::TestCase
       end
     end
 
-    context "destroying" do
+    context "soft deleting" 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
+          vote.soft_delete(updater: vote.user)
           assert_equal(0, @comment.reload.score)
+          assert_equal(true, vote.is_deleted?)
         end
       end
 
@@ -57,8 +67,9 @@ class CommentVoteTest < ActiveSupport::TestCase
           vote = create(:comment_vote, comment: @comment, score: -1)
           assert_equal(-1, @comment.reload.score)
 
-          vote.destroy
+          vote.soft_delete(updater: vote.user)
           assert_equal(0, @comment.reload.score)
+          assert_equal(true, vote.is_deleted?)
         end
       end
     end