From ae546c5f2221194ac575596c2b21f0159455ccca Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Sat, 26 Oct 2024 21:11:54 +0100
Subject: [PATCH 01/15] Inline shared header on /account page

To more closely match the designs.
---
 app/views/users/show.html.erb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index afc8a3cf..9c1cc037 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -1,4 +1,4 @@
-<%= render('shared/page_header', text: 'My account') %>
+<h1 class="text-2xl font-bold">My account</h1>
 
 <ul>
   <li><%= text_link_to "Change password", edit_password_path %></li>

From 84d6b94abe7531ef37998a0e27fca13c9b44b0e4 Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Sat, 26 Oct 2024 22:06:43 +0100
Subject: [PATCH 02/15] Add sections to /account page

With some placeholder content for now.
---
 app/views/users/show.html.erb | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index 9c1cc037..d9e254b0 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -1,5 +1,32 @@
 <h1 class="text-2xl font-bold">My account</h1>
 
+<div class="flex flex-col divide-y divide-slate-300">
+  <section class="flex flex-row py-6">
+    <div class="basis-1/4">
+      <h2 class="text-lg font-bold">Password</h2>
+    </div>
+    <div class="basis-3/4">
+      <p>form goes here</p>
+    </div>
+  </section>
+  <section class="flex flex-row py-6">
+    <div class="basis-1/4">
+      <h2 class="text-lg font-bold">Email address</h2>
+    </div>
+    <div class="basis-3/4">
+      <p>form goes here</p>
+    </div>
+  </section>
+  <section class="flex flex-row py-6">
+    <div class="basis-1/4">
+      <h2 class="text-lg font-bold">Payout details</h2>
+    </div>
+    <div class="basis-3/4">
+      <p>form goes here</p>
+    </div>
+  </section>
+</div>
+
 <ul>
   <li><%= text_link_to "Change password", edit_password_path %></li>
   <li><%= text_link_to "Change email address", edit_identity_email_path %></li>

From 82f9933924df5c6a21308de340c8216569ea4c89 Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Sat, 26 Oct 2024 22:28:46 +0100
Subject: [PATCH 03/15] Inline password change form in /account

We inline the password edit form into the /account page and ensure
that successful, or unsuccessful form edits redisplay/redirect to this
page. I've also added a system test to cover validation errors to give
me more confidence to make the change.
---
 app/controllers/passwords_controller.rb       |  6 +++---
 app/controllers/users_controller.rb           | 10 +++++++++-
 app/views/users/show.html.erb                 | 10 +++++++++-
 test/controllers/passwords_controller_test.rb |  4 ++--
 test/system/passwords_test.rb                 | 17 ++++++++++++++---
 5 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
index adc9f0c7..c12e1701 100644
--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -11,11 +11,11 @@ def update
     authorize @user
 
     if !@user.authenticate(params[:current_password])
-      redirect_to edit_password_path, alert: 'The current password you entered is incorrect'
+      redirect_to account_path, alert: 'The current password you entered is incorrect'
     elsif @user.update(user_params)
-      redirect_to root_path, notice: 'Your password has been changed'
+      redirect_to account_path, notice: 'Your password has been changed'
     else
-      render :edit, status: :unprocessable_entity
+      render 'users/show', status: :unprocessable_entity
     end
   end
 
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 6d9c46ee..8de12246 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -1,7 +1,15 @@
 # frozen_string_literal: true
 
 class UsersController < ApplicationController
+  before_action :set_user
+
   def show
-    authorize User
+    authorize @user
+  end
+
+  private
+
+  def set_user
+    @user = Current.user
   end
 end
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index d9e254b0..8439fc1b 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -6,7 +6,15 @@
       <h2 class="text-lg font-bold">Password</h2>
     </div>
     <div class="basis-3/4">
-      <p>form goes here</p>
+      <%= form_with(url: password_path, method: :patch, builder: TailwindFormBuilder) do |form| %>
+        <%= render 'shared/errors', model: @user %>
+        <%= form.password_field :current_password, required: true, autofocus: true, autocomplete: "current-password", class: "w-full mb-3" %>
+        <%= form.password_field :password, label: { text: "New password"}, required: true, autocomplete: "new-password", class: "w-full mb-3" %>
+        <%= form.password_field :password_confirmation, label: { text: "Confirm new password"}, required: true, autocomplete: "new-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">
diff --git a/test/controllers/passwords_controller_test.rb b/test/controllers/passwords_controller_test.rb
index c453d4f2..fc7e1da2 100644
--- a/test/controllers/passwords_controller_test.rb
+++ b/test/controllers/passwords_controller_test.rb
@@ -15,7 +15,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
   test 'should update password' do
     patch password_url,
           params: { current_password: 'Secret1*3*5*', password: 'Secret6*4*2*', password_confirmation: 'Secret6*4*2*' }
-    assert_redirected_to root_url
+    assert_redirected_to account_path
   end
 
   test 'should not update password with wrong current password' do
@@ -23,7 +23,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
           params: { current_password: 'SecretWrong1*3', password: 'Secret6*4*2*',
                     password_confirmation: 'Secret6*4*2*' }
 
-    assert_redirected_to edit_password_url
+    assert_redirected_to account_path
     assert_equal 'The current password you entered is incorrect', flash[:alert]
   end
 end
diff --git a/test/system/passwords_test.rb b/test/system/passwords_test.rb
index 41608de0..c4d07f65 100644
--- a/test/system/passwords_test.rb
+++ b/test/system/passwords_test.rb
@@ -8,9 +8,7 @@ class PasswordsTest < ApplicationSystemTestCase
   end
 
   test 'updating the password' do
-    click_on 'avatar'
-    click_on 'My account'
-    click_on 'Change password'
+    visit account_path
 
     fill_in 'Current password', with: 'Secret1*3*5*'
     fill_in 'New password', with: 'Secret6*4*2*'
@@ -18,5 +16,18 @@ class PasswordsTest < ApplicationSystemTestCase
     click_on 'Save changes'
 
     assert_text 'Your password has been changed'
+    assert_current_path account_path
+  end
+
+  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'
+
+    assert_text 'Password is too short'
+    assert_current_path account_path
   end
 end

From 86101f455007599c8b8e91dcc456af979cb2dca6 Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Mon, 28 Oct 2024 15:11:07 +0000
Subject: [PATCH 04/15] Remove redundant passwords/edit page

