Mod tooling: add tags via /mod (#4750) [deploy]

* added tests to specs

* added view logic

* added controller/model functionality

* minor tweak to tagadjustment view

* adjusted mod view to reduce query

* added instance variables to controller

* show already adjusted tags in mod view

* removed unnecessary ifs from mods controller

* catch and display errors to tag adjustment forms

* update specs and refactor

app/controllers/moderations_controller.rb

@@ -18,6 +18,11 @@ class ModerationsController < ApplicationController
   def article
     authorize(User, :moderation_routes?)
     @moderatable = Article.find_by(slug: params[:slug])
+    @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))
     render template: "moderations/mod"
   end
 

app/controllers/tag_adjustments_controller.rb

@@ -1,14 +1,22 @@
 class TagAdjustmentsController < ApplicationController
   def create
     authorize(User, :moderation_routes?)
-    TagAdjustmentCreationService.new(
+    service = TagAdjustmentCreationService.new(
       current_user,
-      adjustment_type: "removal",
+      adjustment_type: params[:tag_adjustment][:adjustment_type],
       status: "committed",
       tag_name: params[:tag_adjustment][:tag_name],
       article_id: params[:tag_adjustment][:article_id],
       reason_for_adjustment: params[:tag_adjustment][:reason_for_adjustment],
-    ).create
+    )
+    tag_adjustment = service.tag_adjustment
+    if tag_adjustment.save
+      service.update_tags_and_notify
+    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"
+    end
     @article = Article.find(params[:tag_adjustment][:article_id])
     redirect_to "#{URI.parse(@article.path).path}/mod"
   end
@@ -18,7 +26,8 @@ class TagAdjustmentsController < ApplicationController
     tag_adjustment = TagAdjustment.find(params[:id])
     tag_adjustment.destroy
     @article = Article.find(tag_adjustment.article_id)
-    @article.update!(tag_list: @article.tag_list.add(tag_adjustment.tag_name))
+    @article.update!(tag_list: @article.tag_list.add(tag_adjustment.tag_name)) if tag_adjustment.adjustment_type == "removal"
+    @article.update!(tag_list: @article.tag_list.remove(tag_adjustment.tag_name)) if tag_adjustment.adjustment_type == "addition"
     redirect_to "#{URI.parse(@article.path).path}/mod"
   end
 end

app/models/article.rb

@@ -486,6 +486,7 @@ class Article < ApplicationRecord
       self.tag_list = [] # overwrite any existing tag with those from the front matter
       tag_list.add(front_matter["tags"], parser: ActsAsTaggableOn::TagParser)
       remove_tag_adjustments_from_tag_list
+      add_tag_adjustments_to_tag_list
     end
     self.published = front_matter["published"] if %w[true false].include?(front_matter["published"].to_s)
     self.published_at = parse_date(front_matter["date"]) if published
@@ -505,6 +506,7 @@ class Article < ApplicationRecord
   def validate_tag
     # remove adjusted tags
     remove_tag_adjustments_from_tag_list
+    add_tag_adjustments_to_tag_list
 
     # check there are not too many tags
     return errors.add(:tag_list, "exceed the maximum of 4 tags") if tag_list.size > 4
@@ -520,6 +522,11 @@ class Article < ApplicationRecord
     tag_list.remove(tags_to_remove, parser: ActsAsTaggableOn::TagParser) if tags_to_remove
   end
 
+  def add_tag_adjustments_to_tag_list
+    tags_to_add = TagAdjustment.where(article_id: id, adjustment_type: "addition", status: "committed").pluck(:tag_name)
+    tag_list.add(tags_to_add, parser: ActsAsTaggableOn::TagParser) if tags_to_add
+  end
+
   def validate_video
     return errors.add(:published, "cannot be set to true if video is still processing") if published && video_state == "PROGRESSING"
     return errors.add(:video, "cannot be added member without permission") if video.present? && user.created_at > 2.weeks.ago

app/models/tag_adjustment.rb

