From fc0194b589238df6171ba3f4aeec79f8034b7128 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Fri, 14 Sep 2018 09:52:09 +0000 Subject: Resolve "Add functionality to change what email address online actions commit using" --- app/controllers/profiles_controller.rb | 1 + app/models/user.rb | 39 ++++++++++++++++++++++++++++++- app/views/profiles/emails/index.html.haml | 8 ++++++- app/views/profiles/show.html.haml | 3 +++ 4 files changed, 49 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 94e6efca487..a43d47797f1 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -96,6 +96,7 @@ class ProfilesController < Profiles::ApplicationController :location, :name, :public_email, + :commit_email, :skype, :twitter, :username, diff --git a/app/models/user.rb b/app/models/user.rb index dac8779488d..d68108a8e8e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -161,6 +161,7 @@ class User < ActiveRecord::Base validates :notification_email, presence: true validates :notification_email, email: true, if: ->(user) { user.notification_email != user.email } validates :public_email, presence: true, uniqueness: true, email: true, allow_blank: true + validates :commit_email, email: true, allow_nil: true, if: ->(user) { user.commit_email != user.email } validates :bio, length: { maximum: 255 }, allow_blank: true validates :projects_limit, presence: true, @@ -173,12 +174,15 @@ class User < ActiveRecord::Base validate :unique_email, if: :email_changed? validate :owns_notification_email, if: :notification_email_changed? validate :owns_public_email, if: :public_email_changed? + validate :owns_commit_email, if: :commit_email_changed? validate :signup_domain_valid?, on: :create, if: ->(user) { !user.created_by_id } before_validation :sanitize_attrs before_validation :set_notification_email, if: :new_record? before_validation :set_public_email, if: :public_email_changed? + before_validation :set_commit_email, if: :commit_email_changed? before_save :set_public_email, if: :public_email_changed? # in case validation is skipped + before_save :set_commit_email, if: :commit_email_changed? # in case validation is skipped before_save :ensure_incoming_email_token before_save :ensure_user_rights_and_limits, if: ->(user) { user.new_record? || user.external_changed? } before_save :skip_reconfirmation!, if: ->(user) { user.email_changed? && user.read_only_attribute?(:email) } @@ -619,6 +623,32 @@ class User < ActiveRecord::Base errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email) end + def owns_commit_email + return if read_attribute(:commit_email).blank? + + errors.add(:commit_email, "is not an email you own") unless verified_emails.include?(commit_email) + end + + # Define commit_email-related attribute methods explicitly instead of relying + # on ActiveRecord to provide them. Some of the specs use the current state of + # the model code but an older database schema, so we need to guard against the + # possibility of the commit_email column not existing. + + def commit_email + return unless has_attribute?(:commit_email) + + # The commit email is the same as the primary email if undefined + super.presence || self.email + end + + def commit_email=(email) + super if has_attribute?(:commit_email) + end + + def commit_email_changed? + has_attribute?(:commit_email) && super + end + # see if the new email is already a verified secondary email def check_for_verified_email skip_reconfirmation! if emails.confirmed.where(email: self.email).any? @@ -873,10 +903,17 @@ class User < ActiveRecord::Base end end + def set_commit_email + if commit_email.blank? || verified_emails.exclude?(commit_email) + self.commit_email = nil + end + end + def update_secondary_emails! set_notification_email set_public_email - save if notification_email_changed? || public_email_changed? + set_commit_email + save if notification_email_changed? || public_email_changed? || commit_email_changed? end def set_projects_limit diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml index 04a19ab14dd..c8faf2b3af3 100644 --- a/app/views/profiles/emails/index.html.haml +++ b/app/views/profiles/emails/index.html.haml @@ -22,7 +22,9 @@ .account-well.append-bottom-default %ul %li - Your Primary Email will be used for avatar detection and web based operations, such as edits and merges. + Your Primary Email will be used for avatar detection. + %li + Your Commit Email will be used for web based operations, such as edits and merges. %li Your Notification Email will be used for account notifications. %li @@ -34,6 +36,8 @@ = render partial: 'shared/email_with_badge', locals: { email: @primary_email, verified: current_user.confirmed? } %span.float-right %span.badge.badge-success Primary email + - if @primary_email === current_user.commit_email + %span.badge.badge-info Commit email - if @primary_email === current_user.public_email %span.badge.badge-info Public email - if @primary_email === current_user.notification_email @@ -42,6 +46,8 @@ %li = render partial: 'shared/email_with_badge', locals: { email: email.email, verified: email.confirmed? } %span.float-right + - if email.email === current_user.commit_email + %span.badge.badge-info Commit email - if email.email === current_user.public_email %span.badge.badge-info Public email - if email.email === current_user.notification_email diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 0a1ee648d97..51f5ecf2166 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -91,6 +91,9 @@ = f.select :public_email, options_for_select(@user.all_emails, selected: @user.public_email), { help: s_("Profiles|This email will be displayed on your public profile."), include_blank: s_("Profiles|Do not show on profile") }, control_class: 'select2' + = f.select :commit_email, options_for_select(@user.verified_emails, selected: @user.commit_email), + { help: 'This email will be used for web based operations, such as edits and merges.' }, + control_class: 'select2' = f.select :preferred_language, Gitlab::I18n::AVAILABLE_LANGUAGES.map { |value, label| [label, value] }, { help: s_("Profiles|This feature is experimental and translations are not complete yet.") }, control_class: 'select2' -- cgit v1.2.1