Merge branch 'master' into bootstrap4

.gitlab-ci.yml

@@ -189,7 +189,7 @@ stages:
   <<: *dedicated-no-docs-and-no-qa-pull-cache-job
   <<: *use-pg
   variables:
-    CREATE_DB_USER: "true"
+    SETUP_DB: "false"
   script:
     # Manually clone gitlab-test and only seed this project in
     # db/fixtures/development/04_project.rb thanks to SIZE=1 below
@@ -233,7 +233,7 @@ stages:
 .migration-paths: &migration-paths
   <<: *dedicated-no-docs-and-no-qa-pull-cache-job
   variables:
-    CREATE_DB_USER: "true"
+    SETUP_DB: "false"
   script:
     - git fetch https://gitlab.com/gitlab-org/gitlab-ce.git v9.3.0
     - git checkout -f FETCH_HEAD
@@ -242,7 +242,7 @@ stages:
     - cp config/gitlab.yml.example config/gitlab.yml
     - bundle exec rake db:drop db:create db:schema:load db:seed_fu
     - date
-    - git checkout $CI_COMMIT_SHA
+    - git checkout -f $CI_COMMIT_SHA
     - bundle install $BUNDLE_INSTALL_FLAGS
     - date
     - . scripts/prepare_build.sh

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

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

app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_graph.js

@@ -1,4 +1,4 @@
-/* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, max-len, no-restricted-syntax, vars-on-top, no-use-before-define, no-param-reassign, new-cap, no-underscore-dangle, wrap-iife, comma-dangle, no-return-assign, prefer-arrow-callback, quotes, prefer-template, newline-per-chained-call, no-else-return, no-shadow */
+/* eslint-disable func-names, space-before-function-paren, prefer-rest-params, max-len, no-restricted-syntax, vars-on-top, no-use-before-define, no-param-reassign, new-cap, no-underscore-dangle, wrap-iife, comma-dangle, no-return-assign, prefer-arrow-callback, quotes, prefer-template, newline-per-chained-call, no-else-return, no-shadow */
 
 import $ from 'jquery';
 import _ from 'underscore';
@@ -13,17 +13,17 @@ import { dateTickFormat } from '~/lib/utils/tick_formats';
 
 const d3 = { extent, max, select, scaleTime, scaleLinear, axisLeft, axisBottom, area, brushX, timeParse };
 