@@ -2,7 +2,7 @@ class TagAdjustment < ApplicationRecord
   validates :user_id, presence: true
   validates :article_id, presence: true
   validates :tag_id, presence: true
-  validates :tag_name, presence: true, uniqueness: { scope: :article_id }
+  validates :tag_name, presence: true, uniqueness: { scope: :article_id, message: "can't be an already adjusted tag" }
   validates :reason_for_adjustment, 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

app/services/tag_adjustment_creation_service.rb

@@ -4,11 +4,13 @@ class TagAdjustmentCreationService
     @tag_adjustment_params = tag_adjustment_params
   end
 
-  def create
-    @tag_adjustment = TagAdjustment.create!(creation_args)
+  def tag_adjustment
+    @tag_adjustment ||= TagAdjustment.new(creation_args)
+  end
+
+  def update_tags_and_notify
     update_article
-    Notification.send_tag_adjustment_notification(@tag_adjustment)
-    @tag_adjustment
+    Notification.send_tag_adjustment_notification(tag_adjustment)
   end
 
   private
@@ -16,6 +18,7 @@ class TagAdjustmentCreationService
   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
 
   def creation_args

app/views/moderations/mod.html.erb

@@ -91,6 +91,11 @@
   input.undo {
     background: #ff0000; /* $red */
   }
+
+  .adjustment-error {
+    text-align:center;
+    color:red;
+  }
 </style>
 
 <div class="container" style="text-align: center">
@@ -132,20 +137,27 @@
             <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>
           </ul>
         <% end %>
-        <b>Current live tags:</b> <%= @moderatable.tag_list %>
+        <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>
+          </div>
+        <% end %>
         <%= f.hidden_field :article_id, value: @moderatable.id %>
-        <%= f.text_field :tag_name, placeholder: "Tag Name" %>
-        <%= 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." %>
+        <%= 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 %>
-      <% adjustments = TagAdjustment.where(article_id: @moderatable.id, adjustment_type: "removal") %>
-      <% if adjustments.any? %>
+      <% if @removed_adjustments.any? %>
         <br /><br />
         <b>Removed tags: </b>
-        <% adjustments.each do |adjustment| %>
+        <% @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 %>
@@ -156,6 +168,52 @@
       <% 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" %>
+        <% end %>
+      <% else %>
+        <b>Unable to add new tags. 4 tags max.</b>
+      <% end %>
+      <% if @added_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 %>
+            <% end %>
+        <% end %>
+      <% end %>
+    </div>
+  <% end %>
+
   <% if @moderatable.class.name == "Article" %>
     <% if @moderatable.last_buffered.blank? %>
       <div class="tag-mod-form">

app/views/notifications/_tagadjustment.html.erb

@@ -4,20 +4,23 @@
 </div>
 <div class="content notification-content comment-content">
   <% data = notification.json_data %>
+  <% removal = data["adjustment_type"] == "removal" %>
   Hey there 👋
   <br /><br />
-  Moderators removed the
-  <b><a href="/t/<%= data["tag_name"] %>" style="font-weight: 900">#<%= data["tag_name"] %></a></b> tag from your post
+  Moderators <%= removal ? "removed" : "added" %>  the
+  <b><a href="/t/<%= data["tag_name"] %>" style="font-weight: 900">#<%= data["tag_name"] %></a></b> tag <%= removal ? "from" : "to" %> your post
   <a href="<%= data["article"]["path"] %>"><%= h(data["article"]["title"]) %></a>.
   <br /><br />
-  Certain tags have special submission guidelines, and mods need to adjust tags to best fit the community.
+  <%= removal ? "Certain tags have special submission guidelines, and mods need to adjust tags to best fit the community." : "Mods felt this tag represented your article." %>
   <br /><br />
   <div class="comment-text">
     <div style="font-size:0.8em;margin-top: -3px;font-weight: 900;margin-bottom: 5px;">Reason for action</div>
     <%= data["reason_for_adjustment"] %>
   </div>
   <br />
