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

Merge 9-5-stable into security-9-5

See merge request gitlab/gitlabhq!2184

CHANGELOG.md

@@ -2,6 +2,24 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 9.5.3 (2017-09-03)
+
+- [SECURITY] Filter additional secrets from Rails logs.
+- [FIXED] Make username update fail if the namespace update fails. !13642
+- [FIXED] Fix failure when issue is authored by a deleted user. !13807
+- [FIXED] Reverts changes made to signin_enabled. !13956
+- [FIXED] Fix Merge when pipeline succeeds button dropdown caret icon horizontal alignment.
+- [FIXED] Fixed diff changes bar buttons from showing/hiding whilst scrolling.
+- [FIXED] Fix events error importing GitLab projects.
+- [FIXED] Fix pipeline trigger via API fails with 500 Internal Server Error in 9.5.
+- [FIXED] Fixed fly-out nav flashing in & out.
+- [FIXED] Remove closing external issues by reference error.
+- [FIXED] Re-allow appearances.description_html to be NULL.
+- [CHANGED] Update and fix resolvable note icons for easier recognition.
+- [OTHER] Eager load head pipeline projects for MRs index.
+- [OTHER] Instrument MergeRequest#fetch_ref.
+- [OTHER] Instrument MergeRequest#ensure_ref_fetched.
+
 ## 9.5.2 (2017-08-28)
 
 - [FIXED] Fix signing in using LDAP when attribute mapping uses simple strings instead of arrays.

GITALY_SERVER_VERSION

-0.33.0
+0.35.0

Gemfile

@@ -289,7 +289,7 @@ group :metrics do
   gem 'influxdb', '~> 0.2', require: false
 
   # Prometheus
-  gem 'prometheus-client-mmap', '~>0.7.0.beta11'
+  gem 'prometheus-client-mmap', '~>0.7.0.beta14'
   gem 'raindrops', '~> 0.18'
 end
 

Gemfile.lock

@@ -610,7 +610,12 @@ GEM
     premailer-rails (1.9.7)
       actionmailer (>= 3, < 6)
       premailer (~> 1.7, >= 1.7.9)
-    prometheus-client-mmap (0.7.0.beta11)
+    proc_to_ast (0.1.0)
+      coderay
+      parser
+      unparser
+    procto (0.0.3)
+    prometheus-client-mmap (0.7.0.beta14)
       mmap2 (~> 2.2, >= 2.2.7)
     pry (0.10.4)
       coderay (~> 1.1.0)
@@ -1066,7 +1071,7 @@ DEPENDENCIES
   pg (~> 0.18.2)
   poltergeist (~> 1.9.0)
   premailer-rails (~> 1.9.7)
-  prometheus-client-mmap (~> 0.7.0.beta11)
+  prometheus-client-mmap (~> 0.7.0.beta14)
   pry-byebug (~> 3.4.1)
   pry-rails (~> 0.3.4)
   rack-attack (~> 4.4.1)

VERSION

-9.5.2
+9.5.3

app/assets/javascripts/fly_out_nav.js

 import bp from './breakpoints';
 
-let headerHeight = 50;
+const HIDE_INTERVAL_TIMEOUT = 300;
+const IS_OVER_CLASS = 'is-over';
+const IS_ABOVE_CLASS = 'is-above';
+const IS_SHOWING_FLY_OUT_CLASS = 'is-showing-fly-out';
+let currentOpenMenu = null;
+let menuCornerLocs;
+let timeoutId;
 let sidebar;
 
+export const mousePos = [];
+
 export const setSidebar = (el) => { sidebar = el; };
+export const getOpenMenu = () => currentOpenMenu;
+export const setOpenMenu = (menu = null) => { currentOpenMenu = menu; };
+
+export const slope = (a, b) => (b.y - a.y) / (b.x - a.x);
+
+let headerHeight = 50;
 
 export const getHeaderHeight = () => headerHeight;
 
@@ -14,8 +28,28 @@ export const canShowActiveSubItems = (el) => {
 
   return true;
 };
+
 export const canShowSubItems = () => bp.getBreakpointSize() === 'sm' || bp.getBreakpointSize() === 'md' || bp.getBreakpointSize() === 'lg';
 
