AO3-6783 Removing a tag nomination should not result in a blank tag n…

…omination (#4888) * AO3-6783 Fix removing character nomination when tag set doesn't allow fandom nominations * AO3-6783 Fix that noncanonical autocomplete for space would suggest broken blank tag * AO3-6783 Fix that removing tag nomination could result in broken blank tag * Make reviewdog slightly happier * AO3-6783 Fix autocomplete tests * AO3-6783 Minor review fixes
otwcode · Aug 4, 2024 · cd20f8e · cd20f8e
1 parent 79054fc
commit cd20f8e
Show file tree

Hide file tree

Showing 7 changed files with 106 additions and 22 deletions.
diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb
@@ -92,7 +92,7 @@ def noncanonical_tag
     raise "Redshirt: Attempted to constantize invalid class initialize noncanonical_tag #{params[:type].classify}" unless Tag::TYPES.include?(params[:type].classify)
 
     tag_class = params[:type].classify.constantize
-    one_tag = tag_class.find_by(canonical: false, name: params[:term])
+    one_tag = tag_class.find_by(canonical: false, name: params[:term]) if params[:term].present?
     # If there is an exact match in the database, ensure it is the first thing suggested.
     match = if one_tag
               [one_tag.name]

diff --git a/app/models/tagset_models/cast_nomination.rb b/app/models/tagset_models/cast_nomination.rb
@@ -3,7 +3,7 @@ class CastNomination < TagNomination
   has_one :owned_tag_set, through: :tag_set_nomination
   belongs_to :fandom_nomination
 
-  validate :known_fandom
+  validate :known_fandom, unless: :blank_tagname?
   def known_fandom
     return true if (!parent_tagname.blank? || self.fandom_nomination || from_fandom_nomination)
     return true if (tag = Tag.find_by_name(self.tagname)) && tag.parents.any? {|p| p.is_a?(Fandom)}

diff --git a/app/models/tagset_models/tag_nomination.rb b/app/models/tagset_models/tag_nomination.rb
@@ -13,11 +13,11 @@ class TagNomination < ApplicationRecord
     with: /\A[^,*<>^{}=`\\%]+\z/,
     message: ts("^Tag nominations cannot include the following restricted characters: , &#94; * < > { } = ` \\ %")
 
-  validate :type_validity
+  validate :type_validity, unless: :blank_tagname?
   def type_validity
-    if !tagname.blank? && (tag = Tag.find_by_name(tagname)) && "#{tag.type}Nomination" != self.type
-      errors.add(:base, ts("^The tag %{tagname} is already in the archive as a #{tag.type} tag. (All tags have to be unique.) Try being more specific, for instance tacking on the medium or the fandom.", tagname: self.tagname))
-    end
+    return unless (tag = Tag.find_by_name(tagname)) && "#{tag.type}Nomination" != self.type
+
+    errors.add(:base, ts("^The tag %{tagname} is already in the archive as a #{tag.type} tag. (All tags have to be unique.) Try being more specific, for instance tacking on the medium or the fandom.", tagname: self.tagname))
   end
 
   validate :not_already_reviewed, on: :update
@@ -32,7 +32,7 @@ def not_already_reviewed
   end
 
   # This makes sure no tagnames are nominated for different parents in this tag set
-  validate :require_unique_tagname_with_parent
+  validate :require_unique_tagname_with_parent, unless: :blank_tagname?
   def require_unique_tagname_with_parent
     query = TagNomination.for_tag_set(get_owned_tag_set).where(tagname: self.tagname).where("parent_tagname != ?", (self.get_parent_tagname || ''))
     # let people change their own!
@@ -43,18 +43,15 @@ def require_unique_tagname_with_parent
     end
   end
 
-  after_save :destroy_if_blank
-  def destroy_if_blank
-    if tagname.blank?
-      self.destroy
-    end
-  end
-
   def get_owned_tag_set
     @tag_set || self.tag_set_nomination.owned_tag_set
   end
 
-  before_save :set_tag_status
+  def blank_tagname?
+    tagname.blank?
+  end
+
+  before_save :set_tag_status, unless: :blank_tagname?
   def set_tag_status
     if (tag = Tag.find_by_name(tagname))
       self.exists = true
@@ -69,7 +66,7 @@ def set_tag_status
     true
   end
 
-  before_save :set_parented
+  before_save :set_parented, unless: :blank_tagname?
   def set_parented
     if type == "FreeformNomination"
       # skip freeforms
@@ -88,7 +85,7 @@ def set_parented
 
   # sneaky bit: if the tag set moderator has already rejected or approved this tag, don't
   # show it to them again.
-  before_save :set_approval_status
+  before_save :set_approval_status, unless: :blank_tagname?
   def set_approval_status
     set_noms = tag_set_nomination
     set_noms = fandom_nomination.tag_set_nomination if !set_noms && from_fandom_nomination
@@ -105,6 +102,11 @@ def set_approval_status
     true
   end
 
+  after_save :destroy_if_blank
+  def destroy_if_blank
+    self.destroy if blank_tagname?
+  end
+
   def self.for_tag_set(tag_set)
     joins(tag_set_nomination: :owned_tag_set).
     where("owned_tag_sets.id = ?", tag_set.id)

diff --git a/features/other_a/autocomplete.feature b/features/other_a/autocomplete.feature
@@ -217,3 +217,23 @@ Feature: Display autocomplete for tags
     Then I should see "日月" in the autocomplete
       But I should not see "大小" in the autocomplete
 
+  @javascript
+  Scenario: Zero width space tag doesn't appear in the autocomplete for space
+    Given a canonical character "Gold"
+      And a zero width space tag exists
+      And I am logged in as a tag wrangler
+    When I go to the "Gold" tag edit page
+    Then I should see "This is the official name for the Character"
+    When I enter " " in the "tag_merger_string_autocomplete" autocomplete field
+    Then I should see "No suggestions found" in the autocomplete
+
+  @javascript
+  Scenario: Zero width space tag appears in the autocomplete for zero width space
+    Given a canonical character "Gold"
+      And a zero width space tag exists
+      And I am logged in as a tag wrangler
+    When I go to the "Gold" tag edit page
+    Then I should see "This is the official name for the Character"
+    # Zero width space tag
+    When I enter "" in the "tag_merger_string_autocomplete" autocomplete field
+    Then I should not see "No suggestions found" in the autocomplete
diff --git a/features/step_definitions/tag_steps.rb b/features/step_definitions/tag_steps.rb
@@ -221,6 +221,11 @@
   tag.destroy if tag.present?
 end
 
+Given "a zero width space tag exists" do
+  blank_tag = FactoryBot.build(:character, name: ["200B".hex].pack("U"))
+  blank_tag.save!(validate: true) # TODO: Change to validate: false when AO3-6777 is fixed
+end
+
 ### WHEN
 
 When /^the periodic tag count task is run$/i do

diff --git a/features/support/paths.rb b/features/support/paths.rb
@@ -263,8 +263,6 @@ def path_to(page_name)
       edit_tag_set_path(OwnedTagSet.find_by(title: $1))
     when /^the "(.*)" tag ?set page$/i
       tag_set_path(OwnedTagSet.find_by(title: $1))
-    when /^the "(.*)" tag ?set nominations page$/i
-      tag_set_nominations_path(OwnedTagSet.find_by(title: Regexp.last_match(1)))
     when /^the Open Doors tools page$/i
       opendoors_tools_path
     when /^the Open Doors external authors page$/i

diff --git a/features/tag_sets/tag_set_nominations.feature b/features/tag_sets/tag_set_nominations.feature
@@ -114,6 +114,25 @@ Feature: Nominating and reviewing nominations for a tag set
     And I should see "My Favorite Character/Their Worst Enemy"
     But I should not see "Their Best Friend"
 
+  Scenario: You should be able to delete a nominated character and its fandom at once
+  when the tagset doesn't allow fandom nominations
+    Given a canonical character "Common Character" in fandom "Canon"
+      And I am logged in as "tagsetter"
+      And I set up the nominated tag set "Nominated Tags" with 0 fandom noms and 3 character noms
+    When I follow "Nominate"
+      And I fill in "Character 1" with "Obscure Character"
+      And I fill in "Fandom?" with "Canon"
+      And I press "Submit"
+    Then I should see "Your nominations were successfully submitted."
+      And I should see "Obscure Character"
+    When I follow "Edit"
+      And I fill in "Character 1" with ""
+      And I fill in "Fandom?" with ""
+      And I press "Submit"
+    Then I should see "Your nominations were successfully updated."
+      And I should see "None nominated in this category."
+      But I should not see "We need to know what fandom belongs in."
+
   Scenario: Owner of a tag set can clear all nominations
   Given I am logged in as "tagsetter"
     And I set up the nominated tag set "Nominated Tags" with 3 fandom noms and 3 character noms
@@ -240,7 +259,7 @@ Feature: Nominating and reviewing nominations for a tag set
     When I follow "My Nominations"
     Then I should see "My Nominations for bad"
       And I should see "rel tag"
-    When I go to the "bad" tag set nominations page
+    When I review nominations for "bad"
     Then I should see "Fandoms (1 left to review)"
       And I should see "rel tag"
     # canonical tag
@@ -249,7 +268,7 @@ Feature: Nominating and reviewing nominations for a tag set
     When I follow "My Nominations"
     Then I should see "My Nominations for bad"
       And I should see "rel tag"
-    When I go to the "bad" tag set nominations page
+    When I review nominations for "bad"
     Then I should see "Fandoms (1 left to review)"
       And I should see "rel tag"
     # synonym tag
@@ -260,6 +279,46 @@ Feature: Nominating and reviewing nominations for a tag set
     When I follow "My Nominations"
     Then I should see "My Nominations for bad"
       And I should see "rel tag"
-    When I go to the "bad" tag set nominations page
+    When I review nominations for "bad"
     Then I should see "Fandoms (1 left to review)"
       And I should see "rel tag (canon tag)"
+
+  Scenario: The zero width space tag doesn't replace deleted tag nominations
+    Given a canonical fandom "Treasure Chest"
+      And a zero width space tag exists
+      And I am logged in as "tagsetter"
+      And I set up the nominated tag set "Nominated Tags" with 2 fandom noms and 3 character noms
+      And I nominate fandom "Treasure Chest" and character "Gold" in "Nominated Tags"
+      And I go to the "Nominated Tags" tag set page
+      And I follow "My Nominations"
+    Then I should see "Not Yet Reviewed (may be edited or deleted)"
+    When I follow "Edit"
+      And I fill in "Character" with ""
+      And I press "Submit"
+    Then I should see "Your nominations were successfully updated."
+      And I should see "Treasure Chest"
+      But I should not see "Gold"
+      And I should see "None nominated in this fandom."
+
+  Scenario: A tag nomination with the zero width space tag doesn't prevent removing other tag nominations
+    Given a canonical fandom "First"
+      And a canonical fandom "Treasure Chest"
+      And a zero width space tag exists
+      And I am logged in as "tagsetter"
+      And I set up the nominated tag set "Nominated Tags" with 2 fandom noms and 1 character noms
+      # Zero width space character tag in first fandom
+      And I nominate fandom "First,Treasure Chest" and character ",Gold" in "Nominated Tags"
+      And I go to the "Nominated Tags" tag set page
+      And I follow "My Nominations"
+    Then I should see "First"
+      And I should not see "None nominated in this fandom."
+      And I should see "Treasure Chest"
+      And I should see "Gold"
+    When I follow "Edit"
+      # Empty second fandom character field
+      And I fill in "tag_set_nomination_fandom_nominations_attributes_1_character_nominations_attributes_0_tagname" with ""
+      And I press "Submit"
+    Then I should see "Your nominations were successfully updated."
+      And I should not see "Someone else has already nominated the tag for this set but in fandom First."
+      And I should not see "Gold"
+      And I should see "None nominated in this fandom."