diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-05-03 14:00:51 +0300 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-05-03 14:00:51 +0300 |
commit | 4291e28af72890ee4f9c0f306ba691ba84c3435d (patch) | |
tree | 980036c1fb0affa984da0c8135d0ffca115baa7c | |
parent | 0df317f7297b9a72e888edc09aed51f83414f92a (diff) | |
parent | d386bb780864f4fc36490e19ea654f31bf193d0f (diff) |
Merge branch 'change-primary-email' into 'master'
Allow primary email to be set to an email that you've already added.
Fixes gitlab-com/support-forum#106.
When the user sets their primary email to an email that they've already added to their account, this patch makes sure that secondary email record is destroyed, and a new email record is created for the old primary email. This is based on the assumption that in this case no email was meant to be deleted, but the user simply wanted to change which of their emails is primary.
See merge request !591
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/controllers/admin/users_controller.rb | 3 | ||||
-rw-r--r-- | app/controllers/profiles/emails_controller.rb | 11 | ||||
-rw-r--r-- | app/models/email.rb | 5 | ||||
-rw-r--r-- | app/models/user.rb | 28 | ||||
-rw-r--r-- | app/views/profiles/emails/index.html.haml | 16 |
6 files changed, 46 insertions, 18 deletions
diff --git a/CHANGELOG b/CHANGELOG index 104d3869f93..52783cbdd92 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ v 7.11.0 (unreleased) - Add application setting to restrict user signups to e-mail domains (Stan Hu) - Don't allow a merge request to be merged when its title starts with "WIP". - Add a page title to every page. + - Allow primary email to be set to an email that you've already added. - Get Gitorious importer to work again. - Fix clone URL field and X11 Primary selection (Dmitry Medvinsky) - Ignore invalid lines in .gitmodules diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index adb83996f8b..d36e359934c 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -102,8 +102,7 @@ class Admin::UsersController < Admin::ApplicationController email = user.emails.find(params[:email_id]) email.destroy - user.set_notification_email - user.save if user.notification_email_changed? + user.update_secondary_emails! respond_to do |format| format.html { redirect_to :back, notice: "Successfully removed email." } diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb index 3e904700de5..0ede9b8e21b 100644 --- a/app/controllers/profiles/emails_controller.rb +++ b/app/controllers/profiles/emails_controller.rb @@ -1,14 +1,17 @@ class Profiles::EmailsController < Profiles::ApplicationController def index @primary = current_user.email - @public_email = current_user.public_email @emails = current_user.emails end def create @email = current_user.emails.new(email_params) - flash[:alert] = @email.errors.full_messages.first unless @email.save + if @email.save + NotificationService.new.new_email(@email) + else + flash[:alert] = @email.errors.full_messages.first + end redirect_to profile_emails_url end @@ -17,9 +20,7 @@ class Profiles::EmailsController < Profiles::ApplicationController @email = current_user.emails.find(params[:id]) @email.destroy - current_user.set_notification_email - current_user.set_public_email - current_user.save if current_user.notification_email_changed? or current_user.public_email_changed? + current_user.update_secondary_emails! respond_to do |format| format.html { redirect_to profile_emails_url } diff --git a/app/models/email.rb b/app/models/email.rb index 556b0e9586e..935705e2ed4 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -18,7 +18,6 @@ class Email < ActiveRecord::Base validates :email, presence: true, email: { strict_mode: true }, uniqueness: true validate :unique_email, if: ->(email) { email.email_changed? } - after_create :notify before_validation :cleanup_email def cleanup_email @@ -28,8 +27,4 @@ class Email < ActiveRecord::Base def unique_email self.errors.add(:email, 'has already been taken') if User.exists?(email: self.email) end - - def notify - NotificationService.new.new_email(self) - end end diff --git a/app/models/user.rb b/app/models/user.rb index f22fdc28435..9f198368129 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -139,6 +139,7 @@ class User < ActiveRecord::Base validate :avatar_type, if: ->(user) { user.avatar_changed? } validate :unique_email, if: ->(user) { user.email_changed? } validate :owns_notification_email, if: ->(user) { user.notification_email_changed? } + validate :owns_public_email, if: ->(user) { user.public_email_changed? } validates :avatar, file_size: { maximum: 200.kilobytes.to_i } before_validation :generate_password, on: :create @@ -147,6 +148,7 @@ class User < ActiveRecord::Base before_validation :set_notification_email, if: ->(user) { user.email_changed? } before_validation :set_public_email, if: ->(user) { user.public_email_changed? } + after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? } before_save :ensure_authentication_token after_save :ensure_namespace_correct after_initialize :set_projects_limit @@ -277,13 +279,29 @@ class User < ActiveRecord::Base end def unique_email - self.errors.add(:email, 'has already been taken') if Email.exists?(email: self.email) + if !self.emails.exists?(email: self.email) && Email.exists?(email: self.email) + self.errors.add(:email, 'has already been taken') + end end def owns_notification_email self.errors.add(:notification_email, "is not an email you own") unless self.all_emails.include?(self.notification_email) end + def owns_public_email + self.errors.add(:public_email, "is not an email you own") unless self.all_emails.include?(self.public_email) + end + + def update_emails_with_primary_email + primary_email_record = self.emails.find_by(email: self.email) + if primary_email_record + primary_email_record.destroy + self.emails.create(email: self.email_was) + + self.update_secondary_emails! + end + end + # Groups user has access to def authorized_groups @authorized_groups ||= begin @@ -449,10 +467,16 @@ class User < ActiveRecord::Base def set_public_email if self.public_email.blank? || !self.all_emails.include?(self.public_email) - self.public_email = '' + self.public_email = nil end end + def update_secondary_emails! + self.set_notification_email + self.set_public_email + self.save if self.notification_email_changed? || self.public_email_changed? + end + def set_projects_limit connection_default_value_defined = new_record? && !projects_limit_changed? return unless self.projects_limit.nil? || connection_default_value_defined diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml index c17e01425d8..2c0d0e10a4c 100644 --- a/app/views/profiles/emails/index.html.haml +++ b/app/views/profiles/emails/index.html.haml @@ -5,11 +5,15 @@ Your %b Primary Email will be used for avatar detection and web based operations, such as edits and merges. - %br +%p.light Your %b Notification Email will be used for account notifications. - %br +%p.light + Your + %b Public Email + will be displayed on your public profile. +%p.light All email addresses will be used to identify your commits. %hr @@ -21,13 +25,17 @@ %li %strong= @primary %span.label.label-success Primary Email - - if @primary === @public_email + - if @primary === current_user.public_email %span.label.label-info Public Email + - if @primary === current_user.notification_email + %span.label.label-info Notification Email - @emails.each do |email| %li %strong= email.email - - if email.email === @public_email + - if email.email === current_user.public_email %span.label.label-info Public Email + - if email.email === current_user.notification_email + %span.label.label-info Notification Email %span.cgray added #{time_ago_with_tooltip(email.created_at)} = link_to 'Remove', profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-remove pull-right' |