Merge branch '9-5-stable' into security-9-5

app/assets/stylesheets/framework/typography.scss

@@ -13,6 +13,9 @@
   img {
     /*max-width: 100%;*/
     margin: 0 0 8px;
+  }
+
+  img.lazy {
     min-width: 200px;
     min-height: 100px;
     background-color: $gray-lightest;

app/assets/stylesheets/new_sidebar.scss

@@ -70,8 +70,7 @@ $new-sidebar-collapsed-width: 50px;
     background-color: $white-light;
   }
 
-  .project-title,
-  .group-title {
+  .sidebar-context-title {
     overflow: hidden;
     text-overflow: ellipsis;
   }
@@ -110,7 +109,7 @@ $new-sidebar-collapsed-width: 50px;
     }
 
     .badge,
-    .project-title {
+    .sidebar-context-title {
       display: none;
     }
 

app/assets/stylesheets/pages/commits.scss

@@ -286,6 +286,9 @@
 
 
 .gpg-status-box {
+  padding: 2px 10px;
+  margin-right: $gl-padding;
+
   &:empty {
     display: none;
   }
@@ -314,7 +317,6 @@
   &.valid {
     svg {
       border: 1px solid $brand-success;
-
       fill: $brand-success;
     }
   }
@@ -322,7 +324,6 @@
   &.invalid {
     svg {
       border: 1px solid $common-gray-light;
-
       fill: $common-gray-light;
     }
   }

app/controllers/projects/services_controller.rb

@@ -4,7 +4,6 @@ class Projects::ServicesController < Projects::ApplicationController
   # Authorize
   before_action :authorize_admin_project!
   before_action :service, only: [:edit, :update, :test]
-  before_action :update_service, only: [:update, :test]
 
   respond_to :html
 
@@ -14,6 +13,8 @@ class Projects::ServicesController < Projects::ApplicationController
   end
 
   def update
+    @service.attributes = service_params[:service]
+
     if @service.save(context: :manual_change)
       redirect_to(project_settings_integrations_path(@project), notice: success_message)
     else
@@ -24,7 +25,7 @@ class Projects::ServicesController < Projects::ApplicationController
   def test
     message = {}
 
-    if @service.can_test?
+    if @service.can_test? && @service.update_attributes(service_params[:service])
       data = @service.test_data(project, current_user)
       outcome = @service.test(data)
 
@@ -50,10 +51,6 @@ class Projects::ServicesController < Projects::ApplicationController
     end
   end
 
-  def update_service
-    @service.assign_attributes(service_params[:service])
-  end
-
   def service
     @service ||= @project.find_or_initialize_service(params[:id])
   end

app/helpers/avatars_helper.rb

@@ -12,11 +12,18 @@ module AvatarsHelper
     avatar_size = options[:size] || 16
     user_name = options[:user].try(:name) || options[:user_name]
     avatar_url = options[:url] || avatar_icon(options[:user] || options[:user_email], avatar_size)
+    has_tooltip = options[:has_tooltip].nil? ? true : options[:has_tooltip]
+    data_attributes = {}
+    css_class = %W[avatar s#{avatar_size}].push(*options[:css_class])
+
+    if has_tooltip
+      css_class.push('has-tooltip')
       data_attributes = { container: 'body' }
+    end
 
     image_tag(
       avatar_url,
-      class: %W[avatar has-tooltip s#{avatar_size}].push(*options[:css_class]),
+      class: css_class,
       alt: "#{user_name}'s avatar",
       title: user_name,
       data: data_attributes,

app/helpers/commits_helper.rb

@@ -114,7 +114,7 @@ module CommitsHelper
   end
 
   def commit_signature_badge_classes(additional_classes)
-    %w(btn status-box gpg-status-box) + Array(additional_classes)
+    %w(btn gpg-status-box) + Array(additional_classes)
   end
 
   protected

app/helpers/events_helper.rb

@@ -181,6 +181,7 @@ module EventsHelper
   end
 
   def event_commit_title(message)
+    message ||= ''
     (message.split("\n").first || "").truncate(70)
   rescue
     "--broken encoding"

app/models/event.rb

@@ -406,7 +406,7 @@ class Event < ActiveRecord::Base
 
   def body?
     if push?
-      push_with_commits? || rm_ref?
+      push_with_commits?
     elsif note?
       true
     else

app/views/events/_event_push.atom.haml

@@ -5,6 +5,7 @@
     %i
       at
       = event.created_at.to_s(:short)
+  - unless event.rm_ref?
     %blockquote= markdown(escape_once(event.commit_title), pipeline: :atom, project: event.project, author: event.author)
     - if event.commits_count > 1
       %p

app/views/events/event/_push.html.haml

@@ -41,7 +41,3 @@
         %li.commits-stat
           = link_to create_mr_path(project.default_branch, event.ref_name, project) do
             Create Merge Request
-- elsif event.rm_ref?
-  .event-body
-    %ul.well-list.event_commits
-      = render "events/commit", project: project, event: event

app/views/layouts/nav/_new_admin_sidebar.html.haml

@@ -4,7 +4,7 @@
       = link_to admin_root_path, title: 'Admin Overview' do
         .avatar-container.s40.settings-avatar
           = icon('wrench')
-        .project-title Admin Area
+        .sidebar-context-title Admin Area
     %ul.sidebar-top-level-items
       = nav_link(controller: %w(dashboard admin projects users groups jobs runners cohorts), html_options: {class: 'home'}) do
         = link_to admin_root_path, title: 'Overview', class: 'shortcuts-tree' do

app/views/layouts/nav/_new_group_sidebar.html.haml

@@ -4,7 +4,7 @@
       = link_to group_path(@group), title: @group.name do
         .avatar-container.s40.group-avatar
           = image_tag group_icon(@group), class: "avatar s40 avatar-tile"
-        .group-title
+        .sidebar-context-title
           = @group.name
     %ul.sidebar-top-level-items
       = nav_link(path: ['groups#show', 'groups#activity', 'groups#subgroups'], html_options: { class: 'home' }) do

app/views/layouts/nav/_new_profile_sidebar.html.haml

@@ -4,7 +4,7 @@
       = link_to profile_path, title: 'Profile Settings' do
         .avatar-container.s40.settings-avatar
           = icon('user')
-        .project-title User Settings
+        .sidebar-context-title User Settings
     %ul.sidebar-top-level-items
       = nav_link(path: 'profiles#show', html_options: {class: 'home'}) do
         = link_to profile_path, title: 'Profile Settings' do

app/views/layouts/nav/_new_project_sidebar.html.haml

@@ -5,7 +5,7 @@
       = link_to project_path(@project), title: @project.name do
         .avatar-container.s40.project-avatar
           = project_icon(@project, alt: @project.name, class: 'avatar s40 avatar-tile')
-        .project-title
+        .sidebar-context-title
           = @project.name
     %ul.sidebar-top-level-items
       = nav_link(path: ['projects#show', 'projects#activity', 'cycle_analytics#show'], html_options: { class: 'home' }) do

app/views/projects/_project_templates.html.haml

 .project-templates-buttons.import-buttons{ data: { toggle: "buttons" } }
   .btn.blank-option.active
-    %input{ type: "radio",  autocomplete: "off", name: "project_templates", id: "blank", checked: "true" }
+    %input{ type: "radio",  autocomplete: "off", name: "project[template_name]", id: "blank", checked: "true", value: "" }
     = icon('file-o', class: 'btn-template-icon')
     Blank
   - Gitlab::ProjectTemplate.all.each do |template|

app/views/projects/commits/_commit.html.haml

@@ -37,14 +37,14 @@
 
 
       .commit-actions.hidden-xs
-        - if commit.status(ref)
-          = render_commit_status(commit, ref: ref)
-
         - if request.xhr?
           = render partial: 'projects/commit/signature', object: commit.signature
         - else
           = render partial: 'projects/commit/ajax_signature', locals: { commit: commit }
 
+        - if commit.status(ref)
+          = render_commit_status(commit, ref: ref)
+
         = link_to commit.short_id, project_commit_path(project, commit), class: "commit-sha btn btn-transparent"
         = clipboard_button(text: commit.id, title: _("Copy commit SHA to clipboard"))
         = link_to_browse_code(project, commit)

app/views/shared/issuable/_user_dropdown_item.html.haml

@@ -4,7 +4,7 @@
 %li.filter-dropdown-item{ class: ('js-current-user' if user == current_user) }
   %button.btn.btn-link.dropdown-user{ type: :button }
     .avatar-container.s40
-      = user_avatar_without_link(user: user, lazy: avatar[:lazy], url: avatar[:url], size: 40).gsub('/images/{{avatar_url}}','{{avatar_url}}').html_safe
+      = user_avatar_without_link(user: user, lazy: avatar[:lazy], url: avatar[:url], size: 40, has_tooltip: false).gsub('/images/{{avatar_url}}','{{avatar_url}}').html_safe
     .dropdown-user-details
       %span
         = user.name

changelogs/unreleased/dm-ldap-adapter-attributes.yml

+---
+title: Fix signing in using LDAP when attribute mapping uses simple strings instead
+  of arrays
+merge_request:
+author:
+type: fixed

changelogs/unreleased/fix-old-mr-diffs.yml

+---
+title: Show un-highlighted text diffs when we do not have references to the correct
+  blobs
+merge_request:
+author:
+type: fixed

changelogs/unreleased/fix-push-events-branch-removals.yml

+---
+title: Fix display of push events for removed refs
+merge_request:
+author:
+type: fixed

changelogs/unreleased/rd-fix-broken-configuration-for-some-integrations.yml

+---
+title: Testing of some integrations were broken due to missing ServiceHook record.
+merge_request:
+author:
+type: fixed

changelogs/unreleased/zj-fix-fe-blank-button.yml

+---
+title: Fix new project form not resetting the template value
+merge_request:
+author:
+type: fixed

lib/gitlab/diff/file.rb

@@ -186,7 +186,10 @@ module Gitlab
       end
 
       def content_changed?
-        old_blob && new_blob && old_blob.id != new_blob.id
+        return blobs_changed? if diff_refs
+        return false if new_file? || deleted_file? || renamed_file?
+
+        text? && diff_lines.any?
       end
 
       def different_type?
@@ -225,6 +228,10 @@ module Gitlab
 
       private
 
+      def blobs_changed?
+        old_blob && new_blob && old_blob.id != new_blob.id
+      end
+
       def simple_viewer_class
         return DiffViewer::NotDiffable unless diffable?
 
@@ -250,6 +257,8 @@ module Gitlab
           DiffViewer::Renamed
         elsif mode_changed?
           DiffViewer::ModeChanged
+        else
+          DiffViewer::NoPreview
         end
       end
 

lib/gitlab/gitaly_client/commit_service.rb

@@ -159,8 +159,8 @@ module Gitlab
       def raw_blame(revision, path)
         request = Gitaly::RawBlameRequest.new(
           repository: @gitaly_repo,
-          revision: revision,
-          path: path
+          revision: GitalyClient.encode(revision),
+          path: GitalyClient.encode(path)
         )
 
         response = GitalyClient.call(@repository.storage, :commit_service, :raw_blame, request)

lib/gitlab/ldap/adapter.rb

@@ -73,7 +73,7 @@ module Gitlab
       private
 
       def user_options(field, value, limit)
-        options = { attributes: user_attributes }
+        options = { attributes: Gitlab::LDAP::Person.ldap_attributes(config).compact.uniq }
         options[:size] = limit if limit
 
         if field.to_sym == :dn
@@ -99,10 +99,6 @@ module Gitlab
           filter
         end
       end
-
-      def user_attributes
-        %W(#{config.uid} cn dn) + config.attributes['username'] + config.attributes['email']
-      end
     end
   end
 end

lib/gitlab/ldap/person.rb

@@ -21,6 +21,15 @@ module Gitlab
         adapter.dn_matches_filter?(dn, AD_USER_DISABLED)
       end
 
+      def self.ldap_attributes(config)
+        [
+          'dn', # Used in `dn`
+          config.uid, # Used in `uid`
+          *config.attributes['name'], # Used in `name`
+          *config.attributes['email'] # Used in `email`
+        ]
+      end
+
       def initialize(entry, provider)
         Rails.logger.debug { "Instantiating #{self.class.name} with LDIF:\n#{entry.to_ldif}" }
         @entry = entry

spec/controllers/projects/services_controller_spec.rb

@@ -10,9 +10,6 @@ describe Projects::ServicesController do
   before do
     sign_in(user)
     project.team << [user, :master]
-
-    controller.instance_variable_set(:@project, project)
-    controller.instance_variable_set(:@service, service)
   end
 
   describe '#test' do
@@ -20,7 +17,7 @@ describe Projects::ServicesController do
       it 'renders 404' do
         allow_any_instance_of(Service).to receive(:can_test?).and_return(false)
 
-        put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id
+        put :test, namespace_id: project.namespace, project_id: project, id: service.to_param
 
         expect(response).to have_http_status(404)
       end
@@ -36,7 +33,7 @@ describe Projects::ServicesController do
           it 'returns success' do
             allow_any_instance_of(MicrosoftTeams::Notifier).to receive(:ping).and_return(true)
 
-            put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id
+            put :test, namespace_id: project.namespace, project_id: project, id: service.to_param
 
             expect(response.status).to eq(200)
           end
@@ -45,7 +42,7 @@ describe Projects::ServicesController do
         it 'returns success' do
           expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client)
 
-          put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params
+          put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params
 
           expect(response.status).to eq(200)
         end
@@ -54,17 +51,42 @@ describe Projects::ServicesController do
       it 'returns success' do
         expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client)
 
-        put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params
+        put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params
 
         expect(response.status).to eq(200)
       end
+
+      context 'when service is configured for the first time' do
+        before do
+          allow_any_instance_of(ServiceHook).to receive(:execute).and_return(true)
+        end
+
+        it 'persist the object' do
+          do_put
+
+          expect(BuildkiteService.first).to be_present
+        end
+
+        it 'creates the ServiceHook object' do
+          do_put
+
+          expect(BuildkiteService.first.service_hook).to be_present
+        end
+
+        def do_put
+          put :test, namespace_id: project.namespace,
+                     project_id: project,
+                     id: 'buildkite',
+                     service: { 'active' => '1', 'push_events' => '1', token: 'token', 'project_url' => 'http://test.com' }
+        end
+      end
     end
 
     context 'failure' do
       it 'returns success status code and the error message' do
         expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_raise('Bad test')
 
-        put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params
+        put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params
 
         expect(response.status).to eq(200)
         expect(JSON.parse(response.body))
@@ -77,7 +99,7 @@ describe Projects::ServicesController do
     context 'when param `active` is set to true' do
       it 'activates the service and redirects to integrations paths' do
         put :update,
-          namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: { active: true }
+          namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: true }
 
         expect(response).to redirect_to(project_settings_integrations_path(project))
         expect(flash[:notice]).to eq 'HipChat activated.'
@@ -87,7 +109,7 @@ describe Projects::ServicesController do
     context 'when param `active` is set to false' do
       it 'does not  activate the service but saves the settings' do
         put :update,
-          namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: { active: false }
+          namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: false }
 
         expect(flash[:notice]).to eq 'HipChat settings saved, but not activated.'
       end

spec/helpers/avatars_helper_spec.rb

@@ -28,7 +28,7 @@ describe AvatarsHelper do
     it 'displays user avatar' do
       is_expected.to eq image_tag(
         LazyImageTagHelper.placeholder_image,
-        class: 'avatar has-tooltip s16  lazy',
+        class: 'avatar s16 has-tooltip  lazy',
         alt: "#{user.name}'s avatar",
         title: user.name,
         data: { container: 'body', src: avatar_icon(user, 16) }
@@ -41,7 +41,7 @@ describe AvatarsHelper do
       it 'uses provided css_class' do
         is_expected.to eq image_tag(
           LazyImageTagHelper.placeholder_image,
-          class: "avatar has-tooltip s16 #{options[:css_class]}  lazy",
+          class: "avatar s16 #{options[:css_class]} has-tooltip  lazy",
           alt: "#{user.name}'s avatar",
           title: user.name,
           data: { container: 'body', src: avatar_icon(user, 16) }
@@ -55,7 +55,7 @@ describe AvatarsHelper do
       it 'uses provided size' do
         is_expected.to eq image_tag(
           LazyImageTagHelper.placeholder_image,
-          class: "avatar has-tooltip s#{options[:size]}  lazy",
+          class: "avatar s#{options[:size]} has-tooltip  lazy",
           alt: "#{user.name}'s avatar",
           title: user.name,
           data: { container: 'body', src: avatar_icon(user, options[:size]) }
@@ -69,7 +69,7 @@ describe AvatarsHelper do
       it 'uses provided url' do
         is_expected.to eq image_tag(
           LazyImageTagHelper.placeholder_image,
-          class: 'avatar has-tooltip s16  lazy',
+          class: 'avatar s16 has-tooltip  lazy',
           alt: "#{user.name}'s avatar",
           title: user.name,
           data: { container: 'body', src: options[:url] }
@@ -77,6 +77,36 @@ describe AvatarsHelper do
       end
     end
 
+    context 'with has_tooltip parameter' do
+      context 'with has_tooltip set to true' do
+        let(:options) { { user: user, has_tooltip: true } }
+
+        it 'adds has-tooltip' do
+          is_expected.to eq image_tag(
+            LazyImageTagHelper.placeholder_image,
+            class: 'avatar s16 has-tooltip  lazy',
+            alt: "#{user.name}'s avatar",
+            title: user.name,
+            data: { container: 'body', src: avatar_icon(user, 16) }
+          )
+        end
+      end
+
+      context 'with has_tooltip set to false' do
+        let(:options) { { user: user, has_tooltip: false } }
+
+        it 'does not add has-tooltip or data container' do
+          is_expected.to eq image_tag(
+            LazyImageTagHelper.placeholder_image,
+            class: 'avatar s16  lazy',
+            alt: "#{user.name}'s avatar",
+            title: user.name,
+            data: { src: avatar_icon(user, 16) }
+          )
+        end
+      end
+    end
+
     context 'with user_name parameter' do
       let(:options) { { user_name: 'Tinky Winky', user_email: 'no@f.un' } }
 
@@ -86,7 +116,7 @@ describe AvatarsHelper do
         it 'prefers user parameter' do
           is_expected.to eq image_tag(
             LazyImageTagHelper.placeholder_image,
-            class: 'avatar has-tooltip s16  lazy',
+            class: 'avatar s16 has-tooltip  lazy',
             alt: "#{user.name}'s avatar",
             title: user.name,
             data: { container: 'body', src: avatar_icon(user, 16) }
@@ -97,7 +127,7 @@ describe AvatarsHelper do
       it 'uses user_name and user_email parameter if user is not present' do
         is_expected.to eq image_tag(
           LazyImageTagHelper.placeholder_image,
-          class: 'avatar has-tooltip s16  lazy',
+          class: 'avatar s16 has-tooltip  lazy',
           alt: "#{options[:user_name]}'s avatar",
           title: options[:user_name],
           data: { container: 'body', src: avatar_icon(options[:user_email], 16) }

spec/helpers/events_helper_spec.rb

@@ -106,5 +106,9 @@ describe EventsHelper do
     it "handles empty strings" do
       expect(helper.event_commit_title("")).to eq("")
     end
+
+    it 'handles nil values' do
+      expect(helper.event_commit_title(nil)).to eq('')
+    end
   end
 end

spec/lib/gitlab/diff/file_spec.rb

@@ -15,6 +15,17 @@ describe Gitlab::Diff::File do
     it { expect(diff_lines.first).to be_kind_of(Gitlab::Diff::Line) }
   end
 
+  describe '#highlighted_diff_lines' do
+    it 'highlights the diff and memoises the result' do
+      expect(Gitlab::Diff::Highlight).to receive(:new)
+                                           .with(diff_file, repository: project.repository)
+                                           .once
+                                           .and_call_original
+
+      diff_file.highlighted_diff_lines
+    end
+  end
+
   describe '#mode_changed?' do
     it { expect(diff_file.mode_changed?).to be_falsey }
   end
@@ -122,19 +133,43 @@ describe Gitlab::Diff::File do
         let(:commit) { project.commit('2f63565e7aac07bcdadb654e253078b727143ec4') }
         let(:diff_file) { commit.diffs.diff_file_with_new_path('files/images/6049019_460s.jpg') }
 
+        context 'when the blobs are different' do
           it 'returns true' do
             expect(diff_file.content_changed?).to be_truthy
           end
         end
 
+        context 'when there are no diff refs' do
+          before do
+            allow(diff_file).to receive(:diff_refs).and_return(nil)
+          end
+
+          it 'returns false' do
+            expect(diff_file.content_changed?).to be_falsey
+          end
+        end
+      end
+
       context 'when not binary' do
         let(:commit) { project.commit('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') }
         let(:diff_file) { commit.diffs.diff_file_with_new_path('files/ruby/popen.rb') }
 
+        context 'when the blobs are different' do
           it 'returns true' do
             expect(diff_file.content_changed?).to be_truthy
           end
         end
+
+        context 'when there are no diff refs' do
+          before do
+            allow(diff_file).to receive(:diff_refs).and_return(nil)
+          end
+
+          it 'returns true' do
+            expect(diff_file.content_changed?).to be_truthy
+          end
+        end
+      end
     end
   end
 
@@ -270,6 +305,21 @@ describe Gitlab::Diff::File do
         expect(diff_file.simple_viewer).to be_a(DiffViewer::ModeChanged)
       end
     end
+
+    context 'when no other conditions apply' do
+      before do
+        allow(diff_file).to receive(:content_changed?).and_return(false)
+        allow(diff_file).to receive(:new_file?).and_return(false)
+        allow(diff_file).to receive(:deleted_file?).and_return(false)
+        allow(diff_file).to receive(:renamed_file?).and_return(false)
+        allow(diff_file).to receive(:mode_changed?).and_return(false)
+        allow(diff_file).to receive(:raw_text?).and_return(false)
+      end
+
+      it 'returns a No Preview viewer' do
+        expect(diff_file.simple_viewer).to be_a(DiffViewer::NoPreview)
+      end
+    end
   end
 
   describe '#rich_viewer' do

spec/lib/gitlab/git/storage/forked_storage_check_spec.rb

 require 'spec_helper'
 
-describe Gitlab::Git::Storage::ForkedStorageCheck, skip_database_cleaner: true do
+describe Gitlab::Git::Storage::ForkedStorageCheck, broken_storage: true, skip_database_cleaner: true do
   let(:existing_path) do
     existing_path = TestEnv.repos_path
     FileUtils.mkdir_p(existing_path)

spec/lib/gitlab/ldap/adapter_spec.rb

@@ -16,7 +16,7 @@ describe Gitlab::LDAP::Adapter do
       expect(adapter).to receive(:ldap_search) do |arg|
         expect(arg[:filter].to_s).to eq('(uid=johndoe)')
         expect(arg[:base]).to eq('dc=example,dc=com')
-        expect(arg[:attributes]).to match(%w{uid cn dn uid userid sAMAccountName mail email userPrincipalName})
+        expect(arg[:attributes]).to match(%w{dn uid cn mail email userPrincipalName})
       end.and_return({})
 
       adapter.users('uid', 'johndoe')
@@ -26,7 +26,7 @@ describe Gitlab::LDAP::Adapter do
       expect(adapter).to receive(:ldap_search).with(
         base: 'uid=johndoe,ou=users,dc=example,dc=com',
         scope: Net::LDAP::SearchScope_BaseObject,
-        attributes: %w{uid cn dn uid userid sAMAccountName mail email userPrincipalName},
+        attributes: %w{dn uid cn mail email userPrincipalName},
         filter: nil
       ).and_return({})
 
@@ -63,7 +63,7 @@ describe Gitlab::LDAP::Adapter do
     it 'uses the right uid attribute when non-default' do
       stub_ldap_config(uid: 'sAMAccountName')
       expect(adapter).to receive(:ldap_search).with(
-        hash_including(attributes: %w{sAMAccountName cn dn uid userid sAMAccountName mail email userPrincipalName})
+        hash_including(attributes: %w{dn sAMAccountName cn mail email userPrincipalName})
       ).and_return({})
 
       adapter.users('sAMAccountName', 'johndoe')

spec/models/event_spec.rb

@@ -304,6 +304,50 @@ describe Event do
     end
   end
 
+  describe '#body?' do
+    let(:push_event) do
+      event = build(:push_event)
+
+      allow(event).to receive(:push?).and_return(true)
+
+      event
+    end
+
+    it 'returns true for a push event with commits' do
+      allow(push_event).to receive(:push_with_commits?).and_return(true)
+
+      expect(push_event).to be_body
+    end
+
+    it 'returns false for a push event without a valid commit range' do
+      allow(push_event).to receive(:push_with_commits?).and_return(false)
+
+      expect(push_event).not_to be_body
+    end
+
+    it 'returns true for a Note event' do
+      event = build(:event)
+
+      allow(event).to receive(:note?).and_return(true)
+
+      expect(event).to be_body
+    end
+
+    it 'returns true if the target responds to #title' do
+      event = build(:event)
+
+      allow(event).to receive(:target).and_return(double(:target, title: 'foo'))
+
+      expect(event).to be_body
+    end
+
+    it 'returns false for a regular event without a target' do
+      event = build(:event)
+
+      expect(event).not_to be_body
+    end
+  end
+
   def create_push_event(project, user)
     event = create(:push_event, project: project, author: user)
 

spec/spec_helper.rb

@@ -97,9 +97,15 @@ RSpec.configure do |config|
     reset_delivered_emails!
   end
 
-  if ENV['CI']
-    config.around(:each) do |ex|
-      ex.run_with_retry retry: 2
+  # Stub the `ForkedStorageCheck.storage_available?` method unless
+  # `:broken_storage` metadata is defined
+  #
+  # This check can be slow and is unnecessary in a test environment where we
+  # know the storage is available, because we create it at runtime
+  config.before(:example) do |example|
+    unless example.metadata[:broken_storage]
+      allow(Gitlab::Git::Storage::ForkedStorageCheck)
+        .to receive(:storage_available?).and_return(true)
     end
   end