Passwords can now be edited from the account page so this page is no
longer needed.
---
 app/controllers/passwords_controller.rb       |  4 ----
 app/views/passwords/edit.html.erb             | 13 -------------
 app/views/users/show.html.erb                 |  1 -
 config/routes.rb                              |  2 +-
 test/controllers/passwords_controller_test.rb |  5 -----
 test/controllers/users_controller_test.rb     |  5 -----
 6 files changed, 1 insertion(+), 29 deletions(-)
 delete mode 100644 app/views/passwords/edit.html.erb

diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
index c12e1701..4a6015bf 100644
--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -3,10 +3,6 @@
 class PasswordsController < ApplicationController
   before_action :set_user
 
-  def edit
-    authorize @user
-  end
-
   def update
     authorize @user
 
diff --git a/app/views/passwords/edit.html.erb b/app/views/passwords/edit.html.erb
deleted file mode 100644
index c4d68565..00000000
--- a/app/views/passwords/edit.html.erb
+++ /dev/null
@@ -1,13 +0,0 @@
-<%= render 'shared/page_header', text: 'Change your password' %>
-
-<%= form_with(url: password_path, method: :patch, builder: TailwindFormBuilder) do |form| %>
-  <%= render 'shared/errors', model: @user %>
-  <%= form.password_field :current_password, required: true, autofocus: true, autocomplete: "current-password", class: "w-full mb-3" %>
-  <%= form.password_field :password, label: { text: "New password"}, required: true, autocomplete: "new-password", class: "w-full mb-3" %>
-  <%= form.password_field :password_confirmation, label: { text: "Confirm new password"}, required: true, autocomplete: "new-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 8439fc1b..f4c041bb 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -36,7 +36,6 @@
 </div>
 
 <ul>
-  <li><%= text_link_to "Change password", edit_password_path %></li>
   <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>
diff --git a/config/routes.rb b/config/routes.rb
index 1cb37ef3..60f2b7af 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -17,7 +17,7 @@
   get 'collection', to: 'collections#show'
 
   resources :sessions, only: %i[index show destroy]
-  resource  :password, only: %i[edit update]
+  resource  :password, only: %i[update]
   resource :payout_detail, only: %i[new create edit update]
   resolve('PayoutDetail') { [:payout_detail] }
 
diff --git a/test/controllers/passwords_controller_test.rb b/test/controllers/passwords_controller_test.rb
index fc7e1da2..58fb0b50 100644
--- a/test/controllers/passwords_controller_test.rb
+++ b/test/controllers/passwords_controller_test.rb
@@ -7,11 +7,6 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
     @user = log_in_as(create(:user))
   end
 
-  test 'should get edit' do
-    get edit_password_url
-    assert_response :success
-  end
-
   test 'should update password' do
     patch password_url,
           params: { current_password: 'Secret1*3*5*', password: 'Secret6*4*2*', password_confirmation: 'Secret6*4*2*' }
diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index 54c24379..24e55e72 100644
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -13,11 +13,6 @@ class UsersControllerTestSignedIn < ActionDispatch::IntegrationTest
     assert_response :success
   end
 
-  test '#show has a link to change password' do
-    get account_url
-    assert_select 'a', href: edit_password_path
-  end
-
   test '#show has a link to change email address' do
     get account_url
     assert_select 'a', href: edit_identity_email_path

From 717aa4bba4c9b35f5a889b8ad3085414284d781c Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Mon, 28 Oct 2024 15:57:43 +0000
Subject: [PATCH 05/15] Extract a ModelErrorComponent

To replace our use of the `shared/errors` partial.
---
 app/components/model_error_component.rb       | 15 +++++++++
 .../model_error_component.html.erb            | 11 +++++++
 app/views/albums/_form.html.erb               |  2 +-
 app/views/artists/_form.html.erb              |  2 +-
 app/views/identity/emails/edit.html.erb       |  2 +-
 .../identity/password_resets/edit.html.erb    |  2 +-
 app/views/payout_details/_form.html.erb       |  2 +-
 app/views/registrations/new.html.erb          |  2 +-
 app/views/shared/_errors.html.erb             | 13 --------
 app/views/users/show.html.erb                 |  2 +-
 test/components/model_error_component_test.rb | 31 +++++++++++++++++++
 .../previews/model_error_component_preview.rb | 10 ++++++
 12 files changed, 74 insertions(+), 20 deletions(-)
 create mode 100644 app/components/model_error_component.rb
 create mode 100644 app/components/model_error_component/model_error_component.html.erb
 delete mode 100644 app/views/shared/_errors.html.erb
 create mode 100644 test/components/model_error_component_test.rb
 create mode 100644 test/components/previews/model_error_component_preview.rb

diff --git a/app/components/model_error_component.rb b/app/components/model_error_component.rb
new file mode 100644
index 00000000..43d19ced
--- /dev/null
+++ b/app/components/model_error_component.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class ModelErrorComponent < ViewComponent::Base
+  def initialize(model:)
+    @model = model
+
+    super
+  end
+
+  def render?
+    return false unless @model
+
+    @model.errors.any?
+  end
+end
diff --git a/app/components/model_error_component/model_error_component.html.erb b/app/components/model_error_component/model_error_component.html.erb
new file mode 100644
index 00000000..0862c444
--- /dev/null
+++ b/app/components/model_error_component/model_error_component.html.erb
@@ -0,0 +1,11 @@
+<div class="w-full border border-red-600 p-4 my-4">
+  <h2 class="font-bold mb-3">
+    <%= pluralize(@model.errors.count, "error") %> prohibited this <%= @model.class.name.downcase %> from being saved:
+  </h2>
+
+  <ul>
+    <% @model.errors.each do |error| %>
+      <li><%= error.full_message %></li>
+    <% end %>
+  </ul>
+</div>
diff --git a/app/views/albums/_form.html.erb b/app/views/albums/_form.html.erb
index 0aded7ee..9120885f 100644
--- a/app/views/albums/_form.html.erb
+++ b/app/views/albums/_form.html.erb
@@ -1,5 +1,5 @@
 <%= form_for([album.artist, album], builder: TailwindFormBuilder, data: { controller: 'multiple-upload', action: 'direct-upload:initialize->multiple-upload#init direct-upload:progress->multiple-upload#progress' }) do |form| %>
