From 82f5bb9cdda01b3c9fd47d5f41b91a504893b977 Mon Sep 17 00:00:00 2001
From: Spencer <pagespence@yahoo.com>
Date: Fri, 13 Dec 2019 10:02:45 -0800
Subject: [PATCH] Mod tooling: add tags via /mod refactor (#5017)

* mod view form changes

* add error validation for removal tag

* add error validation to tag_list count

* re-render tag_adjustment form errors

* refactor admin delete adjusted tags form

* add prompt to tag adjustment form select

* front end validations to add/remove tag_adjustment

* allow tag_mod to undo own tag adjustment

* update/add specs

* update send spec

* update tag adjustment spec
---
 app/controllers/moderations_controller.rb     |   6 +-
 app/controllers/tag_adjustments_controller.rb |  15 +-
 app/models/tag_adjustment.rb                  |   6 +
 .../tag_adjustment_creation_service.rb        |   5 +-
 app/views/moderations/mod.html.erb            | 141 +++++++++---------
 spec/factories/tag_adjustments.rb             |   2 +-
 spec/models/tag_adjustment_spec.rb            | 104 ++++++++-----
 .../tag_adjustment_notification/send_spec.rb  |  10 +-
 .../tag_adjustment_update_service_spec.rb     |   2 +-
 9 files changed, 167 insertions(+), 124 deletions(-)

diff --git a/app/controllers/moderations_controller.rb b/app/controllers/moderations_controller.rb
index a40e1584f..d8594de94 100644
--- a/app/controllers/moderations_controller.rb
+++ b/app/controllers/moderations_controller.rb
@@ -17,12 +17,12 @@ class ModerationsController < ApplicationController
 
   def article
     authorize(User, :moderation_routes?)
+    @tag_adjustment = TagAdjustment.new
     @moderatable = Article.find_by(slug: params[:slug])
+    @tag_moderator_tags = Tag.with_role(:tag_moderator, current_user)
     @adjustments = TagAdjustment.where(article_id: @moderatable.id)
-    @removed_adjustments = @adjustments.filter { |a| a.adjustment_type == "removal" }
-    @added_adjustments = @adjustments.filter { |a| a.adjustment_type == "addition" }
     @already_adjusted_tags = @adjustments.map(&:tag_name).join(", ")
-    @allowed_to_add = @moderatable.class.name == "Article" && (current_user.has_role?(:super_admin) || current_user.has_role?(:tag_moderator, :any))
+    @allowed_to_adjust = @moderatable.class.name == "Article" && (current_user.has_role?(:super_admin) || @tag_moderator_tags.any?)
     render template: "moderations/mod"
   end
 
diff --git a/app/controllers/tag_adjustments_controller.rb b/app/controllers/tag_adjustments_controller.rb
index 8ed51359d..e3a7d16bd 100644
--- a/app/controllers/tag_adjustments_controller.rb
+++ b/app/controllers/tag_adjustments_controller.rb
@@ -10,15 +10,20 @@ class TagAdjustmentsController < ApplicationController
       reason_for_adjustment: params[:tag_adjustment][:reason_for_adjustment],
     )
     tag_adjustment = service.tag_adjustment
+    article = service.article
     if tag_adjustment.save
       service.update_tags_and_notify
+      redirect_to "#{URI.parse(article.path).path}/mod"
     else
-      errors = tag_adjustment.errors.full_messages.join(", ")
-      flash[:error_removal] = errors if tag_adjustment.adjustment_type == "removal"
-      flash[:error_addition] = errors if tag_adjustment.adjustment_type == "addition"
+      authorize(User, :moderation_routes?)
+      @tag_adjustment = tag_adjustment
+      @moderatable = article
+      @tag_moderator_tags = Tag.with_role(:tag_moderator, current_user)
+      @adjustments = TagAdjustment.where(article_id: article.id)
+      @already_adjusted_tags = @adjustments.map(&:tag_name).join(", ")
+      @allowed_to_adjust = @moderatable.class.name == "Article" && (current_user.any_admin? || @tag_moderator_tags.any?)
+      render template: "moderations/mod"
     end
-    @article = Article.find(params[:tag_adjustment][:article_id])
-    redirect_to "#{URI.parse(@article.path).path}/mod"
   end
 
   def destroy
