diff --git a/app/models/tag.rb b/app/models/tag.rb index b8b694a543..951afa2f87 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -630,9 +630,9 @@ def unfilterable? !(self.canonical? || self.unwrangleable? || self.merger_id.present? || self.mergers.any?) end - # Returns true if a tag has been used in posted works - def has_posted_works? - self.works.posted.any? + # Returns true if a tag has been used in posted works that are revealed and not hidden + def has_posted_works? # rubocop:disable Naming/PredicateName + self.works.posted.revealed.unhidden.any? end # sort tags by name diff --git a/app/models/work.rb b/app/models/work.rb index 0881823f95..d6a87ed630 100755 --- a/app/models/work.rb +++ b/app/models/work.rb @@ -310,10 +310,11 @@ def should_reindex_pseuds? destroyed? || (saved_changes.keys & pertinent_attributes).present? end - # If the work gets posted, we should (potentially) reindex the tags, - # so they get the correct draft-only status. + # If the work gets posted, (un)hidden, or (un)revealed, we should (potentially) reindex the tags, + # so they get the correct visibility status. def update_tag_index - return unless saved_change_to_posted? + return unless saved_change_to_posted? || saved_change_to_hidden_by_admin? || saved_change_to_in_unrevealed_collection? + taggings.each(&:update_search) end diff --git a/lib/tasks/after_tasks.rake b/lib/tasks/after_tasks.rake index a77239aaa9..9042aff1f0 100644 --- a/lib/tasks/after_tasks.rake +++ b/lib/tasks/after_tasks.rake @@ -400,5 +400,26 @@ namespace :After do puts "Finished up to ID #{skin.id}" if skin.id.modulo(100).zero? end end + + desc "Reindex tags associated with works that are hidden or unrevealed" + task(reindex_hidden_unrevealed_tags: :environment) do + hidden_count = Work.hidden.count + hidden_batches = (hidden_count + 999) / 1_000 + puts "Inspecting #{hidden_count} hidden works in #{hidden_batches} batches" + Work.hidden.find_in_batches.with_index do |batch, index| + batch.each { |work| work.taggings.each(&:update_search) } + puts "Finished batch #{index + 1} of #{hidden_batches}" + end + + unrevealed_count = Work.unrevealed.count + unrevealed_batches = (unrevealed_count + 999) / 1_000 + puts "Inspecting #{unrevealed_count} unrevealed works in #{unrevealed_batches} batches" + Work.unrevealed.find_in_batches.with_index do |batch, index| + batch.each { |work| work.taggings.each(&:update_search) } + puts "Finished batch #{index + 1} of #{unrevealed_batches}" + end + + puts "Finished reindexing tags on hidden and unrevealed works" + end # This is the end that you have to put new tasks above. end diff --git a/spec/lib/tasks/after_tasks.rake_spec.rb b/spec/lib/tasks/after_tasks.rake_spec.rb index b670e77189..d1e36ddaff 100644 --- a/spec/lib/tasks/after_tasks.rake_spec.rb +++ b/spec/lib/tasks/after_tasks.rake_spec.rb @@ -424,3 +424,39 @@ end end end + +describe "rake After:reindex_hidden_unrevealed_tags" do + context "with a posted work" do + let!(:work) { create(:work) } + + it "does not reindex the work's tags" do + expect do + subject.invoke + end.not_to add_to_reindex_queue(work.tags.first, :main) + end + end + + context "with a hidden work" do + let!(:work) { create(:work, hidden_by_admin: true) } + + it "reindexes the work's tags" do + expect do + subject.invoke + end.to add_to_reindex_queue(work.tags.first, :main) + end + end + + context "with an unrevealed work" do + let(:work) { create(:work) } + + before do + work.update!(in_unrevealed_collection: true) + end + + it "reindexes the work's tags" do + expect do + subject.invoke + end.to add_to_reindex_queue(work.tags.first, :main) + end + end +end diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 340c2d8799..4d891a5599 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -1,5 +1,4 @@ -# encoding: UTF-8 -require 'spec_helper' +require "spec_helper" describe Tag do after(:each) do @@ -66,7 +65,7 @@ end it "Writes to the database do not happen immediately" do - (1..40 * ArchiveConfig.TAGGINGS_COUNT_CACHE_DIVISOR - 1).each do |try| + (1..(40 * ArchiveConfig.TAGGINGS_COUNT_CACHE_DIVISOR) - 1).each do |try| @fandom_tag.taggings_count = try @fandom_tag.reload expect(@fandom_tag.taggings_count_cache).to eq 0 @@ -343,15 +342,19 @@ def expect_tag_update_flag_in_redis_to_be(flag) end describe "has_posted_works?" do - before do - create(:work, fandom_string: "love live,jjba") - create(:draft, fandom_string: "zombie land saga,jjba") - end - - it "is true if used in posted works" do - expect(Tag.find_by(name: "zombie land saga").has_posted_works?).to be_falsey - expect(Tag.find_by(name: "love live").has_posted_works?).to be_truthy - expect(Tag.find_by(name: "jjba").has_posted_works?).to be_truthy + { + "draft" => { posted: false }, + "unrevealed" => { in_unrevealed_collection: true }, + "hidden" => { hidden_by_admin: true } + }.each do |description, attributes| + it "is false if only used on #{description} works" do + create(:work, fandom_string: "love live,jjba") + non_visible_work = create(:work, fandom_string: "zombie land saga,jjba") + non_visible_work.update!(**attributes) + expect(Tag.find_by(name: "zombie land saga").has_posted_works?).to be_falsey + expect(Tag.find_by(name: "love live").has_posted_works?).to be_truthy + expect(Tag.find_by(name: "jjba").has_posted_works?).to be_truthy + end end end diff --git a/spec/models/work_spec.rb b/spec/models/work_spec.rb index 51d989108e..ad54b4ea7f 100644 --- a/spec/models/work_spec.rb +++ b/spec/models/work_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require "spec_helper" describe Work do # see lib/collectible_spec for collection-related tests @@ -10,16 +10,17 @@ context "when posted" do it "posts the first chapter" do work = create(:work) - work.first_chapter.posted.should == true + expect(work.first_chapter.posted).to eq true end end context "create_stat_counter" do it "creates a stat counter for that work id" do - expect { + expect do @work = build(:work) @work.save! - }.to change{ StatCounter.all.count }.by(1) + end.to change { StatCounter.all.count } + .by(1) expect(StatCounter.where(work_id: @work.id)).to exist end end @@ -48,10 +49,10 @@ let(:too_short) { ArchiveConfig.TITLE_MIN - 1 } it "errors if the title without leading spaces is shorter than #{ArchiveConfig.TITLE_MIN}" do - expect { + expect do @work = create(:work, title: " #{too_short}") @work.reload - }.to raise_error(ActiveRecord::RecordInvalid,"Validation failed: Title must be at least #{ArchiveConfig.TITLE_MIN} characters long without leading spaces.") + end.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Title must be at least #{ArchiveConfig.TITLE_MIN} characters long without leading spaces.") end # Reset the min characters in the title, so that the factory is valid @@ -121,6 +122,34 @@ end end + describe "reindexing" do + let!(:work) { create(:work) } + + context "when draft status is changed" do + it "enqueues tags for reindex" do + expect do + work.update!(posted: false) + end.to add_to_reindex_queue(work.fandoms.last, :main) + end + end + + context "when hidden by an admin" do + it "enqueues tags for reindex" do + expect do + work.update!(hidden_by_admin: true) + end.to add_to_reindex_queue(work.fandoms.last, :main) + end + end + + context "when put in an unrevealed collection" do + it "enqueues tags for reindex" do + expect do + work.update!(in_unrevealed_collection: true) + end.to add_to_reindex_queue(work.fandoms.last, :main) + end + end + end + describe "#crossover" do it "is not crossover with one fandom" do fandom = create(:canonical_fandom, name: "nge") @@ -392,7 +421,7 @@ let(:work) { build(:work) } before do - work.recipients = recipient1 + "," + recipient2 + work.recipients = "#{recipient1},#{recipient2}" end it "contains gifts for the same recipients when they are first added" do @@ -410,7 +439,7 @@ end it "only contains one gift if the same recipient is entered twice" do - work.recipients = recipient2 + "," + recipient2 + work.recipients = "#{recipient2},#{recipient2}" expect(work.new_gifts.collect(&:recipient)).to eq([recipient2]) end end @@ -511,8 +540,9 @@ @admin_setting.update_attribute(:hide_spam, true) end it "automatically hides spam works and sends an email" do - expect { @work.update!(spam: true) }. - to change { ActionMailer::Base.deliveries.count }.by(1) + expect { @work.update!(spam: true) } + .to change { ActionMailer::Base.deliveries.count } + .by(1) expect(@work.reload.hidden_by_admin).to be_truthy expect(ActionMailer::Base.deliveries.last.subject).to eq("[AO3] Your work was hidden as spam") end @@ -522,8 +552,8 @@ @admin_setting.update_attribute(:hide_spam, false) end it "does not automatically hide spam works and does not send an email" do - expect { @work.update!(spam: true) }. - not_to change { ActionMailer::Base.deliveries.count } + expect { @work.update!(spam: true) } + .not_to change { ActionMailer::Base.deliveries.count } expect(@work.reload.hidden_by_admin).to be_falsey end end @@ -580,9 +610,10 @@ before { work.reload } it "raises an error" do - expect { work.remove_author(to_remove) }.to raise_exception( - "Sorry, we can't remove all creators of a work." - ) + expect { work.remove_author(to_remove) } + .to raise_exception( + "Sorry, we can't remove all creators of a work." + ) end end @@ -610,7 +641,8 @@ let(:work) { create(:work) } it "does not save an original creator record" do - expect { work.destroy }.not_to change { WorkOriginalCreator.count } + expect { work.destroy } + .not_to change { WorkOriginalCreator.count } end context "when an original creator exists" do @@ -618,7 +650,8 @@ it "deletes the original creator" do work.destroy - expect { original_creator.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { original_creator.reload } + .to raise_error(ActiveRecord::RecordNotFound) end end end