diff options
author | Luke Duncalfe <lduncalfe@gitlab.com> | 2019-02-06 15:33:11 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2019-02-06 15:33:11 +0300 |
commit | 2b7dd017af7de4d09ef3a1cd835e8d07c8800b6a (patch) | |
tree | 47615a573f6dc932353f0f6695cd4fcd050b1201 /app | |
parent | 5bfa8e2f5e03849645570ba8c2dbfcc5c834f1b1 (diff) |
Allow custom squash commit messages
Diffstat (limited to 'app')
-rw-r--r-- | app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js | 4 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 2 | ||||
-rw-r--r-- | app/graphql/types/merge_request_type.rb | 3 | ||||
-rw-r--r-- | app/models/commit.rb | 2 | ||||
-rw-r--r-- | app/models/commit_collection.rb | 9 | ||||
-rw-r--r-- | app/models/merge_request.rb | 10 | ||||
-rw-r--r-- | app/models/repository.rb | 4 | ||||
-rw-r--r-- | app/serializers/merge_request_widget_commit_entity.rb | 7 | ||||
-rw-r--r-- | app/serializers/merge_request_widget_entity.rb | 20 | ||||
-rw-r--r-- | app/services/merge_requests/merge_service.rb | 31 | ||||
-rw-r--r-- | app/services/merge_requests/squash_service.rb | 33 |
11 files changed, 90 insertions, 35 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index ab194e84ab4..36cac230d9d 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -32,10 +32,10 @@ export default class MergeRequestStore { this.sourceBranchProtected = data.source_branch_protected; this.conflictsDocsPath = data.conflicts_docs_path; this.mergeStatus = data.merge_status; - this.commitMessage = data.merge_commit_message; + this.commitMessage = data.default_merge_commit_message; this.shortMergeCommitSha = data.short_merge_commit_sha; this.mergeCommitSha = data.merge_commit_sha; - this.commitMessageWithDescription = data.merge_commit_message_with_description; + this.commitMessageWithDescription = data.default_merge_commit_message_with_description; this.commitsCount = data.commits_count; this.divergedCommitsCount = data.diverged_commits_count; this.pipeline = data.pipeline || {}; diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 7c4dc95529a..5cf7fa3422d 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -240,7 +240,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo end def merge_params_attributes - [:should_remove_source_branch, :commit_message, :squash] + [:should_remove_source_branch, :commit_message, :squash_commit_message, :squash] end def merge_when_pipeline_succeeds_active? diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index fb740b6fb1c..47b915b451e 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -39,7 +39,8 @@ module Types field :rebase_commit_sha, GraphQL::STRING_TYPE, null: true field :rebase_in_progress, GraphQL::BOOLEAN_TYPE, method: :rebase_in_progress?, null: false field :diff_head_sha, GraphQL::STRING_TYPE, null: true - field :merge_commit_message, GraphQL::STRING_TYPE, null: true + field :merge_commit_message, GraphQL::STRING_TYPE, method: :default_merge_commit_message, null: true, deprecation_reason: "Renamed to defaultMergeCommitMessage" + field :default_merge_commit_message, GraphQL::STRING_TYPE, null: true field :merge_ongoing, GraphQL::BOOLEAN_TYPE, method: :merge_ongoing?, null: false field :source_branch_exists, GraphQL::BOOLEAN_TYPE, method: :source_branch_exists?, null: false field :mergeable_discussions_state, GraphQL::BOOLEAN_TYPE, null: true diff --git a/app/models/commit.rb b/app/models/commit.rb index 982e13e2845..f412d252e5c 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -379,7 +379,7 @@ class Commit end def merge_commit? - parents.size > 1 + parent_ids.size > 1 end def merged_merge_request(current_user) diff --git a/app/models/commit_collection.rb b/app/models/commit_collection.rb index 885f61beb05..42ec5b5e664 100644 --- a/app/models/commit_collection.rb +++ b/app/models/commit_collection.rb @@ -3,6 +3,7 @@ # A collection of Commit instances for a specific project and Git reference. class CommitCollection include Enumerable + include Gitlab::Utils::StrongMemoize attr_reader :project, :ref, :commits @@ -20,11 +21,17 @@ class CommitCollection end def committers - emails = commits.reject(&:merge_commit?).map(&:committer_email).uniq + emails = without_merge_commits.map(&:committer_email).uniq User.by_any_email(emails) end + def without_merge_commits + strong_memoize(:without_merge_commits) do + commits.reject(&:merge_commit?) + end + end + # Sets the pipeline status for every commit. # # Setting this status ahead of time removes the need for running a query for diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 84cb8e1c50b..2035bffd829 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -939,7 +939,7 @@ class MergeRequest < ActiveRecord::Base self.target_project.repository.branch_exists?(self.target_branch) end - def merge_commit_message(include_description: false) + def default_merge_commit_message(include_description: false) closes_issues_references = visible_closing_issues_for.map do |issue| issue.to_reference(target_project) end @@ -959,6 +959,13 @@ class MergeRequest < ActiveRecord::Base message.join("\n\n") end + # Returns the oldest multi-line commit message, or the MR title if none found + def default_squash_commit_message + strong_memoize(:default_squash_commit_message) do + commits.without_merge_commits.reverse.find(&:description?)&.safe_message || title + end + end + def reset_merge_when_pipeline_succeeds return unless merge_when_pipeline_succeeds? @@ -967,6 +974,7 @@ class MergeRequest < ActiveRecord::Base if merge_params merge_params.delete('should_remove_source_branch') merge_params.delete('commit_message') + merge_params.delete('squash_commit_message') end self.save diff --git a/app/models/repository.rb b/app/models/repository.rb index 9ae13fbaa80..bfd2608bed4 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1030,12 +1030,12 @@ class Repository remote_branch: merge_request.target_branch) end - def squash(user, merge_request) + def squash(user, merge_request, message) raw.squash(user, merge_request.id, branch: merge_request.target_branch, start_sha: merge_request.diff_start_sha, end_sha: merge_request.diff_head_sha, author: merge_request.author, - message: merge_request.title) + message: message) end def update_submodule(user, submodule, commit_sha, message:, branch:) diff --git a/app/serializers/merge_request_widget_commit_entity.rb b/app/serializers/merge_request_widget_commit_entity.rb new file mode 100644 index 00000000000..50a5c44a6ad --- /dev/null +++ b/app/serializers/merge_request_widget_commit_entity.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class MergeRequestWidgetCommitEntity < Grape::Entity + expose :safe_message, as: :message + expose :short_id + expose :title +end diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index f42abf06e1e..2142ceb6122 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -56,10 +56,23 @@ class MergeRequestWidgetEntity < IssuableEntity merge_request.diff_head_sha.presence end - expose :merge_commit_message expose :actual_head_pipeline, with: PipelineDetailsEntity, as: :pipeline, if: -> (mr, _) { presenter(mr).can_read_pipeline? } + expose :merge_pipeline, with: PipelineDetailsEntity, if: ->(mr, _) { mr.merged? && can?(request.current_user, :read_pipeline, mr.target_project)} + expose :default_squash_commit_message + expose :default_merge_commit_message + + expose :default_merge_commit_message_with_description do |merge_request| + merge_request.default_merge_commit_message(include_description: true) + end + + expose :commits_without_merge_commits, using: MergeRequestWidgetCommitEntity do |merge_request| + merge_request.commits.without_merge_commits + end + + expose :commits_count + # Booleans expose :merge_ongoing?, as: :merge_ongoing expose :work_in_progress?, as: :work_in_progress @@ -77,7 +90,6 @@ class MergeRequestWidgetEntity < IssuableEntity end expose :branch_missing?, as: :branch_missing - expose :commits_count expose :cannot_be_merged?, as: :has_conflicts expose :can_be_merged?, as: :can_be_merged expose :mergeable?, as: :mergeable @@ -205,10 +217,6 @@ class MergeRequestWidgetEntity < IssuableEntity ci_environments_status_project_merge_request_path(merge_request.project, merge_request) end - expose :merge_commit_message_with_description do |merge_request| - merge_request.merge_commit_message(include_description: true) - end - expose :diverged_commits_count do |merge_request| if merge_request.open? && merge_request.diverged_from_target_branch? merge_request.diverged_commits_count diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 70a67baa01c..449997bcf07 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -8,6 +8,8 @@ module MergeRequests # Executed when you do merge via GitLab UI # class MergeService < MergeRequests::BaseService + include Gitlab::Utils::StrongMemoize + MergeError = Class.new(StandardError) attr_reader :merge_request, :source @@ -37,15 +39,10 @@ module MergeRequests end def source - return merge_request.diff_head_sha unless merge_request.squash - - squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute(merge_request) - - case squash_result[:status] - when :success - squash_result[:squash_sha] - when :error - raise ::MergeRequests::MergeService::MergeError, squash_result[:message] + if merge_request.squash + squash_sha! + else + merge_request.diff_head_sha end end @@ -82,8 +79,22 @@ module MergeRequests merge_request.update!(merge_commit_sha: commit_id) 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 + end + end + def try_merge - message = params[:commit_message] || merge_request.merge_commit_message + message = params[:commit_message] || merge_request.default_merge_commit_message repository.merge(current_user, source, merge_request, message) rescue Gitlab::Git::PreReceiveError => e diff --git a/app/services/merge_requests/squash_service.rb b/app/services/merge_requests/squash_service.rb index a439a380255..9d1a5d5e6d4 100644 --- a/app/services/merge_requests/squash_service.rb +++ b/app/services/merge_requests/squash_service.rb @@ -2,15 +2,10 @@ module MergeRequests class SquashService < MergeRequests::WorkingCopyBaseService - def execute(merge_request) - @merge_request = merge_request - @repository = target_project.repository - - squash || error('Failed to squash. Should be done manually.') - end - - def squash - if merge_request.commits_count < 2 + def execute + # If performing a squash would result in no change, then + # immediately return a success message without performing a squash + if merge_request.commits_count < 2 && message.nil? return success(squash_sha: merge_request.diff_head_sha) end @@ -18,7 +13,13 @@ module MergeRequests return error('Squash task canceled: another squash is already in progress.') end - squash_sha = repository.squash(current_user, merge_request) + squash! || error('Failed to squash. Should be done manually.') + end + + private + + def squash! + squash_sha = repository.squash(current_user, merge_request, message || merge_request.default_squash_commit_message) success(squash_sha: squash_sha) rescue => e @@ -26,5 +27,17 @@ module MergeRequests log_error(e.message) false end + + def repository + target_project.repository + end + + def merge_request + params[:merge_request] + end + + def message + params[:squash_commit_message].presence + end end end |