-  <%= render('shared/errors', model: album) %>
+  <%= render ModelErrorComponent.new(model: album) %>
   <%= form.text_field :title, class: 'w-full mb-3' %>
   <%= form.currency_field :price, symbol: '£', class: 'w-full mb-2' %>
   <%= form.text_area :about, rows: 10, class: 'w-full mb-3' %>
diff --git a/app/views/artists/_form.html.erb b/app/views/artists/_form.html.erb
index c3e5ee7c..70a59eb5 100644
--- a/app/views/artists/_form.html.erb
+++ b/app/views/artists/_form.html.erb
@@ -1,5 +1,5 @@
 <%= form_with(model: artist, class: "contents", builder: TailwindFormBuilder) do |form| %>
-  <%= render('shared/errors', model: artist) %>
+  <%= render ModelErrorComponent.new(model: artist) %>
   <%= form.text_field :name, class: 'w-full mb-3' %>
   <%= form.text_field :location, class: 'w-full mb-3' %>
   <%= form.text_field :description, class: 'w-full mb-3' %>
diff --git a/app/views/identity/emails/edit.html.erb b/app/views/identity/emails/edit.html.erb
index 343c61ab..46fe3926 100644
--- a/app/views/identity/emails/edit.html.erb
+++ b/app/views/identity/emails/edit.html.erb
@@ -10,7 +10,7 @@
 <% end %>
 
 <%= form_with(url: identity_email_path, method: :patch, builder: TailwindFormBuilder) do |form| %>
-  <%= render 'shared/errors', model: @user %>
+  <%= 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' %>
diff --git a/app/views/identity/password_resets/edit.html.erb b/app/views/identity/password_resets/edit.html.erb
index 1b0efcc2..0fbe5824 100644
--- a/app/views/identity/password_resets/edit.html.erb
+++ b/app/views/identity/password_resets/edit.html.erb
@@ -1,7 +1,7 @@
 <%= render('shared/page_header', text: 'Reset your password') %>
 
 <%= form_with(url: identity_password_reset_path, method: :patch, builder: TailwindFormBuilder) do |form| %>
-  <%= render 'shared/errors', model: @user %>
+  <%= render ModelErrorComponent.new(model: @user) %>
   <%= form.hidden_field :sid, value: params[:sid] %>
   <%= form.password_field :password, label: { text: 'New password' }, required: true, autofocus: true, autocomplete: "new-password", class: 'w-full mb-3' %>
   <%= form.password_field :password_confirmation, label: { text: 'Confirm new password' }, required: true, autocomplete: "new-password", class: 'w-full mb-3' %>
diff --git a/app/views/payout_details/_form.html.erb b/app/views/payout_details/_form.html.erb
index 144454a8..7a4c1e6a 100644
--- a/app/views/payout_details/_form.html.erb
+++ b/app/views/payout_details/_form.html.erb
@@ -1,7 +1,7 @@
 <p class="text-slate-500 mb-6">We use <a class="underline" href="https://wise.com">Wise</a> to make payouts once a month. Wise will email you at <span class="font-bold"><%= Current.user.email %></span> to ask for your bank details. To make the payment we need the following:</p>
 
 <%= form_with(model: payout_detail, class: "contents", builder: TailwindFormBuilder) do |form| %>
-  <%= render('shared/errors', model: payout_detail) %>
+  <%= render ModelErrorComponent.new(model: payout_detail) %>
   <%= form.text_field :name, placeholder: 'Bartholomew J. Simpson', class: 'w-full' %>
   <p class="text-slate-500 italic text-sm mb-3">Your full name as used on your bank account.</p>
 
diff --git a/app/views/registrations/new.html.erb b/app/views/registrations/new.html.erb
index 1069539d..726543bd 100644
--- a/app/views/registrations/new.html.erb
+++ b/app/views/registrations/new.html.erb
@@ -5,7 +5,7 @@
 <%= render 'shared/page_header', text: 'Sign up' %>
 
 <%= form_with(url: sign_up_path, builder: TailwindFormBuilder, data: { turbo: false }) do |form| %>
-  <%= render('shared/errors', model: @user) %>
+  <%= render ModelErrorComponent.new(model: @user) %>
   <%= form.email_field :email, value: @user.email, required: true, autofocus: true, autocomplete: "email", class: 'w-full mb-3' %>
   <%= form.password_field :password, required: true, autocomplete: "new-password", class: 'w-full mb-3' %>
   <%= form.password_field :password_confirmation, required: true, autocomplete: "new-password", class: 'w-full mb-6' %>
diff --git a/app/views/shared/_errors.html.erb b/app/views/shared/_errors.html.erb
deleted file mode 100644
index 9c6e1e22..00000000
--- a/app/views/shared/_errors.html.erb
+++ /dev/null
@@ -1,13 +0,0 @@
-<% if model.errors.any? %>
-<div class="w-full border border-red-600 p-4 my-4">
-  <h2 class="font-bold mb-3">
-    <%= pluralize(model.errors.count, "error") %> prohibited this <%= model.class.name.downcase %> from being saved:
-  </h2>
-
-  <ul>
-    <% model.errors.each do |error| %>
-    <li><%= error.full_message %></li>
-    <% end %>
-  </ul>
-</div>
-<% end %>
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index f4c041bb..4222fecc 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -7,7 +7,7 @@
     </div>
     <div class="basis-3/4">
       <%= form_with(url: password_path, method: :patch, builder: TailwindFormBuilder) do |form| %>
-        <%= render 'shared/errors', model: @user %>
+        <%= render ModelErrorComponent.new(model: @user) %>
         <%= form.password_field :current_password, required: true, autofocus: true, autocomplete: "current-password", class: "w-full mb-3" %>
         <%= form.password_field :password, label: { text: "New password"}, required: true, autocomplete: "new-password", class: "w-full mb-3" %>
         <%= form.password_field :password_confirmation, label: { text: "Confirm new password"}, required: true, autocomplete: "new-password", class: "w-full mb-3" %>
