From f81a88d616ef2c794f333aea9058ee85135c503d Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Tue, 29 Oct 2024 12:27:21 +0000
Subject: [PATCH] Inline email update form into /account

Note there are two fields on this page labelled "Current password" so
we have to scope the Capybara selector in the system test more
specifically to avoid errors.
---
 .../email_verifications_controller.rb         |  2 +-
 app/controllers/identity/emails_controller.rb | 17 ++++-----
 app/views/identity/emails/edit.html.erb       | 21 ----------
 app/views/users/show.html.erb                 | 22 ++++++++++-
 config/routes.rb                              |  2 +-
 .../email_verifications_controller_test.rb    |  2 +-
 .../identity/emails_controller_test.rb        | 11 ++----
 test/controllers/users_controller_test.rb     |  5 ---
 test/system/identity/emails_test.rb           | 38 ++++++++++++-------
 test/system/passwords_test.rb                 | 26 +++++++++----
 10 files changed, 76 insertions(+), 70 deletions(-)
 delete mode 100644 app/views/identity/emails/edit.html.erb

diff --git a/app/controllers/identity/email_verifications_controller.rb b/app/controllers/identity/email_verifications_controller.rb
index ff7520eb..8805b46a 100644
--- a/app/controllers/identity/email_verifications_controller.rb
+++ b/app/controllers/identity/email_verifications_controller.rb
@@ -26,7 +26,7 @@ def set_user
       token = EmailVerificationToken.find_signed!(params[:sid])
       @user = token.user
     rescue StandardError
-      redirect_to edit_identity_email_path, alert: 'That email verification link is invalid'
+      redirect_to root_path, alert: 'That email verification link is invalid'
     end
 
     def send_email_verification
diff --git a/app/controllers/identity/emails_controller.rb b/app/controllers/identity/emails_controller.rb
index b317dd26..59a2c642 100644
--- a/app/controllers/identity/emails_controller.rb
+++ b/app/controllers/identity/emails_controller.rb
@@ -4,19 +4,16 @@ module Identity
   class EmailsController < ApplicationController
     before_action :set_user
 
-    def edit
-      authorize @user
-    end
-
     def update
       authorize @user
 
       if !@user.authenticate(params[:current_password])
-        redirect_to edit_identity_email_path, alert: 'The password you entered is incorrect'
+        flash[:emails_update_password_incorrect] = 'The password you entered is incorrect'
+        redirect_to account_path
       elsif @user.update(email: params[:email])
-        redirect_to_root
+        resend_verification_email_and_redirect
       else
-        render :edit, status: :unprocessable_entity
+        render 'users/show', status: :unprocessable_entity
       end
     end
 
@@ -26,12 +23,12 @@ def set_user
       @user = Current.user
     end
 
-    def redirect_to_root
+    def resend_verification_email_and_redirect
       if @user.email_previously_changed?
         resend_email_verification
-        redirect_to root_path, notice: 'Your email has been changed'
+        redirect_to account_path, notice: 'Your email has been changed'
       else
-        redirect_to root_path
+        redirect_to account_path
       end
     end
 
diff --git a/app/views/identity/emails/edit.html.erb b/app/views/identity/emails/edit.html.erb
deleted file mode 100644
index 46fe3926..00000000
--- a/app/views/identity/emails/edit.html.erb
+++ /dev/null
@@ -1,21 +0,0 @@
-<% if Current.user.verified? %>
-  <%= render 'shared/page_header', text: 'Change your email' %>
-<% else %>
-  <%= render 'shared/page_header', text: 'Verify your email' %>
-
-  <div class="my-6 border border-slate-300 p-4">
-    <p>We sent a verification email to the address below. Check that email and follow those instructions to confirm it's your email address.</p>
-    <p><%= button_to "Re-send verification email", identity_email_verification_path, class: 'py-3 px-5 bg-amber-600 hover:bg-amber-500 text-white font-medium cursor-pointer my-3' %></p>
-  </div>
-<% end %>
-
-<%= form_with(url: identity_email_path, method: :patch, builder: TailwindFormBuilder) do |form| %>
-  <%= render ModelErrorComponent.new(model: @user) %>
-  <%= form.email_field :email, label: { text: "New email" }, required: true, autofocus: true, class: 'w-full mb-3' %>
-  <%= form.password_field :current_password, required: true, autocomplete: "current-password", class: 'w-full mb-3' %>
-  <%= form.submit "Save changes", class: 'w-full mt-3' %>
-<% end %>
-
-<div class="mt-6">
-  <%= text_link_to "Back", root_path %>
-</div>
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index a03946b9..4ebb5a17 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -27,7 +27,26 @@
       <h2 class="text-lg font-bold">Email address</h2>
     </div>
     <div class="basis-3/4">
