diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/commit_edit.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/commit_edit.vue index 3eda2828e97ec5ec8e4b6e90340d61e370e5c03d..18761d04c2ed2878acee26c285105d5837d8b88a 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/commit_edit.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/commit_edit.vue @@ -41,7 +41,6 @@ export default { rows="7" @input="$emit('input', $event.target.value)" > - diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index 08a44d81bf0ee56406dbb457ebdb82c49c76cdcf..8830128b7d6ee2f8659e6c8156f14d828eda8952 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -181,9 +181,16 @@ export default { return this.mr.canRemoveSourceBranch; }, commitTemplateHelpPage() { - return helpPagePath('user/project/merge_requests/commit_templates.md', { - anchor: 'merge-commit-message-template', - }); + return helpPagePath('user/project/merge_requests/commit_templates.md'); + }, + commitTemplateHintText() { + if (this.shouldShowSquashEdit && this.shouldShowMergeEdit) { + return this.$options.i18n.mergeAndSquashCommitTemplatesHintText; + } + if (this.shouldShowSquashEdit) { + return this.$options.i18n.squashCommitTemplateHintText; + } + return this.$options.i18n.mergeCommitTemplateHintText; }, commits() { if (this.glFeatures.mergeRequestWidgetGraphql) { @@ -509,6 +516,12 @@ export default { mergeCommitTemplateHintText: s__( 'mrWidget|To change this default message, edit the template for merge commit messages. %{linkStart}Learn more.%{linkEnd}', ), + squashCommitTemplateHintText: s__( + 'mrWidget|To change this default message, edit the template for squash commit messages. %{linkStart}Learn more.%{linkEnd}', + ), + mergeAndSquashCommitTemplatesHintText: s__( + 'mrWidget|To change these default messages, edit the templates for both the merge and squash commit messages. %{linkStart}Learn more.%{linkEnd}', + ), }, }; @@ -674,23 +687,22 @@ export default { :label="__('Merge commit message')" input-id="merge-message-edit" class="gl-m-0! gl-p-0!" - > - - + /> +
  • +

    + + + +

    +
    • - - + /> +
  • +

    + + + +

    +
    • diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 5b17b75a963758fa177e1d5b05cb854d7ee24bcc..ace6fbbf184c16e9401ab4b893a0a5db319c43b7 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -452,6 +452,7 @@ def project_params_attributes :packages_enabled, :service_desk_enabled, :merge_commit_template, + :squash_commit_template, project_setting_attributes: project_setting_attributes ] + [project_feature_attributes: project_feature_attributes] end diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index b6cb9cd330296b3eec39ed5e5c022c16c8521e2e..8ef53ae001b1dbe0bb6ab5a88b6c12b27a161ad9 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -386,6 +386,11 @@ class ProjectType < BaseObject null: true, description: 'Template used to create merge commit message in merge requests.' + field :squash_commit_template, + GraphQL::Types::String, + null: true, + description: 'Template used to create squash commit message in merge requests.' + def label(title:) BatchLoader::GraphQL.for(title).batch(key: project) do |titles, loader, args| LabelsFinder diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 37776cacaeb3e29a49bd6660e11701b62a38d9a5..14e797327fe92abca3c70b6c8a760b576fa04ac7 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1317,7 +1317,7 @@ def target_branch_exists? def default_merge_commit_message(include_description: false) if self.target_project.merge_commit_template.present? && !include_description - return ::Gitlab::MergeRequests::MergeCommitMessage.new(merge_request: self).message + return ::Gitlab::MergeRequests::CommitMessageGenerator.new(merge_request: self).merge_message end closes_issues_references = visible_closing_issues_for.map do |issue| @@ -1340,6 +1340,10 @@ def default_merge_commit_message(include_description: false) end def default_squash_commit_message + if self.target_project.squash_commit_template.present? + return ::Gitlab::MergeRequests::CommitMessageGenerator.new(merge_request: self).squash_message + end + title end diff --git a/app/models/project.rb b/app/models/project.rb index 83eeb43bd21d2326147fc89a7c7882f80c058e8f..b488aca018e3d44a1d6f6d447be1a2916463e199 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -452,6 +452,7 @@ def self.integration_association_name(name) to: :project_setting delegate :active?, to: :prometheus_integration, allow_nil: true, prefix: true delegate :merge_commit_template, :merge_commit_template=, to: :project_setting, allow_nil: true + delegate :squash_commit_template, :squash_commit_template=, to: :project_setting, allow_nil: true delegate :log_jira_dvcs_integration_usage, :jira_dvcs_server_last_sync_at, :jira_dvcs_cloud_last_sync_at, to: :feature_usage diff --git a/app/models/project_setting.rb b/app/models/project_setting.rb index 6c8d2226bc924077ae4bd7d8621e18f1616fdb77..fc834286876f2518ff7e414e58194a40316cc4dc 100644 --- a/app/models/project_setting.rb +++ b/app/models/project_setting.rb @@ -13,6 +13,7 @@ class ProjectSetting < ApplicationRecord self.primary_key = :project_id validates :merge_commit_template, length: { maximum: 500 } + validates :squash_commit_template, length: { maximum: 500 } def squash_enabled_by_default? %w[always default_on].include?(squash_option) diff --git a/app/views/projects/_merge_request_merge_commit_template.html.haml b/app/views/projects/_merge_request_merge_commit_template.html.haml index 185b730e0bb1ced04ea4571ef7dcfa335d4e25e5..ba170af9de5afb4737cab241a5ab50baeab3f259 100644 --- a/app/views/projects/_merge_request_merge_commit_template.html.haml +++ b/app/views/projects/_merge_request_merge_commit_template.html.haml @@ -12,6 +12,6 @@ %p.form-text.text-muted = s_('ProjectSettings|Maximum 500 characters.') = s_('ProjectSettings|Supported variables:') - - Gitlab::MergeRequests::MergeCommitMessage::PLACEHOLDERS.keys.each do |placeholder| + - Gitlab::MergeRequests::CommitMessageGenerator::PLACEHOLDERS.keys.each do |placeholder| %code = "%{#{placeholder}}".html_safe diff --git a/app/views/projects/_merge_request_settings.html.haml b/app/views/projects/_merge_request_settings.html.haml index c5a25bec6eb3d5b92dcc69563bda31c272f5b592..728ff597860efbc003f34f70bb411b6a89e72c0a 100644 --- a/app/views/projects/_merge_request_settings.html.haml +++ b/app/views/projects/_merge_request_settings.html.haml @@ -12,5 +12,7 @@ = render 'projects/merge_request_merge_commit_template', project: @project, form: form += render 'projects/merge_request_squash_commit_template', project: @project, form: form + - if @project.forked? = render 'projects/merge_request_target_project_settings', project: @project, form: form diff --git a/app/views/projects/_merge_request_squash_commit_template.html.haml b/app/views/projects/_merge_request_squash_commit_template.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..51617bc027f1ade755b36057c55f5cadd12be6e9 --- /dev/null +++ b/app/views/projects/_merge_request_squash_commit_template.html.haml @@ -0,0 +1,16 @@ +- form = local_assigns.fetch(:form) + +.form-group + %b= s_('ProjectSettings|Squash commit message template') + %p.text-secondary + - configure_the_squash_commit_message_help_link_url = help_page_path('user/project/merge_requests/commit_templates.md', anchor: 'squash-commit-message-template') + - configure_the_squash_commit_message_help_link_start = ''.html_safe % { url: configure_the_squash_commit_message_help_link_url } + = s_('ProjectSettings|The commit message used when squashing commits. %{link_start}Learn more about syntax and variables.%{link_end}').html_safe % { link_start: configure_the_squash_commit_message_help_link_start, link_end: ''.html_safe } + .mb-2 + = form.text_area :squash_commit_template, class: 'form-control gl-form-input', rows: 8, maxlength: 500, placeholder: '%{title}' + %p.form-text.text-muted + = s_('ProjectSettings|Maximum 500 characters.') + = s_('ProjectSettings|Supported variables:') + - Gitlab::MergeRequests::CommitMessageGenerator::PLACEHOLDERS.keys.each do |placeholder| + %code + = "%{#{placeholder}}".html_safe diff --git a/db/migrate/20211111164025_add_squash_commit_template_to_project_settings.rb b/db/migrate/20211111164025_add_squash_commit_template_to_project_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..6120a6ed0b41a45853f0a44371c70e644d3340bb --- /dev/null +++ b/db/migrate/20211111164025_add_squash_commit_template_to_project_settings.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddSquashCommitTemplateToProjectSettings < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + def change + add_column :project_settings, :squash_commit_template, :text # rubocop:disable Migration/AddLimitToTextColumns + end +end diff --git a/db/migrate/20211111164047_add_squash_commit_template_limit_to_project_settings.rb b/db/migrate/20211111164047_add_squash_commit_template_limit_to_project_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..578d2271d60138bf3715cac1547cb8b9b7bb46c1 --- /dev/null +++ b/db/migrate/20211111164047_add_squash_commit_template_limit_to_project_settings.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddSquashCommitTemplateLimitToProjectSettings < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + add_text_limit :project_settings, :squash_commit_template, 500 + end + + def down + remove_text_limit :project_settings, :squash_commit_template + end +end diff --git a/db/schema_migrations/20211111164025 b/db/schema_migrations/20211111164025 new file mode 100644 index 0000000000000000000000000000000000000000..409cc160b9eafd706375282ced6bc3db1beb528b --- /dev/null +++ b/db/schema_migrations/20211111164025 @@ -0,0 +1 @@ +d78fe687517e14ff67dc76eff63391e33b73d29446d2a0445595175c7cd6806a \ No newline at end of file diff --git a/db/schema_migrations/20211111164047 b/db/schema_migrations/20211111164047 new file mode 100644 index 0000000000000000000000000000000000000000..30e0875cf73f0e1daaff52d7144eecf7c6b731e4 --- /dev/null +++ b/db/schema_migrations/20211111164047 @@ -0,0 +1 @@ +c8ed7f8c0f818156dba9c25be848da97d4eb6dbf0aa9c48f87e940f3ca0967d9 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c9b3243f02536ecb972484e11fe7b25687360179..e9a274f0129be9c563b23a32c4a002b0fd850c1a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18284,7 +18284,9 @@ CREATE TABLE project_settings ( warn_about_potentially_unwanted_characters boolean DEFAULT true NOT NULL, merge_commit_template text, has_shimo boolean DEFAULT false NOT NULL, + squash_commit_template text, CONSTRAINT check_3a03e7557a CHECK ((char_length(previous_default_branch) <= 4096)), + CONSTRAINT check_b09644994b CHECK ((char_length(squash_commit_template) <= 500)), CONSTRAINT check_bde223416c CHECK ((show_default_award_emojis IS NOT NULL)), CONSTRAINT check_eaf7cfb6a7 CHECK ((char_length(merge_commit_template) <= 500)) ); diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 0400dc31128634760ee58610e488d3f11e661334..d49eee5d15c3491f002278eaa4989867571ab9fa 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -12867,6 +12867,7 @@ Represents vulnerability finding of a security report on the pipeline. | `serviceDeskEnabled` | [`Boolean`](#boolean) | Indicates if the project has service desk enabled. | | `sharedRunnersEnabled` | [`Boolean`](#boolean) | Indicates if shared runners are enabled for the project. | | `snippetsEnabled` | [`Boolean`](#boolean) | Indicates if Snippets are enabled for the current user. | +| `squashCommitTemplate` | [`String`](#string) | Template used to create squash commit message in merge requests. | | `squashReadOnly` | [`Boolean!`](#boolean) | Indicates if `squashReadOnly` is enabled. | | `sshUrlToRepo` | [`String`](#string) | URL to connect to the project via SSH. | | `starCount` | [`Int!`](#int) | Number of times the project has been starred. | diff --git a/doc/api/projects.md b/doc/api/projects.md index fb3099091ab10b84fbcb639fcec3c14b5c9473c9..ee84b0b6a01d70ff7f13fd91e5252ea72874f668 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -183,6 +183,7 @@ When the user is authenticated and `simple` is not set this returns something li "autoclose_referenced_issues": true, "suggestion_commit_message": null, "merge_commit_template": null, + "squash_commit_template": null, "marked_for_deletion_at": "2020-04-03", // Deprecated and will be removed in API v5 in favor of marked_for_deletion_on "marked_for_deletion_on": "2020-04-03", "statistics": { @@ -300,6 +301,7 @@ When the user is authenticated and `simple` is not set this returns something li "autoclose_referenced_issues": true, "suggestion_commit_message": null, "merge_commit_template": null, + "squash_commit_template": null, "statistics": { "commit_count": 12, "storage_size": 2066080, @@ -470,6 +472,7 @@ GET /users/:user_id/projects "autoclose_referenced_issues": true, "suggestion_commit_message": null, "merge_commit_template": null, + "squash_commit_template": null, "marked_for_deletion_at": "2020-04-03", // Deprecated and will be removed in API v5 in favor of marked_for_deletion_on "marked_for_deletion_on": "2020-04-03", "statistics": { @@ -587,6 +590,7 @@ GET /users/:user_id/projects "autoclose_referenced_issues": true, "suggestion_commit_message": null, "merge_commit_template": null, + "squash_commit_template": null, "statistics": { "commit_count": 12, "storage_size": 2066080, @@ -714,6 +718,7 @@ Example response: "autoclose_referenced_issues": true, "suggestion_commit_message": null, "merge_commit_template": null, + "squash_commit_template": null, "statistics": { "commit_count": 37, "storage_size": 1038090, @@ -826,6 +831,7 @@ Example response: "autoclose_referenced_issues": true, "suggestion_commit_message": null, "merge_commit_template": null, + "squash_commit_template": null, "statistics": { "commit_count": 12, "storage_size": 2066080, @@ -994,6 +1000,7 @@ GET /projects/:id "autoclose_referenced_issues": true, "suggestion_commit_message": null, "merge_commit_template": null, + "squash_commit_template": null, "marked_for_deletion_at": "2020-04-03", // Deprecated and will be removed in API v5 in favor of marked_for_deletion_on "marked_for_deletion_on": "2020-04-03", "compliance_frameworks": [ "sox" ], @@ -1307,6 +1314,7 @@ POST /projects/user/:user_id | `jobs_enabled` | boolean | **{dotted-circle}** No | _(Deprecated)_ Enable jobs for this project. Use `builds_access_level` instead. | | `lfs_enabled` | boolean | **{dotted-circle}** No | Enable LFS. | | `merge_commit_template` | string | **{dotted-circle}** No | [Template](../user/project/merge_requests/commit_templates.md) used to create merge commit message in merge requests. _([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/20263) in GitLab 14.5.)_ | +| `squash_commit_template` | string | **{dotted-circle}** No | [Template](../user/project/merge_requests/commit_templates.md#squash-commit-message-template) used to create squash commit message in merge requests. _([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/345275) in GitLab 14.6.)_ | | `merge_method` | string | **{dotted-circle}** No | Set the [merge method](#project-merge-method) used. | | `merge_requests_access_level` | string | **{dotted-circle}** No | One of `disabled`, `private`, or `enabled`. | | `merge_requests_enabled` | boolean | **{dotted-circle}** No | _(Deprecated)_ Enable merge requests for this project. Use `merge_requests_access_level` instead. | diff --git a/doc/user/project/merge_requests/commit_templates.md b/doc/user/project/merge_requests/commit_templates.md index b615c86288cdba9041b34c6ad1d5c9e110e424eb..c1a8754b4a2e82e52cc03f1dfc7278703ed851c6 100644 --- a/doc/user/project/merge_requests/commit_templates.md +++ b/doc/user/project/merge_requests/commit_templates.md @@ -31,7 +31,7 @@ This commit message can be customized to follow any guidelines you might have. To do so, expand the **Merge requests** tab within your project's **General** settings and change the **Merge commit message template** text: -![Custom commit message for applied suggestions](img/merge_commit_message_template_v14_5.png) +![Custom commit message for merge commit](img/merge_commit_message_template_v14_5.png) You can use static text and following variables: @@ -49,3 +49,18 @@ Empty variables that are the only word in a line will be removed along with all Merge commit template field has a limit of 500 characters. This limit only applies to the template itself. + +## Squash commit message template + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/345275) in GitLab 14.6. + +As a project maintainer, you're able to configure squash commit message template. It will be used during merge with +squash to create squash commit message. It uses the same syntax and variables as merge commit message template. + +![Custom commit message for squash commit](img/squash_commit_message_template_v14_6.png) + +Default squash commit message can be recreated using following template: + +```plaintext +%{title} +``` diff --git a/doc/user/project/merge_requests/img/squash_commit_message_template_v14_6.png b/doc/user/project/merge_requests/img/squash_commit_message_template_v14_6.png new file mode 100644 index 0000000000000000000000000000000000000000..b4b982354672a66eda75221886a277080bb677b3 Binary files /dev/null and b/doc/user/project/merge_requests/img/squash_commit_message_template_v14_6.png differ diff --git a/doc/user/project/merge_requests/squash_and_merge.md b/doc/user/project/merge_requests/squash_and_merge.md index e551f65b75dda67fca84aee0ba3c9182531966f7..b6c1f29788ce17f5121d0819a490aa1b0628f2e6 100644 --- a/doc/user/project/merge_requests/squash_and_merge.md +++ b/doc/user/project/merge_requests/squash_and_merge.md @@ -28,9 +28,9 @@ NOTE: The squashed commit in this example is followed by a merge commit, because the merge method for this repository uses a merge commit. You can disable merge commits in **Project Settings > General > Merge requests > Merge method > Fast-forward merge**. -The squashed commit's default commit message is taken from the merge request title. +The squashed commit's default commit message is taken from the merge request title. It can be changed using [squash commit message template](commit_templates.md#squash-commit-message-template). -It can be customized before merging a merge request. +It can also be customized before merging a merge request. ![A squash commit message editor](img/squash_mr_message.png) diff --git a/doc/user/project/settings/index.md b/doc/user/project/settings/index.md index 0a3b9d5b2a5d8822598b2ee1e6c06ceddaf32252..3fcb18ca31aeafe0c3ba84ae37a0c44a6972da97 100644 --- a/doc/user/project/settings/index.md +++ b/doc/user/project/settings/index.md @@ -315,7 +315,7 @@ Set up your project's merge request settings: - Enable [require an associated issue from Jira](../../../integration/jira/issues.md#require-associated-jira-issue-for-merge-requests-to-be-merged). - Enable [`delete source branch after merge` option by default](../merge_requests/getting_started.md#deleting-the-source-branch). - Configure [suggested changes commit messages](../merge_requests/reviews/suggestions.md#configure-the-commit-message-for-applied-suggestions). -- Configure [merge commit message template](../merge_requests/commit_templates.md). +- Configure [merge and squash commit message templates](../merge_requests/commit_templates.md). - Configure [the default target project](../merge_requests/creating_merge_requests.md#set-the-default-target-project) for merge requests coming from forks. ### Service Desk diff --git a/ee/app/views/projects/_merge_request_settings.html.haml b/ee/app/views/projects/_merge_request_settings.html.haml index 8ffa06f99a415064ba37fde686c19f558f8a4442..387b0dd7fdfed198bd7ad4e0e9742bf935bd7c7e 100644 --- a/ee/app/views/projects/_merge_request_settings.html.haml +++ b/ee/app/views/projects/_merge_request_settings.html.haml @@ -15,6 +15,8 @@ = render_ce 'projects/merge_request_merge_commit_template', project: @project, form: form += render_ce 'projects/merge_request_squash_commit_template', project: @project, form: form + - if @project.forked? = render_ce 'projects/merge_request_target_project_settings', project: @project, form: form diff --git a/lib/api/entities/project.rb b/lib/api/entities/project.rb index e3f1e90b80f0c643233bc55dc071eac36a443b5e..d7600f8a9b5c4c328bc0a8ea52b60eea1da891e7 100644 --- a/lib/api/entities/project.rb +++ b/lib/api/entities/project.rb @@ -115,6 +115,7 @@ class Project < BasicProjectDetails expose :squash_option expose :suggestion_commit_message expose :merge_commit_template + expose :squash_commit_template expose :statistics, using: 'API::Entities::ProjectStatistics', if: -> (project, options) { options[:statistics] && Ability.allowed?(options[:current_user], :read_statistics, project) } diff --git a/lib/api/helpers/projects_helpers.rb b/lib/api/helpers/projects_helpers.rb index 2425982d405f1fa48f20eb6b377ec51903671653..d7de8bd8b8b3e50a99ec34431975c723de22355b 100644 --- a/lib/api/helpers/projects_helpers.rb +++ b/lib/api/helpers/projects_helpers.rb @@ -62,6 +62,7 @@ module ProjectsHelpers optional :merge_method, type: String, values: %w(ff rebase_merge merge), desc: 'The merge method used when merging merge requests' optional :suggestion_commit_message, type: String, desc: 'The commit message used to apply merge request suggestions' optional :merge_commit_template, type: String, desc: 'Template used to create merge commit message' + optional :squash_commit_template, type: String, desc: 'Template used to create squash commit message' optional :initialize_with_readme, type: Boolean, desc: "Initialize a project with a README.md" optional :ci_default_git_depth, type: Integer, desc: 'Default number of revisions for shallow cloning' optional :auto_devops_enabled, type: Boolean, desc: 'Flag indication if Auto DevOps is enabled' @@ -162,6 +163,7 @@ def self.update_params_at_least_one_of :avatar, :suggestion_commit_message, :merge_commit_template, + :squash_commit_template, :repository_storage, :compliance_framework_setting, :packages_enabled, diff --git a/lib/gitlab/merge_requests/merge_commit_message.rb b/lib/gitlab/merge_requests/commit_message_generator.rb similarity index 83% rename from lib/gitlab/merge_requests/merge_commit_message.rb rename to lib/gitlab/merge_requests/commit_message_generator.rb index 2a6a7859b33667dfe2e8148f323612b3495f0d41..c420385b7c1ba23270754e5d9667e035cca57696 100644 --- a/lib/gitlab/merge_requests/merge_commit_message.rb +++ b/lib/gitlab/merge_requests/commit_message_generator.rb @@ -1,31 +1,21 @@ # frozen_string_literal: true module Gitlab module MergeRequests - class MergeCommitMessage + class CommitMessageGenerator def initialize(merge_request:) @merge_request = merge_request end - def message + def merge_message return unless @merge_request.target_project.merge_commit_template.present? - message = @merge_request.target_project.merge_commit_template - message = message.delete("\r") + replace_placeholders(@merge_request.target_project.merge_commit_template) + end - # Remove placeholders that correspond to empty values and are the last word in the line - # along with all whitespace characters preceding them. - # This allows us to recreate previous default merge commit message behaviour - we skipped new line character - # before empty description and before closed issues when none were present. - PLACEHOLDERS.each do |key, value| - unless value.call(merge_request).present? - message = message.gsub(BLANK_PLACEHOLDERS_REGEXES[key], '') - end - end + def squash_message + return unless @merge_request.target_project.squash_commit_template.present? - Gitlab::StringPlaceholderReplacer - .replace_string_placeholders(message, PLACEHOLDERS_REGEX) do |key| - PLACEHOLDERS[key].call(merge_request) - end + replace_placeholders(@merge_request.target_project.squash_commit_template) end private @@ -55,6 +45,26 @@ def message BLANK_PLACEHOLDERS_REGEXES = (PLACEHOLDERS.map do |key, value| [key, Regexp.new("[\n\r]+%{#{Regexp.escape(key)}}$")] end).to_h.freeze + + def replace_placeholders(message) + # convert CRLF to LF + message = message.delete("\r") + + # Remove placeholders that correspond to empty values and are the last word in the line + # along with all whitespace characters preceding them. + # This allows us to recreate previous default merge commit message behaviour - we skipped new line character + # before empty description and before closed issues when none were present. + PLACEHOLDERS.each do |key, value| + unless value.call(merge_request).present? + message = message.gsub(BLANK_PLACEHOLDERS_REGEXES[key], '') + end + end + + Gitlab::StringPlaceholderReplacer + .replace_string_placeholders(message, PLACEHOLDERS_REGEX) do |key| + PLACEHOLDERS[key].call(merge_request) + end + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index adc07db990ca99197772d2d36cd58b141a6a3ba0..b2fa528d3b63dd9ad34dbea7c96f9e2962993e9b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -27407,6 +27407,9 @@ msgstr "" msgid "ProjectSettings|Snippets" msgstr "" +msgid "ProjectSettings|Squash commit message template" +msgstr "" + msgid "ProjectSettings|Squash commits when merging" msgstr "" @@ -27431,6 +27434,9 @@ msgstr "" msgid "ProjectSettings|The commit message used when merging, if the merge method creates a merge commit. %{link_start}Learn more about syntax and variables.%{link_end}" msgstr "" +msgid "ProjectSettings|The commit message used when squashing commits. %{link_start}Learn more about syntax and variables.%{link_end}" +msgstr "" + msgid "ProjectSettings|The default target project for merge requests created in this fork project." msgstr "" @@ -41660,9 +41666,15 @@ msgstr "" msgid "mrWidget|To approve this merge request, please enter your password. This project requires all approvals to be authenticated." msgstr "" +msgid "mrWidget|To change these default messages, edit the templates for both the merge and squash commit messages. %{linkStart}Learn more.%{linkEnd}" +msgstr "" + msgid "mrWidget|To change this default message, edit the template for merge commit messages. %{linkStart}Learn more.%{linkEnd}" msgstr "" +msgid "mrWidget|To change this default message, edit the template for squash commit messages. %{linkStart}Learn more.%{linkEnd}" +msgstr "" + msgid "mrWidget|To merge, a Jira issue key must be mentioned in the title or description." msgstr "" diff --git a/spec/features/merge_request/user_customizes_merge_commit_message_spec.rb b/spec/features/merge_request/user_customizes_merge_commit_message_spec.rb index 06795344c5c816f8178867aa455def0e92d276a6..67a232607cd019ca27e1173cdb2554b39c44115a 100644 --- a/spec/features/merge_request/user_customizes_merge_commit_message_spec.rb +++ b/spec/features/merge_request/user_customizes_merge_commit_message_spec.rb @@ -7,19 +7,26 @@ let(:user) { project.creator } let(:issue_1) { create(:issue, project: project)} let(:issue_2) { create(:issue, project: project)} + let(:source_branch) { 'csv' } + let(:target_branch) { 'master' } + let(:squash) { false } let(:merge_request) do create( :merge_request, - :simple, source_project: project, - description: "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}" + target_project: project, + source_branch: source_branch, + target_branch: target_branch, + description: "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}", + squash: squash ) end - let(:textbox) { page.find(:css, '#merge-message-edit', visible: false) } - let(:default_message) do + let(:merge_textbox) { page.find(:css, '#merge-message-edit', visible: false) } + let(:squash_textbox) { page.find(:css, '#squash-message-edit', visible: false) } + let(:default_merge_commit_message) do [ - "Merge branch 'feature' into 'master'", + "Merge branch '#{source_branch}' into '#{target_branch}'", merge_request.title, "Closes #{issue_1.to_reference} and #{issue_2.to_reference}", "See merge request #{merge_request.to_reference(full: true)}" @@ -35,8 +42,8 @@ it 'has commit message without description' do expect(page).not_to have_selector('#merge-message-edit') first('.js-mr-widget-commits-count').click - expect(textbox).to be_visible - expect(textbox.value).to eq(default_message) + expect(merge_textbox).to be_visible + expect(merge_textbox.value).to eq(default_merge_commit_message) end context 'when target project has merge commit template set' do @@ -45,8 +52,34 @@ it 'uses merge commit template' do expect(page).not_to have_selector('#merge-message-edit') first('.js-mr-widget-commits-count').click - expect(textbox).to be_visible - expect(textbox.value).to eq(merge_request.title) + expect(merge_textbox).to be_visible + expect(merge_textbox.value).to eq(merge_request.title) + end + end + + context 'when squash is performed' do + let(:squash) { true } + + it 'has default message with merge request title' do + expect(page).not_to have_selector('#squash-message-edit') + first('.js-mr-widget-commits-count').click + expect(squash_textbox).to be_visible + expect(merge_textbox).to be_visible + expect(squash_textbox.value).to eq(merge_request.title) + expect(merge_textbox.value).to eq(default_merge_commit_message) + end + + context 'when target project has squash commit template set' do + let(:project) { create(:project, :public, :repository, squash_commit_template: '%{description}') } + + it 'uses squash commit template' do + expect(page).not_to have_selector('#squash-message-edit') + first('.js-mr-widget-commits-count').click + expect(squash_textbox).to be_visible + expect(merge_textbox).to be_visible + expect(squash_textbox.value).to eq(merge_request.description) + expect(merge_textbox.value).to eq(default_merge_commit_message) + end end end end diff --git a/spec/frontend/vue_mr_widget/components/states/commit_edit_spec.js b/spec/frontend/vue_mr_widget/components/states/commit_edit_spec.js index f965fc32dc12e9cfab6c587f466ab8198c23e525..c30f6f1dfd101a710d8f016046b727eaa0ac609c 100644 --- a/spec/frontend/vue_mr_widget/components/states/commit_edit_spec.js +++ b/spec/frontend/vue_mr_widget/components/states/commit_edit_spec.js @@ -3,7 +3,6 @@ import CommitEdit from '~/vue_merge_request_widget/components/states/commit_edit const testCommitMessage = 'Test commit message'; const testLabel = 'Test label'; -const testTextMuted = 'Test text muted'; const testInputId = 'test-input-id'; describe('Commits edit component', () => { @@ -64,7 +63,6 @@ describe('Commits edit component', () => { beforeEach(() => { createComponent({ header: `
    ${testCommitMessage}
    `, - 'text-muted': `

    ${testTextMuted}

    `, }); }); @@ -74,12 +72,5 @@ describe('Commits edit component', () => { expect(headerSlotElement.exists()).toBe(true); expect(headerSlotElement.text()).toBe(testCommitMessage); }); - - it('renders text-muted slot correctly', () => { - const textMutedElement = wrapper.find('.test-text-muted'); - - expect(textMutedElement.exists()).toBe(true); - expect(textMutedElement.text()).toBe(testTextMuted); - }); }); }); diff --git a/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index a9cf085f84ca083a4178205ba46f1c75088524c2..7082a19a8e72abe51498c7c427788a5c8645d2ca 100644 --- a/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -1,5 +1,6 @@ import { shallowMount } from '@vue/test-utils'; import Vue from 'vue'; +import { GlSprintf } from '@gitlab/ui'; import simplePoll from '~/lib/utils/simple_poll'; import CommitEdit from '~/vue_merge_request_widget/components/states/commit_edit.vue'; import CommitMessageDropdown from '~/vue_merge_request_widget/components/states/commit_message_dropdown.vue'; @@ -487,6 +488,7 @@ describe('ReadyToMerge', () => { const findCommitEditElements = () => wrapper.findAll(CommitEdit); const findCommitDropdownElement = () => wrapper.find(CommitMessageDropdown); const findFirstCommitEditLabel = () => findCommitEditElements().at(0).props('label'); + const findTipLink = () => wrapper.find(GlSprintf); describe('squash checkbox', () => { it('should be rendered when squash before merge is enabled and there is more than 1 commit', () => { @@ -751,6 +753,12 @@ describe('ReadyToMerge', () => { expect(findCommitDropdownElement().exists()).toBeTruthy(); }); }); + + it('renders a tip including a link to docs on templates', () => { + createComponent(); + + expect(findTipLink().exists()).toBe(true); + }); }); describe('Merge request project settings', () => { diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index 4f205e861dd4b139a6611164056475a0bae3d225..adf5507571b4538b5728bc535fd1530411600ad4 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -34,7 +34,7 @@ container_repositories container_repositories_count pipeline_analytics squash_read_only sast_ci_configuration cluster_agent cluster_agents agent_configurations - ci_template timelogs merge_commit_template + ci_template timelogs merge_commit_template squash_commit_template ] expect(described_class).to include_graphql_fields(*expected_fields) diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 9daa3b32fd121f408220c853601a89c80b85fb4e..2e047290d2cdc338071000c874af6fbc1ad2501d 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -562,6 +562,7 @@ Project: - autoclose_referenced_issues - suggestion_commit_message - merge_commit_template +- squash_commit_template ProjectTracingSetting: - external_url Author: diff --git a/spec/lib/gitlab/merge_requests/commit_message_generator_spec.rb b/spec/lib/gitlab/merge_requests/commit_message_generator_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..4de5c9b9c82ae09140d3973fedb3f15dbac1f17b --- /dev/null +++ b/spec/lib/gitlab/merge_requests/commit_message_generator_spec.rb @@ -0,0 +1,243 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do + let(:merge_commit_template) { nil } + let(:squash_commit_template) { nil } + let(:project) do + create( + :project, + :public, + :repository, + merge_commit_template: merge_commit_template, + squash_commit_template: squash_commit_template + ) + end + + let(:user) { project.creator } + let(:merge_request_description) { "Merge Request Description\nNext line" } + let(:merge_request_title) { 'Bugfix' } + let(:merge_request) do + create( + :merge_request, + :simple, + source_project: project, + target_project: project, + author: user, + description: merge_request_description, + title: merge_request_title + ) + end + + subject { described_class.new(merge_request: merge_request) } + + shared_examples_for 'commit message with template' do |message_template_name| + it 'returns nil when template is not set in target project' do + expect(result_message).to be_nil + end + + context 'when project has custom commit template' do + let(message_template_name) { <<~MSG.rstrip } + %{title} + + See merge request %{reference} + MSG + + it 'uses custom template' do + expect(result_message).to eq <<~MSG.rstrip + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + + context 'when project has commit template with closed issues' do + let(message_template_name) { <<~MSG.rstrip } + Merge branch '%{source_branch}' into '%{target_branch}' + + %{title} + + %{issues} + + See merge request %{reference} + MSG + + it 'omits issues and new lines when no issues are mentioned in description' do + expect(result_message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + + context 'when MR closes issues' do + let(:issue_1) { create(:issue, project: project) } + let(:issue_2) { create(:issue, project: project) } + let(:merge_request_description) { "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}" } + + it 'includes them and keeps new line characters' do + expect(result_message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + Closes #{issue_1.to_reference} and #{issue_2.to_reference} + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + end + + context 'when project has commit template with description' do + let(message_template_name) { <<~MSG.rstrip } + Merge branch '%{source_branch}' into '%{target_branch}' + + %{title} + + %{description} + + See merge request %{reference} + MSG + + it 'uses template' do + expect(result_message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + Merge Request Description + Next line + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + + context 'when description is empty string' do + let(:merge_request_description) { '' } + + it 'skips description placeholder and removes new line characters before it' do + expect(result_message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + + context 'when description is nil' do + let(:merge_request_description) { nil } + + it 'skips description placeholder and removes new line characters before it' do + expect(result_message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + + context 'when description is blank string' do + let(:merge_request_description) { "\n\r \n" } + + it 'skips description placeholder and removes new line characters before it' do + expect(result_message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + end + + context 'when custom commit template contains placeholder in the middle or beginning of the line' do + let(message_template_name) { <<~MSG.rstrip } + Merge branch '%{source_branch}' into '%{target_branch}' + + %{description} %{title} + + See merge request %{reference} + MSG + + it 'uses custom template' do + expect(result_message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Merge Request Description + Next line Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + + context 'when description is empty string' do + let(:merge_request_description) { '' } + + it 'does not remove new line characters before empty placeholder' do + expect(result_message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + end + + context 'when project has template with CRLF newlines' do + let(message_template_name) do + "Merge branch '%{source_branch}' into '%{target_branch}'\r\n\r\n%{title}\r\n\r\n%{description}\r\n\r\nSee merge request %{reference}" + end + + it 'converts it to LF newlines' do + expect(result_message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + Merge Request Description + Next line + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + + context 'when description is empty string' do + let(:merge_request_description) { '' } + + it 'skips description placeholder and removes new line characters before it' do + expect(result_message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + end + end + + describe '#merge_message' do + let(:result_message) { subject.merge_message } + + it_behaves_like 'commit message with template', :merge_commit_template + end + + describe '#squash_message' do + let(:result_message) { subject.squash_message } + + it_behaves_like 'commit message with template', :squash_commit_template + end +end diff --git a/spec/lib/gitlab/merge_requests/merge_commit_message_spec.rb b/spec/lib/gitlab/merge_requests/merge_commit_message_spec.rb deleted file mode 100644 index 884f8df5e562c76e34d7fece6f23776ec4c63568..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/merge_requests/merge_commit_message_spec.rb +++ /dev/null @@ -1,219 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::MergeRequests::MergeCommitMessage do - let(:merge_commit_template) { nil } - let(:project) { create(:project, :public, :repository, merge_commit_template: merge_commit_template) } - let(:user) { project.creator } - let(:merge_request_description) { "Merge Request Description\nNext line" } - let(:merge_request_title) { 'Bugfix' } - let(:merge_request) do - create( - :merge_request, - :simple, - source_project: project, - target_project: project, - author: user, - description: merge_request_description, - title: merge_request_title - ) - end - - subject { described_class.new(merge_request: merge_request) } - - it 'returns nil when template is not set in target project' do - expect(subject.message).to be_nil - end - - context 'when project has custom merge commit template' do - let(:merge_commit_template) { <<~MSG.rstrip } - %{title} - - See merge request %{reference} - MSG - - it 'uses custom template' do - expect(subject.message).to eq <<~MSG.rstrip - Bugfix - - See merge request #{merge_request.to_reference(full: true)} - MSG - end - end - - context 'when project has merge commit template with closed issues' do - let(:merge_commit_template) { <<~MSG.rstrip } - Merge branch '%{source_branch}' into '%{target_branch}' - - %{title} - - %{issues} - - See merge request %{reference} - MSG - - it 'omits issues and new lines when no issues are mentioned in description' do - expect(subject.message).to eq <<~MSG.rstrip - Merge branch 'feature' into 'master' - - Bugfix - - See merge request #{merge_request.to_reference(full: true)} - MSG - end - - context 'when MR closes issues' do - let(:issue_1) { create(:issue, project: project) } - let(:issue_2) { create(:issue, project: project) } - let(:merge_request_description) { "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}" } - - it 'includes them and keeps new line characters' do - expect(subject.message).to eq <<~MSG.rstrip - Merge branch 'feature' into 'master' - - Bugfix - - Closes #{issue_1.to_reference} and #{issue_2.to_reference} - - See merge request #{merge_request.to_reference(full: true)} - MSG - end - end - end - - context 'when project has merge commit template with description' do - let(:merge_commit_template) { <<~MSG.rstrip } - Merge branch '%{source_branch}' into '%{target_branch}' - - %{title} - - %{description} - - See merge request %{reference} - MSG - - it 'uses template' do - expect(subject.message).to eq <<~MSG.rstrip - Merge branch 'feature' into 'master' - - Bugfix - - Merge Request Description - Next line - - See merge request #{merge_request.to_reference(full: true)} - MSG - end - - context 'when description is empty string' do - let(:merge_request_description) { '' } - - it 'skips description placeholder and removes new line characters before it' do - expect(subject.message).to eq <<~MSG.rstrip - Merge branch 'feature' into 'master' - - Bugfix - - See merge request #{merge_request.to_reference(full: true)} - MSG - end - end - - context 'when description is nil' do - let(:merge_request_description) { nil } - - it 'skips description placeholder and removes new line characters before it' do - expect(subject.message).to eq <<~MSG.rstrip - Merge branch 'feature' into 'master' - - Bugfix - - See merge request #{merge_request.to_reference(full: true)} - MSG - end - end - - context 'when description is blank string' do - let(:merge_request_description) { "\n\r \n" } - - it 'skips description placeholder and removes new line characters before it' do - expect(subject.message).to eq <<~MSG.rstrip - Merge branch 'feature' into 'master' - - Bugfix - - See merge request #{merge_request.to_reference(full: true)} - MSG - end - end - end - - context 'when custom merge commit template contains placeholder in the middle or beginning of the line' do - let(:merge_commit_template) { <<~MSG.rstrip } - Merge branch '%{source_branch}' into '%{target_branch}' - - %{description} %{title} - - See merge request %{reference} - MSG - - it 'uses custom template' do - expect(subject.message).to eq <<~MSG.rstrip - Merge branch 'feature' into 'master' - - Merge Request Description - Next line Bugfix - - See merge request #{merge_request.to_reference(full: true)} - MSG - end - - context 'when description is empty string' do - let(:merge_request_description) { '' } - - it 'does not remove new line characters before empty placeholder' do - expect(subject.message).to eq <<~MSG.rstrip - Merge branch 'feature' into 'master' - - Bugfix - - See merge request #{merge_request.to_reference(full: true)} - MSG - end - end - end - - context 'when project has template with CRLF newlines' do - let(:merge_commit_template) do - "Merge branch '%{source_branch}' into '%{target_branch}'\r\n\r\n%{title}\r\n\r\n%{description}\r\n\r\nSee merge request %{reference}" - end - - it 'converts it to LF newlines' do - expect(subject.message).to eq <<~MSG.rstrip - Merge branch 'feature' into 'master' - - Bugfix - - Merge Request Description - Next line - - See merge request #{merge_request.to_reference(full: true)} - MSG - end - - context 'when description is empty string' do - let(:merge_request_description) { '' } - - it 'skips description placeholder and removes new line characters before it' do - expect(subject.message).to eq <<~MSG.rstrip - Merge branch 'feature' into 'master' - - Bugfix - - See merge request #{merge_request.to_reference(full: true)} - MSG - end - end - end -end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 92ef9aeb2d8c4a7dc7d9ae54c0010a393014c465..102800bcca28a485d8cef1b2e085c499ed4697a2 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -178,6 +178,13 @@ it 'returns the merge request title' do expect(subject.default_squash_commit_message).to eq(subject.title) end + + it 'uses template from target project' do + subject.target_project.squash_commit_template = 'Squashed branch %{source_branch} into %{target_branch}' + + expect(subject.default_squash_commit_message) + .to eq('Squashed branch master into feature') + end end describe 'modules' do diff --git a/spec/views/projects/edit.html.haml_spec.rb b/spec/views/projects/edit.html.haml_spec.rb index 60f4c1664f7549e976dd8091b49519f053530141..8c96f286c79cb9712799b3592defa77cb2978af7 100644 --- a/spec/views/projects/edit.html.haml_spec.rb +++ b/spec/views/projects/edit.html.haml_spec.rb @@ -92,6 +92,22 @@ end end + context 'squash template' do + it 'displays a placeholder if none is set' do + render + + expect(rendered).to have_field('project[squash_commit_template]', placeholder: '%{title}') + end + + it 'displays the user entered value' do + project.update!(squash_commit_template: '%{first_multiline_commit}') + + render + + expect(rendered).to have_field('project[squash_commit_template]', with: '%{first_multiline_commit}') + end + end + context 'forking' do before do assign(:project, project)