-  Check out <a href="/tags">other popular tags</a> which may be more appropriate.
+  <% if removal %>
+    Check out <a href="/tags">other popular tags</a> which may be more appropriate
+  <% end %>
   <br><br />
   Thanks for being part of DEV! If you feel like this mod action was a mistake, feel free to contact
   <a href="mailto:<%= ApplicationConfig["DEFAULT_SITE_EMAIL"] %>"><%= ApplicationConfig["DEFAULT_SITE_EMAIL"] %></a>. 🤗

spec/requests/tag_adjustments_spec.rb

@@ -8,7 +8,8 @@ RSpec.describe "TagAdjustments", type: :request do
     {
       tag_name: tag.name,
       article_id: article.id,
-      reason_for_adjustment: "Test #{rand(100)}"
+      reason_for_adjustment: "Test #{rand(100)}",
+      adjustment_type: "removal"
     }
   end
 
@@ -48,6 +49,43 @@ RSpec.describe "TagAdjustments", type: :request do
     end
   end
 
+  describe "POST /tag_adjustments with adjustment_type addition" do
+    before do
+      tag_adjustment_params[:adjustment_type] = "addition"
+      user.add_role(:tag_moderator, tag)
+      user.add_role(:trusted)
+      sign_in user
+      post "/tag_adjustments", params: {
+        tag_adjustment: tag_adjustment_params,
+        article_id: article.id
+      }
+    end
+
+    context "when an article doesn't use front matter" do
+      let(:article) { Article.create(user: user, title: "something", body_markdown: "blah blah #{rand(100)}", tag_list: "yoyo, bobo", published: true) }
+
+      it "adds the tag" do
+        expect(article.reload.tag_list.include?(tag.name)).to be true
+      end
+
+      it "keeps the other tags" do
+        expect(article.reload.tag_list.include?("yoyo")).to be true
+      end
+    end
+
+    context "when an article uses front matter" do
+      let(:article) { create(:article, user: user, body_markdown: "---\ntitle: Hellohnnnn#{rand(1000)}\npublished: true\ntags: heyheyhey\n---\n\nHello") }
+
+      it "adds the tag" do
+        expect(article.reload.tag_list.include?(tag.name)).to be true
+      end
+
+      it "keeps the other tags" do
+        expect(article.reload.tag_list.include?("heyheyhey")).to be true
+      end
+    end
+  end
+
   describe "DELETE /tag_adjustments/:id" do
     let(:tag_adjustment) { create(:tag_adjustment, article_id: article.id, user: user, tag: tag) }
 
@@ -80,4 +118,37 @@ RSpec.describe "TagAdjustments", type: :request do
       end
     end
   end
+
+  describe "DELETE /tag_adjustments/:id with adjustment_type addition" do
+    let(:tag_adjustment) { create(:tag_adjustment, article_id: article.id, user: user, tag: tag, adjustment_type: "addition") }
+
+    before do
+      user.add_role(:admin)
+      user.add_role(:trusted)
+      tag_adjustment
+      sign_in user
+    end
+
+    context "when an article doesn't use front matter" do
+      let(:article) { Article.create(user: user, title: "something", body_markdown: "blah blah #{rand(100)}", tag_list: "yoyo, bobo", published: true) }
+
+      it "removes the added tag" do
+        delete "/tag_adjustments/#{tag_adjustment.id}", params: {
+          id: tag_adjustment.id
+        }
+        expect(article.tag_list).not_to include tag.name
+      end
+    end
+
+    context "when an article uses front matter" do
+      let(:article) { create(:article, user: user, body_markdown: "---\ntitle: Hellohnnnn#{rand(1000)}\npublished: true\ntags: heyheyhey\n---\n\nHello") }
+
+      it "removes the added tag" do
+        delete "/tag_adjustments/#{tag_adjustment.id}", params: {
+          id: tag_adjustment.id
+        }
+        expect(article.tag_list).not_to include tag.name
+      end
+    end
+  end
 end

