Fix N+1 queries on admin workshop show

app/controllers/concerns/admin/workshop_concerns.rb

@@ -7,12 +7,13 @@ module Admin::WorkshopConcerns
 
   module InstanceMethods
     def set_admin_workshop_data
-      @attending_students = InvitationPresenter.decorate_collection(
-        @workshop.attending_students.all.with_notes_and_their_authors
-      )
-      @attending_coaches = InvitationPresenter.decorate_collection(
-        @workshop.attending_coaches.all.with_notes_and_their_authors
-      )
+      students = @workshop.attending_students.all.with_notes_and_their_authors
+      coaches  = @workshop.attending_coaches.all.with_notes_and_their_authors
+
+      inject_attendance_flags(students, coaches)
+
+      @attending_students = InvitationPresenter.decorate_collection(students)
+      @attending_coaches  = InvitationPresenter.decorate_collection(coaches)
 
       @coach_waiting_list = WaitingListPresenter.new(
         WaitingList.coaches_for(@workshop).with_notes_and_their_authors
@@ -22,6 +23,12 @@ def set_admin_workshop_data
       )
     end
 
+    def inject_attendance_flags(*collections)
+      members = collections.flat_map { |collection| collection.map(&:member) }.uniq(&:id)
+      flags = AdminWorkshopAttendeeFlags.for_members(members.map(&:id))
+      members.each { |member| member.admin_workshop_flags = flags[member.id] }
+    end
+
     private
 
     def set_workshop

app/models/member.rb

@@ -25,6 +25,12 @@ class Member < ApplicationRecord
   has_many :member_notes
   has_many :chapters, -> { distinct }, through: :groups
   has_many :announcements, -> { distinct }, through: :groups
+
+  # Per-request aggregate results for the admin workshop attendance page, set by
+  # set_admin_workshop_data. When present, the flag helpers below avoid issuing
+  # one query per attending row. nil elsewhere -> fall back to live queries.
+  attr_accessor :admin_workshop_flags
+
   has_many :meeting_invitations
   has_many :member_email_deliveries
 
@@ -164,6 +170,8 @@ def clear_attending_event_ids_cache!
   end
 
   def flag_to_organisers?
+    return admin_workshop_flags[:flag_to_organisers] if admin_workshop_flags
+
     multiple_no_shows? && attendance_warnings.last_six_months.length >= 2
   end
 
@@ -173,6 +181,11 @@ def multiple_no_shows?
   end
 
   def recent_notes
+    if admin_workshop_flags
+      # Only used as recent_notes.any? on the attendance rows
+      return (admin_workshop_flags[:recent_notes] ? [:note] : [])
+    end
+
     last_five_workshops = workshop_invitations.order_by_latest.attended.take(5)
     return [] if last_five_workshops.empty?
 

app/presenters/member_presenter.rb

@@ -11,6 +11,8 @@ def event_organiser?(event)
   end
 
   def newbie?
+    return model.admin_workshop_flags[:newbie] if model.admin_workshop_flags
+
     !workshop_invitations.attended.exists?
   end
 

app/queriers/admin_workshop_attendee_flags.rb

@@ -0,0 +1,102 @@
+# Computes, in a bounded number of queries, the per-attendee flags the admin
+# workshop show page renders (newbie / flag-to-organisers / recent-notes).
+# Replaces per-row N+1 queries (Member#flag_to_organisers?, #recent_notes and
+# MemberPresenter#newbie?) with a small set of aggregate queries.
+class AdminWorkshopAttendeeFlags
+  def self.for_members(member_ids)
+    new(member_ids).to_h
+  end
+
+  def initialize(member_ids)
+    @member_ids = member_ids
+  end
+
+  def to_h
+    return {} if @member_ids.empty?
+
+    @member_ids.index_with do |id|
+      {
+        newbie: newbies.include?(id),
+        flag_to_organisers: flag_for?(id),
+        recent_notes: recent_notes_member_ids.include?(id)
+      }
+    end
+  end
+
+  private
+
+  def newbies
+    @newbies ||= @member_ids - attended_ever
+  end
+
+  def attended_ever
+    WorkshopInvitation.where(member_id: @member_ids, attended: true).distinct.pluck(:member_id)
+  end
+
+  def flag_for?(member_id)
+    (no_shows.fetch(member_id, 0) > 3) && (warning_counts.fetch(member_id, 0) >= 2)
+  end
+
+  # Accepted (attending) invitations for taken-place workshops in the last six
+  # months, grouped by member (mirrors Member#multiple_no_shows?).
+  def accepted_counts
+    @accepted_counts ||= six_months_taken_place.group(:member_id).count
+  end
+
+  def attended_counts
+    @attended_counts ||= six_months_taken_place.where(attended: true).group(:member_id).count
+  end
+
+  def no_shows
+    @no_shows ||= accepted_counts.merge(attended_counts) do |_id, accepted, attended|
+      accepted - attended
+    end
+  end
+
+  def warning_counts
+    @warning_counts ||= AttendanceWarning.where(member_id: @member_ids)
+                                         .last_six_months
+                                         .group(:member_id)
+                                         .count
+  end
+
+  def six_months_taken_place
+    WorkshopInvitation.joins(:workshop)
+                      .where(member_id: @member_ids, attending: true)
+                      .where(workshops: { date_and_time: 6.months.ago...Time.zone.now })
+  end
+
+  # Members with a note created after the (date - 1 day) of their fifth most
+  # recent attended workshop, replicating Member#recent_notes.
+  RECENT_NOTES_SQL = <<~SQL.freeze
+    SELECT DISTINCT mn.member_id
+    FROM member_notes mn
+    JOIN (
+      SELECT member_id, (MIN(d.date_and_time) - INTERVAL '1 day') AS cutoff
+      FROM (
+        SELECT wi.member_id, ws.date_and_time,
+               ROW_NUMBER() OVER (
+                 PARTITION BY wi.member_id ORDER BY ws.date_and_time DESC
+               ) AS rn
+        FROM workshop_invitations wi
+        JOIN workshops ws ON ws.id = wi.workshop_id
+        WHERE wi.member_id IN (%<member_ids>s)
+          AND wi.attended = TRUE
+          AND ws.date_and_time IS NOT NULL
+      ) d
+      WHERE d.rn <= 5
+      GROUP BY d.member_id
+    ) c ON c.member_id = mn.member_id
+    WHERE mn.created_at > c.cutoff
+  SQL
+
+  def recent_notes_member_ids
+    @recent_notes_member_ids ||= begin
+      rows = WorkshopInvitation.connection.select_rows(
+        format(RECENT_NOTES_SQL, member_ids: @member_ids.join(',')),
+        'admin-workshop-recent-notes'
+      )
+      rows.flatten.map(&:to_i)
+    end
+  end
+end

