Override Tag Moderation Actions (#20088)

* feat: remove the validation for the article_id and tag_name to be unique on a tag_adjustment

* chore: remove unique index

* feat: update the validate_tag method to align with the removal of unique indexes on the table

* feat: do not allow removal and addition of tags that were added/removed by admins

* spec: add tests for the tag_adjustment_spec

* fix: update the validation of tags

* spec: article validation

* spec: test the action_panel helper

* chore: amend language

* add styles

* Empty commit

app/assets/stylesheets/views/mod-actions.scss

@@ -224,6 +224,21 @@
     }
   }
 
+  .adjustment-restriction__container--text {
+    color: var(--color-secondary);
+  }
+
+  .adjustment-restriction__container--tag {
+    font-family: var(--ff-monospace);
+    line-height: var(--lh-base);
+    letter-spacing: -0.32px;
+    color: var(--tag-name);
+
+    .num-sign {
+      color: var(--tag-hash);
+    }
+  }
+
   .additional-subtext-section {
     font-weight: var(--fw-medium);
     font-size: var(--fs-xs);
@@ -290,6 +305,7 @@
   .reaction-vomit-button.reacted .reacted-emoji {
     display: block;
   }
+
   .other-things-container {
     border-top: 1px solid var(--base-20);
     overflow-y: auto;
@@ -335,6 +351,7 @@
           }
         }
       }
+
       .toggle-chevron-container {
         width: 16px;
         height: 16px;
@@ -350,6 +367,7 @@
           transform: rotateZ(-180deg);
         }
       }
+
       &:hover {
         background-color: var(--base-inverted);
         .label-wrapper > .icon {
@@ -362,11 +380,13 @@
           color: var(--accent-brand);
         }
       }
+
       &.active {
         header h2 {
           font-weight: var(--fw-bold);
         }
       }
+
       &.inactive:not(.active) {
         header h2 {
           color: var(--base-50);
@@ -385,6 +405,7 @@
         }
       }
     }
+
     .dropdown-options {
       flex-direction: column;
       padding: 0 var(--su-4);
@@ -394,6 +415,7 @@
         display: flex;
       }
     }
+
     .adjust-tags-options {
       .add-tag-container {
         padding: var(--su-2) var(--su-1);
@@ -482,6 +504,7 @@
           }
         }
       }
+
       button.add-tag {
         background-color: inherit;
         border: none;
@@ -552,6 +575,7 @@
         }
       }
     }