spec/services/notifications/tag_adjustment_notification/send_spec.rb

@@ -36,6 +36,13 @@ RSpec.describe Notifications::TagAdjustmentNotification::Send, type: :service do
     expect(json["adjustment_type"]). to eq "removal"
   end
 
+  it "tests JSON data for addition" do
+    tag_adjustment.adjustment_type = "addition"
+    json = notification.json_data
+    expect(json["article"]["title"]).to start_with("Hello")
+    expect(json["adjustment_type"]). to eq "addition"
+  end
+
   specify "notification to be inserted on DB" do
     expect do
       described_class.call(tag_adjustment)

spec/services/tag_adjustment_creation_service_spec.rb

@@ -2,37 +2,63 @@ require "rails_helper"
 
 RSpec.describe TagAdjustmentCreationService, 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
+  def create_service(type)
     described_class.new(
       user,
-      adjustment_type: "removal",
+      adjustment_type: type,
       status: "committed",
       tag_name: tag.name,
       article_id: article.id,
       reason_for_adjustment: "Test",
-    ).create
+    )
   end
 
   before do
     user.add_role(:tag_moderator, tag)
   end
 
-  it "creates tag adjustment" do
-    tag_adjustment = create_tag_adjustment
+  describe "creates tag adjustment" do
+    it "with adjustment_type removal" do
+      tag_adjustment = create_service("removal").tag_adjustment
+      tag_adjustment.save
+      expect(tag_adjustment).to be_valid
+      expect(tag_adjustment.tag_id).to eq(tag.id)
+      expect(tag_adjustment.status).to eq("committed")
+    end
 
-    expect(tag_adjustment).to be_valid
-    expect(tag_adjustment.tag_id).to eq(tag.id)
-    expect(tag_adjustment.status).to eq("committed")
+    it "with adjustment_type addition" do
+      tag_adjustment = create_service("addition").tag_adjustment
+      tag_adjustment.save
+      expect(tag_adjustment).to be_valid
+      expect(tag_adjustment.tag_id).to eq(tag.id)
+      expect(tag_adjustment.status).to eq("committed")
+    end
   end
 
-  it "creates notification" do
-    perform_enqueued_jobs do
-      tag_adjustment = create_tag_adjustment
-      expect(Notification.last.user_id).to eq(article.user_id)
-      expect(Notification.last.json_data["adjustment_type"]).to eq(tag_adjustment.adjustment_type)
+  describe "creates notification" do
+    it "with adjustment_type removal" do
+      perform_enqueued_jobs do
+        service = create_service("removal")
+        tag_adjustment = service.tag_adjustment
+        tag_adjustment.save
+        service.update_tags_and_notify
+        expect(Notification.last.user_id).to eq(article.user_id)
+        expect(Notification.last.json_data["adjustment_type"]).to eq(tag_adjustment.adjustment_type)
+      end
+    end
+
+    it "with adjustment_type addition" do
+      perform_enqueued_jobs do
+        service = create_service("addition")
+        tag_adjustment = service.tag_adjustment
+        tag_adjustment.save
+        service.update_tags_and_notify
+        expect(Notification.last.user_id).to eq(article.user_id)
+        expect(Notification.last.json_data["adjustment_type"]).to eq(tag_adjustment.adjustment_type)
+      end
     end
   end
 end

spec/services/tag_adjustment_update_service_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe TagAdjustmentUpdateService, type: :service do
       tag_name: tag.name,
       article_id: article.id,
       reason_for_adjustment: "reasons",
-    ).create
+    )
   end
 
   before do
@@ -21,7 +21,8 @@ RSpec.describe TagAdjustmentUpdateService, type: :service do
   end
 
   it "creates tag adjustment" do
-    tag_adjustment = create_tag_adjustment
+    tag_adjustment = create_tag_adjustment.tag_adjustment
+    tag_adjustment.save
     described_class.new(tag_adjustment, status: "resolved").update
 
     expect(tag_adjustment).to be_valid