Skip to content

Commit d5751b5

Browse files
committed
fix: #students and #coaches return Chapter-specific students and coaches
This fixes a bug where the methods returned _all_ of the students and coaches that the user could access Signed-off-by: jonathan.kerr <3410350+jonodrew@users.noreply.github.com>
1 parent 9d34006 commit d5751b5

2 files changed

Lines changed: 36 additions & 6 deletions

File tree

app/models/chapter.rb

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,22 @@ def organisers
3636
@organisers ||= Member.with_role(:organiser, self)
3737
end
3838

39+
3940
def students
40-
Member.joins(:groups)
41-
.merge(Group.students)
42-
.distinct
41+
members_for_group('Students')
4342
end
4443

4544
def coaches
46-
Member.joins(:groups)
47-
.merge(Group.coaches)
48-
.distinct
45+
members_for_group('Coaches')
4946
end
5047

48+
5149
private
5250

51+
def members_for_group(name)
52+
members.where(groups: { name: name }).distinct
53+
end
54+
5355
def expire_chapters_sidebar_cache
5456
Rails.cache.delete('chapters-sidebar')
5557
end

spec/models/chapter_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,32 @@
6565
expect(Rails.cache.read(cache_key)).to be_nil
6666
end
6767
end
68+
69+
describe "helper methods return only that chapter's coaches/students" do
70+
RSpec.shared_examples 'group-scoped members' do |group_name, method_name|
71+
let(:this_chapter) { Fabricate(:chapter) }
72+
let(:that_chapter) { Fabricate(:chapter) }
73+
74+
let!(:this_group) { Fabricate(:group, chapter: this_chapter, name: group_name) }
75+
let!(:that_group) { Fabricate(:group, chapter: that_chapter, name: group_name) }
76+
77+
let!(:this_member) { Fabricate(:member) }
78+
let!(:that_member) { Fabricate(:member) }
79+
80+
before do
81+
Fabricate(:subscription, group: this_group, member: this_member)
82+
Fabricate(:subscription, group: that_group, member: that_member)
83+
end
84+
85+
it "returns only #{group_name.downcase} for the chapter" do
86+
expect(this_chapter.public_send(method_name))
87+
.to contain_exactly(this_member)
88+
end
89+
90+
it "does not include #{group_name.downcase} from another chapter" do
91+
expect(this_chapter.public_send(method_name))
92+
.not_to include(that_member)
93+
end
94+
end
95+
end
6896
end

0 commit comments

Comments
 (0)