Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring registration emails in line with v2 statuses #10720

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions app/mailers/registrations_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,6 @@ def notify_registrant_of_accepted_registration(registration)
reply_to: registration.competition.organizers_or_delegates.map(&:email)
end

def notify_registrant_of_pending_registration(registration)
@registration = registration
localized_mail @registration.user.locale,
-> { I18n.t('registrations.mailer.pending.mail_subject', comp_name: registration.competition.name) },
to: registration.user.email,
reply_to: registration.competition.organizers_or_delegates.map(&:email)
end

def notify_registrant_of_deleted_registration(registration)
@registration = registration
localized_mail @registration.user.locale,
Expand All @@ -80,4 +72,12 @@ def notify_registrant_of_locked_account_creation(user, competition)
subject: "Unlock your new account on the WCA website",
)
end

def notify_registrant_of_waitlisted_registration(registration)
@registration = registration
localized_mail @registration.user.locale,
-> { I18n.t('registrations.mailer.waiting_list.mail_subject', comp_name: registration.competition.name) },
to: registration.user.email,
reply_to: registration.competition.organizers_or_delegates.map(&:email)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@
<li>
<%= t 'registrations.mailer.deleted.causes.missing_info' %>
</li>
<li>
<%= t 'registrations.mailer.pending.causes.withdrawal' %>
</li>
<li>
<%= t 'registrations.mailer.deleted.causes.incomplete' %>
</li>
Expand All @@ -26,7 +23,7 @@
</ul>

<p>
<%= t 'registrations.mailer.pending.email_if_error' %>
<%= t 'registrations.mailer.deleted.email_if_error' %>
</p>

<%= render partial: 'organizers_or_delegates_signature' %>

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<% @competition = @registration.competition %>

<p>
<%= t 'registrations.mailer.waiting_list.waitlisted_html', here: link_to(t('common.here'), competition_register_url(@competition)) %>
</p>

<p>
<%= t 'registrations.mailer.waiting_list.email_if_error' %>
</p>

<%= render partial: 'organizers_or_delegates_signature' %>
14 changes: 5 additions & 9 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1192,27 +1192,23 @@ en:
#context: "comp_name" contains the competition's name
see_you_soon: "We look forward to seeing you at %{comp_name}!"
#context: when registration is moved to waiting list
pending:
waiting_list:
#context: "comp_name" contains the competition's name
mail_subject: "Registration Moved to Waitlist: %{comp_name}"
#context: "comp_name" contains the competition's name
moved_html: "Your registration for %{comp_name} has been moved to the waiting list. Possible causes are:"
#context: this key is one of the reasons explaining why a registration could be moved to the waiting list
causes:
accepted: "Your registration was accidentally accepted"
withdrawal: "You asked for withdrawal"
mail_subject: "You're on the Waiting List: %{comp_name}"
waitlisted_html: "The competition is full, but you have been placed on a waiting list, and you will receive an email if enough spots open up for you to be able to attend. Check your position on the waiting list %{here}."
email_if_error: "If you think this is an error, please reply to this email."
#context: when registration is deleted
deleted:
#context: "comp_name" contains the competition's name
mail_subject: "Registration Deleted: %{comp_name}"
message_html: "Your registration for %{comp_name} has been deleted. Possible causes are:"
email_if_error: "If you think this is an error, please reply to this email."
#context: this key is one of the reasons explaining why a registration could be deleted
causes:
missing_info: "You did not submit all the required details"
incomplete: "You didn't meet the registration criteria (for example, paying the registration fee in advance or meeting the qualifying times for events)"
registrations_full: "The competitor limit has been reached and there are no more spots available"
multiple_registrations: "You registered twice or more times"
multiple_registrations: "You registered two or more times"
competition_canceled: "The competition has been canceled"
#context: on the registrations list
list:
Expand Down
8 changes: 4 additions & 4 deletions lib/registrations/lanes/competing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def self.update!(update_params, competition, current_user_id)
registration.add_history_entry(changes, 'user', current_user_id, Registrations::Helper.action_type(update_params, current_user_id))
end

