From cc4d3ae61aedfdc6556a09e2defb582335bad667 Mon Sep 17 00:00:00 2001 From: Bilka Date: Sat, 3 Aug 2024 20:57:34 +0200 Subject: [PATCH 1/6] AO3-6783 Fix removing character nomination when tag set doesn't allow fandom nominations --- app/models/tagset_models/cast_nomination.rb | 2 +- app/models/tagset_models/tag_nomination.rb | 12 +++++++----- features/tag_sets/tag_set_nominations.feature | 19 +++++++++++++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/app/models/tagset_models/cast_nomination.rb b/app/models/tagset_models/cast_nomination.rb index c06bd2f6dd8..4600934050a 100644 --- 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 index b0d14d29dc7..892627af2bf 100644 --- a/app/models/tagset_models/tag_nomination.rb +++ b/app/models/tagset_models/tag_nomination.rb @@ -43,15 +43,17 @@ def require_unique_tagname_with_parent end end + def get_owned_tag_set + @tag_set || self.tag_set_nomination.owned_tag_set + end + after_save :destroy_if_blank def destroy_if_blank - if tagname.blank? - self.destroy - end + self.destroy if blank_tagname? end - def get_owned_tag_set - @tag_set || self.tag_set_nomination.owned_tag_set + def blank_tagname? + tagname.blank? end before_save :set_tag_status diff --git a/features/tag_sets/tag_set_nominations.feature b/features/tag_sets/tag_set_nominations.feature index e98fe44b0c8..1cb641b16cf 100644 --- 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 From f33a26315b0ec07cf07fc01568289efa4213faf9 Mon Sep 17 00:00:00 2001 From: Bilka Date: Sat, 3 Aug 2024 22:34:33 +0200 Subject: [PATCH 2/6] AO3-6783 Fix that noncanonical autocomplete for space would suggest broken blank tag --- app/controllers/autocomplete_controller.rb | 2 +- features/other_a/autocomplete.feature | 20 ++++++++++++++++++++ features/step_definitions/tag_steps.rb | 5 +++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index e4b0b590037..ced58417532 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -94,7 +94,7 @@ def noncanonical_tag tag_class = params[:type].classify.constantize one_tag = tag_class.find_by(canonical: false, name: params[:term]) # If there is an exact match in the database, ensure it is the first thing suggested. - match = if one_tag + match = if one_tag && params[:term].present? [one_tag.name] else [] diff --git a/features/other_a/autocomplete.feature b/features/other_a/autocomplete.feature index 216455f09e0..1f4a03579d2 100644 --- 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 "A/B" 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 "A/B" 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 index f2d3ba09b72..03c2874f5b7 100644 --- 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 From 758b777c578bb501e8946398d53ee190e4a4e170 Mon Sep 17 00:00:00 2001 From: Bilka Date: Sat, 3 Aug 2024 22:36:57 +0200 Subject: [PATCH 3/6] AO3-6783 Fix that removing tag nomination could result in broken blank tag --- app/models/tagset_models/tag_nomination.rb | 12 ++--- features/support/paths.rb | 2 - features/tag_sets/tag_set_nominations.feature | 46 +++++++++++++++++-- 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/app/models/tagset_models/tag_nomination.rb b/app/models/tagset_models/tag_nomination.rb index 892627af2bf..66f71052d68 100644 --- a/app/models/tagset_models/tag_nomination.rb +++ b/app/models/tagset_models/tag_nomination.rb @@ -13,9 +13,9 @@ class TagNomination < ApplicationRecord with: /\A[^,*<>^{}=`\\%]+\z/, message: ts("^Tag nominations cannot include the following restricted characters: , ^ * < > { } = ` \\ %") - 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 + if (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 end @@ -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! @@ -56,7 +56,7 @@ def blank_tagname? tagname.blank? end - before_save :set_tag_status + before_save :set_tag_status, unless: :blank_tagname? def set_tag_status if (tag = Tag.find_by_name(tagname)) self.exists = true @@ -71,7 +71,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 @@ -90,7 +90,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 diff --git a/features/support/paths.rb b/features/support/paths.rb index cade8257446..237112b7b48 100644 --- 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 index 1cb641b16cf..923954b3184 100644 --- a/features/tag_sets/tag_set_nominations.feature +++ b/features/tag_sets/tag_set_nominations.feature @@ -259,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 @@ -268,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 @@ -279,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 for 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." From 702b3d00b8db0cecbcad5ca50832651df89849e7 Mon Sep 17 00:00:00 2001 From: Bilka Date: Sat, 3 Aug 2024 23:09:45 +0200 Subject: [PATCH 4/6] Make reviewdog slightly happier --- app/models/tagset_models/tag_nomination.rb | 16 +++++++------- features/tag_sets/tag_set_nominations.feature | 22 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/app/models/tagset_models/tag_nomination.rb b/app/models/tagset_models/tag_nomination.rb index 66f71052d68..f6bcaaf69ba 100644 --- a/app/models/tagset_models/tag_nomination.rb +++ b/app/models/tagset_models/tag_nomination.rb @@ -15,9 +15,9 @@ class TagNomination < ApplicationRecord validate :type_validity, unless: :blank_tagname? def type_validity - if (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 @@ -47,11 +47,6 @@ def get_owned_tag_set @tag_set || self.tag_set_nomination.owned_tag_set end - after_save :destroy_if_blank - def destroy_if_blank - self.destroy if blank_tagname? - end - def blank_tagname? tagname.blank? end @@ -107,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/tag_sets/tag_set_nominations.feature b/features/tag_sets/tag_set_nominations.feature index 923954b3184..5a593315487 100644 --- a/features/tag_sets/tag_set_nominations.feature +++ b/features/tag_sets/tag_set_nominations.feature @@ -117,21 +117,21 @@ Feature: Nominating and reviewing nominations for a tag set 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 + 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" + 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" + 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" + 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." + 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" From 82f63ca7f657c5ffaba7965b9d5a7c54dc438aee Mon Sep 17 00:00:00 2001 From: Bilka Date: Sat, 3 Aug 2024 23:16:22 +0200 Subject: [PATCH 5/6] AO3-6783 Fix autocomplete tests --- features/other_a/autocomplete.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/other_a/autocomplete.feature b/features/other_a/autocomplete.feature index 1f4a03579d2..80b5ea03773 100644 --- a/features/other_a/autocomplete.feature +++ b/features/other_a/autocomplete.feature @@ -222,7 +222,7 @@ Feature: Display autocomplete for tags 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 "A/B" tag edit page + 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 @@ -232,7 +232,7 @@ Feature: Display autocomplete for tags 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 "A/B" tag edit page + 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 From f80eb52c78743f4b663bd9ffd2a4ccbcd9e55d50 Mon Sep 17 00:00:00 2001 From: Bilka Date: Sun, 4 Aug 2024 10:56:51 +0200 Subject: [PATCH 6/6] AO3-6783 Minor review fixes --- app/controllers/autocomplete_controller.rb | 4 ++-- features/tag_sets/tag_set_nominations.feature | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index ced58417532..a7ba9f7d38a 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -92,9 +92,9 @@ 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 && params[:term].present? + match = if one_tag [one_tag.name] else [] diff --git a/features/tag_sets/tag_set_nominations.feature b/features/tag_sets/tag_set_nominations.feature index 5a593315487..d82f636159b 100644 --- a/features/tag_sets/tag_set_nominations.feature +++ b/features/tag_sets/tag_set_nominations.feature @@ -300,7 +300,7 @@ Feature: Nominating and reviewing nominations for a tag set But I should not see "Gold" And I should see "None nominated in this fandom." - Scenario: A tag nomination with for the zero width space tag doesn't prevent removing other tag nominations + 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