Merge branch 'master' into 41249-clearing-the-cache

13 files changed, 106 insertions, 50 deletions

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 5ca4a7086cb..4251561a0a0 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -96,7 +96,7 @@ module Issuable
 
     strip_attributes :title
 
-    after_save :record_metrics, unless: :imported?
+    after_save :ensure_metrics, unless: :imported?
 
     # We want to use optimistic lock for cases when only title or description are involved
     # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html
@@ -335,11 +335,6 @@ module Issuable
     false
   end
 
-  def record_metrics
-    metrics = self.metrics || create_metrics
-    metrics.record!
-  end
-
   ##
   # Override in issuable specialization
   #
@@ -347,6 +342,10 @@ module Issuable
     false
   end
 
+  def ensure_metrics
+    self.metrics || create_metrics
+  end
+
   ##
   # Overriden in MergeRequest
   #
diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb
index b3020484738..99dbd4fbacf 100644
--- a/app/models/concerns/storage/legacy_namespace.rb
+++ b/app/models/concerns/storage/legacy_namespace.rb
@@ -34,6 +34,8 @@ module Storage
       # So we basically we mute exceptions in next actions
       begin
         send_update_instructions
+        write_projects_repository_config
+
         true
       rescue
         # Returning false does not rollback after_* transaction but gives
diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb
index d67b16584a4..bd6af622bfb 100644
--- a/app/models/diff_discussion.rb
+++ b/app/models/diff_discussion.rb
@@ -23,8 +23,13 @@ class DiffDiscussion < Discussion
   def merge_request_version_params
     return unless for_merge_request?
 
+    version_params = get_params
+
+    return version_params unless on_merge_request_commit? && commit_id
+
+    version_params ||= {}
     version_params.tap do |params|
-      params[:commit_id] = commit_id if on_merge_request_commit?
+      params[:commit_id] = commit_id
     end
   end
 
@@ -37,7 +42,7 @@ class DiffDiscussion < Discussion
 
   private
 
-  def version_params
+  def get_params
     return {} if active?
 
     noteable.version_params_for(position.diff_refs)
diff --git a/app/models/event.rb b/app/models/event.rb
index 0997b056c6a..8a79100de5a 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -48,7 +48,18 @@ class Event < ActiveRecord::Base
 
   belongs_to :author, class_name: "User"
   belongs_to :project
-  belongs_to :target, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
+
+  belongs_to :target, -> {
+    # If the association for "target" defines an "author" association we want to
+    # eager-load this so Banzai & friends don't end up performing N+1 queries to
+    # get the authors of notes, issues, etc.
+    if reflections['events'].active_record.reflect_on_association(:author)
+      includes(:author)
+    else
+      self
+    end
+  }, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
+
   has_one :push_event_payload
 
   # Callbacks
diff --git a/app/models/issue.rb b/app/models/issue.rb
index dc64888b6fc..4eafc1316d6 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -276,6 +276,11 @@ class Issue < ActiveRecord::Base
 
   private
 
+  def ensure_metrics
+    super
+    metrics.record!
+  end
+
   # Returns `true` if the given User can read the current Issue.
   #
   # This method duplicates the same check of issue_policy.rb
diff --git a/app/models/merge_request/metrics.rb b/app/models/merge_request/metrics.rb
index cdc408738be..9e660eccd86 100644
--- a/app/models/merge_request/metrics.rb
+++ b/app/models/merge_request/metrics.rb
@@ -1,12 +1,6 @@
 class MergeRequest::Metrics < ActiveRecord::Base
   belongs_to :merge_request
   belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :pipeline_id
-
-  def record!
-    if merge_request.merged? && self.merged_at.blank?
-      self.merged_at = Time.now
-    end
-
-    self.save
-  end
+  belongs_to :latest_closed_by, class_name: 'User'
+  belongs_to :merged_by, class_name: 'User'
 end
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 0ff169d4531..bdcc9159d26 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -268,4 +268,11 @@ class Namespace < ActiveRecord::Base
   def namespace_previously_created_with_same_path?
     RedirectRoute.permanent.exists?(path: path)
   end
