fix: #students and #coaches return Chapter-specific students and coaches

app/models/README.md

@@ -40,6 +40,13 @@ is only accessed via the `Permission` class and Rolify.
 A `chapter` represents a local organisation. It primarily serves to
 collate things by location and organisers.
 
+`Chapter` exposes two tiers of member access methods. The raw methods
+(`#students`, `#coaches`) return all members subscribed to that group
+regardless of eligibility. The filtered methods (`#eligible_students`,
+`#eligible_coaches`) exclude banned members and those who have not
+accepted the terms of conduct, and should be preferred when building
+workshop invitation lists.
+
 ## Sponsor
 
 A `sponsor` represents a location and organisation. Sponsors have a

app/models/chapter.rb

@@ -36,28 +36,34 @@ def organisers
     @organisers ||= Member.with_role(:organiser, self)
   end
 
+  # All members subscribed to this chapter's Students group, regardless of ban or TOC status.
+  # Use #eligible_students when building invitation lists.
   def students
-    Member.joins(:groups)
-          .merge(Group.students)
-          .distinct
+    members_for_group('Students')
   end
 
+  # All members subscribed to this chapter's Coaches group, regardless of ban or TOC status.
+  # Use #eligible_coaches when building invitation lists.
   def coaches
-    Member.joins(:groups)
-          .merge(Group.coaches)
-          .distinct
+    members_for_group('Coaches')
   end
 
+  # Chapter students who are not banned and have accepted the TOC — safe to invite to workshops.
   def eligible_students
     Member.in_group(groups.students).distinct
   end
 
+  # Chapter coaches who are not banned and have accepted the TOC — safe to invite to workshops.
   def eligible_coaches
     Member.in_group(groups.coaches).distinct
   end
 
   private
 
+  def members_for_group(name)
+    members.where(groups: { name: name }).distinct
+  end
+
   def expire_chapters_sidebar_cache
     Rails.cache.delete('chapters-sidebar')
   end

spec/models/chapter_spec.rb

@@ -66,6 +66,34 @@
     end
   end
 
+  describe '#students' do
+    let(:chapter) { Fabricate(:chapter) }
+    let(:student_group) { Fabricate(:group, chapter: chapter, name: 'Students') }
+
+    it 'returns only students from this chapter' do
+      other_chapter = Fabricate(:chapter)
+      other_group = Fabricate(:group, chapter: other_chapter, name: 'Students')
+      this_student = Fabricate(:member, groups: [student_group])
+      _other_student = Fabricate(:member, groups: [other_group])
+
+      expect(chapter.students).to contain_exactly(this_student)
+    end
+  end
+
+  describe '#coaches' do
+    let(:chapter) { Fabricate(:chapter) }
+    let(:coach_group) { Fabricate(:group, chapter: chapter, name: 'Coaches') }
+
+    it 'returns only coaches from this chapter' do
+      other_chapter = Fabricate(:chapter)
+      other_group = Fabricate(:group, chapter: other_chapter, name: 'Coaches')
+      this_coach = Fabricate(:member, groups: [coach_group])
+      _other_coach = Fabricate(:member, groups: [other_group])
+
+      expect(chapter.coaches).to contain_exactly(this_coach)
+    end
+  end
+
   describe '#eligible_students' do
     let(:chapter) { Fabricate(:chapter) }
     let(:student_group) { Fabricate(:group, chapter: chapter, name: 'Students') }