From 95c1a4f39119112eba9345407ccad9f4578b2d88 Mon Sep 17 00:00:00 2001
From: Emi Matchu <emi@matchu.dev>
Date: Tue, 9 Apr 2024 06:23:54 -0700
Subject: [PATCH] Fix bugs in Settings page when changes to the model are
 incomplete

Ahh okay tricky lil thing: if you show the settings page with a partial
change to `AuthUser` that didn't get saved, it can throw off the state
of some stuff. For example, if you don't have a password yet, then
enter a new password but leave the confirmation box blank, then you'll
correctly see "Password confirmation can't be blank", but you'll *also*
then be prompted for your "Current password", even though you don't
have one yet, because `@auth_user.uses_password?` is true now.

In this change, we extend the Settings form to use two copies of the
`AuthUser`. One is the copy with changes on it, and the other is the
"persisted" copy, which we check for parts of the UI that care about
what's actually saved, vs form state.
---
 app/controllers/auth_users_controller.rb | 13 ++++++++++++-
 app/views/auth_users/edit.html.erb       | 22 ++++++++++++----------
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/app/controllers/auth_users_controller.rb b/app/controllers/auth_users_controller.rb
index 77f9f6d3..45c1241e 100644
--- a/app/controllers/auth_users_controller.rb
+++ b/app/controllers/auth_users_controller.rb
@@ -14,7 +14,9 @@ class AuthUsersController < ApplicationController
 	end
 
 	def edit
-		@auth_user = current_auth_user
+		# For the edit form, the auth user *is* the persisted auth user.
+		@persisted_auth_user = current_auth_user
+		@auth_user = @persisted_auth_user
 	end
 
 	def new
@@ -22,7 +24,16 @@ class AuthUsersController < ApplicationController
 	end
 
 	def update
+		# When updating, we hold onto the original `@persisted_auth_user`, then
+		# make our changes to `@auth_user`. That way, the form can check the *live*
+		# value of `uses_password?` to decide whether to show the "Current
+		# password" field, instead of getting thrown off if the password changed
+		# but the record didn't get saved.
+		#
+		# HACK: Is there a way to get the kind of copy we want for real? `dup`
+		#       actually returns a *new* unsaved record with the same attributes.
 		@auth_user = load_auth_user
+		@persisted_auth_user = @auth_user.dup
 
 		if @auth_user.update_with_password(auth_user_params)
 			# NOTE: Changing the password will sign you out, so make sure we stay
diff --git a/app/views/auth_users/edit.html.erb b/app/views/auth_users/edit.html.erb
index 839c0a3f..7266316e 100644
--- a/app/views/auth_users/edit.html.erb
+++ b/app/views/auth_users/edit.html.erb
@@ -39,7 +39,7 @@
   </fieldset>
 
   <%# Current password is only required if you have one! %>
-  <% if @auth_user.uses_password? %>
+  <% if @persisted_auth_user.uses_password? %>
     <fieldset>
       <div class="field">
         <%= f.label :current_password %>
@@ -55,13 +55,14 @@
   </div>
 <% end %>
 
-<% if @auth_user.uses_neopass? %>
+<% if @persisted_auth_user.uses_neopass? %>
   <%= form_with url: user_neopass_connection_path(@auth_user.user),
       method: :delete, class: "settings-form", data: {
         turbo_confirm: "Are you sure? Without a NeoPass, you'll need to use " +
-          "your password or your recovery email \"#{@auth_user.email}\" to " +
-          "log in again.\n\nMake sure you have everything all set up first! " +
-          "Otherwise, you might be locked out of this account forever!"
+          "your password or your recovery email " +
+          "\"#{@persisted_auth_user.email}\" to log in again.\n\nMake sure " +
+          "you have everything all set up first! Otherwise, you might be " +
+          "locked out of this account forever!"
       } do |form|
   %>
     <h2>Your NeoPass</h2>
@@ -69,7 +70,7 @@
       <strong>
         NeoPass ID:
       </strong>
-      <%= @auth_user.neopass_friendly_id %>
+      <%= @persisted_auth_user.neopass_friendly_id %>
     </section>
     <section class="neopass-explanation">
       <p>
@@ -78,23 +79,24 @@
         you can still use "Forgot your password?" to recover your Dress to
         Impress account, using the Email saved in "Your info".
       </p>
-      <% if !@auth_user.uses_password? && !@auth_user.email %>
+      <% if !@persisted_auth_user.uses_password? && !@persisted_auth_user.email? %>
         <p>
           You can't remove this NeoPass yet, because you need to either set a
           password or a recovery email first. (Ideally both!)
         </p>
-      <% elsif !@auth_user.uses_password? %>
+      <% elsif !@persisted_auth_user.uses_password? %>
         <p>
           Be extra careful here! Your account doesn't have a password set.
         </p>
-      <% elsif !@auth_user.email? %>
+      <% elsif !@persisted_auth_user.email? %>
         <p>
           Be extra careful here! Your account doesn't have an email set.
         </p>
       <% end %>
     </section>
     <%= form.submit "Disconnect your NeoPass",
-        disabled: !@auth_user.uses_password? && !@auth_user.email? %>
+        disabled: !@persisted_auth_user.uses_password? &&
+          !@persisted_auth_user.email? %>
   <% end %>
 <% elsif can_use_neopass %>
   <%= form_with url: auth_user_neopass_omniauth_authorize_path(intent: "connect"),