diff --git a/test/components/model_error_component_test.rb b/test/components/model_error_component_test.rb
new file mode 100644
index 00000000..c35a03db
--- /dev/null
+++ b/test/components/model_error_component_test.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class ModelErrorComponentTest < ViewComponent::TestCase
+  def setup
+    @model = User.new
+  end
+
+  def test_component_renders_nothing_when_model_has_no_errors
+    assert render_inline(ModelErrorComponent.new(model: @model)).to_html.blank?
+  end
+
+  def test_component_renders_title_model_has_errors
+    @model.email = nil
+    @model.save
+
+    render_inline(ModelErrorComponent.new(model: @model))
+
+    assert_text 'errors prohibited this user from being saved'
+  end
+
+  def test_component_renders_errors_when_model_has_errors
+    @model.email = nil
+    @model.save
+
+    render_inline(ModelErrorComponent.new(model: @model))
+
+    assert_text "Email can't be blank"
+  end
+end
diff --git a/test/components/previews/model_error_component_preview.rb b/test/components/previews/model_error_component_preview.rb
new file mode 100644
index 00000000..9f36e49e
--- /dev/null
+++ b/test/components/previews/model_error_component_preview.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class ModelErrorComponentPreview < ViewComponent::Preview
+  def default
+    model = User.new
+    model.save
+
+    render(ModelErrorComponent.new(model:))
+  end
+end

From d82c3a8c4ec94b52d9d6730ed08e18f2d872f507 Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Mon, 28 Oct 2024 16:13:42 +0000
Subject: [PATCH 06/15] Extract ErrorBoxComponent

So that I can use this to render a controller generated error message
on /account
---
 app/components/error_box_component.rb           |  9 +++++++++
 .../error_box_component.html.erb                |  7 +++++++
 .../model_error_component.html.erb              | 10 ++++------
 test/components/error_box_component_test.rb     | 17 +++++++++++++++++
 .../previews/error_box_component_preview.rb     | 13 +++++++++++++
 5 files changed, 50 insertions(+), 6 deletions(-)
 create mode 100644 app/components/error_box_component.rb
 create mode 100644 app/components/error_box_component/error_box_component.html.erb
 create mode 100644 test/components/error_box_component_test.rb
 create mode 100644 test/components/previews/error_box_component_preview.rb

diff --git a/app/components/error_box_component.rb b/app/components/error_box_component.rb
new file mode 100644
index 00000000..34fb045a
--- /dev/null
+++ b/app/components/error_box_component.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class ErrorBoxComponent < ViewComponent::Base
+  def initialize(title:)
+    @title = title
+
+    super
+  end
+end
diff --git a/app/components/error_box_component/error_box_component.html.erb b/app/components/error_box_component/error_box_component.html.erb
new file mode 100644
index 00000000..aca09a9c
--- /dev/null
+++ b/app/components/error_box_component/error_box_component.html.erb
@@ -0,0 +1,7 @@
+<div class="w-full border border-red-600 p-4 mb-3">
+  <h2 class="font-bold mb-3">
+    <%= @title %>
+  </h2>
+
+  <%= content %>
+</div>
diff --git a/app/components/model_error_component/model_error_component.html.erb b/app/components/model_error_component/model_error_component.html.erb
index 0862c444..06bd3a7d 100644
--- a/app/components/model_error_component/model_error_component.html.erb
+++ b/app/components/model_error_component/model_error_component.html.erb
@@ -1,11 +1,9 @@
-<div class="w-full border border-red-600 p-4 my-4">
-  <h2 class="font-bold mb-3">
-    <%= pluralize(@model.errors.count, "error") %> prohibited this <%= @model.class.name.downcase %> from being saved:
-  </h2>
-
+<%= render(ErrorBoxComponent.new(
+  title: "#{pluralize(@model.errors.count, "error")} prohibited this #{@model.class.name.downcase} from being saved"
+)) do %>
   <ul>
     <% @model.errors.each do |error| %>
       <li><%= error.full_message %></li>
     <% end %>
   </ul>
-</div>
+<% end %>
diff --git a/test/components/error_box_component_test.rb b/test/components/error_box_component_test.rb
new file mode 100644
index 00000000..fb1a0a45
--- /dev/null
+++ b/test/components/error_box_component_test.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class ErrorBoxComponentTest < ViewComponent::TestCase
+  def setup
+    render_inline(ErrorBoxComponent.new(title: 'Title').with_content('Content'))
+  end
+
+  def test_component_renders_title
+    assert_text 'Title'
+  end
+
+  def test_component_renders_content
+    assert_text 'Content'
+  end
+end
diff --git a/test/components/previews/error_box_component_preview.rb b/test/components/previews/error_box_component_preview.rb
new file mode 100644
index 00000000..e9df9a95
--- /dev/null
+++ b/test/components/previews/error_box_component_preview.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class ErrorBoxComponentPreview < ViewComponent::Preview
+  def default
+    render(
+      ErrorBoxComponent.new(title: 'Title')
+    ) do
+      tag.div do
+        content_tag(:span, 'Error box content')
+      end
+    end
+  end
+end

From ba09a005bad61cc3ff810cf573b264761512834e Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Mon, 28 Oct 2024 16:25:46 +0000
Subject: [PATCH 07/15] Display the 'password incorrect' message in an
 ErrorBoxComponent

This matches the style of the ModelErrorComponent shown if the User
model itself has validation errors. I've added a specific flash key so
that this error box is not shown in the case where the `alert` key is
set for other reasons.
---
 app/controllers/passwords_controller.rb       | 3 ++-
 app/views/users/show.html.erb                 | 3 +++
 test/controllers/passwords_controller_test.rb | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
index 4a6015bf..98455c82 100644
--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -7,7 +7,8 @@ def update
     authorize @user
 
     if !@user.authenticate(params[:current_password])
-      redirect_to account_path, alert: 'The current password you entered is incorrect'
+      flash[:incorrect_password] = 'The current password you entered is incorrect'
+      redirect_to account_path
     elsif @user.update(user_params)
       redirect_to account_path, notice: 'Your password has been changed'
     else
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index 4222fecc..ef190b28 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -7,6 +7,9 @@
     </div>
     <div class="basis-3/4">
       <%= form_with(url: password_path, method: :patch, builder: TailwindFormBuilder) do |form| %>
+        <% if flash[:incorrect_password] %>
+          <%= render ErrorBoxComponent.new(title: 'Incorrect password').with_content(flash[:incorrect_password]) %>
+        <% end %>
         <%= render ModelErrorComponent.new(model: @user) %>
         <%= form.password_field :current_password, required: true, autofocus: true, autocomplete: "current-password", class: "w-full mb-3" %>
         <%= form.password_field :password, label: { text: "New password"}, required: true, autocomplete: "new-password", class: "w-full mb-3" %>