-      <p>form goes here</p>
+      <% unless Current.user.verified? %>
+        <div class="mb-3 border border-slate-300 p-4">
+          <p>We sent a verification email to the address below. Check that email and follow those instructions to confirm it's your email address.</p>
+          <p><%= button_to "Re-send verification email", identity_email_verification_path, class: 'py-3 px-5 bg-amber-600 hover:bg-amber-500 text-white font-medium cursor-pointer my-3' %></p>
+        </div>
+      <% end %>
+
+      <%= form_with(url: identity_email_path, method: :patch, builder: TailwindFormBuilder) do |form| %>
+        <% if flash[:emails_update_password_incorrect] %>
+          <%= render ErrorBoxComponent.new(title: 'Incorrect password').with_content(flash[:emails_update_password_incorrect]) %>
+        <% end %>
+        <% if @user.errors.include?(:email) %>
+          <%= render ModelErrorComponent.new(model: @user) %>
+        <% end %>
+        <%= form.email_field :email, label: { text: "New email" }, required: true, autofocus: true, class: 'w-full mb-3' %>
+        <%= form.password_field :current_password, required: true, autocomplete: "current-password", class: 'w-full mb-3' %>
+        <div class="flex flex-row justify-end">
+          <%= form.submit "Save changes", class: 'mt-3' %>
+        </div>
+      <% end %>
     </div>
   </section>
   <section class="flex flex-row py-6">
@@ -41,7 +60,6 @@
 </div>
 
 <ul>
-  <li><%= text_link_to "Change email address", edit_identity_email_path %></li>
   <% if Current.user.payout_detail %>
     <li><%= text_link_to "Edit payout details", edit_payout_detail_path %></li>
   <% else %>
diff --git a/config/routes.rb b/config/routes.rb
index 60f2b7af..0ca31703 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -22,7 +22,7 @@
   resolve('PayoutDetail') { [:payout_detail] }
 
   namespace :identity do
-    resource :email,              only: %i[edit update]
+    resource :email,              only: %i[update]
     resource :email_verification, only: %i[show create]
     resource :password_reset,     only: %i[new edit create update]
   end
diff --git a/test/controllers/identity/email_verifications_controller_test.rb b/test/controllers/identity/email_verifications_controller_test.rb
index 84956f50..5449941f 100644
--- a/test/controllers/identity/email_verifications_controller_test.rb
+++ b/test/controllers/identity/email_verifications_controller_test.rb
@@ -29,7 +29,7 @@ class EmailVerificationsControllerTest < ActionDispatch::IntegrationTest
 
       get identity_email_verification_url(sid: sid_exp, email: @user.email)
 
-      assert_redirected_to edit_identity_email_url
+      assert_redirected_to root_url
       assert_equal 'That email verification link is invalid', flash[:alert]
     end
   end
diff --git a/test/controllers/identity/emails_controller_test.rb b/test/controllers/identity/emails_controller_test.rb
index c0199fb0..35989d26 100644
--- a/test/controllers/identity/emails_controller_test.rb
+++ b/test/controllers/identity/emails_controller_test.rb
@@ -8,21 +8,16 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest
       @user = log_in_as(create(:user))
     end
 
-    test 'should get edit' do
-      get edit_identity_email_url
-      assert_response :success
-    end
-
     test 'should update email' do
       patch identity_email_url, params: { email: 'new_email@hey.com', current_password: 'Secret1*3*5*' }
-      assert_redirected_to root_url
+      assert_redirected_to account_path
     end
 
     test 'should not update email with wrong current password' do
       patch identity_email_url, params: { email: 'new_email@hey.com', current_password: 'SecretWrong1*3' }
 
-      assert_redirected_to edit_identity_email_url
-      assert_equal 'The password you entered is incorrect', flash[:alert]
+      assert_redirected_to account_path
+      assert_equal 'The password you entered is incorrect', flash[:emails_update_password_incorrect]
     end
   end
 end
diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index 24e55e72..ce2fb635 100644
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -12,11 +12,6 @@ class UsersControllerTestSignedIn < ActionDispatch::IntegrationTest
     get account_url
     assert_response :success
   end