+
     .set-experience-options {
       button.level-rating-button {
         background: inherit;

app/helpers/moderations/actions_panel_helper.rb

@@ -0,0 +1,10 @@
+module Moderations
+  module ActionsPanelHelper
+    def last_adjusted_by_admin?(article, tag, adjustment_type)
+      last_user_id = TagAdjustment.where(article_id: article.id, adjustment_type: adjustment_type, status: "committed",
+                                         tag_id: tag.id)
+        .last&.user_id
+      User.find_by(id: last_user_id)&.any_admin?
+    end
+  end
+end

app/models/article.rb

@@ -817,10 +817,20 @@ class Article < ApplicationRecord
     published_at || date || Time.current
   end
 
+  # When an article is saved, it ensures that the tags that were adjusted by moderators and admins
+  # remain adjusted. We do not allow the author to add or remove tags that were previously added or
+  # removed by moderators and admins.
+  #
+  # This method is called before validation, so that the tag_list can be validated.
+  #
+  # @return [String] an array of tag names.
   def validate_tag
-    # remove adjusted tags
-    remove_tag_adjustments_from_tag_list
-    add_tag_adjustments_to_tag_list
+    distinct_tag_adjustments = TagAdjustment.where(article_id: id, status: "committed")
+      .select('DISTINCT ON ("tag_id") *')
+      .order(:tag_id, updated_at: :desc, id: :desc)
+
+    remove_tag_adjustments_from_tag_list(distinct_tag_adjustments)
+    add_tag_adjustments_to_tag_list(distinct_tag_adjustments)
 
     # check there are not too many tags
     return errors.add(:tag_list, I18n.t("models.article.too_many_tags")) if tag_list.size > MAX_TAG_LIST_SIZE
@@ -828,14 +838,13 @@ class Article < ApplicationRecord
     validate_tag_name(tag_list)
   end
 
-  def remove_tag_adjustments_from_tag_list
-    tags_to_remove = TagAdjustment.where(article_id: id, adjustment_type: "removal",
-                                         status: "committed").pluck(:tag_name)
+  def remove_tag_adjustments_from_tag_list(distinct_adjustments)
+    tags_to_remove = distinct_adjustments.select { |adj| adj.adjustment_type == "removal" }.pluck(:tag_name)
     tag_list.remove(tags_to_remove, parse: true) if tags_to_remove.present?
   end
 
-  def add_tag_adjustments_to_tag_list
-    tags_to_add = TagAdjustment.where(article_id: id, adjustment_type: "addition", status: "committed").pluck(:tag_name)
+  def add_tag_adjustments_to_tag_list(distinct_adjustments)
+    tags_to_add = distinct_adjustments.select { |adj| adj.adjustment_type == "addition" }.pluck(:tag_name)
     return if tags_to_add.blank?
 
     tag_list.add(tags_to_add, parse: true)

app/models/tag_adjustment.rb

@@ -1,6 +1,5 @@
 class TagAdjustment < ApplicationRecord
-  validates :tag_name, presence: true,
-                       uniqueness: { scope: :article_id, message: I18n.t("models.tag_adjustment.unique") }
+  validates :tag_name, presence: true
   validates :adjustment_type, inclusion: { in: %w[removal addition] }, presence: true
   validates :status, inclusion: { in: %w[committed pending committed_and_resolvable resolved] }, presence: true
   has_many :notifications, as: :notifiable, inverse_of: :notifiable, dependent: :delete_all

app/views/moderations/actions_panel.html.erb

@@ -110,7 +110,12 @@
 
       <div id="adjust-tags-options" class="adjust-tags-options dropdown-options hidden">
         <% @moderatable.tags.each do |tag| %>
-          <% moderatable_tag = @tag_moderator_tags.pluck(:name).include?(tag.name) || policy(@moderatable).can_adjust_any_tag? %>
+          <% last_added_by_admin = last_adjusted_by_admin?(@moderatable, tag, "addition") %>
+          <% current_tag_moderator = @tag_moderator_tags.pluck(:name).include?(tag.name) %>
+
+          <%# The tag is moderatable if the user is a current moderator of that tag and that tag was not added by admin OR the user is an admin %>
+          <% moderatable_tag = (current_tag_moderator && !last_added_by_admin) || policy(@moderatable).can_adjust_any_tag? %>
+
           <button
             id="remove-tag-button-<%= tag.name %>"
             class="adjustable-tag" type="button"
@@ -125,6 +130,13 @@
               </div>
             <% end %>
           </button>
+
+          <% if current_tag_moderator && last_added_by_admin %>
+            <div class="fs-s px-3 adjustment-restriction">
+              <%= t("views.moderations.actions.adjust.cannot_remove_admin_adjusted_tag_html", contact_email: ForemInstance.contact_email) %>
+            </div>
+          <% end %>
+
           <% if moderatable_tag %>
             <div id="remove-tag-container-<%= tag.name %>" class="hidden">
               <div id="adjustment-reason-container-<%= tag.name %>" class="reason-container">
@@ -169,28 +181,41 @@
               <hr />
             <% end %>
             <% @tag_moderator_tags.each do |tag| %>
+              <% last_removed_by_admin = last_adjusted_by_admin?(@moderatable, tag, "removal") %>
+
               <% if !@moderatable.tags.pluck(:name).include?(tag.name) %>
-                <button
-                  id="add-tag-button-<%= tag.name %>"
-                  class="adjustable-tag add-tag" type="button"
-                  data-adjustment-type="plus"
-                  data-tag-name="<%= tag.name %>"
-                  data-article-id="<%= @moderatable.id %>">
-                  <span class="num-sign">#</span><%= tag.name %>
-                  <div id="add-tag-icon-<%= tag.name %>" class="circle centered-icon add-icon">
-                    <%= crayons_icon_tag(:plus, title: t("views.moderations.actions.adjust.add")) %>
-                  </div>
-                </button>
-                <div id="add-tag-container-<%= tag.name %>" class="hidden">
-                  <div id="add-reason-container-<%= tag.name %>" class="reason-container">
-                    <textarea id="tag-add-reason-<%= tag.name %>" class="crayons-textfield mt-3" placeholder="<%= t("views.moderations.actions.adjust.reason.add_tag") %>"></textarea>
-                    <div class="flex gap-3">
-                      <button class="w-100 c-btn c-btn--primary" id="tag-add-submit-<%= tag.name %>"><%= t("views.moderations.actions.adjust.add") %></button>
-                      <button class="w-100 c-btn" id="cancel-add-tag-button-<%= tag.name %>" type="button"><%= t("views.moderations.actions.adjust.cancel") %></button>
+
+                <% if last_removed_by_admin %>
+                  <div class="adjustment-restriction__container px-3 align-left">
+                    <div class="adjustment-restriction__container--tag fs-base"><span class="num-sign px-05">#</span><%= tag.name %></div>
+                    <div class="fs-s adjustment-restriction__container--text">
+                      <%= t("views.moderations.actions.adjust.cannot_add_admin_adjusted_tag_html", contact_email: ForemInstance.contact_email) %>
                     </div>
                   </div>
-                </div>
+                <% else %>
+                  <button
+                    id="add-tag-button-<%= tag.name %>"
+                    class="adjustable-tag add-tag" type="button"
+                    data-adjustment-type="plus"
+                    data-tag-name="<%= tag.name %>"
+                    data-article-id="<%= @moderatable.id %>">
+                    <span class="num-sign">#</span><%= tag.name %>
+                    <div id="add-tag-icon-<%= tag.name %>" class="circle centered-icon add-icon">
+                      <%= crayons_icon_tag(:plus, title: t("views.moderations.actions.adjust.add")) %>
+                    </div>
+                  </button>
+                  <div id="add-tag-container-<%= tag.name %>" class="hidden">
+                    <div id="add-reason-container-<%= tag.name %>" class="reason-container">
+                      <textarea id="tag-add-reason-<%= tag.name %>" class="crayons-textfield mt-3" placeholder="<%= t("views.moderations.actions.adjust.reason.add_tag") %>"></textarea>
+                      <div class="flex gap-3">
+                        <button class="w-100 c-btn c-btn--primary" id="tag-add-submit-<%= tag.name %>"><%= t("views.moderations.actions.adjust.add") %></button>
+                        <button class="w-100 c-btn" id="cancel-add-tag-button-<%= tag.name %>" type="button"><%= t("views.moderations.actions.adjust.cancel") %></button>
+                      </div>
+                    </div>
+                  </div>
+                <% end %>
               <% end %>
+
             <% end %>
           <% end %>
           <hr />

config/locales/views/moderations/en.yml

@@ -23,19 +23,21 @@ en:
           history:
             added_by: added by
             removed_by: removed by
-          reason: 
+          reason:
             remove_tag: Reason to remove tag (optional)
             add_tag: Reason to add tag (optional)
           remove: Remove tag
           submit: Submit
           cancel: Cancel
-          maximum_tags: 
+          maximum_tags:
             icon: Info
             text: A post can only have four tags.
           add_tag:
             header: Add new tag
             button_text: Add new tag
             icon: Add tag
+          cannot_remove_admin_adjusted_tag_html: Tag has been added by an Admin. You can't remove it now. If you feel that's a mistake, please contact us at <a href="mailto:%{contact_email}">%{contact_email}</a>.
+          cannot_add_admin_adjusted_tag_html: Tag has been removed by an Admin. You can't add it now. If you feel that's a mistake, please contact us at <a href="mailto:%{contact_email}">%{contact_email}</a>.
         approve:
           add: Mark as approved
           remove: Remove approval

config/locales/views/moderations/fr.yml

@@ -23,7 +23,7 @@ fr:
           history:
             added_by: ajouté par
             removed_by: supprimé par
-          reason: 
+          reason:
             remove_tag: Raison de supprimer la balise
             add_tag: Raison d'ajouter la balise (optionnel)
           remove: Retirer l'étiquette
@@ -36,6 +36,8 @@ fr:
             header: Ajouter une nouvelle balise
             button_text: Ajouter une nouvelle balise
             icon: Ajouter une étiquette
+        cannot_add_admin_adjusted_tag_html: La balise aurait ajoutée par l'Admin. Vous pouvez pas la supprimer maintenant. Si vous pensez que c'est une erreur, veuillez nous contacter à %{support_email}.
+        cannot_remove_admin_adjusted_tag_html: La balise aurait supprimée par l'Admin. Vous pouvez pas l'ajouter maintenant. Si vous pensez que c'est une erreur, veuillez nous contacter à %{support_email}.
         approve:
           add: Marquer comme approuvé
           remove: Retirer l'approbation

db/migrate/20230910135738_remove_index_tag_adjustments_on_tag_name_and_article_id.rb

@@ -0,0 +1,19 @@
+class RemoveIndexTagAdjustmentsOnTagNameAndArticleId < ActiveRecord::Migration[7.0]
+  disable_ddl_transaction!
+
+  def up
+    return unless index_exists?(:tag_adjustments, %i[tag_name article_id])
+
+    remove_index :tag_adjustments,
+                 column: %i[tag_name article_id],
+                 unique: true,
+                 algorithm: :concurrently,
+                 name: "index_tag_adjustments_on_tag_name_and_article_id"
+  end
+
+  def down
+    return if index_exists?(:tag_adjustments, %i[tag_name article_id])
+
+    add_index :tag_adjustments, %i[tag_name article_id], unique: true, algorithm: :concurrently
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_09_06_112602) do
+ActiveRecord::Schema[7.0].define(version: 2023_09_10_135738) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
   enable_extension "ltree"
@@ -1092,7 +1092,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_09_06_112602) do
     t.string "tag_name"
     t.datetime "updated_at", precision: nil, null: false
     t.bigint "user_id"
