Merge pull request #2501 from mroderick/fix/member-attending-cache

feat: cache member attending event IDs to eliminate N+1 queries

Author: olleolleolle

app/models/invitation.rb

@@ -13,6 +13,8 @@ class Invitation < ApplicationRecord
   scope :coaches, -> { where(role: 'Coach') }
   scope :verified, -> { where(verified: true).order(:updated_at) }
 
+  after_save :clear_member_cache, if: :saved_change_to_attending?
+
   def student_spaces?
     for_student? && event.student_spaces?
   end
@@ -24,4 +26,10 @@ def coach_spaces?
   def to_param
     token
   end
+
+  private
+
+  def clear_member_cache
+    member.clear_attending_event_ids_cache!
+  end
 end

app/models/meeting_invitation.rb

@@ -10,5 +10,13 @@ class MeetingInvitation < ApplicationRecord
   scope :accepted, -> { where(attending: true) }
   scope :attended, -> { where(attended: true) }
 
+  after_save :clear_member_cache, if: :saved_change_to_attending?
+
   alias event meeting
+
+  private
+
+  def clear_member_cache
+    member.clear_attending_event_ids_cache!
+  end
 end

app/models/member.rb

@@ -121,6 +121,19 @@ def past_rsvps
     @past_rsvps ||= rsvps(period: :past).reverse
   end
 
+  def attending_event_ids
+    @attending_event_ids ||= begin
+      event_ids     = invitations.accepted.pluck(:event_id)
+      workshop_ids  = workshop_invitations.accepted.pluck(:workshop_id)
+      meeting_ids   = meeting_invitations.accepted.pluck(:meeting_id)
+      (event_ids + workshop_ids + meeting_ids).to_set
+    end
+  end
+
+  def clear_attending_event_ids_cache!
+    @attending_event_ids = nil
+  end
+
   def flag_to_organisers?
     multiple_no_shows? && attendance_warnings.last_six_months.length >= 2
   end

app/models/workshop_invitation.rb

@@ -25,6 +25,8 @@ class WorkshopInvitation < ApplicationRecord
   scope :on_waiting_list, -> { joins(:waiting_list) }
   scope :with_notes_and_their_authors, -> { includes(member: { member_notes: :author }) }
 
+  after_save :clear_member_cache, if: :saved_change_to_attending?
+
   def waiting_list_position
     @waiting_list_position ||= WaitingList.by_workshop(workshop)
                                           .where_role(role)
@@ -47,4 +49,10 @@ def student_attending?
   def not_attending?
     attending == false
   end
+
+  private
+
+  def clear_member_cache
+    member.clear_attending_event_ids_cache!
+  end
 end

app/presenters/member_presenter.rb

@@ -12,7 +12,7 @@ def newbie?
   end
 
   def attending?(event)
-    event.invitations.accepted.where(member: model).exists?
+    model.attending_event_ids.include?(event.id)
   end
 
   def subscribed_to_newsletter?

spec/models/invitation_spec.rb

@@ -12,19 +12,30 @@
     it { is_expected.to validate_inclusion_of(:role).in_array(%w[Student Coach]) }
   end
 
-  context '#student_spaces?' do
+  describe '#student_spaces?' do
     it 'checks if there are any available spaces for students at the event' do
       student_invitation = Fabricate(:invitation)
 
       expect(student_invitation.student_spaces?).to eq(true)
     end
   end
 
-  context '#coach_spaces?' do
+  describe '#coach_spaces?' do
     it 'checks if there are any available spaces for coaches at the event' do
       coach_invitation = Fabricate(:coach_invitation)
 
       expect(coach_invitation.coach_spaces?).to eq(true)
     end
   end
+
+  describe 'cache invalidation' do
+    let(:member) { Fabricate(:member) }
+    let(:event) { Fabricate(:event) }
+
+    it 'clears member cache when attending changes' do
+      invitation = Fabricate(:invitation, member: member, event: event, attending: false)
+      expect(member).to receive(:clear_attending_event_ids_cache!)
+      invitation.update!(attending: true)
+    end
+  end
 end

spec/models/member_spec.rb

@@ -44,7 +44,7 @@
           outside_deadline = latest_workshops.last.workshop.date_and_time
           within_deadline = latest_workshops.fifth.workshop.date_and_time
 
-          old_note = Fabricate.create(:member_note, member: member, created_at: outside_deadline)
+          Fabricate.create(:member_note, member: member, created_at: outside_deadline)
           new_note = Fabricate.create(:member_note, member: member, created_at: within_deadline)
 
           expect(member.recent_notes.to_a).to eq([new_note])
@@ -210,4 +210,47 @@
       expect(managers.size).to eq(managers.distinct.size)
     end
   end
+
+  describe '#attending_event_ids' do
+    let(:member) { Fabricate(:member) }
+
+    it 'returns event IDs where member has accepted invitation' do
+      event = Fabricate(:event)
+      Fabricate(:invitation, member: member, event: event, attending: true)
+      expect(member.attending_event_ids).to include(event.id)
+    end
+
+    it 'does not include events where invitation is not accepted' do
+      event = Fabricate(:event)
+      Fabricate(:invitation, member: member, event: event, attending: false)
+      expect(member.attending_event_ids).not_to include(event.id)
+    end
+
+    it 'includes workshop IDs' do
+      workshop = Fabricate(:workshop)
+      Fabricate(:workshop_invitation, member: member, workshop: workshop, attending: true)
+      expect(member.attending_event_ids).to include(workshop.id)
+    end
+
+    it 'includes meeting IDs' do
+      meeting = Fabricate(:meeting)
+      Fabricate(:meeting_invitation, member: member, meeting: meeting, attending: true)
+      expect(member.attending_event_ids).to include(meeting.id)
+    end
+
+    it 'caches result in instance variable' do
+      event = Fabricate(:event)
+      Fabricate(:invitation, member: member, event: event, attending: true)
+      first_call = member.attending_event_ids
+      expect(member.attending_event_ids).to equal(first_call)
+    end
+
+    it 'can be cleared and re-queries on next call' do
+      event = Fabricate(:event)
+      Fabricate(:invitation, member: member, event: event, attending: true)
+      member.attending_event_ids
+      member.clear_attending_event_ids_cache!
+      expect(member.attending_event_ids).to include(event.id)
+    end
+  end
 end

spec/presenters/member_presenter_spec.rb

@@ -14,7 +14,7 @@
     member_presenter.subscribed_to_newsletter?
   end
 
-  context '#pairing_details_array' do
+  describe '#pairing_details_array' do
     it 'returns student pairing information' do
       expect(member_presenter.pairing_details_array('Student', 'Tutorial', 'Note'))
         .to eq([member_presenter.newbie?, member.full_name, 'Student', 'Tutorial', 'Note', 'N/A'])
@@ -25,4 +25,17 @@
         .to eq([member_presenter.newbie?, member.full_name, 'Coach', 'N/A', 'A note', 'java, ruby'])
     end
   end
+
+  describe '#attending?' do
+    let(:event) { Fabricate(:event) }
+
+    it 'returns true when member is attending event' do
+      Fabricate(:invitation, member: member, event: event, attending: true)
+      expect(member_presenter.attending?(event)).to be true
+    end
+
+    it 'returns false when member is not attending' do
+      expect(member_presenter.attending?(event)).to be false
+    end
+  end
 end