Skip to content

Commit d71f4ca

Browse files
jonodrewjonodrew
authored
Merge pull request #2518 from octoberclub/email-users-code
Fix ThreeMonthEmailService query to check member selection for chaser emails
2 parents 1996d15 + 6515edd commit d71f4ca

3 files changed

Lines changed: 202 additions & 36 deletions

File tree

‎app/mailers/member_mailer.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ class MemberMailer < ApplicationMailer
22
include EmailHeaderHelper
33
include EmailDelivery
44

5-
after_deliver :log_sent_mail , only: [:chaser]
5+
after_deliver :log_sent_email, only: [:chaser]
66

77
def chaser
88
@member = params[:member]

‎app/services/three_month_email_service.rb‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,32 @@
22

33
class ThreeMonthEmailService
44
def self.send_chaser
5-
recent_attendees = Member.joins(:workshop_invitations)
6-
.merge(
7-
WorkshopInvitation.attended.to_students
8-
.joins(:workshop)
9-
.where('workshops.date_and_time >= ?', 3.months.ago.beginning_of_day)
10-
)
11-
.distinct
5+
three_month_cutoff = 3.months.ago.beginning_of_day
6+
one_year_cutoff = 1.year.ago.beginning_of_day
127

13-
members = Member.not_banned
14-
.accepted_toc
15-
.joins(:groups)
16-
.merge(Group.students)
17-
.left_joins(:member_email_deliveries)
18-
.where(member_email_deliveries: { id: nil })
19-
.where.not(id: recent_attendees.select(:id))
20-
.distinct
8+
recent_attendee_ids = WorkshopInvitation.to_students
9+
.attended
10+
.joins(:workshop)
11+
.where('workshops.date_and_time >= ?', three_month_cutoff)
12+
.select(:member_id)
13+
14+
past_year_attendee_ids = WorkshopInvitation.to_students
15+
.attended
16+
.joins(:workshop)
17+
.where('workshops.date_and_time >= ?', one_year_cutoff)
18+
.select(:member_id)
19+
20+
members = Member.not_banned
21+
.accepted_toc
22+
.joins(:groups)
23+
.merge(Group.students)
24+
.left_joins(:member_email_deliveries)
25+
.where(member_email_deliveries: { id: nil })
26+
.where.not(id: recent_attendee_ids)
27+
.where(id: past_year_attendee_ids)
28+
.distinct
2129
return if members.empty?
30+
2231
members.find_each do |member|
2332
MemberMailer.with(member: member).chaser.deliver_later
2433
end

‎spec/services/three_month_email_service_spec.rb‎

Lines changed: 177 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,42 +2,199 @@
22
describe "#send_chaser" do
33
subject(:call) { described_class.send_chaser }
44

5-
let!(:eligible_member) { Fabricate(:member) }
6-
let!(:emailed_member) { Fabricate(:member) }
7-
let!(:old_invite_member) { Fabricate(:member) }
5+
let(:chapter) { Fabricate(:chapter) }
6+
let(:students_group) { Fabricate(:group, name: "Students", chapter: chapter) }
7+
let(:coaches_group) { Fabricate(:group, name: "Coaches", chapter: chapter) }
88

9-
before do
10-
# Eligible: recent invite, no email delivery
9+
let!(:eligible_student) do
10+
member = Fabricate(:member)
11+
Fabricate(:subscription, member: member, group: students_group)
1112
Fabricate(
1213
:workshop_invitation,
13-
member: eligible_member,
14-
created_at: 3.months.ago,
15-
attended: false
14+
member: member,
15+
workshop: Fabricate(:workshop, chapter: chapter, date_and_time: 6.months.ago),
16+
role: "Student",
17+
attended: true
1618
)
19+
member
20+
end
21+
22+
let!(:already_emailed_student) do
23+
member = Fabricate(:member)
24+
Fabricate(:subscription, member: member, group: students_group)
25+
Fabricate(:member_email_delivery, member: member)
26+
member
27+
end
1728

18-
# Already emailed: recent invite, but has email delivery
29+
let!(:student_with_recent_attendance) do
30+
member = Fabricate(:member)
31+
Fabricate(:subscription, member: member, group: students_group)
1932
Fabricate(
2033
:workshop_invitation,
21-
member: emailed_member,
22-
created_at: 2.months.ago
34+
member: member,
35+
workshop: Fabricate(:workshop, chapter: chapter, date_and_time: 1.month.ago),
36+
role: "Student",
37+
attended: true
2338
)
39+
member
40+
end
41+
42+
let!(:student_with_old_attendance) do
43+
member = Fabricate(:member)
44+
Fabricate(:subscription, member: member, group: students_group)
2445
Fabricate(
25-
:member_email_delivery,
26-
member: emailed_member
46+
:workshop_invitation,
47+
member: member,
48+
workshop: Fabricate(:workshop, chapter: chapter, date_and_time: 4.months.ago),
49+
role: "Student",
50+
attended: true
2751
)
52+
member
53+
end
2854

