Merge branch 'ce-to-ee-2018-05-17' into 'master'

CE upstream - 2018-05-17 14:52 UTC

Closes gitaly#1196

See merge request gitlab-org/gitlab-ee!5750

app/assets/javascripts/notes/components/note_header.vue

@@ -93,10 +93,13 @@ export default {
           v-html="actionTextHtml"
           class="system-note-message">
         </span>
+        <span class="system-note-separator">
+          &middot;
+        </span>
         <a
           :href="noteTimestampLink"
           @click="updateTargetNoteHash"
-          class="note-timestamp">
+          class="note-timestamp system-note-separator">
           <time-ago-tooltip
             :time="createdAt"
             tooltip-placement="bottom"

app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue

@@ -40,7 +40,7 @@ export default {
     :class="cssClass"
     :title="tooltipTitle(time)"
     :data-placement="tooltipPlacement"
-    data-container="body">
-    {{ timeFormated(time) }}
+    data-container="body"
+    v-text="timeFormated(time)">
   </time>
 </template>

app/assets/stylesheets/pages/boards.scss

-@import './issues/issue_count_badge';
-
 [v-cloak] {
   display: none;
 }

app/assets/stylesheets/pages/issues.scss

-@import "./issues/issue_count_badge";
-
 .issues-list {
   .issue {
     padding: 10px 0 10px $gl-padding;

app/assets/stylesheets/pages/notes.scss

@@ -455,6 +455,10 @@ ul.notes {
     white-space: normal;
   }
 
+  .system-note-separator {
+    color: $gl-text-color-disabled;
+  }
+
   a:hover {
     text-decoration: underline;
   }

app/controllers/concerns/accepts_pending_invitations.rb

+module AcceptsPendingInvitations
+  extend ActiveSupport::Concern
+
+  def accept_pending_invitations
+    return unless resource.active_for_authentication?
+
+    clear_stored_location_for_resource if resource.accept_pending_invitations!.any?
+  end
+
+  def clear_stored_location_for_resource
+    session_key = stored_location_key_for(resource)
+
+    session.delete(session_key)
+  end
+end

app/controllers/confirmations_controller.rb

 class ConfirmationsController < Devise::ConfirmationsController
   prepend ::EE::ConfirmationsController
 
+  include AcceptsPendingInvitations
+
   def almost_there
     flash[:notice] = nil
     render layout: "devise_empty"
@@ -13,6 +15,8 @@ class ConfirmationsController < Devise::ConfirmationsController
   end
 
   def after_confirmation_path_for(resource_name, resource)
+    accept_pending_invitations
+
     # incoming resource can either be a :user or an :email
     if signed_in?(:user)
       after_sign_in(resource)

app/controllers/projects/commit_controller.rb

@@ -23,8 +23,12 @@ class Projects::CommitController < Projects::ApplicationController
 
     respond_to do |format|
       format.html  { render }
-      format.diff  { render text: @commit.to_diff }
-      format.patch { render text: @commit.to_patch }
+      format.diff  do
+        send_git_diff(@project.repository, @commit.diff_refs)
+      end
+      format.patch do
+        send_git_patch(@project.repository, @commit.diff_refs)
+      end
     end
   end
 

app/controllers/projects/pipelines_controller.rb

@@ -20,19 +20,12 @@ class Projects::PipelinesController < Projects::ApplicationController
       .page(params[:page])
       .per(30)
 
-    @running_count = PipelinesFinder
-      .new(project, scope: 'running').execute.count
+    @running_count = limited_pipelines_count(project, 'running')
+    @pending_count = limited_pipelines_count(project, 'pending')
+    @finished_count = limited_pipelines_count(project, 'finished')
+    @pipelines_count = limited_pipelines_count(project)
 
-    @pending_count = PipelinesFinder
-      .new(project, scope: 'pending').execute.count
-
-    @finished_count = PipelinesFinder
-      .new(project, scope: 'finished').execute.count
-
-    @pipelines_count = PipelinesFinder
-      .new(project).execute.count
-
-    @pipelines.map(&:commit) # List commits for batch loading
+    Gitlab::Ci::Pipeline::Preloader.preload(@pipelines)
 
     respond_to do |format|
       format.html
@@ -43,7 +36,7 @@ class Projects::PipelinesController < Projects::ApplicationController
           pipelines: PipelineSerializer
             .new(project: @project, current_user: @current_user)
             .with_pagination(request, response)
-            .represent(@pipelines),
+            .represent(@pipelines, disable_coverage: true),
           count: {
             all: @pipelines_count,
             running: @running_count,
@@ -187,4 +180,10 @@ class Projects::PipelinesController < Projects::ApplicationController
   def authorize_update_pipeline!
     return access_denied! unless can?(current_user, :update_pipeline, @pipeline)
   end
+
+  def limited_pipelines_count(project, scope = nil)
+    finder = PipelinesFinder.new(project, scope: scope)
+
+    view_context.limited_counter_with_delimiter(finder.execute)
+  end
 end

app/controllers/registrations_controller.rb

 class RegistrationsController < Devise::RegistrationsController
   include Recaptcha::Verify
-  prepend EE::RegistrationsController
+  include AcceptsPendingInvitations
 
-  before_action :whitelist_query_limiting, only: [:destroy]
+  prepend EE::RegistrationsController
 
   before_action :whitelist_query_limiting, only: [:destroy]
 
@@ -19,6 +19,7 @@ class RegistrationsController < Devise::RegistrationsController
     end
 
     if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha
+      accept_pending_invitations
       super
     else
       flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
@@ -63,7 +64,7 @@ class RegistrationsController < Devise::RegistrationsController
 
   def after_sign_up_path_for(user)
     Gitlab::AppLogger.info("User Created: username=#{user.username} email=#{user.email} ip=#{request.remote_ip} confirmed:#{user.confirmed?}")
-    user.confirmed? ? dashboard_projects_path : users_almost_there_path
+    user.confirmed? ? stored_location_for(user) || dashboard_projects_path : users_almost_there_path
   end
 
   def after_inactive_sign_up_path_for(resource)

app/models/ci/pipeline.rb

@@ -420,7 +420,18 @@ module Ci
     end
 
     def has_warnings?
-      builds.latest.failed_but_allowed.any?
+      number_of_warnings.positive?
+    end
+
+    def number_of_warnings
+      BatchLoader.for(id).batch(default_value: 0) do |pipeline_ids, loader|
+        Build.where(commit_id: pipeline_ids)
+          .latest
+          .failed_but_allowed
+          .group(:commit_id)
+          .count
+          .each { |id, amount| loader.call(id, amount) }
+      end
     end
 
     def set_config_source

app/models/commit.rb

@@ -224,8 +224,34 @@ class Commit
     Gitlab::ClosingIssueExtractor.new(project, current_user).closed_by_message(safe_message)
   end
 
+  def lazy_author
+    BatchLoader.for(author_email.downcase).batch do |emails, loader|
+      # A Hash that maps user Emails to the corresponding User objects. The
+      # Emails at this point are the _primary_ Emails of the Users.
+      users_for_emails = User
+        .by_any_email(emails)
+        .each_with_object({}) { |user, hash| hash[user.email] = user }
+
+      users_for_ids = users_for_emails
+        .values
+        .each_with_object({}) { |user, hash| hash[user.id] = user }
+
+      # Some commits may have used an alternative Email address. In this case we
+      # need to query the "emails" table to map those addresses to User objects.
+      Email
+        .where(email: emails - users_for_emails.keys)
+        .pluck(:email, :user_id)
+        .each { |(email, id)| users_for_emails[email] = users_for_ids[id] }
+
+      users_for_emails.each { |email, user| loader.call(email, user) }
+    end
+  end
+
   def author
-    User.find_by_any_email(author_email.downcase)
+    # We use __sync so that we get the actual objects back (including an actual
+    # nil), instead of a wrapper, as returning a wrapped nil breaks a lot of
+    # code.
+    lazy_author.__sync
   end
   request_cache(:author) { author_email.downcase }
 

app/models/commit_status.rb

@@ -2,6 +2,7 @@ class CommitStatus < ActiveRecord::Base
   include HasStatus
   include Importable
   include AfterCommitQueue
+  include Presentable
 
   self.table_name = 'ci_builds'
 

app/models/user.rb

@@ -878,6 +878,16 @@ class User < ActiveRecord::Base
     confirmed? && !temp_oauth_email?
   end
 
+  def accept_pending_invitations!
+    pending_invitations.select do |member|
+      member.accept_invite!(self)
+    end
+  end
+
+  def pending_invitations
+    Member.where(invite_email: verified_emails).invite
+  end
+
   def all_emails
     all_emails = []
     all_emails << email unless temp_oauth_email?

app/presenters/ci/build_presenter.rb

 module Ci
-  class BuildPresenter < Gitlab::View::Presenter::Delegated
-    CALLOUT_FAILURE_MESSAGES = {
-      unknown_failure: 'There is an unknown failure, please try again',
-      script_failure: 'There has been a script failure. Check the job log for more information',
-      api_failure: 'There has been an API failure, please try again',
-      stuck_or_timeout_failure: 'There has been a timeout failure or the job got stuck. Check your timeout limits or try again',
-      runner_system_failure: 'There has been a runner system failure, please try again',
-      missing_dependency_failure: 'There has been a missing dependency failure, check the job log for more information'
-    }.freeze
-
-    presents :build
-
+  class BuildPresenter < CommitStatusPresenter
     def erased_by_user?
       # Build can be erased through API, therefore it does not have
       # `erased_by` user assigned in that case.
@@ -44,14 +33,6 @@ module Ci
       "#{subject.name} - #{detailed_status.status_tooltip}"
     end
 
-    def callout_failure_message
-      CALLOUT_FAILURE_MESSAGES[failure_reason.to_sym]
-    end
-
-    def recoverable?
-      failed? && !unrecoverable?
-    end
-
     private
 
     def tooltip_for_badge
@@ -61,9 +42,5 @@ module Ci
     def detailed_status
       @detailed_status ||= subject.detailed_status(user)
     end
-
-    def unrecoverable?
-      script_failure? || missing_dependency_failure?
-    end
   end
 end

app/presenters/commit_status_presenter.rb

+class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
+  CALLOUT_FAILURE_MESSAGES = {
+      unknown_failure: 'There is an unknown failure, please try again',
+      script_failure: 'There has been a script failure. Check the job log for more information',
+      api_failure: 'There has been an API failure, please try again',
+      stuck_or_timeout_failure: 'There has been a timeout failure or the job got stuck. Check your timeout limits or try again',
+      runner_system_failure: 'There has been a runner system failure, please try again',
+      missing_dependency_failure: 'There has been a missing dependency failure, check the job log for more information'
+  }.freeze
+
+  presents :build
+
+  def callout_failure_message
+    CALLOUT_FAILURE_MESSAGES[failure_reason.to_sym]
+  end
+
+  def recoverable?
+    failed? && !unrecoverable?
+  end
+
+  def unrecoverable?
+    script_failure? || missing_dependency_failure?
+  end
+end

app/presenters/generic_commit_status_presenter.rb

+class GenericCommitStatusPresenter < CommitStatusPresenter
+end

app/serializers/pipeline_entity.rb

@@ -4,7 +4,11 @@ class PipelineEntity < Grape::Entity
   expose :id
   expose :user, using: UserEntity
   expose :active?, as: :active
-  expose :coverage
+
+  # Coverage isn't always necessary (e.g. when displaying project pipelines in
+  # the UI). Instead of creating an entirely different entity we just allow the
+  # disabling of this specific field whenever necessary.
+  expose :coverage, unless: proc { options[:disable_coverage] }
   expose :source
 
   expose :created_at, :updated_at

app/views/notify/member_invited_email.html.haml

@@ -4,7 +4,7 @@
     by
     = link_to member.created_by.name, user_url(member.created_by)
   to join the
-  = link_to member_source.human_name, member_source.web_url
+  = link_to member_source.human_name, member_source.public? ? member_source.web_url : invite_url(@token)
   #{member_source.model_name.singular} as #{member.human_access}.
 
 %p

app/views/shared/notes/_note.html.haml

@@ -41,8 +41,9 @@
               - if note.system
                 %span.system-note-message
                   = markdown_field(note, :note)
-              %a{ href: "##{dom_id(note)}" }
-                = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
+              %span.system-note-separator
+                ·
+              %a.system-note-separator{ href: "##{dom_id(note)}" }= time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
         - unless note.system?
           .note-actions
             - if note.for_personal_snippet?

changelogs/unreleased/42531-open-invite-404.yml

+---
+title: Automatically accepts project/group invite by email after user signup
+merge_request: 17634
+author: Jacopo Beschi @jacopo-beschi
+type: changed

changelogs/unreleased/46177-fix-present-on-generic-commit-status.yml

+---
+title: Allow CommitStatus class to use presentable methods
+merge_request: 18979
+author:
+type: fixed

changelogs/unreleased/jivl-add-dot-system-notes.yml

+---
+title: Add dot to separate system notes content
+merge_request: 18864
+author:
+type: changed

changelogs/unreleased/pipelines-index-performance.yml

+---
+title: Improve performance of project pipelines pages
+merge_request:
+author:
+type: performance

changelogs/unreleased/zj-workhorse-commit-patch-diff.yml

+---
+title: Workhorse to send raw diff and patch for commits
+merge_request:
+author:
+type: other

doc/topics/autodevops/quick_start_guide.md

@@ -23,6 +23,10 @@ page](https://gitlab.com/auto-devops-examples/minimal-ruby-app) and press the
 **Fork** button. Soon you should have a project under your namespace with the
 necessary files.
 
+You can also start a new project from a
+[GitLab project template](https://gitlab.com/gitlab-org/project-templates) if
+you want to use a different language.
+
 ## Setup your own cluster on Google Kubernetes Engine
 
 If you do not already have a Google Cloud account, create one at

lib/gitlab/ci/pipeline/preloader.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Pipeline
+      # Class for preloading data associated with pipelines such as commit
+      # authors.
+      module Preloader
+        def self.preload(pipelines)
+          # This ensures that all the pipeline commits are eager loaded before we
+          # start using them.
+          pipelines.each(&:commit)
+
+          pipelines.each do |pipeline|
+            # This preloads the author of every commit. We're using "lazy_author"
+            # here since "author" immediately loads the data on the first call.
+            pipeline.commit.try(:lazy_author)
+
+            # This preloads the number of warnings for every pipeline, ensuring
+            # that Ci::Pipeline#has_warnings? doesn't execute any additional
+            # queries.
+            pipeline.number_of_warnings
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/git/commit.rb

@@ -342,21 +342,6 @@ module Gitlab
         parent_ids.first
       end
 
-      # Shows the diff between the commit's parent and the commit.
-      #
-      # Cuts out the header and stats from #to_patch and returns only the diff.
-      #
-      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/324
-      def to_diff
-        Gitlab::GitalyClient.migrate(:commit_patch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
-          if is_enabled
-            @repository.gitaly_commit_client.patch(id)
-          else
-            rugged_diff_from_parent.patch
-          end
-        end
-      end
-
       # Returns a diff object for the changes from this commit's first parent.
       # If there is no parent, then the diff is between this commit and an
       # empty repo. See Repository#diff for keys allowed in the +options+
@@ -432,16 +417,6 @@ module Gitlab
         Gitlab::Git::CommitStats.new(@repository, self)
       end
 
-      def to_patch(options = {})
-        begin
-          rugged_commit.to_mbox(options)
-        rescue Rugged::InvalidError => ex
-          if ex.message =~ /commit \w+ is a merge commit/i
-            'Patch format is not currently supported for merge commits.'
-          end
-        end
-      end
-
       # Get ref names collection
       #
       # Ex.

spec/controllers/projects/commit_controller_spec.rb

@@ -79,41 +79,18 @@ describe Projects::CommitController do
     end
 
     describe "as diff" do
-      include_examples "export as", :diff
-      let(:format) { :diff }
+      it "triggers workhorse to serve the request" do
+        go(id: commit.id, format: :diff)
 
-      it "should really only be a git diff" do
-        go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format)
-
-        expect(response.body).to start_with("diff --git")
-      end
-
-      it "is only be a git diff without whitespace changes" do
-        go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format, w: 1)
-
-        expect(response.body).to start_with("diff --git")
-
-        # without whitespace option, there are more than 2 diff_splits for other formats
-        diff_splits = assigns(:diffs).diff_files.first.diff.diff.split("\n")
-        expect(diff_splits.length).to be <= 2
+        expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-diff:")
       end
     end
 
     describe "as patch" do
-      include_examples "export as", :patch
-      let(:format) { :patch }
-      let(:commit2) { project.commit('498214de67004b1da3d820901307bed2a68a8ef6') }
-
-      it "is a git email patch" do
-        go(id: commit2.id, format: format)
-
-        expect(response.body).to start_with("From #{commit2.id}")
-      end
-
       it "contains a git diff" do
-        go(id: commit2.id, format: format)
+        go(id: commit.id, format: :patch)
 
-        expect(response.body).to match(/^diff --git/)
+        expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-format-patch:")
       end
     end
 

spec/controllers/projects/pipelines_controller_spec.rb

@@ -35,10 +35,16 @@ describe Projects::PipelinesController do
 
       expect(json_response).to include('pipelines')
       expect(json_response['pipelines'].count).to eq 4
-      expect(json_response['count']['all']).to eq 4
-      expect(json_response['count']['running']).to eq 1
-      expect(json_response['count']['pending']).to eq 1
-      expect(json_response['count']['finished']).to eq 1
+      expect(json_response['count']['all']).to eq '4'
+      expect(json_response['count']['running']).to eq '1'
+      expect(json_response['count']['pending']).to eq '1'
+      expect(json_response['count']['finished']).to eq '1'
+    end
+
+    it 'does not include coverage data for the pipelines' do
+      subject
+
+      expect(json_response['pipelines'][0]).not_to include('coverage')
     end
 
     context 'when performing gitaly calls', :request_store do

spec/features/invites_spec.rb

@@ -5,18 +5,41 @@ describe 'Invites' do
   let(:owner) { create(:user, name: 'John Doe') }
   let(:group) { create(:group, name: 'Owned') }
   let(:project) { create(:project, :repository, namespace: group) }
-  let(:invite) { group.group_members.invite.last }
+  let(:group_invite) { group.group_members.invite.last }
 
   before do
     project.add_master(owner)
     group.add_user(owner, Gitlab::Access::OWNER)
     group.add_developer('user@example.com', owner)
-    invite.generate_invite_token!
+    group_invite.generate_invite_token!
+  end
+
+  def confirm_email_and_sign_in(new_user)
+    new_user_token = User.find_by_email(new_user.email).confirmation_token
+
+    visit user_confirmation_path(confirmation_token: new_user_token)
+    fill_in_sign_in_form(new_user)
+  end
+
+  def fill_in_sign_up_form(new_user)
+    fill_in 'new_user_name',                with: new_user.name
+    fill_in 'new_user_username',            with: new_user.username
+    fill_in 'new_user_email',               with: new_user.email
+    fill_in 'new_user_email_confirmation',  with: new_user.email
+    fill_in 'new_user_password',            with: new_user.password
+    click_button "Register"
+  end
+
+  def fill_in_sign_in_form(user)
+    fill_in 'user_login', with: user.email
+    fill_in 'user_password', with: user.password
+    check 'user_remember_me'
+    click_button 'Sign in'
   end
 
   context 'when signed out' do
     before do
-      visit invite_path(invite.raw_invite_token)
+      visit invite_path(group_invite.raw_invite_token)
     end
 
     it 'renders sign in page with sign in notice' do
@@ -25,12 +48,9 @@ describe 'Invites' do
     end
 
     it 'sign in and redirects to invitation page' do
-      fill_in 'user_login', with: user.email
-      fill_in 'user_password', with: user.password
-      check 'user_remember_me'
-      click_button 'Sign in'
+      fill_in_sign_in_form(user)
 
-      expect(current_path).to eq(invite_path(invite.raw_invite_token))
+      expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
       expect(page).to have_content(
         'You have been invited by John Doe to join group Owned as Developer.'
       )
@@ -45,7 +65,7 @@ describe 'Invites' do
     end
 
     it 'shows message user already a member' do
-      visit invite_path(invite.raw_invite_token)
+      visit invite_path(group_invite.raw_invite_token)
       expect(page).to have_content('However, you are already a member of this group.')
     end
   end
@@ -53,7 +73,7 @@ describe 'Invites' do
   describe 'accepting the invitation' do
     before do
       sign_in(user)
-      visit invite_path(invite.raw_invite_token)
+      visit invite_path(group_invite.raw_invite_token)
     end
 
     it 'grants access and redirects to group page' do
@@ -69,7 +89,7 @@ describe 'Invites' do
     context 'when signed in' do
       before do
         sign_in(user)
-        visit invite_path(invite.raw_invite_token)
+        visit invite_path(group_invite.raw_invite_token)
       end
 
       it 'declines application and redirects to dashboard' do
@@ -83,7 +103,7 @@ describe 'Invites' do
 
     context 'when signed out' do
       before do
-        visit decline_invite_path(invite.raw_invite_token)
+        visit decline_invite_path(group_invite.raw_invite_token)
       end
 
       it 'declines application and redirects to sign in page' do
@@ -94,4 +114,72 @@ describe 'Invites' do
       end
     end
   end
+
+  describe 'invite an user using their email address' do
+    let(:new_user) { build_stubbed(:user) }
+    let(:invite_email) { new_user.email }
+    let(:group_invite) { create(:group_member, :invited, group: group, invite_email: invite_email) }
+    let!(:project_invite) { create(:project_member, :invited, project: project, invite_email: invite_email) }
+
+    before do
+      stub_application_setting(send_user_confirmation_email: send_email_confirmation)
+      visit invite_path(group_invite.raw_invite_token)
+    end
+
+    context 'email confirmation disabled' do
+      let(:send_email_confirmation) { false }
+
+      it 'signs up and redirects to the dashboard page with all the projects/groups invitations automatically accepted' do
+        fill_in_sign_up_form(new_user)
+
+        expect(current_path).to eq(dashboard_projects_path)
+        expect(page).to have_content(project.full_name)
+        visit group_path(group)
+        expect(page).to have_content(group.full_name)
+      end
+
+      context 'the user sign-up using a different email address' do
+        let(:invite_email) { build_stubbed(:user).email }
+
+        it 'signs up and redirects to the invitation page' do
+          fill_in_sign_up_form(new_user)
+
+          expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
+        end
+      end
+    end
+
+    context 'email confirmation enabled' do
+      let(:send_email_confirmation) { true }
+
+      it 'signs up and redirects to root page with all the project/groups invitation automatically accepted' do
+        fill_in_sign_up_form(new_user)
+        confirm_email_and_sign_in(new_user)
+
+        expect(current_path).to eq(root_path)
+        expect(page).to have_content(project.full_name)
+        visit group_path(group)
+        expect(page).to have_content(group.full_name)
+      end
+
+      it "doesn't accept invitations until the user confirm his email" do
+        fill_in_sign_up_form(new_user)
+        sign_in(owner)
+
+        visit project_project_members_path(project)
+        expect(page).to have_content 'Invited'
+      end
+
+      context 'the user sign-up using a different email address' do
+        let(:invite_email) { build_stubbed(:user).email }
+
+        it 'signs up and redirects to the invitation page' do
+          fill_in_sign_up_form(new_user)
+          confirm_email_and_sign_in(new_user)
+
+          expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
+        end
+      end
+    end
+  end
 end

spec/lib/gitlab/ci/pipeline/preloader_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Ci::Pipeline::Preloader do
+  describe '.preload' do
+    it 'preloads the author of every pipeline commit' do
+      commit = double(:commit)
+      pipeline = double(:pipeline, commit: commit)
+
+      expect(commit)
+        .to receive(:lazy_author)
+
+      expect(pipeline)
+        .to receive(:number_of_warnings)
+
+      described_class.preload([pipeline])
+    end
+  end
+end

spec/lib/gitlab/git/commit_spec.rb

@@ -554,24 +554,10 @@ describe Gitlab::Git::Commit, seed_helper: true do
     it_should_behave_like '#stats'
   end
 
-  describe '#to_diff' do
-    subject { commit.to_diff }
-
-    it { is_expected.not_to include "From #{SeedRepo::Commit::ID}" }
-    it { is_expected.to include 'diff --git a/files/ruby/popen.rb b/files/ruby/popen.rb'}
-  end
-
   describe '#has_zero_stats?' do
     it { expect(commit.has_zero_stats?).to eq(false) }
   end
 
-  describe '#to_patch' do
-    subject { commit.to_patch }
-
-    it { is_expected.to include "From #{SeedRepo::Commit::ID}" }
-    it { is_expected.to include 'diff --git a/files/ruby/popen.rb b/files/ruby/popen.rb'}
-  end
-
   describe '#to_hash' do
     let(:hash) { commit.to_hash }
     subject { hash }

spec/mailers/notify_spec.rb

@@ -605,7 +605,7 @@ describe Notify do
       it 'contains all the useful information' do
         is_expected.to have_subject "Invitation to join the #{project.full_name} project"
         is_expected.to have_html_escaped_body_text project.full_name
-        is_expected.to have_body_text project.web_url
+        is_expected.to have_body_text project.full_name
         is_expected.to have_body_text project_member.human_access
         is_expected.to have_body_text project_member.invite_token
       end

spec/models/ci/pipeline_spec.rb

@@ -783,6 +783,33 @@ describe Ci::Pipeline, :mailer do
     end
   end
 
+  describe '#number_of_warnings' do
+    it 'returns the number of warnings' do
+      create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop')
+
+      expect(pipeline.number_of_warnings).to eq(1)
+    end
+
+    it 'supports eager loading of the number of warnings' do
+      pipeline2 = create(:ci_empty_pipeline, status: :created, project: project)
+
+      create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop')
+      create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline2, name: 'rubocop')
+
+      pipelines = project.pipelines.to_a
+
+      pipelines.each(&:number_of_warnings)
+
+      # To run the queries we need to actually use the lazy objects, which we do
+      # by just sending "to_i" to them.
+      amount = ActiveRecord::QueryRecorder
+        .new { pipelines.each { |p| p.number_of_warnings.to_i } }
+        .count
+
+      expect(amount).to eq(1)
+    end
+  end
+
   shared_context 'with some outdated pipelines' do
     before do
       create_pipeline(:canceled, 'ref', 'A', project)

spec/models/commit_spec.rb

@@ -52,22 +52,98 @@ describe Commit do
     end
   end
 
-  describe '#author' do
+  describe '#author', :request_store do
     it 'looks up the author in a case-insensitive way' do
       user = create(:user, email: commit.author_email.upcase)
       expect(commit.author).to eq(user)
     end
 
-    it 'caches the author', :request_store do
+    it 'caches the author' do
       user = create(:user, email: commit.author_email)
-      expect(User).to receive(:find_by_any_email).and_call_original
 
       expect(commit.author).to eq(user)
+
       key = "Commit:author:#{commit.author_email.downcase}"
-      expect(RequestStore.store[key]).to eq(user)
 
+      expect(RequestStore.store[key]).to eq(user)
       expect(commit.author).to eq(user)
     end
+
+    context 'using eager loading' do
+      let!(:alice) { create(:user, email: 'alice@example.com') }
+      let!(:bob) { create(:user, email: 'hunter2@example.com') }
+
+      let(:alice_commit) do
+        described_class.new(RepoHelpers.sample_commit, project).tap do |c|
+          c.author_email = 'alice@example.com'
+        end
+      end
+
+      let(:bob_commit) do
+        # The commit for Bob uses one of his alternative Emails, instead of the
+        # primary one.
+        described_class.new(RepoHelpers.sample_commit, project).tap do |c|
+          c.author_email = 'bob@example.com'
+        end
+      end
+
+      let(:eve_commit) do
+        described_class.new(RepoHelpers.sample_commit, project).tap do |c|
+          c.author_email = 'eve@example.com'
+        end
+      end
+
+      let!(:commits) { [alice_commit, bob_commit, eve_commit] }
+
+      before do
+        create(:email, user: bob, email: 'bob@example.com')
+      end
+
+      it 'executes only two SQL queries' do
+        recorder = ActiveRecord::QueryRecorder.new do
+          # Running this first ensures we don't run one query for every
+          # commit.
+          commits.each(&:lazy_author)
+
+          # This forces the execution of the SQL queries necessary to load the
+          # data.
+          commits.each { |c| c.author.try(:id) }
+        end
+
+        expect(recorder.count).to eq(2)
+      end
+
+      it "preloads the authors for Commits matching a user's primary Email" do
+        commits.each(&:lazy_author)
+
+        expect(alice_commit.author).to eq(alice)
+      end
+
+      it "preloads the authors for Commits using a User's alternative Email" do
+        commits.each(&:lazy_author)
+
+        expect(bob_commit.author).to eq(bob)
+      end
+
+      it 'sets the author to Nil if an author could not be found for a Commit' do
+        commits.each(&:lazy_author)
+
+        expect(eve_commit.author).to be_nil
+      end
+
+      it 'does not execute SQL queries once the authors are preloaded' do
+        commits.each(&:lazy_author)
+        commits.each { |c| c.author.try(:id) }
+
+        recorder = ActiveRecord::QueryRecorder.new do
+          alice_commit.author
+          bob_commit.author
+          eve_commit.author
+        end
+
+        expect(recorder.count).to be_zero
+      end
+    end
   end
 
   describe '#to_reference' do
@@ -182,7 +258,6 @@ eos
     it { is_expected.to respond_to(:date) }
     it { is_expected.to respond_to(:diffs) }
     it { is_expected.to respond_to(:id) }
-    it { is_expected.to respond_to(:to_patch) }
   end
 
   describe '#closes_issues' do

spec/models/commit_status_spec.rb

@@ -565,4 +565,10 @@ describe CommitStatus do
       it_behaves_like 'commit status enqueued'
     end
   end
+
+  describe '#present' do
+    subject { commit_status.present }
+
+    it { is_expected.to be_a(CommitStatusPresenter) }
+  end
 end

spec/models/generic_commit_status_spec.rb

@@ -78,4 +78,10 @@ describe GenericCommitStatus do
       it { is_expected.not_to be_nil }
     end
   end
+
+  describe '#present' do
+    subject { generic_commit_status.present }
+
+    it { is_expected.to be_a(GenericCommitStatusPresenter) }
+  end
 end

spec/models/user_spec.rb

@@ -1251,6 +1251,24 @@ describe User do
     end
   end
 
+  describe '#accept_pending_invitations!' do
+    let(:user) { create(:user, email: 'user@email.com') }
+    let!(:project_member_invite) { create(:project_member, :invited, invite_email: user.email) }
+    let!(:group_member_invite) { create(:group_member, :invited, invite_email: user.email) }
+    let!(:external_project_member_invite) { create(:project_member, :invited, invite_email: 'external@email.com') }
+    let!(:external_group_member_invite) { create(:group_member, :invited, invite_email: 'external@email.com') }
+
+    it 'accepts all the user members pending invitations and returns the accepted_members' do
+      accepted_members = user.accept_pending_invitations!
+
+      expect(accepted_members).to match_array([project_member_invite, group_member_invite])
+      expect(group_member_invite.reload).not_to be_invite
+      expect(project_member_invite.reload).not_to be_invite
+      expect(external_project_member_invite.reload).to be_invite
+      expect(external_group_member_invite.reload).to be_invite
+    end
+  end
+
   describe '#all_emails' do
     let(:user) { create(:user) }
 

spec/presenters/ci/build_presenter_spec.rb

@@ -10,7 +10,7 @@ describe Ci::BuildPresenter do
   end
 
   it 'inherits from Gitlab::View::Presenter::Delegated' do
-    expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated)
+    expect(described_class.ancestors).to include(Gitlab::View::Presenter::Delegated)
   end
 
   describe '#initialize' do

spec/presenters/commit_status_presenter_spec.rb

+require 'spec_helper'
+
+describe CommitStatusPresenter do
+  let(:project) { create(:project) }
+  let(:pipeline) { create(:ci_pipeline, project: project) }
+  let(:build) { create(:ci_build, pipeline: pipeline) }
+
+  subject(:presenter) do
+    described_class.new(build)
+  end
+
+  it 'inherits from Gitlab::View::Presenter::Delegated' do
+    expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated)
+  end
+end

spec/serializers/pipeline_entity_spec.rb

@@ -26,6 +26,13 @@ describe PipelineEntity do
         expect(subject).to include :updated_at, :created_at
       end
 
+      it 'excludes coverage data when disabled' do
+        entity = described_class
+          .represent(pipeline, request: request, disable_coverage: true)
+
+        expect(entity.as_json).not_to include(:coverage)
+      end
+
       it 'contains details' do
         expect(subject).to include :details
         expect(subject[:details])