Merge pull request #5104 from nonamethanks/no-wiki-no-deprecation

Tags: don't allow deprecation of tags without wiki

app/logical/bulk_update_request_processor.rb

@@ -131,13 +131,19 @@ class BulkUpdateRequestProcessor
 
         when :deprecate
           tag = Tag.find_by_name(args[0])
-          if tag.is_deprecated?
+          if tag.nil?
+            errors.add(:base, "Can't deprecate #{args[0]} (tag doesn't exist)")
+          elsif tag.is_deprecated?
             errors.add(:base, "Can't deprecate #{args[0]} (tag is already deprecated)")
+          elsif tag.wiki_page.blank?
+            errors.add(:base, "Can't deprecate #{args[0]} (tag must have a wiki page)")
           end
 
         when :undeprecate
           tag = Tag.find_by_name(args[0])
-          if !tag.is_deprecated?
+          if tag.nil?
+            errors.add(:base, "Can't undeprecate #{args[0]} (tag doesn't exist)")
+          elsif !tag.is_deprecated?
             errors.add(:base, "Can't undeprecate #{args[0]} (tag is not deprecated)")
           end
 

app/policies/tag_policy.rb

@@ -8,13 +8,14 @@ class TagPolicy < ApplicationPolicy
   end
 
   def can_change_deprecated_status?
+    return false if record.wiki_page.blank? && !record.is_deprecated?
     user.is_admin? || (record.post_count == 0 && !record.is_deprecated?)
   end
 
   def permitted_attributes
-    [
+    permitted = []
-      (:category if can_change_category?),
+    permitted << :category if can_change_category?
-      (:is_deprecated if can_change_deprecated_status?),
+    permitted << :is_deprecated if can_change_deprecated_status?
-    ].compact
+    permitted
   end
 end

test/functional/tags_controller_test.rb

@@ -124,7 +124,13 @@ class TagsControllerTest < ActionDispatch::IntegrationTest
           @deprecated_tag = create(:tag, name: "bad_tag", category: Tag.categories.general, post_count: 0, is_deprecated: true)
           @nondeprecated_tag = create(:tag, name: "log", category: Tag.categories.general, post_count: 0)
           @normal_tag = create(:tag, name: "random", category: Tag.categories.general, post_count: 1000)
-          @normal_user = create(:user)
+          create(:wiki_page, title: "bad_tag", body: "[[bad_tag]]")
+          create(:wiki_page, title: "log", body: "[[log]]")
+          create(:wiki_page, title: "random", body: "[[random]]")
+
+          @tag_without_wiki = create(:tag, name: "no_wiki", category: Tag.categories.general, post_count: 0)
+
+          @normal_user = create(:member_user)
           @admin = create(:admin_user)
         end
 
@@ -132,35 +138,42 @@ class TagsControllerTest < ActionDispatch::IntegrationTest
           put_auth tag_path(@deprecated_tag), @normal_user, params: {tag: { is_deprecated: false }}
 
           assert_response 403
-          assert(true, @deprecated_tag.reload.is_deprecated?)
+          assert_equal(true, @deprecated_tag.reload.is_deprecated?)
         end
 
         should "remove the deprecated status if the user is admin" do
           put_auth tag_path(@deprecated_tag), @admin, params: {tag: { is_deprecated: false }}
 
           assert_redirected_to @deprecated_tag
-          assert(false, @deprecated_tag.reload.is_deprecated?)
+          assert_equal(false, @deprecated_tag.reload.is_deprecated?)
         end
 
         should "allow marking a tag as deprecated if it's empty" do
           put_auth tag_path(@nondeprecated_tag), @normal_user, params: {tag: { is_deprecated: true }}
 
           assert_redirected_to @nondeprecated_tag
-          assert(true, @nondeprecated_tag.reload.is_deprecated?)
+          assert_equal(true, @nondeprecated_tag.reload.is_deprecated?)
         end
 
         should "not allow marking a tag as deprecated if it's not empty" do
           put_auth tag_path(@normal_tag), @normal_user, params: {tag: { is_deprecated: true }}
 
           assert_response 403