send_status_change_email(registration, status, user_id, current_user_id) if status.present? && old_status != status
send_status_change_email(registration, status, old_status, user_id, current_user_id) if status.present? && old_status != status

# TODO: V3-REG Cleanup Figure out a way to get rid of this reload
registration.reload
Expand All @@ -85,12 +85,12 @@ def self.update_status(registration, status)
registration.competing_status = status
end

def self.send_status_change_email(registration, status, user_id, current_user_id)
def self.send_status_change_email(registration, status, old_status, user_id, current_user_id)
case status
when Registrations::Helper::STATUS_WAITING_LIST
# TODO: V3-REG Cleanup, at new waiting list email
RegistrationsMailer.notify_registrant_of_waitlisted_registration(registration).deliver_later
when Registrations::Helper::STATUS_PENDING
RegistrationsMailer.notify_registrant_of_pending_registration(registration).deliver_later
RegistrationsMailer.notify_registrant_of_new_registration(registration).deliver_later
when Registrations::Helper::STATUS_ACCEPTED
RegistrationsMailer.notify_registrant_of_accepted_registration(registration).deliver_later
when Registrations::Helper::STATUS_REJECTED, Registrations::Helper::STATUS_DELETED, Registrations::Helper::STATUS_CANCELLED
Expand Down
17 changes: 6 additions & 11 deletions spec/mailers/registrations_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
let(:registration) { FactoryBot.create(:registration, user: french_user, competition: competition_with_organizers) }
let(:mail_new) { RegistrationsMailer.notify_registrant_of_new_registration(registration) }
let(:mail_accepted) { RegistrationsMailer.notify_registrant_of_accepted_registration(registration) }
let(:mail_pending) { RegistrationsMailer.notify_registrant_of_pending_registration(registration) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this disappearing without replacement? There is notify_registrant_of_waitlisted_registration now, which you could well put here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The header test seems to rely on a french translation key which we don't have - should I add it in in order for the test to pass? I can find some french people to consult with

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, just a comment that we should add this as soon as French is translated should suffice

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, French does get updated rather infrequently.
If you do indeed have contacts to French speakers (ideally, cubers) it would be great if you could add the relevant snippets.

let(:mail_deleted) { RegistrationsMailer.notify_registrant_of_deleted_registration(registration) }

it "renders the headers in foreign locale" do
Expand All @@ -26,7 +25,6 @@
# scope this call, but rather compare the result to the expected locale.
expect(mail_new.subject).to eq(I18n.t('registrations.mailer.new.mail_subject', comp_name: registration.competition.name, locale: :fr))
expect(mail_accepted.subject).to eq(I18n.t('registrations.mailer.accepted.mail_subject', comp_name: registration.competition.name, locale: :fr))
expect(mail_pending.subject).to eq(I18n.t('registrations.mailer.pending.mail_subject', comp_name: registration.competition.name, locale: :fr))
expect(mail_deleted.subject).to eq(I18n.t('registrations.mailer.deleted.mail_subject', comp_name: registration.competition.name, locale: :fr))
end

Expand All @@ -41,7 +39,6 @@
end
expect(mail_new.body.encoded).to match(regards_in_french)
expect(mail_accepted.body.encoded).to match(regards_in_french)
expect(mail_pending.body.encoded).to match(regards_in_french)
expect(mail_deleted.body.encoded).to match(regards_in_french)
end
end
Expand Down Expand Up @@ -169,26 +166,24 @@
end
end

describe "notify_registrant_of_pending_registration for a competition without organizers" do
let(:mail) { RegistrationsMailer.notify_registrant_of_pending_registration(registration) }
describe "notify_registrant_of_waitlisted_registration for competition without organizers" do
let(:mail) { RegistrationsMailer.notify_registrant_of_waitlisted_registration(registration) }
let(:registration) { FactoryBot.create(:registration, competition: competition_without_organizers) }

it "renders the headers" do
expect(mail.subject).to eq("Registration Moved to Waitlist: #{competition_without_organizers.name}")
expect(mail.subject).to eq("You're on the Waiting List: #{competition_without_organizers.name}")
expect(mail.to).to eq([registration.email])
expect(mail.reply_to).to eq(competition_without_organizers.delegates.map(&:email))
expect(mail.from).to eq(["[email protected]"])
end

it "renders the body" do
expect(mail.body.encoded).to match("Your registration for .{1,200}#{registration.competition.name}.{1,200} has been moved to the waiting list")
expect(mail.body.encoded).to match("If you think this is an error, please reply to this email.")
expect(mail.body.encoded).to match("Regards, #{users_to_sentence(competition_without_organizers.organizers_or_delegates)}.")
expect(mail.body.encoded).to match("The competition is full, but you have been placed on a waiting list, and you will receive an email if enough spots open up for you to be able to attend.")
end
end

describe "notify_registrant_of_pending_registration for a competition with organizers" do
let(:mail) { RegistrationsMailer.notify_registrant_of_pending_registration(registration) }
describe "notify_registrant_of_waitlisted_registration for competition with organizers" do
let(:mail) { RegistrationsMailer.notify_registrant_of_waitlisted_registration(registration) }
let(:registration) { FactoryBot.create(:registration, competition: competition_with_organizers) }

it "sets organizers in the reply_to" do
Expand Down
45 changes: 44 additions & 1 deletion spec/requests/api_registrations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@

describe 'PATCH #update' do
let(:user) { FactoryBot.create :user }
let(:competition) { FactoryBot.create :competition, :registration_open, :editable_registrations }
let(:competition) { FactoryBot.create :competition, :registration_open, :editable_registrations, :with_organizer }
let(:registration) { FactoryBot.create(:registration, competition: competition, user: user) }

it 'updates a registration' do
Expand Down Expand Up @@ -164,6 +164,49 @@
registration = Registration.find_by(user_id: user.id, competition_id: favourites_reg.competition.id)
expect(registration.event_ids.sort).to eq(new_event_ids.sort)
end

it 'user gets registration email if they cancel and re-register' do
perform_enqueued_jobs do
gregorbg marked this conversation as resolved.
Show resolved Hide resolved
cancelled_reg = FactoryBot.create(:registration, :cancelled, competition: competition)

update_request = FactoryBot.build(
:update_request,
user_id: cancelled_reg.user_id,
competition_id: cancelled_reg.competition.id,
competing: { 'status' => 'pending' },
)
headers = { 'Authorization' => update_request['jwt_token'] }

patch api_v1_registrations_register_path, params: update_request, headers: headers

expect(response.status).to eq(200)

email = ActionMailer::Base.deliveries.last
expect(email.subject).to eq(I18n.t('registrations.mailer.new.mail_subject', comp_name: registration.competition.name))
end
end

it 'user gets registration email if they were rejected and get moved to pending' do
perform_enqueued_jobs do
gregorbg marked this conversation as resolved.
Show resolved Hide resolved
rejected_reg = FactoryBot.create(:registration, :rejected, competition: competition)

update_request = FactoryBot.build(
:update_request,
user_id: rejected_reg.user_id,
competition_id: rejected_reg.competition.id,
submitted_by: competition.organizers.first.id,
competing: { 'status' => 'pending' },
)
headers = { 'Authorization' => update_request['jwt_token'] }

patch api_v1_registrations_register_path, params: update_request, headers: headers

expect(response.status).to eq(200)

email = ActionMailer::Base.deliveries.last
expect(email.subject).to eq(I18n.t('registrations.mailer.new.mail_subject', comp_name: registration.competition.name))
end
end
end

describe 'PATCH #bulk_update' do
Expand Down