-const extend = function(child, parent) { for (var key in parent) { if (hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; };
 const hasProp = {}.hasOwnProperty;
+const extend = function(child, parent) { for (const key in parent) { if (hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; };
 
 export const ContributorsGraph = (function() {
   function ContributorsGraph() {}
 
   ContributorsGraph.prototype.MARGIN = {
     top: 20,
-    right: 20,
+    right: 10,
     bottom: 30,
-    left: 50
+    left: 40
   };
 
   ContributorsGraph.prototype.x_domain = null;
@@ -32,6 +32,12 @@ export const ContributorsGraph = (function() {
 
   ContributorsGraph.prototype.dates = [];
 
+  ContributorsGraph.prototype.determine_width = function(baseWidth, $parentElement) {
+    const parentPaddingWidth = parseFloat($parentElement.css('padding-left')) + parseFloat($parentElement.css('padding-right'));
+    const marginWidth = this.MARGIN.left + this.MARGIN.right;
+    return baseWidth - parentPaddingWidth - marginWidth;
+  };
+
   ContributorsGraph.set_x_domain = function(data) {
     return ContributorsGraph.prototype.x_domain = data;
   };
@@ -105,11 +111,10 @@ export const ContributorsMasterGraph = (function(superClass) {
 
   function ContributorsMasterGraph(data1) {
     const $parentElement = $('#contributors-master');
-    const parentPadding = parseFloat($parentElement.css('padding-left')) + parseFloat($parentElement.css('padding-right'));
 
     this.data = data1;
     this.update_content = this.update_content.bind(this);
-    this.width = $('.content').width() - parentPadding - (this.MARGIN.left + this.MARGIN.right);
+    this.width = this.determine_width($('.js-graphs-show').width(), $parentElement);
     this.height = 200;
     this.x = null;
     this.y = null;
@@ -122,8 +127,7 @@ export const ContributorsMasterGraph = (function(superClass) {
   }
 
   ContributorsMasterGraph.prototype.process_dates = function(data) {
-    var dates;
-    dates = this.get_dates(data);
+    const dates = this.get_dates(data);
     this.parse_dates(data);
     return ContributorsGraph.set_dates(dates);
   };
@@ -133,8 +137,7 @@ export const ContributorsMasterGraph = (function(superClass) {
   };
 
   ContributorsMasterGraph.prototype.parse_dates = function(data) {
-    var parseDate;
-    parseDate = d3.timeParse("%Y-%m-%d");
+    const parseDate = d3.timeParse("%Y-%m-%d");
     return data.forEach(function(d) {
       return d.date = parseDate(d.date);
     });
@@ -152,7 +155,14 @@ export const ContributorsMasterGraph = (function(superClass) {
   };
 
   ContributorsMasterGraph.prototype.create_svg = function() {
-    return this.svg = d3.select("#contributors-master").append("svg").attr("width", this.width + this.MARGIN.left + this.MARGIN.right).attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom).attr("class", "tint-box").append("g").attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")");
+    this.svg = d3.select("#contributors-master")
+      .append("svg")
+      .attr("width", this.width + this.MARGIN.left + this.MARGIN.right)
+      .attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom)
+      .attr("class", "tint-box")
+      .append("g")
+      .attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")");
+    return this.svg;
   };
 
   ContributorsMasterGraph.prototype.create_area = function(x, y) {
@@ -218,12 +228,14 @@ export const ContributorsAuthorGraph = (function(superClass) {
   extend(ContributorsAuthorGraph, superClass);
 
   function ContributorsAuthorGraph(data1) {
+    const $parentElements = $('.person');
+
     this.data = data1;
     // Don't split graph size in half for mobile devices.
-    if ($(window).width() < 768) {
-      this.width = $('.content').width() - 80;
+    if ($(window).width() < 790) {
+      this.width = this.determine_width($('.js-graphs-show').width(), $parentElements);
     } else {
-      this.width = ($('.content').width() / 2) - 100;
+      this.width = this.determine_width($('.js-graphs-show').width() / 2, $parentElements);
     }
     this.height = 200;
     this.x = null;
@@ -249,8 +261,7 @@ export const ContributorsAuthorGraph = (function(superClass) {
 
   ContributorsAuthorGraph.prototype.create_area = function(x, y) {
     return this.area = d3.area().x(function(d) {
-      var parseDate;
-      parseDate = d3.timeParse("%Y-%m-%d");
+      const parseDate = d3.timeParse("%Y-%m-%d");
       return x(parseDate(d));
     }).y0(this.height).y1((function(_this) {
       return function(d) {
@@ -264,9 +275,16 @@ export const ContributorsAuthorGraph = (function(superClass) {
   };
 
   ContributorsAuthorGraph.prototype.create_svg = function() {
-    var persons = document.querySelectorAll('.person');
+    const persons = document.querySelectorAll('.person');
     this.list_item = persons[persons.length - 1];
-    return this.svg = d3.select(this.list_item).append("svg").attr("width", this.width + this.MARGIN.left + this.MARGIN.right).attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom).attr("class", "spark").append("g").attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")");
+    this.svg = d3.select(this.list_item)
+      .append("svg")
+      .attr("width", this.width + this.MARGIN.left + this.MARGIN.right)
+      .attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom)
+      .attr("class", "spark")
+      .append("g")
+      .attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")");
+    return this.svg;
   };
 
   ContributorsAuthorGraph.prototype.draw_path = function(data) {

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

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

app/assets/stylesheets/pages/boards.scss

@@ -1,5 +1,3 @@
-@import './issues/issue_count_badge';
-
 [v-cloak] {
   display: none;
 }

app/assets/stylesheets/pages/issues.scss

@@ -1,5 +1,3 @@
-@import "./issues/issue_count_badge";
-
 .issues-list {
   .issue {
     padding: 10px 0 10px $gl-padding;

app/assets/stylesheets/pages/notes.scss

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

app/controllers/admin/dashboard_controller.rb

@@ -1,4 +1,6 @@
 class Admin::DashboardController < Admin::ApplicationController
+  include CountHelper
+
   def index
     @projects = Project.order_id_desc.without_deleted.with_route.limit(10)
     @users = User.order_id_desc.limit(10)

app/controllers/concerns/accepts_pending_invitations.rb

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

app/controllers/confirmations_controller.rb

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

app/controllers/projects/commit_controller.rb

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

app/controllers/projects/settings/ci_cd_controller.rb

@@ -69,7 +69,7 @@ module Projects
         @project_runners = @project.runners.ordered
 
         @assignable_runners = current_user
-          .ci_authorized_runners
+          .ci_owned_runners
           .assignable_for(project)
           .ordered
           .page(params[:page]).per(20)

app/controllers/registrations_controller.rb

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

app/helpers/count_helper.rb

@@ -0,0 +1,5 @@
+module CountHelper
+  def approximate_count_with_delimiters(model)
+    number_with_delimiter(Gitlab::Database::Count.approximate_count(model))
+  end
+end

app/models/appearance.rb

@@ -2,6 +2,7 @@ class Appearance < ActiveRecord::Base
   include CacheMarkdownField
   include AfterCommitQueue
   include ObjectStorage::BackgroundMove
+  include WithUploads
 
   cache_markdown_field :description
   cache_markdown_field :new_project_guidelines
@@ -14,8 +15,6 @@ class Appearance < ActiveRecord::Base
   mount_uploader :logo,         AttachmentUploader
   mount_uploader :header_logo,  AttachmentUploader
 
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-
   CACHE_KEY = "current_appearance:#{Gitlab::VERSION}".freeze
 
   after_commit :flush_redis_cache

app/models/ci/runner.rb

@@ -52,7 +52,7 @@ module Ci
       #        Without that, placeholders would miss one and couldn't match.
       where(locked: false)
         .where.not("ci_runners.id IN (#{project.runners.select(:id).to_sql})")
-        .specific
+        .project_type
     end
 
     validate :tag_constraints

app/models/concerns/with_uploads.rb

@@ -0,0 +1,39 @@
+# Mounted uploaders are destroyed by carrierwave's after_commit
+# hook. This hook fetches upload location (local vs remote) from
+# Upload model. So it's neccessary to make sure that during that
+# after_commit hook model's associated uploads are not deleted yet.
+# IOW we can not use dependent: :destroy :
+# has_many :uploads, as: :model, dependent: :destroy
+#
+# And because not-mounted uploads require presence of upload's
+# object model when destroying them (FileUploader's `build_upload` method
+# references `model` on delete), we can not use after_commit hook for these
+# uploads.
+#
+# Instead FileUploads are destroyed in before_destroy hook and remaining uploads
+# are destroyed by the carrierwave's after_commit hook.
+
+module WithUploads
+  extend ActiveSupport::Concern
+
+  # Currently there is no simple way how to select only not-mounted
+  # uploads, it should be all FileUploaders so we select them by
+  # `uploader` class
+  FILE_UPLOADERS = %w(PersonalFileUploader NamespaceFileUploader FileUploader).freeze
+
+  included do
+    has_many :uploads, as: :model
+
+    before_destroy :destroy_file_uploads
+  end
+
+  # mounted uploads are deleted in carrierwave's after_commit hook,
+  # but FileUploaders which are not mounted must be deleted explicitly and
+  # it can not be done in after_commit because FileUploader requires loads
+  # associated model on destroy (which is already deleted in after_commit)
+  def destroy_file_uploads
+    self.uploads.where(uploader: FILE_UPLOADERS).find_each do |upload|
+      upload.destroy
+    end
+  end
+end

app/models/group.rb

@@ -10,6 +10,7 @@ class Group < Namespace
   include LoadedInGroupList
   include GroupDescendant
   include TokenAuthenticatable
+  include WithUploads
 
   has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent
   alias_method :members, :group_members
@@ -30,8 +31,6 @@ class Group < Namespace
   has_many :variables, class_name: 'Ci::GroupVariable'
   has_many :custom_attributes, class_name: 'GroupCustomAttribute'
 
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-
   has_many :boards
   has_many :badges, class_name: 'GroupBadge'
 

app/models/project.rb

@@ -23,6 +23,7 @@ class Project < ActiveRecord::Base
   include ::Gitlab::Utils::StrongMemoize
   include ChronicDurationAttribute
   include FastDestroyAll::Helpers
+  include WithUploads
 
   extend Gitlab::ConfigHelper
 
@@ -301,8 +302,6 @@ class Project < ActiveRecord::Base
     inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } }
   validates :variables, variable_duplicates: { scope: :environment_scope }
 
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-
   # Scopes
   scope :pending_delete, -> { where(pending_delete: true) }
   scope :without_deleted, -> { where(pending_delete: false) }

app/models/user.rb

@@ -17,6 +17,7 @@ class User < ActiveRecord::Base
   include IgnorableColumn
   include BulkMemberAccessLoad
   include BlocksJsonSerialization
+  include WithUploads
 
   DEFAULT_NOTIFICATION_LEVEL = :participating
 
@@ -137,7 +138,6 @@ class User < ActiveRecord::Base
 
   has_many :custom_attributes, class_name: 'UserCustomAttribute'
   has_many :callouts, class_name: 'UserCallout'
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :term_agreements
   belongs_to :accepted_term, class_name: 'ApplicationSetting::Term'
 
@@ -860,6 +860,16 @@ class User < ActiveRecord::Base
     confirmed? && !temp_oauth_email?
   end
 
+  def accept_pending_invitations!
+    pending_invitations.select do |member|
+      member.accept_invite!(self)
+    end
+  end
+
+  def pending_invitations
+    Member.where(invite_email: verified_emails).invite
+  end
+
   def all_emails
     all_emails = []
     all_emails << email unless temp_oauth_email?
@@ -999,12 +1009,19 @@ class User < ActiveRecord::Base
     !solo_owned_groups.present?
   end
 
-  def ci_authorized_runners
-    @ci_authorized_runners ||= begin
-      runner_ids = Ci::RunnerProject
+  def ci_owned_runners
+    @ci_owned_runners ||= begin
+      project_runner_ids = Ci::RunnerProject
         .where(project: authorized_projects(Gitlab::Access::MASTER))
         .select(:runner_id)
-      Ci::Runner.specific.where(id: runner_ids)
+
+      group_runner_ids = Ci::RunnerNamespace
+        .where(namespace_id: owned_or_masters_groups.select(:id))
+        .select(:runner_id)
+
+      union = Gitlab::SQL::Union.new([project_runner_ids, group_runner_ids])
+
+      Ci::Runner.specific.where("ci_runners.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
     end
   end
 
@@ -1205,6 +1222,11 @@ class User < ActiveRecord::Base
       !terms_accepted?
   end
 
+  def owned_or_masters_groups
+    union = Gitlab::SQL::Union.new([owned_groups, masters_groups])
+    Group.from("(#{union.to_sql}) namespaces")
+  end
+
   protected
 
   # override, from Devise::Validatable

app/policies/ci/runner_policy.rb

@@ -1,16 +1,19 @@
 module Ci
   class RunnerPolicy < BasePolicy
-    with_options scope: :subject, score: 0
-    condition(:shared) { @subject.is_shared? }
-
     with_options scope: :subject, score: 0
     condition(:locked, scope: :subject) { @subject.locked? }
 
-    condition(:authorized_runner) { @user.ci_authorized_runners.include?(@subject) }
+    condition(:owned_runner) { @user.ci_owned_runners.exists?(@subject.id) }
 
     rule { anonymous }.prevent_all
-    rule { admin | authorized_runner }.enable :assign_runner
-    rule { ~admin & shared }.prevent :assign_runner
+
+    rule { admin | owned_runner }.policy do
+      enable :assign_runner
+      enable :read_runner
+      enable :update_runner
+      enable :delete_runner
+    end
+
     rule { ~admin & locked }.prevent :assign_runner
   end
 end

app/views/admin/dashboard/index.html.haml

@@ -10,7 +10,7 @@
             = link_to admin_projects_path do
               %h3.text-center
                 Projects:
-                = number_with_delimiter(Project.cached_count)
+                = approximate_count_with_delimiters(Project)
             %hr
             = link_to('New project', new_project_path, class: "btn btn-new")
       .col-sm-4
@@ -19,7 +19,7 @@
             = link_to admin_users_path do
               %h3.text-center
                 Users:
-                = number_with_delimiter(User.count)
+                = approximate_count_with_delimiters(User)
             %hr
             = link_to 'New user', new_admin_user_path, class: "btn btn-new"
       .col-sm-4
@@ -28,7 +28,7 @@
             = link_to admin_groups_path do
               %h3.text-center
                 Groups:
-                = number_with_delimiter(Group.count)
+                = approximate_count_with_delimiters(Group)
             %hr
             = link_to 'New group', new_admin_group_path, class: "btn btn-new"
     .row
@@ -39,31 +39,31 @@
             %p
               Forks
               %span.light.float-right
-                = number_with_delimiter(ForkedProjectLink.count)
+                = approximate_count_with_delimiters(ForkedProjectLink)
             %p
               Issues
               %span.light.float-right
-                = number_with_delimiter(Issue.count)
+                = approximate_count_with_delimiters(Issue)
             %p
               Merge Requests
               %span.light.float-right
-                = number_with_delimiter(MergeRequest.count)
+                = approximate_count_with_delimiters(MergeRequest)
             %p
               Notes
               %span.light.float-right
-                = number_with_delimiter(Note.count)
+                = approximate_count_with_delimiters(Note)
             %p
               Snippets
               %span.light.float-right
-                = number_with_delimiter(Snippet.count)
+                = approximate_count_with_delimiters(Snippet)
             %p
               SSH Keys
               %span.light.float-right
-                = number_with_delimiter(Key.count)
+                = approximate_count_with_delimiters(Key)
             %p
               Milestones
               %span.light.float-right
-                = number_with_delimiter(Milestone.count)
+                = approximate_count_with_delimiters(Milestone)
             %p
               Active Users
               %span.light.float-right

app/views/notify/member_invited_email.html.haml

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

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

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

changelogs/unreleased/22647-width-contributors-graphs.yml

@@ -0,0 +1,5 @@
+---
+title: Fix width of contributors graphs
+merge_request: 18639
+author: Paul Vorbach
+type: fixed

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

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

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

@@ -0,0 +1,5 @@
+---
+title: Add dot to separate system notes content
+merge_request: 18864
+author:
+type: changed

changelogs/unreleased/jprovazn-remote-upload-destroy.yml

@@ -0,0 +1,5 @@
+---
+title: Fix deletion of Object Store uploads
+merge_request:
+author:
+type: fixed

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

@@ -0,0 +1,5 @@
+---
+title: Workhorse to send raw diff and patch for commits
+merge_request:
+author:
+type: other

lib/api/groups.rb

@@ -165,6 +165,7 @@ module API
         group = find_group!(params[:id])
         authorize! :admin_group, group
 
+        Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/46285')
         destroy_conditionally!(group) do |group|
           ::Groups::DestroyService.new(group, current_user).execute
         end

lib/api/runners.rb

@@ -14,7 +14,7 @@ module API
         use :pagination
       end
       get do
-        runners = filter_runners(current_user.ci_authorized_runners, params[:scope], without: %w(specific shared))
+        runners = filter_runners(current_user.ci_owned_runners, params[:scope], without: %w(specific shared))
         present paginate(runners), with: Entities::Runner
       end
 
@@ -184,40 +184,35 @@ module API
       def authenticate_show_runner!(runner)
         return if runner.is_shared || current_user.admin?
 
-        forbidden!("No access granted") unless user_can_access_runner?(runner)
+        forbidden!("No access granted") unless can?(current_user, :read_runner, runner)
       end
 
       def authenticate_update_runner!(runner)
         return if current_user.admin?
 
-        forbidden!("Runner is shared") if runner.is_shared?
-        forbidden!("No access granted") unless user_can_access_runner?(runner)
+        forbidden!("No access granted") unless can?(current_user, :update_runner, runner)
       end
 
       def authenticate_delete_runner!(runner)
         return if current_user.admin?
 
-        forbidden!("Runner is shared") if runner.is_shared?
         forbidden!("Runner associated with more than one project") if runner.projects.count > 1
-        forbidden!("No access granted") unless user_can_access_runner?(runner)
+        forbidden!("No access granted") unless can?(current_user, :delete_runner, runner)
       end
 
       def authenticate_enable_runner!(runner)
-        forbidden!("Runner is shared") if runner.is_shared?
-        forbidden!("Runner is locked") if runner.locked?
+        forbidden!("Runner is a group runner") if runner.group_type?
+
         return if current_user.admin?
 
-        forbidden!("No access granted") unless user_can_access_runner?(runner)
+        forbidden!("Runner is locked") if runner.locked?
+        forbidden!("No access granted") unless can?(current_user, :assign_runner, runner)
       end
 
       def authenticate_list_runners_jobs!(runner)
         return if current_user.admin?
 
-        forbidden!("No access granted") unless user_can_access_runner?(runner)
-      end
-
-      def user_can_access_runner?(runner)
-        current_user.ci_authorized_runners.exists?(runner.id)
+        forbidden!("No access granted") unless can?(current_user, :read_runner, runner)
       end
     end
   end

lib/api/v3/groups.rb

@@ -131,6 +131,7 @@ module API
         delete ":id" do
           group = find_group!(params[:id])
           authorize! :admin_group, group
+          Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/46285')
           present ::Groups::DestroyService.new(group, current_user).execute, with: Entities::GroupDetail, current_user: current_user
         end
 

lib/api/v3/runners.rb

@@ -58,7 +58,7 @@ module API
         end
 
         def user_can_access_runner?(runner)
-          current_user.ci_authorized_runners.exists?(runner.id)
+          current_user.ci_owned_runners.exists?(runner.id)
         end
       end
     end

lib/gitlab/database/count.rb

@@ -0,0 +1,48 @@
+# For large tables, PostgreSQL can take a long time to count rows due to MVCC.
+# We can optimize this by using the reltuples count as described in https://wiki.postgresql.org/wiki/Slow_Counting.
+module Gitlab
+  module Database
+    module Count
+      CONNECTION_ERRORS =
+        if defined?(PG)
+          [
+            ActionView::Template::Error,
+            ActiveRecord::StatementInvalid,
+            PG::Error
+          ].freeze
+        else
+          [
+            ActionView::Template::Error,
+            ActiveRecord::StatementInvalid
+          ].freeze
+        end
+
+      def self.approximate_count(model)
+        return model.count unless Gitlab::Database.postgresql?
+
+        execute_estimate_if_updated_recently(model) || model.count
+      end
+
+      def self.execute_estimate_if_updated_recently(model)
+        ActiveRecord::Base.connection.select_value(postgresql_estimate_query(model)).to_i if reltuples_updated_recently?(model)
+      rescue *CONNECTION_ERRORS
+      end
+
+      def self.reltuples_updated_recently?(model)
+        time = "to_timestamp(#{1.hour.ago.to_i})"
+        query = <<~SQL
+          SELECT 1 FROM pg_stat_user_tables WHERE relname = '#{model.table_name}' AND
+          (last_vacuum > #{time} OR last_autovacuum > #{time} OR last_analyze > #{time} OR last_autoanalyze > #{time})
+        SQL
+
+        ActiveRecord::Base.connection.select_all(query).count > 0
+      rescue *CONNECTION_ERRORS
+        false
+      end
+
+      def self.postgresql_estimate_query(model)
+        "SELECT reltuples::bigint AS estimate FROM pg_class where relname = '#{model.table_name}'"
+      end
+    end
+  end
+end

lib/gitlab/git/commit.rb

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

scripts/create_mysql_user.sh

@@ -1,7 +1,6 @@
 #!/bin/bash
 
 mysql --user=root --host=mysql <<EOF
-CREATE DATABASE IF NOT EXISTS gitlabhq_test DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;
 CREATE USER IF NOT EXISTS 'gitlab'@'%';
 GRANT ALL PRIVILEGES ON gitlabhq_test.* TO 'gitlab'@'%';
 FLUSH PRIVILEGES;

scripts/create_postgres_user.sh

@@ -1,8 +1,6 @@
 #!/bin/bash
 
 psql -h postgres -U postgres postgres <<EOF
-DROP DATABASE IF EXISTS gitlabhq_test;
-CREATE DATABASE gitlabhq_test;
 CREATE USER gitlab;
-GRANT ALL PRIVILEGES ON DATABASE gitlabhq_test TO gitlab;
+GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO gitlab;
 EOF

scripts/prepare_build.sh

@@ -49,20 +49,8 @@ sed -i 's/localhost/redis/g' config/redis.queues.yml
 cp config/redis.shared_state.yml.example config/redis.shared_state.yml
 sed -i 's/localhost/redis/g' config/redis.shared_state.yml
 
-# Some tasks (e.g. db:seed_fu) need to have a properly-configured database
-# user but not necessarily a full schema loaded
-if [ "$CREATE_DB_USER" != "false" ]; then
-    if [ "$GITLAB_DATABASE" = 'postgresql' ]; then
-        . scripts/create_postgres_user.sh
-    else
-        . scripts/create_mysql_user.sh
-    fi
-fi
-
 if [ "$SETUP_DB" != "false" ]; then
-    bundle exec rake db:drop db:create db:schema:load db:migrate
-
-    if [ "$GITLAB_DATABASE" = "mysql" ]; then
-        bundle exec rake add_limits_mysql
-    fi
+    setup_db
+elif getent hosts postgres || getent hosts mysql; then
+    setup_db_user_only
 fi

scripts/utils.sh

@@ -12,3 +12,21 @@ retry() {
     done
     return 1
 }
+
+setup_db_user_only() {
+    if [ "$GITLAB_DATABASE" = "postgresql" ]; then
+        . scripts/create_postgres_user.sh
+    else
+        . scripts/create_mysql_user.sh
+    fi
+}
+
+setup_db() {
+    setup_db_user_only
+
+    bundle exec rake db:drop db:create db:schema:load db:migrate
+
+    if [ "$GITLAB_DATABASE" = "mysql" ]; then
+        bundle exec rake add_limits_mysql
+    fi
+}

spec/controllers/projects/commit_controller_spec.rb

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

spec/controllers/projects/settings/ci_cd_controller_spec.rb

@@ -19,11 +19,11 @@ describe Projects::Settings::CiCdController do
     end
 
     context 'with group runners' do
-      let(:group_runner) { create(:ci_runner) }
+      let(:group_runner) { create(:ci_runner, runner_type: :group_type) }
       let(:parent_group) { create(:group) }
       let(:group) { create(:group, runners: [group_runner], parent: parent_group) }
       let(:other_project) { create(:project, group: group) }
-      let!(:project_runner) { create(:ci_runner, projects: [other_project]) }
+      let!(:project_runner) { create(:ci_runner, projects: [other_project], runner_type: :project_type) }
       let!(:shared_runner) { create(:ci_runner, :shared) }
 
       it 'sets assignable project runners only' do
@@ -31,7 +31,7 @@ describe Projects::Settings::CiCdController do
 
         get :show, namespace_id: project.namespace, project_id: project
 
-        expect(assigns(:assignable_runners)).to eq [project_runner]
+        expect(assigns(:assignable_runners)).to contain_exactly(project_runner)
       end
     end
   end

spec/features/invites_spec.rb

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

spec/lib/gitlab/database/count_spec.rb

@@ -0,0 +1,62 @@
+require 'spec_helper'
+
+describe Gitlab::Database::Count do
+  before do
+    create_list(:project, 3)
+  end
+
+  describe '.execute_estimate_if_updated_recently', :postgresql do
+    context 'when reltuples have not been updated' do
+      before do
+        expect(described_class).to receive(:reltuples_updated_recently?).and_return(false)
+      end
+
+      it 'returns nil' do
+        expect(described_class.execute_estimate_if_updated_recently(Project)).to be nil
+      end
+    end
+
+    context 'when reltuples have been updated' do
+      before do
+        ActiveRecord::Base.connection.execute('ANALYZE projects')
+      end
+
+      it 'calls postgresql_estimate_query' do
+        expect(described_class).to receive(:postgresql_estimate_query).with(Project).and_call_original
+        expect(described_class.execute_estimate_if_updated_recently(Project)).to eq(3)
+      end
+    end
+  end
+
+  describe '.approximate_count' do
+    context 'when reltuples have not been updated' do
+      it 'counts all projects the normal way' do
+        allow(described_class).to receive(:reltuples_updated_recently?).and_return(false)
+
+        expect(Project).to receive(:count).and_call_original
+        expect(described_class.approximate_count(Project)).to eq(3)
+      end
+    end
+
+    context 'no permission' do
+      it 'falls back to standard query' do
+        allow(described_class).to receive(:reltuples_updated_recently?).and_raise(PG::InsufficientPrivilege)
+
+        expect(Project).to receive(:count).and_call_original
+        expect(described_class.approximate_count(Project)).to eq(3)
+      end
+    end
+
+    describe 'when reltuples have been updated', :postgresql do
+      before do
+        ActiveRecord::Base.connection.execute('ANALYZE projects')
+      end
+
+      it 'counts all projects in the fast way' do
+        expect(described_class).to receive(:postgresql_estimate_query).with(Project).and_call_original
+
+        expect(described_class.approximate_count(Project)).to eq(3)
+      end
+    end
+  end
+end

spec/lib/gitlab/git/commit_spec.rb

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

spec/mailers/notify_spec.rb

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

spec/models/appearance_spec.rb

@@ -5,7 +5,7 @@ describe Appearance do
 
   it { is_expected.to be_valid }
 
-  it { is_expected.to have_many(:uploads).dependent(:destroy) }
+  it { is_expected.to have_many(:uploads) }
 
   describe '.current', :use_clean_rails_memory_store_caching do
     let!(:appearance) { create(:appearance) }
@@ -41,4 +41,12 @@ describe Appearance do
       expect(new_row.valid?).to eq(false)
     end
   end
+
+  context 'with uploads' do
+    it_behaves_like 'model with mounted uploader', false do
+      let(:model_object) { create(:appearance, :with_logo) }
+      let(:upload_attribute) { :logo }
+      let(:uploader_class) { AttachmentUploader }
+    end
+  end
 end

spec/models/ci/runner_spec.rb

@@ -626,62 +626,26 @@ describe Ci::Runner do
   end
 
   describe '.assignable_for' do
-    let(:runner) { create(:ci_runner) }
+    let!(:unlocked_project_runner) { create(:ci_runner, runner_type: :project_type, projects: [project]) }
+    let!(:locked_project_runner) { create(:ci_runner, runner_type: :project_type, locked: true, projects: [project]) }
+    let!(:group_runner) { create(:ci_runner, runner_type: :group_type) }
+    let!(:instance_runner) { create(:ci_runner, :shared) }
     let(:project) { create(:project) }
     let(:another_project) { create(:project) }
 
-    before do
-      project.runners << runner
+    context 'with already assigned project' do
+      subject { described_class.assignable_for(project) }
+
+      it { is_expected.to be_empty }
     end
 
-    context 'with shared runners' do
-      before do
-        runner.update(is_shared: true)
-      end
+    context 'with a different project' do
+      subject { described_class.assignable_for(another_project) }
 
-      context 'does not give owned runner' do
-        subject { described_class.assignable_for(project) }
-
-        it { is_expected.to be_empty }
-      end
-
-      context 'does not give shared runner' do
-        subject { described_class.assignable_for(another_project) }
-
-        it { is_expected.to be_empty }
-      end
-    end
-
-    context 'with unlocked runner' do
-      context 'does not give owned runner' do
-        subject { described_class.assignable_for(project) }
-
-        it { is_expected.to be_empty }
-      end
-
-      context 'does give a specific runner' do
-        subject { described_class.assignable_for(another_project) }
-
-        it { is_expected.to contain_exactly(runner) }
-      end
-    end
-
-    context 'with locked runner' do
-      before do
-        runner.update(locked: true)
-      end
-
-      context 'does not give owned runner' do
-        subject { described_class.assignable_for(project) }
-
-        it { is_expected.to be_empty }
-      end
-
-      context 'does not give a locked runner' do
-        subject { described_class.assignable_for(another_project) }
-
-        it { is_expected.to be_empty }
-      end
+      it { is_expected.to include(unlocked_project_runner) }
+      it { is_expected.not_to include(group_runner) }
+      it { is_expected.not_to include(locked_project_runner) }
+      it { is_expected.not_to include(instance_runner) }
     end
   end
 

spec/models/commit_spec.rb

@@ -182,7 +182,6 @@ eos
     it { is_expected.to respond_to(:date) }
     it { is_expected.to respond_to(:diffs) }
     it { is_expected.to respond_to(:id) }
-    it { is_expected.to respond_to(:to_patch) }
   end
 
   describe '#closes_issues' do

spec/models/group_spec.rb

@@ -15,7 +15,7 @@ describe Group do
     it { is_expected.to have_many(:notification_settings).dependent(:destroy) }
     it { is_expected.to have_many(:labels).class_name('GroupLabel') }
     it { is_expected.to have_many(:variables).class_name('Ci::GroupVariable') }
-    it { is_expected.to have_many(:uploads).dependent(:destroy) }
+    it { is_expected.to have_many(:uploads) }
     it { is_expected.to have_one(:chat_team) }
     it { is_expected.to have_many(:custom_attributes).class_name('GroupCustomAttribute') }
     it { is_expected.to have_many(:badges).class_name('GroupBadge') }
@@ -691,4 +691,12 @@ describe Group do
       end
     end
   end
+
+  context 'with uploads' do
+    it_behaves_like 'model with mounted uploader', true do
+      let(:model_object) { create(:group, :with_avatar) }
+      let(:upload_attribute) { :avatar }
+      let(:uploader_class) { AttachmentUploader }
+    end
+  end
 end

spec/models/project_spec.rb

@@ -76,7 +76,7 @@ describe Project do
     it { is_expected.to have_many(:project_group_links) }
     it { is_expected.to have_many(:notification_settings).dependent(:delete_all) }
     it { is_expected.to have_many(:forks).through(:forked_project_links) }
-    it { is_expected.to have_many(:uploads).dependent(:destroy) }
+    it { is_expected.to have_many(:uploads) }
     it { is_expected.to have_many(:pipeline_schedules) }
     it { is_expected.to have_many(:members_and_requesters) }
     it { is_expected.to have_many(:clusters) }
@@ -3739,4 +3739,12 @@ describe Project do
       it { is_expected.to be_nil }
     end
   end
+
+  context 'with uploads' do
+    it_behaves_like 'model with mounted uploader', true do
+      let(:model_object) { create(:project, :with_avatar) }
+      let(:upload_attribute) { :avatar }
+      let(:uploader_class) { AttachmentUploader }
+    end
+  end
 end

spec/models/user_spec.rb

@@ -39,7 +39,7 @@ describe User do
     it { is_expected.to have_many(:builds).dependent(:nullify) }
     it { is_expected.to have_many(:pipelines).dependent(:nullify) }
     it { is_expected.to have_many(:chat_names).dependent(:destroy) }
-    it { is_expected.to have_many(:uploads).dependent(:destroy) }
+    it { is_expected.to have_many(:uploads) }
     it { is_expected.to have_many(:reported_abuse_reports).dependent(:destroy).class_name('AbuseReport') }
     it { is_expected.to have_many(:custom_attributes).class_name('UserCustomAttribute') }
 
@@ -1223,6 +1223,24 @@ describe User do
     end
   end
 
+  describe '#accept_pending_invitations!' do
+    let(:user) { create(:user, email: 'user@email.com') }
+    let!(:project_member_invite) { create(:project_member, :invited, invite_email: user.email) }
+    let!(:group_member_invite) { create(:group_member, :invited, invite_email: user.email) }
+    let!(:external_project_member_invite) { create(:project_member, :invited, invite_email: 'external@email.com') }
+    let!(:external_group_member_invite) { create(:group_member, :invited, invite_email: 'external@email.com') }
+
+    it 'accepts all the user members pending invitations and returns the accepted_members' do
+      accepted_members = user.accept_pending_invitations!
+
+      expect(accepted_members).to match_array([project_member_invite, group_member_invite])
+      expect(group_member_invite.reload).not_to be_invite
+      expect(project_member_invite.reload).not_to be_invite
+      expect(external_project_member_invite.reload).to be_invite
+      expect(external_group_member_invite.reload).to be_invite
+    end
+  end
+
   describe '#all_emails' do
     let(:user) { create(:user) }
 
@@ -1786,28 +1804,54 @@ describe User do
     end
   end
 
-  describe '#ci_authorized_runners' do
+  describe '#ci_owned_runners' do
     let(:user) { create(:user) }
-    let(:runner) { create(:ci_runner) }
+    let(:runner_1) { create(:ci_runner) }
+    let(:runner_2) { create(:ci_runner) }
 
-    before do
-      project.runners << runner
-    end
-
-    context 'without any projects' do
-      let(:project) { create(:project) }
+    context 'without any projects nor groups' do
+      let!(:project) { create(:project, runners: [runner_1]) }
+      let!(:group) { create(:group) }
 
       it 'does not load' do
-        expect(user.ci_authorized_runners).to be_empty
+        expect(user.ci_owned_runners).to be_empty
       end
     end
 
     context 'with personal projects runners' do
       let(:namespace) { create(:namespace, owner: user) }
-      let(:project) { create(:project, namespace: namespace) }
+      let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) }
 
       it 'loads' do
-        expect(user.ci_authorized_runners).to contain_exactly(runner)
+        expect(user.ci_owned_runners).to contain_exactly(runner_1)
+      end
+    end
+
+    context 'with personal group runner' do
+      let!(:project) { create(:project, runners: [runner_1]) }
+      let!(:group) do
+        create(:group, runners: [runner_2]).tap do |group|
+          group.add_owner(user)
+        end
+      end
+
+      it 'loads' do
+        expect(user.ci_owned_runners).to contain_exactly(runner_2)
+      end
+    end
+
+    context 'with personal project and group runner' do
+      let(:namespace) { create(:namespace, owner: user) }
+      let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) }
+
+      let!(:group) do
+        create(:group, runners: [runner_2]).tap do |group|
+          group.add_owner(user)
+        end
+      end
+
+      it 'loads' do
+        expect(user.ci_owned_runners).to contain_exactly(runner_1, runner_2)
       end
     end
 
@@ -1818,7 +1862,7 @@ describe User do
         end
 
         it 'loads' do
-          expect(user.ci_authorized_runners).to contain_exactly(runner)
+          expect(user.ci_owned_runners).to contain_exactly(runner_1)
         end
       end
 
@@ -1828,14 +1872,28 @@ describe User do
         end
 
         it 'does not load' do
-          expect(user.ci_authorized_runners).to be_empty
+          expect(user.ci_owned_runners).to be_empty
         end
       end
     end
 
     context 'with groups projects runners' do
       let(:group) { create(:group) }
-      let(:project) { create(:project, group: group) }
+      let!(:project) { create(:project, group: group, runners: [runner_1]) }
+
+      def add_user(access)
+        group.add_user(user, access)
+      end
+
+      it_behaves_like :member
+    end
+
+    context 'with groups runners' do
+      let!(:group) do
+        create(:group, runners: [runner_1]).tap do |group|
+          group.add_owner(user)
+        end
+      end
 
       def add_user(access)
         group.add_user(user, access)
@@ -1845,7 +1903,7 @@ describe User do
     end
 
     context 'with other projects runners' do
-      let(:project) { create(:project) }
+      let!(:project) { create(:project, runners: [runner_1]) }
 
       def add_user(access)
         project.add_role(user, access)
@@ -1858,7 +1916,7 @@ describe User do
       let(:group) { create(:group) }
       let(:another_user) { create(:user) }
       let(:subgroup) { create(:group, parent: group) }
-      let(:project) { create(:project, group: subgroup) }
+      let!(:project) { create(:project, group: subgroup, runners: [runner_1]) }
 
       def add_user(access)
         group.add_user(user, access)
@@ -2769,4 +2827,12 @@ describe User do
       expect { user.increment_failed_attempts! }.not_to change(user, :failed_attempts)
     end
   end
+
+  context 'with uploads' do
+    it_behaves_like 'model with mounted uploader', false do
+      let(:model_object) { create(:user, :with_avatar) }
+      let(:upload_attribute) { :avatar }
+      let(:uploader_class) { AttachmentUploader }
+    end
+  end
 end

spec/requests/api/runners_spec.rb

@@ -27,7 +27,7 @@ describe API::Runners do
     end
   end
 
-  let!(:group_runner) { create(:ci_runner, description: 'Group runner', groups: [group]) }
+  let!(:group_runner) { create(:ci_runner, description: 'Group runner', groups: [group], runner_type: :group_type) }
 
   before do
     # Set project access for users
@@ -48,7 +48,7 @@ describe API::Runners do
         expect(json_response).to be_an Array
         expect(json_response[0]).to have_key('ip_address')
         expect(descriptions).to contain_exactly(
-          'Project runner', 'Two projects runner'
+          'Project runner', 'Two projects runner', 'Group runner'
         )
         expect(shared).to be_falsey
       end
@@ -592,6 +592,15 @@ describe API::Runners do
           end.to change { project.runners.count }.by(+1)
           expect(response).to have_gitlab_http_status(201)
         end
+
+        it 'enables a shared runner' do
+          expect do
+            post api("/projects/#{project.id}/runners", admin), runner_id: shared_runner.id
+          end.to change { project.runners.count }.by(1)
+
+          expect(shared_runner.reload).not_to be_shared
+          expect(response).to have_gitlab_http_status(201)
+        end
       end
 
       context 'user is not admin' do

spec/support/shared_examples/models/with_uploads_shared_examples.rb

@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+shared_examples_for 'model with mounted uploader' do |supports_fileuploads|
+  describe '.destroy' do
+    before do
+      stub_uploads_object_storage(uploader_class)
+
+      model_object.public_send(upload_attribute).migrate!(ObjectStorage::Store::REMOTE)
+    end
+
+    it 'deletes remote uploads' do
+      expect_any_instance_of(CarrierWave::Storage::Fog::File).to receive(:delete).and_call_original
+
+      expect { model_object.destroy }.to change { Upload.count }.by(-1)
+    end
+
+    it 'deletes any FileUploader uploads which are not mounted', skip: !supports_fileuploads do
+      create(:upload, uploader: FileUploader, model: model_object)
+
+      expect { model_object.destroy }.to change { Upload.count }.by(-2)
+    end
+  end
+end