diff options
20 files changed, 783 insertions, 215 deletions
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 new file mode 100644 index 00000000000..a38f25cce35 --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/commit_edit.vue @@ -0,0 +1,40 @@ +<script> +export default { + props: { + value: { + type: String, + required: true, + }, + label: { + type: String, + required: true, + }, + inputId: { + type: String, + required: true, + }, + }, +}; +</script> + +<template> + <li> + <div class="commit-message-editor"> + <div class="d-flex flex-wrap align-items-center justify-content-between"> + <label class="col-form-label" :for="inputId"> + <strong>{{ label }}</strong> + </label> + <slot name="header"></slot> + </div> + <textarea + :id="inputId" + :value="value" + class="form-control js-gfm-input append-bottom-default commit-message-edit" + required="required" + rows="7" + @input="$emit('input', $event.target.value)" + ></textarea> + <slot name="checkbox"></slot> + </div> + </li> +</template> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/commit_message_dropdown.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/commit_message_dropdown.vue new file mode 100644 index 00000000000..b3c1c0e329d --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/commit_message_dropdown.vue @@ -0,0 +1,38 @@ +<script> +import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; + +export default { + components: { + GlDropdown, + GlDropdownItem, + }, + props: { + commits: { + type: Array, + required: true, + default: () => [], + }, + }, +}; +</script> + +<template> + <div> + <gl-dropdown + right + no-caret + text="Use an existing commit message" + variant="link" + class="mr-commit-dropdown" + > + <gl-dropdown-item + v-for="commit in commits" + :key="commit.short_id" + class="text-nowrap text-truncate" + @click="$emit('input', commit.message)" + > + <span class="monospace mr-2">{{ commit.short_id }}</span> {{ commit.title }} + </gl-dropdown-item> + </gl-dropdown> + </div> +</template> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/commits_header.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/commits_header.vue new file mode 100644 index 00000000000..a1d3a09cca4 --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/commits_header.vue @@ -0,0 +1,91 @@ +<script> +import { GlButton } from '@gitlab/ui'; +import _ from 'underscore'; +import { __, n__, sprintf, s__ } from '~/locale'; +import Icon from '~/vue_shared/components/icon.vue'; + +export default { + components: { + Icon, + GlButton, + }, + props: { + isSquashEnabled: { + type: Boolean, + required: true, + }, + commitsCount: { + type: Number, + required: false, + default: 0, + }, + targetBranch: { + type: String, + required: true, + }, + }, + data() { + return { + expanded: false, + }; + }, + computed: { + collapseIcon() { + return this.expanded ? 'chevron-down' : 'chevron-right'; + }, + commitsCountMessage() { + return n__(__('%d commit'), __('%d commits'), this.isSquashEnabled ? 1 : this.commitsCount); + }, + modifyLinkMessage() { + return this.isSquashEnabled ? __('Modify commit messages') : __('Modify merge commit'); + }, + ariaLabel() { + return this.expanded ? __('Collapse') : __('Expand'); + }, + message() { + return sprintf( + s__( + 'mrWidgetCommitsAdded|%{commitCount} and %{mergeCommitCount} will be added to %{targetBranch}.', + ), + { + commitCount: `<strong class="commits-count-message">${this.commitsCountMessage}</strong>`, + mergeCommitCount: `<strong>${s__('mrWidgetCommitsAdded|1 merge commit')}</strong>`, + targetBranch: `<span class="label-branch">${_.escape(this.targetBranch)}</span>`, + }, + false, + ); + }, + }, + methods: { + toggle() { + this.expanded = !this.expanded; + }, + }, +}; +</script> + +<template> + <div> + <div + class="js-mr-widget-commits-count mr-widget-extension clickable d-flex align-items-center px-3 py-2" + @click="toggle()" + > + <gl-button + :aria-label="ariaLabel" + variant="blank" + class="commit-edit-toggle mr-2" + @click.stop="toggle()" + > + <icon :name="collapseIcon" :size="16" /> + </gl-button> + <span v-if="expanded">{{ __('Collapse') }}</span> + <span v-else> + <span v-html="message"></span> + <gl-button variant="link" class="modify-message-button"> + {{ modifyLinkMessage }} + </gl-button> + </span> + </div> + <div v-show="expanded"><slot></slot></div> + </div> +</template> 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 b8f29649eb5..ce4207864ea 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 @@ -2,17 +2,24 @@ import successSvg from 'icons/_icon_status_success.svg'; import warningSvg from 'icons/_icon_status_warning.svg'; import simplePoll from '~/lib/utils/simple_poll'; +import { __ } from '~/locale'; import MergeRequest from '../../../merge_request'; import Flash from '../../../flash'; import statusIcon from '../mr_widget_status_icon.vue'; import eventHub from '../../event_hub'; import SquashBeforeMerge from './squash_before_merge.vue'; +import CommitsHeader from './commits_header.vue'; +import CommitEdit from './commit_edit.vue'; +import CommitMessageDropdown from './commit_message_dropdown.vue'; export default { name: 'ReadyToMerge', components: { statusIcon, SquashBeforeMerge, + CommitsHeader, + CommitEdit, + CommitMessageDropdown, }, props: { mr: { type: Object, required: true }, @@ -22,27 +29,20 @@ export default { return { removeSourceBranch: this.mr.shouldRemoveSourceBranch, mergeWhenBuildSucceeds: false, - useCommitMessageWithDescription: false, setToMergeWhenPipelineSucceeds: false, - showCommitMessageEditor: false, isMakingRequest: false, isMergingImmediately: false, commitMessage: this.mr.commitMessage, squashBeforeMerge: this.mr.squash, successSvg, warningSvg, + squashCommitMessage: this.mr.squashCommitMessage, }; }, computed: { shouldShowMergeWhenPipelineSucceedsText() { return this.mr.isPipelineActive; }, - commitMessageLinkTitle() { - const withDesc = 'Include description in commit message'; - const withoutDesc = "Don't include description in commit message"; - - return this.useCommitMessageWithDescription ? withoutDesc : withDesc; - }, status() { const { pipeline, isPipelineActive, isPipelineFailed, hasCI, ciStatus } = this.mr; @@ -84,9 +84,9 @@ export default { }, mergeButtonText() { if (this.isMergingImmediately) { - return 'Merge in progress'; + return __('Merge in progress'); } else if (this.shouldShowMergeWhenPipelineSucceedsText) { - return 'Merge when pipeline succeeds'; + return __('Merge when pipeline succeeds'); } return 'Merge'; @@ -98,7 +98,7 @@ export default { const { commitMessage } = this; return Boolean( !commitMessage.length || - !this.shouldShowMergeControls() || + !this.shouldShowMergeControls || this.isMakingRequest || this.mr.preventMerge, ); @@ -110,18 +110,14 @@ export default { const { commitsCount, enableSquashBeforeMerge } = this.mr; return enableSquashBeforeMerge && commitsCount > 1; }, - }, - methods: { shouldShowMergeControls() { return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText; }, - updateCommitMessage() { - const cmwd = this.mr.commitMessageWithDescription; - this.useCommitMessageWithDescription = !this.useCommitMessageWithDescription; - this.commitMessage = this.useCommitMessageWithDescription ? cmwd : this.mr.commitMessage; - }, - toggleCommitMessageEditor() { - this.showCommitMessageEditor = !this.showCommitMessageEditor; + }, + methods: { + updateMergeCommitMessage(includeDescription) { + const { commitMessageWithDescription, commitMessage } = this.mr; + this.commitMessage = includeDescription ? commitMessageWithDescription : commitMessage; }, handleMergeButtonClick(mergeWhenBuildSucceeds, mergeImmediately) { // TODO: Remove no-param-reassign @@ -139,6 +135,7 @@ export default { merge_when_pipeline_succeeds: this.setToMergeWhenPipelineSucceeds, should_remove_source_branch: this.removeSourceBranch === true, squash: this.squashBeforeMerge, + squash_commit_message: this.squashCommitMessage, }; this.isMakingRequest = true; @@ -158,7 +155,7 @@ export default { }) .catch(() => { this.isMakingRequest = false; - new Flash('Something went wrong. Please try again.'); // eslint-disable-line + new Flash(__('Something went wrong. Please try again.')); // eslint-disable-line }); }, initiateMergePolling() { @@ -194,7 +191,7 @@ export default { } }) .catch(() => { - new Flash('Something went wrong while merging this merge request. Please try again.'); // eslint-disable-line + new Flash(__('Something went wrong while merging this merge request. Please try again.')); // eslint-disable-line }); }, initiateRemoveSourceBranchPolling() { @@ -223,7 +220,7 @@ export default { } }) .catch(() => { - new Flash('Something went wrong while deleting the source branch. Please try again.'); // eslint-disable-line + new Flash(__('Something went wrong while deleting the source branch. Please try again.')); // eslint-disable-line }); }, }, @@ -231,127 +228,136 @@ export default { </script> <template> - <div class="mr-widget-body media"> - <status-icon :status="iconClass" /> - <div class="media-body"> - <div class="mr-widget-body-controls media space-children"> - <span class="btn-group"> - <button - :disabled="isMergeButtonDisabled" - :class="mergeButtonClass" - type="button" - class="qa-merge-button" - @click="handleMergeButtonClick()" - > - <i v-if="isMakingRequest" class="fa fa-spinner fa-spin" aria-hidden="true"></i> - {{ mergeButtonText }} - </button> - <button - v-if="shouldShowMergeOptionsDropdown" - :disabled="isMergeButtonDisabled" - type="button" - class="btn btn-sm btn-info dropdown-toggle js-merge-moment" - data-toggle="dropdown" - aria-label="Select merge moment" - > - <i class="fa fa-chevron-down qa-merge-moment-dropdown" aria-hidden="true"></i> - </button> - <ul - v-if="shouldShowMergeOptionsDropdown" - class="dropdown-menu dropdown-menu-right" - role="menu" - > - <li> - <a - class="merge_when_pipeline_succeeds qa-merge-when-pipeline-succeeds-option" - href="#" - @click.prevent="handleMergeButtonClick(true)" - > - <span class="media"> - <span class="merge-opt-icon" aria-hidden="true" v-html="successSvg"></span> - <span class="media-body merge-opt-title">Merge when pipeline succeeds</span> - </span> - </a> - </li> - <li> - <a - class="accept-merge-request qa-merge-immediately-option" - href="#" - @click.prevent="handleMergeButtonClick(false, true)" - > - <span class="media"> - <span class="merge-opt-icon" aria-hidden="true" v-html="warningSvg"></span> - <span class="media-body merge-opt-title">Merge immediately</span> - </span> - </a> - </li> - </ul> - </span> - <div class="media-body-wrap space-children"> - <template v-if="shouldShowMergeControls()"> - <label v-if="mr.canRemoveSourceBranch"> - <input - id="remove-source-branch-input" - v-model="removeSourceBranch" - :disabled="isRemoveSourceBranchButtonDisabled" - class="js-remove-source-branch-checkbox" - type="checkbox" - /> - Delete source branch - </label> - - <!-- Placeholder for EE extension of this component --> - <squash-before-merge - v-if="shouldShowSquashBeforeMerge" - v-model="squashBeforeMerge" - :help-path="mr.squashBeforeMergeHelpPath" - :is-disabled="isMergeButtonDisabled" - /> - - <span v-if="mr.ffOnlyEnabled" class="js-fast-forward-message"> - Fast-forward merge without a merge commit - </span> + <div> + <div class="mr-widget-body media"> + <status-icon :status="iconClass" /> + <div class="media-body"> + <div class="mr-widget-body-controls media space-children"> + <span class="btn-group"> <button - v-else :disabled="isMergeButtonDisabled" - class="js-modify-commit-message-button btn btn-default btn-sm" + :class="mergeButtonClass" type="button" - @click="toggleCommitMessageEditor" + class="qa-merge-button" + @click="handleMergeButtonClick()" > - Modify commit message + <i v-if="isMakingRequest" class="fa fa-spinner fa-spin" aria-hidden="true"></i> + {{ mergeButtonText }} </button> - </template> - <template v-else> - <span class="bold js-resolve-mr-widget-items-message"> - You can only merge once the items above are resolved - </span> - </template> - </div> - </div> - <div v-if="showCommitMessageEditor" class="prepend-top-default commit-message-editor"> - <div class="form-group clearfix"> - <label class="col-form-label" for="commit-message"> Commit message </label> - <div class="col-sm-10"> - <div class="commit-message-container"> - <div class="max-width-marker"></div> - <textarea - id="commit-message" - v-model="commitMessage" - class="form-control js-commit-message" - required="required" - rows="14" - name="Commit message" - ></textarea> - </div> - <p class="hint"> - Try to keep the first line under 52 characters and the others under 72 - </p> - <div class="hint"> - <a href="#" @click.prevent="updateCommitMessage"> {{ commitMessageLinkTitle }} </a> - </div> + <button + v-if="shouldShowMergeOptionsDropdown" + :disabled="isMergeButtonDisabled" + type="button" + class="btn btn-sm btn-info dropdown-toggle js-merge-moment" + data-toggle="dropdown" + aria-label="Select merge moment" + > + <i class="fa fa-chevron-down qa-merge-moment-dropdown" aria-hidden="true"></i> + </button> + <ul + v-if="shouldShowMergeOptionsDropdown" + class="dropdown-menu dropdown-menu-right" + role="menu" + > + <li> + <a + class="merge_when_pipeline_succeeds qa-merge-when-pipeline-succeeds-option" + href="#" + @click.prevent="handleMergeButtonClick(true)" + > + <span class="media"> + <span class="merge-opt-icon" aria-hidden="true" v-html="successSvg"></span> + <span class="media-body merge-opt-title">{{ + __('Merge when pipeline succeeds') + }}</span> + </span> + </a> + </li> + <li> + <a + class="accept-merge-request qa-merge-immediately-option" + href="#" + @click.prevent="handleMergeButtonClick(false, true)" + > + <span class="media"> + <span class="merge-opt-icon" aria-hidden="true" v-html="warningSvg"></span> + <span class="media-body merge-opt-title">{{ __('Merge immediately') }}</span> + </span> + </a> + </li> + </ul> + </span> + <div class="media-body-wrap space-children"> + <template v-if="shouldShowMergeControls"> + <label v-if="mr.canRemoveSourceBranch"> + <input + id="remove-source-branch-input" + v-model="removeSourceBranch" + :disabled="isRemoveSourceBranchButtonDisabled" + class="js-remove-source-branch-checkbox" + type="checkbox" + /> + {{ __('Delete source branch') }} + </label> + + <!-- Placeholder for EE extension of this component --> + <squash-before-merge + v-if="shouldShowSquashBeforeMerge" + v-model="squashBeforeMerge" + :help-path="mr.squashBeforeMergeHelpPath" + :is-disabled="isMergeButtonDisabled" + /> + </template> + <template v-else> + <span class="bold js-resolve-mr-widget-items-message"> + {{ __('You can only merge once the items above are resolved') }} + </span> + </template> </div> </div> </div> </div> + <template v-if="shouldShowMergeControls"> + <div v-if="mr.ffOnlyEnabled" class="mr-fast-forward-message"> + {{ __('Fast-forward merge without a merge commit') }} + </div> + <template v-else> + <commits-header + :is-squash-enabled="squashBeforeMerge" + :commits-count="mr.commitsCount" + :target-branch="mr.targetBranch" + > + <ul class="border-top content-list commits-list flex-list"> + <commit-edit + v-if="squashBeforeMerge" + v-model="squashCommitMessage" + :label="__('Squash commit message')" + input-id="squash-message-edit" + squash + > + <commit-message-dropdown + slot="header" + v-model="squashCommitMessage" + :commits="mr.commits" + /> + </commit-edit> + <commit-edit + v-model="commitMessage" + :label="__('Merge commit message')" + input-id="merge-message-edit" + > + <label slot="checkbox"> + <input + id="include-description" + type="checkbox" + @change="updateMergeCommitMessage($event.target.checked)" + /> + {{ __('Include merge request description') }} + </label> + </commit-edit> + </ul> + </commits-header> + </template> + </template> </div> </template> diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index 57c4dfbe3b7..abbbe19c5ef 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -315,7 +315,7 @@ export default { :endpoint="mr.testResultsPath" /> - <div class="mr-widget-section"> + <div class="mr-widget-section p-0"> <component :is="componentName" :mr="mr" :service="service" /> <section v-if="shouldRenderCollaborationStatus" class="mr-info-list mr-links"> 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 36cac230d9d..58363f632a9 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 @@ -42,6 +42,8 @@ export default class MergeRequestStore { this.mergePipeline = data.merge_pipeline || {}; this.deployments = this.deployments || data.deployments || []; this.postMergeDeployments = this.postMergeDeployments || []; + this.commits = data.commits_without_merge_commits || []; + this.squashCommitMessage = data.default_squash_commit_message; this.initRebase(data); if (data.issues_links) { diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index cb449b642e7..c5c3b66438c 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -391,6 +391,11 @@ img.emoji { .flex-no-shrink { flex-shrink: 0; } .ws-initial { white-space: initial; } .overflow-auto { overflow: auto; } +.d-flex-center { + display: flex; + align-items: center; + justify-content: center; +} /** COMMON SIZING CLASSES **/ .w-0 { width: 0; } @@ -402,6 +407,10 @@ img.emoji { .min-height-0 { min-height: 0; } +.w-3 { width: #{3 * $grid-size}; } + +.h-3 { width: #{3 * $grid-size}; } + /** COMMON SPACING CLASSES **/ .gl-pl-0 { padding-left: 0; } .gl-pl-1 { padding-left: #{0.5 * $grid-size}; } diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 7088a6f59dc..96dab609a13 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -243,6 +243,7 @@ $gl-padding-8: 8px; $gl-padding: 16px; $gl-padding-24: 24px; $gl-padding-32: 32px; +$gl-padding-50: 50px; $gl-col-padding: 15px; $gl-input-padding: 10px; $gl-vert-padding: 6px; diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 38a7e199c6a..135730d71e9 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -38,9 +38,7 @@ } .mr-widget-section { - .media { - align-items: center; - } + border-radius: $border-radius-default $border-radius-default 0 0; .code-text { flex: 1; @@ -56,6 +54,11 @@ .mr-widget-extension { border-top: 1px solid $border-color; background-color: $gray-light; + + &.clickable:hover { + background-color: $gl-gray-200; + cursor: pointer; + } } .mr-widget-workflow { @@ -78,6 +81,7 @@ border-top: 0; } +.mr-widget-body, .mr-widget-section, .mr-widget-content, .mr-widget-footer { @@ -87,11 +91,38 @@ .mr-state-widget { color: $gl-text-color; + .commit-message-edit { + border-radius: $border-radius-default; + } + .mr-widget-section, .mr-widget-footer { border-top: solid 1px $border-color; } + .mr-fast-forward-message { + padding-left: $gl-padding-50; + padding-bottom: $gl-padding; + } + + .commits-list { + > li { + padding: $gl-padding; + + @include media-breakpoint-up(md) { + padding-left: $gl-padding-50; + } + } + } + + .mr-commit-dropdown { + .dropdown-menu { + @include media-breakpoint-up(md) { + width: 150%; + } + } + } + .mr-widget-footer { padding: 0; } @@ -405,7 +436,7 @@ } .mr-widget-help { - padding: 10px 16px 10px 48px; + padding: 10px 16px 10px $gl-padding-50; font-style: italic; } @@ -423,10 +454,6 @@ } } -.mr-widget-body-controls { - flex-wrap: wrap; -} - .mr_source_commit, .mr_target_commit { margin-bottom: 0; diff --git a/changelogs/unreleased/56014-better-squash-commit-messages.yml b/changelogs/unreleased/56014-better-squash-commit-messages.yml deleted file mode 100644 index b08d584ac0a..00000000000 --- a/changelogs/unreleased/56014-better-squash-commit-messages.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: Default squash commit message is now selected from the longest commit when - squashing merge requests -merge_request: 24518 -author: -type: changed diff --git a/doc/user/project/merge_requests/img/squash_mr_message.png b/doc/user/project/merge_requests/img/squash_mr_message.png Binary files differnew file mode 100644 index 00000000000..8734cab29aa --- /dev/null +++ b/doc/user/project/merge_requests/img/squash_mr_message.png diff --git a/doc/user/project/merge_requests/squash_and_merge.md b/doc/user/project/merge_requests/squash_and_merge.md index 34cba867e2c..4ff8ec3a7e6 100644 --- a/doc/user/project/merge_requests/squash_and_merge.md +++ b/doc/user/project/merge_requests/squash_and_merge.md @@ -23,11 +23,14 @@ The squashed commit's commit message will be either: - Taken from the first multi-line commit message in the merge. - The merge request's title if no multi-line commit message is found. -Note that the squashed commit is still followed by a merge commit, -as the merge method for this example repository uses a merge commit. -Squashing also works with the fast-forward merge strategy, see -[squashing and fast-forward merge](#squash-and-fast-forward-merge) for more -details. +It can be customized before merging a merge request. + +![A squash commit message editor](img/squash_mr_message.png) + +NOTE: **Note:** +The squashed commit in this example is followed by a merge commit, as the merge method for this example repository uses a merge commit. + +Squashing also works with the fast-forward merge strategy, see [squashing and fast-forward merge](#squash-and-fast-forward-merge) for more details. ## Use cases @@ -60,7 +63,7 @@ This can then be overridden at the time of accepting the merge request: The squashed commit has the following metadata: -- Message: the message of the squash commit. +- Message: the message of the squash commit, or a customized message. - Author: the author of the merge request. - Committer: the user who initiated the squash. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index df4975877df..06af4e21d1e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -32,6 +32,9 @@ msgid_plural "%d commits behind" msgstr[0] "" msgstr[1] "" +msgid "%d commits" +msgstr "" + msgid "%d exporter" msgid_plural "%d exporters" msgstr[0] "" @@ -2531,6 +2534,9 @@ msgstr "" msgid "Delete list" msgstr "" +msgid "Delete source branch" +msgstr "" + msgid "Delete this attachment" msgstr "" @@ -3256,6 +3262,9 @@ msgstr "" msgid "Failure" msgstr "" +msgid "Fast-forward merge without a merge commit" +msgstr "" + msgid "Faster as it re-uses the project workspace (falling back to clone if it doesn't exist)" msgstr "" @@ -3905,6 +3914,9 @@ msgstr "" msgid "Include a Terms of Service agreement and Privacy Policy that all users must accept." msgstr "" +msgid "Include merge request description" +msgstr "" + msgid "Include the username in the URL if required: <code>https://username@gitlab.company.com/group/project.git</code>." msgstr "" @@ -4437,9 +4449,18 @@ msgstr "" msgid "Merge Requests" msgstr "" +msgid "Merge commit message" +msgstr "" + msgid "Merge events" msgstr "" +msgid "Merge immediately" +msgstr "" + +msgid "Merge in progress" +msgstr "" + msgid "Merge request" msgstr "" @@ -4449,6 +4470,9 @@ msgstr "" msgid "Merge requests are a place to propose changes you've made to a project and discuss those changes with others" msgstr "" +msgid "Merge when pipeline succeeds" +msgstr "" + msgid "MergeRequests|Add a reply" msgstr "" @@ -4605,6 +4629,12 @@ msgstr "" msgid "Modal|Close" msgstr "" +msgid "Modify commit messages" +msgstr "" + +msgid "Modify merge commit" +msgstr "" + msgid "Monitor your errors by integrating with Sentry" msgstr "" @@ -6622,6 +6652,9 @@ msgstr "" msgid "Something went wrong while closing the %{issuable}. Please try again later" msgstr "" +msgid "Something went wrong while deleting the source branch. Please try again." +msgstr "" + msgid "Something went wrong while fetching comments. Please try again." msgstr "" @@ -6634,6 +6667,9 @@ msgstr "" msgid "Something went wrong while fetching the registry list." msgstr "" +msgid "Something went wrong while merging this merge request. Please try again." +msgstr "" + msgid "Something went wrong while reopening the %{issuable}. Please try again later" msgstr "" @@ -6778,6 +6814,9 @@ msgstr "" msgid "Specify the following URL during the Runner setup:" msgstr "" +msgid "Squash commit message" +msgstr "" + msgid "Squash commits" msgstr "" @@ -8294,6 +8333,9 @@ msgstr "" msgid "You can only edit files when you are on a branch" msgstr "" +msgid "You can only merge once the items above are resolved" +msgstr "" + msgid "You can resolve the merge conflict using either the Interactive mode, by choosing %{use_ours} or %{use_theirs} buttons, or by editing the files directly. Commit these changes into %{branch_name}" msgstr "" @@ -8588,6 +8630,12 @@ msgstr[1] "" msgid "missing" msgstr "" +msgid "mrWidgetCommitsAdded|%{commitCount} and %{mergeCommitCount} will be added to %{targetBranch}." +msgstr "" + +msgid "mrWidgetCommitsAdded|1 merge commit" +msgstr "" + msgid "mrWidget| Please restore it or use a different %{missingBranchName} branch" msgstr "" diff --git a/spec/features/merge_request/user_accepts_merge_request_spec.rb b/spec/features/merge_request/user_accepts_merge_request_spec.rb index 00ac7c72a11..5fa23dbb998 100644 --- a/spec/features/merge_request/user_accepts_merge_request_spec.rb +++ b/spec/features/merge_request/user_accepts_merge_request_spec.rb @@ -80,8 +80,8 @@ describe 'User accepts a merge request', :js do end it 'accepts a merge request' do - click_button('Modify commit message') - fill_in('Commit message', with: 'wow such merge') + find('.js-mr-widget-commits-count').click + fill_in('merge-message-edit', with: 'wow such merge') click_button('Merge') 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 8d2d4279d3c..c6b11fce388 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 @@ -13,7 +13,7 @@ describe 'Merge request < User customizes merge commit message', :js do description: "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}" ) end - let(:textbox) { page.find(:css, '.js-commit-message', visible: false) } + let(:textbox) { page.find(:css, '#merge-message-edit', visible: false) } let(:default_message) do [ "Merge branch 'feature' into 'master'", @@ -38,16 +38,16 @@ describe 'Merge request < User customizes merge commit message', :js do end it 'toggles commit message between message with description and without description' do - expect(page).not_to have_selector('.js-commit-message') - click_button "Modify commit message" + 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) - click_link "Include description in commit message" + check('Include merge request description') expect(textbox.value).to eq(message_with_description) - click_link "Don't include description in commit message" + uncheck('Include merge request description') expect(textbox.value).to eq(default_message) end diff --git a/spec/javascripts/vue_mr_widget/components/states/commit_edit_spec.js b/spec/javascripts/vue_mr_widget/components/states/commit_edit_spec.js new file mode 100644 index 00000000000..994d6255324 --- /dev/null +++ b/spec/javascripts/vue_mr_widget/components/states/commit_edit_spec.js @@ -0,0 +1,85 @@ +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import CommitEdit from '~/vue_merge_request_widget/components/states/commit_edit.vue'; + +const localVue = createLocalVue(); +const testCommitMessage = 'Test commit message'; +const testLabel = 'Test label'; +const testInputId = 'test-input-id'; + +describe('Commits edit component', () => { + let wrapper; + + const createComponent = (slots = {}) => { + wrapper = shallowMount(localVue.extend(CommitEdit), { + localVue, + sync: false, + propsData: { + value: testCommitMessage, + label: testLabel, + inputId: testInputId, + }, + slots: { + ...slots, + }, + }); + }; + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + const findTextarea = () => wrapper.find('.form-control'); + + it('has a correct label', () => { + const labelElement = wrapper.find('.col-form-label'); + + expect(labelElement.text()).toBe(testLabel); + }); + + describe('textarea', () => { + it('has a correct ID', () => { + expect(findTextarea().attributes('id')).toBe(testInputId); + }); + + it('has a correct value', () => { + expect(findTextarea().element.value).toBe(testCommitMessage); + }); + + it('emits an input event and receives changed value', () => { + const changedCommitMessage = 'Changed commit message'; + + findTextarea().element.value = changedCommitMessage; + findTextarea().trigger('input'); + + expect(wrapper.emitted().input[0]).toEqual([changedCommitMessage]); + expect(findTextarea().element.value).toBe(changedCommitMessage); + }); + }); + + describe('when slots are present', () => { + beforeEach(() => { + createComponent({ + header: `<div class="test-header">${testCommitMessage}</div>`, + checkbox: `<label slot="checkbox" class="test-checkbox">${testLabel}</label >`, + }); + }); + + it('renders header slot correctly', () => { + const headerSlotElement = wrapper.find('.test-header'); + + expect(headerSlotElement.exists()).toBe(true); + expect(headerSlotElement.text()).toBe(testCommitMessage); + }); + + it('renders checkbox slot correctly', () => { + const checkboxSlotElement = wrapper.find('.test-checkbox'); + + expect(checkboxSlotElement.exists()).toBe(true); + expect(checkboxSlotElement.text()).toBe(testLabel); + }); + }); +}); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_commit_message_dropdown_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_commit_message_dropdown_spec.js new file mode 100644 index 00000000000..daf1cc2d98b --- /dev/null +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_commit_message_dropdown_spec.js @@ -0,0 +1,61 @@ +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { GlDropdownItem } from '@gitlab/ui'; +import CommitMessageDropdown from '~/vue_merge_request_widget/components/states/commit_message_dropdown.vue'; + +const localVue = createLocalVue(); +const commits = [ + { + title: 'Commit 1', + short_id: '78d5b7', + message: 'Update test.txt', + }, + { + title: 'Commit 2', + short_id: '34cbe28b', + message: 'Fixed test', + }, + { + title: 'Commit 3', + short_id: 'fa42932a', + message: 'Added changelog', + }, +]; + +describe('Commits message dropdown component', () => { + let wrapper; + + const createComponent = () => { + wrapper = shallowMount(localVue.extend(CommitMessageDropdown), { + localVue, + sync: false, + propsData: { + commits, + }, + }); + }; + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + const findDropdownElements = () => wrapper.findAll(GlDropdownItem); + const findFirstDropdownElement = () => findDropdownElements().at(0); + + it('should have 3 elements in dropdown list', () => { + expect(findDropdownElements().length).toBe(3); + }); + + it('should have correct message for the first dropdown list element', () => { + expect(findFirstDropdownElement().text()).toBe('78d5b7 Commit 1'); + }); + + it('should emit a commit title on selecting commit', () => { + findFirstDropdownElement().vm.$emit('click'); + + expect(wrapper.emitted().input[0]).toEqual(['Update test.txt']); + }); +}); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_commits_header_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_commits_header_spec.js new file mode 100644 index 00000000000..5cf6408cf34 --- /dev/null +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_commits_header_spec.js @@ -0,0 +1,110 @@ +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import CommitsHeader from '~/vue_merge_request_widget/components/states/commits_header.vue'; +import Icon from '~/vue_shared/components/icon.vue'; + +const localVue = createLocalVue(); + +describe('Commits header component', () => { + let wrapper; + + const createComponent = props => { + wrapper = shallowMount(localVue.extend(CommitsHeader), { + localVue, + sync: false, + propsData: { + isSquashEnabled: false, + targetBranch: 'master', + commitsCount: 5, + ...props, + }, + }); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + const findHeaderWrapper = () => wrapper.find('.js-mr-widget-commits-count'); + const findCommitToggle = () => wrapper.find('.commit-edit-toggle'); + const findIcon = () => wrapper.find(Icon); + const findCommitsCountMessage = () => wrapper.find('.commits-count-message'); + const findTargetBranchMessage = () => wrapper.find('.label-branch'); + const findModifyButton = () => wrapper.find('.modify-message-button'); + + describe('when collapsed', () => { + it('toggle has aria-label equal to Expand', () => { + createComponent(); + + expect(findCommitToggle().attributes('aria-label')).toBe('Expand'); + }); + + it('has a chevron-right icon', () => { + createComponent(); + wrapper.setData({ expanded: false }); + + expect(findIcon().props('name')).toBe('chevron-right'); + }); + + describe('when squash is disabled', () => { + beforeEach(() => { + createComponent(); + }); + + it('has commits count message showing correct amount of commits', () => { + expect(findCommitsCountMessage().text()).toBe('5 commits'); + }); + + it('has button with modify merge commit message', () => { + expect(findModifyButton().text()).toBe('Modify merge commit'); + }); + }); + + describe('when squash is enabled', () => { + beforeEach(() => { + createComponent({ isSquashEnabled: true }); + }); + + it('has commits count message showing one commit when squash is enabled', () => { + expect(findCommitsCountMessage().text()).toBe('1 commit'); + }); + + it('has button with modify commit messages text', () => { + expect(findModifyButton().text()).toBe('Modify commit messages'); + }); + }); + + it('has correct target branch displayed', () => { + createComponent(); + + expect(findTargetBranchMessage().text()).toBe('master'); + }); + }); + + describe('when expanded', () => { + beforeEach(() => { + createComponent(); + wrapper.setData({ expanded: true }); + }); + + it('toggle has aria-label equal to collapse', done => { + wrapper.vm.$nextTick(() => { + expect(findCommitToggle().attributes('aria-label')).toBe('Collapse'); + done(); + }); + }); + + it('has a chevron-down icon', done => { + wrapper.vm.$nextTick(() => { + expect(findIcon().props('name')).toBe('chevron-down'); + done(); + }); + }); + + it('has a collapse text', done => { + wrapper.vm.$nextTick(() => { + expect(findHeaderWrapper().text()).toBe('Collapse'); + done(); + }); + }); + }); +}); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index e387367d1a2..631da202d1d 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -1,10 +1,14 @@ import Vue from 'vue'; import ReadyToMerge from '~/vue_merge_request_widget/components/states/ready_to_merge.vue'; import SquashBeforeMerge from '~/vue_merge_request_widget/components/states/squash_before_merge.vue'; +import CommitsHeader from '~/vue_merge_request_widget/components/states/commits_header.vue'; +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'; import eventHub from '~/vue_merge_request_widget/event_hub'; import { createLocalVue, shallowMount } from '@vue/test-utils'; const commitMessage = 'This is the commit message'; +const squashCommitMessage = 'This is the squash commit message'; const commitMessageWithDescription = 'This is the commit message description'; const createTestMr = customConfig => { const mr = { @@ -19,9 +23,11 @@ const createTestMr = customConfig => { sha: '12345678', squash: false, commitMessage, + squashCommitMessage, commitMessageWithDescription, shouldRemoveSourceBranch: true, canRemoveSourceBranch: false, + targetBranch: 'master', }; Object.assign(mr, customConfig.mr); @@ -98,21 +104,6 @@ describe('ReadyToMerge', () => { }); }); - describe('commitMessageLinkTitle', () => { - const withDesc = 'Include description in commit message'; - const withoutDesc = "Don't include description in commit message"; - - it('should return message with description', () => { - expect(vm.commitMessageLinkTitle).toEqual(withDesc); - }); - - it('should return message without description', () => { - vm.useCommitMessageWithDescription = true; - - expect(vm.commitMessageLinkTitle).toEqual(withoutDesc); - }); - }); - describe('status', () => { it('defaults to success', () => { vm.mr.pipeline = true; @@ -279,55 +270,43 @@ describe('ReadyToMerge', () => { vm.mr.isMergeAllowed = false; vm.mr.isPipelineActive = false; - expect(vm.shouldShowMergeControls()).toBeFalsy(); + expect(vm.shouldShowMergeControls).toBeFalsy(); }); it('should return true when the build succeeded or build not required to succeed', () => { vm.mr.isMergeAllowed = true; vm.mr.isPipelineActive = false; - expect(vm.shouldShowMergeControls()).toBeTruthy(); + expect(vm.shouldShowMergeControls).toBeTruthy(); }); it('should return true when showing the MWPS button and a pipeline is running that needs to be successful', () => { vm.mr.isMergeAllowed = false; vm.mr.isPipelineActive = true; - expect(vm.shouldShowMergeControls()).toBeTruthy(); + expect(vm.shouldShowMergeControls).toBeTruthy(); }); it('should return true when showing the MWPS button but not required for the pipeline to succeed', () => { vm.mr.isMergeAllowed = true; vm.mr.isPipelineActive = true; - expect(vm.shouldShowMergeControls()).toBeTruthy(); + expect(vm.shouldShowMergeControls).toBeTruthy(); }); }); - describe('updateCommitMessage', () => { + describe('updateMergeCommitMessage', () => { it('should revert flag and change commitMessage', () => { - expect(vm.useCommitMessageWithDescription).toBeFalsy(); expect(vm.commitMessage).toEqual(commitMessage); - vm.updateCommitMessage(); + vm.updateMergeCommitMessage(true); - expect(vm.useCommitMessageWithDescription).toBeTruthy(); expect(vm.commitMessage).toEqual(commitMessageWithDescription); - vm.updateCommitMessage(); + vm.updateMergeCommitMessage(false); - expect(vm.useCommitMessageWithDescription).toBeFalsy(); expect(vm.commitMessage).toEqual(commitMessage); }); }); - describe('toggleCommitMessageEditor', () => { - it('should toggle showCommitMessageEditor flag', () => { - expect(vm.showCommitMessageEditor).toBeFalsy(); - vm.toggleCommitMessageEditor(); - - expect(vm.showCommitMessageEditor).toBeTruthy(); - }); - }); - describe('handleMergeButtonClick', () => { const returnPromise = status => new Promise(resolve => { @@ -623,7 +602,7 @@ describe('ReadyToMerge', () => { }); }); - describe('Squash checkbox component', () => { + describe('render children components', () => { let wrapper; const localVue = createLocalVue(); @@ -642,25 +621,101 @@ describe('ReadyToMerge', () => { }); const findCheckboxElement = () => wrapper.find(SquashBeforeMerge); + const findCommitsHeaderElement = () => wrapper.find(CommitsHeader); + const findCommitEditElements = () => wrapper.findAll(CommitEdit); + const findCommitDropdownElement = () => wrapper.find(CommitMessageDropdown); + + describe('squash checkbox', () => { + it('should be rendered when squash before merge is enabled and there is more than 1 commit', () => { + createLocalComponent({ + mr: { commitsCount: 2, enableSquashBeforeMerge: true }, + }); - it('should be rendered when squash before merge is enabled and there is more than 1 commit', () => { - createLocalComponent({ - mr: { commitsCount: 2, enableSquashBeforeMerge: true }, + expect(findCheckboxElement().exists()).toBeTruthy(); }); - expect(findCheckboxElement().exists()).toBeTruthy(); + it('should not be rendered when squash before merge is disabled', () => { + createLocalComponent({ mr: { commitsCount: 2, enableSquashBeforeMerge: false } }); + + expect(findCheckboxElement().exists()).toBeFalsy(); + }); + + it('should not be rendered when there is only 1 commit', () => { + createLocalComponent({ mr: { commitsCount: 1, enableSquashBeforeMerge: true } }); + + expect(findCheckboxElement().exists()).toBeFalsy(); + }); }); - it('should not be rendered when squash before merge is disabled', () => { - createLocalComponent({ mr: { commitsCount: 2, enableSquashBeforeMerge: false } }); + describe('commits count collapsible header', () => { + it('should be rendered if fast-forward is disabled', () => { + createLocalComponent(); - expect(findCheckboxElement().exists()).toBeFalsy(); + expect(findCommitsHeaderElement().exists()).toBeTruthy(); + }); + + it('should not be rendered if fast-forward is enabled', () => { + createLocalComponent({ mr: { ffOnlyEnabled: true } }); + + expect(findCommitsHeaderElement().exists()).toBeFalsy(); + }); }); - it('should not be rendered when there is only 1 commit', () => { - createLocalComponent({ mr: { commitsCount: 1, enableSquashBeforeMerge: true } }); + describe('commits edit components', () => { + it('should have one edit component when squash is disabled', () => { + createLocalComponent(); + + expect(findCommitEditElements().length).toBe(1); + }); - expect(findCheckboxElement().exists()).toBeFalsy(); + const findFirstCommitEditLabel = () => + findCommitEditElements() + .at(0) + .props('label'); + + it('should have two edit components when squash is enabled', () => { + createLocalComponent({ + mr: { + commitsCount: 2, + squash: true, + enableSquashBeforeMerge: true, + }, + }); + + expect(findCommitEditElements().length).toBe(2); + }); + + it('should have correct edit merge commit label', () => { + createLocalComponent(); + + expect(findFirstCommitEditLabel()).toBe('Merge commit message'); + }); + + it('should have correct edit squash commit label', () => { + createLocalComponent({ + mr: { + commitsCount: 2, + squash: true, + enableSquashBeforeMerge: true, + }, + }); + + expect(findFirstCommitEditLabel()).toBe('Squash commit message'); + }); + }); + + describe('commits dropdown', () => { + it('should not be rendered if squash is disabled', () => { + createLocalComponent(); + + expect(findCommitDropdownElement().exists()).toBeFalsy(); + }); + + it('should be rendered if squash is enabled', () => { + createLocalComponent({ mr: { squash: true } }); + + expect(findCommitDropdownElement().exists()).toBeTruthy(); + }); }); }); @@ -696,10 +751,6 @@ describe('ReadyToMerge', () => { expect(vm.$el.querySelector('.js-remove-source-branch-checkbox')).toBeNull(); }); - it('does not show modify commit message button', () => { - expect(vm.$el.querySelector('.js-modify-commit-message-button')).toBeNull(); - }); - it('shows message to resolve all items before being allowed to merge', () => { expect(vm.$el.querySelector('.js-resolve-mr-widget-items-message')).toBeDefined(); }); @@ -712,7 +763,7 @@ describe('ReadyToMerge', () => { mr: { ffOnlyEnabled: false }, }); - expect(customVm.$el.querySelector('.js-fast-forward-message')).toBeNull(); + expect(customVm.$el.querySelector('.mr-fast-forward-message')).toBeNull(); expect(customVm.$el.querySelector('.js-modify-commit-message-button')).toBeDefined(); }); @@ -721,7 +772,7 @@ describe('ReadyToMerge', () => { mr: { ffOnlyEnabled: true }, }); - expect(customVm.$el.querySelector('.js-fast-forward-message')).toBeDefined(); + expect(customVm.$el.querySelector('.mr-fast-forward-message')).toBeDefined(); expect(customVm.$el.querySelector('.js-modify-commit-message-button')).toBeNull(); }); }); diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index 75b197fb2ba..6ef07f81705 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -215,12 +215,14 @@ export default { project_archived: false, default_merge_commit_message_with_description: "Merge branch 'daaaa' into 'master'\n\nUpdate README.md\n\nSee merge request !22", + default_squash_commit_message: 'Test squash commit message', diverged_commits_count: 0, only_allow_merge_if_pipeline_succeeds: false, commit_change_content_path: '/root/acets-app/merge_requests/22/commit_change_content', merge_commit_path: 'http://localhost:3000/root/acets-app/commit/53027d060246c8f47e4a9310fb332aa52f221775', troubleshooting_docs_path: 'help', + squash: true, }; export const mockStore = { |