diff --git a/app/models/tag_adjustment.rb b/app/models/tag_adjustment.rb
index bb99fea21..1218ac425 100644
--- a/app/models/tag_adjustment.rb
+++ b/app/models/tag_adjustment.rb
@@ -8,6 +8,7 @@ class TagAdjustment < ApplicationRecord
   validates :status, inclusion: { in: %w[committed pending committed_and_resolvable resolved] }, presence: true
   has_many :notifications, as: :notifiable, inverse_of: :notifiable, dependent: :delete_all
   validate :user_permissions
+  validate :article_tag_list
 
   belongs_to :user
   belongs_to :tag
@@ -26,4 +27,9 @@ class TagAdjustment < ApplicationRecord
       user.has_role?(:admin) ||
       user.has_role?(:super_admin)
   end
+
+  def article_tag_list
+    errors.add(:tag_id, "selected for removal is not a current live tag.") if adjustment_type == "removal" && article.tag_list.exclude?(tag_name)
+    errors.add(:base, "4 tags max per article.") if adjustment_type == "addition" && article.tag_list.count > 3
+  end
 end
diff --git a/app/services/tag_adjustment_creation_service.rb b/app/services/tag_adjustment_creation_service.rb
index 148ddc5d6..3d35b12d2 100644
--- a/app/services/tag_adjustment_creation_service.rb
+++ b/app/services/tag_adjustment_creation_service.rb
@@ -8,6 +8,10 @@ class TagAdjustmentCreationService
     @tag_adjustment ||= TagAdjustment.new(creation_args)
   end
 
+  def article
+    @article ||= Article.find(creation_args[:article_id])
+  end
+
   def update_tags_and_notify
     update_article
     Notification.send_tag_adjustment_notification(tag_adjustment)
@@ -16,7 +20,6 @@ class TagAdjustmentCreationService
   private
 
   def update_article
-    article = Article.find(creation_args[:article_id])
     article.update!(tag_list: article.tag_list.remove(@tag_adjustment.tag_name)) if @tag_adjustment.adjustment_type == "removal"
     article.update!(tag_list: article.tag_list.add(@tag_adjustment.tag_name)) if @tag_adjustment.adjustment_type == "addition"
   end
diff --git a/app/views/moderations/mod.html.erb b/app/views/moderations/mod.html.erb
index 1a630b3f0..ad41d5f8d 100644
--- a/app/views/moderations/mod.html.erb
+++ b/app/views/moderations/mod.html.erb
@@ -92,9 +92,34 @@
     background: #ff0000; /* $red */
   }
 
