AO3-6592 Optimize the database query for the series count in the user…

… sidebar (#4725) * AO3-6592 Add test for correct counts * AO3-6592 Optimize getting the count * AO3-6592 Optimize series for_pseuds * Rubocop * AO3-6592 Add pseud with no series to test * AO3-6592 Split series for_pseuds scope into user and pseud Based on the EXPLAIN, the database is not clever enough to get rid of the extra select for the user pseuds. And for the pseud case, the JOIN is generally unnecessary.
otwcode · Apr 4, 2024 · 6adbd88 · 6adbd88
1 parent e9030d0
commit 6adbd88
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 26 deletions.
diff --git a/app/controllers/series_controller.rb b/app/controllers/series_controller.rb
@@ -22,20 +22,19 @@ def index
     end
     @user = User.find_by!(login: params[:user_id])
     @page_subtitle = ts("%{username} - Series", username: @user.login)
-    pseuds = @user.pseuds
+
+    @series = if current_user.nil?
+                Series.visible_to_all
+              else
+                Series.visible_to_registered_user
+              end
+
     if params[:pseud_id]
-      @pseud = @user.pseuds.find_by!(name: params[:pseud_id])   
+      @pseud = @user.pseuds.find_by!(name: params[:pseud_id])
       @page_subtitle = ts("by ") + @pseud.byline
-      pseuds = [@pseud]
-    end
-
-    if current_user.nil?
-      @series = Series.visible_to_all
+      @series = @series.exclude_anonymous.for_pseud(@pseud)
     else
-      @series = Series.visible_to_registered_user
-    end
-    if pseuds.present?
-      @series = @series.exclude_anonymous.for_pseuds(pseuds)
+      @series = @series.exclude_anonymous.for_user(@user)
     end
     @series = @series.paginate(page: params[:page])
   end

diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb
@@ -82,21 +82,21 @@ def pseud_works_link(pseud)
   def series_link(user, pseud = nil)
     return pseud_series_link(pseud) if pseud.present? && !pseud.new_record?
 
-    if current_user.nil?
-      total = Series.visible_to_all.exclude_anonymous.for_pseuds(user.pseuds).length
-    else
-      total = Series.visible_to_registered_user.exclude_anonymous.for_pseuds(user.pseuds).length
-    end
-    span_if_current ts('Series (%{series_number})', series_number: total.to_s), user_series_index_path(@user)
+    total = if current_user.nil?
+              Series.visible_to_all.exclude_anonymous.for_user(user).count.size
+            else
+              Series.visible_to_registered_user.exclude_anonymous.for_user(user).count.size
+            end
+    span_if_current ts("Series (%{series_number})", series_number: total.to_s), user_series_index_path(user)
   end
 
   def pseud_series_link(pseud)
-    if current_user.nil?
-      total = Series.visible_to_all.exclude_anonymous.for_pseuds([pseud]).length
-    else
-      total = Series.visible_to_registered_user.exclude_anonymous.for_pseuds([pseud]).length
-    end
-    span_if_current ts('Series (%{series_number})', series_number: total.to_s), user_pseud_series_index_path(@user, pseud)
+    total = if current_user.nil?
+              Series.visible_to_all.exclude_anonymous.for_pseud(pseud).count.size
+            else
+              Series.visible_to_registered_user.exclude_anonymous.for_pseud(pseud).count.size
+            end
+    span_if_current ts("Series (%{series_number})", series_number: total.to_s), user_pseud_series_index_path(pseud.user, pseud)
   end
 
   def gifts_link(user)

diff --git a/app/models/series.rb b/app/models/series.rb
@@ -50,9 +50,12 @@ def title
     having("MAX(works.in_anon_collection) = 0 AND MAX(works.in_unrevealed_collection) = 0")
   }
 
-  scope :for_pseuds, lambda {|pseuds|
-    joins(:approved_creatorships).
-    where("creatorships.pseud_id IN (?)", pseuds.collect(&:id))
+  scope :for_pseud, lambda { |pseud|
+    joins(:approved_creatorships).where(creatorships: { pseud: pseud })
+  }
+
+  scope :for_user, lambda { |user|
+    joins(approved_creatorships: :pseud).where(pseuds: { user: user })
   }
 
   scope :for_blurb, -> { includes(:work_tags, :pseuds) }

diff --git a/features/users/user_dashboard.feature b/features/users/user_dashboard.feature
@@ -196,3 +196,46 @@ Feature: User dashboard
   Then I should see "2 Works by meatloaf in Star Trek"
   When I press "Sort and Filter"
   Then I should see "2 Works by meatloaf in Star Trek"
+
+  Scenario: The dashboard sidebar series count should exclude restricted series when logged out
+    Given I have the anonymous collection "Anon works"
+      And I am logged in as "Accumulator"
+      And I add the pseud "Battery"
+      And I add the pseud "Centrifuge"
+      And I post the work "Normal work" as part of a series "Mine" using the pseud "Battery"
+      And I post the work "Normal work 2" as part of a series "Mine" using the pseud "Battery"
+      And I post the work "Restricted work" as part of a series "Restricted" using the pseud "Battery"
+      And I lock the work "Restricted work"
+      And I post the work "Another restricted work" as part of a series "Restricted" using the pseud "Battery"
+      And I lock the work "Another restricted work"
+    When I go to Accumulator's user page
+    Then I should see "Series (2)" within "#dashboard"
+    When I go to Accumulator's "Battery" pseud page
+    Then I should see "Series (2)" within "#dashboard"
+    When I go to Accumulator's "Centrifuge" pseud page
+    Then I should see "Series (0)" within "#dashboard"
+    When I am logged out
+      And I go to Accumulator's user page
+    Then I should see "Series (1)" within "#dashboard"
+    When I go to Accumulator's "Battery" pseud page
+    Then I should see "Series (1)" within "#dashboard"
+    When I go to Accumulator's "Centrifuge" pseud page
+    Then I should see "Series (0)" within "#dashboard"
+    # Series with anon works are never counted
+    When I am logged in as "Accumulator"
+      And I post the work "Another normal work" as part of a series "Anon" using the pseud "Battery"
+      And I post the work "Anon work" in the collection "Anon works" as part of a series "Anon" using the pseud "Battery"
+      And I go to Accumulator's user page
+    Then I should see "Series (2)" within "#dashboard"
+    When I go to Accumulator's "Battery" pseud page
+    Then I should see "Series (2)" within "#dashboard"
+    When I go to Accumulator's "Centrifuge" pseud page
+    Then I should see "Series (0)" within "#dashboard"
+    When I am logged out
+      And I go to Accumulator's user page
+    Then I should see "Series (1)" within "#dashboard"
+    When I go to Accumulator's "Battery" pseud page
+    Then I should see "Series (1)" within "#dashboard"
+    When I go to Accumulator's "Centrifuge" pseud page
+    Then I should see "Series (0)" within "#dashboard"
+