FIX: Delete unconfirmed emails first if available (#13046)

Users can end up with the same email both as secondary and unconfirmed.
When they tried to delete the unconfirmed ones, the secondary one was
deleted.

app/assets/javascripts/discourse/app/models/user.js

@@ -416,8 +416,11 @@ const User = RestModel.extend({
       type: "DELETE",
       data: { email },
     }).then(() => {
-      this.secondary_emails.removeObject(email);
+      if (this.unconfirmed_emails.includes(email)) {
         this.unconfirmed_emails.removeObject(email);
+      } else {
+        this.secondary_emails.removeObject(email);
+      }
     });
   },
 

app/controllers/users_controller.rb

@@ -285,11 +285,10 @@ class UsersController < ApplicationController
     guardian.ensure_can_edit!(user)
 
     ActiveRecord::Base.transaction do
-      if email = user.user_emails.find_by(email: params[:email], primary: false)
+      if change_requests = user.email_change_requests.where(new_email: params[:email]).presence
-        email.destroy
-        DiscourseEvent.trigger(:user_updated, user)
-      elsif change_requests = user.email_change_requests.where(new_email: params[:email]).presence
         change_requests.destroy_all
+      elsif user.user_emails.where(email: params[:email], primary: false).destroy_all.present?
+        DiscourseEvent.trigger(:user_updated, user)
       else
         return render json: failed_json, status: 428
       end

spec/requests/users_controller_spec.rb

@@ -2898,16 +2898,29 @@ describe UsersController do
       expect(event[:params].first).to eq(user)
     end
 
-    it "can destroy duplicate emails" do
+    it "can destroy unconfirmed emails" do
-      EmailChangeRequest.create!(
+      request_1 = EmailChangeRequest.create!(
         user: user,
-        new_email: user.email,
+        new_email: user_email.email,
         change_state: EmailChangeRequest.states[:authorizing_new]
       )
 
-      delete "/u/#{user.username}/preferences/email.json", params: { email: user_email.email }
+      EmailChangeRequest.create!(
+        user: user,
+        new_email: other_email.email,
+        change_state: EmailChangeRequest.states[:authorizing_new]
+      )
 
-      expect(user.email_change_requests).to be_empty
+      EmailChangeRequest.create!(
+        user: user,
+        new_email: other_email.email,
+        change_state: EmailChangeRequest.states[:authorizing_new]
+      )
+
+      delete "/u/#{user.username}/preferences/email.json", params: { email: other_email.email }
+
+      expect(user.user_emails.pluck(:email)).to contain_exactly(user_email.email, other_email.email)
+      expect(user.email_change_requests).to contain_exactly(request_1)
     end
   end