-          assert(false, @normal_tag.reload.is_deprecated?)
+          assert_equal(false, @normal_tag.reload.is_deprecated?)
         end
 
         should "allow admins to mark tags as deprecated" do
           put_auth tag_path(@normal_tag), @admin, params: {tag: { is_deprecated: true }}
 
           assert_redirected_to @normal_tag
-          assert(true, @normal_tag.reload.is_deprecated?)
+          assert_equal(true, @normal_tag.reload.is_deprecated?)
+        end
+
+        should "not allow deprecation of a tag with no wiki" do
+          put_auth tag_path(@tag_without_wiki), @user, params: {tag: { is_deprecated: true }}
+
+          assert_response 403
+          assert_equal(false, @tag_without_wiki.reload.is_deprecated?)
         end
       end
 

test/unit/bulk_update_request_test.rb

@@ -446,34 +446,6 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
         end
       end
 
-      context "the deprecate command" do
-        should "deprecate the tag" do
-          @tag = create(:tag, name: "silver_hair")
-          @bur = create_bur!("deprecate silver_hair", @admin)
-
-          assert_equal(true, @tag.reload.is_deprecated?)
-        end
-
-        should "remove implications" do
-          @ti1 = create(:tag_implication, antecedent_name: "silver_hair", consequent_name: "old_woman")
-          @ti2 = create(:tag_implication, antecedent_name: "my_literal_dog", consequent_name: "silver_hair")
-          @bur = create_bur!("deprecate silver_hair", @admin)
-
-          assert_equal("deleted", @ti1.reload.status)
-          assert_equal("deleted", @ti2.reload.status)
-          assert_equal("approved", @bur.reload.status)
-        end
-      end
-
-      context "the undeprecate command" do
-        should "undeprecate the tag" do
-          @tag = create(:tag, name: "silver_hair", is_deprecated: true)
-          @bur = create_bur!("undeprecate silver_hair", @admin)
-
-          assert_equal(false, @tag.reload.is_deprecated?)
-        end
-      end
-
       context "that contains a mass update followed by an alias" do
         should "make the alias take effect after the mass update" do
           @p1 = create(:post, tag_string: "maid_dress")
@@ -578,6 +550,42 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
       end
     end
 
+    context "using the deprecate command" do
+      should "deprecate the tag" do
+        @tag = create(:tag, name: "bad_tag")
+        create(:wiki_page, title: "bad_tag")
+        @bur = create_bur!("deprecate bad_tag", @admin)
+
+        assert_equal(true, @tag.reload.is_deprecated?)
+      end
+
+      should "remove implications" do
+        @ti1 = create(:tag_implication, antecedent_name: "grey_hair", consequent_name: "old_woman")
+        @ti2 = create(:tag_implication, antecedent_name: "my_literal_dog", consequent_name: "grey_hair")
+        @wiki = create(:wiki_page, title: "grey_hair")
+        @bur = create_bur!("deprecate grey_hair", @admin)
+
+        assert_equal("deleted", @ti1.reload.status)
+        assert_equal("deleted", @ti2.reload.status)
+        assert_equal("approved", @bur.reload.status)
+      end
+
+      should "not work for tags without a wiki page" do
+        @bur = build(:bulk_update_request, script: "deprecate no_wiki")
+        assert_equal(false, @bur.valid?)
+        assert_equal(["Can't deprecate no_wiki (tag must have a wiki page)"], @bur.errors[:base])
+      end
+    end
+
+    context "using the undeprecate command" do
+      should "undeprecate the tag" do
+        @tag = create(:tag, name: "silver_hair", is_deprecated: true)
+        @bur = create_bur!("undeprecate silver_hair", @admin)
+
+        assert_equal(false, @tag.reload.is_deprecated?)
+      end
+    end
+
     context "on approval" do
       setup do
         @post = create(:post, tag_string: "foo aaa")