-    t.index ["tag_name", "article_id"], name: "index_tag_adjustments_on_tag_name_and_article_id", unique: true
   end
 
   create_table "taggings", force: :cascade do |t|

spec/helpers/moderations/actions_panel_helper_spec.rb

@@ -0,0 +1,26 @@
+require "rails_helper"
+
+describe Moderations::ActionsPanelHelper do
+  describe "#last_adjusted_by_admin?" do
+    let(:tag1) { create(:tag) }
+    let(:tag2) { create(:tag, name: "tag2") }
+
+    let(:admin) { create(:user, :admin) }
+    let(:user) { create(:user) }
+
+    let(:article) { create(:article, tags: "tag2") }
+
+    it "returns false if the last adjustment was made by a non-admin" do
+      user.add_role(:tag_moderator, tag2)
+      create(:tag_adjustment, article: article, tag_id: tag2.id, tag_name: tag2.name, user: user,
+                              status: "committed", adjustment_type: "removal")
+      expect(helper.last_adjusted_by_admin?(article, tag2, "removal")).to be(false)
+    end
+
+    it "returns true if the last adjustment was made by any admin" do
+      create(:tag_adjustment, article: article, tag_id: tag1.id, tag_name: tag1.name, user: admin,
+                              status: "committed", adjustment_type: "addition")
+      expect(helper.last_adjusted_by_admin?(article, tag1, "addition")).to be(true)
+    end
+  end
+end

