[AIBUR] Prevent approval for DNP antecedents/consequents (#773)

app/controllers/tag_aliases_controller.rb

@@ -34,18 +34,16 @@ class TagAliasesController < ApplicationController
 
   def destroy
     @tag_alias = TagAlias.find(params[:id])
-    if @tag_alias.deletable_by?(CurrentUser.user)
-      @tag_alias.reject!
-      respond_with(@tag_alias, :location => tag_aliases_path)
-    else
-      access_denied
-    end
+    return access_denied unless @tag_alias.deletable_by?(CurrentUser.user)
+    @tag_alias.reject!
+    respond_with(@tag_alias, location: tag_aliases_path)
   end
 
   def approve
     @tag_alias = TagAlias.find(params[:id])
+    return access_denied unless @tag_alias.approvable_by?(CurrentUser.user)
     @tag_alias.approve!(approver: CurrentUser.user)
-    respond_with(@tag_alias, :location => tag_alias_path(@tag_alias))
+    respond_with(@tag_alias, location: tag_alias_path(@tag_alias))
   end
 
   private

app/controllers/tag_implications_controller.rb

@@ -30,31 +30,24 @@ class TagImplicationsController < ApplicationController
 
   def destroy
     @tag_implication = TagImplication.find(params[:id])
-    if @tag_implication.deletable_by?(CurrentUser.user)
-      @tag_implication.reject!
-      if @tag_implication.errors.any?
-        flash[:notice] = @tag_implication.errors.full_messages.join('; ')
+    return access_denied unless @tag_implication.deletable_by?(CurrentUser.user)
+    @tag_implication.reject!
+    respond_with(@tag_implication) do |format|
+      format.html do
+        flash[:notice] = @tag_implication.errors.any? ? @tag_implication.errors.full_messages.join("; ") : "Tag implication was deleted"
         redirect_to(tag_implications_path)
-        return
       end
-      respond_with(@tag_implication) do |format|
-        format.html do
-          flash[:notice] = "Tag implication was deleted"
-          redirect_to(tag_implications_path)
-        end
-      end
-    else
-      access_denied
     end
   end
 
   def approve
     @tag_implication = TagImplication.find(params[:id])
+    return access_denied unless @tag_implication.approvable_by?(CurrentUser.user)
     @tag_implication.approve!(approver: CurrentUser.user)
-    respond_with(@tag_implication, :location => tag_implication_path(@tag_implication))
+    respond_with(@tag_implication, location: tag_implication_path(@tag_implication))
   end
 
-private
+  private
 
   def tag_implication_params
     params.require(:tag_implication).permit(%i[antecedent_name consequent_name forum_topic_id])

app/models/bulk_update_request.rb

@@ -194,7 +194,7 @@ class BulkUpdateRequest < ApplicationRecord
 
   def initialize_attributes
     self.user_id = CurrentUser.user.id unless self.user_id
-    self.user_ip_addr = Currentuser.ip_addr unless self.user_ip_addr
+    self.user_ip_addr = CurrentUser.ip_addr unless self.user_ip_addr
     self.status = "pending"
   end
 

app/models/tag_relationship.rb

@@ -75,7 +75,7 @@ class TagRelationship < ApplicationRecord
   end
 
   def approvable_by?(user)
-    is_pending? && user.is_admin?
+    is_pending? && user.is_admin? && (user.is_bd_staff? || !(consequent_tag&.artist&.is_dnp? || antecedent_tag&.artist&.is_dnp?))
   end
 
   def deletable_by?(user)

test/unit/tag_alias_test.rb

@@ -7,8 +7,8 @@ class TagAliasTest < ActiveSupport::TestCase
     setup do
       @admin = create(:admin_user)
 
-      user = create(:user, created_at: 1.month.ago)
-      CurrentUser.user = user
+      @user = create(:user, created_at: 1.month.ago)
+      CurrentUser.user = @user
     end
 
     context "on validation" do
@@ -60,6 +60,59 @@ class TagAliasTest < ActiveSupport::TestCase
       end
     end
 
+    context "#approvable_by?" do
+      setup do
+        @mod = create(:moderator_user)
+        @bd = create(:bd_staff_user)
+        @ta = as(@user) { create(:tag_alias, status: "pending") }
+        @dnp = as(@bd) { create(:avoid_posting) }
+        @ta2 = as(@user) { create(:tag_alias, antecedent_name: @dnp.artist_name, consequent_name: "ccc", status: "pending") }
+        @ta3 = as(@user) { create(:tag_alias, antecedent_name: "ddd", consequent_name: @dnp.artist_name, status: "pending") }
+      end
+
+      should "not allow creator" do
+        assert_equal(false, @ta.approvable_by?(@user))
+      end
+
+      should "allow admins" do
+        assert_equal(true, @ta.approvable_by?(@admin))
+      end
+
+      should "now allow mods" do
+        assert_equal(false, @ta.approvable_by?(@mod))
+      end
+
+      should "not allow admins if antecedent/consequent is dnp" do
+        assert_equal(false, @ta2.approvable_by?(@admin))
+        assert_equal(false, @ta3.approvable_by?(@admin))
+      end
+
+      should "allow bd staff" do
+        assert_equal(true, @ta2.approvable_by?(@bd))
+        assert_equal(true, @ta3.approvable_by?(@bd))
+      end
+    end
+
+    context "#deletable_by?" do
+      setup do
+        @user = create(:user)
+        @mod = create(:moderator_user)
+        @ta = as(@user) { create(:tag_alias, status: "pending") }
+      end
+
+      should "allow creator" do
+        assert_equal(true, @ta.deletable_by?(@user))
+      end
+
+      should "allow admins" do
+        assert_equal(true, @ta.deletable_by?(@admin))
+      end
+
+      should "now allow mods" do
+        assert_equal(false, @ta.deletable_by?(@mod))
+      end
+    end
+
     should "populate the creator information" do
       ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb")
       assert_equal(CurrentUser.user.id, ta.creator_id)

test/unit/tag_implication_test.rb

@@ -5,9 +5,9 @@ require "test_helper"
 class TagImplicationTest < ActiveSupport::TestCase
   context "A tag implication" do
     setup do
-      user = create(:admin_user)
-      CurrentUser.user = user
-      @user = create(:user)
+      @admin = create(:admin_user)
+      CurrentUser.user = @admin
+      @user = create(:user, created_at: 1.month.ago)
     end
 
     context "on validation" do
@@ -57,6 +57,58 @@ class TagImplicationTest < ActiveSupport::TestCase
       end
     end
 
+    context "#approvable_by?" do
+      setup do
+        @mod = create(:moderator_user)
+        @bd = create(:bd_staff_user)
+        @ti = as(@user) { create(:tag_implication, status: "pending") }
+        @dnp = as(@bd) { create(:avoid_posting) }
+        @ti2 = as(@user) { create(:tag_implication, antecedent_name: @dnp.artist_name, consequent_name: "ccc", status: "pending") }
+        @ti3 = as(@user) { create(:tag_implication, antecedent_name: "ddd", consequent_name: @dnp.artist_name, status: "pending") }
+      end
+
+      should "not allow creator" do
+        assert_equal(false, @ti.approvable_by?(@user))
+      end
+
+      should "allow admins" do
+        assert_equal(true, @ti.approvable_by?(@admin))
+      end
+
+      should "now allow mods" do
+        assert_equal(false, @ti.approvable_by?(@mod))
+      end
+
+      should "not allow admins if antecedent/consequent is dnp" do
+        assert_equal(false, @ti2.approvable_by?(@admin))
+        assert_equal(false, @ti3.approvable_by?(@admin))
+      end
+
+      should "allow bd staff" do
+        assert_equal(true, @ti2.approvable_by?(@bd))
+        assert_equal(true, @ti3.approvable_by?(@bd))
+      end
+    end
+
+    context "#deletable_by?" do
+      setup do
+        @mod = create(:moderator_user)
+        @ti = as(@user) { create(:tag_implication, status: "pending") }
+      end
+
+      should "allow creator" do
+        assert_equal(true, @ti.deletable_by?(@user))
+      end
+
+      should "allow admins" do
+        assert_equal(true, @ti.deletable_by?(@admin))
+      end
+
+      should "now allow mods" do
+        assert_equal(false, @ti.deletable_by?(@mod))
+      end
+    end
+
     should "ignore pending implications when building descendant names" do
       ti2 = build(:tag_implication, antecedent_name: "b", consequent_name: "c", status: "pending")
       ti2.save