spec/controllers/admin/workshops_controller_spec.rb

@@ -6,13 +6,37 @@
     login_as_organiser(admin, workshop.chapter)
   end
 
+  def count_queries(&block)
+    n = 0
+    callback = ->(*) { n += 1 }
+    ActiveSupport::Notifications.subscribed(callback, 'sql.active_record', &block)
+    n
+  end
+
   describe 'GET #show' do
     it 'loads the workshop attendance page with attendees' do
       Fabricate(:workshop_invitation, workshop: workshop, attending: true)
       get :show, params: { id: workshop.id }
 
       expect(response).to have_http_status(:success)
     end
+
+    it 'loads the attendance page in a bounded number of queries, regardless of attendee count' do
+      attendee = Fabricate(:member)
+      4.times { Fabricate(:past_attending_workshop_invitation, member: attendee) }
+      2.times { Fabricate(:attendance_warning, member: attendee) }
+      Fabricate(:member_note, member: attendee, created_at: 1.day.ago)
+      Fabricate(:workshop_invitation, workshop: workshop, member: attendee, attending: true, role: 'Student')
+      Fabricate(:workshop_invitation, workshop: workshop, attending: true, role: 'Coach')
+
+      # adds a second attendee to catch per-row scaling
+      Fabricate(:workshop_invitation, workshop: workshop, attending: true, role: 'Student')
+
+      count = count_queries { get :show, params: { id: workshop.id } }
+
+      expect(response).to have_http_status(:success)
+      expect(count).to be < 50
+    end
   end
 
   describe 'POST #create' do

spec/queriers/admin_workshop_attendee_flags_spec.rb

@@ -0,0 +1,81 @@
+RSpec.describe AdminWorkshopAttendeeFlags do
+  subject(:flags) { described_class.for_members([member.id])[member.id] }
+
+  let(:workshop) { Fabricate(:workshop) }
+  let(:member) { Fabricate(:member) }
+
+  def count_queries(&block)
+    n = 0
+    callback = ->(*) { n += 1 }
+    ActiveSupport::Notifications.subscribed(callback, 'sql.active_record', &block)
+    n
+  end
+
+  describe '.for_members' do
+    context 'when determining if a member is a newbie' do
+      it 'is true when the member has never attended a workshop' do
+        Fabricate(:attending_workshop_invitation, member: member, workshop: workshop)
+
+        expect(flags[:newbie]).to be(true)
+      end
+
+      it 'is false when the member has attended a workshop in the past' do
+        Fabricate(:attended_workshop_invitation, member: member)
+
+        expect(flags[:newbie]).to be(false)
+      end
+    end
+
+    context 'when determining the flag to organisers' do
+      it 'is true when the member has multiple no-shows and two recent warnings' do
+        4.times { Fabricate(:past_attending_workshop_invitation, member: member) }
+        2.times { Fabricate(:attendance_warning, member: member) }
+
+        expect(flags[:flag_to_organisers]).to be(true)
+      end
+
+      it 'is false when the member has few no-shows' do
+        Fabricate(:past_attending_workshop_invitation, member: member)
+        2.times { Fabricate(:attendance_warning, member: member) }
+
+        expect(flags[:flag_to_organisers]).to be(false)
+      end
+
+      it 'is false when the member has no recent warnings' do
+        4.times { Fabricate(:past_attending_workshop_invitation, member: member) }
+
+        expect(flags[:flag_to_organisers]).to be(false)
+      end
+    end
+
+    context 'when determining recent notes' do
+      it 'is true when a note exists after the member\'s fifth most recent attended workshop' do
+        5.times { Fabricate(:attended_workshop_invitation, member: member) }
+
+        Fabricate(:member_note, member: member, created_at: 1.day.ago)
+
+        expect(flags[:recent_notes]).to be(true)
+      end
+
+      it 'is false when there are no notes' do
+        5.times { Fabricate(:attended_workshop_invitation, member: member) }
+
+        expect(flags[:recent_notes]).to be(false)
+      end
+    end
+
+    it 'runs a constant number of queries regardless of member count' do
+      members = Array.new(5) { Fabricate(:member) }
+      members.each do |current_member|
+        4.times { Fabricate(:past_attending_workshop_invitation, member: current_member) }
+        2.times { Fabricate(:attendance_warning, member: current_member) }
+        Fabricate(:member_note, member: current_member, created_at: 1.day.ago)
+      end
+
+      one = count_queries { described_class.for_members(members.take(1).map(&:id)) }
+      many = count_queries { described_class.for_members(members.map(&:id)) }
+
+      expect(many).to be <= one + 2
+    end
+  end
+end