UX: Rename "Keep Post" to "Keep Post Hidden" when hidden (#7767)

* UX: Rename "Keep Post" to "Keep Post Hidden" when hidden

This is based on this feedback:
https://meta.discourse.org/t/category-group-review-moderation/116478/19

When a post is hidden this makes the operation much more clear.

* REFACTOR: Better support for aliases for actions

Allow calls on alias actions and delegate to the original one.
This is less code but also simplifies tests where the action might
be "agree_and_keep" or "agree_and_keep_hidden" which are the same.

app/models/reviewable.rb

@@ -41,6 +41,11 @@ class Reviewable < ActiveRecord::Base
     Jobs.enqueue(:notify_reviewable, reviewable_id: self.id) if pending?
   end
 
+  # Can be used if several actions are equivalent
+  def self.action_aliases
+    {}
+  end
+
   # The gaps are in case we want more precision in the future
   def self.priorities
     @priorities ||= Enum.new(
@@ -283,11 +288,15 @@ class Reviewable < ActiveRecord::Base
   def perform(performed_by, action_id, args = nil)
     args ||= {}
 
+    # Support this action or any aliases
+    aliases = self.class.action_aliases
+    valid = [ action_id, aliases.to_a.select { |k, v| v == action_id }.map(&:first) ].flatten
+
     # Ensure the user has access to the action
     actions = actions_for(Guardian.new(performed_by), args)
-    raise InvalidAction.new(action_id, self.class) unless actions.has?(action_id)
+    raise InvalidAction.new(action_id, self.class) unless valid.any? { |a| actions.has?(a) }
 
-    perform_method = "perform_#{action_id}".to_sym
+    perform_method = "perform_#{aliases[action_id] || action_id}".to_sym
     raise InvalidAction.new(action_id, self.class) unless respond_to?(perform_method)
 
     result = nil

app/models/reviewable_flagged_post.rb

@@ -4,6 +4,14 @@ require_dependency 'reviewable'
 
 class ReviewableFlaggedPost < Reviewable
 
+  # Penalties are handled by the modal after the action is performed
+  def self.action_aliases
+    { agree_and_keep_hidden: :agree_and_keep,
+      agree_and_silence: :agree_and_keep,
+      agree_and_suspend: :agree_and_keep,
+      disagree_and_restore: :disagree }
+  end
+
   def self.counts_for(posts)
     result = {}
 
@@ -40,7 +48,12 @@ class ReviewableFlaggedPost < Reviewable
       build_action(actions, :agree_and_hide, icon: 'far-eye-slash', bundle: agree)
     end
 
-    build_action(actions, :agree_and_keep, icon: 'thumbs-up', bundle: agree)
+    if post.hidden?
+      build_action(actions, :agree_and_keep_hidden, icon: 'thumbs-up', bundle: agree)
+    else
+      build_action(actions, :agree_and_keep, icon: 'thumbs-up', bundle: agree)
+    end
+
     if guardian.can_suspend?(target_created_by)
       build_action(actions, :agree_and_suspend, icon: 'ban', bundle: agree, client_action: 'suspend')
       build_action(actions, :agree_and_silence, icon: 'microphone-slash', bundle: agree, client_action: 'silence')
@@ -119,19 +132,10 @@ class ReviewableFlaggedPost < Reviewable
     end
   end
 
-  # Penalties are handled by the modal after the action is performed
   def perform_agree_and_keep(performed_by, args)
     agree(performed_by, args)
   end
 
-  def perform_agree_and_suspend(performed_by, args)
-    agree(performed_by, args)
-  end
-
-  def perform_agree_and_silence(performed_by, args)
-    agree(performed_by, args)
-  end
-
   def perform_delete_spammer(performed_by, args)
     UserDestroyer.new(performed_by).destroy(
       post.user,
@@ -159,10 +163,6 @@ class ReviewableFlaggedPost < Reviewable
     end
   end
 
-  def perform_disagree_and_restore(performed_by, args)
-    perform_disagree(performed_by, args)
-  end
-
   def perform_disagree(performed_by, args)
     # -1 is the automatic system clear
     action_type_ids =

config/locales/server.en.yml

@@ -4492,6 +4492,9 @@ en:
       agree_and_keep:
         title: "Keep Post"
         description: "Agree with flag and keep the post unchanged."
+      agree_and_keep_hidden:
+        title: "Keep Post Hidden"
+        description: "Agree with flag and leave the post hidden."
       agree_and_suspend:
         title: "Suspend User"
         description: "Agree with flag and suspend the user."

spec/models/reviewable_flagged_post_spec.rb

@@ -31,6 +31,7 @@ RSpec.describe ReviewableFlaggedPost, type: :model do
         actions = reviewable.actions_for(guardian)
         expect(actions.has?(:agree_and_hide)).to eq(true)
         expect(actions.has?(:agree_and_keep)).to eq(true)
+        expect(actions.has?(:agree_and_keep_hidden)).to eq(false)
         expect(actions.has?(:agree_and_silence)).to eq(true)
         expect(actions.has?(:agree_and_suspend)).to eq(true)
         expect(actions.has?(:delete_spammer)).to eq(true)
@@ -54,6 +55,13 @@ RSpec.describe ReviewableFlaggedPost, type: :model do
         expect(actions.has?(:delete_and_replies)).to eq(false)
       end
 
+      it "changes `agree_and_keep` to `agree_and_keep_hidden` if it's been hidden" do
+        post.hidden = true
+        actions = reviewable.actions_for(guardian)
+        expect(actions.has?(:agree_and_keep)).to eq(false)
+        expect(actions.has?(:agree_and_keep_hidden)).to eq(true)
+      end
+
       it "returns `agree_and_restore` if the post is user deleted" do
         post.update(user_deleted: true)
         expect(reviewable.actions_for(guardian).has?(:agree_and_restore)).to eq(true)