From 6a879736abc86e1f4306af261ca3933b897a59a4 Mon Sep 17 00:00:00 2001 From: Pablo Monfort Date: Wed, 21 Oct 2026 20:38:02 -0300 Subject: [PATCH 1/2] AO3-7627 Add tests for series index eager loading --- spec/requests/series_n_plus_one_spec.rb | 43 +++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/spec/requests/series_n_plus_one_spec.rb b/spec/requests/series_n_plus_one_spec.rb index ae9874bce8f..e997919a317 100644 --- a/spec/requests/series_n_plus_one_spec.rb +++ b/spec/requests/series_n_plus_one_spec.rb @@ -3,6 +3,49 @@ describe "n+1 queries in the series controller" do include LoginMacros + describe "#index", n_plus_one: true do + let!(:user) { create(:user) } + + subject do + proc do + get user_series_index_path(user_id: user) + end + end + + before do + fake_login_known_user(user) + end + + context "when all blurbs are cached" do + populate do |n| + create_list(:series, n, authors: user.pseuds) + subject.call # this caches the blurbs + end + + it "performs a constant number of queries" do + expect do + subject.call + expect(response.body.scan('
  • Date: Sun, 25 Oct 2026 17:08:16 -0300 Subject: [PATCH 2/2] AO3-7627 Eager load series blurb associations on index --- app/controllers/series_controller.rb | 2 +- app/models/series.rb | 74 +++++++++++++++--- app/views/series/_series_module.html.erb | 2 +- spec/models/series_spec.rb | 97 ++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 11 deletions(-) diff --git a/app/controllers/series_controller.rb b/app/controllers/series_controller.rb index e34d60cc010..c56a5b8fbe2 100644 --- a/app/controllers/series_controller.rb +++ b/app/controllers/series_controller.rb @@ -36,7 +36,7 @@ def index else @series = @series.exclude_anonymous.for_user(@user) end - @series = @series.paginate(page: params[:page]) + @series = @series.for_blurb.paginate(page: params[:page]) end # GET /series/1 diff --git a/app/models/series.rb b/app/models/series.rb index 1b665fbb7d5..df2bd63ad27 100644 --- a/app/models/series.rb +++ b/app/models/series.rb @@ -58,7 +58,7 @@ def title joins(approved_creatorships: :pseud).where(pseuds: { user: user }) } - scope :for_blurb, -> { includes(:work_tags, :pseuds) } + scope :for_blurb, -> { preload({ pseuds: :user }, works: :tags) } def posted_works self.works.posted @@ -113,9 +113,15 @@ def user_is_owner_or_invited?(user) def visible_work_count if User.current_user.nil? - self.works.posted.unrestricted.count + if works.loaded? + works.count { |work| work.posted && !work.restricted } + else + works.posted.unrestricted.count + end + elsif works.loaded? + works.count(&:posted) else - self.works.posted.count + works.posted.count end end @@ -192,11 +198,27 @@ def fandoms tag_groups["Fandom"]&.sort || [] end + # Memoized because the blurb calls this three times per series (fandoms, + # symbols and tag block). The result depends on User.current_user. def tag_groups - tags = self.work_tags.where(works: { hidden_by_admin: false, posted: true }) - tags = tags.where(works: { restricted: false }) unless User.current_user - tags.group_by { |t| t.type.to_s } + @tag_groups ||= + if works_and_tags_loaded? + visible_works = works.select do |work| + work.posted && !work.hidden_by_admin + end + visible_works = visible_works.reject(&:restricted) unless User.current_user + visible_works.flat_map(&:tags).uniq.group_by { |t| t.type.to_s } + else + tags = work_tags.where(works: { hidden_by_admin: false, posted: true }) + tags = tags.where(works: { restricted: false }) unless User.current_user + tags.group_by { |t| t.type.to_s } + end + end + + def works_and_tags_loaded? + works.loaded? && works.all? { |work| work.association(:tags).loaded? } end + private :works_and_tags_loaded? # Grabs the earliest published_at date of the visible works in the series def published_at @@ -208,13 +230,37 @@ def published_at end def revised_at - if self.works.visible.posted.blank? + if works.loaded? + dates = works_visible_to_current_user.map(&:revised_at).compact + dates.empty? ? self.updated_at : dates.max + elsif self.works.visible.posted.blank? self.updated_at else Work.in_series(self).visible.collect(&:revised_at).compact.uniq.sort.last end end + # Mirrors Work.visible_to_user for preloaded works. Only hidden works + # check ownership (an extra query), and those are rare. + def works_visible_to_current_user + user = User.current_user + case user + when Admin + works.select(&:posted) + when User + works.select do |work| + work.posted && + (!work.hidden_by_admin || + work.pseuds.any? { |pseud| pseud.user_id == user.id }) + end + else + works.select do |work| + work.posted && !work.restricted && !work.hidden_by_admin + end + end + end + private :works_visible_to_current_user + ###################### # SEARCH ###################### @@ -261,7 +307,12 @@ def word_count # Word count as seen by guests def public_word_count # Exclude restricted works and those hidden by admin from the word count - self.works.posted.unrestricted.unhidden.pluck(:word_count).compact.sum + if works.loaded? + works.select { |work| work.posted && !work.restricted && !work.hidden_by_admin } + .sum { |work| work.word_count.to_i } + else + self.works.posted.unrestricted.unhidden.pluck(:word_count).compact.sum + end end # Word count as seen by registered users @@ -269,7 +320,12 @@ def public_word_count # in the series word count def general_word_count # Exclude works hidden by admin from the word count - self.works.posted.unhidden.pluck(:word_count).compact.sum + if works.loaded? + works.select { |work| work.posted && !work.hidden_by_admin } + .sum { |work| work.word_count.to_i } + else + self.works.posted.unhidden.pluck(:word_count).compact.sum + end end # FIXME: should series have their own language? diff --git a/app/views/series/_series_module.html.erb b/app/views/series/_series_module.html.erb index c136d134d72..ba4add8ef76 100644 --- a/app/views/series/_series_module.html.erb +++ b/app/views/series/_series_module.html.erb @@ -41,7 +41,7 @@
    <%= number_with_delimiter(series.visible_word_count) %>
    <%= Work.model_name.human(count: :many) %>:
    <%= number_with_delimiter(series.visible_work_count) %>
    - <% if (bookmark_count = series.bookmarks.is_public.count) > 0 %> + <% if (bookmark_count = series.public_bookmark_count) > 0 %>
    <%= Bookmark.model_name.human(count: :many) %>:
    <%= link_to_bookmarkable_bookmarks(series, number_with_delimiter(bookmark_count)) %>
    <% end %> diff --git a/spec/models/series_spec.rb b/spec/models/series_spec.rb index 8c04cf5ce96..aa0d8062fe8 100644 --- a/spec/models/series_spec.rb +++ b/spec/models/series_spec.rb @@ -361,6 +361,103 @@ end end + describe "when associations are preloaded for blurbs" do + let(:hidden_work) { create(:work, hidden_by_admin: true, freeform_string: "Testing3") } + let(:draft_work) { create(:draft, freeform_string: "Testing4") } + let(:preloaded_series) { Series.for_blurb.find(series.id) } + + before do + series.works = [unrestricted_work, restricted_work, hidden_work, draft_work] + # Stagger the dates so the examples can tell which works each + # revised_at branch uses (the column only has second precision). + series.works.each_with_index do |work, index| + work.update_column(:revised_at, (index + 1).days.from_now) + end + series.reload + end + + it "preloads the works and their tags" do + expect(preloaded_series.works).to be_loaded + expect(preloaded_series.works.first.association(:tags)).to be_loaded + end + + shared_examples "matches the results without preloading" do + it "returns the same tag groups" do + expect(preloaded_series.tag_groups.keys).to match_array(series.tag_groups.keys) + series.tag_groups.each_pair do |type, tags| + expect(preloaded_series.tag_groups[type]).to match_array(tags) + end + end + + it "returns the same visible work count" do + expect(preloaded_series.visible_work_count).to eq(series.visible_work_count) + end + + it "returns the same visible word count" do + expect(preloaded_series.visible_word_count).to eq(series.visible_word_count) + end + + it "returns the same revised date" do + expect(preloaded_series.revised_at).to eq(series.revised_at) + end + end + + context "when no user is logged in" do + it_behaves_like "matches the results without preloading" + end + + context "when logged in as a regular user" do + before do + User.current_user = create(:user) + end + + it_behaves_like "matches the results without preloading" + end + + context "when logged in as an admin" do + before do + User.current_user = create(:admin) + end + + it_behaves_like "matches the results without preloading" + end + + context "when the logged-in user created a work hidden by an admin" do + let(:owner) { create(:user) } + let(:hidden_work) { create(:work, hidden_by_admin: true, authors: [owner.default_pseud]) } + + before do + hidden_work.update_column(:revised_at, 10.days.from_now) + User.current_user = owner + end + + it "includes the hidden work's revision date" do + expect(preloaded_series.revised_at).to eq(hidden_work.reload.revised_at) + end + end + + context "when a work has no word count" do + before do + unrestricted_work.update_column(:word_count, nil) + end + + it "returns the same visible word count" do + expect(preloaded_series.visible_word_count).to eq(series.visible_word_count) + end + end + + context "when a guest views a series with only restricted works" do + before do + series.works = [restricted_work] + series.reload + end + + it "falls back to the series update date" do + expect(preloaded_series.revised_at).to eq(series.updated_at) + end + end + end + describe "#general_tags" do it "includes tags on unrestricted works" do series.works = [unrestricted_work]