+export const getHideSubItemsInterval = () => {
+  if (!currentOpenMenu) return 0;
+
+  const currentMousePos = mousePos[mousePos.length - 1];
+  const prevMousePos = mousePos[0];
+  const currentMousePosY = currentMousePos.y;
+  const [menuTop, menuBottom] = menuCornerLocs;
+
+  if (currentMousePosY < menuTop.y ||
+    currentMousePosY > menuBottom.y) return 0;
+
+  if (slope(prevMousePos, menuBottom) < slope(currentMousePos, menuBottom) &&
+    slope(prevMousePos, menuTop) > slope(currentMousePos, menuTop)) {
+    return HIDE_INTERVAL_TIMEOUT;
+  }
+
+  return 0;
+};
+
 export const calculateTop = (boundingRect, outerHeight) => {
   const windowHeight = window.innerHeight;
   const bottomOverflow = windowHeight - (boundingRect.top + outerHeight);
@@ -24,51 +58,125 @@ export const calculateTop = (boundingRect, outerHeight) => {
     boundingRect.top;
 };
 
-export const showSubLevelItems = (el) => {
-  const subItems = el.querySelector('.sidebar-sub-level-items');
+export const hideMenu = (el) => {
+  if (!el) return;
 
-  if (!subItems || !canShowSubItems() || !canShowActiveSubItems(el)) return;
+  const parentEl = el.parentNode;
 
-  subItems.style.display = 'block';
-  el.classList.add('is-showing-fly-out');
-  el.classList.add('is-over');
+  el.style.display = ''; // eslint-disable-line no-param-reassign
+  el.style.transform = ''; // eslint-disable-line no-param-reassign
+  el.classList.remove(IS_ABOVE_CLASS);
+  parentEl.classList.remove(IS_OVER_CLASS);
+  parentEl.classList.remove(IS_SHOWING_FLY_OUT_CLASS);
+
+  setOpenMenu();
+};
 
+export const moveSubItemsToPosition = (el, subItems) => {
   const boundingRect = el.getBoundingClientRect();
   const top = calculateTop(boundingRect, subItems.offsetHeight);
   const isAbove = top < boundingRect.top;
 
   subItems.classList.add('fly-out-list');
-  subItems.style.transform = `translate3d(0, ${Math.floor(top) - headerHeight}px, 0)`;
+  subItems.style.transform = `translate3d(0, ${Math.floor(top) - headerHeight}px, 0)`; // eslint-disable-line no-param-reassign
+
+  const subItemsRect = subItems.getBoundingClientRect();
+
+  menuCornerLocs = [
+    {
+      x: subItemsRect.left, // left position of the sub items
+      y: subItemsRect.top, // top position of the sub items
+    },
+    {
+      x: subItemsRect.left, // left position of the sub items
+      y: subItemsRect.top + subItemsRect.height, // bottom position of the sub items
+    },
+  ];
 
   if (isAbove) {
-    subItems.classList.add('is-above');
+    subItems.classList.add(IS_ABOVE_CLASS);
   }
 };
 
-export const hideSubLevelItems = (el) => {
+export const showSubLevelItems = (el) => {
   const subItems = el.querySelector('.sidebar-sub-level-items');
 
-  if (!subItems || !canShowSubItems() || !canShowActiveSubItems(el)) return;
+  if (!canShowSubItems() || !canShowActiveSubItems(el)) return;
 
-  el.classList.remove('is-showing-fly-out');
-  el.classList.remove('is-over');
-  subItems.style.display = '';
-  subItems.style.transform = '';
-  subItems.classList.remove('is-above');
+  el.classList.add(IS_OVER_CLASS);
+
+  if (!subItems) return;
+
+  subItems.style.display = 'block';
+  el.classList.add(IS_SHOWING_FLY_OUT_CLASS);
+
+  setOpenMenu(subItems);
+  moveSubItemsToPosition(el, subItems);
 };
 
-export default () => {
-  const items = [...document.querySelectorAll('.sidebar-top-level-items > li')]
-    .filter(el => el.querySelector('.sidebar-sub-level-items'));
+export const mouseEnterTopItems = (el) => {
+  clearTimeout(timeoutId);
+
+  timeoutId = setTimeout(() => {
+    if (currentOpenMenu) hideMenu(currentOpenMenu);
 
+    showSubLevelItems(el);
+  }, getHideSubItemsInterval());
+};
+
+export const mouseLeaveTopItem = (el) => {
+  const subItems = el.querySelector('.sidebar-sub-level-items');
+
+  if (!canShowSubItems() || !canShowActiveSubItems(el) ||
+    (subItems && subItems === currentOpenMenu)) return;
+
+  el.classList.remove(IS_OVER_CLASS);
+};
+
+export const documentMouseMove = (e) => {
+  mousePos.push({
+    x: e.clientX,
+    y: e.clientY,
+  });
+
+  if (mousePos.length > 6) mousePos.shift();
+};
+
+export const subItemsMouseLeave = (relatedTarget) => {
+  clearTimeout(timeoutId);
+
+  if (!relatedTarget.closest(`.${IS_OVER_CLASS}`)) {
+    hideMenu(currentOpenMenu);
+  }
+};
+
+export default () => {
   sidebar = document.querySelector('.nav-sidebar');
 
-  if (sidebar) {
-    headerHeight = sidebar.offsetTop;
+  if (!sidebar) return;
 
-    items.forEach((el) => {
-      el.addEventListener('mouseenter', e => showSubLevelItems(e.currentTarget));
-      el.addEventListener('mouseleave', e => hideSubLevelItems(e.currentTarget));
+  const items = [...sidebar.querySelectorAll('.sidebar-top-level-items > li')];
+
+  sidebar.querySelector('.sidebar-top-level-items').addEventListener('mouseleave', () => {
+    clearTimeout(timeoutId);
+
+    timeoutId = setTimeout(() => {
+      if (currentOpenMenu) hideMenu(currentOpenMenu);
+    }, getHideSubItemsInterval());
   });
+
+  headerHeight = document.querySelector('.nav-sidebar').offsetTop;
+
+  items.forEach((el) => {
+    const subItems = el.querySelector('.sidebar-sub-level-items');
+
+    if (subItems) {
+      subItems.addEventListener('mouseleave', e => subItemsMouseLeave(e.relatedTarget));
     }
+
+    el.addEventListener('mouseenter', e => mouseEnterTopItems(e.currentTarget));
+    el.addEventListener('mouseleave', e => mouseLeaveTopItem(e.currentTarget));
+  });
+
+  document.addEventListener('mousemove', documentMouseMove);
 };

app/assets/javascripts/lib/utils/sticky.js

 export const isSticky = (el, scrollY, stickyTop) => {
-  const top = el.offsetTop - scrollY;
+  const top = Math.floor(el.offsetTop - scrollY);
 
   if (top <= stickyTop) {
     el.classList.add('is-stuck');

app/assets/javascripts/merge_request_tabs.js

@@ -253,6 +253,7 @@ import bp from './breakpoints';
 
     loadDiff(source) {
       if (this.diffsLoaded) {
+        document.dispatchEvent(new CustomEvent('scroll'));
         return;
       }
 

app/assets/javascripts/new_sidebar.js

@@ -47,7 +47,6 @@ export default class NewNavSidebar {
 
     if (this.$sidebar.length) {
       this.$sidebar.toggleClass('sidebar-icons-only', collapsed);
-      this.$page.toggleClass('page-with-new-sidebar', !collapsed);
       this.$page.toggleClass('page-with-icon-sidebar', breakpoint === 'sm' ? true : collapsed);
     }
     NewNavSidebar.setCollapsedCookie(collapsed);

app/assets/javascripts/project_select_combo_button.js

@@ -57,10 +57,10 @@ export default class ProjectSelectComboButton {
   setNewItemBtnAttributes(project) {
     if (project) {
       this.newItemBtn.attr('href', project.url);
-      this.newItemBtn.text(`${this.newItemBtnBaseText} in ${project.name}`);
+      this.newItemBtn.text(`New ${this.deriveItemTypeFromLabel()} in ${project.name}`);
       this.newItemBtn.enable();
     } else {
-      this.newItemBtn.text(`Select project to create ${this.itemType}`);
+      this.newItemBtn.text(`Select project to create ${this.deriveItemTypeFromLabel()}`);
       this.newItemBtn.disable();
     }
   }

app/assets/stylesheets/framework/dropdowns.scss

@@ -189,7 +189,7 @@
   width: auto;
   top: 100%;
   left: 0;
-  z-index: 9;
+  z-index: 200;
   min-width: 240px;
   max-width: 500px;
   margin-top: 2px;

app/assets/stylesheets/framework/typography.scss

@@ -26,6 +26,7 @@
     min-width: inherit;
     min-height: inherit;
     background-color: inherit;
+    max-width: 100%;
   }
 
   p a:not(.no-attachment-icon) img {

app/assets/stylesheets/framework/variables.scss

@@ -118,6 +118,7 @@ $gl-text-color-quaternary: #d6d6d6;
 $gl-text-color-inverted: rgba(255, 255, 255, 1.0);
 $gl-text-color-secondary-inverted: rgba(255, 255, 255, .85);
 $gl-text-green: $green-600;
+$gl-text-green-hover: $green-700;
 $gl-text-red: $red-500;
 $gl-text-orange: $orange-600;
 $gl-link-color: $blue-600;

app/assets/stylesheets/new_sidebar.scss

@@ -70,7 +70,8 @@ $new-sidebar-collapsed-width: 50px;
     background-color: $white-light;
   }
 
-  .sidebar-context-title {
+  .project-title,
+  .group-title {
     overflow: hidden;
     text-overflow: ellipsis;
   }
@@ -96,29 +97,21 @@ $new-sidebar-collapsed-width: 50px;
   top: $header-height;
   bottom: 0;
   left: 0;
+  overflow: auto;
   background-color: $gray-normal;
   box-shadow: inset -2px 0 0 $border-color;
-  transform: translate3d(0, 0, 0);
 
   &.sidebar-icons-only {
     width: $new-sidebar-collapsed-width;
     overflow-x: hidden;
 
-    .nav-sidebar-inner-scroll {
-      overflow-x: hidden;
-    }
-
     .badge,
-    .sidebar-context-title {
+    .project-title {
       display: none;
     }
 
     .nav-item-name {
-      display: none;
-    }
-
-    .sidebar-top-level-items > li > a {
-      min-height: 44px;
+      opacity: 0;
     }
   }
 
@@ -183,12 +176,6 @@ $new-sidebar-collapsed-width: 50px;
   }
 }
 
-.nav-sidebar-inner-scroll {
-  height: 100%;
-  width: 100%;
-  overflow: auto;
-}
-
 .with-performance-bar .nav-sidebar {
   top: $header-height + $performance-bar-height;
 }
@@ -263,32 +250,13 @@ $new-sidebar-collapsed-width: 50px;
             position: absolute;
             top: -30px;
             bottom: -30px;
-            left: 0;
+            left: -10px;
             right: -30px;
             z-index: -1;
           }
 
-          &::after {
-            content: "";
-            position: absolute;
-            top: 44px;
-            left: -30px;
-            right: 35px;
-            bottom: 0;
-            height: 100%;
-            max-height: 150px;
-            z-index: -1;
-            transform: skew(33deg);
-          }
-
           &.is-above {
             margin-top: 1px;
-
-            &::after {
-              top: auto;
-              bottom: 44px;
-              transform: skew(-30deg);
-            }
           }
 
           > .active {
@@ -335,8 +303,7 @@ $new-sidebar-collapsed-width: 50px;
       }
     }
 
-    &:not(.active):hover > a,
-    > a:hover,
+    &.active > a:hover,
     &.is-over > a {
       background-color: $white-light;
     }

app/assets/stylesheets/pages/merge_requests.scss

@@ -291,6 +291,7 @@
 
     .dropdown-toggle {
       .fa {
+        margin-left: 0;
         color: inherit;
       }
     }

app/assets/stylesheets/pages/notes.scss

@@ -766,17 +766,25 @@ ul.notes {
   background-color: transparent;
   border: none;
   outline: 0;
+  transition: color $general-hover-transition-duration $general-hover-transition-curve;
 
   &.is-disabled {
     cursor: default;
   }
 
-  &:not(.is-disabled):hover,
+  &:not(.is-disabled) {
+    &:hover,
+    &:focus {
+      color: $gl-text-green;
+    }
+  }
+
   &.is-active {
     color: $gl-text-green;
 
-    svg {
-      fill: $gl-text-green;
+    &:hover,
+    &:focus {
+      color: $gl-text-green-hover;
     }
   }
 

app/controllers/application_controller.rb

@@ -202,7 +202,7 @@ class ApplicationController < ActionController::Base
   end
 
   def check_password_expiration
-    if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now && current_user.allow_password_authentication?
+    if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now && !current_user.ldap_user?
       return redirect_to new_profile_password_path
     end
   end

app/controllers/concerns/issuable_collections.rb

@@ -15,7 +15,17 @@ module IssuableCollections
   end
 
   def merge_requests_collection
-    merge_requests_finder.execute.preload(:source_project, :target_project, :author, :assignee, :labels, :milestone, :head_pipeline, target_project: :namespace, merge_request_diff: :merge_request_diff_commits)
+    merge_requests_finder.execute.preload(
+      :source_project,
+      :target_project,
+      :author,
+      :assignee,
+      :labels,
+      :milestone,
+      head_pipeline: :project,
+      target_project: :namespace,
+      merge_request_diff: :merge_request_diff_commits
+    )
   end
 
   def issues_finder

app/controllers/passwords_controller.rb

 class PasswordsController < Devise::PasswordsController
-  include Gitlab::CurrentSettings
-
   before_action :resource_from_email, only: [:create]
-  before_action :check_password_authentication_available, only: [:create]
+  before_action :prevent_ldap_reset, only: [:create]
   before_action :throttle_reset,      only: [:create]
 
   def edit
@@ -40,11 +38,11 @@ class PasswordsController < Devise::PasswordsController
     self.resource = resource_class.find_by_email(email)
   end
 
-  def check_password_authentication_available
-    return if current_application_settings.password_authentication_enabled? && (resource.nil? || resource.allow_password_authentication?)
+  def prevent_ldap_reset
+    return unless resource&.ldap_user?
 
     redirect_to after_sending_reset_password_instructions_path_for(resource_name),
-      alert: "Password authentication is unavailable."
+      alert: "Cannot reset password for LDAP user."
   end
 
   def throttle_reset

app/controllers/profiles/passwords_controller.rb

@@ -77,7 +77,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController
   end
 
   def authorize_change_password!
-    render_404 unless @user.allow_password_authentication?
+    render_404 if @user.ldap_user?
   end
 
   def user_params

app/models/commit.rb

@@ -55,7 +55,8 @@ class Commit
     end
 
     def from_hash(hash, project)
-      new(Gitlab::Git::Commit.new(hash), project)
+      raw_commit = Gitlab::Git::Commit.new(project.repository.raw, hash)
+      new(raw_commit, project)
     end
 
     def valid_hash?(key)

app/models/merge_request.rb

@@ -598,6 +598,8 @@ class MergeRequest < ActiveRecord::Base
       self.merge_requests_closing_issues.delete_all
 
       closes_issues(current_user).each do |issue|
+        next if issue.is_a?(ExternalIssue)
+
         self.merge_requests_closing_issues.create!(issue: issue)
       end
     end

app/models/merge_request_diff.rb

@@ -282,9 +282,7 @@ class MergeRequestDiff < ActiveRecord::Base
   def load_commits
     commits = st_commits.presence || merge_request_diff_commits
 
-    commits.map do |commit|
-      Commit.new(Gitlab::Git::Commit.new(commit.to_hash), merge_request.source_project)
-    end
+    commits.map { |commit| Commit.from_hash(commit.to_hash, project) }
   end
 
   def save_diffs

app/models/notification_recipient.rb

@@ -19,46 +19,45 @@ class NotificationRecipient
     @notification_setting ||= find_notification_setting
   end
 
-  def raw_notification_level
-    notification_setting&.level&.to_sym
-  end
-
   def notification_level
-    # custom is treated the same as watch if it's enabled - otherwise it's
-    # set to :custom, meaning to send exactly when our type is :participating
-    # or :mention.
-    @notification_level ||=
-      case raw_notification_level
-      when :custom
-        if @custom_action && notification_setting&.event_enabled?(@custom_action)
-          :watch
-        else
-          :custom
-        end
-      else
-        raw_notification_level
-      end
+    @notification_level ||= notification_setting&.level&.to_sym
   end
 
   def notifiable?
     return false unless has_access?
     return false if own_activity?
 
-    return true if @type == :subscription
-
-    return false if notification_level.nil? || notification_level == :disabled
-
-    return %i[participating mention].include?(@type) if notification_level == :custom
+    # even users with :disabled notifications receive manual subscriptions
+    return !unsubscribed? if @type == :subscription
 
-    return false if %i[watch participating].include?(notification_level) && excluded_watcher_action?
-
-    return false unless NotificationSetting.levels[notification_level] <= NotificationSetting.levels[@type]
+    return false unless suitable_notification_level?
 
+    # check this last because it's expensive
+    # nobody should receive notifications if they've specifically unsubscribed
     return false if unsubscribed?
 
     true
   end
 
+  def suitable_notification_level?
+    case notification_level
+    when :disabled, nil
+      false
+    when :custom
+      custom_enabled? || %i[participating mention].include?(@type)
+    when :watch, :participating
+      !excluded_watcher_action?
+    when :mention
+      @type == :mention
+    else
+      false
+    end
+  end
+
+  def custom_enabled?
+    @custom_action && notification_setting&.event_enabled?(@custom_action)
+  end
+
   def unsubscribed?
     return false unless @target
     return false unless @target.respond_to?(:subscriptions)
@@ -88,7 +87,7 @@ class NotificationRecipient
 
   def excluded_watcher_action?
     return false unless @custom_action
-    return false if raw_notification_level == :custom
+    return false if notification_level == :custom
 
     NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(@custom_action)
   end

app/models/repository.rb

@@ -64,6 +64,8 @@ class Repository
     @raw_repository ||= initialize_raw_repository
   end
 
+  alias_method :raw, :raw_repository
+
   # Return absolute path to repository
   def path_to_repo
     @path_to_repo ||= File.expand_path(

app/models/user.rb

@@ -598,7 +598,7 @@ class User < ActiveRecord::Base
   end
 
   def require_personal_access_token_creation_for_git_auth?
-    return false if allow_password_authentication? || ldap_user?
+    return false if current_application_settings.password_authentication_enabled? || ldap_user?
 
     PersonalAccessTokensFinder.new(user: self, impersonation: false, state: 'active').execute.none?
   end
@@ -834,7 +834,12 @@ class User < ActiveRecord::Base
     create_namespace!(path: username, name: username) unless namespace
 
     if username_changed?
-      namespace.update_attributes(path: username, name: username)
+      unless namespace.update_attributes(path: username, name: username)
+        namespace.errors.each do |attribute, message|
+          self.errors.add(:"namespace_#{attribute}", message)
+        end
+        raise ActiveRecord::RecordInvalid.new(namespace)
+      end
     end
   end
 

app/services/ci/create_pipeline_service.rb

@@ -15,7 +15,7 @@ module Ci
         pipeline_schedule: schedule
       )
 
-      result = validate(current_user || trigger_request.trigger.owner,
+      result = validate(current_user,
                         ignore_skip_ci: ignore_skip_ci,
                         save_on_errors: save_on_errors)
 

app/services/notification_recipient_service.rb

@@ -95,7 +95,7 @@ module NotificationRecipientService
       def add_participants(user)
         return unless target.respond_to?(:participants)
 
-        self << [target.participants(user), :watch]
+        self << [target.participants(user), :participating]
       end
 
       # Get project/group users with CUSTOM notification level

app/services/test_hooks/system_service.rb

@@ -2,47 +2,16 @@ module TestHooks
   class SystemService < TestHooks::BaseService
     private
 
-    def project
-      @project ||= begin
-        project = Project.first
-
-        throw(:validation_error, 'Ensure that at least one project exists.') unless project
-
-        project
-      end
-    end
-
     def push_events_data
-      if project.empty_repo?
-        throw(:validation_error, "Ensure project \"#{project.human_name}\" has commits.")
-      end
-
-      Gitlab::DataBuilder::Push.build_sample(project, current_user)
+      Gitlab::DataBuilder::Push.sample_data
     end
 
     def tag_push_events_data
-      if project.repository.tags.empty?
-        throw(:validation_error, "Ensure project \"#{project.human_name}\" has tags.")
-      end
-
-      Gitlab::DataBuilder::Push.build_sample(project, current_user)
+      Gitlab::DataBuilder::Push.sample_data
     end
 
     def repository_update_events_data
-      commit = project.commit
-      ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{project.default_branch}"
-
-      unless commit
-        throw(:validation_error, "Ensure project \"#{project.human_name}\" has commits.")
-      end
-
-      change = Gitlab::DataBuilder::Repository.single_change(
-        commit.parent_id || Gitlab::Git::BLANK_SHA,
-        commit.id,
-        ref
-      )
-
-      Gitlab::DataBuilder::Repository.update(project, current_user, [change], [ref])
+      Gitlab::DataBuilder::Repository.sample_data
     end
   end
 end

app/views/admin/application_settings/_form.html.haml

@@ -147,7 +147,7 @@
         .checkbox
           = f.label :password_authentication_enabled do
             = f.check_box :password_authentication_enabled
-            Password authentication enabled
+            Sign-in enabled
     - if omniauth_enabled? && button_based_providers.any?
       .form-group
         = f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'control-label col-sm-2'

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

@@ -29,7 +29,7 @@
       = link_to profile_emails_path, title: 'Emails' do
         %span
           Emails
-    - if current_user.allow_password_authentication?
+    - unless current_user.ldap_user?
       = nav_link(controller: :passwords) do
         = link_to edit_profile_password_path, title: 'Password' do
           %span

app/views/projects/issues/show.html.haml

@@ -32,7 +32,8 @@
         %ul
           - if can_update_issue
             %li= link_to 'Edit', edit_project_issue_path(@project, @issue)
-          - unless current_user == @issue.author
+            / TODO: simplify condition back #36860
+          - if @issue.author && current_user != @issue.author
             %li= link_to 'Report abuse', new_abuse_report_path(user_id: @issue.author.id, ref_url: issue_url(@issue))
           - if can_update_issue
             %li= link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, format: 'json'), class: "btn-close #{issue_button_visibility(@issue, true)}", title: 'Close issue'

app/views/projects/notes/_actions.html.haml

@@ -26,8 +26,12 @@
           ":title" => "buttonText",
           ":ref" => "'button'" }
 
-        = icon('spin spinner', 'v-show' => 'loading', class: 'loading', 'aria-hidden' => 'true', 'aria-label' => 'Loading')
-        %div{ 'v-show' => '!loading' }= render 'shared/icons/icon_status_success.svg'
+        = icon('spin spinner', 'v-if' => 'loading', class: 'loading', 'aria-hidden' => 'true', 'aria-label' => 'Loading')
+        %div{ 'v-else' => '' }
+          %template{ 'v-if' => 'isResolved' }
+            = render 'shared/icons/icon_status_success_solid.svg'
+          %template{ 'v-else' => '' }
+            = render 'shared/icons/icon_status_success.svg'
 
 - if current_user
   - if note.emoji_awardable?

app/views/shared/icons/_icon_status_success.svg

-<svg width="14" height="14" viewBox="0 0 14 14" xmlns="http://www.w3.org/2000/svg"><g fill-rule="evenodd"><path d="M0 7a7 7 0 1 1 14 0A7 7 0 0 1 0 7z"/><path d="M13 7A6 6 0 1 0 1 7a6 6 0 0 0 12 0z" fill="#FFF"/><path d="M6.278 7.697L5.045 6.464a.296.296 0 0 0-.42-.002l-.613.614a.298.298 0 0 0 .002.42l1.91 1.909a.5.5 0 0 0 .703.005l.265-.265L9.997 6.04a.291.291 0 0 0-.009-.408l-.614-.614a.29.29 0 0 0-.408-.009L6.278 7.697z"/></g></svg>
+<svg width="14" height="14" viewBox="0 0 14 14" xmlns="http://www.w3.org/2000/svg"><path d="M0 7a7 7 0 1 1 14 0A7 7 0 0 1 0 7z M13 7A6 6 0 1 0 1 7a6 6 0 0 0 12 0z" fill-rule="evenodd"/><path d="M6.278 7.697L5.045 6.464a.296.296 0 0 0-.42-.002l-.613.614a.298.298 0 0 0 .002.42l1.91 1.909a.5.5 0 0 0 .703.005l.265-.265L9.997 6.04a.291.291 0 0 0-.009-.408l-.614-.614a.29.29 0 0 0-.408-.009L6.278 7.697z"/></svg>

app/views/shared/icons/_icon_status_success_solid.svg

+<svg width="14" height="14" viewBox="0 0 14 14" xmlns="http://www.w3.org/2000/svg"><path d="M0 7a7 7 0 1 1 14 0A7 7 0 0 1 0 7z M6.278 7.697L5.045 6.464a.296.296 0 0 0-.42-.002l-.613.614a.298.298 0 0 0 .002.42l1.91 1.909a.5.5 0 0 0 .703.005l.265-.265L9.997 6.04a.291.291 0 0 0-.009-.408l-.614-.614a.29.29 0 0 0-.408-.009L6.278 7.697z" fill-rule="evenodd"/></svg>

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

@@ -9,6 +9,7 @@
             class: "hidden-xs hidden-sm btn btn-grouped btn-reopen #{issuable_button_visibility(issuable, false)}", title: "Reopen #{display_issuable_type}"
 - elsif can_update && !is_current_user
   = render 'shared/issuable/close_reopen_report_toggle', issuable: issuable
-- else
+- elsif issuable.author
+  / TODO: change back to else #36860
   = link_to 'Report abuse', new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)),
             class: 'hidden-xs hidden-sm btn btn-grouped btn-close-color', title: 'Report abuse'

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

@@ -37,6 +37,8 @@
 
       %li.divider.droplab-item-ignore
 
+      / TODO: remove condition #36860
+      - if issuable.author
         %li.report-item{ data: { text: 'Report abuse', url: new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)),
                          button_class: "#{button_class} btn-close-color", toggle_class: "#{toggle_class} btn-close-color", method: '' } }
           %button.btn.btn-transparent

config/application.rb

@@ -51,31 +51,24 @@ module Gitlab
     # Configure sensitive parameters which will be filtered from the log file.
     #
     # Parameters filtered:
-    # - Password (:password, :password_confirmation)
-    # - Private tokens
+    # - Any parameter ending with `_token`
+    # - Any parameter containing `password`
+    # - Any parameter containing `secret`
     # - Two-factor tokens (:otp_attempt)
     # - Repo/Project Import URLs (:import_url)
     # - Build variables (:variables)
     # - GitLab Pages SSL cert/key info (:certificate, :encrypted_key)
     # - Webhook URLs (:hook)
-    # - GitLab-shell secret token (:secret_token)
     # - Sentry DSN (:sentry_dsn)
     # - Deploy keys (:key)
+    config.filter_parameters += [/_token$/, /password/, /secret/]
     config.filter_parameters += %i(
-      authentication_token
       certificate
       encrypted_key
       hook
       import_url
-      incoming_email_token
-      rss_token
       key
       otp_attempt
-      password
-      password_confirmation
-      private_token
-      runners_token
-      secret_token
       sentry_dsn
       variables
     )

config/initializers/7_prometheus_metrics.rb

 require 'prometheus/client'
+require 'prometheus/client/support/unicorn'
 
 Prometheus::Client.configure do |config|
   config.logger = Rails.logger
@@ -9,6 +10,8 @@ Prometheus::Client.configure do |config|
   if Rails.env.development? || Rails.env.test?
     config.multiprocess_files_dir ||= Rails.root.join('tmp/prometheus_multiproc_dir')
   end
+
+  config.pid_provider = Prometheus::Client::Support::Unicorn.method(:worker_pid_provider)
 end
 
 Sidekiq.configure_server do |config|

config/initializers/8_metrics.rb

@@ -119,6 +119,10 @@ def instrument_classes(instrumentation)
 
   # Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/30224#note_32306159
   instrumentation.instrument_instance_method(MergeRequestDiff, :load_commits)
+
+  # Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/36061
+  instrumentation.instrument_instance_method(MergeRequest, :ensure_ref_fetched)
+  instrumentation.instrument_instance_method(MergeRequest, :fetch_ref)
 end
 # rubocop:enable Metrics/AbcSize
 

db/migrate/20170809142252_cleanup_appearances_schema.rb

@@ -7,7 +7,7 @@ class CleanupAppearancesSchema < ActiveRecord::Migration
   # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  NOT_NULL_COLUMNS = %i[title description description_html created_at updated_at]
+  NOT_NULL_COLUMNS = %i[title description created_at updated_at]
 
   TIME_COLUMNS = %i[created_at updated_at]
 

db/migrate/20170824162758_allow_appearances_description_html_null.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AllowAppearancesDescriptionHtmlNull < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def up
+    change_column_null :appearances, :description_html, true
+  end
+
+  def down
+    # This column should not have a `NOT NULL` class, so we don't want to revert
+    # back to re-adding it.
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170820100558) do
+ActiveRecord::Schema.define(version: 20170824162758) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -34,7 +34,7 @@ ActiveRecord::Schema.define(version: 20170820100558) do
     t.string "logo"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
-    t.text "description_html", null: false
+    t.text "description_html"
     t.integer "cached_markdown_version"
   end
 

doc/raketasks/backup_restore.md

@@ -144,9 +144,8 @@ gitlab_rails['backup_upload_connection'] = {
   'region' => 'eu-west-1',
   'aws_access_key_id' => 'AKIAKIAKI',
   'aws_secret_access_key' => 'secret123'
-  # If using an IAM Profile, leave aws_access_key_id & aws_secret_access_key empty
-  # ie. 'aws_access_key_id' => '',
-  # 'use_iam_profile' => 'true'
+  # If using an IAM Profile, don't configure aws_access_key_id & aws_secret_access_key
+  # 'use_iam_profile' => true
 }
 gitlab_rails['backup_upload_remote_directory'] = 'my.s3.bucket'
 ```

lib/gitlab/auth.rb

@@ -48,10 +48,6 @@ module Gitlab
         # Avoid resource intensive login checks if password is not provided
         return unless password.present?
 
-        # Nothing to do here if internal auth is disabled and LDAP is
-        # not configured
-        return unless current_application_settings.password_authentication_enabled? || Gitlab::LDAP::Config.enabled?
-
         Gitlab::Auth::UniqueIpsLimiter.limit_user! do
           user = User.by_login(login)
 

lib/gitlab/cycle_analytics/plan_event_fetcher.rb

@@ -54,7 +54,7 @@ module Gitlab
       end
 
       def serialize_commit(event, commit, query)
-        commit = Commit.new(Gitlab::Git::Commit.new(commit.to_hash), @project)
+        commit = Commit.from_hash(commit.to_hash, @project)
 
         AnalyticsCommitSerializer.new(project: @project, total_time: event['total_time']).represent(commit)
       end

lib/gitlab/data_builder/push.rb

@@ -3,6 +3,35 @@ module Gitlab
     module Push
       extend self
 
+      SAMPLE_DATA =
+        {
+          object_kind: "push",
+          event_name: "push",
+          before: "95790bf891e76fee5e1747ab589903a6a1f80f22",
+          after: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
+          ref: "refs/heads/master",
+          checkout_sha: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
+          message: "Hello World",
+          user_id: 4,
+          user_name: "John Smith",
+          user_email: "john@example.com",
+          user_avatar: "https://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=8://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=80",
+          project_id: 15,
+          commits: [
+            {
+              id: "c5feabde2d8cd023215af4d2ceeb7a64839fc428",
+              message: "Add simple search to projects in public area",
+              timestamp: "2013-05-13T18:18:08+00:00",
+              url: "https://test.example.com/gitlab/gitlabhq/commit/c5feabde2d8cd023215af4d2ceeb7a64839fc428",
+              author: {
+                name: "Test User",
+                email: "test@example.com"
+              }
+            }
+          ],
+          total_commits_count: 1
+        }.freeze
+
       # Produce a hash of post-receive data
       #
       # data = {
@@ -74,6 +103,10 @@ module Gitlab
         build(project, user, commits.last&.id, commits.first&.id, ref, commits)
       end
 
+      def sample_data
+        SAMPLE_DATA
+      end
+
       private
 
       def checkout_sha(repository, newrev, ref)

lib/gitlab/data_builder/repository.rb

@@ -3,6 +3,23 @@ module Gitlab
     module Repository
       extend self
 
+      SAMPLE_DATA = {
+        event_name: 'repository_update',
+        user_id: 10,
+        user_name: 'john.doe',
+        user_email: 'test@example.com',
+        user_avatar: 'http://example.com/avatar/user.png',
+        project_id: 40,
+        changes: [
+          {
+            before: "8205ea8d81ce0c6b90fbe8280d118cc9fdad6130",
+            after: "4045ea7a3df38697b3730a20fb73c8bed8a3e69e",
+            ref: "refs/heads/master"
+          }
+        ],
+        "refs": ["refs/heads/master"]
+      }.freeze
+
       # Produce a hash of post-receive data
       def update(project, user, changes, refs)
         {
@@ -30,6 +47,10 @@ module Gitlab
           ref: ref
         }
       end
+
+      def sample_data
+        SAMPLE_DATA
+      end
     end
   end
 end

lib/gitlab/git/blame.rb

@@ -16,7 +16,7 @@ module Gitlab
       def each
         @blames.each do |blame|
           yield(
-            Gitlab::Git::Commit.new(blame.commit),
+            Gitlab::Git::Commit.new(@repo, blame.commit),
             blame.line
           )
         end

lib/gitlab/git/commit.rb

@@ -51,7 +51,7 @@ module Gitlab
         # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/321
         def find(repo, commit_id = "HEAD")
           return commit_id if commit_id.is_a?(Gitlab::Git::Commit)
-          return decorate(commit_id) if commit_id.is_a?(Rugged::Commit)
+          return decorate(repo, commit_id) if commit_id.is_a?(Rugged::Commit)
 
           obj = if commit_id.is_a?(String)
                   repo.rev_parse_target(commit_id)
@@ -61,7 +61,7 @@ module Gitlab
 
           return nil unless obj.is_a?(Rugged::Commit)
 
-          decorate(obj)
+          decorate(repo, obj)
         rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError, Gitlab::Git::Repository::NoRepository
           nil
         end
@@ -102,7 +102,7 @@ module Gitlab
             if is_enabled
               repo.gitaly_commit_client.between(base, head)
             else
-              repo.rugged_commits_between(base, head).map { |c| decorate(c) }
+              repo.rugged_commits_between(base, head).map { |c| decorate(repo, c) }
             end
           end
         rescue Rugged::ReferenceError
@@ -169,7 +169,7 @@ module Gitlab
           offset = actual_options[:skip]
           limit = actual_options[:max_count]
           walker.each(offset: offset, limit: limit) do |commit|
-            commits.push(decorate(commit))
+            commits.push(decorate(repo, commit))
           end
 
           walker.reset
@@ -183,8 +183,8 @@ module Gitlab
           Gitlab::GitalyClient::CommitService.new(repo).find_all_commits(options)
         end
 
-        def decorate(commit, ref = nil)
-          Gitlab::Git::Commit.new(commit, ref)
+        def decorate(repository, commit, ref = nil)
+          Gitlab::Git::Commit.new(repository, commit, ref)
         end
 
         # Returns a diff object for the changes introduced by +rugged_commit+.
@@ -231,7 +231,7 @@ module Gitlab
         end
       end
 
-      def initialize(raw_commit, head = nil)
+      def initialize(repository, raw_commit, head = nil)
         raise "Nil as raw commit passed" unless raw_commit
 
         case raw_commit
@@ -239,12 +239,13 @@ module Gitlab
           init_from_hash(raw_commit)
         when Rugged::Commit
           init_from_rugged(raw_commit)
-        when Gitlab::GitalyClient::Commit
+        when Gitaly::GitCommit
           init_from_gitaly(raw_commit)
         else
           raise "Invalid raw commit type: #{raw_commit.class}"
         end
 
+        @repository = repository
         @head = head
       end
 
@@ -319,14 +320,7 @@ module Gitlab
       end
 
       def parents
-        case raw_commit
-        when Rugged::Commit
-          raw_commit.parents.map { |c| Gitlab::Git::Commit.new(c) }
-        when Gitlab::GitalyClient::Commit
-          parent_ids.map { |oid| self.class.find(raw_commit.repository, oid) }.compact
-        else
-          raise NotImplementedError, "commit source doesn't support #parents"
-        end
+        parent_ids.map { |oid| self.class.find(@repository, oid) }.compact
       end
 
       def stats

lib/gitlab/git/diff_collection.rb

@@ -28,7 +28,6 @@ module Gitlab
         @limits = self.class.collection_limits(options)
         @enforce_limits = !!options.fetch(:limits, true)
         @expanded = !!options.fetch(:expanded, true)
-        @from_gitaly = options.fetch(:from_gitaly, false)
 
         @line_count = 0
         @byte_count = 0
@@ -44,7 +43,7 @@ module Gitlab
         return if @iterator.nil?
 
         Gitlab::GitalyClient.migrate(:commit_raw_diffs) do |is_enabled|
-          if is_enabled && @from_gitaly
+          if is_enabled && @iterator.is_a?(Gitlab::GitalyClient::DiffStitcher)
             each_gitaly_patch(&block)
           else
             each_rugged_patch(&block)

lib/gitlab/git/repository.rb

@@ -321,7 +321,7 @@ module Gitlab
         options[:limit] ||= 0
         options[:offset] ||= 0
 
-        raw_log(options).map { |c| Commit.decorate(c) }
+        raw_log(options).map { |c| Commit.decorate(self, c) }
       end
 
       def count_commits(options)

lib/gitlab/gitaly_client/commit.rb

-module Gitlab
-  module GitalyClient
-    class Commit
-      attr_reader :repository, :gitaly_commit
-
-      delegate :id, :subject, :body, :author, :committer, :parent_ids, to: :gitaly_commit
-
-      def initialize(repository, gitaly_commit)
-        @repository = repository
-        @gitaly_commit = gitaly_commit
-      end
-    end
-  end
-end

lib/gitlab/gitaly_client/commit_service.rb

@@ -107,8 +107,7 @@ module Gitlab
         gitaly_commit = GitalyClient.call(@repository.storage, :commit_service, :last_commit_for_path, request).commit
         return unless gitaly_commit
 
-        commit = GitalyClient::Commit.new(@repository, gitaly_commit)
-        Gitlab::Git::Commit.new(commit)
+        Gitlab::Git::Commit.new(@repository, gitaly_commit)
       end
 
       def between(from, to)
@@ -170,7 +169,7 @@ module Gitlab
       private
 
       def commit_diff_request_params(commit, options = {})
-        parent_id = commit.parents[0]&.id || EMPTY_TREE_ID
+        parent_id = commit.parent_ids.first || EMPTY_TREE_ID
 
         {
           repository: @gitaly_repo,
@@ -183,8 +182,7 @@ module Gitlab
       def consume_commits_response(response)
         response.flat_map do |message|
           message.commits.map do |gitaly_commit|
-            commit = GitalyClient::Commit.new(@repository, gitaly_commit)
-            Gitlab::Git::Commit.new(commit)
+            Gitlab::Git::Commit.new(@repository, gitaly_commit)
           end
         end
       end

lib/gitlab/gitaly_client/ref_service.rb

@@ -16,8 +16,7 @@ module Gitlab
 
         response.flat_map do |message|
           message.branches.map do |branch|
-            gitaly_commit = GitalyClient::Commit.new(@repository, branch.target)
-            target_commit = Gitlab::Git::Commit.decorate(gitaly_commit)
+            target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target)
             Gitlab::Git::Branch.new(@repository, branch.name, branch.target.id, target_commit)
           end
         end
@@ -102,8 +101,7 @@ module Gitlab
         response.flat_map do |message|
           message.tags.map do |gitaly_tag|
             if gitaly_tag.target_commit.present?
-              commit = GitalyClient::Commit.new(@repository, gitaly_tag.target_commit)
-              gitaly_commit = Gitlab::Git::Commit.new(commit)
+              gitaly_commit = Gitlab::Git::Commit.decorate(@repository, gitaly_tag.target_commit)
             end
 
             Gitlab::Git::Tag.new(
@@ -141,7 +139,7 @@ module Gitlab
           committer_email: response.commit_committer.email.dup
         }
 
-        Gitlab::Git::Commit.decorate(hash)
+        Gitlab::Git::Commit.decorate(@repository, hash)
       end
     end
   end

lib/gitlab/import_export/import_export.yml

@@ -135,3 +135,7 @@ methods:
     - :diff_head_sha
   project:
     - :description_html
+  events:
+    - :action
+  push_event_payload:
+    - :action
\ No newline at end of file

locale/zh_TW/gitlab.po

@@ -1111,31 +1111,17 @@ msgstr "因該階段的資料不足而無法顯示相關資訊"
 msgid "Withdraw Access Request"
 msgstr "取消權限申請"
 
-msgid ""
-"You are going to remove %{group_name}.\n"
-"Removed groups CANNOT be restored!\n"
-"Are you ABSOLUTELY sure?"
-msgstr "即將要刪除 %{group_name}。\n"
-"被刪除的群組完全無法救回來喔！\n"
-"真的「100%確定」要這麼做嗎？"
+msgid "You are going to remove %{group_name}. Removed groups CANNOT be restored! Are you ABSOLUTELY sure?"
+msgstr "將要刪除 %{group_name}。被刪除的群組無法復原！真的「確定」要這麼做嗎？"
 
-msgid ""
-"You are going to remove %{project_name_with_namespace}.\n"
-"Removed project CANNOT be restored!\n"
-"Are you ABSOLUTELY sure?"
-msgstr "即將要刪除 %{project_name_with_namespace}。被刪除的專案完全無法救回來喔！真的「100%確定」要這麼做嗎？"
+msgid "You are going to remove %{project_name_with_namespace}. Removed project CANNOT be restored! Are you ABSOLUTELY sure?"
+msgstr "將要刪除 %{project_name_with_namespace}。被刪除的專案無法復原！真的「確定」要這麼做嗎？"
 
-msgid ""
-"You are going to remove the fork relationship to source project "
-"%{forked_from_project}. Are you ABSOLUTELY sure?"
-msgstr ""
-"將要刪除本分支專案與主幹的所有關聯 (fork relationship) 。 %{forked_from_project} "
-"真的「100%確定」要這麼做嗎？"
+msgid "You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?"
+msgstr "將要刪除本分支專案與主幹 %{forked_from_project} 的所有關聯。 真的「確定」要這麼做嗎？"
 
-msgid ""
-"You are going to transfer %{project_name_with_namespace} to another owner. "
-"Are you ABSOLUTELY sure?"
-msgstr "將要把 %{project_name_with_namespace} 的所有權轉移給另一個人。真的「100%確定」要這麼做嗎？"
+msgid "You are going to transfer %{project_name_with_namespace} to another owner. Are you ABSOLUTELY sure?"
+msgstr "將要把 %{project_name_with_namespace} 的所有權轉移給另一個人。真的「確定」要這麼做嗎？"
 
 msgid "You can only add files when you are on a branch"
 msgstr "只能在分支 (branch) 上建立檔案"

spec/controllers/application_controller_spec.rb

@@ -8,34 +8,43 @@ describe ApplicationController do
 
     it 'redirects if the user is over their password expiry' do
       user.password_expires_at = Time.new(2002)
+
       expect(user.ldap_user?).to be_falsey
       allow(controller).to receive(:current_user).and_return(user)
       expect(controller).to receive(:redirect_to)
       expect(controller).to receive(:new_profile_password_path)
+
       controller.send(:check_password_expiration)
     end
 
     it 'does not redirect if the user is under their password expiry' do
       user.password_expires_at = Time.now + 20010101
+
       expect(user.ldap_user?).to be_falsey
       allow(controller).to receive(:current_user).and_return(user)
       expect(controller).not_to receive(:redirect_to)
+
       controller.send(:check_password_expiration)
     end
 
     it 'does not redirect if the user is over their password expiry but they are an ldap user' do
       user.password_expires_at = Time.new(2002)
+
       allow(user).to receive(:ldap_user?).and_return(true)
       allow(controller).to receive(:current_user).and_return(user)
       expect(controller).not_to receive(:redirect_to)
+
       controller.send(:check_password_expiration)
     end
 
-    it 'does not redirect if the user is over their password expiry but sign-in is disabled' do
+    it 'redirects if the user is over their password expiry and sign-in is disabled' do
       stub_application_setting(password_authentication_enabled: false)
       user.password_expires_at = Time.new(2002)
+
+      expect(user.ldap_user?).to be_falsey
       allow(controller).to receive(:current_user).and_return(user)
-      expect(controller).not_to receive(:redirect_to)
+      expect(controller).to receive(:redirect_to)
+      expect(controller).to receive(:new_profile_password_path)
 
       controller.send(:check_password_expiration)
     end

spec/controllers/passwords_controller_spec.rb

 require 'spec_helper'
 
 describe PasswordsController do
-  describe '#check_password_authentication_available' do
+  describe '#prevent_ldap_reset' do
     before do
       @request.env["devise.mapping"] = Devise.mappings[:user]
     end
 
     context 'when password authentication is disabled' do
-      it 'prevents a password reset' do
+      it 'allows password reset' do
         stub_application_setting(password_authentication_enabled: false)
 
         post :create
 
-        expect(flash[:alert]).to eq 'Password authentication is unavailable.'
+        expect(response).to have_http_status(302)
       end
     end
 
@@ -22,7 +22,7 @@ describe PasswordsController do
       it 'prevents a password reset' do
         post :create, user: { email: user.email }
 
-        expect(flash[:alert]).to eq 'Password authentication is unavailable.'
+        expect(flash[:alert]).to eq('Cannot reset password for LDAP user.')
       end
     end
   end

spec/features/issues/filtered_search/filter_issues_spec.rb

@@ -101,14 +101,6 @@ describe 'Filter issues', js: true do
         expect_issues_list_count(5)
         expect_filtered_search_input_empty
       end
-
-      it 'filters issues by invalid author' do
-        skip('to be tested, issue #26546')
-      end
-
-      it 'filters issues by multiple authors' do
-        skip('to be tested, issue #26546')
-      end
     end
 
     context 'author with other filters' do
@@ -158,10 +150,6 @@ describe 'Filter issues', js: true do
         expect_filtered_search_input(search_term)
       end
     end
-
-    it 'sorting' do
-      skip('to be tested, issue #26546')
-    end
   end
 
   describe 'filter issues by assignee' do
@@ -181,14 +169,6 @@ describe 'Filter issues', js: true do
         expect_issues_list_count(8, 1)
         expect_filtered_search_input_empty
       end
-
-      it 'filters issues by invalid assignee' do
-        skip('to be tested, issue #26546')
-      end
-
-      it 'filters issues by multiple assignees' do
-        skip('to be tested, issue #26546')
-      end
     end
 
     context 'assignee with other filters' do
@@ -238,12 +218,6 @@ describe 'Filter issues', js: true do
         expect_filtered_search_input(search_term)
       end
     end
-
-    context 'sorting' do
-      it 'sorts' do
-        skip('to be tested, issue #26546')
-      end
-    end
   end
 
   describe 'filter issues by label' do
@@ -266,10 +240,6 @@ describe 'Filter issues', js: true do
         expect_filtered_search_input_empty
       end
 
-      it 'filters issues by invalid label' do
-        skip('to be tested, issue #26546')
-      end
-
       it 'filters issues by multiple labels' do
         input_filtered_search("label:~#{bug_label.title} label:~#{caps_sensitive_label.title}")
 
@@ -471,12 +441,6 @@ describe 'Filter issues', js: true do
         expect_filtered_search_input_empty
       end
     end
-
-    context 'sorting' do
-      it 'sorts' do
-        skip('to be tested, issue #26546')
-      end
-    end
   end
 
   describe 'filter issues by milestone' do
@@ -513,14 +477,6 @@ describe 'Filter issues', js: true do
         expect_filtered_search_input_empty
       end
 
-      it 'filters issues by invalid milestones' do
-        skip('to be tested, issue #26546')
-      end
-
-      it 'filters issues by multiple milestones' do
-        skip('to be tested, issue #26546')
-      end
-
       it 'filters issues by milestone containing special characters' do
         special_milestone = create(:milestone, title: '!@\#{$%^&*()}', project: project)
         create(:issue, title: "Issue with special character milestone", project: project, milestone: special_milestone)
@@ -590,12 +546,6 @@ describe 'Filter issues', js: true do
         expect_filtered_search_input(search_term)
       end
     end
-
-    context 'sorting' do
-      it 'sorts' do
-        skip('to be tested, issue #26546')
-      end
-    end
   end
 
   describe 'filter issues by text' do

spec/features/issues/issue_detail_spec.rb

@@ -40,4 +40,18 @@ feature 'Issue Detail', :js do
       end
     end
   end
+
+  context 'when authored by a user who is later deleted' do
+    before do
+      issue.update_attribute(:author_id, nil)
+      sign_in(user)
+      visit project_issue_path(project, issue)
+    end
+
+    it 'shows the issue' do
+      page.within('.issuable-details') do
+        expect(find('h2')).to have_content(issue.title)
+      end
+    end
+  end
 end

spec/features/profiles/password_spec.rb

@@ -53,12 +53,12 @@ describe 'Profile > Password' do
     context 'Regular user' do
       let(:user) { create(:user) }
 
-      it 'renders 404 when sign-in is disabled' do
+      it 'renders 200 when sign-in is disabled' do
         stub_application_setting(password_authentication_enabled: false)
 
         visit edit_profile_password_path
 
-        expect(page).to have_http_status(404)
+        expect(page).to have_http_status(200)
       end
     end
 

spec/features/projects/user_creates_files_spec.rb

@@ -56,11 +56,10 @@ describe 'User creates files' do
 
         find('.add-to-tree').click
         click_link('New file')
+        expect(page).to have_selector('.file-editor')
       end
 
       it 'creates and commit a new file', js: true do
-        expect(page).to have_selector('.file-editor')
-
         execute_script("ace.edit('editor').setValue('*.rbca')")
         fill_in(:file_name, with: 'not_a_file.md')
         fill_in(:commit_message, with: 'New commit message', visible: true)

spec/javascripts/fly_out_nav_spec.js

 import {
   calculateTop,
-  hideSubLevelItems,
   showSubLevelItems,
   canShowSubItems,
   canShowActiveSubItems,
+  mouseEnterTopItems,
+  mouseLeaveTopItem,
+  getOpenMenu,
+  setOpenMenu,
+  mousePos,
+  getHideSubItemsInterval,
+  documentMouseMove,
   getHeaderHeight,
   setSidebar,
+  subItemsMouseLeave,
 } from '~/fly_out_nav';
 import bp from '~/breakpoints';
 
@@ -19,11 +26,14 @@ describe('Fly out sidebar navigation', () => {
     document.body.appendChild(el);
 
     spyOn(bp, 'getBreakpointSize').and.callFake(() => breakpointSize);
+
+    setOpenMenu(null);
   });
 
   afterEach(() => {
-    el.remove();
+    document.body.innerHTML = '';
     breakpointSize = 'lg';
+    mousePos.length = 0;
   });
 
   describe('calculateTop', () => {
@@ -50,61 +60,152 @@ describe('Fly out sidebar navigation', () => {
     });
   });
 
-  describe('hideSubLevelItems', () => {
+  describe('getHideSubItemsInterval', () => {
     beforeEach(() => {
-      el.innerHTML = '<div class="sidebar-sub-level-items"></div>';
+      el.innerHTML = '<div class="sidebar-sub-level-items" style="position: fixed; top: 0; left: 100px; height: 150px;"></div>';
     });
 
-    it('hides subitems', () => {
-      hideSubLevelItems(el);
+    it('returns 0 if currentOpenMenu is nil', () => {
+      expect(
+        getHideSubItemsInterval(),
+      ).toBe(0);
+    });
+
+    it('returns 0 when mouse above sub-items', () => {
+      showSubLevelItems(el);
+      documentMouseMove({
+        clientX: el.getBoundingClientRect().left,
+        clientY: el.getBoundingClientRect().top,
+      });
+      documentMouseMove({
+        clientX: el.getBoundingClientRect().left,
+        clientY: el.getBoundingClientRect().top - 50,
+      });
 
       expect(
-        el.querySelector('.sidebar-sub-level-items').style.display,
-      ).toBe('');
+        getHideSubItemsInterval(),
+      ).toBe(0);
     });
 
-    it('does not hude subitems on mobile', () => {
-      breakpointSize = 'xs';
+    it('returns 0 when mouse is below sub-items', () => {
+      const subItems = el.querySelector('.sidebar-sub-level-items');
 
-      hideSubLevelItems(el);
+      showSubLevelItems(el);
+      documentMouseMove({
+        clientX: el.getBoundingClientRect().left,
+        clientY: el.getBoundingClientRect().top,
+      });
+      documentMouseMove({
+        clientX: el.getBoundingClientRect().left,
+        clientY: (el.getBoundingClientRect().top - subItems.getBoundingClientRect().height) + 50,
+      });
 
       expect(
-        el.querySelector('.sidebar-sub-level-items').style.display,
-      ).not.toBe('none');
+        getHideSubItemsInterval(),
+      ).toBe(0);
+    });
+
+    it('returns 300 when mouse is moved towards sub-items', () => {
+      documentMouseMove({
+        clientX: el.getBoundingClientRect().left,
+        clientY: el.getBoundingClientRect().top,
+      });
+      showSubLevelItems(el);
+      documentMouseMove({
+        clientX: el.getBoundingClientRect().left + 20,
+        clientY: el.getBoundingClientRect().top + 10,
+      });
+
+      expect(
+        getHideSubItemsInterval(),
+      ).toBe(300);
+    });
   });
 
-    it('removes is-over class', () => {
+  describe('mouseLeaveTopItem', () => {
+    beforeEach(() => {
       spyOn(el.classList, 'remove');
+    });
 
-      hideSubLevelItems(el);
+    it('removes is-over class if currentOpenMenu is null', () => {
+      mouseLeaveTopItem(el);
 
       expect(
         el.classList.remove,
       ).toHaveBeenCalledWith('is-over');
     });
 
-    it('removes is-above class from sub-items', () => {
-      const subItems = el.querySelector('.sidebar-sub-level-items');
+    it('removes is-over class if currentOpenMenu is null & there are sub-items', () => {
+      el.innerHTML = '<div class="sidebar-sub-level-items" style="position: absolute;"></div>';
 
-      spyOn(subItems.classList, 'remove');
+      mouseLeaveTopItem(el);
 
-      hideSubLevelItems(el);
+      expect(
+        el.classList.remove,
+      ).toHaveBeenCalledWith('is-over');
+    });
+
+    it('does not remove is-over class if currentOpenMenu is the passed in sub-items', () => {
+      el.innerHTML = '<div class="sidebar-sub-level-items" style="position: absolute;"></div>';
+
+      setOpenMenu(el.querySelector('.sidebar-sub-level-items'));
+      mouseLeaveTopItem(el);
 
       expect(
-        subItems.classList.remove,
-      ).toHaveBeenCalledWith('is-above');
+        el.classList.remove,
+      ).not.toHaveBeenCalled();
+    });
   });
 
-    it('does nothing if el has no sub-items', () => {
-      el.innerHTML = '';
+  describe('mouseEnterTopItems', () => {
+    beforeEach(() => {
+      jasmine.clock().install();
 
-      spyOn(el.classList, 'remove');
+      el.innerHTML = '<div class="sidebar-sub-level-items" style="position: absolute; top: 0; left: 100px; height: 200px;"></div>';
+    });
 
-      hideSubLevelItems(el);
+    afterEach(() => {
+      jasmine.clock().uninstall();
+    });
+
+    it('shows sub-items after 0ms if no menu is open', () => {
+      mouseEnterTopItems(el);
 
       expect(
-        el.classList.remove,
-      ).not.toHaveBeenCalledWith();
+        getHideSubItemsInterval(),
+      ).toBe(0);
+
+      jasmine.clock().tick(0);
+
+      expect(
+        el.querySelector('.sidebar-sub-level-items').style.display,
+      ).toBe('block');
+    });
+
+    it('shows sub-items after 300ms if a menu is currently open', () => {
+      documentMouseMove({
+        clientX: el.getBoundingClientRect().left,
+        clientY: el.getBoundingClientRect().top,
+      });
+
+      setOpenMenu(el.querySelector('.sidebar-sub-level-items'));
+
+      documentMouseMove({
+        clientX: el.getBoundingClientRect().left + 20,
+        clientY: el.getBoundingClientRect().top + 10,
+      });
+
+      mouseEnterTopItems(el);
+
+      expect(
+        getHideSubItemsInterval(),
+      ).toBe(300);
+
+      jasmine.clock().tick(300);
+
+      expect(
+        el.querySelector('.sidebar-sub-level-items').style.display,
+      ).toBe('block');
     });
   });
 
@@ -133,7 +234,7 @@ describe('Fly out sidebar navigation', () => {
       ).not.toBe('block');
     });
 
-    it('does not shows sub-items', () => {
+    it('shows sub-items', () => {
       showSubLevelItems(el);
 
       expect(
@@ -215,4 +316,29 @@ describe('Fly out sidebar navigation', () => {
       ).toBeTruthy();
     });
   });
+
+  describe('subItemsMouseLeave', () => {
+    beforeEach(() => {
+      el.innerHTML = '<div class="sidebar-sub-level-items" style="position: absolute;"></div>';
+
+      setOpenMenu(el.querySelector('.sidebar-sub-level-items'));
+    });
+
+    it('hides subMenu if element is not hovered', () => {
+      subItemsMouseLeave(el);
+
+      expect(
+        getOpenMenu(),
+      ).toBeNull();
+    });
+
+    it('does not hide subMenu if element is hovered', () => {
+      el.classList.add('is-over');
+      subItemsMouseLeave(el);
+
+      expect(
+        getOpenMenu(),
+      ).not.toBeNull();
+    });
+  });
 });

spec/javascripts/merge_request_tabs_spec.js

@@ -295,6 +295,17 @@ import 'vendor/jquery.scrollTo';
         this.class.loadDiff('/foo/bar/merge_requests/1/diffs');
       });
 
+      it('triggers scroll event when diff already loaded', function () {
+        spyOn(document, 'dispatchEvent');
+
+        this.class.diffsLoaded = true;
+        this.class.loadDiff('/foo/bar/merge_requests/1/diffs');
+
+        expect(
+          document.dispatchEvent,
+        ).toHaveBeenCalledWith(new CustomEvent('scroll'));
+      });
+
       describe('with inline diff', () => {
         let noteId;
         let noteLineNumId;

spec/lib/gitlab/auth_spec.rb

@@ -279,16 +279,6 @@ describe Gitlab::Auth do
         gl_auth.find_with_user_password('ldap_user', 'password')
       end
     end
-
-    context "with sign-in disabled" do
-      before do
-        stub_application_setting(password_authentication_enabled: false)
-      end
-
-      it "does not find user by valid login/password" do
-        expect(gl_auth.find_with_user_password(username, password)).to be_nil
-      end
-    end
   end
 
   private

spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb

@@ -210,7 +210,11 @@ describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads::Event do
   end
 end
 
-describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads do
+##
+# The background migration relies on a temporary table, hence we're migrating
+# to a specific version of the database where said table is still present.
+#
+describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads, :migration, schema: 20170608152748 do
   let(:migration) { described_class.new }
   let(:project) { create(:project_empty_repo) }
   let(:author) { create(:user) }
@@ -229,21 +233,6 @@ describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads do
     )
   end
 
-  # The background migration relies on a temporary table, hence we're migrating
-  # to a specific version of the database where said table is still present.
-  before :all do
-    ActiveRecord::Migration.verbose = false
-
-    ActiveRecord::Migrator
-      .migrate(ActiveRecord::Migrator.migrations_paths, 20170608152748)
-  end
-
-  after :all do
-    ActiveRecord::Migrator.migrate(ActiveRecord::Migrator.migrations_paths)
-
-    ActiveRecord::Migration.verbose = true
-  end
-
   describe '#perform' do
     it 'returns if data should not be migrated' do
       allow(migration).to receive(:migrate?).and_return(false)

spec/lib/gitlab/git/commit_spec.rb

@@ -2,7 +2,7 @@ require "spec_helper"
 
 describe Gitlab::Git::Commit, seed_helper: true do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) }
-  let(:commit) { Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID) }
+  let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) }
   let(:rugged_commit) do
     repository.rugged.lookup(SeedRepo::Commit::ID)
   end
@@ -24,7 +24,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
       }
 
       @parents = [repo.head.target]
-      @gitlab_parents = @parents.map { |c| Gitlab::Git::Commit.decorate(c) }
+      @gitlab_parents = @parents.map { |c| described_class.decorate(repository, c) }
       @tree = @parents.first.tree
 
       sha = Rugged::Commit.create(
@@ -38,7 +38,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
       )
 
       @raw_commit = repo.lookup(sha)
-      @commit = Gitlab::Git::Commit.new(@raw_commit)
+      @commit = described_class.new(repository, @raw_commit)
     end
 
     it { expect(@commit.short_id).to eq(@raw_commit.oid[0..10]) }
@@ -91,7 +91,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
         committer: committer
       )
     end
-    let(:commit) { described_class.new(Gitlab::GitalyClient::Commit.new(repository, gitaly_commit)) }
+    let(:commit) { described_class.new(repository, gitaly_commit) }
 
     it { expect(commit.short_id).to eq(id[0..10]) }
     it { expect(commit.id).to eq(id) }
@@ -113,45 +113,45 @@ describe Gitlab::Git::Commit, seed_helper: true do
   context 'Class methods' do
     describe '.find' do
       it "should return first head commit if without params" do
-        expect(Gitlab::Git::Commit.last(repository).id).to eq(
+        expect(described_class.last(repository).id).to eq(
           repository.rugged.head.target.oid
         )
       end
 
       it "should return valid commit" do
-        expect(Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID)).to be_valid_commit
+        expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_valid_commit
       end
 
       it "should return valid commit for tag" do
-        expect(Gitlab::Git::Commit.find(repository, 'v1.0.0').id).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9')
+        expect(described_class.find(repository, 'v1.0.0').id).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9')
       end
 
       it "should return nil for non-commit ids" do
         blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb")
-        expect(Gitlab::Git::Commit.find(repository, blob.id)).to be_nil
+        expect(described_class.find(repository, blob.id)).to be_nil
       end
 
       it "should return nil for parent of non-commit object" do
         blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb")
-        expect(Gitlab::Git::Commit.find(repository, "#{blob.id}^")).to be_nil
+        expect(described_class.find(repository, "#{blob.id}^")).to be_nil
       end
 
       it "should return nil for nonexisting ids" do
-        expect(Gitlab::Git::Commit.find(repository, "+123_4532530XYZ")).to be_nil
+        expect(described_class.find(repository, "+123_4532530XYZ")).to be_nil
       end
 
       context 'with broken repo' do
         let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH) }
 
         it 'returns nil' do
-          expect(Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID)).to be_nil
+          expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil
         end
       end
     end
 
     describe '.last_for_path' do
       context 'no path' do
-        subject { Gitlab::Git::Commit.last_for_path(repository, 'master') }
+        subject { described_class.last_for_path(repository, 'master') }
 
         describe '#id' do
           subject { super().id }
@@ -160,7 +160,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
       end
 
       context 'path' do
-        subject { Gitlab::Git::Commit.last_for_path(repository, 'master', 'files/ruby') }
+        subject { described_class.last_for_path(repository, 'master', 'files/ruby') }
 
         describe '#id' do
           subject { super().id }
@@ -169,7 +169,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
       end
 
       context 'ref + path' do
-        subject { Gitlab::Git::Commit.last_for_path(repository, SeedRepo::Commit::ID, 'encoding') }
+        subject { described_class.last_for_path(repository, SeedRepo::Commit::ID, 'encoding') }
 
         describe '#id' do
           subject { super().id }
@@ -181,7 +181,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
     describe '.where' do
       context 'path is empty string' do
         subject do
-          commits = Gitlab::Git::Commit.where(
+          commits = described_class.where(
             repo: repository,
             ref: 'master',
             path: '',
@@ -199,7 +199,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
 
       context 'path is nil' do
         subject do
-          commits = Gitlab::Git::Commit.where(
+          commits = described_class.where(
             repo: repository,
             ref: 'master',
             path: nil,
@@ -217,7 +217,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
 
       context 'ref is branch name' do
         subject do
-          commits = Gitlab::Git::Commit.where(
+          commits = described_class.where(
             repo: repository,
             ref: 'master',
             path: 'files',
@@ -237,7 +237,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
 
       context 'ref is commit id' do
         subject do
-          commits = Gitlab::Git::Commit.where(
+          commits = described_class.where(
             repo: repository,
             ref: "874797c3a73b60d2187ed6e2fcabd289ff75171e",
             path: 'files',
@@ -257,7 +257,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
 
       context 'ref is tag' do
         subject do
-          commits = Gitlab::Git::Commit.where(
+          commits = described_class.where(
             repo: repository,
             ref: 'v1.0.0',
             path: 'files',
@@ -278,7 +278,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
 
     describe '.between' do
       subject do
-        commits = Gitlab::Git::Commit.between(repository, SeedRepo::Commit::PARENT_ID, SeedRepo::Commit::ID)
+        commits = described_class.between(repository, SeedRepo::Commit::PARENT_ID, SeedRepo::Commit::ID)
         commits.map { |c| c.id }
       end
 
@@ -294,12 +294,12 @@ describe Gitlab::Git::Commit, seed_helper: true do
         it 'should return a return a collection of commits' do
           commits = described_class.find_all(repository)
 
-          expect(commits).to all( be_a_kind_of(Gitlab::Git::Commit) )
+          expect(commits).to all( be_a_kind_of(described_class) )
         end
 
         context 'max_count' do
           subject do
-            commits = Gitlab::Git::Commit.find_all(
+            commits = described_class.find_all(
               repository,
               max_count: 50
             )
@@ -322,7 +322,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
 
         context 'ref + max_count + skip' do
           subject do
-            commits = Gitlab::Git::Commit.find_all(
+            commits = described_class.find_all(
               repository,
               ref: 'master',
               max_count: 50,
@@ -374,7 +374,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
   end
 
   describe '#init_from_rugged' do
-    let(:gitlab_commit) { Gitlab::Git::Commit.new(rugged_commit) }
+    let(:gitlab_commit) { described_class.new(repository, rugged_commit) }
     subject { gitlab_commit }
 
     describe '#id' do
@@ -384,7 +384,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
   end
 
   describe '#init_from_hash' do
-    let(:commit) { Gitlab::Git::Commit.new(sample_commit_hash) }
+    let(:commit) { described_class.new(repository, sample_commit_hash) }
     subject { commit }
 
     describe '#id' do
@@ -451,7 +451,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
   end
 
   describe '#ref_names' do
-    let(:commit) { Gitlab::Git::Commit.find(repository, 'master') }
+    let(:commit) { described_class.find(repository, 'master') }
     subject { commit.ref_names(repository) }
 
     it 'has 1 element' do
@@ -461,6 +461,17 @@ describe Gitlab::Git::Commit, seed_helper: true do
     it { is_expected.not_to include("feature") }
   end
 
+  describe '#rugged_commit' do
+    let(:raw_commit) { { id: SeedRepo::Commit::ID } }
+    let(:commit) { described_class.decorate(repository, raw_commit) }
+    subject { commit.send(:rugged_commit) }
+
+    it 'returns a rugged commit based on the id of a non-rugged raw commit' do
+      expect(subject.class).to be(Rugged::Commit)
+      expect(subject).to eq(rugged_commit)
+    end
+  end
+
   def sample_commit_hash
     {
       author_email: "dmitriy.zaporozhets@gmail.com",

spec/lib/gitlab/git/repository_spec.rb

@@ -511,17 +511,22 @@ describe Gitlab::Git::Repository, seed_helper: true do
   end
 
   describe "#log" do
-    commit_with_old_name = nil
-    commit_with_new_name = nil
-    rename_commit = nil
+    let(:commit_with_old_name) do
+      Gitlab::Git::Commit.decorate(repository, @commit_with_old_name_id)
+    end
+    let(:commit_with_new_name) do
+      Gitlab::Git::Commit.decorate(repository, @commit_with_new_name_id)
+    end
+    let(:rename_commit) do
+      Gitlab::Git::Commit.decorate(repository, @rename_commit_id)
+    end
 
     before(:context) do
       # Add new commits so that there's a renamed file in the commit history
       repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged
-
-      commit_with_old_name = Gitlab::Git::Commit.decorate(new_commit_edit_old_file(repo))
-      rename_commit = Gitlab::Git::Commit.decorate(new_commit_move_file(repo))
-      commit_with_new_name = Gitlab::Git::Commit.decorate(new_commit_edit_new_file(repo))
+      @commit_with_old_name_id = new_commit_edit_old_file(repo)
+      @rename_commit_id = new_commit_move_file(repo)
+      @commit_with_new_name_id = new_commit_edit_new_file(repo)
     end
 
     after(:context) do

spec/lib/gitlab/gitaly_client/commit_service_spec.rb

@@ -30,7 +30,7 @@ describe Gitlab::GitalyClient::CommitService do
 
     context 'when a commit does not have a parent' do
       it 'sends an RPC request with empty tree ref as left commit' do
-        initial_commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863')
+        initial_commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863').raw
         request        = Gitaly::CommitDiffRequest.new(
           repository: repository_message,
           left_commit_id: '4b825dc642cb6eb9a060e54bf8d69288fbee4904',

spec/lib/gitlab/import_export/project_tree_restorer_spec.rb

@@ -79,6 +79,10 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
           expect(event).not_to be_nil
         end
 
+        it 'has the action' do
+          expect(event.action).not_to be_nil
+        end
+
         it 'event belongs to note, belongs to merge request, belongs to a project' do
           expect(event.note.noteable.project).not_to be_nil
         end

spec/mailers/notify_spec.rb

@@ -8,6 +8,25 @@ describe Notify do
 
   include_context 'gitlab email notification'
 
+  set(:user) { create(:user) }
+  set(:current_user) { create(:user, email: "current@email.com") }
+  set(:assignee) { create(:user, email: 'assignee@example.com', name: 'John Doe') }
+
+  set(:merge_request) do
+    create(:merge_request, source_project: project,
+                           target_project: project,
+                           author: current_user,
+                           assignee: assignee,
+                           description: 'Awesome description')
+  end
+
+  set(:issue) do
+    create(:issue, author: current_user,
+                   assignees: [assignee],
+                   project: project,
+                   description: 'My awesome description!')
+  end
+
   def have_referable_subject(referable, reply: false)
     prefix = referable.project.name if referable.project
     prefix = "Re: #{prefix}" if reply
@@ -19,8 +38,6 @@ describe Notify do
 
   context 'for a project' do
     describe 'items that are assignable, the email' do
-      let(:current_user) { create(:user, email: "current@email.com") }
-      let(:assignee) { create(:user, email: 'assignee@example.com', name: 'John Doe') }
       let(:previous_assignee) { create(:user, name: 'Previous Assignee') }
 
       shared_examples 'an assignee email' do
@@ -36,9 +53,6 @@ describe Notify do
       end
 
       context 'for issues' do
-        let(:issue) { create(:issue, author: current_user, assignees: [assignee], project: project) }
-        let(:issue_with_description) { create(:issue, author: current_user, assignees: [assignee], project: project, description: 'My awesome description') }
-
         describe 'that are new' do
           subject { described_class.new_issue_email(issue.assignees.first.id, issue.id) }
 
@@ -56,6 +70,10 @@ describe Notify do
             end
           end
 
+          it 'contains the description' do
+            is_expected.to have_html_escaped_body_text issue.description
+          end
+
           context 'when enabled email_author_in_body' do
             before do
               stub_application_setting(email_author_in_body: true)
@@ -68,16 +86,6 @@ describe Notify do
           end
         end
 
-        describe 'that are new with a description' do
-          subject { described_class.new_issue_email(issue_with_description.assignees.first.id, issue_with_description.id) }
-
-          it_behaves_like 'it should show Gmail Actions View Issue link'
-
-          it 'contains the description' do
-            is_expected.to have_html_escaped_body_text issue_with_description.description
-          end
-        end
-
         describe 'that have been reassigned' do
           subject { described_class.reassigned_issue_email(recipient.id, issue.id, [previous_assignee.id], current_user.id) }
 
@@ -197,11 +205,6 @@ describe Notify do
       end
 
       context 'for merge requests' do
-        let(:project) { create(:project, :repository) }
-        let(:merge_author) { create(:user) }
-        let(:merge_request) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project) }
-        let(:merge_request_with_description) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project, description: 'My awesome description') }
-
         describe 'that are new' do
           subject { described_class.new_merge_request_email(merge_request.assignee_id, merge_request.id) }
 
@@ -221,6 +224,10 @@ describe Notify do
             end
           end
 
+          it 'contains the description' do
+            is_expected.to have_html_escaped_body_text merge_request.description
+          end
+
           context 'when enabled email_author_in_body' do
             before do
               stub_application_setting(email_author_in_body: true)
@@ -233,17 +240,6 @@ describe Notify do
           end
         end
 
-        describe 'that are new with a description' do
-          subject { described_class.new_merge_request_email(merge_request_with_description.assignee_id, merge_request_with_description.id) }
-
-          it_behaves_like 'it should show Gmail Actions View Merge request link'
-          it_behaves_like "an unsubscribeable thread"
-
-          it 'contains the description' do
-            is_expected.to have_html_escaped_body_text merge_request_with_description.description
-          end
-        end
-
         describe 'that are reassigned' do
           subject { described_class.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) }
 
@@ -321,6 +317,7 @@ describe Notify do
         end
 
         describe 'that are merged' do
+          let(:merge_author) { create(:user) }
           subject { described_class.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) }
 
           it_behaves_like 'a multiple recipients email'
@@ -348,8 +345,6 @@ describe Notify do
     end
 
     describe 'project was moved' do
-      let(:project) { create(:project) }
-      let(:user) { create(:user) }
       subject { described_class.project_was_moved_email(project.id, user.id, "gitlab/gitlab") }
 
       it_behaves_like 'an email sent from GitLab'
@@ -371,7 +366,6 @@ describe Notify do
           end
         end
 
-        let(:user) { create(:user) }
         let(:project_member) do
           project.request_access(user)
           project.requesters.find_by(user_id: user.id)
@@ -398,7 +392,6 @@ describe Notify do
         let(:group_owner) { create(:user) }
         let(:group) { create(:group).tap { |g| g.add_owner(group_owner) } }
         let(:project) { create(:project, :public, :access_requestable, namespace: group) }
-        let(:user) { create(:user) }
         let(:project_member) do
           project.request_access(user)
           project.requesters.find_by(user_id: user.id)
@@ -424,7 +417,6 @@ describe Notify do
 
     describe 'project access denied' do
       let(:project) { create(:project, :public, :access_requestable) }
-      let(:user) { create(:user) }
       let(:project_member) do
         project.request_access(user)
         project.requesters.find_by(user_id: user.id)
@@ -445,7 +437,6 @@ describe Notify do
     describe 'project access changed' do
       let(:owner) { create(:user, name: "Chang O'Keefe") }
       let(:project) { create(:project, :public, :access_requestable, namespace: owner.namespace) }
-      let(:user) { create(:user) }
       let(:project_member) { create(:project_member, project: project, user: user) }
       subject { described_class.member_access_granted_email('project', project_member.id) }
 
@@ -474,7 +465,6 @@ describe Notify do
     end
 
     describe 'project invitation' do
-      let(:project) { create(:project) }
       let(:master) { create(:user).tap { |u| project.team << [u, :master] } }
       let(:project_member) { invite_to_project(project, inviter: master) }
 
@@ -494,7 +484,6 @@ describe Notify do
     end
 
     describe 'project invitation accepted' do
-      let(:project) { create(:project) }
       let(:invited_user) { create(:user, name: 'invited user') }
       let(:master) { create(:user).tap { |u| project.team << [u, :master] } }
       let(:project_member) do
@@ -519,7 +508,6 @@ describe Notify do
     end
 
     describe 'project invitation declined' do
-      let(:project) { create(:project) }
       let(:master) { create(:user).tap { |u| project.team << [u, :master] } }
       let(:project_member) do
         invitee = invite_to_project(project, inviter: master)
@@ -582,7 +570,6 @@ describe Notify do
       end
 
       describe 'on a commit' do
-        let(:project) { create(:project, :repository) }
         let(:commit) { project.commit }
 
         before do
@@ -607,7 +594,6 @@ describe Notify do
       end
 
       describe 'on a merge request' do
-        let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
         let(:note_on_merge_request_path) { project_merge_request_path(project, merge_request, anchor: "note_#{note.id}") }
 
         before do
@@ -632,7 +618,6 @@ describe Notify do
       end
 
       describe 'on an issue' do
-        let(:issue) { create(:issue, project: project) }
         let(:note_on_issue_path) { project_issue_path(project, issue, anchor: "note_#{note.id}") }
 
         before do
@@ -658,7 +643,6 @@ describe Notify do
     end
 
     context 'items that are noteable, the email for a discussion note' do
-      let(:project) { create(:project, :repository) }
       let(:note_author) { create(:user, name: 'author_name') }
 
       before :each do
@@ -722,7 +706,6 @@ describe Notify do
       end
 
       describe 'on a merge request' do
-        let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
         let(:note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, author: note_author) }
         let(:note_on_merge_request_path) { project_merge_request_path(project, merge_request, anchor: "note_#{note.id}") }
 
@@ -749,7 +732,6 @@ describe Notify do
       end
 
       describe 'on an issue' do
-        let(:issue) { create(:issue, project: project) }
         let(:note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: note_author) }
         let(:note_on_issue_path) { project_issue_path(project, issue, anchor: "note_#{note.id}") }
 
@@ -835,7 +817,6 @@ describe Notify do
       end
 
       describe 'on a merge request' do
-        let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
         let(:note) { create(:diff_note_on_merge_request) }
 
         subject { described_class.note_merge_request_email(recipient.id, note.id) }
@@ -848,9 +829,10 @@ describe Notify do
   end
 
   context 'for a group' do
+    set(:group) { create(:group) }
+
     describe 'group access requested' do
       let(:group) { create(:group, :public, :access_requestable) }
-      let(:user) { create(:user) }
       let(:group_member) do
         group.request_access(user)
         group.requesters.find_by(user_id: user.id)
@@ -870,8 +852,6 @@ describe Notify do
     end
 
     describe 'group access denied' do
-      let(:group) { create(:group) }
-      let(:user) { create(:user) }
       let(:group_member) do
         group.request_access(user)
         group.requesters.find_by(user_id: user.id)
@@ -890,8 +870,6 @@ describe Notify do
     end
 
     describe 'group access changed' do
-      let(:group) { create(:group) }
-      let(:user) { create(:user) }
       let(:group_member) { create(:group_member, group: group, user: user) }
 
       subject { described_class.member_access_granted_email('group', group_member.id) }
@@ -921,7 +899,6 @@ describe Notify do
     end
 
     describe 'group invitation' do
-      let(:group) { create(:group) }
       let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } }
       let(:group_member) { invite_to_group(group, inviter: owner) }
 
@@ -941,7 +918,6 @@ describe Notify do
     end
 
     describe 'group invitation accepted' do
-      let(:group) { create(:group) }
       let(:invited_user) { create(:user, name: 'invited user') }
       let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } }
       let(:group_member) do
@@ -966,7 +942,6 @@ describe Notify do
     end
 
     describe 'group invitation declined' do
-      let(:group) { create(:group) }
       let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } }
       let(:group_member) do
         invitee = invite_to_group(group, inviter: owner)
@@ -1020,7 +995,6 @@ describe Notify do
 
   describe 'email on push for a created branch' do
     let(:example_site_path) { root_path }
-    let(:user) { create(:user) }
     let(:tree_path) { project_tree_path(project, "empty-branch") }
 
     subject { described_class.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/empty-branch', action: :create) }
@@ -1046,7 +1020,6 @@ describe Notify do
 
   describe 'email on push for a created tag' do
     let(:example_site_path) { root_path }
-    let(:user) { create(:user) }
     let(:tree_path) { project_tree_path(project, "v1.0") }
 
     subject { described_class.repository_push_email(project.id, author_id: user.id, ref: 'refs/tags/v1.0', action: :create) }
@@ -1072,7 +1045,6 @@ describe Notify do
 
   describe 'email on push for a deleted branch' do
     let(:example_site_path) { root_path }
-    let(:user) { create(:user) }
 
     subject { described_class.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :delete) }
 
@@ -1094,7 +1066,6 @@ describe Notify do
 
   describe 'email on push for a deleted tag' do
     let(:example_site_path) { root_path }
-    let(:user) { create(:user) }
 
     subject { described_class.repository_push_email(project.id, author_id: user.id, ref: 'refs/tags/v1.0', action: :delete) }
 
@@ -1115,9 +1086,7 @@ describe Notify do
   end
 
   describe 'email on push with multiple commits' do
-    let(:project) { create(:project, :repository) }
     let(:example_site_path) { root_path }
-    let(:user) { create(:user) }
     let(:raw_compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, sample_image_commit.id, sample_commit.id) }
     let(:compare) { Compare.decorate(raw_compare, project) }
     let(:commits) { compare.commits }
@@ -1209,9 +1178,7 @@ describe Notify do
   end
 
   describe 'email on push with a single commit' do
-    let(:project) { create(:project, :repository) }
     let(:example_site_path) { root_path }
-    let(:user) { create(:user) }
     let(:raw_compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, sample_commit.parent_id, sample_commit.id) }
     let(:compare) { Compare.decorate(raw_compare, project) }
     let(:commits) { compare.commits }
@@ -1242,8 +1209,6 @@ describe Notify do
   end
 
   describe 'HTML emails setting' do
-    let(:project) { create(:project) }
-    let(:user) { create(:user) }
     let(:multipart_mail) { described_class.project_was_moved_email(project.id, user.id, "gitlab/gitlab") }
 
     context 'when disabled' do

spec/migrations/README.md

@@ -28,6 +28,14 @@ The `after` hook will migrate the database **up** and reinstitutes the latest
 schema version, so that the process does not affect subsequent specs and
 ensures proper isolation.
 
+## Testing a class that is not an ActiveRecord::Migration
+
+In order to test a class that is not a migration itself, you will need to
+manually provide a required schema version. Please add a `schema` tag to a
+context that you want to switch the database schema within.
+
+Example: `describe SomeClass, :migration, schema: 20170608152748`.
+
 ## Available helpers
 
 Use `table` helper to create a temporary `ActiveRecord::Base` derived model
@@ -80,8 +88,6 @@ end
 
 ## Best practices
 
-1. Use only one test example per migration unless there is a good reason to
-use more.
 1. Note that this type of tests do not run within the transaction, we use
 a truncation database cleanup strategy. Do not depend on transaction being
 present.

spec/migrations/remove_dot_git_from_usernames_spec.rb

@@ -51,7 +51,6 @@ describe RemoveDotGitFromUsernames do
     namespace.path = path
     namespace.save!(validate: false)
 
-    user.username = path
-    user.save!(validate: false)
+    user.update_column(:username, path)
   end
 end

spec/models/commit_spec.rb

@@ -33,7 +33,6 @@ describe Commit do
 
   describe '#to_reference' do
     let(:project) { create(:project, :repository, path: 'sample-project') }
-    let(:commit)  { project.commit }
 
     it 'returns a String reference to the object' do
       expect(commit.to_reference).to eq commit.id
@@ -47,7 +46,6 @@ describe Commit do
 
   describe '#reference_link_text' do
     let(:project) { create(:project, :repository, path: 'sample-project') }
-    let(:commit)  { project.commit }
 
     it 'returns a String reference to the object' do
       expect(commit.reference_link_text).to eq commit.short_id

spec/models/merge_request_spec.rb

@@ -159,6 +159,7 @@ describe MergeRequest do
       before do
         subject.project.has_external_issue_tracker = true
         subject.project.save!
+        create(:jira_service, project: subject.project)
       end
 
       it 'does not cache issues from external trackers' do
@@ -166,6 +167,7 @@ describe MergeRequest do
         commit = double('commit1', safe_message: "Fixes #{issue.to_reference}")
         allow(subject).to receive(:commits).and_return([commit])
 
+        expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to raise_error
         expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to change(subject.merge_requests_closing_issues, :count)
       end
 

spec/models/user_spec.rb

@@ -2013,4 +2013,65 @@ describe User do
       expect(user.projects_limit_left).to eq(5)
     end
   end
+
+  describe '#ensure_namespace_correct' do
+    context 'for a new user' do
+      let(:user) { build(:user) }
+
+      it 'creates the namespace' do
+        expect(user.namespace).to be_nil
+        user.save!
+        expect(user.namespace).not_to be_nil
+      end
+    end
+
+    context 'for an existing user' do
+      let(:username) { 'foo' }
+      let(:user) { create(:user, username: username) }
+
+      context 'when the user is updated' do
+        context 'when the username is changed' do
+          let(:new_username) { 'bar' }
+
+          it 'changes the namespace (just to compare to when username is not changed)' do
+            expect do
+              user.update_attributes!(username: new_username)
+            end.to change { user.namespace.updated_at }
+          end
+
+          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
+
+          context 'when there is a validation error (namespace name taken) while updating namespace' do
+            let!(:conflicting_namespace) { create(:group, name: new_username, path: 'quz') }
+
+            it 'causes the user save to fail' do
+              expect(user.update_attributes(username: new_username)).to be_falsey
+              expect(user.namespace.errors.messages[:name].first).to eq('has already been taken')
+            end
+
+            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
+        end
+
+        context 'when the username is not changed' do
+          it 'does not change the namespace' do
+            expect do
+              user.update_attributes!(email: 'asdf@asdf.com')
+            end.not_to change { user.namespace.updated_at }
+          end
+        end
+      end
+    end
+  end
 end

spec/requests/api/triggers_spec.rb

@@ -85,6 +85,22 @@ describe API::Triggers do
           expect(pipeline.variables.map { |v| { v.key => v.value } }.last).to eq(variables)
         end
       end
+
+      context 'when legacy trigger' do
+        before do
+          trigger.update(owner: nil)
+        end
+
+        it 'creates pipeline' do
+          post api("/projects/#{project.id}/trigger/pipeline"), options.merge(ref: 'master')
+
+          expect(response).to have_http_status(201)
+          expect(json_response).to include('id' => pipeline.id)
+          pipeline.builds.reload
+          expect(pipeline.builds.pending.size).to eq(2)
+          expect(pipeline.builds.size).to eq(5)
+        end
+      end
     end
 
     context 'when triggering a pipeline from a trigger token' do

spec/services/ci/create_pipeline_service_spec.rb

@@ -413,14 +413,12 @@ describe Ci::CreatePipelineService do
       end
 
       context 'when trigger belongs to a developer' do
-        let(:user) {}
+        let(:user) { create(:user) }
+        let(:trigger) { create(:ci_trigger, owner: user) }
+        let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) }
 
-        let(:trigger_request) do
-          create(:ci_trigger_request).tap do |request|
-            user = create(:user)
+        before do
           project.add_developer(user)
-            request.trigger.update(owner: user)
-          end
         end
 
         it 'does not create a pipeline' do
@@ -431,17 +429,15 @@ describe Ci::CreatePipelineService do
       end
 
       context 'when trigger belongs to a master' do
-        let(:user) {}
+        let(:user) { create(:user) }
+        let(:trigger) { create(:ci_trigger, owner: user) }
+        let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) }
 
-        let(:trigger_request) do
-          create(:ci_trigger_request).tap do |request|
-            user = create(:user)
+        before do
           project.add_master(user)
-            request.trigger.update(owner: user)
-          end
         end
 
-        it 'does not create a pipeline' do
+        it 'creates a pipeline' do
           expect(execute_service(trigger_request: trigger_request))
             .to be_persisted
           expect(Ci::Pipeline.count).to eq(1)

spec/services/notification_service_spec.rb

@@ -126,7 +126,18 @@ describe NotificationService, :mailer do
         project.add_master(issue.author)
         project.add_master(assignee)
         project.add_master(note.author)
-        create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy')
+
+        @u_custom_off = create_user_with_notification(:custom, 'custom_off')
+        project.add_guest(@u_custom_off)
+
+        create(
+          :note_on_issue,
+          author: @u_custom_off,
+          noteable: issue,
+          project_id: issue.project_id,
+          note: 'i think @subscribed_participant should see this'
+        )
+
         update_custom_notification(:new_note, @u_guest_custom, resource: project)
         update_custom_notification(:new_note, @u_custom_global)
       end
@@ -136,8 +147,7 @@ describe NotificationService, :mailer do
           add_users_with_subscription(note.project, issue)
           reset_delivered_emails!
 
-          # Ensure create SentNotification by noteable = issue 6 times, not noteable = note
-          expect(SentNotification).to receive(:record).with(issue, any_args).exactly(8).times
+          expect(SentNotification).to receive(:record).with(issue, any_args).exactly(9).times
 
           notification.new_note(note)
 
@@ -149,6 +159,7 @@ describe NotificationService, :mailer do
           should_email(@subscriber)
           should_email(@watcher_and_subscriber)
           should_email(@subscribed_participant)
+          should_email(@u_custom_off)
           should_not_email(@u_guest_custom)
           should_not_email(@u_guest_watcher)
           should_not_email(note.author)

spec/services/test_hooks/system_service_spec.rb

@@ -7,7 +7,6 @@ describe TestHooks::SystemService do
     let(:project) { create(:project, :repository) }
     let(:hook)    { create(:system_hook) }
     let(:service) { described_class.new(hook, current_user, trigger) }
-    let(:sample_data) { { data: 'sample' }}
     let(:success_result) { { status: :success, http_status: 200, message: 'ok' } }
 
     before do
@@ -26,18 +25,11 @@ describe TestHooks::SystemService do
     context 'push_events' do
       let(:trigger) { 'push_events' }
 
-      it 'returns error message if not enough data' do
-        allow(project).to receive(:empty_repo?).and_return(true)
-
-        expect(hook).not_to receive(:execute)
-        expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." })
-      end
-
       it 'executes hook' do
         allow(project).to receive(:empty_repo?).and_return(false)
-        allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data)
+        expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original
 
-        expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result)
+        expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result)
         expect(service.execute).to include(success_result)
       end
     end
@@ -45,18 +37,11 @@ describe TestHooks::SystemService do
     context 'tag_push_events' do
       let(:trigger) { 'tag_push_events' }
 
-      it 'returns error message if not enough data' do
-        allow(project.repository).to receive(:tags).and_return([])
-
-        expect(hook).not_to receive(:execute)
-        expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has tags." })
-      end
-
       it 'executes hook' do
         allow(project.repository).to receive(:tags).and_return(['tag'])
-        allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data)
+        expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original
 
-        expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result)
+        expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result)
         expect(service.execute).to include(success_result)
       end
     end
@@ -64,17 +49,11 @@ describe TestHooks::SystemService do
     context 'repository_update_events' do
       let(:trigger) { 'repository_update_events' }
 
-      it 'returns error message if not enough data' do
-        allow(project).to receive(:commit).and_return(nil)
-        expect(hook).not_to receive(:execute)
-        expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." })
-      end
-
       it 'executes hook' do
         allow(project).to receive(:empty_repo?).and_return(false)
-        allow(Gitlab::DataBuilder::Repository).to receive(:update).and_return(sample_data)
+        expect(Gitlab::DataBuilder::Repository).to receive(:sample_data).and_call_original
 
-        expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result)
+        expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Repository::SAMPLE_DATA, trigger).and_return(success_result)
         expect(service.execute).to include(success_result)
       end
     end

spec/services/users/update_service_spec.rb

@@ -12,9 +12,22 @@ describe Users::UpdateService do
     end
 
     it 'returns an error result when record cannot be updated' do
+      result = {}
       expect do
-        update_user(user, { email: 'invalid' })
+        result = update_user(user, { email: 'invalid' })
       end.not_to change { user.reload.email }
+      expect(result[:status]).to eq(:error)
+      expect(result[:message]).to eq('Email is invalid')
+    end
+
+    it 'includes namespace error messages' do
+      create(:group, name: 'taken', path: 'something_else')
+      result = {}
+      expect do
+        result = update_user(user, { username: 'taken' })
+      end.not_to change { user.reload.username }
+      expect(result[:status]).to eq(:error)
+      expect(result[:message]).to eq('Namespace name has already been taken')
     end
 
     def update_user(user, opts)

spec/spec_helper.rb

@@ -136,17 +136,12 @@ RSpec.configure do |config|
     Sidekiq.redis(&:flushall)
   end
 
-  config.before(:example, :migration) do
-    ActiveRecord::Migrator
-      .migrate(migrations_paths, previous_migration.version)
-
-    reset_column_in_migration_models
+  config.before(:each, :migration) do
+    schema_migrate_down!
   end
 
-  config.after(:example, :migration) do
-    ActiveRecord::Migrator.migrate(migrations_paths)
-
-    reset_column_in_migration_models
+  config.after(:context, :migration) do
+    schema_migrate_up!
   end
 
   config.around(:each, :nested_groups) do |example|

spec/support/db_cleaner.rb

@@ -20,7 +20,7 @@ RSpec.configure do |config|
   end
 
   config.before(:each, :migration) do
-    DatabaseCleaner.strategy = :truncation
+    DatabaseCleaner.strategy = :truncation, { cache_tables: false }
   end
 
   config.before(:each) do

spec/support/import_export/export_file_helper.rb

@@ -35,7 +35,8 @@ module ExportFileHelper
            project: project,
            commit_id: ci_pipeline.sha)
 
-    create(:event, :created, target: milestone, project: project, author: user)
+    event = create(:event, :created, target: milestone, project: project, author: user, action: 5)
+    create(:push_event_payload, event: event)
     create(:project_member, :master, user: user, project: project)
     create(:ci_variable, project: project)
     create(:ci_trigger, project: project)

spec/support/migrations_helpers.rb

@@ -16,6 +16,8 @@ module MigrationsHelpers
   end
 
   def reset_column_in_migration_models
+    ActiveRecord::Base.clear_cache!
+
     described_class.constants.sort.each do |name|
       const = described_class.const_get(name)
 
@@ -31,6 +33,35 @@ module MigrationsHelpers
     end
   end
 
+  def migration_schema_version
+    self.class.metadata[:schema] || previous_migration.version
+  end
+
+  def schema_migrate_down!
+    disable_migrations_output do
+      ActiveRecord::Migrator.migrate(migrations_paths,
+                                     migration_schema_version)
+    end
+
+    reset_column_in_migration_models
+  end
+
+  def schema_migrate_up!
+    disable_migrations_output do
+      ActiveRecord::Migrator.migrate(migrations_paths)
+    end
+
+    reset_column_in_migration_models
+  end
+
+  def disable_migrations_output
+    ActiveRecord::Migration.verbose = false
+
+    yield
+  ensure
+    ActiveRecord::Migration.verbose = true
+  end
+
   def migrate!
     ActiveRecord::Migrator.up(migrations_paths) do |migration|
       migration.name == described_class.name

spec/support/notify_shared_examples.rb

 shared_context 'gitlab email notification' do
+  set(:project) { create(:project, :repository) }
+  set(:recipient) { create(:user, email: 'recipient@example.com') }
+
   let(:gitlab_sender_display_name) { Gitlab.config.gitlab.email_display_name }
   let(:gitlab_sender) { Gitlab.config.gitlab.email_from }
   let(:gitlab_sender_reply_to) { Gitlab.config.gitlab.email_reply_to }
-  let(:recipient) { create(:user, email: 'recipient@example.com') }
-  let(:project) { create(:project) }
   let(:new_user_address) { 'newguy@example.com' }
 
   before do