diff --git a/test/controllers/passwords_controller_test.rb b/test/controllers/passwords_controller_test.rb
index 58fb0b50..1efec87f 100644
--- a/test/controllers/passwords_controller_test.rb
+++ b/test/controllers/passwords_controller_test.rb
@@ -19,6 +19,6 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
                     password_confirmation: 'Secret6*4*2*' }
 
     assert_redirected_to account_path
-    assert_equal 'The current password you entered is incorrect', flash[:alert]
+    assert_equal 'The current password you entered is incorrect', flash[:incorrect_password]
   end
 end

From c3e693601bfd7b4702a1ae009283d38861806c75 Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Tue, 29 Oct 2024 11:35:16 +0000
Subject: [PATCH 08/15] Improve system test coverage for updating email

---
 test/system/identity/emails_test.rb | 39 ++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/test/system/identity/emails_test.rb b/test/system/identity/emails_test.rb
index 0d1c3073..0afe38c5 100644
--- a/test/system/identity/emails_test.rb
+++ b/test/system/identity/emails_test.rb
@@ -8,27 +8,42 @@ class EmailsTest < ApplicationSystemTestCase
       @user = log_in_as(create(:user))
     end
 
-    test 'updating the email' do
-      click_on 'avatar'
-      click_on 'My account'
-      click_on 'Change email address'
+    test 'when I update my email address I should be prompted to verify it' do
+      visit edit_identity_email_path
 
-      fill_in 'New email', with: 'new_email@hey.com'
+      fill_in 'New email', with: 'new_email@example.com'
       fill_in 'Current password', with: 'Secret1*3*5*'
       click_on 'Save changes'
 
       assert_text 'Your email has been changed'
-    end
 
-    test 'sending a verification email' do
-      @user.update! verified: false
+      visit edit_identity_email_path
+      assert_text 'We sent a verification email to the address below'
 
-      click_on 'avatar'
-      click_on 'My account'
-      click_on 'Change email address'
       click_on 'Re-send verification email'
-
       assert_text 'We sent a verification email to your email address'
     end
+
+    test 'updating my email address fails if my current password is wrong' do
+      visit edit_identity_email_path
+
+      fill_in 'New email', with: 'new_email@example.com'
+      fill_in 'Current password', with: 'wrongpassword'
+      click_on 'Save changes'
+
+      assert_text 'The password you entered is incorrect'
+    end
+
+    test 'updating my email address fails if I use an existing email' do
+      existing_user = create(:user)
+
+      visit edit_identity_email_path
+
+      fill_in 'New email', with: existing_user.email
+      fill_in 'Current password', with: 'Secret1*3*5*'
+      click_on 'Save changes'
+
+      assert_text 'Email has already been taken'
+    end
   end
 end

From 0e52cf3460d24d18407a4dd505d541ccfb96106f Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Tue, 29 Oct 2024 11:52:41 +0000
Subject: [PATCH 09/15] Don't un-verify user if changing email address fails
 validation

Before this commit the `verified` flag on a user would be set to false
even if the change to the email address failed validation (e.g. by
setting the email address to one that was taken).
---
 app/models/user.rb                  | 2 +-
 test/system/identity/emails_test.rb | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 81fe6ed0..589d0706 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -17,7 +17,7 @@ class User < ApplicationRecord
     self.email = email.downcase.strip
   end
 
-  before_validation if: :email_changed?, on: :update do
+  before_update if: :email_changed? do
     self.verified = false
   end
 
diff --git a/test/system/identity/emails_test.rb b/test/system/identity/emails_test.rb
index 0afe38c5..d4236261 100644
--- a/test/system/identity/emails_test.rb
+++ b/test/system/identity/emails_test.rb
@@ -32,6 +32,7 @@ class EmailsTest < ApplicationSystemTestCase
       click_on 'Save changes'
 
       assert_text 'The password you entered is incorrect'
+      refute_text 'We sent a verification email to the address below'
     end
 
     test 'updating my email address fails if I use an existing email' do
@@ -44,6 +45,7 @@ class EmailsTest < ApplicationSystemTestCase
       click_on 'Save changes'
 
       assert_text 'Email has already been taken'
+      refute_text 'We sent a verification email to the address below'
     end
   end
 end

From 12ebd7b3151dc413c48e04b493474c0533bff738 Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Tue, 29 Oct 2024 12:09:01 +0000
Subject: [PATCH 10/15] Only show password validation errors in password change
 form

We're about to add another form to this page to allow the user to
change their email. Since that form will also act on the User model we
want to make sure that the relevant validation errors are shown on
each form. This form deals with errors on the password field so we add
a guard clause for those.
---
 app/views/users/show.html.erb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index ef190b28..a03946b9 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -10,7 +10,9 @@
         <% if flash[:incorrect_password] %>
           <%= render ErrorBoxComponent.new(title: 'Incorrect password').with_content(flash[:incorrect_password]) %>
         <% end %>
-        <%= render ModelErrorComponent.new(model: @user) %>
+        <% if @user.errors.include?(:password) %>
+          <%= render ModelErrorComponent.new(model: @user) %>
+        <% end %>
         <%= form.password_field :current_password, required: true, autofocus: true, autocomplete: "current-password", class: "w-full mb-3" %>
         <%= form.password_field :password, label: { text: "New password"}, required: true, autocomplete: "new-password", class: "w-full mb-3" %>
         <%= form.password_field :password_confirmation, label: { text: "Confirm new password"}, required: true, autocomplete: "new-password", class: "w-full mb-3" %>

From 4d5473a438e7b7a36d85e7ef7f5da6cdd815e9e3 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 11/15] 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

From d93af09ea891e662b4cea076a83c925da173646c Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Tue, 29 Oct 2024 14:45:53 +0000
Subject: [PATCH 12/15] Inline payout details form into /account

I've decided to set a local variable `payout_detail` in the
`users/show` template to either use the persisted payout detail or
create a new one. I considered setting this as an instance variable in
the controller, but the issue is that any controller that renders
`user/show` (PasswordsController, EmailsController,
PayoutDetailsController) would have to also set this instance
variable.

I've also updated the link in the notify_artist email to point to this
new page.
---
 app/controllers/payout_details_controller.rb  | 24 +++++---------
 .../purchase_mailer/notify_artist.html.erb    |  4 +--
 app/views/users/show.html.erb                 | 28 +++++++++++------
 config/routes.rb                              |  2 +-
 .../payout_details_controller_test.rb         | 27 ----------------
 test/mailers/purchase_mailer_test.rb          |  4 +--
 test/system/payout_details_test.rb            | 31 ++++++++++++++++---
 7 files changed, 58 insertions(+), 62 deletions(-)