spec/models/article_spec.rb

@@ -107,6 +107,46 @@ RSpec.describe Article do
       end
     end
 
+    describe "#validate_tag" do
+      # rubocop:disable RSpec/VerifiedDoubles
+      it "does not modify the tag list if there are no adjustments" do
+        # See https://github.com/forem/forem/pull/6302
+        article = build(:article, user: user)
+        allow(TagAdjustment).to receive(:where).and_return(TagAdjustment.none)
+        allow(article).to receive(:tag_list).and_return(spy("tag_list"))
+
+        article.save
+
+        # We expect this to happen once in #evaluate_front_matter
+        expect(article.tag_list).to have_received(:add).once
+        expect(article.tag_list).not_to have_received(:remove)
+      end
+      # rubocop:enable RSpec/VerifiedDoubles
+
+      it "adjusts the tags in the tag_list based on the tag_adjustments" do
+        user = create(:user, :admin)
+        tag1 = create(:tag, name: "tag1")
+        tag2 = create(:tag, name: "tag2")
+
+        # try save an article with a tag_list of tag 1, tag 3
+        # in the tag adjustments we have a removal of tag1 and an addition of tag2
+        # hence the tag_list should be tag2, tag3
+        article = build(:article, user: user, tags: "#{tag1.name}, tag3")
+
+        create(:tag_adjustment, adjustment_type: "addition", tag_id: tag2.id,
+                                tag_name: tag2.name, article: article, user: user)
+
+        create(:tag_adjustment, adjustment_type: "removal", tag_id: tag1.id,
+                                tag_name: tag1.name, article: article, user: user)
+
+        article.save
+
+        expect(article.tag_list).to include("tag3")
+        expect(article.tag_list).to include("tag2")
+        expect(article.tag_list).not_to include("tag1")
+      end
+    end
+
     describe "#validate co_authors" do
       it "is invalid if the co_author is the same as the author" do
         article.co_author_ids = [user.id]
@@ -275,24 +315,6 @@ RSpec.describe Article do
         expect(test_article.errors_as_sentence).to match("Title can't be blank")
       end
     end
-
-    describe "tag validation" do
-      let(:article) { build(:article, user: user) }
-
-      # See https://github.com/forem/forem/pull/6302
-      # rubocop:disable RSpec/VerifiedDoubles
-      it "does not modify the tag list if there are no adjustments" do
-        allow(TagAdjustment).to receive(:where).and_return(TagAdjustment.none)
-        allow(article).to receive(:tag_list).and_return(spy("tag_list"))
-
-        article.save
-
-        # We expect this to happen once in #evaluate_front_matter
-        expect(article.tag_list).to have_received(:add).once
-        expect(article.tag_list).not_to have_received(:remove)
-      end
-      # rubocop:enable RSpec/VerifiedDoubles
-    end
   end
 
   context "when data is extracted from evaluation of the front matter during validation" do