-  .adjustment-error {
-    text-align:center;
-    color:red;
+  .tag-mod-form input[type=radio] {
+    position: absolute;
+  }
+
+  .tag-mod-form input[type=radio] + label {
+    display: inline-block;
+    cursor: pointer;
+    font-weight: bold;
+    padding-left: 25px;
+    padding-right: 25px;
+    font-size: 20px;
+    margin: auto;
+    margin-bottom: 15px;
+  }
+
+  .tag-mod-form input[type=radio]:checked + label{
+    color: white;
+    background: #0045ff;
+  }
+
+  .tag-mod-form select {
+    display: block;
+    font-size: 20px;
+    padding: 12px;
+    font-weight: bold;
+    margin: auto;
+    border: solid 2px;
+    margin-top: 5px;
   }
 </style>
 
@@ -125,90 +150,64 @@
       <br><br>The DEV team will be notified about vomits and take appropriate action, but leaving a level-headed comment addressing the behavior is welcome if you feel up for it.
     </p>
   <% end %>
-  <% is_mod = (@moderatable.tag_list.select { |t| current_user.has_role?(:tag_moderator, Tag.find_by(name: t)) }).any? if @moderatable.class.name == "Article" %>
-  <% if @moderatable.class.name == "Article" && (current_user.has_role?(:super_admin) || is_mod) %>
-    <a href="/community-moderation" class="mod-link">Community moderation guide</a>
+  <% if @allowed_to_adjust %>
     <div class="tag-mod-form">
-      <%= form_for(TagAdjustment.new) do |f| %>
-        <h2> Remove Inappropriate Tags</h2>
+      <h2>Tag Adjustment</h2>
+      <%= form_for(@tag_adjustment) do |f| %>
+        <h3> Add or Remove Tags</h3>
         <% unless current_user.has_role?(:super_admin) %>
           <ul>
-            <li>You can only remove tags that you moderate.</li>
-            <li>Only remove tags which are tagged innapropriately.</li>
-            <li>Use negative reactions if tag is appropriate but quality is low.</li>
-            <li>Always be friendly in your tag removal reason.</li>
-            <li>Only remove tags which haven't been adjusted already.</li>
+            <li>You can only adjust tags that you moderate.</li>
+            <li>Only adjust tags that are appropriate.</li>
+            <li>Always be friendly in your tag adjustment reason.</li>
+            <li>Only adjust tags which haven't been adjusted already.</li>
+            <li>Each article can have a maximun of four tags</li>
           </ul>
         <% end %>
         <b>Current live tags:</b> <%= @moderatable.tag_list %><br>
         <b>Already adjusted tags:</b> <%= @already_adjusted_tags %>
         <br /><br />
-        <% if flash[:error_removal].present? %>
-          <div class="adjustment-error">
-            <p>Error: <%= flash[:error_removal] %></p><br>
+        <% if @tag_adjustment.errors.any? %>
+          <div id="error_explanation">
+            <h2><%= pluralize(@tag_adjustment.errors.count, "error") %> prohibited this block from being saved:</h2>
+            <ul>
+              <% @tag_adjustment.errors.full_messages.each do |message| %>
+                <li><%= message %></li>
+              <% end %>
+            </ul>
           </div>
+          <br><br><br><br>
         <% end %>
         <%= f.hidden_field :article_id, value: @moderatable.id %>
-        <%= f.hidden_field :adjustment_type, value: "removal" %>
-        <%= f.text_field :tag_name, placeholder: "Tag Name", required: true %>
-        <%= f.text_area :reason_for_adjustment, placeholder: "Reason for Removal (Be super kind) - Only the reason is needed, the notification will take care of the rest.", required: true %>
-        <%= f.submit "Remove Tag" %>
-      <% end %>
-      <% if @removed_adjustments.any? %>
-        <br /><br />
-        <b>Removed tags: </b>
-        <% @removed_adjustments.each do |adjustment| %>
-          <% if current_user.any_admin? %>
-              <%= form_for(adjustment, url: "/tag_adjustments/#{adjustment.id}", html: { method: :delete, onsubmit: "return confirm('Are you sure you want to undo the removal of the #{adjustment.tag_name} tag?')" }) do |f| %>
-                <%= adjustment.tag_name %>
-                <%= f.submit "X", id: "undo" %>
-              <% end %>
-            <% end %>
+        <% if current_user.any_admin? || @tag_moderator_tags.any? { |tag| @moderatable.tag_list.include?(tag.name) } %>
+          <%= f.radio_button :adjustment_type, "removal", required: true %>
+          <%= f.label :adjustment_type, "Remove", value: "removal" %>
         <% end %>
-      <% end %>
-    </div>
-  <% end %>
-
-  <% if @allowed_to_add %>
-    <div class="tag-mod-form">
-      <h2> Add Appropriate Tags</h2>
-      <% if @moderatable.tag_list.count < 4 %>
-        <%= form_for(TagAdjustment.new) do |f| %>
-          <% unless current_user.has_role?(:super_admin) %>
-            <ul>
-              <li>You can only add tags that you moderate.</li>
-              <li>Only add tags that are appropriate.</li>
-              <li>Always be friendly in your tag adding reason.</li>
-              <li>Only add tags which haven't been adjusted already.</li>
-            </ul>
-          <% end %>
-          <b>Current live tags:</b> <%= @moderatable.tag_list %><br>
-          <b>Already adjusted tags:</b> <%= @already_adjusted_tags %>
-          <br /><br />
-          <% if flash[:error_addition].present? %>
-            <div class="adjustment-error">
-              <p>Error: <%= flash[:error_addition] %></p><br>
-            </div>
-          <% end %>
-          <%= f.hidden_field :article_id, value: @moderatable.id %>
-          <%= f.hidden_field :adjustment_type, value: "addition" %>
-          <%= f.text_field :tag_name, placeholder: "Tag Name - only add one at a time", required: true %>
-          <%= f.text_area :reason_for_adjustment, placeholder: "Reason for addition (Be super kind) - Only the reason is needed, the notification will take care of the rest.", required: true %>
-          <%= f.submit "Add Tag" %>
+        <% if @moderatable.tag_list.count < 4 %>
+          <%= f.radio_button :adjustment_type, "addition", required: true %>
+          <%= f.label :adjustment_type, "Add", value: "addition" %>
         <% end %>
-      <% else %>
-        <b>Unable to add new tags. 4 tags max.</b>
+        <br />
+        <% if current_user.has_role?(:super_admin) %>
+          <%= f.text_field :tag_name, placeholder: "Tag Name", required: true %>
+        <% else %>
+          <%= f.select :tag_name, @tag_moderator_tags, { prompt: "Select Tag" }, required: true %>
+          <br />
+        <% end %>
+        <%= f.text_area :reason_for_adjustment, placeholder: "Reason for adjustment (Be super kind) - Only the reason is needed, the notification will take care of the rest.", required: true %>
+        <%= f.submit "Submit Tag Adjustment" %>
       <% end %>
-      <% if @added_adjustments.any? %>
+      <% if @adjustments.any? %>
         <br /><br />
-        <b>Added tags: </b>
-        <% @added_adjustments.each do |adjustment| %>
-          <% if current_user.any_admin? %>
-              <%= form_for(adjustment, url: "/tag_adjustments/#{adjustment.id}", html: { method: :delete, onsubmit: "return confirm('Are you sure you want to undo the addition of the #{adjustment.tag_name} tag?')" }) do |f| %>
-                <%= adjustment.tag_name %>
-                <%= f.submit "X", id: "undo" %>
-              <% end %>
+        <b>Adjusted Tags:</b><br /><br />
+        <% @adjustments.each do |adjustment| %>
+          <% if current_user.any_admin? || adjustment.user_id == current_user.id %>
+            <%= form_for(adjustment, url: "/tag_adjustments/#{adjustment.id}", html: { method: :delete, onsubmit: "return confirm('Are you sure you want to undo the #{adjustment.adjustment_type} of the #{adjustment.tag_name} tag?')" }) do |f| %>
+              <%= adjustment.adjustment_type == "removal" ? "Removed" : "Added" %>: <%= adjustment.tag_name %>
+              <%= f.submit "X", id: "undo" %>
             <% end %>
+          <% end %>
+          <br />
         <% end %>
       <% end %>
     </div>
diff --git a/spec/factories/tag_adjustments.rb b/spec/factories/tag_adjustments.rb
index cf6f56b26..af3bc1e67 100644
--- a/spec/factories/tag_adjustments.rb
+++ b/spec/factories/tag_adjustments.rb
@@ -4,7 +4,7 @@ FactoryBot.define do
     article_id            { 1 }
     tag_id                { 1 }
     tag_name              { "NOTHING" }
-    adjustment_type       { "removal" }
+    adjustment_type       { "addition" }
     reason_for_adjustment { "reason #{rand(10_000)}" }
     status                { "committed" }
   end
diff --git a/spec/models/tag_adjustment_spec.rb b/spec/models/tag_adjustment_spec.rb
index 296534921..67d14878d 100644
--- a/spec/models/tag_adjustment_spec.rb
+++ b/spec/models/tag_adjustment_spec.rb
@@ -1,57 +1,87 @@
 require "rails_helper"
 
 RSpec.describe TagAdjustment, type: :model do
-  let_it_be(:article) { create(:article) }
-  let_it_be(:admin_user) { create(:user, :admin) }
-  let_it_be(:regular_user) { create(:user) }
+  before do
+    mod_user.add_role(:tag_moderator, tag)
+    admin_user.add_role(:admin)
+  end
+
+  let(:article) { create(:article, tags: nil) }
+  let(:tag) { create(:tag) }
+  let(:admin_user) { create(:user) }
+  let(:mod_user) { create(:user) }
+  let(:regular_user) { create(:user) }
 
   it { is_expected.to validate_presence_of(:user_id) }
   it { is_expected.to validate_presence_of(:article_id) }
   it { is_expected.to validate_presence_of(:tag_id) }
   it { is_expected.to validate_presence_of(:tag_name) }
   it { is_expected.to validate_presence_of(:adjustment_type) }
-  it { is_expected.to validate_inclusion_of(:adjustment_type).in_array(%w[removal addition]) }
   it { is_expected.to validate_presence_of(:status) }
-
-  it do
-    # rubocop:disable RSpec/NamedSubject
-    expect(subject).to validate_inclusion_of(:status).in_array(
-      %w[committed pending committed_and_resolvable resolved],
-    )
-    # rubocop:enable RSpec/NamedSubject
-  end
-
   it { is_expected.to have_many(:notifications).dependent(:delete_all) }
 
-  describe "validations" do
-    let(:tag) { create(:tag) }
-    let(:mod_user) { create(:user) }
+  describe "privileges" do
+    it "allows tag mods to create for their tags" do
+      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
 
-    describe "privileges" do
-      before do
-        mod_user.add_role(:tag_moderator, tag)
-      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
+    end
 
-      it "allows tag mods to create for their tags" do
-        tag_adjustment = build(:tag_adjustment, user: mod_user, article: article, tag: tag)
-        expect(tag_adjustment).to be_valid
-      end
+    it "allows admins to create 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 tag mods to create for other tags" do
-        another_tag = create(:tag)
-        tag_adjustment = build(:tag_adjustment, user: mod_user, article: article, tag: another_tag)
-        expect(tag_adjustment).to be_invalid
-      end
+    it "does not allow normal users to create 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
+    end
+  end
 
-      it "allows admins to create for any tags" do
-        tag_adjustment = build(:tag_adjustment, user: admin_user, article: article, tag: tag)
-        expect(tag_adjustment).to be_valid
-      end
+  describe "allowed attribute states" do
+    it "allows addition adjustment_types" do
+      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 "does not allow normal users to create for any tags" do
-        tag_adjustment = build(:tag_adjustment, user: regular_user, article: article, tag: tag)
-        expect(tag_adjustment).to be_invalid
-      end
+    it "allows removal adjustment_types" do
+      article = create(:article, tags: tag.name)
+      tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id, tag_name: tag.name, adjustment_type: "removal")
+      expect(tag_adjustment).to be_valid
+    end
+
+    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
+    end
+
+    it "allows proper status" do
+      tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id, status: "committed")
+      expect(tag_adjustment).to be_valid
+    end
+
+    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
+    end
+  end
+
+  describe "validates article tag_list" do
+    it "does not allow addition on articles with 4 tags" do
+      article_tags_maxed = create(:article)
+      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
+    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
     end
   end
 end