diff --git a/app/controllers/payout_details_controller.rb b/app/controllers/payout_details_controller.rb
index de64947b..2deb0386 100644
--- a/app/controllers/payout_details_controller.rb
+++ b/app/controllers/payout_details_controller.rb
@@ -1,31 +1,21 @@
 # frozen_string_literal: true
 
 class PayoutDetailsController < ApplicationController
-  def new
-    @payout_detail = PayoutDetail.new(user:)
-    authorize @payout_detail
-  end
-
-  def edit
-    @payout_detail = user.payout_detail
-    raise ActiveRecord::RecordNotFound unless @payout_detail
-
-    authorize @payout_detail
-  end
+  before_action :set_user
 
   def create
-    @payout_detail = PayoutDetail.new(payout_detail_params.merge(user:))
+    @payout_detail = PayoutDetail.new(payout_detail_params.merge(user: @user))
     authorize @payout_detail
 
     if @payout_detail.save
       redirect_to account_url, notice: 'Payout details added'
     else
-      render :new, status: :unprocessable_entity
+      render 'users/show', status: :unprocessable_entity
     end
   end
 
   def update
-    @payout_detail = user.payout_detail
+    @payout_detail = @user.payout_detail
     raise ActiveRecord::RecordNotFound unless @payout_detail
 
     authorize @payout_detail
@@ -33,14 +23,14 @@ def update
     if @payout_detail.update(payout_detail_params)
       redirect_to account_url, notice: 'Payout details updated'
     else
-      render :edit, status: :unprocessable_entity
+      render 'users/show', status: :unprocessable_entity
     end
   end
 
   private
 
-  def user
-    Current.user
+  def set_user
+    @user = Current.user
   end
 
   def payout_detail_params
diff --git a/app/views/purchase_mailer/notify_artist.html.erb b/app/views/purchase_mailer/notify_artist.html.erb
index a738a702..0aca450b 100644
--- a/app/views/purchase_mailer/notify_artist.html.erb
+++ b/app/views/purchase_mailer/notify_artist.html.erb
@@ -4,9 +4,9 @@
   for <%= number_to_currency(@purchase.price, unit: "£") %>.</p>
 
 <% if @user.payout_detail %>
-<p>You have already provided your payout details. If you need to, you can <%= link_to 'change them', edit_payout_detail_url %>.</p>
+<p>You have already provided your payout details. If you need to, you can <%= link_to 'change them', account_url %>.</p>
 <% else %>
-<p><b>Important:</b> we need to know <%= link_to 'some details', new_payout_detail_url %> before we can make a
+<p><b>Important:</b> we need to know <%= link_to 'some details', account_url %> before we can make a
 payment to you.</p>
 <% end %>
 
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index 4ebb5a17..c4969abe 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -54,15 +54,25 @@
       <h2 class="text-lg font-bold">Payout details</h2>
     </div>
     <div class="basis-3/4">
-      <p>form goes here</p>
+      <p class="text-slate-500 mb-6">We use <a class="underline" href="https://wise.com">Wise</a> to make payouts once a month. Wise will email you at <span class="font-bold"><%= Current.user.email %></span> to ask for your bank details. To make the payment we need the following:</p>
+
+      <% payout_detail = @user.payout_detail || PayoutDetail.new(user: @user) %>
+      <%= form_with(model: payout_detail, class: "contents", builder: TailwindFormBuilder) do |form| %>
+        <%= render ModelErrorComponent.new(model: payout_detail) %>
+        <%= form.text_field :name, placeholder: 'Bartholomew J. Simpson', class: 'w-full' %>
+        <p class="text-slate-500 italic text-sm mb-3">Your full name as used on your bank account.</p>
+
+        <%= form.select :country, options_for_payout_details_country_select, include_blank: true, label: { text: 'Country / Region' }, class: 'w-full' %>
+        <p class="text-slate-500 italic text-sm mb-3">We support payouts to the regions/countries <a href="https://wise.com/help/articles/2571942/what-countriesregions-can-i-send-to" class="underline">that Wise supports</a>.</p>
+
+        <div class="flex flex-row justify-end">
+          <% if payout_detail.persisted? %>
+            <%= form.submit "Save changes", class: 'mt-3'%>
+          <% else %>
+            <%= form.submit "Add payout details", class: 'mt-3'%>
+          <% end %>
+        </div>
+      <% end %>
     </div>
   </section>
 </div>
-
-<ul>
-  <% if Current.user.payout_detail %>
-    <li><%= text_link_to "Edit payout details", edit_payout_detail_path %></li>
-  <% else %>
-    <li><%= text_link_to "Add payout details", new_payout_detail_path %></li>
-  <% end %>
-</ul>
diff --git a/config/routes.rb b/config/routes.rb
index 0ca31703..f06f9382 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -18,7 +18,7 @@
 
   resources :sessions, only: %i[index show destroy]
   resource  :password, only: %i[update]
-  resource :payout_detail, only: %i[new create edit update]
+  resource :payout_detail, only: %i[create update]
   resolve('PayoutDetail') { [:payout_detail] }
 
   namespace :identity do
diff --git a/test/controllers/payout_details_controller_test.rb b/test/controllers/payout_details_controller_test.rb
index 037931ba..306de1e9 100644
--- a/test/controllers/payout_details_controller_test.rb
+++ b/test/controllers/payout_details_controller_test.rb
@@ -8,11 +8,6 @@ def setup
     log_in_as(@user)
   end
 
-  test '#new' do
-    get new_payout_detail_url
-    assert_response :success
-  end
-
   test '#create creates a new PayoutDetail and redirects to account_url' do
     assert_difference('PayoutDetail.count') do
       post payout_detail_url, params: { payout_detail: { name: 'Alice', country: 'country' } }
@@ -47,31 +42,9 @@ def setup
 
     assert_response :not_found
   end
-
-  test '#edit' do
-    create(:payout_detail, user: @user)
-
-    get edit_payout_detail_url
-    assert_response :success
-  end
-
-  test '#edit 404s if user has no payout detail' do
-    get edit_payout_detail_url
-    assert_response :not_found
-  end
 end
 
 class PayoutDetailsControllerTestSignedOut < ActionDispatch::IntegrationTest
