Merge remote-tracking branch 'origin/master' into...

Merge remote-tracking branch 'origin/master' into feature/sm/35954-create-kubernetes-cluster-on-gke-from-k8s-service

CHANGELOG.md

@@ -2,6 +2,23 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 10.0.3 (2017-10-05)
+
+- [FIXED] find_user Users helper method no longer overrides find_user API helper method. !14418
+- [FIXED] Fix CSRF validation issue when closing/opening merge requests from the UI. !14555
+- [FIXED] Kubernetes integration: ensure v1.8.0 compatibility. !14635
+- [FIXED] Fixes data parameter not being sent in ajax request for jobs log.
+- [FIXED] Improves UX of autodevops popover to match gpg one.
+- [FIXED] Fixed commenting on side-by-side commit diff.
+- [FIXED] Make sure API responds with 401 when invalid authentication info is provided.
+- [FIXED] Fix merge request counter updates after merge.
+- [FIXED] Fix gitlab-rake gitlab:import:repos task failing.
+- [FIXED] Fix pushes to an empty repository not invalidating has_visible_content? cache.
+- [FIXED] Ensure all refs are restored on a restore from backup.
+- [FIXED] Gitaly RepositoryExists remains opt-in for all method calls.
+- [FIXED] Fix 500 error on merged merge requests when GitLab is restored from a backup.
+- [FIXED] Adjust MRs being stuck on "process of being merged" for more than 2 hours.
+
 ## 10.0.2 (2017-09-27)
 
 - [FIXED] Notes will not show an empty bubble when the author isn't a member. !14450
@@ -195,6 +212,10 @@ entry.
 - Added type to CHANGELOG entries. (Jacopo Beschi @jacopo-beschi)
 - [BUGIFX] Improves subgroup creation permissions. !13418
 
+## 9.5.8 (2017-10-04)
+
+- [FIXED] Fixed fork button being disabled for users who can fork to a group.
+
 ## 9.5.7 (2017-10-03)
 
 - Fix gitlab rake:import:repos task.

app/assets/javascripts/monitoring/components/dashboard.vue

@@ -29,6 +29,7 @@
         showEmptyState: true,
         updateAspectRatio: false,
         updatedAspectRatios: 0,
+        hoverData: {},
         resizeThrottled: {},
       };
     },
@@ -64,6 +65,10 @@
           this.updatedAspectRatios = 0;
         }
       },
+
+      hoverChanged(data) {
+        this.hoverData = data;
+      },
     },
 
     created() {
@@ -72,10 +77,12 @@
         deploymentEndpoint: this.deploymentEndpoint,
       });
       eventHub.$on('toggleAspectRatio', this.toggleAspectRatio);
+      eventHub.$on('hoverChanged', this.hoverChanged);
     },
 
     beforeDestroy() {
       eventHub.$off('toggleAspectRatio', this.toggleAspectRatio);
+      eventHub.$off('hoverChanged', this.hoverChanged);
       window.removeEventListener('resize', this.resizeThrottled, false);
     },
 
@@ -102,6 +109,7 @@
         v-for="(graphData, index) in groupData.metrics"
         :key="index"
         :graph-data="graphData"
+        :hover-data="hoverData"
         :update-aspect-ratio="updateAspectRatio"
         :deployment-data="store.deploymentData"
       />

app/assets/javascripts/monitoring/components/empty_state.vue

@@ -73,34 +73,22 @@
 
 <template>
   <div class="prometheus-state">
-    <div class="row">
-      <div class="col-md-4 col-md-offset-4 state-svg svg-content">
-        <img :src="currentState.svgUrl"/>
-      </div>
+    <div class="state-svg svg-content">
+      <img :src="currentState.svgUrl"/>
     </div>
-    <div class="row">
-      <div class="col-md-6 col-md-offset-3">
-        <h4 class="text-center state-title">
-          {{currentState.title}}
-        </h4>
-      </div>
-    </div>
-    <div class="row">
-      <div class="col-md-6 col-md-offset-3">
-        <div class="description-text text-center state-description">
-          {{currentState.description}}
-          <a v-if="showButtonDescription" :href="settingsPath">
-            Prometheus server
-          </a>
-        </div>
-      </div>
-    </div>
-    <div class="row state-button-section">
-      <div class="col-md-4 col-md-offset-4 text-center state-button">
-        <a class="btn btn-success" :href="buttonPath">
-          {{currentState.buttonText}}
-        </a>
-      </div>
+    <h4 class="state-title">
+      {{currentState.title}}
+    </h4>
+    <p class="state-description">
+      {{currentState.description}}
+      <a v-if="showButtonDescription" :href="settingsPath">
+        Prometheus server
+      </a>
+    </p>
+    <div class="state-button">
+      <a class="btn btn-success" :href="buttonPath">
+        {{currentState.buttonText}}
+      </a>
     </div>
   </div>
 </template>

app/assets/javascripts/monitoring/components/graph.vue

@@ -7,12 +7,10 @@
   import MonitoringMixin from '../mixins/monitoring_mixins';
   import eventHub from '../event_hub';
   import measurements from '../utils/measurements';
-  import { timeScaleFormat } from '../utils/date_time_formatters';
+  import { timeScaleFormat, bisectDate } from '../utils/date_time_formatters';
   import createTimeSeries from '../utils/multiple_time_series';
   import bp from '../../breakpoints';
 
-  const bisectDate = d3.bisector(d => d.time).left;
-
   export default {
     props: {
       graphData: {
@@ -27,6 +25,11 @@
         type: Array,
         required: true,
       },
+      hoverData: {
+        type: Object,
+        required: false,
+        default: () => ({}),
+      },
     },
 
     mixins: [MonitoringMixin],
@@ -52,6 +55,7 @@
         currentXCoordinate: 0,
         currentFlagPosition: 0,
         showFlag: false,
+        showFlagContent: false,
         showDeployInfo: true,
         timeSeries: [],
       };
@@ -122,22 +126,14 @@
         const d1 = firstTimeSeries.values[overlayIndex];
         if (d0 === undefined || d1 === undefined) return;
         const evalTime = timeValueOverlay - d0[0] > d1[0] - timeValueOverlay;
-        this.currentData = evalTime ? d1 : d0;
-        this.currentDataIndex = evalTime ? overlayIndex : (overlayIndex - 1);
-        this.currentXCoordinate = Math.floor(firstTimeSeries.timeSeriesScaleX(this.currentData.time));
+        const hoveredDataIndex = evalTime ? overlayIndex : (overlayIndex - 1);
+        const hoveredDate = firstTimeSeries.values[hoveredDataIndex].time;
         const currentDeployXPos = this.mouseOverDeployInfo(point.x);
 
-        if (this.currentXCoordinate > (this.graphWidth - 200)) {
-          this.currentFlagPosition = this.currentXCoordinate - 103;
-        } else {
-          this.currentFlagPosition = this.currentXCoordinate;
-        }
-
-        if (currentDeployXPos) {
-          this.showFlag = false;
-        } else {
-          this.showFlag = true;
-        }
+        eventHub.$emit('hoverChanged', {
+          hoveredDate,
+          currentDeployXPos,
+        });
       },
 
       renderAxesPaths() {
@@ -194,6 +190,10 @@
           eventHub.$emit('toggleAspectRatio');
         }
       },
+
+      hoverData() {
+        this.positionFlag();
+      },
     },
 
     mounted() {
@@ -203,7 +203,10 @@
 </script>
 
 <template>
-  <div class="prometheus-graph">
+  <div 
+    class="prometheus-graph"
+    @mouseover="showFlagContent = true"
+    @mouseleave="showFlagContent = false">
     <h5 class="text-center graph-title">
       {{graphData.title}}
     </h5>
@@ -247,6 +250,7 @@
             <graph-deployment
               :show-deploy-info="showDeployInfo"
               :deployment-data="reducedDeploymentData"
+              :graph-width="graphWidth"
               :graph-height="graphHeight"
               :graph-height-offset="graphHeightOffset"
             />
@@ -257,6 +261,7 @@
               :current-flag-position="currentFlagPosition"
               :graph-height="graphHeight"
               :graph-height-offset="graphHeightOffset"
+              :show-flag-content="showFlagContent"
             />
             <rect
               class="prometheus-graph-overlay"

app/assets/javascripts/monitoring/components/graph/deployment.vue

@@ -19,6 +19,10 @@
         type: Number,
         required: true,
       },
+      graphWidth: {
+        type: Number,
+        required: true,
+      },
     },
 
     computed: {
@@ -47,6 +51,14 @@
       transformDeploymentGroup(deployment) {
         return `translate(${Math.floor(deployment.xPos) + 1}, 20)`;
       },
+
+      positionFlag(deployment) {
+        let xPosition = 3;
+        if (deployment.xPos > (this.graphWidth - 200)) {
+          xPosition = -97;
+        }
+        return xPosition;
+      },
     },
   };
 </script>
@@ -77,7 +89,7 @@
       <svg
         v-if="deployment.showDeploymentFlag"
         class="js-deploy-info-box"
-        x="3"
+        :x="positionFlag(deployment)"
         y="0"
         width="92"
         height="60">

app/assets/javascripts/monitoring/components/graph/flag.vue

@@ -23,6 +23,10 @@
         type: Number,
         required: true,
       },
+      showFlagContent: {
+        type: Boolean,
+        required: true,
+      },
     },
 
     data() {
@@ -57,6 +61,7 @@
       transform="translate(-5, 20)">
     </line>
     <svg 
+      v-if="showFlagContent"
       class="rect-text-metric"
       :x="currentFlagPosition"
       y="0">

app/assets/javascripts/monitoring/mixins/monitoring_mixins.js

+import { bisectDate } from '../utils/date_time_formatters';
+
 const mixins = {
   methods: {
     mouseOverDeployInfo(mouseXPos) {
@@ -18,6 +20,7 @@ const mixins = {
 
       return dataFound;
     },
+
     formatDeployments() {
       this.reducedDeploymentData = this.deploymentData.reduce((deploymentDataArray, deployment) => {
         const time = new Date(deployment.created_at);
@@ -40,6 +43,25 @@ const mixins = {
         return deploymentDataArray;
       }, []);
     },
+
+    positionFlag() {
+      const timeSeries = this.timeSeries[0];
+      const hoveredDataIndex = bisectDate(timeSeries.values, this.hoverData.hoveredDate, 1);
+      this.currentData = timeSeries.values[hoveredDataIndex];
+      this.currentDataIndex = hoveredDataIndex;
+      this.currentXCoordinate = Math.floor(timeSeries.timeSeriesScaleX(this.currentData.time));
+      if (this.currentXCoordinate > (this.graphWidth - 200)) {
+        this.currentFlagPosition = this.currentXCoordinate - 103;
+      } else {
+        this.currentFlagPosition = this.currentXCoordinate;
+      }
+
+      if (this.hoverData.currentDeployXPos) {
+        this.showFlag = false;
+      } else {
+        this.showFlag = true;
+      }
+    },
   },
 };
 

app/assets/javascripts/monitoring/monitoring_bundle.js

@@ -3,8 +3,5 @@ import Dashboard from './components/dashboard.vue';
 
 document.addEventListener('DOMContentLoaded', () => new Vue({
   el: '#prometheus-graphs',
-  components: {
-    Dashboard,
-  },
-  render: createElement => createElement('dashboard'),
+  render: createElement => createElement(Dashboard),
 }));

app/assets/javascripts/monitoring/stores/monitoring_store.js

@@ -13,7 +13,7 @@ function normalizeMetrics(metrics) {
         ...result,
         values: result.values.map(([timestamp, value]) => ({
           time: new Date(timestamp * 1000),
-          value,
+          value: Number(value),
         })),
       })),
     })),

app/assets/javascripts/monitoring/utils/date_time_formatters.js

@@ -2,6 +2,7 @@ import d3 from 'd3';
 
 export const dateFormat = d3.time.format('%b %-d, %Y');
 export const timeFormat = d3.time.format('%-I:%M%p');