+
+  def write_projects_repository_config
+    all_projects.find_each do |project|
+      project.expires_full_path_cache # we need to clear cache to validate renames correctly
+      project.write_repository_config
+    end
+  end
 end
diff --git a/app/models/project.rb b/app/models/project.rb
index f9c640300ff..9c0bbf697e2 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -226,7 +226,7 @@ class Project < ActiveRecord::Base
   delegate :name, to: :owner, allow_nil: true, prefix: true
   delegate :members, to: :team, prefix: true
   delegate :add_user, :add_users, to: :team
-  delegate :add_guest, :add_reporter, :add_developer, :add_master, to: :team
+  delegate :add_guest, :add_reporter, :add_developer, :add_master, :add_role, to: :team
 
   # Validations
   validates :creator, presence: true, on: :create
@@ -639,7 +639,7 @@ class Project < ActiveRecord::Base
   end
 
   def import?
-    external_import? || forked? || gitlab_project_import?
+    external_import? || forked? || gitlab_project_import? || bare_repository_import?
   end
 
   def no_import?
@@ -679,6 +679,10 @@ class Project < ActiveRecord::Base
     Gitlab::UrlSanitizer.new(import_url).masked_url
   end
 
+  def bare_repository_import?
+    import_type == 'bare_repository'
+  end
+
   def gitlab_project_import?
     import_type == 'gitlab_project'
   end
@@ -1416,6 +1420,8 @@ class Project < ActiveRecord::Base
   end
 
   def after_rename_repo
+    write_repository_config
+
     path_before_change = previous_changes['path'].first
 
     # We need to check if project had been rolled out to move resource to hashed storage or not and decide
@@ -1428,6 +1434,16 @@ class Project < ActiveRecord::Base
     Gitlab::PagesTransfer.new.rename_project(path_before_change, self.path, namespace.full_path)
   end
 
+  def write_repository_config(gl_full_path: full_path)
+    # We'd need to keep track of project full path otherwise directory tree
+    # created with hashed storage enabled cannot be usefully imported using
+    # the import rake task.
+    repo.config['gitlab.fullpath'] = gl_full_path
+  rescue Gitlab::Git::Repository::NoRepository => e
+    Rails.logger.error("Error writing to .git/config for project #{full_path} (#{id}): #{e.message}.")
+    nil
+  end
+
   def rename_repo_notify!
     send_move_instructions(full_path_was)
     expires_full_path_cache
diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb
index b82567ce2b3..c72b01b64af 100644
--- a/app/models/project_services/kubernetes_service.rb
+++ b/app/models/project_services/kubernetes_service.rb
@@ -31,6 +31,7 @@ class KubernetesService < DeploymentService
 
   before_validation :enforce_namespace_to_lower_case
 
+  validate :deprecation_validation, unless: :template?
   validates :namespace,
     allow_blank: true,
     length: 1..63,
@@ -145,6 +146,17 @@ class KubernetesService < DeploymentService
     @kubeclient ||= build_kubeclient!
   end
 
+  def deprecated?
+    !active
+  end
+
+  def deprecation_message
+    content = <<-MESSAGE.strip_heredoc
+    Kubernetes service integration has been deprecated. #{deprecated_message_content} your clusters using the new <a href=\'#{Gitlab::Routing.url_helpers.project_clusters_path(project)}'/>Clusters</a> page
+      MESSAGE
+    content.html_safe
+  end
+
   TEMPLATE_PLACEHOLDER = 'Kubernetes namespace'.freeze
 
   private
@@ -226,4 +238,20 @@ class KubernetesService < DeploymentService
   def enforce_namespace_to_lower_case
     self.namespace = self.namespace&.downcase
   end
