FIX: Admin change email for user process improvements and fixes (#10755)

FIX: Admin change email for user process improvements and fixes (#10755)

See https://meta.discourse.org/t/changing-a-users-email/164512 for context.

When admin changes an email for a user, we were incorrectly sending the password reset email to the user’s old address. Also the new email does not come into effect until the reset password process is done, so this PR adds some notes to the admin to make this clearer.

diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/email.js b/app/assets/javascripts/discourse/app/controllers/preferences/email.js
index f18ed92..2448eed 100644
--- a/app/assets/javascripts/discourse/app/controllers/preferences/email.js
+++ b/app/assets/javascripts/discourse/app/controllers/preferences/email.js
@@ -1,6 +1,6 @@
 import I18n from "I18n";
 import discourseComputed from "discourse-common/utils/decorators";
-import { empty, or } from "@ember/object/computed";
+import { empty, or, alias } from "@ember/object/computed";
 import Controller from "@ember/controller";
 import { propertyEqual } from "discourse/lib/computed";
 import EmberObject from "@ember/object";
@@ -15,6 +15,7 @@ export default Controller.extend({
   success: false,
   oldEmail: null,
   newEmail: null,
+  successMessage: null,
 
   newEmailEmpty: empty("newEmail"),
 
@@ -28,6 +29,8 @@ export default Controller.extend({
 
   unchanged: propertyEqual("newEmailLower", "oldEmail"),
 
+  currentUserAdmin: alias("currentUser.admin"),
+
   @discourseComputed("newEmail")
   newEmailLower(newEmail) {
     return newEmail.toLowerCase().trim();
@@ -77,7 +80,25 @@ export default Controller.extend({
         ? this.model.addEmail(this.newEmail)
         : this.model.changeEmail(this.newEmail)
       ).then(
-        () => this.set("success", true),
+        () => {
+          this.set("success", true);
+
+          if (this.model.staff) {
+            this.set(
+              "successMessage",
+              I18n.t("user.change_email.success_staff")
+            );
+          } else {
+            if (this.currentUser.admin) {
+              this.set(
+                "successMessage",
+                I18n.t("user.change_email.success_via_admin")
+              );
+            } else {
+              this.set("successMessage", I18n.t("user.change_email.success"));
+            }
+          }
+        },
         (e) => {
           this.setProperties({ error: true, saving: false });
           if (
diff --git a/app/assets/javascripts/discourse/app/templates/preferences-email.hbs b/app/assets/javascripts/discourse/app/templates/preferences-email.hbs
index 447d220..d4f4253 100644
--- a/app/assets/javascripts/discourse/app/templates/preferences-email.hbs
+++ b/app/assets/javascripts/discourse/app/templates/preferences-email.hbs
@@ -11,13 +11,7 @@
       <div class="control-group">
         <div class="controls">
           <div class="instructions">
-            <p>
-              {{#if model.staff}}
-                {{i18n "user.change_email.success_staff"}}
-              {{else}}
-                {{i18n "user.change_email.success"}}
-              {{/if}}
-            </p>
+            <p>{{ successMessage }}</p>
           </div>
         </div>
       </div>
@@ -44,6 +38,9 @@
               {{i18n "user.email.instructions"}}
             {{/if}}
           </div>
+          {{#if currentUserAdmin}}
+            <p>{{i18n "user.email.admin_note"}}</p>
+          {{/if}}
         </div>
       </div>
 
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 2396307..34c4cc9 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -1148,6 +1148,7 @@ en:
         taken: "Sorry, that email is not available."
         error: "There was an error changing your email. Perhaps that address is already in use?"
         success: "We've sent an email to that address. Please follow the confirmation instructions."
+        success_via_admin: "Because you are changing the email of the user, we assume that they have lost access to their original email account. We sent a reset password email to the new email address for this user. Please note the user must complete the reset password process for the email address change to take effect."
         success_staff: "We've sent an email to your current address. Please follow the confirmation instructions."
 
       change_avatar:
@@ -1190,6 +1191,7 @@ en:
         sso_override_instructions: "Email can be updated from SSO provider."
         no_secondary: "No secondary emails"
         instructions: "Never shown to the public."
+        admin_note: "Note: An admin user changing another non-admin user's email indicates the user has lost access to their original email account, so a reset password email will be sent to their new address. The user's email will not change until they complete the reset password process."
         ok: "We will email you to confirm"
         required: "Please enter an email address"
         invalid: "Please enter a valid email address"
diff --git a/lib/email_updater.rb b/lib/email_updater.rb
index ad886a0..13633a0 100644
--- a/lib/email_updater.rb
+++ b/lib/email_updater.rb
@@ -120,6 +120,7 @@ class EmailUpdater
     else
       @user.user_emails.create!(email: new_email)
     end
+    @user.reload
 
     DiscourseEvent.trigger(:user_updated, @user)
     @user.set_automatic_groups
diff --git a/spec/components/email_updater_spec.rb b/spec/components/email_updater_spec.rb
index 4fd6cba..ff9d230 100644
--- a/spec/components/email_updater_spec.rb
+++ b/spec/components/email_updater_spec.rb
@@ -49,7 +49,7 @@ describe EmailUpdater do
 
       it "creates a change request authorizing the new email and immediately confirms it " do
         updater.change_to(new_email)
-        change_req = user.email_change_requests.first
+        user.reload
         expect(user.reload.email).to eq(new_email)
       end
 
@@ -59,6 +59,8 @@ describe EmailUpdater do
             updater.change_to(new_email)
           end
         end
+
+        expect(EmailToken.where(user: user).last.email).to eq(new_email)
       end
     end
 

GitHub sha: 3cd601dc

This commit appears in #10755 which was approved by eviltrout. It was merged by martin.