diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-11-15 18:06:12 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-11-15 18:06:12 +0300 |
commit | 6e81d7f6283fae1b22f66b9d9b133243921cbd9e (patch) | |
tree | 8cf8052ef6734ceeb49314f15ff07d2720511f0d /app | |
parent | 3fc9a8e6957ddf75576dc63069c4c0249514499f (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'app')
-rw-r--r-- | app/assets/javascripts/lib/utils/text_utility.js | 15 | ||||
-rw-r--r-- | app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue | 8 | ||||
-rw-r--r-- | app/assets/javascripts/new_branch_form.js | 18 | ||||
-rw-r--r-- | app/controllers/concerns/renders_commits.rb | 13 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests/creations_controller.rb | 8 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 9 | ||||
-rw-r--r-- | app/finders/todos_finder.rb | 8 | ||||
-rw-r--r-- | app/models/merge_request.rb | 11 | ||||
-rw-r--r-- | app/models/merge_request_diff.rb | 11 | ||||
-rw-r--r-- | app/models/project.rb | 7 | ||||
-rw-r--r-- | app/models/todo.rb | 3 | ||||
-rw-r--r-- | app/services/merge_requests/merge_base_service.rb | 28 | ||||
-rw-r--r-- | app/services/merge_requests/merge_service.rb | 16 | ||||
-rw-r--r-- | app/services/merge_requests/update_service.rb | 4 |
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 |