Bring ForumPost ModAction trigger in line with Comments (#542)

* [Tests] Add hide & delete modaction tests to comments and forum posts

* [ForumPosts] Remove user id check on destroy

* [ForumPosts] Don't create edit modaction on hide

* Assert correct amount of ModActions is created

Especially important when it previously created 2 instead of 1 for hiding

---------

Co-authored-by: Earlopain <14981592+Earlopain@users.noreply.github.com>

app/models/forum_post.rb

@@ -22,14 +22,14 @@ class ForumPost < ApplicationRecord
   validate :validate_creator_is_not_limited, on: :create
   before_destroy :validate_topic_is_unlocked
   after_save :delete_topic_if_original_post
-  after_update(:if => ->(rec) {rec.updater_id != rec.creator_id}) do |rec|
-    ModAction.log(:forum_post_update, {forum_post_id: rec.id, forum_topic_id: rec.topic_id, user_id: rec.creator_id})
+  after_update(:if => ->(rec) { !rec.saved_change_to_is_hidden? && rec.updater_id != rec.creator_id }) do |rec|
+    ModAction.log(:forum_post_update, { forum_post_id: rec.id, forum_topic_id: rec.topic_id, user_id: rec.creator_id })
   end
-  after_update(:if => ->(rec) {rec.saved_change_to_is_hidden?}) do |rec|
-    ModAction.log(rec.is_hidden ? :forum_post_hide : :forum_post_unhide, {forum_post_id: rec.id, forum_topic_id: rec.topic_id, user_id: rec.creator_id})
+  after_update(:if => ->(rec) { rec.saved_change_to_is_hidden? }) do |rec|
+    ModAction.log(rec.is_hidden ? :forum_post_hide : :forum_post_unhide, { forum_post_id: rec.id, forum_topic_id: rec.topic_id, user_id: rec.creator_id })
   end
-  after_destroy(:if => ->(rec) {rec.updater_id != rec.creator_id}) do |rec|
-    ModAction.log(:forum_post_delete, {forum_post_id: rec.id, forum_topic_id: rec.topic_id, user_id: rec.creator_id})
+  after_destroy do |rec|
+    ModAction.log(:forum_post_delete, { forum_post_id: rec.id, forum_topic_id: rec.topic_id, user_id: rec.creator_id })
   end
 
   attr_accessor :bypass_limits

test/unit/comment_test.rb

@@ -1,10 +1,10 @@
-require 'test_helper'
+require "test_helper"
 
 class CommentTest < ActiveSupport::TestCase
   context "A comment" do
     setup do
-      user = create(:user)
-      CurrentUser.user = user
+      @user = create(:user)
+      CurrentUser.user = @user
     end
 
     context "created by a limited user" do
@@ -188,7 +188,7 @@ class CommentTest < ActiveSupport::TestCase
 
         should "create a mod action" do
           assert_difference("ModAction.count") do
-            @comment.update(:body => "nope")
+            @comment.update(body: "nope")
           end
         end
 
@@ -198,6 +198,56 @@ class CommentTest < ActiveSupport::TestCase
         end
       end
 
+      context "that is hidden by a moderator" do
+        setup do
+          @comment = create(:comment)
+          @mod = create(:moderator_user)
+          CurrentUser.user = @mod
+        end
+
+        should "create a mod action" do
+          assert_difference(-> { ModAction.count }, 1) do
+            @comment.update(is_hidden: true)
+          end
+        end
+
+        should "credit the moderator as the updater" do
+          @comment.update(is_hidden: true)
+          assert_equal(@mod.id, @comment.updater_id)
+        end
+      end
+
+      context "that is stickied by a moderator" do
+        setup do
+          @comment = create(:comment)
+          @mod = create(:moderator_user)
+          CurrentUser.user = @mod
+        end
+
+        should "create a mod action" do
+          assert_difference(-> { ModAction.count }, 1) do
+            @comment.update(is_sticky: true)
+          end
+        end
+
+        should "credit the moderator as the updater" do
+          @comment.update(is_sticky: true)
+          assert_equal(@mod.id, @comment.updater_id)
+        end
+      end
+
+      context "that is deleted" do
+        setup do
+          @comment = create(:comment)
+        end
+
+        should "create a mod action" do
+          assert_difference(-> { ModAction.count }, 1) do
+            @comment.destroy
+          end
+        end
+      end
+
       context "that is below the score threshold" do
         should "be hidden if not stickied" do
           user = create(:user, comment_threshold: 0)

test/unit/forum_post_test.rb

@@ -1,4 +1,4 @@
-require 'test_helper'
+require "test_helper"
 
 class ForumPostTest < ActiveSupport::TestCase
   context "A forum post" do
@@ -99,17 +99,59 @@ class ForumPostTest < ActiveSupport::TestCase
       assert_equal(@user.id, post.creator_id)
     end
 
-    context "updated by a second user" do
+    context "that is edited by a moderator" do
       setup do
         @post = create(:forum_post, topic_id: @topic.id)
-        @second_user = create(:user)
-        CurrentUser.user = @second_user
+        @mod = create(:moderator_user)
+        CurrentUser.user = @mod
       end
 
-      should "record its updater" do
-        @post.update(:body => "abc")
-        assert_equal(@second_user.id, @post.updater_id)
+      should "create a mod action" do
+        assert_difference(-> { ModAction.count }, 1) do
+          @post.update(body: "nope")
+        end
       end
+
+      should "credit the moderator as the updater" do
+        @post.update(body: "test")
+        assert_equal(@mod.id, @post.updater_id)
+      end
+    end
+
+    context "that is hidden by a moderator" do
+      setup do
+        @post = create(:forum_post, topic_id: @topic.id)
+        @mod = create(:moderator_user)
+        CurrentUser.user = @mod
+      end
+
+      should "create a mod action" do
+        assert_difference(-> { ModAction.count }, 1) do
+          @post.update(is_hidden: true)
+        end
+      end
+
+      should "credit the moderator as the updater" do
+        @post.update(is_hidden: true)
+        assert_equal(@mod.id, @post.updater_id)
+      end
+    end
+
+    context "that is deleted" do
+      setup do
+        @post = create(:forum_post, topic_id: @topic.id)
+      end
+
+      should "create a mod action" do
+        assert_difference(-> { ModAction.count }, 1) do
+          @post.destroy
+        end
+      end
+    end
+
+    context "during validation" do
+      subject { build(:forum_post) }
+      should_not allow_value(" ").for(:body)
     end
   end
 end