29-
# Old invite: more than 3 months ago
55+
let!(:coach_member) do
56+
member = Fabricate(:member)
57+
Fabricate(:subscription, member: member, group: coaches_group)
58+
member
59+
end
60+
61+
let!(:unsubscribed_member) { Fabricate(:member) }
62+
let!(:banned_student) do
63+
member = Fabricate(:banned_member)
64+
Fabricate(:subscription, member: member, group: students_group)
65+
member
66+
end
67+
let!(:student_without_toc) do
68+
member = Fabricate(:member_without_toc)
69+
Fabricate(:subscription, member: member, group: students_group)
70+
member
71+
end
72+
73+
let!(:student_with_very_old_attendance) do
74+
member = Fabricate(:member)
75+
Fabricate(:subscription, member: member, group: students_group)
3076
Fabricate(
3177
:workshop_invitation,
32-
member: old_invite_member,
33-
created_at: 4.months.ago
78+
member: member,
79+
workshop: Fabricate(:workshop, chapter: chapter, date_and_time: 14.months.ago),
80+
role: "Student",
81+
attended: true
3482
)
83+
member
84+
end
85+
86+
it "emails only students who have not attended in the last 3 months and were not emailed before" do
87+
expect { perform_enqueued_jobs { call } }.to change(MemberEmailDelivery, :count).by(2)
88+
89+
expect(MemberEmailDelivery.where(member: eligible_student)).to exist
90+
expect(MemberEmailDelivery.where(member: student_with_old_attendance)).to exist
3591
end
3692

37-
it "enqueues chaser emails only for eligible members" do
38-
expect {
39-
call
40-
}.to have_enqueued_mail(MemberMailer, :chaser).once
93+
it "does not email a member already present in member_email_deliveries" do
94+
expect { perform_enqueued_jobs { call } }
95+
.not_to change { MemberEmailDelivery.where(member: already_emailed_student).count }
96+
end
97+
98+
it "does not email students with a recent attended workshop" do
99+
expect { perform_enqueued_jobs { call } }
100+
.not_to change { MemberEmailDelivery.where(member: student_with_recent_attendance).count }
101+
end
102+
103+
it "does not email members without a student subscription" do
104+
perform_enqueued_jobs { call }
105+
106+
expect(MemberEmailDelivery.where(member: coach_member)).to be_empty
107+
expect(MemberEmailDelivery.where(member: unsubscribed_member)).to be_empty
108+
end
109+
110+
it "does not email banned students or students without accepted terms" do
111+
perform_enqueued_jobs { call }
112+
113+
expect(MemberEmailDelivery.where(member: banned_student)).to be_empty
114+
expect(MemberEmailDelivery.where(member: student_without_toc)).to be_empty
115+
end
116+
117+
it "does not email students who have not attended in the past year" do
118+
perform_enqueued_jobs { call }
119+
120+
expect(MemberEmailDelivery.where(member: student_with_very_old_attendance)).to be_empty
121+
end
122+
123+
it "sends only one chaser for a member with multiple student subscriptions" do
124+
member = Fabricate(:member)
125+
other_chapter = Fabricate(:chapter)
126+
other_students_group = Fabricate(:group, name: "Students", chapter: other_chapter)
127+
Fabricate(:subscription, member: member, group: students_group)
128+
Fabricate(:subscription, member: member, group: other_students_group)
129+
130+
perform_enqueued_jobs { call }
131+
132+
expect(MemberEmailDelivery.where(member: member).count).to eq(1)
133+
end
134+
135+
it "sends only one chaser for a member with multiple qualifying old attendances" do
136+
member = Fabricate(:member)
137+
Fabricate(:subscription, member: member, group: students_group)
138+
Fabricate(
139+
:workshop_invitation,
140+
member: member,
141+
workshop: Fabricate(:workshop, chapter: chapter, date_and_time: 5.months.ago),
142+
role: "Student",
143+
attended: true
144+
)
145+
Fabricate(
146+
:workshop_invitation,
147+
member: member,
148+
workshop: Fabricate(:workshop, chapter: chapter, date_and_time: 4.months.ago),
149+
role: "Student",
150+
attended: true
151+
)
152+
153+
perform_enqueued_jobs { call }
154+
155+
expect(MemberEmailDelivery.where(member: member).count).to eq(1)
156+
end
157+
158+
it "does not send chasers when there are no eligible members" do
159+
Fabricate(
160+
:workshop_invitation,
161+
member: eligible_student,
162+
workshop: Fabricate(:workshop, chapter: chapter, date_and_time: 1.month.ago),
163+
role: "Student",
164+
attended: true
165+
)
166+
Fabricate(
167+
:workshop_invitation,
168+
member: student_with_old_attendance,
169+
workshop: Fabricate(:workshop, chapter: chapter, date_and_time: 1.month.ago),
170+
role: "Student",
171+
attended: true
172+
)
173+
174+
expect { perform_enqueued_jobs { call } }.not_to change(MemberEmailDelivery, :count)
175+
end
176+
177+
it "emails a student member who has recent attendance only as a coach" do
178+
member = Fabricate(:member)
179+
Fabricate(:subscription, member: member, group: students_group)
180+
Fabricate(
181+
:workshop_invitation,
182+
member: member,
183+
workshop: Fabricate(:workshop, chapter: chapter, date_and_time: 6.months.ago),
184+
role: "Student",
185+
attended: true
186+
)
187+
Fabricate(
188+
:workshop_invitation,
189+
member: member,
190+
workshop: Fabricate(:workshop, chapter: chapter, date_and_time: 1.month.ago),
191+
role: "Coach",
192+
attended: true
193+
)
194+
195+
perform_enqueued_jobs { call }
196+
197+
expect(MemberEmailDelivery.where(member: member)).to exist
41198
end
42199
end
43200
end

0 commit comments

Comments
 (0)