diff --git a/spec/services/notifications/tag_adjustment_notification/send_spec.rb b/spec/services/notifications/tag_adjustment_notification/send_spec.rb
index d57c0a701..75da1b8d6 100644
--- a/spec/services/notifications/tag_adjustment_notification/send_spec.rb
+++ b/spec/services/notifications/tag_adjustment_notification/send_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Notifications::TagAdjustmentNotification::Send, type: :service do
   let(:article) { create(:article, title: "My title", user: user2, body_markdown: "---\ntitle: Hellohnnnn#{rand(1000)}\npublished: true\ntags: heyheyhey,#{tag.name}\n---\n\nHello") }
   let(:tag) { create(:tag) }
   let(:mod_user) { create(:user) }
-  let(:tag_adjustment) { create(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id, adjustment_type: "removal") }
+  let(:tag_adjustment) { create(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id, adjustment_type: "addition") }
   let(:notification) { described_class.call(tag_adjustment) }
 
   before do
@@ -33,14 +33,14 @@ RSpec.describe Notifications::TagAdjustmentNotification::Send, type: :service do
   it "tests JSON data" do
     json = notification.json_data
     expect(json["article"]["title"]).to start_with("Hello")
-    expect(json["adjustment_type"]). to eq "removal"
+    expect(json["adjustment_type"]). to eq "addition"
   end
 
-  it "tests JSON data for addition" do
-    tag_adjustment.adjustment_type = "addition"
+  it "tests JSON data for removal" do
+    tag_adjustment.adjustment_type = "removal"
     json = notification.json_data
     expect(json["article"]["title"]).to start_with("Hello")
-    expect(json["adjustment_type"]). to eq "addition"
+    expect(json["adjustment_type"]). to eq "removal"
   end
 
   specify "notification to be inserted on DB" do
diff --git a/spec/services/tag_adjustment_update_service_spec.rb b/spec/services/tag_adjustment_update_service_spec.rb
index 05576b429..21a4d3bfc 100644
--- a/spec/services/tag_adjustment_update_service_spec.rb
+++ b/spec/services/tag_adjustment_update_service_spec.rb
@@ -2,7 +2,7 @@ require "rails_helper"
 
 RSpec.describe TagAdjustmentUpdateService, type: :service do
   let(:user) { create(:user) }
-  let(:article) { create(:article) }
+  let(:article) { create(:article, tags: tag.name) }
   let(:tag) { create(:tag) }
 
   def create_tag_adjustment