Merge branch 'master' into 'backstage/gb/use-persisted-stages-to-improve-pipelines-table'

# Conflicts:
#   db/schema.rb

CONTRIBUTING.md

@@ -349,7 +349,7 @@ on those issues. Please select someone with relevant experience from the
 [GitLab team][team]. If there is nobody mentioned with that expertise look in
 the commit history for the affected files to find someone.
 
-[described in our handbook]: https://about.gitlab.com/handbook/engineering/issues/issue-triage-policies/
+[described in our handbook]: https://about.gitlab.com/handbook/engineering/issue-triage/
 [issue bash events]: https://gitlab.com/gitlab-org/gitlab-ce/issues/17815
 
 ### Feature proposals

PROCESS.md

@@ -168,6 +168,7 @@ the stable branch are:
 
 * Fixes for [regressions](#regressions)
 * Fixes for security issues
+* Fixes or improvements to automated QA scenarios
 * New or updated translations (as long as they do not touch application code)
 
 During the feature freeze all merge requests that are meant to go into the

README.md

@@ -126,5 +126,5 @@ Please see [Getting help for GitLab](https://about.gitlab.com/getting-help/) on
 
 ## Is it awesome?
 
-Thanks for [asking this question](https://twitter.com/supersloth/status/489462789384056832) Joshua.
 [These people](https://twitter.com/gitlab/likes) seem to like it.
+

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -265,10 +265,10 @@ export default {
       />
 
       <section
-        v-if="mr.maintainerEditAllowed"
+        v-if="mr.allowCollaboration"
         class="mr-info-list mr-links"
       >
-        {{ s__("mrWidget|Allows edits from maintainers") }}
+        {{ s__("mrWidget|Allows commits from members who can merge to the target branch") }}
       </section>
 
       <mr-widget-related-links

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -83,7 +83,7 @@ export default class MergeRequestStore {
     this.canBeMerged = data.can_be_merged || false;
     this.isMergeAllowed = data.mergeable || false;
     this.mergeOngoing = data.merge_ongoing;
-    this.maintainerEditAllowed = data.allow_maintainer_to_push;
+    this.allowCollaboration = data.allow_collaboration;
 
     // Cherry-pick and Revert actions related
     this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false;

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

@@ -124,15 +124,18 @@
             break;
         }
       },
+      hideOnSmallScreen(item) {
+        return !item.first && !item.last && !item.next && !item.prev && !item.active;
+      },
     },
   };
 </script>
 <template>
   <div
     v-if="showPagination"
-    class="gl-pagination"
+    class="gl-pagination prepend-top-default"
   >
-    <ul class="pagination clearfix">
+    <ul class="pagination justify-content-center">
       <li
         v-for="(item, index) in getItems"
         :key="index"
@@ -142,12 +145,17 @@
           'js-next-button': item.next,
           'js-last-button': item.last,
           'js-first-button': item.first,
+          'd-none d-md-block': hideOnSmallScreen(item),
           separator: item.separator,
           active: item.active,
-          disabled: item.disabled
+          disabled: item.disabled || item.separator
         }"
+        class="page-item"
+      >
+        <a
+          @click.prevent="changePage(item.title, item.disabled)"
+          class="page-link"
         >
-        <a @click.prevent="changePage(item.title, item.disabled)">
           {{ item.title }}
         </a>
       </li>

app/assets/stylesheets/bootstrap_migration.scss

@@ -24,6 +24,11 @@ html {
   font-size: 14px;
 }
 
+legend {
+  border-bottom: 1px solid $border-color;
+  margin-bottom: 20px;
+}
+
 button,
 html [type="button"],
 [type="reset"],
@@ -183,7 +188,9 @@ table {
 
 .nav-tabs {
   .nav-link {
-    border: 0;
+    border-top: 0;
+    border-left: 0;
+    border-right: 0;
   }
 
   .nav-item {

app/assets/stylesheets/framework/layout.scss

@@ -115,9 +115,3 @@ body {
 .with-performance-bar .layout-page {
   margin-top: $header-height + $performance-bar-height;
 }
-
-.vertical-center {
-  min-height: 100vh;
-  display: flex;
-  align-items: center;
-}

app/assets/stylesheets/framework/pagination.scss

 .gl-pagination {
-  text-align: center;
-  border-top: 1px solid $border-color;
-  margin: 0;
-  margin-top: 0;
-
-  .pagination {
-    padding: 0;
-    margin: 20px 0;
-
   a {
-      cursor: pointer;
-    }
-
-    .separator,
-    .separator:hover {
-      a {
-        cursor: default;
-        background-color: $gray-light;
-        padding: $gl-vert-padding;
-      }
-    }
-  }
-
-
-  .gap,
-  .gap:hover {
-    background-color: $gray-light;
-    padding: $gl-vert-padding;
-    cursor: default;
-  }
-}
-
-.card > .gl-pagination {
-  margin: 0;
-}
-
-/**
- * Extra-small screen pagination.
- */
-@media (max-width: 320px) {
-  .gl-pagination {
-    .first,
-    .last {
-      display: none;
-    }
-
-    .page-item {
-      display: none;
-
-      &.active {
-        display: inline;
-      }
-    }
-  }
-}
-
-/**
- * Small screen pagination
- */
-@include media-breakpoint-down(xs) {
-  .gl-pagination {
-    .pagination li a {
-      padding: 6px 10px;
-    }
-
-    .page-item {
-      display: none;
-
-      &.active {
-        display: inline;
-      }
-    }
-  }
-}
-
-/**
- * Medium screen pagination
- */
-@media (min-width: map-get($grid-breakpoints, xs)) and (max-width: map-get($grid-breakpoints, sm)) {
-  .gl-pagination {
-    .page-item {
-      display: none;
-
-      &.active,
-      &.sibling {
-        display: inline;
-      }
-    }
+    color: inherit;
+    text-decoration: none;
   }
 }

app/assets/stylesheets/pages/issuable.scss

@@ -485,6 +485,15 @@
     .sidebar-collapsed-user {
       padding-bottom: 0;
       margin-bottom: 10px;
+
+      .author_link {
+        padding-left: 0;
+
+        .avatar {
+          position: static;
+          margin: 0;
+        }
+      }
     }
 
     .issuable-header-btn {

app/controllers/application_controller.rb

@@ -130,12 +130,17 @@ class ApplicationController < ActionController::Base
   end
 
   def access_denied!(message = nil)
+    # If we display a custom access denied message to the user, we don't want to
+    # hide existence of the resource, rather tell them they cannot access it using
+    # the provided message
+    status = message.present? ? :forbidden : :not_found
+
     respond_to do |format|
-      format.any { head :not_found }
+      format.any { head status }
       format.html do
         render "errors/access_denied",
                layout: "errors",
-               status: 404,
+               status: status,
                locals: { message: message }
       end
     end

app/controllers/projects/merge_requests/application_controller.rb

@@ -15,7 +15,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
 
   def merge_request_params_attributes
     [
-      :allow_maintainer_to_push,
+      :allow_collaboration,
       :assignee_id,
       :description,
       :force_remove_source_branch,

app/controllers/users/terms_controller.rb

@@ -13,6 +13,10 @@ module Users
 
     def index
       @redirect = redirect_path
+
+      if @term.accepted_by_user?(current_user)
+        flash.now[:notice] = "You have already accepted the Terms of Service as #{current_user.to_reference}"
+      end
     end
 
     def accept

app/helpers/merge_requests_helper.rb

@@ -126,8 +126,8 @@ module MergeRequestsHelper
     link_to(url[merge_request.project, merge_request], data: data_attrs, &block)
   end
 
-  def allow_maintainer_push_unavailable_reason(merge_request)
-    return if merge_request.can_allow_maintainer_to_push?(current_user)
+  def allow_collaboration_unavailable_reason(merge_request)
+    return if merge_request.can_allow_collaboration?(current_user)
 
     minimum_visibility = [merge_request.target_project.visibility_level,
                           merge_request.source_project.visibility_level].min

app/helpers/projects_helper.rb

@@ -412,7 +412,10 @@ module ProjectsHelper
     exports_path = File.join(Settings.shared['path'], 'tmp/project_exports')
     filtered_message = message.strip.gsub(exports_path, "[REPO EXPORT PATH]")
 
-    disk_path = Gitlab.config.repositories.storages[project.repository_storage].legacy_disk_path
+    disk_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      Gitlab.config.repositories.storages[project.repository_storage].legacy_disk_path
+    end
+
     filtered_message.gsub(disk_path.chomp('/'), "[REPOS PATH]")
   end
 

app/models/application_setting/term.rb

 class ApplicationSetting
   class Term < ActiveRecord::Base
     include CacheMarkdownField
+    has_many :term_agreements
 
     validates :terms, presence: true
 
@@ -9,5 +10,10 @@ class ApplicationSetting
     def self.latest
       order(:id).last
     end
+
+    def accepted_by_user?(user)
+      user.accepted_term_id == id ||
+        term_agreements.accepted.where(user: user).exists?
+    end
   end
 end

app/models/ci/runner.rb

@@ -219,10 +219,8 @@ module Ci
 
       cache_attributes(values)
 
-      if persist_cached_data?
-        self.assign_attributes(values)
-        self.save if self.changed?
-      end
+      # We save data without validation, it will always change due to `contacted_at`
+      self.update_columns(values) if persist_cached_data?
     end
 
     def pick_build!(build)

app/models/concerns/avatarable.rb

@@ -4,11 +4,14 @@ module Avatarable
   included do
     prepend ShadowMethods
     include ObjectStorage::BackgroundMove
+    include Gitlab::Utils::StrongMemoize
 
     validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
     validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
     mount_uploader :avatar, AvatarUploader
+
+    after_initialize :add_avatar_to_batch
   end
 
   module ShadowMethods
@@ -18,6 +21,17 @@ module Avatarable
 
       avatar_path(only_path: args.fetch(:only_path, true)) || super
     end
+
+    def retrieve_upload(identifier, paths)
+      upload = retrieve_upload_from_batch(identifier)
+
+      # This fallback is needed when deleting an upload, because we may have
+      # already been removed from the DB. We have to check an explicit `#nil?`
+      # because it's a BatchLoader instance.
+      upload = super if upload.nil?
+
+      upload
+    end
   end
 
   def avatar_type
@@ -52,4 +66,37 @@ module Avatarable
 
     url_base + avatar.local_url
   end
+
+  # Path that is persisted in the tracking Upload model. Used to fetch the
+  # upload from the model.
+  def upload_paths(identifier)
+    avatar_mounter.blank_uploader.store_dirs.map { |store, path| File.join(path, identifier) }
+  end
+
+  private
+
+  def retrieve_upload_from_batch(identifier)
+    BatchLoader.for(identifier: identifier, model: self).batch(key: self.class) do |upload_params, loader, args|
+      model_class = args[:key]
+      paths = upload_params.flat_map do |params|
+        params[:model].upload_paths(params[:identifier])
+      end
+
+      Upload.where(uploader: AvatarUploader, path: paths).find_each do |upload|
+        model = model_class.instantiate('id' => upload.model_id)
+
+        loader.call({ model: model, identifier: File.basename(upload.path) }, upload)
+      end
+    end
+  end
+
+  def add_avatar_to_batch
+    return unless avatar_mounter
+
+    avatar_mounter.read_identifiers.each { |identifier| retrieve_upload_from_batch(identifier) }
+  end
+
+  def avatar_mounter
+    strong_memoize(:avatar_mounter) { _mounter(:avatar) }
+  end
 end

app/models/concerns/with_uploads.rb

@@ -36,4 +36,8 @@ module WithUploads
       upload.destroy
     end
   end
+
+  def retrieve_upload(_identifier, paths)
+    uploads.find_by(path: paths)
+  end
 end

app/models/merge_request.rb

@@ -1125,21 +1125,21 @@ class MergeRequest < ActiveRecord::Base
     project.merge_requests.merged.where(author_id: author_id).empty?
   end
 
-  def allow_maintainer_to_push
-    maintainer_push_possible? && super
+  def allow_collaboration
+    collaborative_push_possible? && super
   end
 
-  alias_method :allow_maintainer_to_push?, :allow_maintainer_to_push
+  alias_method :allow_collaboration?, :allow_collaboration
 
-  def maintainer_push_possible?
+  def collaborative_push_possible?
     source_project.present? && for_fork? &&
       target_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE &&
       source_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE &&
       !ProtectedBranch.protected?(source_project, source_branch)
   end
 
-  def can_allow_maintainer_to_push?(user)
-    maintainer_push_possible? &&
+  def can_allow_collaboration?(user)
+    collaborative_push_possible? &&
       Ability.allowed?(user, :push_code, source_project)
   end
 

app/models/note.rb

@@ -435,6 +435,10 @@ class Note < ActiveRecord::Base
     super.merge(noteable: noteable)
   end
 
+  def retrieve_upload(_identifier, paths)
+    Upload.find_by(model: self, path: paths)
+  end
+
   private
 
   def keep_around_commit

app/models/personal_snippet.rb

 class PersonalSnippet < Snippet
+  include WithUploads
 end

app/models/project.rb

@@ -1975,18 +1975,18 @@ class Project < ActiveRecord::Base
                                 .limit(1)
                                 .select(1)
     source_of_merge_requests.opened
-      .where(allow_maintainer_to_push: true)
+      .where(allow_collaboration: true)
       .where('EXISTS (?)', developer_access_exists)
   end
 
-  def branch_allows_maintainer_push?(user, branch_name)
+  def branch_allows_collaboration?(user, branch_name)
     return false unless user
 
     cache_key = "user:#{user.id}:#{branch_name}:branch_allows_push"
 
-    memoized_results = strong_memoize(:branch_allows_maintainer_push) do
+    memoized_results = strong_memoize(:branch_allows_collaboration) do
       Hash.new do |result, cache_key|
-        result[cache_key] = fetch_branch_allows_maintainer_push?(user, branch_name)
+        result[cache_key] = fetch_branch_allows_collaboration?(user, branch_name)
       end
     end
 
@@ -2128,18 +2128,18 @@ class Project < ActiveRecord::Base
     raise ex
   end
 
-  def fetch_branch_allows_maintainer_push?(user, branch_name)
+  def fetch_branch_allows_collaboration?(user, branch_name)
     check_access = -> do
       next false if empty_repo?
 
       merge_request = source_of_merge_requests.opened
-                        .where(allow_maintainer_to_push: true)
+                        .where(allow_collaboration: true)
                         .find_by(source_branch: branch_name)
       merge_request&.can_be_merged_by?(user)
     end
 
     if RequestStore.active?
-      RequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_maintainer_push") do
+      RequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_collaboration") do
         check_access.call
       end
     else

app/models/term_agreement.rb

@@ -2,5 +2,7 @@ class TermAgreement < ActiveRecord::Base
   belongs_to :term, class_name: 'ApplicationSetting::Term'
   belongs_to :user
 
+  scope :accepted, -> { where(accepted: true) }
+
   validates :user, :term, presence: true
 end

app/policies/ci/build_policy.rb

@@ -14,8 +14,8 @@ module Ci
       @subject.triggered_by?(@user)
     end
 
-    condition(:branch_allows_maintainer_push) do
-      @subject.project.branch_allows_maintainer_push?(@user, @subject.ref)
+    condition(:branch_allows_collaboration) do
+      @subject.project.branch_allows_collaboration?(@user, @subject.ref)
     end
 
     rule { protected_ref }.policy do
@@ -25,7 +25,7 @@ module Ci
 
     rule { can?(:admin_build) | (can?(:update_build) & owner_of_job) }.enable :erase_build
 
-    rule { can?(:public_access) & branch_allows_maintainer_push }.policy do
+    rule { can?(:public_access) & branch_allows_collaboration }.policy do
       enable :update_build
       enable :update_commit_status
     end

app/policies/ci/pipeline_policy.rb

@@ -4,13 +4,13 @@ module Ci
 
     condition(:protected_ref) { ref_protected?(@user, @subject.project, @subject.tag?, @subject.ref) }
 
-    condition(:branch_allows_maintainer_push) do
-      @subject.project.branch_allows_maintainer_push?(@user, @subject.ref)
+    condition(:branch_allows_collaboration) do
+      @subject.project.branch_allows_collaboration?(@user, @subject.ref)
     end
 
     rule { protected_ref }.prevent :update_pipeline
 
-    rule { can?(:public_access) & branch_allows_maintainer_push }.policy do
+    rule { can?(:public_access) & branch_allows_collaboration }.policy do
       enable :update_pipeline
     end
 

app/serializers/merge_request_widget_entity.rb

@@ -13,7 +13,7 @@ class MergeRequestWidgetEntity < IssuableEntity
   expose :squash
   expose :target_branch
   expose :target_project_id
-  expose :allow_maintainer_to_push
+  expose :allow_collaboration
 
   expose :should_be_rebased?, as: :should_be_rebased
   expose :ff_only_enabled do |merge_request|

app/services/application_settings/update_service.rb

 module ApplicationSettings
   class UpdateService < ApplicationSettings::BaseService
+    attr_reader :params, :application_setting
+
     def execute
       update_terms(@params.delete(:terms))
 

app/services/applications/create_service.rb

@@ -2,8 +2,7 @@ module Applications
   class CreateService
     def initialize(current_user, params)
       @current_user = current_user
-      @params = params
-      @ip_address = @params.delete(:ip_address)
+      @params = params.except(:ip_address)
     end
 
     def execute(request = nil)

app/services/merge_requests/base_service.rb

@@ -38,8 +38,8 @@ module MergeRequests
     def filter_params(merge_request)
       super
 
-      unless merge_request.can_allow_maintainer_to_push?(current_user)
-        params.delete(:allow_maintainer_to_push)
+      unless merge_request.can_allow_collaboration?(current_user)
+        params.delete(:allow_collaboration)
       end
     end
 

app/services/test_hooks/project_service.rb

 module TestHooks
   class ProjectService < TestHooks::BaseService
-    private
+    attr_writer :project
 
     def project
       @project ||= hook.project
     end
 
+    private
+
     def push_events_data
       throw(:validation_error, 'Ensure the project has at least one commit.') if project.empty_repo?
 

app/uploaders/object_storage.rb

@@ -33,7 +33,7 @@ module ObjectStorage
 
         unless current_upload_satisfies?(paths, model)
           # the upload we already have isn't right, find the correct one
-          self.upload = uploads.find_by(model: model, path: paths)
+          self.upload = model&.retrieve_upload(identifier, paths)
         end
 
         super
@@ -46,7 +46,7 @@ module ObjectStorage
       end
 
       def upload=(upload)
-        return unless upload
+        return if upload.nil?
 
         self.object_store = upload.store
         super

app/views/admin/users/_access_levels.html.haml

 %fieldset
   %legend Access
-  .form-group
-    = f.label :projects_limit, class: 'col-form-label'
+  .form-group.row
+    = f.label :projects_limit, class: 'col-form-label col-sm-2'
     .col-sm-10= f.number_field :projects_limit, min: 0, max: Gitlab::Database::MAX_INT_VALUE, class: 'form-control'
 
-  .form-group
-    = f.label :can_create_group, class: 'col-form-label'
+  .form-group.row
+    = f.label :can_create_group, class: 'col-form-label col-sm-2'
     .col-sm-10= f.check_box :can_create_group
 
-  .form-group
-    = f.label :access_level, class: 'col-form-label'
+  .form-group.row
+    = f.label :access_level, class: 'col-form-label col-sm-2'
     .col-sm-10
       - editing_current_user = (current_user == @user)
 
       = f.radio_button :access_level, :regular, disabled: editing_current_user
-      = label_tag :regular do
+      = label_tag :regular, class: 'font-weight-bold' do
         Regular
       %p.light
         Regular users have access to their groups and projects
 
       = f.radio_button :access_level, :admin, disabled: editing_current_user
-      = label_tag :admin do
+      = label_tag :admin, class: 'font-weight-bold' do
         Admin
       %p.light
         Administrators have access to all groups, projects and users and can manage all features in this installation
@@ -28,8 +28,8 @@
         %p.light
           You cannot remove your own admin rights.
 
-  .form-group
-    = f.label :external, class: 'col-form-label'
+  .form-group.row
+    = f.label :external, class: 'col-form-label col-sm-2'
     .col-sm-10
       = f.check_box :external do
         External

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

@@ -56,7 +56,7 @@
         = f.label :linkedin, class: 'col-form-label col-sm-2'
         .col-sm-10= f.text_field :linkedin, class: 'form-control'
       .form-group.row
-        = f.label :twitter, class: 'col-form-label'
+        = f.label :twitter, class: 'col-form-label col-sm-2'
         .col-sm-10= f.text_field :twitter, class: 'form-control'
       .form-group.row
         = f.label :website_url, 'Website', class: 'col-form-label col-sm-2'

app/views/kaminari/gitlab/_first_page.html.haml

@@ -5,5 +5,5 @@
 -#    total_pages:   total number of pages
 -#    per_page:      number of items to fetch per page
 -#    remote:        data-remote
-%li.first.page-item
+%li.page-item.js-first-button
   = link_to_unless current_page.first?, raw(t 'views.pagination.first'), url, remote: remote, class: 'page-link'

app/views/kaminari/gitlab/_gap.html.haml

@@ -4,5 +4,5 @@
 -#    total_pages:   total number of pages
 -#    per_page:      number of items to fetch per page
 -#    remote:        data-remote
-%li.page-item.disabled
+%li.page-item.disabled.d-none.d-md-block
   = link_to raw(t 'views.pagination.truncate'), '#', class: 'page-link'

app/views/kaminari/gitlab/_last_page.html.haml

@@ -5,5 +5,5 @@
 -#    total_pages:   total number of pages
 -#    per_page:      number of items to fetch per page
 -#    remote:        data-remote
-%li.last.page-item
+%li.page-item.js-last-button
   = link_to_unless current_page.last?, raw(t 'views.pagination.last'), url, {remote: remote, class: 'page-link'}

app/views/kaminari/gitlab/_next_page.html.haml

@@ -8,5 +8,5 @@
 
 - page_url = current_page.last? ? '#' : url
 
-%li.page-item{ class: ('disabled' if current_page.last?) }
+%li.page-item.js-next-button{ class: ('disabled' if current_page.last?) }
   = link_to raw(t 'views.pagination.next'), page_url, rel: 'next', remote: remote, class: 'page-link'

app/views/kaminari/gitlab/_page.html.haml

@@ -6,5 +6,5 @@
 -#    total_pages:   total number of pages
 -#    per_page:      number of items to fetch per page
 -#    remote:        data-remote
-%li.page-item.js-pagination-page{ class: [active_when(page.current?), ('sibling' if page.next? || page.prev?)] }
+%li.page-item.js-pagination-page{ class: [active_when(page.current?), ('sibling' if page.next? || page.prev?), ('d-none d-md-block' if !page.current?) ] }
   = link_to page, url, { remote: remote, rel: page.next? ? 'next' : page.prev? ? 'prev' : nil, class: 'page-link' }

app/views/kaminari/gitlab/_paginator.html.haml

@@ -6,7 +6,7 @@
 -#    remote:        data-remote
 -#    paginator:     the paginator that renders the pagination tags inside
 = paginator.render do
-  .gl-pagination
+  .gl-pagination.prepend-top-default
     %ul.pagination.justify-content-center
       - unless current_page.first?
         = first_page_tag unless total_pages < 5 # As kaminari will always show the first 5 pages

app/views/kaminari/gitlab/_prev_page.html.haml

@@ -8,5 +8,5 @@
 
 - page_url = current_page.first? ? '#' : url
 
-%li.page-item{ class: ('disabled' if current_page.first?) }
+%li.page-item.js-previous-button{ class: ('disabled' if current_page.first?) }
   = link_to raw(t 'views.pagination.previous'), page_url, rel: 'prev', remote: remote, class: 'page-link'

app/views/kaminari/gitlab/_without_count.html.haml

-.gl-pagination
-  %ul.pagination.clearfix
+.gl-pagination.prepend-top-default
+  %ul.pagination.justify-content-center
     - if previous_path
       %li.page-item.prev
         = link_to(t('views.pagination.previous'), previous_path, rel: 'prev', class: 'page-link')

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

 .file-content.blob_file.blob-no-preview
-  .center.render-error.vertical-center
+  .center.render-error
     = link_to blob_raw_path do
       %h1.light
         = sprite_icon('download')

app/views/projects/new.html.haml

@@ -29,16 +29,16 @@
 
     .col-lg-9.js-toggle-container
       %ul.nav.nav-tabs.nav-links.gitlab-tabs{ role: 'tablist' }
-        %li{ class: active_when(active_tab == 'blank'), role: 'presentation' }
-          %a{ href: '#blank-project-pane', id: 'blank-project-tab', data: { toggle: 'tab' }, role: 'tab' }
+        %li.nav-item{ role: 'presentation' }
+          %a.nav-link.active{ href: '#blank-project-pane', id: 'blank-project-tab', data: { toggle: 'tab' }, role: 'tab' }
             %span.d-none.d-sm-block Blank project
             %span.d-block.d-sm-none Blank
-        %li{ class: active_when(active_tab == 'template'), role: 'presentation' }
-          %a{ href: '#create-from-template-pane', id: 'create-from-template-tab', data: { toggle: 'tab' }, role: 'tab' }
+        %li.nav-item{ role: 'presentation' }
+          %a.nav-link{ href: '#create-from-template-pane', id: 'create-from-template-tab', data: { toggle: 'tab' }, role: 'tab' }
             %span.d-none.d-sm-block Create from template
             %span.d-block.d-sm-none Template
-        %li{ class: active_when(active_tab == 'import'), role: 'presentation' }
-          %a{ href: '#import-project-pane', id: 'import-project-tab', data: { toggle: 'tab' }, role: 'tab' }
+        %li.nav-item{ role: 'presentation' }
+          %a.nav-link{ href: '#import-project-pane', id: 'import-project-tab', data: { toggle: 'tab' }, role: 'tab' }
             %span.d-none.d-sm-block Import project
             %span.d-block.d-sm-none Import
 

app/views/shared/issuable/form/_contribution.html.haml

@@ -12,9 +12,9 @@
     = _('Contribution')
   .col-sm-10
     .form-check
-      = form.check_box :allow_maintainer_to_push, disabled: !issuable.can_allow_maintainer_to_push?(current_user), class: 'form-check-input'
-      = form.label :allow_maintainer_to_push, class: 'form-check-label' do
-        = _('Allow edits from maintainers.')
-        = link_to 'About this feature', help_page_path('user/project/merge_requests/maintainer_access')
+      = form.check_box :allow_collaboration, disabled: !issuable.can_allow_collaboration?(current_user), class: 'form-check-input'
+      = form.label :allow_collaboration, class: 'form-check-label' do
+        = _('Allow commits from members who can merge to the target branch.')
+        = link_to 'About this feature', help_page_path('user/project/merge_requests/allow_collaboration')
         .form-text.text-muted
-          = allow_maintainer_push_unavailable_reason(issuable)
+          = allow_collaboration_unavailable_reason(issuable)

app/views/shared/projects/_edit_information.html.haml

 - unless can?(current_user, :push_code, @project)
   .inline.prepend-left-10
-    - if @project.branch_allows_maintainer_push?(current_user, selected_branch)
+    - if @project.branch_allows_collaboration?(current_user, selected_branch)
       = commit_in_single_accessible_branch
     - else
       = commit_in_fork_help

app/views/users/terms/index.html.haml

@@ -7,6 +7,10 @@
     .float-right
       = button_to accept_term_path(@term, redirect_params), class: 'btn btn-success prepend-left-8' do
         = _('Accept terms')
+  - else
+    .pull-right
+      = link_to root_path, class: 'btn btn-success prepend-left-8' do
+        = _('Continue')
   - if can?(current_user, :decline_terms, @term)
     .float-right
       = button_to decline_term_path(@term, redirect_params), class: 'btn btn-default prepend-left-8' do

changelogs/unreleased/42751-rename-mr-maintainer-push.yml

+---
+title: Rephrasing Merge Request's 'allow edits from maintainer' functionality
+merge_request: 19061
+author:
+type: deprecated

changelogs/unreleased/46585-gdpr-terms-acceptance.yml

+---
+title: Add flash notice if user has already accepted terms and allow users to continue
+  to root path
+merge_request: 19156
+author:
+type: changed

changelogs/unreleased/fix-avatars-n-plus-one.yml

+---
+title: Fix an N+1 when loading user avatars
+merge_request:
+author:
+type: performance

changelogs/unreleased/optimise-runner-update-cached-info.yml

+---
+title: Update runner cached informations without performing validations
+merge_request:
+author:
+type: performance

changelogs/unreleased/rd-44364-deprecate-support-for-dsa-keys.yml

+---
+title: Add migration to disable the usage of DSA keys
+merge_request: 19299
+author:
+type: other

changelogs/unreleased/sh-fix-events-nplus-one.yml

+---
+title: Eliminate N+1 queries with authors and push_data_payload in Events API
+merge_request:
+author:
+type: performance

db/migrate/20180523042841_rename_merge_requests_allow_maintainer_to_push.rb

+class RenameMergeRequestsAllowMaintainerToPush < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    rename_column_concurrently :merge_requests, :allow_maintainer_to_push, :allow_collaboration
+  end
+
+  def down
+    cleanup_concurrent_column_rename :merge_requests, :allow_collaboration, :allow_maintainer_to_push
+  end
+end

db/migrate/20180531220618_change_default_value_for_dsa_key_restriction.rb

+class ChangeDefaultValueForDsaKeyRestriction < ActiveRecord::Migration
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def up
+    change_column :application_settings, :dsa_key_restriction, :integer, null: false,
+                  default: -1
+
+    execute("UPDATE application_settings SET dsa_key_restriction = -1")
+  end
+
+  def down
+    change_column :application_settings, :dsa_key_restriction, :integer, null: false,
+                  default: 0
+  end
+end

db/post_migrate/20180523125103_cleanup_merge_requests_allow_maintainer_to_push_rename.rb

+class CleanupMergeRequestsAllowMaintainerToPushRename < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    cleanup_concurrent_column_rename :merge_requests, :allow_maintainer_to_push, :allow_collaboration
+  end
+
+  def down
+    rename_column_concurrently :merge_requests, :allow_collaboration, :allow_maintainer_to_push
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180530135500) do
+ActiveRecord::Schema.define(version: 20180531220618) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -110,7 +110,7 @@ ActiveRecord::Schema.define(version: 20180530135500) do
     t.text "shared_runners_text_html"
     t.text "after_sign_up_text_html"
     t.integer "rsa_key_restriction", default: 0, null: false
-    t.integer "dsa_key_restriction", default: 0, null: false
+    t.integer "dsa_key_restriction", default: -1, null: false
     t.integer "ecdsa_key_restriction", default: 0, null: false
     t.integer "ed25519_key_restriction", default: 0, null: false
     t.boolean "housekeeping_enabled", default: true, null: false
@@ -1230,7 +1230,7 @@ ActiveRecord::Schema.define(version: 20180530135500) do
     t.boolean "discussion_locked"
     t.integer "latest_merge_request_diff_id"
     t.string "rebase_commit_sha"
-    t.boolean "allow_maintainer_to_push"
+    t.boolean "allow_collaboration"
     t.boolean "squash", default: false, null: false
   end
 

doc/api/merge_requests.md

@@ -651,7 +651,8 @@ POST /projects/:id/merge_requests
 | `labels`                   | string  | no       | Labels for MR as a comma-separated list                                         |
 | `milestone_id`             | integer | no       | The global ID of a milestone                                                           |
 | `remove_source_branch`     | boolean | no       | Flag indicating if a merge request should remove the source branch when merging |
-| `allow_maintainer_to_push` | boolean | no       | Whether or not a maintainer of the target project can push to the source branch |
+| `allow_collaboration`      | boolean | no       | Allow commits from members who can merge to the target branch                   |
+| `allow_maintainer_to_push` | boolean | no       | Deprecated, see allow_collaboration                                             |
 | `squash`                   | boolean | no       | Squash commits into a single commit when merging                                |
 
 ```json
@@ -709,6 +710,7 @@ POST /projects/:id/merge_requests
   "squash": false,
   "web_url": "http://example.com/example/example/merge_requests/1",
   "discussion_locked": false,
+  "allow_collaboration": false,
   "allow_maintainer_to_push": false,
   "time_stats": {
     "time_estimate": 0,
@@ -741,7 +743,8 @@ PUT /projects/:id/merge_requests/:merge_request_iid
 | `remove_source_branch`     | boolean | no       | Flag indicating if a merge request should remove the source branch when merging |
 | `squash`                   | boolean | no       | Squash commits into a single commit when merging |
 | `discussion_locked`        | boolean | no       | Flag indicating if the merge request's discussion is locked. If the discussion is locked only project members can add, edit or resolve comments. |
-| `allow_maintainer_to_push` | boolean | no       | Whether or not a maintainer of the target project can push to the source branch |
+| `allow_collaboration`      | boolean | no       | Allow commits from members who can merge to the target branch                   |
+| `allow_maintainer_to_push` | boolean | no       | Deprecated, see allow_collaboration                                             |
 
 Must include at least one non-required attribute from above.
 
@@ -799,6 +802,7 @@ Must include at least one non-required attribute from above.
   "squash": false,
   "web_url": "http://example.com/example/example/merge_requests/1",
   "discussion_locked": false,
+  "allow_collaboration": false,
   "allow_maintainer_to_push": false,
   "time_stats": {
     "time_estimate": 0,

doc/development/i18n/proofreader.md

@@ -16,7 +16,7 @@ are very appreciative of the work done by translators and proofreaders!
 - Dutch
 - Esperanto
 - French
-  - Rémy Coutable - [GitLab](https://gitlab.com/rymai), [Crowdin](https://crowdin.com/profile/rymai)
+  - Davy Defaud - [GitLab](https://gitlab.com/DevDef), [Crowdin](https://crowdin.com/profile/DevDef)
 - German
 - Indonesian
   - Ahmad Naufal Mukhtar - [GitLab](https://gitlab.com/anaufalm), [Crowdin](https://crowdin.com/profile/anaufalm)

doc/user/project/merge_requests/allow_collaboration.md

+# Allow collaboration on merge requests across forks
+
+> [Introduced][ce-17395] in GitLab 10.6.
+
+This feature is available for merge requests across forked projects that are
+publicly accessible. It makes it easier for members of projects to
+collaborate on merge requests across forks.
+
+When enabled for a merge request, members with merge access to the target
+branch of the project will be granted write permissions to the source branch
+of the merge request.
+
+The feature can only be enabled by users who already have push access to the
+source project, and only lasts while the merge request is open.
+
+Enable this functionality while creating or editing a merge request:
+
+![Enable collaboration](./img/allow_collaboration.png)
+
+[ce-17395]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17395

doc/user/project/merge_requests/index.md

@@ -28,7 +28,7 @@ With GitLab merge requests, you can:
 - Enable [fast-forward merge requests](#fast-forward-merge-requests)
 - Enable [semi-linear history merge requests](#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch
 - [Create new merge requests by email](#create-new-merge-requests-by-email)
-- Allow maintainers of the target project to push directly to the fork by [allowing edits from maintainers](maintainer_access.md)
+- [Allow collaboration](allow_collaboration.md) so members of the target project can push directly to the fork
 - [Squash and merge](squash_and_merge.md) for a cleaner commit history
 
 With **[GitLab Enterprise Edition][ee]**, you can also:

doc/user/project/merge_requests/maintainer_access.md

-# Allow maintainer pushes for merge requests across forks
-
-> [Introduced][ce-17395] in GitLab 10.6.
-
-This feature is available for merge requests across forked projects that are
-publicly accessible. It makes it easier for maintainers of projects to
-collaborate on merge requests across forks.
-
-When enabled for a merge request, members with merge access to the target
-branch of the project will be granted write permissions to the source branch
-of the merge request.
-
-The feature can only be enabled by users who already have push access to the
-source project, and only lasts while the merge request is open.
-
-Enable this functionality while creating a merge request:
-
-![Enable maintainer edits](./img/allow_maintainer_push.png)
-
-[ce-17395]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17395
+This document was moved to [another location](allow_collaboration.md).

lib/api/entities.rb

@@ -559,7 +559,9 @@ module API
       expose :discussion_locked
       expose :should_remove_source_branch?, as: :should_remove_source_branch
       expose :force_remove_source_branch?, as: :force_remove_source_branch
-      expose :allow_maintainer_to_push, if: -> (merge_request, _) { merge_request.for_fork? }
+      expose :allow_collaboration, if: -> (merge_request, _) { merge_request.for_fork? }
+      # Deprecated
+      expose :allow_collaboration, as: :allow_maintainer_to_push, if: -> (merge_request, _) { merge_request.for_fork? }
 
       expose :web_url do |merge_request, options|
         Gitlab::UrlBuilder.build(merge_request)

lib/api/events.rb

@@ -17,6 +17,7 @@ module API
 
       def present_events(events)
         events = events.reorder(created_at: params[:sort])
+                 .with_associations
 
         present paginate(events), with: Entities::Event
       end

lib/api/merge_requests.rb

@@ -162,7 +162,8 @@ module API
           optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request'
           optional :labels, type: String, desc: 'Comma-separated list of label names'
           optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging'
-          optional :allow_maintainer_to_push, type: Boolean, desc: 'Whether a maintainer of the target project can push to the source project'
+          optional :allow_collaboration, type: Boolean, desc: 'Allow commits from members who can merge to the target branch'
+          optional :allow_maintainer_to_push, type: Boolean, as: :allow_collaboration, desc: '[deprecated] See allow_collaboration'
           optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge'
 
           use :optional_params_ee

lib/backup.rb

+module Backup
+  Error = Class.new(StandardError)
+end

lib/backup/database.rb

@@ -44,7 +44,7 @@ module Backup
       end
 
       report_success(success)
-      abort 'Backup failed' unless success
+      raise Backup::Error, 'Backup failed' unless success
     end
 
     def restore
@@ -72,7 +72,7 @@ module Backup
       end
 
       report_success(success)
-      abort 'Restore failed' unless success
+      abort Backup::Error, 'Restore failed' unless success
     end
 
     protected

lib/backup/files.rb

@@ -26,7 +26,7 @@ module Backup
 
         unless status.zero?
           puts output
-          abort 'Backup failed'
+          raise Backup::Error, 'Backup failed'
         end
 
         run_pipeline!([%W(tar --exclude=lost+found -C #{@backup_files_dir} -cf - .), %w(gzip -c -1)], out: [backup_tarball, 'w', 0600])
@@ -39,7 +39,11 @@ module Backup
     def restore
       backup_existing_files_dir
 
-      run_pipeline!([%w(gzip -cd), %W(tar --unlink-first --recursive-unlink -C #{app_files_dir} -xf -)], in: backup_tarball)
+      run_pipeline!([%w(gzip -cd), %W(#{tar} --unlink-first --recursive-unlink -C #{app_files_dir} -xf -)], in: backup_tarball)
+    end
+
+    def tar
+      system(*%w[gtar --version], out: '/dev/null') ? 'gtar' : 'tar'
     end
 
     def backup_existing_files_dir
@@ -61,7 +65,7 @@ module Backup
 
     def run_pipeline!(cmd_list, options = {})
       status_list = Open3.pipeline(*cmd_list, options)
-      abort 'Backup failed' unless status_list.compact.all?(&:success?)
+      raise Backup::Error, 'Backup failed' unless status_list.compact.all?(&:success?)
     end
   end
 end

lib/backup/manager.rb

@@ -27,7 +27,7 @@ module Backup
           progress.puts "done".color(:green)
         else
           puts "creating archive #{tar_file} failed".color(:red)
-          abort 'Backup failed'
+          raise Backup::Error, 'Backup failed'
         end
 
         upload
@@ -52,7 +52,7 @@ module Backup
         progress.puts "done".color(:green)
       else
         puts "uploading backup to #{remote_directory} failed".color(:red)
-        abort 'Backup failed'
+        raise Backup::Error, 'Backup failed'
       end
     end
 
@@ -66,7 +66,7 @@ module Backup
           progress.puts "done".color(:green)
         else
           puts "deleting tmp directory '#{dir}' failed".color(:red)
-          abort 'Backup failed'
+          raise Backup::Error, 'Backup failed'
         end
       end
     end

lib/backup/repository.rb

@@ -17,7 +17,10 @@ module Backup
 
       Project.find_each(batch_size: 1000) do |project|
         progress.print " * #{display_repo_path(project)} ... "
-        path_to_project_repo = path_to_repo(project)
+
+        path_to_project_repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+          path_to_repo(project)
+        end
         path_to_project_bundle = path_to_bundle(project)
 
         # Create namespace dir or hashed path if missing
@@ -51,7 +54,9 @@ module Backup
         end
 
         wiki = ProjectWiki.new(project)
-        path_to_wiki_repo = path_to_repo(wiki)
+        path_to_wiki_repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+          path_to_repo(wiki)
+        end
         path_to_wiki_bundle = path_to_bundle(wiki)
 
         if File.exist?(path_to_wiki_repo)
@@ -111,7 +116,9 @@ module Backup
       # TODO: Need to find a way to do this for gitaly
       # Gitaly migration issue: https://gitlab.com/gitlab-org/gitaly/issues/1195
       in_path(path_to_tars(project)) do |dir|
-        path_to_project_repo = path_to_repo(project)
+        path_to_project_repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+          path_to_repo(project)
+        end
         cmd = %W(tar -xf #{path_to_tars(project, dir)} -C #{path_to_project_repo} #{dir})
 
         output, status = Gitlab::Popen.popen(cmd)

lib/gitlab/git/repository.rb

@@ -109,7 +109,7 @@ module Gitlab
       end
 
       def ==(other)
-        path == other.path
+        [storage, relative_path] == [other.storage, other.relative_path]
       end
 
       def path

lib/gitlab/import_export/repo_saver.rb

@@ -26,10 +26,6 @@ module Gitlab
         @shared.error(e)
         false
       end
-
-      def path_to_repo
-        @project.repository.path_to_repo
-      end
     end
   end
 end

lib/gitlab/import_export/wiki_repo_saver.rb

@@ -22,12 +22,8 @@ module Gitlab
         "project.wiki.bundle"
       end
 
-      def path_to_repo
-        @wiki.repository.path_to_repo
-      end
-
       def wiki_repository_exists?
-        File.exist?(@wiki.repository.path_to_repo) && !@wiki.repository.empty?
+        @wiki.repository.exists? && !@wiki.repository.empty?
       end
     end
   end

lib/gitlab/task_helpers.rb

@@ -128,6 +128,7 @@ module Gitlab
     end
 
     def all_repos
+      Gitlab::GitalyClient::StorageSettings.allow_disk_access do
         Gitlab.config.repositories.storages.each_value do |repository_storage|
           IO.popen(%W(find #{repository_storage.legacy_disk_path} -mindepth 2 -type d -name *.git)) do |find|
             find.each_line do |path|
@@ -136,6 +137,7 @@ module Gitlab
           end
         end
       end
+    end
 
     def repository_storage_paths_args
       Gitlab.config.repositories.storages.values.map { |rs| rs.legacy_disk_path }

lib/gitlab/user_access.rb

@@ -65,7 +65,7 @@ module Gitlab
       return false unless can_access_git?
       return false unless project
 
-      return false if !user.can?(:push_code, project) && !project.branch_allows_maintainer_push?(user, ref)
+      return false if !user.can?(:push_code, project) && !project.branch_allows_collaboration?(user, ref)
 
       if protected?(ProtectedBranch, project, ref)
         protected_branch_accessible_to?(ref, action: :push)

lib/tasks/gitlab/cleanup.rake

@@ -12,7 +12,7 @@ namespace :gitlab do
       namespaces  = Namespace.pluck(:path)
       namespaces << HASHED_REPOSITORY_NAME  # add so that it will be ignored
       Gitlab.config.repositories.storages.each do |name, repository_storage|
-        git_base_path = repository_storage.legacy_disk_path
+        git_base_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository_storage.legacy_disk_path }
         all_dirs = Dir.glob(git_base_path + '/*')
 
         puts git_base_path.color(:yellow)
@@ -54,7 +54,8 @@ namespace :gitlab do
 
       move_suffix = "+orphaned+#{Time.now.to_i}"
       Gitlab.config.repositories.storages.each do |name, repository_storage|
-        repo_root = repository_storage.legacy_disk_path
+        repo_root = Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository_storage.legacy_disk_path }
+
         # Look for global repos (legacy, depth 1) and normal repos (depth 2)
         IO.popen(%W(find #{repo_root} -mindepth 1 -maxdepth 2 -name *.git)) do |find|
           find.each_line do |path|

locale/gitlab.pot

@@ -331,7 +331,7 @@ msgstr ""
 msgid "All features are enabled for blank projects, from templates, or when importing, but you can disable them afterward in the project settings."
 msgstr ""
 
-msgid "Allow edits from maintainers."
+msgid "Allow commits from members who can merge to the target branch."
 msgstr ""
 
 msgid "Allow rendering of PlantUML diagrams in Asciidoc documents."
@@ -4894,7 +4894,7 @@ msgstr ""
 msgid "mrWidget|%{metricsLinkStart} Memory %{metricsLinkEnd} usage is %{emphasisStart} unchanged %{emphasisEnd} at %{memoryFrom}MB"
 msgstr ""
 
-msgid "mrWidget|Allows edits from maintainers"
+msgid "mrWidget|Allows commits from members who can merge to the target branch"
 msgstr ""
 
 msgid "mrWidget|Cancel automatic merge"

spec/controllers/application_controller_spec.rb

@@ -477,4 +477,28 @@ describe ApplicationController do
       end
     end
   end
+
+  describe '#access_denied' do
+    controller(described_class) do
+      def index
+        access_denied!(params[:message])
+      end
+    end
+
+    before do
+      sign_in user
+    end
+
+    it 'renders a 404 without a message' do
+      get :index
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
+    it 'renders a 403 when a message is passed to access denied' do
+      get :index, message: 'None shall pass'
+
+      expect(response).to have_gitlab_http_status(403)
+    end
+  end
 end

spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb

@@ -43,13 +43,13 @@ describe ControllerWithCrossProjectAccessCheck do
         end
       end
 
-      it 'renders a 404 with trying to access a cross project page' do
+      it 'renders a 403 with trying to access a cross project page' do
         message = "This page is unavailable because you are not allowed to read "\
                   "information across multiple projects."
 
         get :index
 
-        expect(response).to have_gitlab_http_status(404)
+        expect(response).to have_gitlab_http_status(403)
         expect(response.body).to match(/#{message}/)
       end
 
@@ -119,7 +119,7 @@ describe ControllerWithCrossProjectAccessCheck do
 
         get :index
 
-        expect(response).to have_gitlab_http_status(404)
+        expect(response).to have_gitlab_http_status(403)
       end
 
       it 'is executed when the `unless` condition returns true' do
@@ -127,19 +127,19 @@ describe ControllerWithCrossProjectAccessCheck do
 
         get :index
 
-        expect(response).to have_gitlab_http_status(404)
+        expect(response).to have_gitlab_http_status(403)
       end
 
       it 'does not skip the check on an action that is not skipped' do
         get :show, id: 'hello'
 
-        expect(response).to have_gitlab_http_status(404)
+        expect(response).to have_gitlab_http_status(403)
       end
 
       it 'does not skip the check on an action that was not defined to skip' do
         get :edit, id: 'hello'
 
-        expect(response).to have_gitlab_http_status(404)
+        expect(response).to have_gitlab_http_status(403)
       end
     end
   end

spec/controllers/search_controller_spec.rb

@@ -32,7 +32,7 @@ describe SearchController do
     it 'still blocks searches without a project_id' do
       get :show, search: 'hello'
 
-      expect(response).to have_gitlab_http_status(404)
+      expect(response).to have_gitlab_http_status(403)
     end
 
     it 'allows searches with a project_id' do

spec/controllers/users/terms_controller_spec.rb

 require 'spec_helper'
 
 describe Users::TermsController do
+  include TermsHelper
   let(:user) { create(:user) }
   let(:term) { create(:term) }
 
@@ -15,11 +16,26 @@ describe Users::TermsController do
       expect(response).to have_gitlab_http_status(:redirect)
     end
 
-    it 'shows terms when they exist' do
+    context 'when terms exist' do
+      before do
+        stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
         term
+      end
+
+      it 'shows terms when they exist' do
+        get :index
 
         expect(response).to have_gitlab_http_status(:success)
       end
+
+      it 'shows a message when the user already accepted the terms' do
+        accept_terms(user)
+
+        get :index
+
+        expect(controller).to set_flash.now[:notice].to(/already accepted/)
+      end
+    end
   end
 
   describe 'POST #accept' do

spec/factories/term_agreements.rb

@@ -3,4 +3,12 @@ FactoryBot.define do
     term
     user
   end
+
+  trait :declined do
+    accepted false
+  end
+
+  trait :accepted do
+    accepted true
+  end
 end

spec/features/merge_request/maintainer_edits_fork_spec.rb

@@ -14,7 +14,7 @@ describe 'a maintainer edits files on a source-branch of an MR from a fork', :js
            source_branch: 'fix',
            target_branch: 'master',
            author: author,
-           allow_maintainer_to_push: true)
+           allow_collaboration: true)
   end
 
   before do

spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb → spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb

 require 'spec_helper'
 
-describe 'create a merge request that allows maintainers to push', :js do
+describe 'create a merge request, allowing commits from members who can merge to the target branch', :js do
   include ProjectForksHelper
   let(:user) { create(:user) }
   let(:target_project) { create(:project, :public, :repository) }
@@ -21,16 +21,16 @@ describe 'create a merge request that allows maintainers to push', :js do
     sign_in(user)
   end
 
-  it 'allows setting maintainer push possible' do
+  it 'allows setting possible' do
     visit_new_merge_request
 
-    check 'Allow edits from maintainers'
+    check 'Allow commits from members who can merge to the target branch'
 
     click_button 'Submit merge request'
 
     wait_for_requests
 
-    expect(page).to have_content('Allows edits from maintainers')
+    expect(page).to have_content('Allows commits from members who can merge to the target branch')
   end
 
   it 'shows a message when one of the projects is private' do
@@ -57,12 +57,12 @@ describe 'create a merge request that allows maintainers to push', :js do
 
       visit_new_merge_request
 
-      expect(page).not_to have_content('Allows edits from maintainers')
+      expect(page).not_to have_content('Allows commits from members who can merge to the target branch')
     end
   end
 
-  context 'when a maintainer tries to edit the option' do
-    let(:maintainer) { create(:user) }
+  context 'when a member who can merge tries to edit the option' do
+    let(:member) { create(:user) }
     let(:merge_request) do
       create(:merge_request,
              source_project: source_project,
@@ -71,15 +71,15 @@ describe 'create a merge request that allows maintainers to push', :js do
     end
 
     before do
-      target_project.add_master(maintainer)
+      target_project.add_master(member)
 
-      sign_in(maintainer)
+      sign_in(member)
     end
 
-    it 'it hides the option from maintainers' do
+    it 'it hides the option from members' do
       visit edit_project_merge_request_path(target_project, merge_request)
 
-      expect(page).not_to have_content('Allows edits from maintainers')
+      expect(page).not_to have_content('Allows commits from members who can merge to the target branch')
     end
   end
 end

spec/features/projects/import_export/import_file_spec.rb

@@ -87,12 +87,14 @@ feature 'Import/Export - project import integration test', :js do
 
   def wiki_exists?(project)
     wiki = ProjectWiki.new(project)
-    File.exist?(wiki.repository.path_to_repo) && !wiki.repository.empty?
+    wiki.repository.exists? && !wiki.repository.empty?
   end
 
   def project_hook_exists?(project)
+    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
       Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists?
     end
+  end
 
   def click_import_project_tab
     find('#import-project-tab').click

spec/features/users/terms_spec.rb

@@ -39,6 +39,22 @@ describe 'Users > Terms' do
     end
   end
 
+  context 'when the user has already accepted the terms' do
+    before do
+      accept_terms(user)
+    end
+
+    it 'allows the user to continue to the app' do
+      visit terms_path
+
+      expect(page).to have_content "You have already accepted the Terms of Service as #{user.to_reference}"
+
+      click_link 'Continue'
+
+      expect(current_path).to eq(root_path)
+    end
+  end
+
   context 'terms were enforced while session is active', :js do
     let(:project) { create(:project) }
 

spec/fixtures/api/schemas/entities/merge_request_basic.json

@@ -13,6 +13,7 @@
     "assignee_id": { "type": ["integer", "null"] },
     "subscribed": { "type": ["boolean", "null"] },
     "participants": { "type": "array" },
+    "allow_collaboration": { "type": "boolean"},
     "allow_maintainer_to_push": { "type": "boolean"}
   },
   "additionalProperties": false

spec/fixtures/api/schemas/entities/merge_request_widget.json

@@ -31,7 +31,7 @@
     "source_project_id": { "type": "integer" },
     "target_branch": { "type": "string" },
     "target_project_id": { "type": "integer" },
-    "allow_maintainer_to_push": { "type": "boolean"},
+    "allow_collaboration": { "type": "boolean"},
     "metrics": {
       "oneOf": [
         { "type": "null" },

spec/fixtures/api/schemas/public_api/v4/merge_requests.json

@@ -82,6 +82,7 @@
         "human_time_estimate": { "type": ["string", "null"] },
         "human_total_time_spent": { "type": ["string", "null"] }
       },
+      "allow_collaboration": { "type": ["boolean", "null"] },
       "allow_maintainer_to_push": { "type": ["boolean", "null"] }
     },
     "required": [

spec/initializers/fog_google_https_private_urls_spec.rb

 require 'spec_helper'
 
-describe 'Fog::Storage::GoogleXML::File' do
+describe 'Fog::Storage::GoogleXML::File', :fog_requests do
   let(:storage) do
     Fog.mock!
-    Fog::Storage.new({
+    Fog::Storage.new(
       google_storage_access_key_id: "asdf",
       google_storage_secret_access_key: "asdf",
       provider: "Google"
-                     })
+    )
   end
 
   let(:file) do

spec/lib/backup/files_spec.rb

@@ -46,7 +46,9 @@ describe Backup::Files do
       end
 
       it 'calls tar command with unlink' do
-        expect(subject).to receive(:run_pipeline!).with([%w(gzip -cd), %w(tar --unlink-first --recursive-unlink -C /var/gitlab-registry -xf -)], any_args)
+        expect(subject).to receive(:tar).and_return('blabla-tar')
+
+        expect(subject).to receive(:run_pipeline!).with([%w(gzip -cd), %w(blabla-tar --unlink-first --recursive-unlink -C /var/gitlab-registry -xf -)], any_args)
         subject.restore
       end
     end

spec/lib/backup/manager_spec.rb

@@ -274,16 +274,13 @@ describe Backup::Manager do
         }
       )
 
-      # the Fog mock only knows about directories we create explicitly
       Fog.mock!
+
+      # the Fog mock only knows about directories we create explicitly
       connection = ::Fog::Storage.new(Gitlab.config.backup.upload.connection.symbolize_keys)
       connection.directories.create(key: Gitlab.config.backup.upload.remote_directory)
     end
 
-    after do
-      Fog.unmock!
-    end
-
     context 'target path' do
       it 'uses the tar filename by default' do
         expect_any_instance_of(Fog::Collection).to receive(:create)

spec/lib/backup/repository_spec.rb

@@ -34,9 +34,11 @@ describe Backup::Repository do
     let(:timestamp) { Time.utc(2017, 3, 22) }
     let(:temp_dirs) do
       Gitlab.config.repositories.storages.map do |name, storage|
+        Gitlab::GitalyClient::StorageSettings.allow_disk_access do
           File.join(storage.legacy_disk_path, '..', 'repositories.old.' + timestamp.to_i.to_s)
         end
       end
+    end
 
     around do |example|
       Timecop.freeze(timestamp) { example.run }

spec/lib/gitlab/bare_repository_import/importer_spec.rb

@@ -20,6 +20,13 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do
     Rainbow.enabled = @rainbow
   end
 
+  around do |example|
+    # TODO migrate BareRepositoryImport https://gitlab.com/gitlab-org/gitaly/issues/953
+    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      example.run
+    end
+  end
+
   shared_examples 'importing a repository' do
     describe '.execute' do
       it 'creates a project for a repository in storage' do

spec/lib/gitlab/bare_repository_import/repository_spec.rb

@@ -62,9 +62,11 @@ describe ::Gitlab::BareRepositoryImport::Repository do
 
     before do
       gitlab_shell.create_repository(repository_storage, hashed_path)
+      Gitlab::GitalyClient::StorageSettings.allow_disk_access do
         repository = Rugged::Repository.new(repo_path)
         repository.config['gitlab.fullpath'] = 'to/repo'
       end
+    end
 
     after do
       gitlab_shell.remove_repository(repository_storage, hashed_path)

spec/lib/gitlab/git/repository_spec.rb

@@ -159,6 +159,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     let(:feature2) { 'feature2' }
 
     around do |example|
+      # discover_default_branch will be moved to gitaly-ruby
       Gitlab::GitalyClient::StorageSettings.allow_disk_access do
         example.run
       end
@@ -373,6 +374,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
 
   context '#submodules' do
     around do |example|
+      # TODO #submodules will be removed, has been migrated to gitaly
       Gitlab::GitalyClient::StorageSettings.allow_disk_access do
         example.run
       end
@@ -1055,6 +1057,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
 
   describe "#rugged_commits_between" do
     around do |example|
+      # TODO #rugged_commits_between will be removed, has been migrated to gitaly
       Gitlab::GitalyClient::StorageSettings.allow_disk_access do
         example.run
       end
@@ -1703,6 +1706,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     let(:refs) { ['deadbeef', SeedRepo::RubyBlob::ID, '909e6157199'] }
 
     around do |example|
+      # TODO #batch_existence isn't used anywhere, can we remove it?
       Gitlab::GitalyClient::StorageSettings.allow_disk_access do
         example.run
       end

spec/lib/gitlab/git/rev_list_spec.rb

@@ -9,9 +9,11 @@ describe Gitlab::Git::RevList do
   end
 
   def stub_popen_rev_list(*additional_args, with_lazy_block: true, output:)
+    repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository.path }
+
     params = [
       args_for_popen(additional_args),
-      repository.path,
+      repo_path,
       {},
       hash_including(lazy_block: with_lazy_block ? anything : nil)
     ]