From 11281d6f58f3d6d86e90cd1741e62c9e6a5ed7e0 Mon Sep 17 00:00:00 2001 From: nonamethanks Date: Sat, 9 Apr 2022 16:26:13 +0200 Subject: [PATCH] Tags: don't allow deprecation of tags without wiki --- app/logical/bulk_update_request_processor.rb | 10 ++- app/policies/tag_policy.rb | 9 +-- test/functional/tags_controller_test.rb | 25 ++++++-- test/unit/bulk_update_request_test.rb | 64 +++++++++++--------- 4 files changed, 68 insertions(+), 40 deletions(-) diff --git a/app/logical/bulk_update_request_processor.rb b/app/logical/bulk_update_request_processor.rb index 7e3a8e7b2..ad84d4e72 100644 --- a/app/logical/bulk_update_request_processor.rb +++ b/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 diff --git a/app/policies/tag_policy.rb b/app/policies/tag_policy.rb index e299ac8f9..72d138d29 100644 --- a/app/policies/tag_policy.rb +++ b/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 - [ - (:category if can_change_category?), - (:is_deprecated if can_change_deprecated_status?), - ].compact + permitted = [] + permitted << :category if can_change_category? + permitted << :is_deprecated if can_change_deprecated_status? + permitted end end diff --git a/test/functional/tags_controller_test.rb b/test/functional/tags_controller_test.rb index 58e2b5fea..755bfc19f 100644 --- a/test/functional/tags_controller_test.rb +++ b/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 diff --git a/test/unit/bulk_update_request_test.rb b/test/unit/bulk_update_request_test.rb index 3ebe44776..037e50170 100644 --- a/test/unit/bulk_update_request_test.rb +++ b/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")