diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index f0a0541bcea..6d09211cd1b 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/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); - this.unconfirmed_emails.removeObject(email); + if (this.unconfirmed_emails.includes(email)) { + this.unconfirmed_emails.removeObject(email); + } else { + this.secondary_emails.removeObject(email); + } }); }, diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index bb796c35fda..5d7a37e9f98 100644 --- a/app/controllers/users_controller.rb +++ b/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) - email.destroy - DiscourseEvent.trigger(:user_updated, user) - elsif change_requests = user.email_change_requests.where(new_email: params[:email]).presence + if 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 diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 2ef955e6f47..4bbf1fdeac9 100644 --- a/spec/requests/users_controller_spec.rb +++ b/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 - EmailChangeRequest.create!( + it "can destroy unconfirmed emails" do + 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