+
+  def deprecation_validation
+    return if active_changed?(from: true, to: false)
+
+    if deprecated?
+      errors[:base] << deprecation_message
+    end
+  end
+
+  def deprecated_message_content
+    if active?
+      "Your cluster information on this page is still editable, but you are advised to disable and reconfigure"
+    else
+      "Fields on this page are now uneditable, you can configure"
+    end
+  end
 end
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index c679758973a..a9e5cfb8240 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -7,36 +7,24 @@ class ProjectTeam
     @project = project
   end
 
-  # Shortcut to add users
-  #
-  # Use:
-  #   @team << [@user, :master]
-  #   @team << [@users, :master]
-  #
-  def <<(args)
-    users, access, current_user = *args
-
-    if users.respond_to?(:each)
-      add_users(users, access, current_user: current_user)
-    else
-      add_user(users, access, current_user: current_user)
-    end
-  end
-
   def add_guest(user, current_user: nil)
-    self << [user, :guest, current_user]
+    add_user(user, :guest, current_user: current_user)
   end
 
   def add_reporter(user, current_user: nil)
-    self << [user, :reporter, current_user]
+    add_user(user, :reporter, current_user: current_user)
   end
 
   def add_developer(user, current_user: nil)
-    self << [user, :developer, current_user]
+    add_user(user, :developer, current_user: current_user)
   end
 
   def add_master(user, current_user: nil)
-    self << [user, :master, current_user]
+    add_user(user, :master, current_user: current_user)
+  end
+
+  def add_role(user, role, current_user: nil)
+    send(:"add_#{role}", user, current_user: current_user) # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def find_member(user_id)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index a34f5e5439b..b1fd981965c 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1010,10 +1010,6 @@ class Repository
     raw_repository.fetch_source_branch!(source_repository.raw_repository, source_branch, local_ref)
   end
 
-  def remote_exists?(name)
-    raw_repository.remote_exists?(name)
-  end
-
   def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:)
     raw_repository.compare_source_branch(target_branch_name, source_repository.raw_repository, source_branch_name, straight: straight)
   end
diff --git a/app/models/service.rb b/app/models/service.rb
index 3c4f1885dd0..176b472e724 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -263,6 +263,14 @@ class Service < ActiveRecord::Base
     service
   end
 
+  def deprecated?
+    false
+  end
+
+  def deprecation_message
+    nil
+  end
+
   private
 
   def cache_project_has_external_issue_tracker
diff --git a/app/models/user.rb b/app/models/user.rb
index b52f17cd6a8..4484ee9ff4c 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -94,8 +94,8 @@ class User < ActiveRecord::Base
   has_one :user_synced_attributes_metadata, autosave: true
 
   # Groups
-  has_many :members, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-  has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, source: 'GroupMember' # rubocop:disable Cop/ActiveRecordDependent
+  has_many :members
+  has_many :group_members, -> { where(requested_at: nil) }, source: 'GroupMember'
   has_many :groups, through: :group_members
   has_many :owned_groups, -> { where members: { access_level: Gitlab::Access::OWNER } }, through: :group_members, source: :group
   has_many :masters_groups, -> { where members: { access_level: Gitlab::Access::MASTER } }, through: :group_members, source: :group
@@ -103,7 +103,7 @@ class User < ActiveRecord::Base
   # Projects
   has_many :groups_projects,          through: :groups, source: :projects
   has_many :personal_projects,        through: :namespace, source: :projects
-  has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+  has_many :project_members, -> { where(requested_at: nil) }
   has_many :projects,                 through: :project_members
   has_many :created_projects,         foreign_key: :creator_id, class_name: 'Project'
   has_many :users_star_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
@@ -794,10 +794,7 @@ class User < ActiveRecord::Base
     # `User.select(:id)` raises
     # `ActiveModel::MissingAttributeError: missing attribute: projects_limit`
     # without this safeguard!
-    return unless has_attribute?(:projects_limit)
-
-    connection_default_value_defined = new_record? && !projects_limit_changed?
-    return unless projects_limit.nil? || connection_default_value_defined
+    return unless has_attribute?(:projects_limit) && projects_limit.nil?
 
     self.projects_limit = current_application_settings.default_projects_limit
   end