Merge branch 'ce_upstream'

CONTRIBUTING.md

@@ -31,7 +31,7 @@ _This notice should stay as the first item in the CONTRIBUTING.MD file._
   - [Issue tracker guidelines](#issue-tracker-guidelines)
   - [Issue weight](#issue-weight)
   - [Regression issues](#regression-issues)
-  - [Technical debt](#technical-debt)
+  - [Technical and UX debt](#technical-and-ux-debt)
   - [Stewardship](#stewardship)
 - [Merge requests](#merge-requests)
   - [Merge request guidelines](#merge-request-guidelines)
@@ -345,27 +345,29 @@ addressed.
 [8.3 Regressions]: https://gitlab.com/gitlab-org/gitlab-ce/issues/4127
 [update the notes]: https://gitlab.com/gitlab-org/release-tools/blob/master/doc/pro-tips.md#update-the-regression-issue
 
-### Technical debt
+### Technical and UX debt
 
-In order to track things that can be improved in GitLab's codebase, we created
-the ~"technical debt" label in [GitLab's issue tracker][ce-tracker].
+In order to track things that can be improved in GitLab's codebase,
+we use the ~"technical debt" label in [GitLab's issue tracker][ce-tracker].
+For user experience improvements, we use the ~"UX debt" label.
 
-This label should be added to issues that describe things that can be improved,
-shortcuts that have been taken, code that needs refactoring, features that need
-additional attention, and all other things that have been left behind due to
-high velocity of development.
+These labels should be added to issues that describe things that can be improved,
+shortcuts that have been taken, features that need additional attention, and all
+other things that have been left behind due to high velocity of development.
+For example, code that needs refactoring should use the ~"technical debt" label,
+user experience refinements should use the ~"UX debt" label.
 
 Everyone can create an issue, though you may need to ask for adding a specific
 label, if you do not have permissions to do it by yourself. Additional labels
-can be combined with the `technical debt` label, to make it easier to schedule
+can be combined with these labels, to make it easier to schedule
 the improvements for a release.
 
-Issues tagged with the `technical debt` label have the same priority like issues
+Issues tagged with these labels have the same priority like issues
 that describe a new feature to be introduced in GitLab, and should be scheduled
 for a release by the appropriate person.
 
-Make sure to mention the merge request that the `technical debt` issue is
-associated with in the description of the issue.
+Make sure to mention the merge request that the ~"technical debt" issue or
+~"UX debt" issue is associated with in the description of the issue.
 
 ### Stewardship
 

Gemfile

@@ -175,7 +175,7 @@ gem 'rainbow', '~> 2.2'
 gem 'settingslogic', '~> 2.0.9'
 
 # Linear-time regex library for untrusted regular expressions
-gem 're2', '~> 1.0.0'
+gem 're2', '~> 1.1.0'
 
 # Misc
 

Gemfile.lock

@@ -620,7 +620,7 @@ GEM
     premailer-rails (1.9.7)
       actionmailer (>= 3, < 6)
       premailer (~> 1.7, >= 1.7.9)
-    prometheus-client-mmap (0.7.0.beta9)
+    prometheus-client-mmap (0.7.0.beta10)
       mmap2 (~> 2.2, >= 2.2.7)
     pry (0.10.4)
       coderay (~> 1.1.0)
@@ -685,7 +685,7 @@ GEM
       debugger-ruby_core_source (~> 1.3)
     rdoc (4.2.2)
       json (~> 1.4)
-    re2 (1.0.0)
+    re2 (1.1.0)
     recaptcha (3.0.0)
       json
     recursive-open-struct (1.0.0)
@@ -1093,7 +1093,7 @@ DEPENDENCIES
   raindrops (~> 0.18)
   rblineprof (~> 0.3.6)
   rdoc (~> 4.2)
-  re2 (~> 1.0.0)
+  re2 (~> 1.1.0)
   recaptcha (~> 3.0)
   redcarpet (~> 3.4)
   redis (~> 3.2)

app/assets/javascripts/copy_as_gfm.js

 /* eslint-disable class-methods-use-this, object-shorthand, no-unused-vars, no-use-before-define, no-new, max-len, no-restricted-syntax, guard-for-in, no-continue */
 
 import './lib/utils/common_utils';
+import { placeholderImage } from './lazy_loader';
 
 const gfmRules = {
   // The filters referenced in lib/banzai/pipeline/gfm_pipeline.rb convert
@@ -56,6 +57,11 @@ const gfmRules = {
       return text;
     },
   },
+  ImageLazyLoadFilter: {
+    'img'(el, text) {
+      return `![${el.getAttribute('alt')}](${el.getAttribute('src')})`;
+    },
+  },
   VideoLinkFilter: {
     '.video-container'(el) {
       const videoEl = el.querySelector('video');
@@ -163,7 +169,9 @@ const gfmRules = {
       return text.trim().split('\n').map(s => `> ${s}`.trim()).join('\n');
     },
     'img'(el) {
-      return `![${el.getAttribute('alt')}](${el.getAttribute('src')})`;
+      const imageSrc = el.src;
+      const imageUrl = imageSrc && imageSrc !== placeholderImage ? imageSrc : (el.dataset.src || '');
+      return `![${el.getAttribute('alt')}](${imageUrl})`;
     },
     'a.anchor'(el, text) {
       // Don't render a Markdown link for the anchor link inside a heading

app/assets/javascripts/how_to_merge.js

+document.addEventListener('DOMContentLoaded', () => {
+  const modal = $('#modal_merge_info').modal({
+    modal: true,
+    show: false,
+  });
+  $('.how_to_merge_link').bind('click', () => {
+    modal.show();
+  });
+  $('.modal-header .close').bind('click', () => {
+    modal.hide();
+  });
+});

app/assets/javascripts/lazy_loader.js

+/* eslint-disable one-export, one-var, one-var-declaration-per-line */
+
+import _ from 'underscore';
+
+export const placeholderImage = 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==';
+const SCROLL_THRESHOLD = 300;
+
+export default class LazyLoader {
+  constructor(options = {}) {
+    this.lazyImages = [];
+    this.observerNode = options.observerNode || '#content-body';
+
+    const throttledScrollCheck = _.throttle(() => this.scrollCheck(), 300);
+    const debouncedElementsInView = _.debounce(() => this.checkElementsInView(), 300);
+
+    window.addEventListener('scroll', throttledScrollCheck);
+    window.addEventListener('resize', debouncedElementsInView);
+
+    const scrollContainer = options.scrollContainer || window;
+    scrollContainer.addEventListener('load', () => this.loadCheck());
+  }
+  searchLazyImages() {
+    this.lazyImages = [].slice.call(document.querySelectorAll('.lazy'));
+    this.checkElementsInView();
+  }
+  startContentObserver() {
+    const contentNode = document.querySelector(this.observerNode) || document.querySelector('body');
+
+    if (contentNode) {
+      const observer = new MutationObserver(() => this.searchLazyImages());
+
+      observer.observe(contentNode, {
+        childList: true,
+        subtree: true,
+      });
+    }
+  }
+  loadCheck() {
+    this.searchLazyImages();
+    this.startContentObserver();
+  }
+  scrollCheck() {
+    requestAnimationFrame(() => this.checkElementsInView());
+  }
+  checkElementsInView() {
+    const scrollTop = pageYOffset;
+    const visHeight = scrollTop + innerHeight + SCROLL_THRESHOLD;
+    let imgBoundRect, imgTop, imgBound;
+
+    // Loading Images which are in the current viewport or close to them
+    this.lazyImages = this.lazyImages.filter((selectedImage) => {
+      if (selectedImage.getAttribute('data-src')) {
+        imgBoundRect = selectedImage.getBoundingClientRect();
+
+        imgTop = scrollTop + imgBoundRect.top;
+        imgBound = imgTop + imgBoundRect.height;
+
+        if (scrollTop < imgBound && visHeight > imgTop) {
+          LazyLoader.loadImage(selectedImage);
+          return false;
+        }
+
+        return true;
+      }
+      return false;
+    });
+  }
+  static loadImage(img) {
+    if (img.getAttribute('data-src')) {
+      img.setAttribute('src', img.getAttribute('data-src'));
+      img.removeAttribute('data-src');
+      img.classList.remove('lazy');
+      img.classList.add('js-lazy-loaded');
+    }
+  }
+}

app/assets/javascripts/main.js

@@ -109,6 +109,7 @@ import './label_manager';
 import './labels';
 import './labels_select';
 import './layout_nav';
+import LazyLoader from './lazy_loader';
 import './line_highlighter';
 import './logo';
 import './member_expiration_date';
@@ -174,6 +175,11 @@ window.addEventListener('load', function onLoad() {
   gl.utils.handleLocationHash();
 }, false);
 
+gl.lazyLoader = new LazyLoader({
+  scrollContainer: window,
+  observerNode: '#content-body'
+});
+
 $(function () {
   var $body = $('body');
   var $document = $(document);
@@ -292,13 +298,7 @@ $(function () {
     return $container.remove();
   // Commit show suppressed diff
   });
-  $('.navbar-toggle').on('click', function () {
-    $('.header-content .title, .header-content .navbar-sub-nav').toggle();
-    $('.header-content .header-logo').toggle();
-    $('.header-content .navbar-collapse').toggle();
-    $('.js-navbar-toggle-left, .js-navbar-toggle-right, .title-container').toggle();
-    return $('.navbar-toggle').toggleClass('active');
-  });
+  $('.navbar-toggle').on('click', () => $('.header-content').toggleClass('menu-expanded'));
   // Show/hide comments on diff
   $body.on('click', '.js-toggle-diff-comments', function (e) {
     var $this = $(this);

app/assets/javascripts/star.js

 /* eslint-disable func-names, space-before-function-paren, wrap-iife, no-unused-vars, one-var, no-var, one-var-declaration-per-line, prefer-arrow-callback, no-new, max-len */
 /* global Flash */
 
+import { __, s__ } from './locale';
+
 export default class Star {
   constructor() {
     $('.project-home-panel .toggle-star').on('ajax:success', function(e, data, status, xhr) {
@@ -11,10 +13,10 @@ export default class Star {
       toggleStar = function(isStarred) {
         $this.parent().find('.star-count').text(data.star_count);
         if (isStarred) {
-          $starSpan.removeClass('starred').text('Star');
+          $starSpan.removeClass('starred').text(s__('StarProject|Star'));
           $starIcon.removeClass('fa-star').addClass('fa-star-o');
         } else {
-          $starSpan.addClass('starred').text('Unstar');
+          $starSpan.addClass('starred').text(__('Unstar'));
           $starIcon.removeClass('fa-star-o').addClass('fa-star');
         }
       };

app/assets/stylesheets/framework/avatar.scss

@@ -35,6 +35,8 @@
   width: 40px;
   height: 40px;
   padding: 0;
+  background: $avatar-background;
+  overflow: hidden;
 
   &.avatar-inline {
     float: none;

app/assets/stylesheets/framework/calendar.scss

@@ -93,7 +93,7 @@
 
   .is-selected .pika-day,
   .pika-day:hover,
-  .is-today .pika-day:hover {
+  .is-today .pika-day {
     background: $gl-primary;
     color: $white-light;
     box-shadow: none;

app/assets/stylesheets/framework/header.scss

@@ -132,6 +132,22 @@ header {
     }
   }
 
+  &.navbar-gitlab-new {
+    .fa-times {
+      display: none;
+    }
+
+    .menu-expanded {
+      .fa-ellipsis-v {
+        display: none;
+      }
+
+      .fa-times {
+        display: block;
+      }
+    }
+  }
+
   .global-dropdown {
     position: absolute;
     left: -10px;
@@ -171,6 +187,19 @@ header {
     min-height: $header-height;
     padding-left: 30px;
 
+    &.menu-expanded {
+      @media (max-width: $screen-xs-max) {
+        .header-logo,
+        .title-container {
+          display: none;
+        }
+
+        .navbar-collapse {
+          display: block;
+        }
+      }
+    }
+
     .dropdown-menu {
       margin-top: -5px;
     }

app/assets/stylesheets/framework/nav.scss

@@ -182,6 +182,12 @@
       }
     }
 
+    &.nav-controls-new-nav {
+      > .dropdown {
+        margin-right: 0;
+      }
+    }
+
     > .btn-grouped {
       float: none;
     }

app/assets/stylesheets/framework/typography.scss

@@ -11,8 +11,17 @@
   }
 
   img {
-    max-width: 100%;
+    /*max-width: 100%;*/
     margin: 0 0 8px;
+    min-width: 200px;
+    min-height: 100px;
+    background-color: $gray-lightest;
+  }
+
+  img.js-lazy-loaded {
+    min-width: none;
+    min-height: none;
+    background-color: none;
   }
 
   p a:not(.no-attachment-icon) img {

app/assets/stylesheets/framework/variables.scss

@@ -384,7 +384,9 @@ $issue-boards-card-shadow: rgba(186, 186, 186, 0.5);
 * Avatar
 */
 $avatar_radius: 50%;
-$avatar-border: $border-color;
+$avatar-border: $gray-normal;
+$avatar-border-hover: $gray-darker;
+$avatar-background: $gray-lightest;
 $gl-avatar-size: 40px;
 
 /*

app/assets/stylesheets/pages/boards.scss

@@ -479,7 +479,10 @@
   border-top: 1px solid $border-color;
 }
 
-.page-with-layout-nav.page-with-sub-nav .issue-boards-sidebar {
+.page-with-layout-nav.page-with-sub-nav .issue-boards-sidebar,
+.page-with-new-sidebar.page-with-sidebar .issue-boards-sidebar {
+  position: absolute;
+
   &.right-sidebar {
     top: 0;
     bottom: 0;

app/controllers/sessions_controller.rb

@@ -5,6 +5,14 @@ class SessionsController < Devise::SessionsController
 
   skip_before_action :check_two_factor_requirement, only: [:destroy]
 
+  # Explicitly call protect from forgery before anything else. Otherwise the
+  # CSFR-token might be cleared before authentication is done. This was the case
+  # when LDAP was enabled and the `OmniauthCallbacksController` is loaded
+  #
+  # *Note:* `prepend: true` is the default for rails4, but this will be changed
+  # to `prepend: false` in rails5.
+  protect_from_forgery prepend: true, with: :exception
+
   prepend_before_action :check_initial_setup, only: [:new]
   prepend_before_action :authenticate_with_two_factor,
     if: :two_factor_enabled?, only: [:create]

app/helpers/avatars_helper.rb

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

app/helpers/emails_helper.rb

@@ -61,8 +61,8 @@ module EmailsHelper
     else
       image_tag(
         image_url('mailers/gitlab_header_logo.gif'),
-        size: "55x50",
-        alt: "GitLab"
+        size: '55x50',
+        alt: 'GitLab'
       )
     end
   end

app/helpers/lazy_image_tag_helper.rb

+module LazyImageTagHelper
+  def placeholder_image
+    "data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=="
+  end
+
+  # Override the default ActionView `image_tag` helper to support lazy-loading
+  def image_tag(source, options = {})
+    options = options.symbolize_keys
+
+    unless options.delete(:lazy) == false
+      options[:data] ||= {}
+      options[:data][:src] = path_to_image(source)
+      options[:class] ||= ""
+      options[:class] << " lazy"
+
+      source = placeholder_image
+    end
+
+    super(source, options)
+  end
+
+  # Required for Banzai::Filter::ImageLazyLoadFilter
+  module_function :placeholder_image
+end

app/helpers/version_check_helper.rb

@@ -2,7 +2,7 @@ module VersionCheckHelper
   def version_status_badge
     if Rails.env.production? && current_application_settings.version_check_enabled
       image_url = VersionCheck.new.url
-      image_tag image_url, class: 'js-version-status-badge'
+      image_tag image_url, class: 'js-version-status-badge', lazy: false
     end
   end
 end

app/models/concerns/cache_markdown_field.rb

@@ -11,7 +11,7 @@ module CacheMarkdownField
   extend ActiveSupport::Concern
 
   # Increment this number every time the renderer changes its output
-  CACHE_VERSION = 1
+  CACHE_VERSION = 2
 
   # changes to these attributes cause the cache to be invalidates
   INVALIDATED_BY = %w[author project].freeze

app/models/group.rb

@@ -186,10 +186,14 @@ class Group < Namespace
   end
 
   def has_owner?(user)
+    return false unless user
+
     members_with_parents.owners.where(user_id: user).any?
   end
 
   def has_master?(user)
+    return false unless user
+
     members_with_parents.masters.where(user_id: user).any?
   end
 
@@ -258,7 +262,7 @@ class Group < Namespace
   end
 
   def members_with_parents
-    GroupMember.non_request.where(source_id: ancestors.pluck(:id).push(id))
+    GroupMember.active.where(source_id: ancestors.pluck(:id).push(id)).where.not(user_id: nil)
   end
 
   def users_with_parents

app/policies/project_policy.rb

@@ -12,7 +12,8 @@ class ProjectPolicy < BasePolicy
 
   desc "User is a project owner"
   condition :owner do
-    @user && project.owner == @user || (project.group && project.group.has_owner?(@user))
+    (project.owner.present? && project.owner == @user) ||
+      project.group&.has_owner?(@user)
   end
 
   desc "Project has public builds enabled"

app/views/layouts/header/_new.html.haml

@@ -81,6 +81,6 @@
       %button.navbar-toggle.hidden-sm.hidden-md.hidden-lg{ type: 'button' }
         %span.sr-only Toggle navigation
         = icon('ellipsis-v', class: 'js-navbar-toggle-right')
-        = icon('times', class: 'js-navbar-toggle-left', style: 'display: none;')
+        = icon('times', class: 'js-navbar-toggle-left')
 
 = render 'shared/outdated_browser'

app/views/projects/blob/viewers/_image.html.haml

 .file-content.image_file
-  %img{ src: blob_raw_url, alt: viewer.blob.name }
+  %img{ 'data-src': blob_raw_url, alt: viewer.blob.name }

app/views/projects/diffs/viewers/_image.html.haml

@@ -8,7 +8,7 @@
   .image
     %span.wrap
       .frame{ class: (diff_file.deleted_file? ? 'deleted' : 'added') }
-        %img{ src: blob_raw_path, alt: diff_file.file_path }
+        %img{ 'data-src': blob_raw_path, alt: diff_file.file_path }
       %p.image-info= number_to_human_size(blob.size)
 - else
   .image
@@ -16,7 +16,7 @@
       %span.wrap
         .frame.deleted
           %a{ href: project_blob_path(@project, tree_join(diff_file.old_content_sha, diff_file.old_path)) }
-            %img{ src: old_blob_raw_path, alt: diff_file.old_path }
+            %img{ 'data-src': old_blob_raw_path, alt: diff_file.old_path }
         %p.image-info.hide
           %span.meta-filesize= number_to_human_size(old_blob.size)
           |
@@ -28,7 +28,7 @@
       %span.wrap
         .frame.added
           %a{ href: project_blob_path(@project, tree_join(diff_file.content_sha, diff_file.new_path)) }
-            %img{ src: blob_raw_path, alt: diff_file.new_path }
+            %img{ 'data-src': blob_raw_path, alt: diff_file.new_path }
         %p.image-info.hide
           %span.meta-filesize= number_to_human_size(blob.size)
           |
@@ -41,10 +41,10 @@
     .swipe.view.hide
       .swipe-frame
         .frame.deleted
-          %img{ src: old_blob_raw_path, alt: diff_file.old_path }
+          %img{ 'data-src': old_blob_raw_path, alt: diff_file.old_path }
         .swipe-wrap
           .frame.added
-            %img{ src: blob_raw_path, alt: diff_file.new_path }
+            %img{ 'data-src': blob_raw_path, alt: diff_file.new_path }
         %span.swipe-bar
           %span.top-handle
           %span.bottom-handle
@@ -52,9 +52,9 @@
     .onion-skin.view.hide
       .onion-skin-frame
         .frame.deleted
-          %img{ src: old_blob_raw_path, alt: diff_file.old_path }
+          %img{ 'data-src': old_blob_raw_path, alt: diff_file.old_path }
         .frame.added
-          %img{ src: blob_raw_path, alt: diff_file.new_path }
+          %img{ 'data-src': blob_raw_path, alt: diff_file.new_path }
         .controls
           .transparent
           .drag-track

app/views/projects/merge_requests/_how_to_merge.html.haml

+- content_for :page_specific_javascripts do
+  = webpack_bundle_tag('how_to_merge')
+
 #modal_merge_info.modal
   .modal-dialog
     .modal-content
@@ -50,14 +53,3 @@
           = succeed '.' do
             You can also checkout merge requests locally by
             = link_to 'following these guidelines', help_page_path('user/project/merge_requests/index.md', anchor: "checkout-merge-requests-locally"), target: '_blank', rel: 'noopener noreferrer'
-
-:javascript
-  $(function(){
-    var modal = $('#modal_merge_info').modal({modal: true, show:false});
-    $('.how_to_merge_link').bind("click", function(){
-      modal.show();
-    });
-    $('.modal-header .close').bind("click", function(){
-      modal.hide();
-    })
-  })

app/views/projects/milestones/index.html.haml

 - @no_container = true
 - page_title 'Milestones'
 
-- if show_new_nav?
+- if show_new_nav? && can?(current_user, :admin_milestone, @project)
   - content_for :breadcrumbs_extra do
     = link_to "New milestone", new_namespace_project_milestone_path(@project.namespace, @project), class: 'btn btn-new', title: 'New milestone'
 
@@ -11,10 +11,10 @@
   .top-area
     = render 'shared/milestones_filter', counts: milestone_counts(@project.milestones)
 
-    .nav-controls
+    .nav-controls{ class: ("nav-controls-new-nav" if show_new_nav?) }
       = render 'shared/milestones_sort_dropdown'
       - if can?(current_user, :admin_milestone, @project)
-        = link_to new_project_milestone_path(@project), class: 'btn btn-new', title: 'New milestone' do
+        = link_to new_project_milestone_path(@project), class: "btn btn-new #{("visible-xs" if show_new_nav?)}", title: 'New milestone' do
           New milestone
 
   .milestones

changelogs/unreleased/31533-usage-data-projects-stats.yml

+---
+title: Add Slack and JIRA services counts to Usage Data
+merge_request:
+author:

changelogs/unreleased/34361-lazy-load-images-on-the-frontend.yml

+---
+title: Lazy load images for better Frontend performance
+merge_request: 12503
+author: 

changelogs/unreleased/35391-fix-star-i18n-in-js.yml

+---
+title: Fix translations for Star/Unstar in JS file
+merge_request:
+author:

changelogs/unreleased/35444-error-500-viewing-notes-with-anonymous-user.yml

+---
+title: Fix anonymous access to public projects in groups with pending invites
+merge_request:
+author:

changelogs/unreleased/bvl-fix-login-issue-with-ldap-enabled.yml

+---
+title: Fix cross site request protection when logging in as a regular user when LDAP
+  is enabled
+merge_request: 13049
+author:

changelogs/unreleased/dz-fix-calendar-today.yml

+---
+title: Fix today day highlight in calendar
+merge_request: 13048
+author:

changelogs/unreleased/issue-boards-close-icon-size.yml

+---
+title: Fixed issue boards sidebar close icon size
+merge_request:
+author:

changelogs/unreleased/new-nav-duplicated-new-milestone-buttons.yml

+---
+title: Fixed duplicate new milestone buttons when new navigation is turned on
+merge_request:
+author:

changelogs/unreleased/pawel-fix-metrics-files-handling.yml

+---
+title: Fix bug causing metrics files to be truncated
+merge_request: 35420
+author:

config/webpack.config.js

@@ -45,6 +45,7 @@ var config = {
     groups_list:          './groups_list.js',
     issuable:             './issuable/issuable_bundle.js',
     issues:               './issues/issues_bundle.js',
+    how_to_merge:         './how_to_merge.js',
     issue_show:           './issue_show/index.js',
     integrations:         './integrations',
     job_details:          './jobs/job_details_bundle.js',

doc/development/code_review.md

@@ -133,6 +133,55 @@ reviewee.
   tomorrow. When you are not able to find the right balance, ask other people
   about their opinion.
 
+### GitLab-specific concerns
+
+GitLab is used in a lot of places. Many users use
+our [Omnibus packages](https://about.gitlab.com/installation/), but some use
+the [Docker images](https://docs.gitlab.com/omnibus/docker/), some are
+[installed from source](https://docs.gitlab.com/ce/install/installation.html),
+and there are other installation methods available. GitLab.com itself is a large
+Enterprise Edition instance. This has some implications:
+
+1. **Query changes** should be tested to ensure that they don't result in worse
+   performance at the scale of GitLab.com:
+  1. Generating large quantities of data locally can help.
+  2. Asking for query plans from GitLab.com is the most reliable way to validate
+     these.
+2. **Database migrations** must be:
+  1. Reversible.
+  2. Performant at the scale of GitLab.com - ask a maintainer to test the
+     migration on the staging environment if you aren't sure.
+  3. Categorised correctly:
+     - Regular migrations run before the new code is running on the instance.
+     - [Post-deployment migrations](post_deployment_migrations.md) run _after_
+       the new code is deployed, when the instance is configured to do that.
+     - [Background migrations](background_migrations.md) run in Sidekiq, and
+       should only be done for migrations that would take an extreme amount of
+       time at GitLab.com scale.
+3. **Sidekiq workers**
+   [cannot change in a backwards-incompatible way](sidekiq_style_guide.md#removing-or-renaming-queues):
+  1. Sidekiq queues are not drained before a deploy happens, so there will be
+     workers in the queue from the previous version of GitLab.
+  2. If you need to change a method signature, try to do so across two releases,
+     and accept both the old and new arguments in the first of those.
+  3. Similarly, if you need to remove a worker, stop it from being scheduled in
+     one release, then remove it in the next. This will allow existing jobs to
+     execute.
+  4. Don't forget, not every instance will upgrade to every intermediate version
+     (some people may go from X.1.0 to X.10.0, or even try bigger upgrades!), so
+     try to be liberal in accepting the old format if it is cheap to do so.
+4. **Cached values** may persist across releases. If you are changing the type a
+   cached value returns (say, from a string or nil to an array), change the
+   cache key at the same time.
+5. **Settings** should be added as a
+   [last resort](https://about.gitlab.com/handbook/product/#convention-over-configuration).
+   If you're adding a new setting in `gitlab.yml`:
+  1. Try to avoid that, and add to `ApplicationSetting` instead.
+  2. Ensure that it is also
+     [added to Omnibus](https://docs.gitlab.com/omnibus/settings/gitlab.yml.html#adding-a-new-setting-to-gitlab-yml).
+6. **Filesystem access** can be slow, so try to avoid
+   [shared files](shared_files.md) when an alternative solution is available.
+
 ### Credits
 
 Largely based on the [thoughtbot code review guide].

doc/development/fe_guide/performance.md

@@ -23,6 +23,18 @@ controlled by the server.
 1. The backend code will most likely be using etags. You do not and should not check for status
 `304 Not Modified`. The browser will transform it for you.
 
+### Lazy Loading
+
+To improve the time to first render we are using lazy loading for images. This works by setting 
+the actual image source on the `data-src` attribute. After the HTML is rendered and JavaScript is loaded, 
+the value of `data-src` will be moved to `src` automatically if the image is in the current viewport.
+
+*  Prepare images in HTML for lazy loading by renaming the `src` attribute to `data-src`
+*  If you are using the Rails `image_tag` helper, all images will be lazy-loaded by default unless `lazy: false` is provided.
+
+If you are asynchronously adding content which contains lazy images then you need to call the function
+`gl.lazyLoader.searchLazyImages()` which will search for lazy images and load them if needed.
+
 ## Reducing Asset Footprint
 
 ### Page-specific JavaScript

features/steps/project/wiki.rb

@@ -114,7 +114,7 @@ class Spinach::Features::ProjectWiki < Spinach::FeatureSteps
   end
 
   step 'Image should be shown on the page' do
-    expect(page).to have_xpath("//img[@src=\"image.jpg\"]")
+    expect(page).to have_xpath("//img[@data-src=\"image.jpg\"]")
   end
 
   step 'I click on image link' do

lib/banzai/filter/gollum_tags_filter.rb

@@ -118,7 +118,7 @@ module Banzai
         end
 
         if path
-          content_tag(:img, nil, src: path, class: 'gfm')
+          content_tag(:img, nil, data: { src: path }, class: 'gfm')
         end
       end
 

lib/banzai/filter/image_lazy_load_filter.rb

+module Banzai
+  module Filter
+    # HTML filter that moves the value of the src attribute to the data-src attribute so it can be lazy loaded
+    class ImageLazyLoadFilter < HTML::Pipeline::Filter
+      def call
+        doc.xpath('descendant-or-self::img').each do |img|
+          img['class'] ||= '' << 'lazy'
+          img['data-src'] = img['src']
+          img['src'] = LazyImageTagHelper.placeholder_image          
+        end
+        
+        doc
+      end
+    end
+  end
+end

lib/banzai/filter/image_link_filter.rb

@@ -10,7 +10,7 @@ module Banzai
           link = doc.document.create_element(
             'a',
             class: 'no-attachment-icon',
-            href: img['src'],
+            href: img['data-src'] || img['src'],
             target: '_blank',
             rel: 'noopener noreferrer'
           )

lib/banzai/filter/relative_link_filter.rb

@@ -22,6 +22,7 @@ module Banzai
 
         doc.css('img, video').each do |el|
           process_link_attr el.attribute('src')
+          process_link_attr el.attribute('data-src')
         end
 
         doc

lib/banzai/pipeline/gfm_pipeline.rb

@@ -16,6 +16,7 @@ module Banzai
           Filter::MathFilter,
           Filter::UploadLinkFilter,
           Filter::VideoLinkFilter,
+          Filter::ImageLazyLoadFilter,
           Filter::ImageLinkFilter,
           Filter::EmojiFilter,
           Filter::TableOfContentsFilter,

lib/gitlab/untrusted_regexp.rb

@@ -22,33 +22,9 @@ module Gitlab
     end
 
     def scan(text)
-      text = text.dup # modified in-place
-      results = []
-
-      loop do
-        match = scan_regexp.match(text)
-        break unless match
-
-        # Ruby scan returns empty strings, not nil
-        groups = match.to_a.map(&:to_s)
-
-        results << 
-          if regexp.number_of_capturing_groups.zero?
-            groups[0]
-          else
-            groups[1..-1]
-          end
-
-        matchsize = match.end(0)
-
-        # No further matches
-        break unless matchsize.present?
-
-        text.slice!(0, matchsize)
-        break unless text.present?
-      end
-
-      results
+      matches = scan_regexp.scan(text).to_a
+      matches.map!(&:first) if regexp.number_of_capturing_groups.zero?
+      matches
     end
 
     def replace(text, rewrite)

lib/gitlab/usage_data.rb

@@ -44,7 +44,6 @@ module Gitlab
             pages_domains: PagesDomain.count,
             projects: Project.count,
             projects_imported_from_github: Project.where(import_type: 'github').count,
-            projects_prometheus_active: PrometheusService.active.count,
             protected_branches: ProtectedBranch.count,
             releases: Release.count,
             remote_mirrors: RemoteMirror.count,
@@ -53,7 +52,7 @@ module Gitlab
             todos: Todo.count,
             uploads: Upload.count,
             web_hooks: WebHook.count
-          }.merge(service_desk_counts)
+          }.merge(service_desk_counts).merge(services_usage)
         }
       end
 
@@ -107,6 +106,18 @@ module Gitlab
           'EE'
         end
       end
+
+      def services_usage
+        types = {
+          JiraService: :projects_jira_active,
+          SlackService: :projects_slack_notifications_active,
+          SlackSlashCommandsService: :projects_slack_slash_active,
+          PrometheusService: :projects_prometheus_active
+        }
+
+        results = Service.unscoped.where(type: types.keys, active: true).group(:type).count
+        results.each_with_object({}) { |(key, value), response| response[types[key.to_sym]] = value  }
+      end
     end
   end
 end

lib/gitlab/workhorse.rb

@@ -62,7 +62,7 @@ module Gitlab
       end
 
       def send_git_blob(repository, blob)
-        params = if Gitlab::GitalyClient.feature_enabled?(:project_raw_show)
+        params = if Gitlab::GitalyClient.feature_enabled?(:workhorse_raw_show)
                    {
                      'GitalyServer' => gitaly_server_hash(repository),
                      'GetBlobRequest' => {

spec/features/admin/admin_appearance_spec.rb

@@ -63,11 +63,11 @@ feature 'Admin Appearance', feature: true do
   end
 
   def logo_selector
-    '//img[@src^="/uploads/-/system/appearance/logo"]'
+    '//img[data-src^="/uploads/-/system/appearance/logo"]'
   end
 
   def header_logo_selector
-    '//img[@src^="/uploads/-/system/appearance/header_logo"]'
+    '//img[data-src^="/uploads/-/system/appearance/header_logo"]'
   end
 
   def logo_fixture

spec/features/markdown_spec.rb

@@ -100,7 +100,7 @@ describe 'GitLab Markdown', feature: true do
       end
 
       it 'permits img elements' do
-        expect(doc).to have_selector('img[src*="smile.png"]')
+        expect(doc).to have_selector('img[data-src*="smile.png"]')
       end
 
       it 'permits br elements' do

spec/features/uploads/user_uploads_avatar_to_group_spec.rb

@@ -18,7 +18,7 @@ feature 'User uploads avatar to group', feature: true do
 
     visit group_path(group)
 
-    expect(page).to have_selector(%Q(img[src$="/uploads/-/system/group/avatar/#{group.id}/dk.png"]))
+    expect(page).to have_selector(%Q(img[data-src$="/uploads/-/system/group/avatar/#{group.id}/dk.png"]))
 
     # Cheating here to verify something that isn't user-facing, but is important
     expect(group.reload.avatar.file).to exist

spec/features/uploads/user_uploads_avatar_to_profile_spec.rb

@@ -16,7 +16,7 @@ feature 'User uploads avatar to profile', feature: true do
 
     visit user_path(user)
 
-    expect(page).to have_selector(%Q(img[src$="/uploads/-/system/user/avatar/#{user.id}/dk.png"]))
+    expect(page).to have_selector(%Q(img[data-src$="/uploads/-/system/user/avatar/#{user.id}/dk.png"]))
 
     # Cheating here to verify something that isn't user-facing, but is important
     expect(user.reload.avatar.file).to exist

spec/helpers/application_helper_spec.rb

@@ -62,13 +62,13 @@ describe ApplicationHelper do
       avatar_url = "/uploads/-/system/project/avatar/#{project.id}/banana_sample.gif"
 
       expect(helper.project_icon(project.full_path).to_s)
-        .to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />"
+        .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
 
       allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
       avatar_url = "#{gitlab_host}/uploads/-/system/project/avatar/#{project.id}/banana_sample.gif"
 
       expect(helper.project_icon(project.full_path).to_s)
-        .to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />"
+        .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
     end
 
     it 'gives uploaded icon when present' do
@@ -77,7 +77,8 @@ describe ApplicationHelper do
       allow_any_instance_of(Project).to receive(:avatar_in_git).and_return(true)
 
       avatar_url = "#{gitlab_host}#{project_avatar_path(project)}"
-      expect(helper.project_icon(project.full_path).to_s).to match(image_tag(avatar_url))
+      expect(helper.project_icon(project.full_path).to_s)
+        .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
     end
   end
 

spec/helpers/avatars_helper_spec.rb

@@ -27,11 +27,11 @@ describe AvatarsHelper do
 
     it 'displays user avatar' do
       is_expected.to eq image_tag(
-        avatar_icon(user, 16),
-        class: 'avatar has-tooltip s16 ',
+        LazyImageTagHelper.placeholder_image,
+        class: 'avatar has-tooltip s16  lazy',
         alt: "#{user.name}'s avatar",
         title: user.name,
-        data: { container: 'body' }
+        data: { container: 'body', src: avatar_icon(user, 16) }
       )
     end
 
@@ -40,22 +40,8 @@ describe AvatarsHelper do
 
       it 'uses provided css_class' do
         is_expected.to eq image_tag(
-          avatar_icon(user, 16),
-          class: "avatar has-tooltip s16 #{options[:css_class]}",
-          alt: "#{user.name}'s avatar",
-          title: user.name,
-          data: { container: 'body' }
-        )
-      end
-    end
-
-    context 'with lazy parameter' do
-      let(:options) { { user: user, lazy: true } }
-
-      it 'uses data-src instead of src' do
-        is_expected.to eq image_tag(
-          '',
-          class: 'avatar has-tooltip s16 ',
+          LazyImageTagHelper.placeholder_image,
+          class: "avatar has-tooltip s16 #{options[:css_class]}  lazy",
           alt: "#{user.name}'s avatar",
           title: user.name,
           data: { container: 'body', src: avatar_icon(user, 16) }
@@ -68,11 +54,11 @@ describe AvatarsHelper do
 
       it 'uses provided size' do
         is_expected.to eq image_tag(
-          avatar_icon(user, options[:size]),
-          class: "avatar has-tooltip s#{options[:size]} ",
+          LazyImageTagHelper.placeholder_image,
+          class: "avatar has-tooltip s#{options[:size]}  lazy",
           alt: "#{user.name}'s avatar",
           title: user.name,
-          data: { container: 'body' }
+          data: { container: 'body', src: avatar_icon(user, options[:size]) }
         )
       end
     end
@@ -82,11 +68,11 @@ describe AvatarsHelper do
 
       it 'uses provided url' do
         is_expected.to eq image_tag(
-          options[:url],
-          class: 'avatar has-tooltip s16 ',
+          LazyImageTagHelper.placeholder_image,
+          class: 'avatar has-tooltip s16  lazy',
           alt: "#{user.name}'s avatar",
           title: user.name,
-          data: { container: 'body' }
+          data: { container: 'body', src: options[:url] }
         )
       end
     end
@@ -99,22 +85,22 @@ describe AvatarsHelper do
 
         it 'prefers user parameter' do
           is_expected.to eq image_tag(
-            avatar_icon(user, 16),
-            class: 'avatar has-tooltip s16 ',
+            LazyImageTagHelper.placeholder_image,
+            class: 'avatar has-tooltip s16  lazy',
             alt: "#{user.name}'s avatar",
             title: user.name,
-            data: { container: 'body' }
+            data: { container: 'body', src: avatar_icon(user, 16) }
           )
         end
       end
 
       it 'uses user_name and user_email parameter if user is not present' do
         is_expected.to eq image_tag(
-          avatar_icon(options[:user_email], 16),
-          class: 'avatar has-tooltip s16 ',
+          LazyImageTagHelper.placeholder_image,
+          class: 'avatar has-tooltip s16  lazy',
           alt: "#{options[:user_name]}'s avatar",
           title: options[:user_name],
-          data: { container: 'body' }
+          data: { container: 'body', src: avatar_icon(options[:user_email], 16) }
         )
       end
     end

spec/javascripts/lazy_loader_spec.js

+import LazyLoader from '~/lazy_loader';
+
+let lazyLoader = null;
+
+describe('LazyLoader', function () {
+  preloadFixtures('issues/issue_with_comment.html.raw');
+
+  beforeEach(function () {
+    loadFixtures('issues/issue_with_comment.html.raw');
+    lazyLoader = new LazyLoader({
+      observerNode: 'body',
+    });
+    // Doing everything that happens normally in onload
+    lazyLoader.loadCheck();
+  });
+  describe('behavior', function () {
+    it('should copy value from data-src to src for img 1', function (done) {
+      const img = document.querySelectorAll('img[data-src]')[0];
+      const originalDataSrc = img.getAttribute('data-src');
+      img.scrollIntoView();
+
+      setTimeout(() => {
+        expect(img.getAttribute('src')).toBe(originalDataSrc);
+        expect(document.getElementsByClassName('js-lazy-loaded').length).toBeGreaterThan(0);
+        done();
+      }, 100);
+    });
+
+    it('should lazy load dynamically added data-src images', function (done) {
+      const newImg = document.createElement('img');
+      const testPath = '/img/testimg.png';
+      newImg.className = 'lazy';
+      newImg.setAttribute('data-src', testPath);
+      document.body.appendChild(newImg);
+      newImg.scrollIntoView();
+
+      setTimeout(() => {
+        expect(newImg.getAttribute('src')).toBe(testPath);
+        expect(document.getElementsByClassName('js-lazy-loaded').length).toBeGreaterThan(0);
+        done();
+      }, 100);
+    });
+
+    it('should not alter normal images', function (done) {
+      const newImg = document.createElement('img');
+      const testPath = '/img/testimg.png';
+      newImg.setAttribute('src', testPath);
+      document.body.appendChild(newImg);
+      newImg.scrollIntoView();
+
+      setTimeout(() => {
+        expect(newImg).not.toHaveClass('js-lazy-loaded');
+        done();
+      }, 100);
+    });
+  });
+});

spec/lib/banzai/filter/gollum_tags_filter_spec.rb

@@ -22,7 +22,7 @@ describe Banzai::Filter::GollumTagsFilter, lib: true do
       tag = '[[images/image.jpg]]'
       doc = filter("See #{tag}", project_wiki: project_wiki)
 
-      expect(doc.at_css('img')['src']).to eq "#{project_wiki.wiki_base_path}/images/image.jpg"
+      expect(doc.at_css('img')['data-src']).to eq "#{project_wiki.wiki_base_path}/images/image.jpg"
     end
 
     it 'does not creates img tag if image does not exist' do
@@ -40,7 +40,7 @@ describe Banzai::Filter::GollumTagsFilter, lib: true do
       tag = '[[http://example.com/image.jpg]]'
       doc = filter("See #{tag}", project_wiki: project_wiki)
 
-      expect(doc.at_css('img')['src']).to eq "http://example.com/image.jpg"
+      expect(doc.at_css('img')['data-src']).to eq "http://example.com/image.jpg"
     end
 
     it 'does not creates img tag for invalid URL' do

spec/lib/banzai/filter/image_lazy_load_filter_spec.rb

+require 'spec_helper'
+
+describe Banzai::Filter::ImageLazyLoadFilter, lib: true do
+  include FilterSpecHelper
+
+  def image(path)
+    %(<img src="#{path}" />)
+  end
+
+  it 'transforms the image src to a data-src' do
+    doc = filter(image('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
+    expect(doc.at_css('img')['data-src']).to eq '/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'
+  end
+
+  it 'works with external images' do
+    doc = filter(image('https://i.imgur.com/DfssX9C.jpg'))
+    expect(doc.at_css('img')['data-src']).to eq 'https://i.imgur.com/DfssX9C.jpg'
+  end
+end

spec/lib/gitlab/ci/trace/stream_spec.rb

@@ -308,6 +308,20 @@ describe Gitlab::Ci::Trace::Stream do
       it { is_expected.to eq('65') }
     end
 
+    context 'long line' do
+      let(:data) { 'a' * 80000 + '100%' + 'a' * 80000 }
+      let(:regex) { '\d+\%' }
+
+      it { is_expected.to eq('100') }
+    end
+
+    context 'many lines' do
+      let(:data) { "foo\n" * 80000 + "100%\n" + "foo\n" * 80000 }
+      let(:regex) { '\d+\%' }
+
+      it { is_expected.to eq('100') }
+    end
+
     context 'empty regex' do
       let(:data) { 'foo' }
       let(:regex) { '' }

spec/lib/gitlab/untrusted_regexp_spec.rb

@@ -54,8 +54,8 @@ describe Gitlab::UntrustedRegexp do
       let(:regexp) { '' }
       let(:text) { 'foo' }
 
-      it 'returns an array of empty matches' do
-        is_expected.to eq([''])
+      it 'returns an array of nil matches' do
+        is_expected.to eq([nil, nil, nil, nil])
       end
     end
 
@@ -63,8 +63,8 @@ describe Gitlab::UntrustedRegexp do
       let(:regexp) { '()' }
       let(:text) { 'foo' }
 
-      it 'returns an array of empty matches in an array' do
-        is_expected.to eq([['']])
+      it 'returns an array of nil matches in an array' do
+        is_expected.to eq([[nil], [nil], [nil], [nil]])
       end
     end
 

spec/lib/gitlab/usage_data_spec.rb

 require 'spec_helper'
 
 describe Gitlab::UsageData do
-  let!(:project) { create(:empty_project) }
-  let!(:project2) { create(:empty_project) }
-  let!(:board) { create(:board, project: project) }
+  let(:project) { create(:empty_project) }
+  let(:projects) { create_list(:project, 3) }
+  let!(:board) { create(:board, project: projects[0]) }
 
   describe '#data' do
+    before do
+      create(:jira_service, project: projects[0])
+      create(:jira_service, project: projects[1])
+      create(:prometheus_service, project: projects[1])
+      create(:service, project: projects[0], type: 'SlackSlashCommandsService', active: true)
+      create(:service, project: projects[1], type: 'SlackService', active: true)
+      create(:service, project: projects[2], type: 'SlackService', active: true)
+    end
+
     subject { described_class.data }
 
     it "gathers usage data" do
@@ -32,7 +41,7 @@ describe Gitlab::UsageData do
       count_data = subject[:counts]
 
       expect(count_data[:boards]).to eq(1)
-      expect(count_data[:projects]).to eq(2)
+      expect(count_data[:projects]).to eq(3)
 
       expect(count_data.keys).to match_array(%i(
         boards
@@ -60,6 +69,9 @@ describe Gitlab::UsageData do
         notes
         projects
         projects_imported_from_github
+        projects_jira_active
+        projects_slack_notifications_active
+        projects_slack_slash_active
         projects_prometheus_active
         pages_domains
         protected_branches
@@ -72,6 +84,16 @@ describe Gitlab::UsageData do
         web_hooks
       ))
     end
+
+    it 'gathers projects data correctly' do
+      count_data = subject[:counts]
+
+      expect(count_data[:projects]).to eq(3)
+      expect(count_data[:projects_prometheus_active]).to eq(1)
+      expect(count_data[:projects_jira_active]).to eq(2)
+      expect(count_data[:projects_slack_notifications_active]).to eq(2)
+      expect(count_data[:projects_slack_slash_active]).to eq(1)
+    end
   end
 
   describe '#license_usage_data' do
@@ -124,7 +146,7 @@ describe Gitlab::UsageData do
         allow(License).to receive(:feature_available?).with(:service_desk).and_return(true)
         allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).with(anything).and_return(true)
 
-        expect(subject).to eq(service_desk_enabled_projects: 2,
+        expect(subject).to eq(service_desk_enabled_projects: 4,
                               service_desk_issues: 3)
       end
     end

spec/lib/gitlab/workhorse_spec.rb

@@ -325,7 +325,7 @@ describe Gitlab::Workhorse, lib: true do
 
     subject { described_class.send_git_blob(repository, blob) }
 
-    context 'when Gitaly project_raw_show feature is enabled' do
+    context 'when Gitaly workhorse_raw_show feature is enabled' do
       it 'sets the header correctly' do
         key, command, params = decode_workhorse_header(subject)
 
@@ -345,7 +345,7 @@ describe Gitlab::Workhorse, lib: true do
       end
     end
 
-    context 'when Gitaly project_raw_show feature is disabled', skip_gitaly_mock: true do
+    context 'when Gitaly workhorse_raw_show feature is disabled', skip_gitaly_mock: true do
       it 'sets the header correctly' do
         key, command, params = decode_workhorse_header(subject)
 

spec/models/ability_spec.rb

@@ -98,7 +98,7 @@ describe Ability, lib: true do
         user2 = build(:user, external: true)
         users = [user1, user2]
 
-        expect(project).to receive(:owner).twice.and_return(user1)
+        expect(project).to receive(:owner).at_least(:once).and_return(user1)
 
         expect(described_class.users_that_can_read_project(users, project))
           .to eq([user1])
@@ -109,7 +109,7 @@ describe Ability, lib: true do
         user2 = build(:user, external: true)
         users = [user1, user2]
 
-        expect(project.team).to receive(:members).twice.and_return([user1])
+        expect(project.team).to receive(:members).at_least(:once).and_return([user1])
 
         expect(described_class.users_that_can_read_project(users, project))
           .to eq([user1])
@@ -140,7 +140,7 @@ describe Ability, lib: true do
         user2 = build(:user, external: true)
         users = [user1, user2]
 
-        expect(project).to receive(:owner).twice.and_return(user1)
+        expect(project).to receive(:owner).at_least(:once).and_return(user1)
 
         expect(described_class.users_that_can_read_project(users, project))
           .to eq([user1])
@@ -151,7 +151,7 @@ describe Ability, lib: true do
         user2 = build(:user, external: true)
         users = [user1, user2]
 
-        expect(project.team).to receive(:members).twice.and_return([user1])
+        expect(project.team).to receive(:members).at_least(:once).and_return([user1])
 
         expect(described_class.users_that_can_read_project(users, project))
           .to eq([user1])

spec/models/group_spec.rb

@@ -255,6 +255,7 @@ describe Group, models: true do
   describe '#has_owner?' do
     before do
       @members = setup_group_members(group)
+      create(:group_member, :invited, :owner, group: group)
     end
 
     it { expect(group.has_owner?(@members[:owner])).to be_truthy }
@@ -263,11 +264,13 @@ describe Group, models: true do
     it { expect(group.has_owner?(@members[:reporter])).to be_falsey }
     it { expect(group.has_owner?(@members[:guest])).to be_falsey }
     it { expect(group.has_owner?(@members[:requester])).to be_falsey }
+    it { expect(group.has_owner?(nil)).to be_falsey }
   end
 
   describe '#has_master?' do
     before do
       @members = setup_group_members(group)
+      create(:group_member, :invited, :master, group: group)
     end
 
     it { expect(group.has_master?(@members[:owner])).to be_falsey }
@@ -276,6 +279,7 @@ describe Group, models: true do
     it { expect(group.has_master?(@members[:reporter])).to be_falsey }
     it { expect(group.has_master?(@members[:guest])).to be_falsey }
     it { expect(group.has_master?(@members[:requester])).to be_falsey }
+    it { expect(group.has_master?(nil)).to be_falsey }
   end
 
   describe '#lfs_enabled?' do

spec/models/merge_request_spec.rb

@@ -164,6 +164,7 @@ describe MergeRequest, models: true do
       it 'does not cache issues from external trackers' do
         issue  = ExternalIssue.new('JIRA-123', subject.project)
         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 change(subject.merge_requests_closing_issues, :count)

spec/policies/project_policy_spec.rb

@@ -139,6 +139,24 @@ describe ProjectPolicy, models: true do
     end
   end
 
+  context 'when a project has pending invites, and the current user is anonymous' do
+    let(:group) { create(:group, :public) }
+    let(:project) { create(:empty_project, :public, namespace: group) }
+    let(:user_permissions) { [:read_issue_link, :create_project, :create_issue, :create_note, :upload_file] }
+    let(:anonymous_permissions) { guest_permissions - user_permissions }
+
+    subject { described_class.new(nil, project) }
+
+    before do
+      create(:group_member, :invited, group: group)
+    end
+
+    it 'does not grant owner access' do
+      expect_allowed(*anonymous_permissions)
+      expect_disallowed(*user_permissions)
+    end
+  end
+
   context 'abilities for non-public projects' do
     let(:project) { create(:empty_project, namespace: owner.namespace) }
 

spec/support/matchers/markdown_matchers.rb

@@ -17,7 +17,7 @@ module MarkdownMatchers
       image = actual.at_css('img[alt="Relative Image"]')
 
       expect(link['href']).to end_with('master/doc/README.md')
-      expect(image['src']).to end_with('master/app/assets/images/touch-icon-ipad.png')
+      expect(image['data-src']).to end_with('master/app/assets/images/touch-icon-ipad.png')
     end
   end
 
@@ -70,7 +70,7 @@ module MarkdownMatchers
   # GollumTagsFilter
   matcher :parse_gollum_tags do
     def have_image(src)
-      have_css("img[src$='#{src}']")
+      have_css("img[data-src$='#{src}']")
     end
 
     prefix = '/namespace1/gitlabhq/wikis'