Add latest changes from gitlab-org/gitlab@master

14 files changed, 91 insertions, 68 deletions

diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js
index b839a994455..0c194d67bce 100644
--- a/app/assets/javascripts/lib/utils/text_utility.js
+++ b/app/assets/javascripts/lib/utils/text_utility.js
@@ -36,25 +36,26 @@ export const humanize = string =>
 export const dasherize = str => str.replace(/[_\s]+/g, '-');
 
 /**
- * Replaces whitespaces with hyphens, convert to lower case and remove non-allowed special characters
- * @param {String} str
+ * Replaces whitespace and non-sluggish characters with a given separator
+ * @param {String} str - The string to slugify
+ * @param {String=} separator - The separator used to separate words (defaults to "-")
  * @returns {String}
  */
-export const slugify = str => {
+export const slugify = (str, separator = '-') => {
   const slug = str
     .trim()
     .toLowerCase()
-    .replace(/[^a-zA-Z0-9_.-]+/g, '-');
+    .replace(/[^a-zA-Z0-9_.-]+/g, separator);
 
-  return slug === '-' ? '' : slug;
+  return slug === separator ? '' : slug;
 };
 
 /**
- * Replaces whitespaces with underscore and converts to lower case
+ * Replaces whitespace and non-sluggish characters with underscores
  * @param {String} str
  * @returns {String}
  */
-export const slugifyWithUnderscore = str => str.toLowerCase().replace(/\s+/g, '_');
+export const slugifyWithUnderscore = str => slugify(str, '_');
 
 /**
  * Truncates given text
diff --git a/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue b/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue
index 46d8f61cab6..8749019c5cd 100644
--- a/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue
+++ b/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue
@@ -55,10 +55,12 @@ export default {
       };
     },
   },
-  mounted() {
-    this.verifyTimeRange();
+  watch: {
+    selectedTimeWindow() {
+      this.verifyTimeRange();
+    },
   },
-  updated() {
+  mounted() {
     this.verifyTimeRange();
   },
   methods: {
diff --git a/app/assets/javascripts/new_branch_form.js b/app/assets/javascripts/new_branch_form.js
index 52361e963bc..918c6e408a2 100644
--- a/app/assets/javascripts/new_branch_form.js
+++ b/app/assets/javascripts/new_branch_form.js
@@ -72,16 +72,14 @@ export default class NewBranchForm {
       });
       return `${restriction.prefix} ${formatted.join(restriction.conjunction)}`;
     };
-    const validator = (function(_this) {
-      return function(errors, restriction) {
-        const matched = _this.name.val().match(restriction.pattern);
-        if (matched) {
-          return errors.concat(formatter(matched.reduce(unique, []), restriction));
-        } else {
-          return errors;
-        }
-      };
-    })(this);
+    const validator = (errors, restriction) => {
+      const matched = this.name.val().match(restriction.pattern);
+      if (matched) {
+        return errors.concat(formatter(matched.reduce(unique, []), restriction));
+      } else {
+        return errors;
+      }
+    };
     const errors = this.restrictions.reduce(validator, []);
     if (errors.length > 0) {
       const errorMessage = $('<span/>').text(errors.join(', '));
diff --git a/app/controllers/concerns/renders_commits.rb b/app/controllers/concerns/renders_commits.rb
index ed9b898a2a3..826fae834fa 100644
--- a/app/controllers/concerns/renders_commits.rb
+++ b/app/controllers/concerns/renders_commits.rb
@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 
 module RendersCommits
-  def limited_commits(commits)
-    if commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE
+  def limited_commits(commits, commits_count)
+    if commits_count > MergeRequestDiff::COMMITS_SAFE_SIZE
       [
         commits.first(MergeRequestDiff::COMMITS_SAFE_SIZE),
-        commits.size - MergeRequestDiff::COMMITS_SAFE_SIZE
+        commits_count - MergeRequestDiff::COMMITS_SAFE_SIZE
       ]
     else
       [commits, 0]
@@ -14,9 +14,10 @@ module RendersCommits
 
   # This is used as a helper method in a controller.
   # rubocop: disable Gitlab/ModuleWithInstanceVariables
-  def set_commits_for_rendering(commits)
-    @total_commit_count = commits.size
-    limited, @hidden_commit_count = limited_commits(commits)
+  def set_commits_for_rendering(commits, commits_count: nil)
+    @total_commit_count = commits_count || commits.size
+    limited, @hidden_commit_count = limited_commits(commits, @total_commit_count)
+    commits.each(&:lazy_author) # preload authors
     prepare_commits_for_rendering(limited)
   end
   # rubocop: enable Gitlab/ModuleWithInstanceVariables
diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb
index 808265634da..78dc196b08e 100644
--- a/app/controllers/projects/merge_requests/creations_controller.rb
+++ b/app/controllers/projects/merge_requests/creations_controller.rb
@@ -109,7 +109,13 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
 
     @target_project = @merge_request.target_project
     @source_project = @merge_request.source_project
-    @commits = set_commits_for_rendering(@merge_request.commits)
+
+    @commits =
+      set_commits_for_rendering(
+        @merge_request.recent_commits.with_latest_pipeline(@merge_request.source_branch),
+          commits_count: @merge_request.commits_count
+      )
+
     @commit = @merge_request.diff_head_commit
 
     # FIXME: We have to assign a presenter to another instance variable
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 9f6f6621bf4..59987da21ec 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -90,7 +90,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     # Get commits from repository
     # or from cache if already merged
     @commits =
-      set_commits_for_rendering(@merge_request.commits.with_latest_pipeline)
+      set_commits_for_rendering(
+        @merge_request.recent_commits.with_latest_pipeline(@merge_request.source_branch),
+          commits_count: @merge_request.commits_count
+      )
 
     render json: { html: view_to_html_string('projects/merge_requests/_commits') }
   end
@@ -252,7 +255,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   end
 
   def merge_params_attributes
-    [:should_remove_source_branch, :commit_message, :squash_commit_message, :squash, :auto_merge_strategy]
+    MergeRequest::KNOWN_MERGE_PARAMS
   end
 
   def auto_merge_requested?
@@ -292,7 +295,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
 
     return :sha_mismatch if params[:sha] != @merge_request.diff_head_sha
 
-    @merge_request.update(merge_error: nil, squash: merge_params.fetch(:squash, false))
+    @merge_request.update(merge_error: nil, squash: params.fetch(:squash, false))
 
     if auto_merge_requested?
       if merge_request.auto_merge_enabled?
diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb
index cc71ea8e916..e56009be33d 100644
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -117,12 +117,6 @@ class TodosFinder
     params[:group_id].present?
   end
 
-  def project
-    strong_memoize(:project) do
-      Project.find_without_deleted(params[:project_id]) if project?
-    end
-  end
-
   def group
     strong_memoize(:group) do
       Group.find(params[:group_id])
@@ -181,7 +175,7 @@ class TodosFinder
 
   def by_project(items)
     if project?
-      items.for_project(project)
+      items.for_undeleted_projects.for_project(params[:project_id])
     else
       items
     end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index df516009397..7e1898e7142 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -374,11 +374,12 @@ class MergeRequest < ApplicationRecord
     "#{project.to_reference(from, full: full)}#{reference}"
   end
 
-  def commits
-    return merge_request_diff.commits if persisted?
+  def commits(limit: nil)
+    return merge_request_diff.commits(limit: limit) if persisted?
 
     commits_arr = if compare_commits
-                    compare_commits.reverse
+                    reversed_commits = compare_commits.reverse
+                    limit ? reversed_commits.take(limit) : reversed_commits
                   else
                     []
                   end
@@ -386,6 +387,10 @@ class MergeRequest < ApplicationRecord
     CommitCollection.new(source_project, commits_arr, source_branch)
   end
 
+  def recent_commits
+    commits(limit: MergeRequestDiff::COMMITS_SAFE_SIZE)
+  end
+
   def commits_count
     if persisted?
       merge_request_diff.commits_count
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 5fe97a13a42..70ce4df5678 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -213,8 +213,10 @@ class MergeRequestDiff < ApplicationRecord
     end
   end
 
-  def commits
-    @commits ||= load_commits
+  def commits(limit: nil)
+    strong_memoize(:"commits_#{limit || 'all'}") do
+      load_commits(limit: limit)
+    end
   end
 
   def last_commit_sha
@@ -529,8 +531,9 @@ class MergeRequestDiff < ApplicationRecord
     end
   end
 
-  def load_commits
-    commits = merge_request_diff_commits.map { |commit| Commit.from_hash(commit.to_hash, project) }
+  def load_commits(limit: nil)
+    commits = merge_request_diff_commits.limit(limit)
+      .map { |commit| Commit.from_hash(commit.to_hash, project) }
 
     CommitCollection
       .new(merge_request.source_project, commits, merge_request.source_branch)
diff --git a/app/models/project.rb b/app/models/project.rb
index 1df2a981658..f4aa336fbcd 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -464,13 +464,6 @@ class Project < ApplicationRecord
   # Used by Projects::CleanupService to hold a map of rewritten object IDs
   mount_uploader :bfg_object_map, AttachmentUploader
 
-  # Returns a project, if it is not about to be removed.
-  #
-  # id - The ID of the project to retrieve.
-  def self.find_without_deleted(id)
-    without_deleted.find_by_id(id)
-  end
-
   def self.eager_load_namespace_and_owner
     includes(namespace: :owner)
   end
diff --git a/app/models/todo.rb b/app/models/todo.rb
index 4e48fb3b782..f217c942e8e 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -55,7 +55,8 @@ class Todo < ApplicationRecord
   scope :done, -> { with_state(:done) }
   scope :for_action, -> (action) { where(action: action) }
   scope :for_author, -> (author) { where(author: author) }
-  scope :for_project, -> (project) { where(project: project) }
+  scope :for_project, -> (projects) { where(project: projects) }
+  scope :for_undeleted_projects, -> { joins(:project).merge(Project.without_deleted) }
   scope :for_group, -> (group) { where(group: group) }
   scope :for_type, -> (type) { where(target_type: type) }
   scope :for_target, -> (id) { where(target_id: id) }
diff --git a/app/services/merge_requests/merge_base_service.rb b/app/services/merge_requests/merge_base_service.rb
index 3f7f8bcdcbf..27b5e31faab 100644
--- a/app/services/merge_requests/merge_base_service.rb
+++ b/app/services/merge_requests/merge_base_service.rb
@@ -19,10 +19,12 @@ module MergeRequests
     end
 
     def source
-      if merge_request.squash
-        squash_sha!
-      else
-        merge_request.diff_head_sha
+      strong_memoize(:source) do
+        if merge_request.squash
+          squash_sha!
+        else
+          merge_request.diff_head_sha
+        end
       end
     end
 
@@ -58,16 +60,14 @@ module MergeRequests
     end
 
     def squash_sha!
-      strong_memoize(:squash_sha) do
-        params[:merge_request] = merge_request
-        squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute
-
-        case squash_result[:status]
-        when :success
-          squash_result[:squash_sha]
-        when :error
-          raise ::MergeRequests::MergeService::MergeError, squash_result[:message]
-        end
+      params[:merge_request] = merge_request
+      squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute
+
+      case squash_result[:status]
+      when :success
+        squash_result[:squash_sha]
+      when :error
+        raise ::MergeRequests::MergeService::MergeError, squash_result[:message]
       end
     end
   end
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index 6309052244d..a45b4f1142e 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -37,6 +37,7 @@ module MergeRequests
     def validate!
       authorization_check!
       error_check!
+      updated_check!
     end
 
     def authorization_check!
@@ -60,6 +61,15 @@ module MergeRequests
       raise_error(error) if error
     end
 
+    def updated_check!
+      return unless Feature.enabled?(:validate_merge_sha, merge_request.target_project, default_enabled: false)
+
+      unless source_matches?
+        raise_error('Branch has been updated since the merge was requested. '\
+                    'Please review the changes.')
+      end
+    end
+
     def commit
       log_info("Git merge started on JID #{merge_jid}")
       commit_id = try_merge
@@ -125,5 +135,11 @@ module MergeRequests
     def merge_request_info
       merge_request.to_reference(full: true)
     end
+
+    def source_matches?
+      # params-keys are symbols coming from the controller, but when they get
+      # loaded from the database they're strings
+      params.with_indifferent_access[:sha] == merge_request.diff_head_sha
+    end
   end
 end
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index 7c9abb12b6e..8a6a7119508 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -88,9 +88,9 @@ module MergeRequests
       merge_request.update(merge_error: nil)
 
       if merge_request.head_pipeline && merge_request.head_pipeline.active?
-        AutoMergeService.new(project, current_user).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
+        AutoMergeService.new(project, current_user, { sha: last_diff_sha }).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
       else
-        merge_request.merge_async(current_user.id, {})
+        merge_request.merge_async(current_user.id, { sha: last_diff_sha })
       end
     end