+export const bisectDate = d3.bisector(d => d.time).left;
 
 export const timeScaleFormat = d3.time.format.multi([
   ['.%L', d => d.getMilliseconds()],

app/assets/javascripts/repo/components/repo_sidebar.vue

@@ -74,8 +74,8 @@ export default {
     <thead v-if="!isMini">
       <tr>
         <th class="name">Name</th>
-        <th class="hidden-sm hidden-xs last-commit">Last Commit</th>
-        <th class="hidden-xs last-update text-right">Last Update</th>
+        <th class="hidden-sm hidden-xs last-commit">Last commit</th>
+        <th class="hidden-xs last-update text-right">Last update</th>
       </tr>
     </thead>
     <tbody>

app/assets/stylesheets/pages/environments.scss

@@ -207,10 +207,13 @@
 }
 
 .prometheus-state {
-  margin-top: 10px;
+  max-width: 430px;
+  margin: 10px auto;
+  text-align: center;
 
-  .state-button-section {
-    margin-top: 10px;
+  .state-svg {
+    max-width: 80vw;
+    margin: 0 auto;
   }
 }
 

app/assets/stylesheets/pages/profile.scss

@@ -392,11 +392,11 @@ table.u2f-registrations {
   }
 }
 
-.gpg-email-badge {
+.email-badge {
   display: inline;
   margin-right: $gl-padding / 2;
 
-  .gpg-email-badge-email {
+  .email-badge-email {
     display: inline;
     margin-right: $gl-padding / 4;
   }

app/controllers/admin/users_controller.rb

@@ -155,7 +155,7 @@ class Admin::UsersController < Admin::ApplicationController
 
   def remove_email
     email = user.emails.find(params[:email_id])
-    success = Emails::DestroyService.new(current_user, user: user, email: email.email).execute
+    success = Emails::DestroyService.new(current_user, user: user).execute(email)
 
     respond_to do |format|
       if success

app/controllers/application_controller.rb

@@ -85,12 +85,21 @@ class ApplicationController < ActionController::Base
     super
     payload[:remote_ip] = request.remote_ip
 
-    if current_user.present?
-      payload[:user_id] = current_user.id
-      payload[:username] = current_user.username
+    logged_user = auth_user
+
+    if logged_user.present?
+      payload[:user_id] = logged_user.try(:id)
+      payload[:username] = logged_user.try(:username)
     end
   end
 
+  # Controllers such as GitHttpController may use alternative methods
+  # (e.g. tokens) to authenticate the user, whereas Devise sets current_user
+  def auth_user
+    return current_user if current_user.present?
+    return try(:authenticated_user)
+  end
+
   # This filter handles both private tokens and personal access tokens
   def authenticate_user_from_private_token!
     token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence

app/controllers/confirmations_controller.rb

@@ -10,13 +10,14 @@ class ConfirmationsController < Devise::ConfirmationsController
     users_almost_there_path
   end
 
-  def after_confirmation_path_for(resource_name, resource)
-    if signed_in?(resource_name)
+  def after_confirmation_path_for(_resource_name, resource)
+    # incoming resource can either be a :user or an :email
+    if signed_in?(:user)
       after_sign_in(resource)
     else
       Gitlab::AppLogger.info("Email Confirmed: username=#{resource.username} email=#{resource.email} ip=#{request.remote_ip}")
       flash[:notice] += " Please sign in."
-      new_session_path(resource_name)
+      new_session_path(:user)
     end
   end
 

app/controllers/profiles/emails_controller.rb

 class Profiles::EmailsController < Profiles::ApplicationController
+  before_action :find_email, only: [:destroy, :resend_confirmation_instructions]
+
   def index
-    @primary = current_user.email
+    @primary_email = current_user.email
     @emails = current_user.emails.order_id_desc
   end
 
   def create
     @email = Emails::CreateService.new(current_user, email_params.merge(user: current_user)).execute
-
-    if @email.errors.blank?
-      NotificationService.new.new_email(@email)
-    else
+    unless @email.errors.blank?
       flash[:alert] = @email.errors.full_messages.first
     end
 
@@ -17,9 +16,7 @@ class Profiles::EmailsController < Profiles::ApplicationController
   end
 
   def destroy
-    @email = current_user.emails.find(params[:id])
-
-    Emails::DestroyService.new(current_user, user: current_user, email: @email.email).execute
+    Emails::DestroyService.new(current_user, user: current_user).execute(@email)
 
     respond_to do |format|
       format.html { redirect_to profile_emails_url, status: 302 }
@@ -27,9 +24,23 @@ class Profiles::EmailsController < Profiles::ApplicationController
     end
   end
 
+  def resend_confirmation_instructions
+    if Emails::ConfirmService.new(current_user, user: current_user).execute(@email)
+      flash[:notice] = "Confirmation email sent to #{@email.email}"
+    else
+      flash[:alert] = "There was a problem sending the confirmation email"
+    end
+
+    redirect_to profile_emails_url
+  end
+
   private
 
   def email_params
     params.require(:email).permit(:email)
   end
+
+  def find_email
+    @email = current_user.emails.find(params[:id])
+  end
 end

app/controllers/projects/git_http_client_controller.rb

@@ -9,6 +9,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   delegate :actor, :authentication_abilities, to: :authentication_result, allow_nil: true
 
   alias_method :user, :actor
+  alias_method :authenticated_user, :actor
 
   # Git clients will not know what authenticity token to send along
   skip_before_action :verify_authenticity_token

app/mailers/emails/profile.rb

@@ -7,12 +7,6 @@ module Emails
       mail(to: @user.notification_email, subject: subject("Account was created for you"))
     end
 
-    def new_email_email(email_id)
-      @email = Email.find(email_id)
-      @current_user = @user = @email.user
-      mail(to: @user.notification_email, subject: subject("Email was added to your account"))
-    end
-
     def new_ssh_key_email(key_id)
       @key = Key.find_by(id: key_id)
 

app/models/email.rb

@@ -7,6 +7,13 @@ class Email < ActiveRecord::Base
   validates :email, presence: true, uniqueness: true, email: true
   validate :unique_email, if: ->(email) { email.email_changed? }
 
+  scope :confirmed, -> { where.not(confirmed_at: nil) }
+
+  after_commit :update_invalid_gpg_signatures, if: -> { previous_changes.key?('confirmed_at') }
+
+  devise :confirmable
+  self.reconfirmable = false  # currently email can't be changed, no need to reconfirm
+
   def email=(value)
     write_attribute(:email, value.downcase.strip)
   end
@@ -14,4 +21,9 @@ class Email < ActiveRecord::Base
   def unique_email
     self.errors.add(:email, 'has already been taken') if User.exists?(email: self.email)
   end
+
+  # once email is confirmed, update the gpg signatures
+  def update_invalid_gpg_signatures
+    user.update_invalid_gpg_signatures if confirmed?
+  end
 end

app/models/user.rb

@@ -163,15 +163,16 @@ class User < ActiveRecord::Base
   before_validation :sanitize_attrs
   before_validation :set_notification_email, if: :email_changed?
   before_validation :set_public_email, if: :public_email_changed?
-
-  after_update :update_emails_with_primary_email, if: :email_changed?
   before_save :ensure_authentication_token, :ensure_incoming_email_token
   before_save :ensure_user_rights_and_limits, if: :external_changed?
   before_save :skip_reconfirmation!, if: ->(user) { user.email_changed? && user.read_only_attribute?(:email) }
+  before_save :check_for_verified_email, if: ->(user) { user.email_changed? && !user.new_record? }
   after_save :ensure_namespace_correct
+  after_destroy :post_destroy_hook
+  after_commit :update_emails_with_primary_email, on: :update, if: -> { previous_changes.key?('email') }
   after_commit :update_invalid_gpg_signatures, on: :update, if: -> { previous_changes.key?('email') }
+
   after_initialize :set_projects_limit
-  after_destroy :post_destroy_hook
 
   # User's Layout preference
   enum layout: [:fixed, :fluid]
@@ -525,12 +526,24 @@ class User < ActiveRecord::Base
     errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email)
   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?
+  end
+
+  # Note: the use of the Emails services will cause `saves` on the user object, running
+  # through the callbacks again and can have side effects, such as the `previous_changes`
+  # hash and `_was` variables getting munged.
+  # By using an `after_commit` instead of `after_update`, we avoid the recursive callback
+  # scenario, though it then requires us to use the `previous_changes` hash
   def update_emails_with_primary_email
+    previous_email = previous_changes[:email][0]  # grab this before the DestroyService is called
     primary_email_record = emails.find_by(email: email)
-    if primary_email_record
-      Emails::DestroyService.new(self, user: self, email: email).execute
-      Emails::CreateService.new(self, user: self, email: email_was).execute
-    end
+    Emails::DestroyService.new(self, user: self).execute(primary_email_record) if primary_email_record
+
+    # the original primary email was confirmed, and we want that to carry over.  We don't
+    # have access to the original confirmation values at this point, so just set confirmed_at
+    Emails::CreateService.new(self, user: self, email: previous_email).execute(confirmed_at: confirmed_at)
   end
 
   def update_invalid_gpg_signatures
@@ -816,6 +829,10 @@ class User < ActiveRecord::Base
     avatar_path(args) || GravatarService.new.execute(email, size, scale, username: username)
   end
 
+  def primary_email_verified?
+    confirmed? && !temp_oauth_email?
+  end
+
   def all_emails
     all_emails = []
     all_emails << email unless temp_oauth_email?
@@ -823,6 +840,18 @@ class User < ActiveRecord::Base
     all_emails
   end
 
+  def verified_emails
+    verified_emails = []
+    verified_emails << email if primary_email_verified?
+    verified_emails.concat(emails.confirmed.pluck(:email))
+    verified_emails
+  end
+
+  def verified_email?(check_email)
+    downcased = check_email.downcase
+    email == downcased ? primary_email_verified? : emails.confirmed.where(email: downcased).exists?
+  end
+
   def hook_attrs
     {
       name: name,
@@ -1047,10 +1076,6 @@ class User < ActiveRecord::Base
     ensure_rss_token!
   end
 
-  def verified_email?(email)
-    self.email == email
-  end
-
   def sync_attribute?(attribute)
     return true if ldap_user? && attribute == :email
 

app/serializers/commit_entity.rb

-class CommitEntity < API::Entities::RepoCommit
+class CommitEntity < API::Entities::Commit
   include RequestAwareEntity
 
   expose :author, using: UserEntity

app/services/emails/base_service.rb

 module Emails
   class BaseService
-    def initialize(current_user, opts)
-      @current_user = current_user
-      @user = opts.delete(:user)
-      @email = opts[:email]
+    def initialize(current_user, params = {})
+      @current_user, @params = current_user, params.dup
+      @user = params.delete(:user)
     end
   end
 end

app/services/emails/confirm_service.rb

+module Emails
+  class ConfirmService < ::Emails::BaseService
+    def execute(email)
+      email.resend_confirmation_instructions
+    end
+  end
+end

app/services/emails/create_service.rb

 module Emails
   class CreateService < ::Emails::BaseService
-    def execute
-      @user.emails.create(email: @email)
+    def execute(extra_params = {})
+      @user.emails.create(@params.merge(extra_params))
     end
   end
 end

app/services/emails/destroy_service.rb

 module Emails
   class DestroyService < ::Emails::BaseService
-    def execute
-      update_secondary_emails! if Email.find_by_email!(@email).destroy
+    def execute(email)
+      email.destroy && update_secondary_emails!
     end
 
     private

app/services/notification_service.rb

@@ -31,13 +31,6 @@ class NotificationService
     end
   end
 
-  # Always notify user about email added to profile
-  def new_email(email)
-    if email.user&.can?(:receive_notifications)
-      mailer.new_email_email(email.id).deliver_later
-    end
-  end
-
   # When create an issue we should send an email to:
   #
   #  * issue assignee if their notification level is not Disabled

app/views/devise/mailer/_confirmation_instructions_account.html.haml

+- confirmation_link = confirmation_url(@resource, confirmation_token: @token)
+- if @resource.unconfirmed_email.present?
+  #content
+    = email_default_heading(@resource.unconfirmed_email)
+    %p Click the link below to confirm your email address.
+    #cta
+      = link_to 'Confirm your email address', confirmation_link
+- else
+  #content
+    - if Gitlab.com?
+      = email_default_heading('Thanks for signing up to GitLab!')
+    - else
+      = email_default_heading("Welcome, #{@resource.name}!")
+    %p To get started, click the link below to confirm your account.
+    #cta
+      = link_to 'Confirm your account', confirmation_link

app/views/devise/mailer/_confirmation_instructions_account.text.erb

+<% if @resource.unconfirmed_email.present? %>
+<%= @resource.unconfirmed_email %>,
+
+Use the link below to confirm your email address.
+<% else %>
+  <% if Gitlab.com? %>
+Thanks for signing up to GitLab!
+  <% else %>
+Welcome, <%= @resource.name %>!
+  <% end %>
+To get started, use the link below to confirm your account.
+<% end %>  
+
+<%= confirmation_url(@resource, confirmation_token: @token) %>

app/views/devise/mailer/_confirmation_instructions_secondary.html.haml

+#content
+  = email_default_heading("#{@resource.user.name}, you've added an additional email!")
+  %p Click the link below to confirm your email address (#{@resource.email})
+  #cta
+    = link_to 'Confirm your email address', confirmation_url(@resource, confirmation_token: @token)
+  %p
+    If this email was added in error, you can remove it here:
+    = link_to "Emails", profile_emails_url

app/views/notify/new_email_email.text.erb → app/views/devise/mailer/_confirmation_instructions_secondary.text.erb

-Hi <%= @user.name %>!
+<%= @resource.user.name %>, you've added an additional email!
 
-A new email was added to your account:
+Use the link below to confirm your email address (<%= @resource.email %>)
 
-email.................. <%= @email.email %>
+<%= confirmation_url(@resource, confirmation_token: @token) %>
 
 If this email was added in error, you can remove it here: <%= profile_emails_url %>

app/views/devise/mailer/confirmation_instructions.html.haml

-- confirmation_link = confirmation_url(@resource, confirmation_token: @token)
-- if @resource.unconfirmed_email.present?
-  #content
-    = email_default_heading(@resource.unconfirmed_email)
-    %p Click the link below to confirm your email address.
-  #cta
-    = link_to confirmation_link, confirmation_link
-- else
-  #content
-    - if Gitlab.com?
-      = email_default_heading('Thanks for signing up to GitLab!')
-    - else
-      = email_default_heading("Welcome, #{@resource.name}!")
-    %p To get started, click the link below to confirm your account.
-  #cta
-    = link_to confirmation_link, confirmation_link
+= render partial: "confirmation_instructions_#{@resource.is_a?(User) ? 'account' : 'secondary'}"

app/views/devise/mailer/confirmation_instructions.text.erb

-Welcome, <%= @resource.name %>!
-
-<% if @resource.unconfirmed_email.present? %>
-You can confirm your email (<%= @resource.unconfirmed_email %>) through the link below:
-<% else %>
-You can confirm your account through the link below:
-<% end %>
-
-<%= confirmation_url(@resource, confirmation_token: @token) %>
+<%= render partial: "confirmation_instructions_#{@resource.is_a?(User) ? 'account' : 'secondary'}" %>
\ No newline at end of file

app/views/groups/milestones/_header_title.html.haml

-- header_title group_title(@group, "Milestones", group_milestones_path(@group))
+- breadcrumb_title @milestone.title
+- add_to_breadcrumbs "Milestones", group_milestones_path(@group)

app/views/notify/new_email_email.html.haml

-%p
-  Hi #{@user.name}!
-%p
-  A new email was added to your account:
-%p
-  email:
-  %code= @email.email
-%p
-  If this email was added in error, you can remove it here:
-  = link_to "Emails", profile_emails_url

app/views/profiles/emails/index.html.haml

@@ -32,19 +32,25 @@
           All email addresses will be used to identify your commits.
     %ul.well-list
       %li
-        = @primary
+        = render partial: 'shared/email_with_badge', locals: { email: @primary_email, verified: current_user.confirmed? }
         %span.pull-right
           %span.label.label-success Primary email
-          - if @primary === current_user.public_email
+          - if @primary_email === current_user.public_email
             %span.label.label-info Public email
-          - if @primary === current_user.notification_email
+          - if @primary_email === current_user.notification_email
             %span.label.label-info Notification email
       - @emails.each do |email|
         %li
-          = email.email
+          = render partial: 'shared/email_with_badge', locals: { email: email.email, verified: email.confirmed? }
           %span.pull-right
             - 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
-            = link_to 'Remove', profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-warning prepend-left-10'
+            - unless email.confirmed?
+              - confirm_title = "#{email.confirmation_sent_at ? 'Resend' : 'Send'} confirmation email"
+              = link_to confirm_title, resend_confirmation_instructions_profile_email_path(email), method: :put, class: 'btn btn-sm btn-warning prepend-left-10'
+
+            = link_to profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-danger prepend-left-10' do
+              %span.sr-only Remove
+              = icon('trash')

app/views/profiles/gpg_keys/_key.html.haml

@@ -3,7 +3,7 @@
     = icon 'key', class: "settings-list-icon hidden-xs"
   .key-list-item-info
     - key.emails_with_verified_status.map do |email, verified|
-      = render partial: 'email_with_badge', locals: { email: email, verified: verified }
+      = render partial: 'shared/email_with_badge', locals: { email: email, verified: verified }
 
     .description
       %code= key.fingerprint

app/views/projects/tree/_old_tree_content.html.haml

@@ -6,7 +6,7 @@
           %th= s_('ProjectFileTree|Name')
           %th.hidden-xs
             .pull-left= _('Last commit')
-          %th.text-right= _('Last Update')
+          %th.text-right= _('Last update')
       - if @path.present?
         %tr.tree-item
           %td.tree-item-file-name

app/views/profiles/gpg_keys/_email_with_badge.html.haml → app/views/shared/_email_with_badge.html.haml

@@ -2,7 +2,7 @@
 - css_classes << (verified ? 'verified': 'unverified')
 - text = verified ? 'Verified' : 'Unverified'
 
-.gpg-email-badge
-  .gpg-email-badge-email= email
+.email-badge
+  .email-badge-email= email
   %div{ class: css_classes }
     = text

changelogs/unreleased/37467-helper-method-from-users-endpoint-overrides-api-helper-method.yml

----
-title: find_user Users helper method no longer overrides find_user API helper method.
-merge_request: 14418
-author:
-type: fixed

changelogs/unreleased/38031-monitoring-hover-info-is-clipped.yml

+---
+title: Move the deployment flag content to the left when deployment marker is near
+  the end
+merge_request: 14514
+author:
+type: fixed

changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml → changelogs/unreleased/38036-hover-and-legend-data-should-be-linked.yml

 ---
-title: Adjust MRs being stuck on "process of being merged" for more than 2 hours
+title: Sync up hover and legend data across all graphs for the prometheus dashboard
 merge_request:
 author:
 type: fixed

changelogs/unreleased/38528-build-url.yml

----
-title: Fixes data parameter not being sent in ajax request for jobs log
-merge_request:
-author:
-type: fixed

changelogs/unreleased/38582-popover-badge.yml

----
-title: Improves UX of autodevops popover to match gpg one
-merge_request:
-author:
-type: fixed

changelogs/unreleased/38789-prometheus-graphs-occasionally-have-incorrect-y-scale.yml

+---
+title: Fix broken Y-axis scaling in some Prometheus graphs
+merge_request: 14693
+author:
+type: fixed

changelogs/unreleased/bvl-fix-close-issuable-link.yml

----
-title: Fix CSRF validation issue when closing/opening merge requests from the UI
-merge_request: 14555
-author:
-type: fixed

changelogs/unreleased/declarative-policy-optimisations.yml

+---
+title: Speed up permission checks
+merge_request:
+author:
+type: other

changelogs/unreleased/dm-api-unauthorized.yml

----
-title: Make sure API responds with 401 when invalid authentication info is provided
-merge_request:
-author:
-type: fixed

changelogs/unreleased/feature-verify_secondary_emails.yml

+---
+title: A confirmation email is now sent when adding a secondary email address
+merge_request:
+author: digitalmoksha
+type: added

changelogs/unreleased/fix-kubectl-180.yml

----
-title: 'Kubernetes integration: ensure v1.8.0 compatibility'
-merge_request: 14635
-author:
-type: fixed

changelogs/unreleased/fork-btn-enabled-user-groups.yml

----
-title: Fixed fork button being disabled for users who can fork to a group
-merge_request:
-author:
-type: fixed

changelogs/unreleased/commit-side-by-side-comment.yml → changelogs/unreleased/group-milestones-breadcrumb.yml

 ---
-title: Fixed commenting on side-by-side commit diff
+title: Fixed milestone breadcrumb links
 merge_request:
 author:
 type: fixed

changelogs/unreleased/remove_repo_prefix_from_api.yml

+---
+title: Remove 'Repo' prefix from API entites
+merge_request: 14694
+author: Vitaliy @blackst0ne Klachkov
+type: other

changelogs/unreleased/sh-fix-import-repos.yml

----
-title: Fix gitlab-rake gitlab:import:repos task failing
-merge_request:
-author:
-type: fixed

changelogs/unreleased/sh-fix-issue-38646.yml

----
-title: Fix pushes to an empty repository not invalidating has_visible_content? cache
-merge_request:
-author:
-type: fixed

changelogs/unreleased/fix-mr-sidebar-counter-after-merge.yml → changelogs/unreleased/sh-fix-username-logging.yml

 ---
-title: Fix merge request counter updates after merge
+title: Fix username and ID not logging in production_json.log for Git activity
 merge_request:
 author:
 type: fixed

changelogs/unreleased/sh-restore-all-refs-backup.yml

----
-title: Ensure all refs are restored on a restore from backup
-merge_request:
-author:
-type: fixed

changelogs/unreleased/zj-repo-gitaly.yml

----
-title: Gitaly RepositoryExists remains opt-in for all method calls
-merge_request:
-author:
-type: fixed

config/initializers/devise.rb

@@ -175,7 +175,7 @@ Devise.setup do |config|
 
   # Configure the default scope given to Warden. By default it's the first
   # devise role declared in your routes (usually :user).
-  # config.default_scope = :user
+  config.default_scope = :user  # now have an :email scope as well, so set the default
 
   # Configure sign_out behavior.
   # Sign_out action can be scoped (i.e. /users/sign_out affects only :user scope).

config/routes/profile.rb

+# for secondary email confirmations - uses the same confirmation controller as :users
+devise_for :emails, path: 'profile/emails', controllers: { confirmations: :confirmations }
+
 resource :profile, only: [:show, :update] do
   member do
     get :audit_log
@@ -28,7 +31,11 @@ resource :profile, only: [:show, :update] do
         put :revoke
       end
     end
-    resources :emails, only: [:index, :create, :destroy]
+    resources :emails, only: [:index, :create, :destroy] do
+      member do
+        put :resend_confirmation_instructions
+      end
+    end
     resources :chat_names, only: [:index, :new, :create, :destroy] do
       collection do
         delete :deny

db/migrate/20170904092148_add_email_confirmation.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddEmailConfirmation < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
+  # "add_column_with_default" you must disable the use of transactions
+  # as these methods can not run in an existing transaction.
+  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
+  # that either of them is the _only_ method called in the migration,
+  # any other changes should go in a separate migration.
+  # This ensures that upon failure _only_ the index creation or removing fails
+  # and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  def change
+    add_column :emails, :confirmation_token, :string
+    add_column :emails, :confirmed_at, :datetime_with_timezone
+    add_column :emails, :confirmation_sent_at, :datetime_with_timezone
+  end
+end

db/migrate/20170909090114_add_email_confirmation_index.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddEmailConfirmationIndex < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
+  # "add_column_with_default" you must disable the use of transactions
+  # as these methods can not run in an existing transaction.
+  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
+  # that either of them is the _only_ method called in the migration,
+  # any other changes should go in a separate migration.
+  # This ensures that upon failure _only_ the index creation or removing fails
+  # and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  disable_ddl_transaction!
+
+  # Not necessary to remove duplicates, as :confirmation_token is a new column
+  def up
+    add_concurrent_index :emails, :confirmation_token, unique: true
+  end
+
+  def down
+    remove_concurrent_index :emails, :confirmation_token if index_exists?(:emails, :confirmation_token)
+  end
+end

db/schema.rb

@@ -514,8 +514,12 @@ ActiveRecord::Schema.define(version: 20171004121444) do
     t.string "email", null: false
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.string "confirmation_token"
+    t.datetime "confirmed_at"
+    t.datetime "confirmation_sent_at"
   end
 
+  add_index "emails", ["confirmation_token"], name: "index_emails_on_confirmation_token", unique: true, using: :btree
   add_index "emails", ["email"], name: "index_emails_on_email", unique: true, using: :btree
   add_index "emails", ["user_id"], name: "index_emails_on_user_id", using: :btree
 

doc/administration/job_artifacts.md

@@ -142,9 +142,9 @@ and [projects APIs](../api/projects.md).
 ## Implementation details
 
 When GitLab receives an artifacts archive, an archive metadata file is also
-generated. This metadata file describes all the entries that are located in the
-artifacts archive itself. The metadata file is in a binary format, with
-additional GZIP compression.
+generated by [GitLab Workhorse]. This metadata file describes all the entries
+that are located in the artifacts archive itself.
+The metadata file is in a binary format, with additional GZIP compression.
 
 GitLab does not extract the artifacts archive in order to save space, memory
 and disk I/O. It instead inspects the metadata file which contains all the

doc/user/project/import/github.md

@@ -42,6 +42,11 @@ The importer will create any new namespaces (groups) if they don't exist or in
 the case the namespace is taken, the repository will be imported under the user's
 namespace that started the import process.
 
+The importer will also import branches on forks of projects related to open pull
+requests. These branches will be imported with a naming scheume similar to 
+GH-SHA-Username/Pull-Request-number/fork-name/branch. This may lead to a discrepency
+in branches compared to the GitHub Repository.
+
 ## Importing your GitHub repositories
 
 The importer page is visible when you create a new project.

doc/user/project/repository/gpg_signed_commits/index.md

@@ -26,7 +26,7 @@ to be uploaded to GitLab. For a signature to be verified three conditions need
 to be met:
 
 1. The public key needs to be added your GitLab account
-1. One of the emails in the GPG key matches your **primary** email
+1. One of the emails in the GPG key matches a **verified** email address you use in GitLab
 1. The committer's email matches the verified email from the gpg key
 
 ## Generating a GPG key
@@ -94,7 +94,7 @@ started:
     ```
 
 1. Enter you real name, the email address to be associated with this key (should
-   match the primary email address you use in GitLab) and an optional comment
+   match a verified email address you use in GitLab) and an optional comment
    (press <kbd>Enter</kbd> to skip):
 
     ```

lib/api/branches.rb

@@ -13,7 +13,7 @@ module API
     end
     resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
       desc 'Get a project repository branches' do
-        success Entities::RepoBranch
+        success Entities::Branch
       end
       params do
         use :pagination
@@ -23,13 +23,13 @@ module API
 
         # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37442
         Gitlab::GitalyClient.allow_n_plus_1_calls do
-          present paginate(branches), with: Entities::RepoBranch, project: user_project
+          present paginate(branches), with: Entities::Branch, project: user_project
         end
       end
 
       resource ':id/repository/branches/:branch', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
         desc 'Get a single branch' do
-          success Entities::RepoBranch
+          success Entities::Branch
         end
         params do
           requires :branch, type: String, desc: 'The name of the branch'
@@ -41,7 +41,7 @@ module API
           branch = user_project.repository.find_branch(params[:branch])
           not_found!('Branch') unless branch
 
-          present branch, with: Entities::RepoBranch, project: user_project
+          present branch, with: Entities::Branch, project: user_project
         end
       end
 
@@ -50,7 +50,7 @@ module API
       # in `gitlab-org/gitlab-ce!5081`. The API interface has not been changed (to maintain compatibility),
       # but it works with the changed data model to infer `developers_can_merge` and `developers_can_push`.
       desc 'Protect a single branch' do
-        success Entities::RepoBranch
+        success Entities::Branch
       end
       params do
         requires :branch, type: String, desc: 'The name of the branch'
@@ -80,7 +80,7 @@ module API
                            end
 
         if protected_branch.valid?
-          present branch, with: Entities::RepoBranch, project: user_project
+          present branch, with: Entities::Branch, project: user_project
         else
           render_api_error!(protected_branch.errors.full_messages, 422)
         end
@@ -88,7 +88,7 @@ module API
 
       # Note: This API will be deprecated in favor of the protected branches API.
       desc 'Unprotect a single branch' do
-        success Entities::RepoBranch
+        success Entities::Branch
       end
       params do
         requires :branch, type: String, desc: 'The name of the branch'
@@ -101,11 +101,11 @@ module API
         protected_branch = user_project.protected_branches.find_by(name: branch.name)
         protected_branch&.destroy
 
-        present branch, with: Entities::RepoBranch, project: user_project
+        present branch, with: Entities::Branch, project: user_project
       end
 
       desc 'Create branch' do
-        success Entities::RepoBranch
+        success Entities::Branch
       end
       params do
         requires :branch, type: String, desc: 'The name of the branch'
@@ -119,7 +119,7 @@ module API
 
         if result[:status] == :success
           present result[:branch],
-                  with: Entities::RepoBranch,
+                  with: Entities::Branch,
                   project: user_project
         else
           render_api_error!(result[:message], 400)

lib/api/commits.rb

@@ -13,7 +13,7 @@ module API
     end
     resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
       desc 'Get a project repository commits' do
-        success Entities::RepoCommit
+        success Entities::Commit
       end
       params do
         optional :ref_name, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used'
@@ -46,11 +46,11 @@ module API
 
         paginated_commits = Kaminari.paginate_array(commits, total_count: commit_count)
 
-        present paginate(paginated_commits), with: Entities::RepoCommit
+        present paginate(paginated_commits), with: Entities::Commit
       end
 
       desc 'Commit multiple file changes as one commit' do
-        success Entities::RepoCommitDetail
+        success Entities::CommitDetail
         detail 'This feature was introduced in GitLab 8.13'
       end
       params do
@@ -72,14 +72,14 @@ module API
 
         if result[:status] == :success
           commit_detail = user_project.repository.commit(result[:result])
-          present commit_detail, with: Entities::RepoCommitDetail
+          present commit_detail, with: Entities::CommitDetail
         else
           render_api_error!(result[:message], 400)
         end
       end
 
       desc 'Get a specific commit of a project' do
-        success Entities::RepoCommitDetail
+        success Entities::CommitDetail
         failure [[404, 'Commit Not Found']]
       end
       params do
@@ -90,7 +90,7 @@ module API
 
         not_found! 'Commit' unless commit
 
-        present commit, with: Entities::RepoCommitDetail
+        present commit, with: Entities::CommitDetail
       end
 
       desc 'Get the diff for a specific commit of a project' do
@@ -104,7 +104,7 @@ module API
 
         not_found! 'Commit' unless commit
 
-        present commit.raw_diffs.to_a, with: Entities::RepoDiff
+        present commit.raw_diffs.to_a, with: Entities::Diff
       end
 
       desc "Get a commit's comments" do
@@ -126,7 +126,7 @@ module API
 
       desc 'Cherry pick commit into a branch' do
         detail 'This feature was introduced in GitLab 8.15'
-        success Entities::RepoCommit
+        success Entities::Commit
       end
       params do
         requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag to be cherry picked'
@@ -151,7 +151,7 @@ module API
 
         if result[:status] == :success
           branch = user_project.repository.find_branch(params[:branch])
-          present user_project.repository.commit(branch.dereferenced_target), with: Entities::RepoCommit
+          present user_project.repository.commit(branch.dereferenced_target), with: Entities::Commit
         else
           render_api_error!(result[:message], 400)
         end

lib/api/entities.rb

@@ -220,7 +220,7 @@ module API
       expose :shared_projects, using: Entities::Project
     end
 
-    class RepoCommit < Grape::Entity
+    class Commit < Grape::Entity
       expose :id, :short_id, :title, :created_at
       expose :parent_ids
       expose :safe_message, as: :message
@@ -228,20 +228,20 @@ module API
       expose :committer_name, :committer_email, :committed_date
     end
 
-    class RepoCommitStats < Grape::Entity
+    class CommitStats < Grape::Entity
       expose :additions, :deletions, :total
     end
 
-    class RepoCommitDetail < RepoCommit
-      expose :stats, using: Entities::RepoCommitStats
+    class CommitDetail < Commit
+      expose :stats, using: Entities::CommitStats
       expose :status
       expose :last_pipeline, using: 'API::Entities::PipelineBasic'
     end
 
-    class RepoBranch < Grape::Entity
+    class Branch < Grape::Entity
       expose :name
 
-      expose :commit, using: Entities::RepoCommit do |repo_branch, options|
+      expose :commit, using: Entities::Commit do |repo_branch, options|
         options[:project].repository.commit(repo_branch.dereferenced_target)
       end
 
@@ -265,7 +265,7 @@ module API
       end
     end
 
-    class RepoTreeObject < Grape::Entity
+    class TreeObject < Grape::Entity
       expose :id, :name, :type, :path
 
       expose :mode do |obj, options|
@@ -305,7 +305,7 @@ module API
       expose :state, :created_at, :updated_at
     end
 
-    class RepoDiff < Grape::Entity
+    class Diff < Grape::Entity
       expose :old_path, :new_path, :a_mode, :b_mode
       expose :new_file?, as: :new_file
       expose :renamed_file?, as: :renamed_file
@@ -483,7 +483,7 @@ module API
     end
 
     class MergeRequestChanges < MergeRequest
-      expose :diffs, as: :changes, using: Entities::RepoDiff do |compare, _|
+      expose :diffs, as: :changes, using: Entities::Diff do |compare, _|
         compare.raw_diffs(limits: false).to_a
       end
     end
@@ -494,9 +494,9 @@ module API
     end
 
     class MergeRequestDiffFull < MergeRequestDiff
-      expose :commits, using: Entities::RepoCommit
+      expose :commits, using: Entities::Commit
 
-      expose :diffs, using: Entities::RepoDiff do |compare, _|
+      expose :diffs, using: Entities::Diff do |compare, _|
         compare.raw_diffs(limits: false).to_a
       end
     end
@@ -592,8 +592,7 @@ module API
       expose :target_type
 
       expose :target do |todo, options|
-        target = todo.target_type == 'Commit' ? 'RepoCommit' : todo.target_type
-        Entities.const_get(target).represent(todo.target, options)
+        Entities.const_get(todo.target_type).represent(todo.target, options)
       end
 
       expose :target_url do |todo, options|
@@ -729,15 +728,15 @@ module API
     end
 
     class Compare < Grape::Entity
-      expose :commit, using: Entities::RepoCommit do |compare, options|
-        Commit.decorate(compare.commits, nil).last
+      expose :commit, using: Entities::Commit do |compare, options|
+        ::Commit.decorate(compare.commits, nil).last
       end
 
-      expose :commits, using: Entities::RepoCommit do |compare, options|
-        Commit.decorate(compare.commits, nil)
+      expose :commits, using: Entities::Commit do |compare, options|
+        ::Commit.decorate(compare.commits, nil)
       end
 
-      expose :diffs, using: Entities::RepoDiff do |compare, options|
+      expose :diffs, using: Entities::Diff do |compare, options|
         compare.diffs(limits: false).to_a
       end
 
@@ -773,10 +772,10 @@ module API
       expose :description
     end
 
-    class RepoTag < Grape::Entity
+    class Tag < Grape::Entity
       expose :name, :message
 
-      expose :commit, using: Entities::RepoCommit do |repo_tag, options|
+      expose :commit, using: Entities::Commit do |repo_tag, options|
         options[:project].repository.commit(repo_tag.dereferenced_target)
       end
 
@@ -827,7 +826,7 @@ module API
       expose :created_at, :started_at, :finished_at
       expose :user, with: User
       expose :artifacts_file, using: JobArtifactFile, if: -> (job, opts) { job.artifacts? }
-      expose :commit, with: RepoCommit
+      expose :commit, with: Commit
       expose :runner, with: Runner
       expose :pipeline, with: PipelineBasic
     end
@@ -880,7 +879,7 @@ module API
       expose :deployable,  using: Entities::Job
     end
 
-    class RepoLicense < Grape::Entity
+    class License < Grape::Entity
       expose :key, :name, :nickname
       expose :featured, as: :popular
       expose :url, as: :html_url

lib/api/merge_requests.rb

@@ -183,13 +183,13 @@ module API
       end
 
       desc 'Get the commits of a merge request' do
-        success Entities::RepoCommit
+        success Entities::Commit
       end
       get ':id/merge_requests/:merge_request_iid/commits' do
         merge_request = find_merge_request_with_access(params[:merge_request_iid])
         commits = ::Kaminari.paginate_array(merge_request.commits)
 
-        present paginate(commits), with: Entities::RepoCommit
+        present paginate(commits), with: Entities::Commit
       end
 
       desc 'Show the merge request changes' do

lib/api/repositories.rb

@@ -35,7 +35,7 @@ module API
       end
 
       desc 'Get a project repository tree' do
-        success Entities::RepoTreeObject
+        success Entities::TreeObject
       end
       params do
         optional :ref, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used'
@@ -52,7 +52,7 @@ module API
 
         tree = user_project.repository.tree(commit.id, path, recursive: params[:recursive])
         entries = ::Kaminari.paginate_array(tree.sorted_entries)
-        present paginate(entries), with: Entities::RepoTreeObject
+        present paginate(entries), with: Entities::TreeObject
       end
 
       desc 'Get raw blob contents from the repository'

lib/api/tags.rb

@@ -11,18 +11,18 @@ module API
     end
     resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
       desc 'Get a project repository tags' do
-        success Entities::RepoTag
+        success Entities::Tag
       end
       params do
         use :pagination
       end
       get ':id/repository/tags' do
         tags = ::Kaminari.paginate_array(user_project.repository.tags.sort_by(&:name).reverse)
-        present paginate(tags), with: Entities::RepoTag, project: user_project
+        present paginate(tags), with: Entities::Tag, project: user_project
       end
 
       desc 'Get a single repository tag' do
-        success Entities::RepoTag
+        success Entities::Tag
       end
       params do
         requires :tag_name, type: String, desc: 'The name of the tag'
@@ -31,11 +31,11 @@ module API
         tag = user_project.repository.find_tag(params[:tag_name])
         not_found!('Tag') unless tag
 
-        present tag, with: Entities::RepoTag, project: user_project
+        present tag, with: Entities::Tag, project: user_project
       end
 
       desc 'Create a new repository tag' do
-        success Entities::RepoTag
+        success Entities::Tag
       end
       params do
         requires :tag_name,            type: String, desc: 'The name of the tag'
@@ -51,7 +51,7 @@ module API
 
         if result[:status] == :success
           present result[:tag],
-                  with: Entities::RepoTag,
+                  with: Entities::Tag,
                   project: user_project
         else
           render_api_error!(result[:message], 400)

lib/api/templates.rb

@@ -49,7 +49,7 @@ module API
 
     desc 'Get the list of the available license template' do
       detail 'This feature was introduced in GitLab 8.7.'
-      success ::API::Entities::RepoLicense
+      success ::API::Entities::License
     end
     params do
       optional :popular, type: Boolean, desc: 'If passed, returns only popular licenses'
@@ -60,12 +60,12 @@ module API
         featured: declared(params)[:popular].present? ? true : nil
       }
       licences = ::Kaminari.paginate_array(Licensee::License.all(options))
-      present paginate(licences), with: Entities::RepoLicense
+      present paginate(licences), with: Entities::License
     end
 
     desc 'Get the text for a specific license' do
       detail 'This feature was introduced in GitLab 8.7.'
-      success ::API::Entities::RepoLicense
+      success ::API::Entities::License
     end
     params do
       requires :name, type: String, desc: 'The name of the template'
@@ -75,7 +75,7 @@ module API
 
       template = parsed_license_template
 
-      present template, with: ::API::Entities::RepoLicense
+      present template, with: ::API::Entities::License
     end
 
     GLOBAL_TEMPLATE_TYPES.each do |template_type, properties|

lib/api/users.rb

@@ -331,7 +331,6 @@ module API
         email = Emails::CreateService.new(current_user, declared_params(include_missing: false).merge(user: user)).execute
 
         if email.errors.blank?
-          NotificationService.new.new_email(email)
           present email, with: Entities::Email
         else
           render_validation_error!(email)
@@ -369,10 +368,8 @@ module API
         not_found!('Email') unless email
 
         destroy_conditionally!(email) do |email|
-          Emails::DestroyService.new(current_user, user: user, email: email.email).execute
+          Emails::DestroyService.new(current_user, user: user).execute(email)
         end
-
-        user.update_secondary_emails!
       end
 
       desc 'Delete a user. Available only for admins.' do
@@ -677,7 +674,6 @@ module API
         email = Emails::CreateService.new(current_user, declared_params.merge(user: current_user)).execute
 
         if email.errors.blank?
-          NotificationService.new.new_email(email)
           present email, with: Entities::Email
         else
           render_validation_error!(email)
@@ -693,10 +689,8 @@ module API
         not_found!('Email') unless email
 
         destroy_conditionally!(email) do |email|
-          Emails::DestroyService.new(current_user, user: current_user, email: email.email).execute
+          Emails::DestroyService.new(current_user, user: current_user).execute(email)
         end
-
-        current_user.update_secondary_emails!
       end
 
       desc 'Get a list of user activities'

lib/api/v3/branches.rb

@@ -11,12 +11,12 @@ module API
       end
       resource :projects, requirements: { id: %r{[^/]+} } do
         desc 'Get a project repository branches' do
-          success ::API::Entities::RepoBranch
+          success ::API::Entities::Branch
         end
         get ":id/repository/branches" do
           branches = user_project.repository.branches.sort_by(&:name)
 
-          present branches, with: ::API::Entities::RepoBranch, project: user_project
+          present branches, with: ::API::Entities::Branch, project: user_project
         end
 
         desc 'Delete a branch'
@@ -47,7 +47,7 @@ module API
         end
 
         desc 'Create branch' do
-          success ::API::Entities::RepoBranch
+          success ::API::Entities::Branch
         end
         params do
           requires :branch_name, type: String, desc: 'The name of the branch'
@@ -60,7 +60,7 @@ module API
 
           if result[:status] == :success
             present result[:branch],
-              with: ::API::Entities::RepoBranch,
+              with: ::API::Entities::Branch,
               project: user_project
           else
             render_api_error!(result[:message], 400)

lib/api/v3/commits.rb

@@ -13,7 +13,7 @@ module API
       end
       resource :projects, requirements: { id: %r{[^/]+} } do
         desc 'Get a project repository commits' do
-          success ::API::Entities::RepoCommit
+          success ::API::Entities::Commit
         end
         params do
           optional :ref_name, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used'
@@ -34,11 +34,11 @@ module API
                                                     after: params[:since],
                                                     before: params[:until])
 
-          present commits, with: ::API::Entities::RepoCommit
+          present commits, with: ::API::Entities::Commit
         end
 
         desc 'Commit multiple file changes as one commit' do
-          success ::API::Entities::RepoCommitDetail
+          success ::API::Entities::CommitDetail
           detail 'This feature was introduced in GitLab 8.13'
         end
         params do
@@ -59,14 +59,14 @@ module API
 
           if result[:status] == :success
             commit_detail = user_project.repository.commits(result[:result], limit: 1).first
-            present commit_detail, with: ::API::Entities::RepoCommitDetail
+            present commit_detail, with: ::API::Entities::CommitDetail
           else
             render_api_error!(result[:message], 400)
           end
         end
 
         desc 'Get a specific commit of a project' do
-          success ::API::Entities::RepoCommitDetail
+          success ::API::Entities::CommitDetail
           failure [[404, 'Not Found']]
         end
         params do
@@ -77,7 +77,7 @@ module API
 
           not_found! "Commit" unless commit
 
-          present commit, with: ::API::Entities::RepoCommitDetail
+          present commit, with: ::API::Entities::CommitDetail
         end
 
         desc 'Get the diff for a specific commit of a project' do
@@ -113,7 +113,7 @@ module API
 
         desc 'Cherry pick commit into a branch' do
           detail 'This feature was introduced in GitLab 8.15'
-          success ::API::Entities::RepoCommit
+          success ::API::Entities::Commit
         end
         params do
           requires :sha, type: String, desc: 'A commit sha to be cherry picked'
@@ -138,7 +138,7 @@ module API
 
           if result[:status] == :success
             branch = user_project.repository.find_branch(params[:branch])
-            present user_project.repository.commit(branch.dereferenced_target), with: ::API::Entities::RepoCommit
+            present user_project.repository.commit(branch.dereferenced_target), with: ::API::Entities::Commit
           else
             render_api_error!(result[:message], 400)
           end

lib/api/v3/entities.rb

@@ -220,7 +220,7 @@ module API
         expose :created_at, :started_at, :finished_at
         expose :user, with: ::API::Entities::User
         expose :artifacts_file, using: ::API::Entities::JobArtifactFile, if: -> (build, opts) { build.artifacts? }
-        expose :commit, with: ::API::Entities::RepoCommit
+        expose :commit, with: ::API::Entities::Commit
         expose :runner, with: ::API::Entities::Runner
         expose :pipeline, with: ::API::Entities::PipelineBasic
       end
@@ -237,7 +237,7 @@ module API
       end
 
       class MergeRequestChanges < MergeRequest
-        expose :diffs, as: :changes, using: ::API::Entities::RepoDiff do |compare, _|
+        expose :diffs, as: :changes, using: ::API::Entities::Diff do |compare, _|
           compare.raw_diffs(limits: false).to_a
         end
       end

lib/api/v3/merge_requests.rb

@@ -135,12 +135,12 @@ module API
           end
 
           desc 'Get the commits of a merge request' do
-            success ::API::Entities::RepoCommit
+            success ::API::Entities::Commit
           end
           get "#{path}/commits" do
             merge_request = find_merge_request_with_access(params[:merge_request_id])
 
-            present merge_request.commits, with: ::API::Entities::RepoCommit
+            present merge_request.commits, with: ::API::Entities::Commit
           end
 
           desc 'Show the merge request changes' do

lib/api/v3/repositories.rb

@@ -19,7 +19,7 @@ module API
         end
 
         desc 'Get a project repository tree' do
-          success ::API::Entities::RepoTreeObject
+          success ::API::Entities::TreeObject
         end
         params do
           optional :ref_name, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used'
@@ -35,7 +35,7 @@ module API
 
           tree = user_project.repository.tree(commit.id, path, recursive: params[:recursive])
 
-          present tree.sorted_entries, with: ::API::Entities::RepoTreeObject
+          present tree.sorted_entries, with: ::API::Entities::TreeObject
         end
 
         desc 'Get a raw file contents'

lib/api/v3/tags.rb

@@ -8,11 +8,11 @@ module API
       end
       resource :projects, requirements: { id: %r{[^/]+} } do
         desc 'Get a project repository tags' do
-          success ::API::Entities::RepoTag
+          success ::API::Entities::Tag
         end
         get ":id/repository/tags" do
           tags = user_project.repository.tags.sort_by(&:name).reverse
-          present tags, with: ::API::Entities::RepoTag, project: user_project
+          present tags, with: ::API::Entities::Tag, project: user_project
         end
 
         desc 'Delete a repository tag'

lib/api/v3/templates.rb

@@ -52,7 +52,7 @@ module API
           detailed_desc = 'This feature was introduced in GitLab 8.7.'
           detailed_desc << DEPRECATION_MESSAGE unless status == :ok
           detail detailed_desc
-          success ::API::Entities::RepoLicense
+          success ::API::Entities::License
         end
         params do
           optional :popular, type: Boolean, desc: 'If passed, returns only popular licenses'
@@ -61,7 +61,7 @@ module API
           options = {
             featured: declared(params)[:popular].present? ? true : nil
           }
-          present Licensee::License.all(options), with: ::API::Entities::RepoLicense
+          present Licensee::License.all(options), with: ::API::Entities::License
         end
       end
 
@@ -70,7 +70,7 @@ module API
           detailed_desc = 'This feature was introduced in GitLab 8.7.'
           detailed_desc << DEPRECATION_MESSAGE unless status == :ok
           detail detailed_desc
-          success ::API::Entities::RepoLicense
+          success ::API::Entities::License
         end
         params do
           requires :name, type: String, desc: 'The name of the template'
@@ -80,7 +80,7 @@ module API
 
           template = parsed_license_template
 
-          present template, with: ::API::Entities::RepoLicense
+          present template, with: ::API::Entities::License
         end
       end
 

lib/declarative_policy/rule.rb

@@ -206,11 +206,13 @@ module DeclarativePolicy
       end
 
       def cached_pass?(context)
-        passes = @rules.map { |r| r.cached_pass?(context) }
-        return false if passes.any? { |p| p == false }
-        return true if passes.all? { |p| p == true }
+        @rules.each do |rule|
+          pass = rule.cached_pass?(context)
 
-        nil
+          return pass if pass.nil? || pass == false
+        end
+
+        true
       end
 
       def repr
@@ -245,11 +247,13 @@ module DeclarativePolicy
       end
 
       def cached_pass?(context)
-        passes = @rules.map { |r| r.cached_pass?(context) }
-        return true if passes.any? { |p| p == true }
-        return false if passes.all? { |p| p == false }
+        @rules.each do |rule|
+          pass = rule.cached_pass?(context)
 
-        nil
+          return pass if pass.nil? || pass == true
+        end
+
+        false
       end
 
       def score(context)

lib/declarative_policy/runner.rb

@@ -107,7 +107,7 @@ module DeclarativePolicy
     end
 
     # This is the core spot where all those `#score` methods matter.
-    # It is critcal for performance to run steps in the correct order,
+    # It is critical for performance to run steps in the correct order,
     # so that we don't compute expensive conditions (potentially n times
     # if we're called on, say, a large list of users).
     #
@@ -139,30 +139,39 @@ module DeclarativePolicy
         return
       end
 
-      steps = Set.new(@steps)
-      remaining_enablers = steps.count { |s| s.enable? }
+      remaining_steps = Set.new(@steps)
+      remaining_enablers, remaining_preventers = remaining_steps.partition(&:enable?).map { |s| Set.new(s) }
 
       loop do
-        return if steps.empty?
+        if @state.enabled?
+          # Once we set this, we never need to unset it, because a single
+          # prevent will stop this from being enabled
+          remaining_steps = remaining_preventers
+        else
+          # if the permission hasn't yet been enabled and we only have
+          # prevent steps left, we short-circuit the state here
+          @state.prevent! if remaining_enablers.empty?
+        end
 
-        # if the permission hasn't yet been enabled and we only have
-        # prevent steps left, we short-circuit the state here
-        @state.prevent! if !@state.enabled? && remaining_enablers == 0
+        return if remaining_steps.empty?
 
         lowest_score = Float::INFINITY
         next_step = nil
 
-        steps.each do |step|
+        remaining_steps.each do |step|
           score = step.score
+
           if score < lowest_score
             next_step = step
             lowest_score = score
           end
-        end
 
-        steps.delete(next_step)
+          break if lowest_score.zero?
+        end
 
-        remaining_enablers -= 1 if next_step.enable?
+        [remaining_steps, remaining_enablers, remaining_preventers].each do |set|
+          set.delete(next_step)
+        end
 
         yield next_step, lowest_score
       end

lib/gitlab/git/repository.rb

@@ -990,7 +990,7 @@ module Gitlab
         tmp_ref = fetch_ref(
           start_repository,
           source_ref: "#{Gitlab::Git::BRANCH_REF_PREFIX}#{start_branch_name}",
-          target_ref: "refs/tmp/#{SecureRandom.hex}/head"
+          target_ref: "refs/tmp/#{SecureRandom.hex}"
         )
 
         yield commit(sha)

lib/gitlab/o_auth/user.rb

@@ -64,6 +64,8 @@ module Gitlab
       protected
 
       def add_or_update_user_identities
+        return unless gl_user
+
         # find_or_initialize_by doesn't update `gl_user.identities`, and isn't autosaved.
         identity = gl_user.identities.find { |identity| identity.provider == auth_hash.provider }
 

locale/gitlab.pot

@@ -852,9 +852,6 @@ msgstr[1] ""
 msgid "Last Pipeline"
 msgstr ""
 
-msgid "Last Update"
-msgstr ""
-
 msgid "Last commit"
 msgstr ""
 
@@ -864,6 +861,9 @@ msgstr ""
 msgid "Last edited by %{name}"
 msgstr ""
 
+msgid "Last update"
+msgstr ""
+
 msgid "Last updated"
 msgstr ""
 

qa/Gemfile.lock

@@ -25,9 +25,9 @@ GEM
     mime-types (3.1)
       mime-types-data (~> 3.2015)
     mime-types-data (3.2016.0521)
-    mini_portile2 (2.1.0)
-    nokogiri (1.7.0.1)
-      mini_portile2 (~> 2.1.0)
+    mini_portile2 (2.3.0)
+    nokogiri (1.8.1)
+      mini_portile2 (~> 2.3.0)
     pry (0.10.4)
       coderay (~> 1.1.0)
       method_source (~> 0.8.1)
@@ -75,4 +75,4 @@ DEPENDENCIES
   selenium-webdriver (~> 2.53)
 
 BUNDLED WITH
-   1.14.6
+   1.15.4

qa/README.md

-## Integration tests for GitLab
+# GitLab QA - Integration tests for GitLab
 
 This directory contains integration tests for GitLab.
 
-It is part of [GitLab QA project](https://gitlab.com/gitlab-org/gitlab-qa).
+It is part of the [GitLab QA project](https://gitlab.com/gitlab-org/gitlab-qa).
 
-## What GitLab QA is?
+## What is it?
 
 GitLab QA is an integration tests suite for GitLab.
 
@@ -20,18 +20,34 @@ against any existing instance.
 ## How can I use it?
 
 You can use GitLab QA to exercise tests on any live instance! For example, the
-follow call would login to the local GitLab instance and run all specs in
+following call would login to a local [GDK] instance and run all specs in
 `qa/specs/features`:
 
 ```
-GITLAB_USERNAME='root' GITLAB_PASSWORD='5iveL!fe' bin/qa Test::Instance http://localhost
+bin/qa Test::Instance http://localhost:3000
 ```
 
-You can also supply a specific tests to run as another parameter. For example, to
+### Running specific tests
+
+You can also supply specific tests to run as another parameter. For example, to
 test the EE license specs, you can run:
 
 ```
-EE_LICENSE="<YOUR LICENSE KEY>" GITLAB_USERNAME='root' GITLAB_PASSWORD='5iveL!fe' bin/qa Test::Instance http://localhost qa/ee
+EE_LICENSE="<YOUR LICENSE KEY>" bin/qa Test::Instance http://localhost qa/ee
+```
+
+### Overriding the authenticated user
+
+Unless told otherwise, the QA tests will run as the default `root` user seeded
+by the GDK.
+
+If you need to authenticate as a different user, you can provide the
+`GITLAB_USERNAME` and `GITLAB_PASSWORD` environment variables:
+
+```
+GITLAB_USERNAME=jsmith GITLAB_PASSWORD=password bin/qa Test::Instance https://gitlab.example.com
 ```
 
 All [supported environment variables are here](https://gitlab.com/gitlab-org/gitlab-qa#supported-environment-variables).
+
+[GDK]: https://gitlab.com/gitlab-org/gitlab-development-kit/

qa/qa/runtime/user.rb

@@ -8,7 +8,7 @@ module QA
       end
 
       def password
-        ENV['GITLAB_PASSWORD'] || 'test1234'
+        ENV['GITLAB_PASSWORD'] || '5iveL!fe'
       end
     end
   end

spec/controllers/profiles/emails_controller_spec.rb

+require 'spec_helper'
+
+describe Profiles::EmailsController do
+  let(:user) { create(:user) }
+
+  before do
+    sign_in(user)
+  end
+
+  describe '#create' do
+    let(:email_params) { { email: "add_email@example.com" } }
+
+    it 'sends an email confirmation' do
+      expect { post(:create, { email: email_params }) }.to change { ActionMailer::Base.deliveries.size }
+      expect(ActionMailer::Base.deliveries.last.to).to eq [email_params[:email]]
+      expect(ActionMailer::Base.deliveries.last.subject).to match "Confirmation instructions"
+    end
+  end
+
+  describe '#resend_confirmation_instructions' do
+    let(:email_params) { { email: "add_email@example.com" } }
+
+    it 'resends an email confirmation' do
+      email = user.emails.create(email: 'add_email@example.com')
+
+      expect { put(:resend_confirmation_instructions, { id: email }) }.to change { ActionMailer::Base.deliveries.size }
+      expect(ActionMailer::Base.deliveries.last.to).to eq [email_params[:email]]
+      expect(ActionMailer::Base.deliveries.last.subject).to match "Confirmation instructions"
+    end
+
+    it 'unable to resend an email confirmation' do
+      expect { put(:resend_confirmation_instructions, { id: 1 }) }.not_to change { ActionMailer::Base.deliveries.size }
+    end
+  end
+end

spec/controllers/profiles_controller_spec.rb

@@ -15,6 +15,20 @@ describe ProfilesController do
       expect(user.unconfirmed_email).to eq('john@gmail.com')
     end
 
+    it "allows an email update without confirmation if existing verified email" do
+      user = create(:user)
+      create(:email, :confirmed, user: user, email: 'john@gmail.com')
+      sign_in(user)
+
+      put :update,
+          user: { email: "john@gmail.com", name: "John" }
+
+      user.reload
+
+      expect(response.status).to eq(302)
+      expect(user.unconfirmed_email).to eq nil
+    end
+
     it "ignores an email update from a user with an external email address" do
       stub_omniauth_setting(sync_profile_from_provider: ['ldap'])
       stub_omniauth_setting(sync_profile_attributes: true)

spec/factories/emails.rb

@@ -2,5 +2,7 @@ FactoryGirl.define do
   factory :email do
     user
     email { generate(:email_alias) }
+
+    trait(:confirmed) { confirmed_at Time.now }
   end
 end

spec/features/profiles/emails_spec.rb

+require 'rails_helper'
+
+feature 'Profile > Emails' do
+  let(:user) { create(:user) }
+
+  before do
+    sign_in(user)
+  end
+
+  describe 'User adds an email' do
+    before do
+      visit profile_emails_path
+    end
+
+    scenario 'saves the new email' do
+      fill_in('Email', with: 'my@email.com')
+      click_button('Add email address')
+
+      expect(page).to have_content('my@email.com Unverified')
+      expect(page).to have_content("#{user.email} Verified")
+      expect(page).to have_content('Resend confirmation email')
+    end
+
+    scenario 'does not add a duplicate email' do
+      fill_in('Email', with: user.email)
+      click_button('Add email address')
+
+      email = user.emails.find_by(email: user.email)
+      expect(email).to be_nil
+      expect(page).to have_content('Email has already been taken')
+    end
+  end
+
+  scenario 'User removes email' do
+    user.emails.create(email: 'my@email.com')
+    visit profile_emails_path
+    expect(page).to have_content("my@email.com")
+
+    click_link('Remove')
+    expect(page).not_to have_content("my@email.com")
+  end
+
+  scenario 'User confirms email' do
+    email = user.emails.create(email: 'my@email.com')
+    visit profile_emails_path
+    expect(page).to have_content("#{email.email} Unverified")
+
+    email.confirm
+    expect(email.confirmed?).to be_truthy
+
+    visit profile_emails_path
+    expect(page).to have_content("#{email.email} Verified")
+  end
+
+  scenario 'User re-sends confirmation email' do
+    email = user.emails.create(email: 'my@email.com')
+    visit profile_emails_path
+
+    expect { click_link("Resend confirmation email") }.to change { ActionMailer::Base.deliveries.size }
+    expect(page).to have_content("Confirmation email sent to #{email.email}")
+  end
+
+  scenario 'old unconfirmed emails show Send Confirmation button' do
+    email = user.emails.create(email: 'my@email.com')
+    email.update_attribute(:confirmation_sent_at, nil)
+    visit profile_emails_path
+
+    expect(page).not_to have_content('Resend confirmation email')
+    expect(page).to have_content('Send confirmation email')
+  end
+end

spec/features/profiles/gpg_keys_spec.rb

@@ -4,7 +4,7 @@ feature 'Profile > GPG Keys' do
   let(:user) { create(:user, email: GpgHelpers::User2.emails.first) }
 
   before do
-    login_as(user)
+    sign_in(user)
   end
 
   describe 'User adds a key' do

spec/javascripts/monitoring/graph/deployment_spec.js

@@ -21,6 +21,7 @@ describe('MonitoringDeployment', () => {
       const component = createComponent({
         showDeployInfo: false,
         deploymentData: reducedDeploymentData,
+        graphWidth: 440,
         graphHeight: 300,
         graphHeightOffset: 120,
       });
@@ -36,6 +37,7 @@ describe('MonitoringDeployment', () => {
         showDeployInfo: false,
         deploymentData: reducedDeploymentData,
         graphHeight: 300,
+        graphWidth: 440,
         graphHeightOffset: 120,
       });
 
@@ -49,6 +51,7 @@ describe('MonitoringDeployment', () => {
         showDeployInfo: false,
         deploymentData: reducedDeploymentData,
         graphHeight: 300,
+        graphWidth: 440,
         graphHeightOffset: 120,
       });
 
@@ -62,6 +65,7 @@ describe('MonitoringDeployment', () => {
         showDeployInfo: false,
         deploymentData: reducedDeploymentData,
         graphHeight: 300,
+        graphWidth: 440,
         graphHeightOffset: 120,
       });
 
@@ -75,6 +79,7 @@ describe('MonitoringDeployment', () => {
       const component = createComponent({
         showDeployInfo: true,
         deploymentData: reducedDeploymentData,
+        graphWidth: 440,
         graphHeight: 300,
         graphHeightOffset: 120,
       });
@@ -82,12 +87,29 @@ describe('MonitoringDeployment', () => {
       expect(component.$el.querySelector('.js-deploy-info-box')).toBeNull();
     });
 
+    it('positions the flag to the left when the xPos is too far right', () => {
+      reducedDeploymentData[0].showDeploymentFlag = false;
+      reducedDeploymentData[0].xPos = 250;
+      const component = createComponent({
+        showDeployInfo: true,
+        deploymentData: reducedDeploymentData,
+        graphWidth: 440,
+        graphHeight: 300,
+        graphHeightOffset: 120,
+      });
+
+      expect(
+        component.positionFlag(reducedDeploymentData[0]),
+      ).toBeLessThan(0);
+    });
+
     it('shows the deployment flag', () => {
       reducedDeploymentData[0].showDeploymentFlag = true;
       const component = createComponent({
         showDeployInfo: true,
         deploymentData: reducedDeploymentData,
         graphHeight: 300,
+        graphWidth: 440,
         graphHeightOffset: 120,
       });
 
@@ -102,6 +124,7 @@ describe('MonitoringDeployment', () => {
         showDeployInfo: true,
         deploymentData: reducedDeploymentData,
         graphHeight: 300,
+        graphWidth: 440,
         graphHeightOffset: 120,
       });
 
@@ -115,6 +138,7 @@ describe('MonitoringDeployment', () => {
         showDeployInfo: true,
         deploymentData: reducedDeploymentData,
         graphHeight: 300,
+        graphWidth: 440,
         graphHeightOffset: 120,
       });
 
@@ -127,6 +151,7 @@ describe('MonitoringDeployment', () => {
           showDeployInfo: true,
           deploymentData: reducedDeploymentData,
           graphHeight: 300,
+          graphWidth: 440,
           graphHeightOffset: 120,
         });
 

spec/javascripts/monitoring/graph/flag_spec.js

@@ -14,19 +14,22 @@ function getCoordinate(component, selector, coordinate) {
   return parseInt(coordinateVal, 10);
 }
 
+const defaultValuesComponent = {
+  currentXCoordinate: 200,
+  currentYCoordinate: 100,
+  currentFlagPosition: 100,
+  currentData: {
+    time: new Date('2017-06-04T18:17:33.501Z'),
+    value: '1.49609375',
+  },
+  graphHeight: 300,
+  graphHeightOffset: 120,
+  showFlagContent: true,
+};
+
 describe('GraphFlag', () => {
   it('has a line and a circle located at the currentXCoordinate and currentYCoordinate', () => {
-    const component = createComponent({
-      currentXCoordinate: 200,
-      currentYCoordinate: 100,
-      currentFlagPosition: 100,
-      currentData: {
-        time: new Date('2017-06-04T18:17:33.501Z'),
-        value: '1.49609375',
-      },
-      graphHeight: 300,
-      graphHeightOffset: 120,
-    });
+    const component = createComponent(defaultValuesComponent);
 
     expect(getCoordinate(component, '.selected-metric-line', 'x1'))
       .toEqual(component.currentXCoordinate);
@@ -35,17 +38,7 @@ describe('GraphFlag', () => {
   });
 
   it('has a SVG with the class rect-text-metric at the currentFlagPosition', () => {
-    const component = createComponent({
-      currentXCoordinate: 200,
-      currentYCoordinate: 100,
-      currentFlagPosition: 100,
-      currentData: {
-        time: new Date('2017-06-04T18:17:33.501Z'),
-        value: '1.49609375',
-      },
-      graphHeight: 300,
-      graphHeightOffset: 120,
-    });
+    const component = createComponent(defaultValuesComponent);
 
     const svg = component.$el.querySelector('.rect-text-metric');
     expect(svg.tagName).toEqual('svg');
@@ -54,17 +47,7 @@ describe('GraphFlag', () => {
 
   describe('Computed props', () => {
     it('calculatedHeight', () => {
-      const component = createComponent({
-        currentXCoordinate: 200,
-        currentYCoordinate: 100,
-        currentFlagPosition: 100,
-        currentData: {
-          time: new Date('2017-06-04T18:17:33.501Z'),
-          value: '1.49609375',
-        },
-        graphHeight: 300,
-        graphHeightOffset: 120,
-      });
+      const component = createComponent(defaultValuesComponent);
 
       expect(component.calculatedHeight).toEqual(180);
     });

spec/javascripts/monitoring/graph_spec.js

@@ -86,4 +86,22 @@ describe('Graph', () => {
     expect(component.yAxisLabel).toEqual(component.graphData.y_label);
     expect(component.legendTitle).toEqual(component.graphData.queries[0].label);
   });
+
+  it('sets the currentData object based on the hovered data index', () => {
+    const component = createComponent({
+      graphData: convertedMetrics[1],
+      classType: 'col-md-6',
+      updateAspectRatio: false,
+      deploymentData,
+      graphIdentifier: 0,
+      hoverData: {
+        hoveredDate: new Date('Sun Aug 27 2017 06:11:51 GMT-0500 (CDT)'),
+        currentDeployXPos: null,
+      },
+    });
+
+    component.positionFlag();
+    expect(component.currentData).toBe(component.timeSeries[0].values[10]);
+    expect(component.currentDataIndex).toEqual(10);
+  });
 });

spec/javascripts/repo/components/repo_sidebar_spec.js

@@ -23,8 +23,8 @@ describe('RepoSidebar', () => {
     expect(vm.$el.id).toEqual('sidebar');
     expect(vm.$el.classList.contains('sidebar-mini')).toBeFalsy();
     expect(thead.querySelector('.name').textContent).toEqual('Name');
-    expect(thead.querySelector('.last-commit').textContent).toEqual('Last Commit');
-    expect(thead.querySelector('.last-update').textContent).toEqual('Last Update');
+    expect(thead.querySelector('.last-commit').textContent).toEqual('Last commit');
+    expect(thead.querySelector('.last-update').textContent).toEqual('Last update');
     expect(tbody.querySelector('.repo-file-options')).toBeFalsy();
     expect(tbody.querySelector('.prev-directory')).toBeFalsy();
     expect(tbody.querySelector('.loading-file')).toBeFalsy();

spec/mailers/emails/profile_spec.rb

@@ -120,29 +120,4 @@ describe Emails::Profile do
       it { expect { Notify.new_gpg_key_email('foo') }.not_to raise_error }
     end
   end
-
-  describe 'user added email' do
-    let(:email) { create(:email) }
-
-    subject { Notify.new_email_email(email.id) }
-
-    it_behaves_like 'it should not have Gmail Actions links'
-    it_behaves_like 'a user cannot unsubscribe through footer link'
-
-    it 'is sent to the new user' do
-      is_expected.to deliver_to email.user.email
-    end
-
-    it 'has the correct subject' do
-      is_expected.to have_subject /^Email was added to your account$/i
-    end
-
-    it 'contains the new email address' do
-      is_expected.to have_body_text /#{email.email}/
-    end
-
-    it 'includes a link to emails page' do
-      is_expected.to have_body_text /#{profile_emails_path}/
-    end
-  end
 end

spec/models/email_spec.rb

@@ -11,4 +11,33 @@ describe Email do
     expect(described_class.new(email: ' inFO@exAMPLe.com ').email)
       .to eq 'info@example.com'
   end
+
+  describe '#update_invalid_gpg_signatures' do
+    let(:user) do
+      create(:user, email: 'tula.torphy@abshire.ca').tap do |user|
+        user.skip_reconfirmation!
+      end
+    end
+    let(:user) { create(:user) }
+
+    it 'synchronizes the gpg keys when the email is updated' do
+      email = user.emails.create(email: 'new@email.com')
+
+      expect(user).to receive(:update_invalid_gpg_signatures)
+
+      email.confirm
+    end
+  end
+
+  describe 'scopes' do
+    let(:user) { create(:user) }
+
+    it 'scopes confirmed emails' do
+      create(:email, :confirmed, user: user)
+      create(:email, user: user)
+
+      expect(user.emails.count).to eq 2
+      expect(user.emails.confirmed.count).to eq 1
+    end
+  end
 end

spec/models/gpg_key_spec.rb

@@ -90,11 +90,20 @@ describe GpgKey do
     it 'email is verified if the user has the matching email' do
       user = create :user, email: 'bette.cartwright@example.com'
       gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user
+      create :email, user: user
+      user.reload
 
       expect(gpg_key.emails_with_verified_status).to eq(
         'bette.cartwright@example.com' => true,
         'bette.cartwright@example.net' => false
       )
+
+      create :email, :confirmed, user: user, email: 'bette.cartwright@example.net'
+      user.reload
+      expect(gpg_key.emails_with_verified_status).to eq(
+        'bette.cartwright@example.com' => true,
+        'bette.cartwright@example.net' => true
+      )
     end
   end
 

spec/models/user_spec.rb

@@ -360,9 +360,22 @@ describe User do
         expect(external_user.projects_limit).to be 0
       end
     end
+
+    describe '#check_for_verified_email' do
+      let(:user)      { create(:user) }
+      let(:secondary) { create(:email, :confirmed, email: 'secondary@example.com', user: user) }
+
+      it 'allows a verfied secondary email to be used as the primary without needing reconfirmation' do
+        user.update_attributes!(email: secondary.email)
+        user.reload
+        expect(user.email).to eq secondary.email
+        expect(user.unconfirmed_email).to eq nil
+        expect(user.confirmed?).to be_truthy
+      end
+    end
   end
 
-  describe 'after update hook' do
+  describe 'after commit hook' do
     describe '.update_invalid_gpg_signatures' do
       let(:user) do
         create(:user, email: 'tula.torphy@abshire.ca').tap do |user|
@@ -376,10 +389,50 @@ describe User do
       end
 
       it 'synchronizes the gpg keys when the email is updated' do
-        expect(user).to receive(:update_invalid_gpg_signatures)
+        expect(user).to receive(:update_invalid_gpg_signatures).at_most(:twice)
         user.update_attributes!(email: 'shawnee.ritchie@denesik.com')
       end
     end
+
+    describe '#update_emails_with_primary_email' do
+      before do
+        @user = create(:user, email: 'primary@example.com').tap do |user|
+          user.skip_reconfirmation!
+        end
+        @secondary = create :email, email: 'secondary@example.com', user: @user
+        @user.reload
+      end
+
+      it 'gets called when email updated' do
+        expect(@user).to receive(:update_emails_with_primary_email)
+
+        @user.update_attributes!(email: 'new_primary@example.com')
+      end
+
+      it 'adds old primary to secondary emails when secondary is a new email ' do
+        @user.update_attributes!(email: 'new_primary@example.com')
+        @user.reload
+
+        expect(@user.emails.count).to eq 2
+        expect(@user.emails.pluck(:email)).to match_array([@secondary.email, 'primary@example.com'])
+      end
+
+      it 'adds old primary to secondary emails if secondary is becoming a primary' do
+        @user.update_attributes!(email: @secondary.email)
+        @user.reload
+
+        expect(@user.emails.count).to eq 1
+        expect(@user.emails.first.email).to eq 'primary@example.com'
+      end
+
+      it 'transfers old confirmation values into new secondary' do
+        @user.update_attributes!(email: @secondary.email)
+        @user.reload
+
+        expect(@user.emails.count).to eq 1
+        expect(@user.emails.first.confirmed_at).not_to eq nil
+      end
+    end
   end
 
   describe '#update_tracked_fields!', :clean_gitlab_redis_shared_state do
@@ -467,6 +520,7 @@ describe User do
   describe '#generate_password' do
     it "does not generate password by default" do
       user = create(:user, password: 'abcdefghe')
+
       expect(user.password).to eq('abcdefghe')
     end
   end
@@ -474,6 +528,7 @@ describe User do
   describe 'authentication token' do
     it "has authentication token" do
       user = create(:user)
+
       expect(user.authentication_token).not_to be_blank
     end
   end
@@ -481,6 +536,7 @@ describe User do
   describe 'ensure incoming email token' do
     it 'has incoming email token' do
       user = create(:user)
+
       expect(user.incoming_email_token).not_to be_blank
     end
   end
@@ -523,6 +579,7 @@ describe User do
     it 'ensures an rss token on read' do
       user = create(:user, rss_token: nil)
       rss_token = user.rss_token
+
       expect(rss_token).not_to be_blank
       expect(user.reload.rss_token).to eq rss_token
     end
@@ -633,6 +690,7 @@ describe User do
 
     it "blocks user" do
       user.block
+
       expect(user.blocked?).to be_truthy
     end
   end
@@ -966,6 +1024,7 @@ describe User do
 
     it 'is case-insensitive' do
       user = create(:user, username: 'JohnDoe')
+
       expect(described_class.find_by_username('JOHNDOE')).to eq user
     end
   end
@@ -978,6 +1037,7 @@ describe User do
 
     it 'is case-insensitive' do
       user = create(:user, username: 'JohnDoe')
+
       expect(described_class.find_by_username!('JOHNDOE')).to eq user
     end
   end
@@ -1067,11 +1127,13 @@ describe User do
 
     it 'is true if avatar is image' do
       user.update_attribute(:avatar, 'uploads/avatar.png')
+
       expect(user.avatar_type).to be_truthy
     end
 
     it 'is false if avatar is html page' do
       user.update_attribute(:avatar, 'uploads/avatar.html')
+
       expect(user.avatar_type).to eq(['only images allowed'])
     end
   end
@@ -1094,6 +1156,50 @@ describe User do
     end
   end
 
+  describe '#all_emails' do
+    let(:user) { create(:user) }
+
+    it 'returns all emails' do
+      email_confirmed   = create :email, user: user, confirmed_at: Time.now
+      email_unconfirmed = create :email, user: user
+      user.reload
+
+      expect(user.all_emails).to match_array([user.email, email_unconfirmed.email, email_confirmed.email])
+    end
+  end
+
+  describe '#verified_emails' do
+    let(:user) { create(:user) }
+
+    it 'returns only confirmed emails' do
+      email_confirmed = create :email, user: user, confirmed_at: Time.now
+      create :email, user: user
+      user.reload
+
+      expect(user.verified_emails).to match_array([user.email, email_confirmed.email])
+    end
+  end
+
+  describe '#verified_email?' do
+    let(:user) { create(:user) }
+
+    it 'returns true when the email is verified/confirmed' do
+      email_confirmed = create :email, user: user, confirmed_at: Time.now
+      create :email, user: user
+      user.reload
+
+      expect(user.verified_email?(user.email)).to be_truthy
+      expect(user.verified_email?(email_confirmed.email.titlecase)).to be_truthy
+    end
+
+    it 'returns false when the email is not verified/confirmed' do
+      email_unconfirmed = create :email, user: user
+      user.reload
+
+      expect(user.verified_email?(email_unconfirmed.email)).to be_falsy
+    end
+  end
+
   describe '#requires_ldap_check?' do
     let(:user) { described_class.new }
 
@@ -1101,6 +1207,7 @@ describe User do
       # Create a condition which would otherwise cause 'true' to be returned
       allow(user).to receive(:ldap_user?).and_return(true)
       user.last_credential_check_at = nil
+
       expect(user.requires_ldap_check?).to be_falsey
     end
 
@@ -1111,6 +1218,7 @@ describe User do
 
       it 'is false for non-LDAP users' do
         allow(user).to receive(:ldap_user?).and_return(false)
+
         expect(user.requires_ldap_check?).to be_falsey
       end
 
@@ -1121,11 +1229,13 @@ describe User do
 
         it 'is true when the user has never had an LDAP check before' do
           user.last_credential_check_at = nil
+
           expect(user.requires_ldap_check?).to be_truthy
         end
 
         it 'is true when the last LDAP check happened over 1 hour ago' do
           user.last_credential_check_at = 2.hours.ago
+
           expect(user.requires_ldap_check?).to be_truthy
         end
       end
@@ -1136,16 +1246,19 @@ describe User do
     describe '#ldap_user?' do
       it 'is true if provider name starts with ldap' do
         user = create(:omniauth_user, provider: 'ldapmain')
+
         expect(user.ldap_user?).to be_truthy
       end
 
       it 'is false for other providers' do
         user = create(:omniauth_user, provider: 'other-provider')
+
         expect(user.ldap_user?).to be_falsey
       end
 
       it 'is false if no extern_uid is provided' do
         user = create(:omniauth_user, extern_uid: nil)
+
         expect(user.ldap_user?).to be_falsey
       end
     end
@@ -1153,6 +1266,7 @@ describe User do
     describe '#ldap_identity' do
       it 'returns ldap identity' do
         user = create :omniauth_user
+
         expect(user.ldap_identity.provider).not_to be_empty
       end
     end
@@ -1162,6 +1276,7 @@ describe User do
 
       it 'blocks user flaging the action caming from ldap' do
         user.ldap_block
+
         expect(user.blocked?).to be_truthy
         expect(user.ldap_blocked?).to be_truthy
       end
@@ -1234,18 +1349,22 @@ describe User do
       expect(user.starred?(project2)).to be_falsey
 
       star1 = UsersStarProject.create!(project: project1, user: user)
+
       expect(user.starred?(project1)).to be_truthy
       expect(user.starred?(project2)).to be_falsey
 
       star2 = UsersStarProject.create!(project: project2, user: user)
+
       expect(user.starred?(project1)).to be_truthy
       expect(user.starred?(project2)).to be_truthy
 
       star1.destroy
+
       expect(user.starred?(project1)).to be_falsey
       expect(user.starred?(project2)).to be_truthy
 
       star2.destroy
+
       expect(user.starred?(project1)).to be_falsey
       expect(user.starred?(project2)).to be_falsey
     end
@@ -1257,9 +1376,13 @@ describe User do
       project = create(:project, :public)
 
       expect(user.starred?(project)).to be_falsey
+
       user.toggle_star(project)
+
       expect(user.starred?(project)).to be_truthy
+
       user.toggle_star(project)
+
       expect(user.starred?(project)).to be_falsey
     end
   end
@@ -1438,9 +1561,11 @@ describe User do
       user    = create(:user)
 
       member = group.add_developer(user)
+
       expect(user.authorized_projects).to include(project)
 
       member.destroy
+
       expect(user.authorized_projects).not_to include(project)
     end
 
@@ -1461,9 +1586,11 @@ describe User do
       project = create(:project, :private, namespace: user1.namespace)
 
       project.team << [user2, Gitlab::Access::DEVELOPER]
+
       expect(user2.authorized_projects).to include(project)
 
       project.destroy
+
       expect(user2.authorized_projects).not_to include(project)
     end
 
@@ -1473,9 +1600,11 @@ describe User do
       user    = create(:user)
 
       group.add_developer(user)
+
       expect(user.authorized_projects).to include(project)
 
       group.destroy
+
       expect(user.authorized_projects).not_to include(project)
     end
   end
@@ -2019,7 +2148,9 @@ describe User do
 
       it 'creates the namespace' do
         expect(user.namespace).to be_nil
+
         user.save!
+
         expect(user.namespace).not_to be_nil
       end
     end
@@ -2040,11 +2171,13 @@ describe User do
 
           it 'updates the namespace name' do
             user.update_attributes!(username: new_username)
+
             expect(user.namespace.name).to eq(new_username)
           end
 
           it 'updates the namespace path' do
             user.update_attributes!(username: new_username)
+
             expect(user.namespace.path).to eq(new_username)
           end
 
@@ -2058,6 +2191,7 @@ describe User do
 
             it 'adds the namespace errors to the user' do
               user.update_attributes(username: new_username)
+
               expect(user.errors.full_messages.first).to eq('Namespace name has already been taken')
             end
           end
@@ -2074,56 +2208,49 @@ describe User do
     end
   end
 
-  describe '#verified_email?' do
-    it 'returns true when the email is the primary email' do
-      user = build :user, email: 'email@example.com'
-
-      expect(user.verified_email?('email@example.com')).to be true
-    end
-
-    it 'returns false when the email is not the primary email' do
-      user = build :user, email: 'email@example.com'
-
-      expect(user.verified_email?('other_email@example.com')).to be false
-    end
-  end
-
   describe '#sync_attribute?' do
     let(:user) { described_class.new }
 
     context 'oauth user' do
       it 'returns true if name can be synced' do
         stub_omniauth_setting(sync_profile_attributes: %w(name location))
+
         expect(user.sync_attribute?(:name)).to be_truthy
       end
 
       it 'returns true if email can be synced' do
         stub_omniauth_setting(sync_profile_attributes: %w(name email))
+
         expect(user.sync_attribute?(:email)).to be_truthy
       end
 
       it 'returns true if location can be synced' do
         stub_omniauth_setting(sync_profile_attributes: %w(location email))
+
         expect(user.sync_attribute?(:email)).to be_truthy
       end
 
       it 'returns false if name can not be synced' do
         stub_omniauth_setting(sync_profile_attributes: %w(location email))
+
         expect(user.sync_attribute?(:name)).to be_falsey
       end
 
       it 'returns false if email can not be synced' do
         stub_omniauth_setting(sync_profile_attributes: %w(location email))
+
         expect(user.sync_attribute?(:name)).to be_falsey
       end
 
       it 'returns false if location can not be synced' do
         stub_omniauth_setting(sync_profile_attributes: %w(location email))
+
         expect(user.sync_attribute?(:name)).to be_falsey
       end
 
       it 'returns true for all syncable attributes if all syncable attributes can be synced' do
         stub_omniauth_setting(sync_profile_attributes: true)
+
         expect(user.sync_attribute?(:name)).to be_truthy
         expect(user.sync_attribute?(:email)).to be_truthy
         expect(user.sync_attribute?(:location)).to be_truthy
@@ -2139,6 +2266,7 @@ describe User do
     context 'ldap user' do
       it 'returns true for email if ldap user' do
         allow(user).to receive(:ldap_user?).and_return(true)
+
         expect(user.sync_attribute?(:name)).to be_falsey
         expect(user.sync_attribute?(:email)).to be_truthy
         expect(user.sync_attribute?(:location)).to be_falsey
@@ -2147,6 +2275,7 @@ describe User do
       it 'returns true for email and location if ldap user and location declared as syncable' do
         allow(user).to receive(:ldap_user?).and_return(true)
         stub_omniauth_setting(sync_profile_attributes: %w(location))
+
         expect(user.sync_attribute?(:name)).to be_falsey
         expect(user.sync_attribute?(:email)).to be_truthy
         expect(user.sync_attribute?(:location)).to be_truthy

spec/services/emails/confirm_service_spec.rb

+require 'spec_helper'
+
+describe Emails::ConfirmService do
+  let(:user) { create(:user) }
+
+  subject(:service) { described_class.new(user) }
+
+  describe '#execute' do
+    it 'sends a confirmation email again' do
+      email = user.emails.create(email: 'new@email.com')
+      mail  = service.execute(email)
+      expect(mail.subject).to eq('Confirmation instructions')
+    end
+  end
+end

spec/services/emails/create_service_spec.rb

@@ -12,6 +12,11 @@ describe Emails::CreateService do
       expect(Email.where(opts)).not_to be_empty
     end
 
+    it 'creates an email with additional attributes' do
+      expect { service.execute(confirmation_token: 'abc') }.to change { Email.count }.by(1)
+      expect(Email.where(opts).first.confirmation_token).to eq 'abc'
+    end
+
     it 'has the right user association' do
       service.execute
 

spec/services/emails/destroy_service_spec.rb

@@ -4,11 +4,11 @@ describe Emails::DestroyService do
   let!(:user) { create(:user) }
   let!(:email) { create(:email, user: user) }
 
-  subject(:service) { described_class.new(user, user: user, email: email.email) }
+  subject(:service) { described_class.new(user, user: user) }
 
   describe '#execute' do
     it 'removes an email' do
-      expect { service.execute }.to change { user.emails.count }.by(-1)
+      expect { service.execute(email) }.to change { user.emails.count }.by(-1)
     end
   end
 end

spec/services/notification_service_spec.rb

@@ -105,18 +105,6 @@ describe NotificationService, :mailer do
     end
   end
 
-  describe 'Email' do
-    describe '#new_email' do
-      let!(:email) { create(:email) }
-
-      it { expect(notification.new_email(email)).to be_truthy }
-
-      it 'sends email to email owner' do
-        expect { notification.new_email(email) }.to change { ActionMailer::Base.deliveries.size }.by(1)
-      end
-    end
-  end
-
   describe 'Notes' do
     context 'issue note' do
       let(:project) { create(:project, :private) }