-  test '#new' do
-    get new_payout_detail_url
-    assert_redirected_to log_in_url
-  end
-
-  test '#edit' do
-    get edit_payout_detail_url
-    assert_redirected_to log_in_url
-  end
-
   test '#update' do
     patch payout_detail_url
     assert_redirected_to log_in_url
diff --git a/test/mailers/purchase_mailer_test.rb b/test/mailers/purchase_mailer_test.rb
index cc5d0d27..8e453c98 100644
--- a/test/mailers/purchase_mailer_test.rb
+++ b/test/mailers/purchase_mailer_test.rb
@@ -50,7 +50,7 @@ class PurchaseMailerTest < ActionMailer::TestCase
     purchase = build(:purchase, album:, price: 7.00)
 
     mail = PurchaseMailer.with(purchase:).notify_artist
-    assert_includes mail.body.to_s, new_payout_detail_url
+    assert_includes mail.body.to_s, account_url
   end
 
   test 'notify_artist allows user to change/verify payout details' do
@@ -61,7 +61,7 @@ class PurchaseMailerTest < ActionMailer::TestCase
     purchase = build(:purchase, album:, price: 7.00)
 
     mail = PurchaseMailer.with(purchase:).notify_artist
-    assert_includes mail.body.to_s, edit_payout_detail_url
+    assert_includes mail.body.to_s, account_url
   end
 
   test 'do not notify artist of purchase if sending is suppressed' do
diff --git a/test/system/payout_details_test.rb b/test/system/payout_details_test.rb
index 9d27b9bc..3cedf868 100644
--- a/test/system/payout_details_test.rb
+++ b/test/system/payout_details_test.rb
@@ -10,12 +10,35 @@ class PayoutDetailsTest < ApplicationSystemTestCase
 
   test 'adding payout details' do
     visit account_url
-    click_on 'Add payout details'
-    fill_in 'Name', with: 'John Lennon'
-    select '🇬🇧 United Kingdom (GBP)', from: 'Country / Region'
-    click_on 'Save'
 
+    within(payout_details_section) do
+      fill_in 'Name', with: 'John Lennon'
+      select '🇬🇧 United Kingdom (GBP)', from: 'Country / Region'
+      click_on 'Add payout details'
+    end
+
+    assert_text 'Payout details added'
     assert_equal 'John Lennon', @user.payout_detail.name
     assert_equal 'united_kingdom', @user.payout_detail.country
   end
+
+  test 'updating payout details' do
+    create(:payout_detail, user: @user)
+
+    visit account_url
+
+    within(payout_details_section) do
+      fill_in 'Name', with: 'John Smith'
+      select '🇬🇧 United Kingdom (GBP)', from: 'Country / Region'
+      click_on 'Save changes'
+    end
+
+    assert_text 'Payout details updated'
+  end
+
+  private
+
+  def payout_details_section
+    find('h2', text: 'Payout details').ancestor('section')
+  end
 end

From 022b5b419ccd1c6a7cb9cc59a69716a7b6f07462 Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Tue, 29 Oct 2024 14:53:22 +0000
Subject: [PATCH 13/15] Give my account header a bottom border

---
 app/views/users/show.html.erb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index c4969abe..2cd3b6ca 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -1,6 +1,6 @@
-<h1 class="text-2xl font-bold">My account</h1>
-
 <div class="flex flex-col divide-y divide-slate-300">
+  <h1 class="text-2xl font-bold mb-1">My account</h1>
+
   <section class="flex flex-row py-6">
     <div class="basis-1/4">
       <h2 class="text-lg font-bold">Password</h2>

From e8d48bdfc61337e94578b23f69200df933c4b02f Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Tue, 29 Oct 2024 15:09:38 +0000
Subject: [PATCH 14/15] Extract a SidebarSectionComponent

---
 app/components/sidebar_section_component.rb   |   9 ++
 .../sidebar_section_component.html.erb        |   8 ++
 app/views/users/show.html.erb                 | 122 ++++++++----------
 .../sidebar_section_component_test.rb         |  12 ++
 4 files changed, 83 insertions(+), 68 deletions(-)
 create mode 100644 app/components/sidebar_section_component.rb
 create mode 100644 app/components/sidebar_section_component/sidebar_section_component.html.erb
 create mode 100644 test/components/sidebar_section_component_test.rb

diff --git a/app/components/sidebar_section_component.rb b/app/components/sidebar_section_component.rb
new file mode 100644
index 00000000..70e953f8
--- /dev/null
+++ b/app/components/sidebar_section_component.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class SidebarSectionComponent < ViewComponent::Base
+  def initialize(title:)
+    @title = title
+
+    super
+  end
+end
diff --git a/app/components/sidebar_section_component/sidebar_section_component.html.erb b/app/components/sidebar_section_component/sidebar_section_component.html.erb
new file mode 100644
index 00000000..7c493d4b
--- /dev/null
+++ b/app/components/sidebar_section_component/sidebar_section_component.html.erb
@@ -0,0 +1,8 @@
+<section class="flex flex-row py-6">
+  <div class="basis-1/4">
+    <h2 class="text-lg font-bold"><%= @title %></h2>
+  </div>
+  <div class="basis-3/4">
+    <%= content %>
+  </div>
+</section>
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index 2cd3b6ca..229ce3bd 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -1,78 +1,64 @@
 <div class="flex flex-col divide-y divide-slate-300">
   <h1 class="text-2xl font-bold mb-1">My account</h1>
 
-  <section class="flex flex-row py-6">
-    <div class="basis-1/4">
-      <h2 class="text-lg font-bold">Password</h2>
-    </div>
-    <div class="basis-3/4">
-      <%= form_with(url: password_path, method: :patch, builder: TailwindFormBuilder) do |form| %>
-        <% if flash[:incorrect_password] %>
-          <%= render ErrorBoxComponent.new(title: 'Incorrect password').with_content(flash[:incorrect_password]) %>
-        <% end %>
-        <% if @user.errors.include?(:password) %>
-          <%= render ModelErrorComponent.new(model: @user) %>
-        <% end %>
-        <%= form.password_field :current_password, required: true, autofocus: true, autocomplete: "current-password", class: "w-full mb-3" %>
-        <%= form.password_field :password, label: { text: "New password"}, required: true, autocomplete: "new-password", class: "w-full mb-3" %>
-        <%= form.password_field :password_confirmation, label: { text: "Confirm new password"}, required: true, autocomplete: "new-password", class: "w-full mb-3" %>
-        <div class="flex flex-row justify-end">
-          <%= form.submit "Save changes", class: "mt-3" %>
-        </div>
+  <%= render(SidebarSectionComponent.new(title: 'Password')) do %>
+    <%= form_with(url: password_path, method: :patch, builder: TailwindFormBuilder) do |form| %>
+      <% if flash[:incorrect_password] %>
+        <%= render ErrorBoxComponent.new(title: 'Incorrect password').with_content(flash[:incorrect_password]) %>
       <% end %>
