Skip to content

Commit

Permalink
Assorted improvements to user email reset flow. (#1724)
Browse files Browse the repository at this point in the history
# What it does

We found a couple issues with the way we handle users resetting their
emails. The main changes are:

* Show the new email in the "resend confirmation email" modal to staff
* Tweak the app layout so that the confirmation success flash message is
shown to members
* Store the email that a notification is going to instead of the value
of `user.email` in `Notification#address`.
  • Loading branch information
jim authored Nov 2, 2024
1 parent 3f1ab0f commit c1a0ce7
Show file tree
Hide file tree
Showing 9 changed files with 30 additions and 12 deletions.
4 changes: 4 additions & 0 deletions app/controllers/account/members_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ def edit
def update
respond_to do |format|
if current_member.update(member_params)
if current_member.saved_change_to_email?
current_member.user.send_confirmation_instructions
end

format.html { redirect_to account_member_url, success: "Profile was successfully updated.", status: :see_other }
format.json { render :show, status: :ok, location: current_member }
else
Expand Down
2 changes: 1 addition & 1 deletion app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class UserMailer < Devise::Mailer
def store_notification
Notification.create!(
action: action_name,
address: @user.email,
address: mail.to.join(","),
member: @user.member,
subject: mail.subject,
uuid: SecureRandom.uuid
Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/members/_profile.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
<div class="toast clearfix member-membership">
<p class="float-left"><i class="icon icon-stop"></i> Member's email address has not been verified.</p>
<p class="float-right">
<%= link_to "Resend Verification Email", resend_verification_email_admin_member_path(@member), data: {turbo_method: "post", turbo_confirm: "Resend verification email to #{@member.user.email}?"} %>
<%= link_to "Resend Verification Email", resend_verification_email_admin_member_path(@member), data: {turbo_method: "post", turbo_confirm: "Resend verification email to #{@member.user.unconfirmed_email}?"} %>
</p>
</div>
<% end %>
Expand Down
2 changes: 2 additions & 0 deletions app/views/admin/members/notifications/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<tr>
<th>Sent at</th>
<th>Summary</th>
<th>To</th>
</tr>
</thead>

Expand All @@ -14,6 +15,7 @@
<tr>
<td><%= date_with_time_title notification.created_at %></td>
<td><%= notification.subject %></td>
<td><%= notification.address %></td>
</tr>
<% end %>
</tbody>
Expand Down
1 change: 1 addition & 0 deletions app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
<% end %>
<div class="app-body">
<%= flash_message :notice %>
<%= flash_message :success %>
<%= flash_message :warning %>
<%= flash_message :error %>
Expand Down
9 changes: 0 additions & 9 deletions test/controllers/account/appointments_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,6 @@ class AppointmentsControllerTest < ActionDispatch::IntegrationTest
@appointment.save
end

private def assert_enqueued_email(mailer, method, params: {})
assert_enqueued_with(job: ActionMailer::MailDeliveryJob, args: ->(job_arguments) {
job_mailer, job_method, _delivery, rest = *job_arguments
assert_equal mailer.to_s, job_mailer
assert_equal method.to_s, job_method
assert_equal(params, rest[:params])
})
end

test "creates a new appointment to pickup items on hold" do
@hold = FactoryBot.create(:started_hold, member: @member)
@event = FactoryBot.create(:appointment_slot_event)
Expand Down
10 changes: 10 additions & 0 deletions test/controllers/account/members_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ class MembersControllerTest < ActionDispatch::IntegrationTest
assert_equal "Updated Name", @member.full_name
end

test "member updates email" do
patch account_member_url, params: {member: {email: "new.email@example.com"}}

sent_email = ActionMailer::Base.deliveries.first
assert_equal ["new.email@example.com"], sent_email.to
assert_equal "Confirmation instructions", sent_email.subject

assert_redirected_to account_member_url
end

test "member update redirects to edit when provided invalid params" do
patch account_member_url, params: {member: {full_name: nil}}
assert_template :edit
Expand Down
2 changes: 2 additions & 0 deletions test/mailers/user_mailer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,7 @@ class UserMailerTest < ActionMailer::TestCase
actions = Notification.pluck(:action)
assert_includes actions, "reset_password_instructions"
assert_includes actions, "email_changed"

assert_equal [user.email, user.email], Notification.pluck(:address)
end
end
10 changes: 9 additions & 1 deletion test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ def assert_size(expected, subject)
end

class ActionDispatch::IntegrationTest
# @note Just to make tests pass for now...
include EnsureRequestTenant

def assert_enqueued_email(mailer, method, params: {})
assert_enqueued_with(job: ActionMailer::MailDeliveryJob, args: ->(job_arguments) {
job_mailer, job_method, _delivery, rest = *job_arguments
assert_equal mailer.to_s, job_mailer
assert_equal method.to_s, job_method
assert_equal(params, rest[:params])
})
end
end

0 comments on commit c1a0ce7

Please sign in to comment.