-
-  test '#show has a link to change email address' do
-    get account_url
-    assert_select 'a', href: edit_identity_email_path
-  end
 end
 
 class UserControllerTestSignedOut < ActionDispatch::IntegrationTest
diff --git a/test/system/identity/emails_test.rb b/test/system/identity/emails_test.rb
index d4236261..ab0669d7 100644
--- a/test/system/identity/emails_test.rb
+++ b/test/system/identity/emails_test.rb
@@ -9,15 +9,17 @@ class EmailsTest < ApplicationSystemTestCase
     end
 
     test 'when I update my email address I should be prompted to verify it' do
-      visit edit_identity_email_path
+      visit account_path
 
-      fill_in 'New email', with: 'new_email@example.com'
-      fill_in 'Current password', with: 'Secret1*3*5*'
-      click_on 'Save changes'
+      within(email_address_section) do
+        fill_in 'New email', with: 'new_email@example.com'
+        fill_in 'Current password', with: 'Secret1*3*5*'
+        click_on 'Save changes'
+      end
 
       assert_text 'Your email has been changed'
 
-      visit edit_identity_email_path
+      visit account_path
       assert_text 'We sent a verification email to the address below'
 
       click_on 'Re-send verification email'
@@ -25,11 +27,13 @@ class EmailsTest < ApplicationSystemTestCase
     end
 
     test 'updating my email address fails if my current password is wrong' do
-      visit edit_identity_email_path
+      visit account_path
 
-      fill_in 'New email', with: 'new_email@example.com'
-      fill_in 'Current password', with: 'wrongpassword'
-      click_on 'Save changes'
+      within(email_address_section) do
+        fill_in 'New email', with: 'new_email@example.com'
+        fill_in 'Current password', with: 'wrongpassword'
+        click_on 'Save changes'
+      end
 
       assert_text 'The password you entered is incorrect'
       refute_text 'We sent a verification email to the address below'
@@ -38,14 +42,22 @@ class EmailsTest < ApplicationSystemTestCase
     test 'updating my email address fails if I use an existing email' do
       existing_user = create(:user)
 
-      visit edit_identity_email_path
+      visit account_path
 
-      fill_in 'New email', with: existing_user.email
-      fill_in 'Current password', with: 'Secret1*3*5*'
-      click_on 'Save changes'
+      within(email_address_section) do
+        fill_in 'New email', with: existing_user.email
+        fill_in 'Current password', with: 'Secret1*3*5*'
+        click_on 'Save changes'
+      end
 
       assert_text 'Email has already been taken'
       refute_text 'We sent a verification email to the address below'
     end
+
+    private
+
+    def email_address_section
+      find('h2', text: 'Email address').ancestor('section')
+    end
   end
 end
diff --git a/test/system/passwords_test.rb b/test/system/passwords_test.rb
index c4d07f65..0e0bacdc 100644
--- a/test/system/passwords_test.rb
+++ b/test/system/passwords_test.rb
@@ -10,10 +10,12 @@ class PasswordsTest < ApplicationSystemTestCase
   test 'updating the password' do
     visit account_path
 
-    fill_in 'Current password', with: 'Secret1*3*5*'
-    fill_in 'New password', with: 'Secret6*4*2*'
-    fill_in 'Confirm new password', with: 'Secret6*4*2*'
-    click_on 'Save changes'
+    within(password_section) do
+      fill_in 'Current password', with: 'Secret1*3*5*'
+      fill_in 'New password', with: 'Secret6*4*2*'
+      fill_in 'Confirm new password', with: 'Secret6*4*2*'
+      click_on 'Save changes'
+    end
 
     assert_text 'Your password has been changed'
     assert_current_path account_path
@@ -22,12 +24,20 @@ class PasswordsTest < ApplicationSystemTestCase
   test 'when password is too short' do
     visit account_path
 
-    fill_in 'Current password', with: 'Secret1*3*5*'
-    fill_in 'New password', with: 'short'
-    fill_in 'Confirm new password', with: 'short'
-    click_on 'Save changes'
+    within(password_section) do
+      fill_in 'Current password', with: 'Secret1*3*5*'
+      fill_in 'New password', with: 'short'
+      fill_in 'Confirm new password', with: 'short'
+      click_on 'Save changes'
+    end
 
     assert_text 'Password is too short'
     assert_current_path account_path
   end
+
+  private
+
+  def password_section
+    find('h2', text: 'Password').ancestor('section')
+  end
 end