-    </div>
-  </section>
-  <section class="flex flex-row py-6">
-    <div class="basis-1/4">
-      <h2 class="text-lg font-bold">Email address</h2>
-    </div>
-    <div class="basis-3/4">
-      <% 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>
+      <% if @user.errors.include?(:password) %>
+        <%= render ModelErrorComponent.new(model: @user) %>
       <% end %>
+      <%= form.password_field :current_password, required: true, autofocus: true, autocomplete: "current-password", class: "w-full mb-3" %>
+      <%= form.password_field :password, label: { text: "New password"}, required: true, autocomplete: "new-password", class: "w-full mb-3" %>
+      <%= form.password_field :password_confirmation, label: { text: "Confirm new password"}, required: true, autocomplete: "new-password", class: "w-full mb-3" %>
+      <div class="flex flex-row justify-end">
+        <%= form.submit "Save changes", class: "mt-3" %>
+      </div>
+    <% end %>
+  <% 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>
+  <%= render(SidebarSectionComponent.new(title: 'Email address')) do %>
+    <% 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 %>
-    </div>
-  </section>
-  <section class="flex flex-row py-6">
-    <div class="basis-1/4">
-      <h2 class="text-lg font-bold">Payout details</h2>
-    </div>
-    <div class="basis-3/4">
-      <p class="text-slate-500 mb-6">We use <a class="underline" href="https://wise.com">Wise</a> to make payouts once a month. Wise will email you at <span class="font-bold"><%= Current.user.email %></span> to ask for your bank details. To make the payment we need the following:</p>
+      <% 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 %>
+  <% end %>
 
-      <% payout_detail = @user.payout_detail || PayoutDetail.new(user: @user) %>
-      <%= form_with(model: payout_detail, class: "contents", builder: TailwindFormBuilder) do |form| %>
-        <%= render ModelErrorComponent.new(model: payout_detail) %>
-        <%= form.text_field :name, placeholder: 'Bartholomew J. Simpson', class: 'w-full' %>
-        <p class="text-slate-500 italic text-sm mb-3">Your full name as used on your bank account.</p>
+  <%= render(SidebarSectionComponent.new(title: 'Payout details')) do %>
+    <p class="text-slate-500 mb-6">We use <a class="underline" href="https://wise.com">Wise</a> to make payouts once a month. Wise will email you at <span class="font-bold"><%= Current.user.email %></span> to ask for your bank details. To make the payment we need the following:</p>
 
-        <%= form.select :country, options_for_payout_details_country_select, include_blank: true, label: { text: 'Country / Region' }, class: 'w-full' %>
-        <p class="text-slate-500 italic text-sm mb-3">We support payouts to the regions/countries <a href="https://wise.com/help/articles/2571942/what-countriesregions-can-i-send-to" class="underline">that Wise supports</a>.</p>
+    <% payout_detail = @user.payout_detail || PayoutDetail.new(user: @user) %>
+    <%= form_with(model: payout_detail, class: "contents", builder: TailwindFormBuilder) do |form| %>
+      <%= render ModelErrorComponent.new(model: payout_detail) %>
+      <%= form.text_field :name, placeholder: 'Bartholomew J. Simpson', class: 'w-full' %>
+      <p class="text-slate-500 italic text-sm mb-3">Your full name as used on your bank account.</p>
 
-        <div class="flex flex-row justify-end">
-          <% if payout_detail.persisted? %>
-            <%= form.submit "Save changes", class: 'mt-3'%>
-          <% else %>
-            <%= form.submit "Add payout details", class: 'mt-3'%>
-          <% end %>
-        </div>
-      <% end %>
-    </div>
-  </section>
-</div>
+      <%= form.select :country, options_for_payout_details_country_select, include_blank: true, label: { text: 'Country / Region' }, class: 'w-full' %>
+      <p class="text-slate-500 italic text-sm mb-3">We support payouts to the regions/countries <a href="https://wise.com/help/articles/2571942/what-countriesregions-can-i-send-to" class="underline">that Wise supports</a>.</p>
+
+      <div class="flex flex-row justify-end">
+        <% if payout_detail.persisted? %>
+          <%= form.submit "Save changes", class: 'mt-3'%>
+        <% else %>
+          <%= form.submit "Add payout details", class: 'mt-3'%>
+        <% end %>
+      </div>
+    <% end %>
+  <% end %>
diff --git a/test/components/sidebar_section_component_test.rb b/test/components/sidebar_section_component_test.rb
new file mode 100644
index 00000000..a819d63a
--- /dev/null
+++ b/test/components/sidebar_section_component_test.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class SidebarSectionComponentTest < ViewComponent::TestCase
+  def test_component_renders_something_useful
+    # assert_equal(
+    #   %(<span>Hello, components!</span>),
+    #   render_inline(SidebarSectionComponent.new(message: "Hello, components!")).css("span").to_html
+    # )
+  end
+end

From 294826de8fc738030d2dc4198bf2e9c41652e8d8 Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@gmail.com>
Date: Tue, 29 Oct 2024 15:11:58 +0000
Subject: [PATCH 15/15] User vertical layout for SidebarSectionComponent on
 narrow screens

---
 .../sidebar_section_component.html.erb                      | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/components/sidebar_section_component/sidebar_section_component.html.erb b/app/components/sidebar_section_component/sidebar_section_component.html.erb
index 7c493d4b..71d7ddc6 100644
--- a/app/components/sidebar_section_component/sidebar_section_component.html.erb
+++ b/app/components/sidebar_section_component/sidebar_section_component.html.erb
@@ -1,8 +1,8 @@
-<section class="flex flex-row py-6">
-  <div class="basis-1/4">
+<section class="flex flex-col md:flex-row py-6">
+  <div class="md:basis-1/4 mb-3 md:mb-0">
     <h2 class="text-lg font-bold"><%= @title %></h2>
   </div>
-  <div class="basis-3/4">
+  <div class="md:basis-3/4">
     <%= content %>
   </div>
 </section>