spec/models/tag_adjustment_spec.rb

@@ -3,13 +3,16 @@ require "rails_helper"
 RSpec.describe TagAdjustment do
   before do
     mod_user.add_role(:tag_moderator, tag)
-    admin_user.add_role(:admin)
+    other_mod.add_role(:tag_moderator, tag)
   end
 
   let(:article) { create(:article, tags: nil) }
   let(:tag) { create(:tag) }
-  let(:admin_user) { create(:user) }
+  let(:admin_user) { create(:user, :admin) }
+  let(:other_admin) { create(:user, :admin) }
+  let(:super_mod) { create(:user, :super_moderator) }
   let(:mod_user) { create(:user) }
+  let(:other_mod) { create(:user) }
   let(:regular_user) { create(:user) }
 
   it { is_expected.to validate_presence_of(:tag_name) }
@@ -23,20 +26,37 @@ RSpec.describe TagAdjustment do
       expect(tag_adjustment).to be_valid
     end
 
-    it "does not allow tag mods to create for other tags" do
-      another_tag = create(:tag)
-      tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: another_tag.id)
-      expect(tag_adjustment).to be_invalid
+    it "allows tag mods to create a tag adjustment for a tag that has been adjusted by another tag mod" do
+      create(:tag_adjustment, user_id: other_mod.id, article_id: article.id, tag_id: tag.id)
+
+      tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id)
+      expect(tag_adjustment).to be_valid
     end
 
-    it "allows admins to create for any tags" do
+    it "does not allow tag mods to create a tag adjustment for other tags" do
+      another_tag = create(:tag)
+      tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: another_tag.id)
+      expect(tag_adjustment).not_to be_valid
+    end
+
+    it "allows admins to create a tag adjustment for any tags" do
       tag_adjustment = build(:tag_adjustment, user_id: admin_user.id, article_id: article.id, tag_id: tag.id)
       expect(tag_adjustment).to be_valid
     end
 
-    it "does not allow normal users to create for any tags" do
+    it "allows admins and super moderators to create a tag adjustment for a tag that was adjusted by another admin" do
+      create(:tag_adjustment, user_id: admin_user.id, article_id: article.id, tag_id: tag.id)
+
+      tag_adjustment = build(:tag_adjustment, user_id: other_admin.id, article_id: article.id, tag_id: tag.id)
+      expect(tag_adjustment).to be_valid
+
+      tag_adjustment = build(:tag_adjustment, user_id: super_mod.id, article_id: article.id, tag_id: tag.id)
+      expect(tag_adjustment).to be_valid
+    end
+
+    it "does not allow normal users to create a tag adjustment for any tags" do
       tag_adjustment = build(:tag_adjustment, user_id: regular_user.id, article_id: article.id, tag_id: tag.id)
-      expect(tag_adjustment).to be_invalid
+      expect(tag_adjustment).not_to be_valid
     end
   end
 
@@ -56,7 +76,7 @@ RSpec.describe TagAdjustment do
     it "disallows improper adjustment_types" do
       tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id,
                                               adjustment_type: "slushie")
-      expect(tag_adjustment).to be_invalid
+      expect(tag_adjustment).not_to be_valid
     end
 
     it "allows proper status" do
@@ -68,7 +88,7 @@ RSpec.describe TagAdjustment do
     it "disallows improper status" do
       tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id,
                                               status: "slushiemonkey")
-      expect(tag_adjustment).to be_invalid
+      expect(tag_adjustment).not_to be_valid
     end
   end
 
@@ -77,14 +97,14 @@ RSpec.describe TagAdjustment do
       article_tags_maxed = create(:article, tags: "ruby, javascript, html, css")
       tag_adjustment = build(:tag_adjustment, user_id: admin_user.id, article_id: article_tags_maxed.id,
                                               tag_id: tag.id, tag_name: tag.name)
-      expect(tag_adjustment).to be_invalid
+      expect(tag_adjustment).not_to be_valid
     end
 
     it "does not create if removed tag not on tag_list" do
       article = create(:article, tags: tag.name)
       tag_adjustment = build(:tag_adjustment, user_id: admin_user.id, article_id: article.id,
                                               adjustment_type: "removal")
-      expect(tag_adjustment).to be_invalid
+      expect(tag_adjustment).not_to be_valid
     end
 
     it "ignores case when checking tag_list" do