diff options
Diffstat (limited to 'app')
25 files changed, 240 insertions, 195 deletions
diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index 05f34391323..bdb50606a53 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -567,7 +567,7 @@ GitLabDropdown = (function() { e.stopPropagation(); // This prevents automatic scrolling to the top - if ($target.is('a')) { + if ($target.closest('a').length) { return false; } } diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index 2314f7b80cf..b0de142d9d8 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -1,12 +1,5 @@ <script> -import { - GlButton, - GlDropdown, - GlDropdownItem, - GlModal, - GlModalDirective, - GlLink, -} from '@gitlab/ui'; +import { GlButton, GlDropdown, GlDropdownItem, GlModal, GlModalDirective } from '@gitlab/ui'; import _ from 'underscore'; import { mapActions, mapState } from 'vuex'; import { s__ } from '~/locale'; @@ -31,7 +24,6 @@ export default { GlButton, GlDropdown, GlDropdownItem, - GlLink, GlModal, }, directives: { @@ -255,7 +247,9 @@ export default { v-for="(value, key) in timeWindows" :key="key" :active="activeTimeWindow(key)" - ><gl-link :href="setTimeWindowParameter(key)">{{ value }}</gl-link></gl-dropdown-item + :href="setTimeWindowParameter(key)" + active-class="active" + >{{ value }}</gl-dropdown-item > </gl-dropdown> </div> diff --git a/app/assets/javascripts/pipelines/components/graph/job_name_component.vue b/app/assets/javascripts/pipelines/components/graph/job_name_component.vue index 02451839330..7125790ac3d 100644 --- a/app/assets/javascripts/pipelines/components/graph/job_name_component.vue +++ b/app/assets/javascripts/pipelines/components/graph/job_name_component.vue @@ -25,7 +25,7 @@ export default { }; </script> <template> - <span class="ci-job-name-component"> + <span class="ci-job-name-component mw-100"> <ci-icon :status="status" /> <span class="ci-status-text text-truncate mw-70p gl-pl-1 d-inline-block align-bottom"> {{ name }} diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_auto_merge_enabled.vue index 88e1ccbaf35..5958c2cf87e 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_auto_merge_enabled.vue @@ -1,15 +1,19 @@ <script> +import _ from 'underscore'; +import autoMergeMixin from 'ee_else_ce/vue_merge_request_widget/mixins/auto_merge'; import Flash from '../../../flash'; import statusIcon from '../mr_widget_status_icon.vue'; import MrWidgetAuthor from '../../components/mr_widget_author.vue'; import eventHub from '../../event_hub'; +import { AUTO_MERGE_STRATEGIES } from '../../constants'; export default { - name: 'MRWidgetMergeWhenPipelineSucceeds', + name: 'MRWidgetAutoMergeEnabled', components: { MrWidgetAuthor, statusIcon, }, + mixins: [autoMergeMixin], props: { mr: { type: Object, @@ -57,7 +61,7 @@ export default { removeSourceBranch() { const options = { sha: this.mr.sha, - auto_merge_strategy: 'merge_when_pipeline_succeeds', + auto_merge_strategy: this.mr.autoMergeStrategy, should_remove_source_branch: true, }; @@ -66,7 +70,7 @@ export default { .merge(options) .then(res => res.data) .then(data => { - if (data.status === 'merge_when_pipeline_succeeds') { + if (_.includes(AUTO_MERGE_STRATEGIES, data.status)) { eventHub.$emit('MRWidgetUpdateRequested'); } }) @@ -84,9 +88,9 @@ export default { <div class="media-body"> <h4 class="d-flex align-items-start"> <span class="append-right-10"> - {{ s__('mrWidget|Set by') }} + <span class="js-status-text-before-author">{{ statusTextBeforeAuthor }}</span> <mr-widget-author :author="mr.setToAutoMergeBy" /> - {{ s__('mrWidget|to be merged automatically when the pipeline succeeds') }} + <span class="js-status-text-after-author">{{ statusTextAfterAuthor }}</span> </span> <a v-if="mr.canCancelAutomaticMerge" @@ -97,7 +101,7 @@ export default { @click.prevent="cancelAutomaticMerge" > <i v-if="isCancellingAutoMerge" class="fa fa-spinner fa-spin" aria-hidden="true"> </i> - {{ s__('mrWidget|Cancel automatic merge') }} + {{ cancelButtonText }} </a> </h4> <section class="mr-info-list"> 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 615d59a7b8e..ca1b4a57717 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 @@ -1,4 +1,5 @@ <script> +import _ from 'underscore'; import successSvg from 'icons/_icon_status_success.svg'; import warningSvg from 'icons/_icon_status_warning.svg'; import simplePoll from '~/lib/utils/simple_poll'; @@ -12,6 +13,7 @@ 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'; +import { AUTO_MERGE_STRATEGIES } from '../../constants'; export default { name: 'ReadyToMerge', @@ -30,8 +32,6 @@ export default { data() { return { removeSourceBranch: this.mr.shouldRemoveSourceBranch, - mergeWhenBuildSucceeds: false, - autoMergeStrategy: undefined, isMakingRequest: false, isMergingImmediately: false, commitMessage: this.mr.commitMessage, @@ -42,18 +42,18 @@ export default { }; }, computed: { - shouldShowAutoMergeText() { - return this.mr.isPipelineActive; + isAutoMergeAvailable() { + return !_.isEmpty(this.mr.availableAutoMergeStrategies); }, status() { - const { pipeline, isPipelineActive, isPipelineFailed, hasCI, ciStatus } = this.mr; + const { pipeline, isPipelineFailed, hasCI, ciStatus } = this.mr; if (hasCI && !ciStatus) { return 'failed'; + } else if (this.isAutoMergeAvailable) { + return 'pending'; } else if (!pipeline) { return 'success'; - } else if (isPipelineActive) { - return 'pending'; } else if (isPipelineFailed) { return 'failed'; } @@ -87,14 +87,14 @@ export default { mergeButtonText() { if (this.isMergingImmediately) { return __('Merge in progress'); - } else if (this.shouldShowAutoMergeText) { - return __('Merge when pipeline succeeds'); + } else if (this.isAutoMergeAvailable) { + return this.autoMergeText; } - return 'Merge'; + return __('Merge'); }, shouldShowMergeOptionsDropdown() { - return this.mr.isPipelineActive && !this.mr.onlyAllowMergeIfPipelineSucceeds; + return this.isAutoMergeAvailable && !this.mr.onlyAllowMergeIfPipelineSucceeds; }, isRemoveSourceBranchButtonDisabled() { return this.isMergeButtonDisabled; @@ -104,7 +104,7 @@ export default { return enableSquashBeforeMerge && commitsCount > 1; }, shouldShowMergeControls() { - return this.mr.isMergeAllowed || this.shouldShowAutoMergeText; + return this.mr.isMergeAllowed || this.isAutoMergeAvailable; }, shouldShowSquashEdit() { return this.squashBeforeMerge && this.shouldShowSquashBeforeMerge; @@ -118,20 +118,15 @@ export default { const { commitMessageWithDescription, commitMessage } = this.mr; this.commitMessage = includeDescription ? commitMessageWithDescription : commitMessage; }, - handleMergeButtonClick(mergeWhenBuildSucceeds, mergeImmediately) { - // TODO: Remove no-param-reassign - if (mergeWhenBuildSucceeds === undefined) { - mergeWhenBuildSucceeds = this.mr.isPipelineActive; // eslint-disable-line no-param-reassign - } else if (mergeImmediately) { + handleMergeButtonClick(useAutoMerge, mergeImmediately = false) { + if (mergeImmediately) { this.isMergingImmediately = true; } - this.autoMergeStrategy = mergeWhenBuildSucceeds ? 'merge_when_pipeline_succeeds' : undefined; - const options = { sha: this.mr.sha, commit_message: this.commitMessage, - auto_merge_strategy: this.autoMergeStrategy, + auto_merge_strategy: useAutoMerge ? this.mr.preferredAutoMergeStrategy : undefined, should_remove_source_branch: this.removeSourceBranch === true, squash: this.squashBeforeMerge, squash_commit_message: this.squashCommitMessage, @@ -144,7 +139,7 @@ export default { .then(data => { const hasError = data.status === 'failed' || data.status === 'hook_validation_error'; - if (data.status === 'merge_when_pipeline_succeeds') { + if (_.includes(AUTO_MERGE_STRATEGIES, data.status)) { eventHub.$emit('MRWidgetUpdateRequested'); } else if (data.status === 'success') { this.initiateMergePolling(); @@ -242,13 +237,13 @@ export default { :class="mergeButtonClass" type="button" class="qa-merge-button" - @click="handleMergeButtonClick()" + @click="handleMergeButtonClick(isAutoMergeAvailable)" > <i v-if="isMakingRequest" class="fa fa-spinner fa-spin" aria-hidden="true"></i> {{ mergeButtonText }} </button> <button - v-if="shouldShowMergeOptionsDropdown" + v-if="isAutoMergeAvailable" :disabled="isMergeButtonDisabled" type="button" class="btn btn-sm btn-info dropdown-toggle js-merge-moment" @@ -264,15 +259,13 @@ export default { > <li> <a - class="merge_when_pipeline_succeeds qa-merge-when-pipeline-succeeds-option" + class="auto_merge_enabled 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 class="media-body merge-opt-title">{{ autoMergeText }}</span> </span> </a> </li> diff --git a/app/assets/javascripts/vue_merge_request_widget/constants.js b/app/assets/javascripts/vue_merge_request_widget/constants.js index 0a29d55fbd6..3e65bdf0cb0 100644 --- a/app/assets/javascripts/vue_merge_request_widget/constants.js +++ b/app/assets/javascripts/vue_merge_request_widget/constants.js @@ -3,3 +3,13 @@ export const DANGER = 'danger'; export const WARNING_MESSAGE_CLASS = 'warning_message'; export const DANGER_MESSAGE_CLASS = 'danger_message'; + +export const MWPS_MERGE_STRATEGY = 'merge_when_pipeline_succeeds'; +export const ATMTWPS_MERGE_STRATEGY = 'add_to_merge_train_when_pipeline_succeeds'; +export const MT_MERGE_STRATEGY = 'merge_train'; + +export const AUTO_MERGE_STRATEGIES = [ + MWPS_MERGE_STRATEGY, + ATMTWPS_MERGE_STRATEGY, + MT_MERGE_STRATEGY, +]; diff --git a/app/assets/javascripts/vue_merge_request_widget/mixins/auto_merge.js b/app/assets/javascripts/vue_merge_request_widget/mixins/auto_merge.js new file mode 100644 index 00000000000..23e140623cc --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/mixins/auto_merge.js @@ -0,0 +1,15 @@ +import { s__ } from '~/locale'; + +export default { + computed: { + statusTextBeforeAuthor() { + return s__('mrWidget|Set by'); + }, + statusTextAfterAuthor() { + return s__('mrWidget|to be merged automatically when the pipeline succeeds'); + }, + cancelButtonText() { + return s__('mrWidget|Cancel automatic merge'); + }, + }, +}; diff --git a/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js index b2e64506472..116d537c463 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js +++ b/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js @@ -1,3 +1,5 @@ +import { __ } from '~/locale'; + export default { computed: { isMergeButtonDisabled() { @@ -9,5 +11,9 @@ export default { this.mr.preventMerge, ); }, + autoMergeText() { + // MWPS is currently the only auto merge strategy available in CE + return __('Merge when pipeline succeeds'); + }, }, }; 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 d02bb2f341d..41386178a1e 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 @@ -29,7 +29,7 @@ import UnresolvedDiscussionsState from './components/states/unresolved_discussio import PipelineBlockedState from './components/states/mr_widget_pipeline_blocked.vue'; import PipelineFailedState from './components/states/pipeline_failed.vue'; import FailedToMerge from './components/states/mr_widget_failed_to_merge.vue'; -import MergeWhenPipelineSucceedsState from './components/states/mr_widget_merge_when_pipeline_succeeds.vue'; +import MrWidgetAutoMergeEnabled from './components/states/mr_widget_auto_merge_enabled.vue'; import AutoMergeFailed from './components/states/mr_widget_auto_merge_failed.vue'; import CheckingState from './components/states/mr_widget_checking.vue'; import eventHub from './event_hub'; @@ -64,7 +64,7 @@ export default { 'mr-widget-unresolved-discussions': UnresolvedDiscussionsState, 'mr-widget-pipeline-blocked': PipelineBlockedState, 'mr-widget-pipeline-failed': PipelineFailedState, - 'mr-widget-merge-when-pipeline-succeeds': MergeWhenPipelineSucceedsState, + MrWidgetAutoMergeEnabled, 'mr-widget-auto-merge-failed': AutoMergeFailed, 'mr-widget-rebase': RebaseState, SourceBranchRemovalStatus, @@ -117,14 +117,6 @@ export default { this.mr.mergePipelinesEnabled && this.mr.sourceProjectId !== this.mr.targetProjectId, ); }, - showTargetBranchAdvancedError() { - return Boolean( - this.mr.isOpen && - this.mr.pipeline && - this.mr.pipeline.target_sha && - this.mr.pipeline.target_sha !== this.mr.targetBranchSha, - ); - }, mergeError() { return sprintf(s__('mrWidget|Merge failed: %{mergeError}. Please try again.'), { mergeError: this.mr.mergeError, @@ -363,18 +355,6 @@ export default { }} </mr-widget-alert-message> - <mr-widget-alert-message - v-if="showTargetBranchAdvancedError" - type="danger" - :help-path="mr.mergeRequestPipelinesHelpPath" - > - {{ - s__( - 'mrWidget|The target branch has advanced, which invalidates the merge request pipeline. Please update the source branch and retry merging', - ) - }} - </mr-widget-alert-message> - <mr-widget-alert-message v-if="mr.mergeError" type="danger"> {{ mergeError }} </mr-widget-alert-message> 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 32badb0fb08..bfa3e7f4a59 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 @@ -1,7 +1,9 @@ import Timeago from 'timeago.js'; +import _ from 'underscore'; import getStateKey from 'ee_else_ce/vue_merge_request_widget/stores/get_state_key'; import { stateKey } from './state_maps'; import { formatDate } from '../../lib/utils/datetime_utility'; +import { ATMTWPS_MERGE_STRATEGY, MT_MERGE_STRATEGY, MWPS_MERGE_STRATEGY } from '../constants'; export default class MergeRequestStore { constructor(data) { @@ -77,6 +79,10 @@ export default class MergeRequestStore { this.onlyAllowMergeIfPipelineSucceeds = data.only_allow_merge_if_pipeline_succeeds || false; this.autoMergeEnabled = Boolean(data.auto_merge_enabled); this.autoMergeStrategy = data.auto_merge_strategy; + this.availableAutoMergeStrategies = data.available_auto_merge_strategies; + this.preferredAutoMergeStrategy = MergeRequestStore.getPreferredAutoMergeStrategy( + this.availableAutoMergeStrategies, + ); this.mergePath = data.merge_path; this.ffOnlyEnabled = data.ff_only_enabled; this.shouldBeRebased = Boolean(data.should_be_rebased); @@ -104,7 +110,9 @@ export default class MergeRequestStore { this.sourceProjectFullPath = data.source_project_full_path; this.sourceProjectId = data.source_project_id; this.targetProjectId = data.target_project_id; - this.mergePipelinesEnabled = data.merge_pipelines_enabled; + this.mergePipelinesEnabled = Boolean(data.merge_pipelines_enabled); + this.mergeTrainsCount = data.merge_trains_count || 0; + this.mergeTrainIndex = data.merge_train_index; // Cherry-pick and Revert actions related this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false; @@ -204,4 +212,16 @@ export default class MergeRequestStore { return timeagoInstance.format(date); } + + static getPreferredAutoMergeStrategy(availableAutoMergeStrategies) { + if (_.includes(availableAutoMergeStrategies, ATMTWPS_MERGE_STRATEGY)) { + return ATMTWPS_MERGE_STRATEGY; + } else if (_.includes(availableAutoMergeStrategies, MT_MERGE_STRATEGY)) { + return MT_MERGE_STRATEGY; + } else if (_.includes(availableAutoMergeStrategies, MWPS_MERGE_STRATEGY)) { + return MWPS_MERGE_STRATEGY; + } + + return undefined; + } } diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js index 48bc6a867f4..28507bba3e5 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js @@ -13,7 +13,7 @@ const stateToComponentMap = { unresolvedDiscussions: 'mr-widget-unresolved-discussions', pipelineBlocked: 'mr-widget-pipeline-blocked', pipelineFailed: 'mr-widget-pipeline-failed', - autoMergeEnabled: 'mr-widget-merge-when-pipeline-succeeds', + autoMergeEnabled: 'mr-widget-auto-merge-enabled', failedToMerge: 'mr-widget-failed-to-merge', autoMergeFailed: 'mr-widget-auto-merge-failed', shaMismatch: 'sha-mismatch', diff --git a/app/assets/javascripts/vue_shared/components/commit.vue b/app/assets/javascripts/vue_shared/components/commit.vue index 3ba946e6447..a1168fa0f1e 100644 --- a/app/assets/javascripts/vue_shared/components/commit.vue +++ b/app/assets/javascripts/vue_shared/components/commit.vue @@ -1,6 +1,7 @@ <script> import _ from 'underscore'; import { GlTooltipDirective, GlLink } from '@gitlab/ui'; +import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue'; import UserAvatarLink from './user_avatar/user_avatar_link.vue'; import Icon from '../../vue_shared/components/icon.vue'; @@ -12,6 +13,7 @@ export default { UserAvatarLink, Icon, GlLink, + TooltipOnTruncate, }, props: { /** @@ -165,7 +167,7 @@ export default { <gl-link :href="commitUrl" class="commit-sha mr-0"> {{ shortSha }} </gl-link> <div class="commit-title flex-truncate-parent"> - <span v-if="title" class="flex-truncate-child"> + <tooltip-on-truncate v-if="title" class="flex-truncate-child" :title="title"> <user-avatar-link v-if="hasAuthor" :link-href="author.path" @@ -174,8 +176,10 @@ export default { :tooltip-text="author.username" class="avatar-image-container" /> - <gl-link :href="commitUrl" class="commit-row-message cgray"> {{ title }} </gl-link> - </span> + <gl-link :href="commitUrl" class="commit-row-message cgray"> + {{ title }} + </gl-link> + </tooltip-on-truncate> <span v-else> Can't find HEAD commit for this branch </span> </div> </div> diff --git a/app/assets/javascripts/vue_shared/components/pagination/constants.js b/app/assets/javascripts/vue_shared/components/pagination/constants.js new file mode 100644 index 00000000000..c24b142ac7e --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/pagination/constants.js @@ -0,0 +1,9 @@ +import { s__ } from '~/locale'; + +export const PAGINATION_UI_BUTTON_LIMIT = 4; +export const UI_LIMIT = 6; +export const SPREAD = '...'; +export const PREV = s__('Pagination|Prev'); +export const NEXT = s__('Pagination|Next'); +export const FIRST = s__('Pagination|« First'); +export const LAST = s__('Pagination|Last »'); diff --git a/app/assets/javascripts/vue_shared/components/pagination/graphql_pagination.vue b/app/assets/javascripts/vue_shared/components/pagination/graphql_pagination.vue new file mode 100644 index 00000000000..53e473432db --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/pagination/graphql_pagination.vue @@ -0,0 +1,47 @@ +<script> +import { GlButton } from '@gitlab/ui'; +import { PREV, NEXT } from '~/vue_shared/components/pagination/constants'; + +/** + * Pagination Component for graphql API + */ +export default { + name: 'GraphqlPaginationComponent', + components: { + GlButton, + }, + labels: { + prev: PREV, + next: NEXT, + }, + props: { + hasNextPage: { + required: true, + type: Boolean, + }, + hasPreviousPage: { + required: true, + type: Boolean, + }, + }, +}; +</script> +<template> + <div class="justify-content-center d-flex prepend-top-default"> + <div class="btn-group"> + <gl-button + class="js-prev-btn page-link" + :disabled="!hasPreviousPage" + @click="$emit('previousClicked')" + >{{ $options.labels.prev }}</gl-button + > + + <gl-button + class="js-next-btn page-link" + :disabled="!hasNextPage" + @click="$emit('nextClicked')" + >{{ $options.labels.next }}</gl-button + > + </div> + </div> +</template> diff --git a/app/assets/javascripts/vue_shared/components/table_pagination.vue b/app/assets/javascripts/vue_shared/components/table_pagination.vue index 9cce9a4e542..1e2d4ffa7e3 100644 --- a/app/assets/javascripts/vue_shared/components/table_pagination.vue +++ b/app/assets/javascripts/vue_shared/components/table_pagination.vue @@ -1,13 +1,13 @@ <script> -import { s__ } from '../../locale'; - -const PAGINATION_UI_BUTTON_LIMIT = 4; -const UI_LIMIT = 6; -const SPREAD = '...'; -const PREV = s__('Pagination|Prev'); -const NEXT = s__('Pagination|Next'); -const FIRST = s__('Pagination|« First'); -const LAST = s__('Pagination|Last »'); +import { + PAGINATION_UI_BUTTON_LIMIT, + UI_LIMIT, + SPREAD, + PREV, + NEXT, + FIRST, + LAST, +} from '~/vue_shared/components/pagination/constants'; export default { props: { diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 135117926be..9e7e3ed5afb 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -33,7 +33,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo def show close_merge_request_if_no_source_project - @merge_request.check_mergeability + mark_merge_request_mergeable respond_to do |format| format.html do @@ -251,6 +251,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo @merge_request.has_no_commits? && !@merge_request.target_branch_exists? end + def mark_merge_request_mergeable + @merge_request.check_if_can_be_merged + end + def merge! # Disable the CI check if auto_merge_strategy is specified since we have # to wait until CI completes to know @@ -269,9 +273,15 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo @merge_request.update(merge_error: nil, squash: merge_params.fetch(:squash, false)) if auto_merge_requested? - AutoMergeService.new(project, current_user, merge_params) - .execute(merge_request, - params[:auto_merge_strategy] || AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) + if merge_request.auto_merge_enabled? + # TODO: We should have a dedicated endpoint for updating merge params. + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63130. + AutoMergeService.new(project, current_user, merge_params).update(merge_request) + else + AutoMergeService.new(project, current_user, merge_params) + .execute(merge_request, + params[:auto_merge_strategy] || AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) + end else @merge_request.merge_async(current_user.id, merge_params) diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index c40ad39be61..6a4241c94bc 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -73,7 +73,8 @@ module Ci private def ideal_next_run_at - Gitlab::Ci::CronParser.new(cron, cron_timezone).next_time_from(Time.now) + Gitlab::Ci::CronParser.new(cron, cron_timezone) + .next_time_from(Time.zone.now) end end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 4fcaac75655..7481f1939ad 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -725,16 +725,19 @@ class MergeRequest < ApplicationRecord MergeRequests::ReloadDiffsService.new(self, current_user).execute end - - def check_mergeability - MergeRequests::MergeabilityCheckService.new(self).execute - end # rubocop: enable CodeReuse/ServiceClass - # Returns boolean indicating the merge_status should be rechecked in order to - # switch to either can_be_merged or cannot_be_merged. - def recheck_merge_status? - self.class.state_machines[:merge_status].check_state?(merge_status) + def check_if_can_be_merged + return unless self.class.state_machines[:merge_status].check_state?(merge_status) && Gitlab::Database.read_write? + + can_be_merged = + !broken? && project.repository.can_be_merged?(diff_head_sha, target_branch) + + if can_be_merged + mark_as_mergeable + else + mark_as_unmergeable + end end def merge_event @@ -760,7 +763,7 @@ class MergeRequest < ApplicationRecord def mergeable?(skip_ci_check: false) return false unless mergeable_state?(skip_ci_check: skip_ci_check) - check_mergeability + check_if_can_be_merged can_be_merged? && !should_be_rebased? end @@ -775,6 +778,15 @@ class MergeRequest < ApplicationRecord true end + def mergeable_to_ref? + return false unless mergeable_state?(skip_ci_check: true, skip_discussions_check: true) + + # Given the `merge_ref_path` will have the same + # state the `target_branch` would have. Ideally + # we need to check if it can be merged to it. + project.repository.can_be_merged?(diff_head_sha, target_branch) + end + def ff_merge_possible? project.repository.ancestor?(target_branch_sha, diff_head_sha) end @@ -1087,12 +1099,6 @@ class MergeRequest < ApplicationRecord target_project.repository.fetch_source_branch!(source_project.repository, source_branch, ref_path) end - # Returns the current merge-ref HEAD commit. - # - def merge_ref_head - project.repository.commit(merge_ref_path) - end - def ref_path "refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/head" end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 3218c04b219..728a3040227 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -164,6 +164,7 @@ class ProjectPolicy < BasePolicy enable :set_issue_iid enable :set_issue_created_at + enable :set_issue_updated_at enable :set_note_created_at end diff --git a/app/services/auto_merge/base_service.rb b/app/services/auto_merge/base_service.rb index 7f0a41b3dfa..d726085b89a 100644 --- a/app/services/auto_merge/base_service.rb +++ b/app/services/auto_merge/base_service.rb @@ -20,6 +20,14 @@ module AutoMerge strategy.to_sym end + def update(merge_request) + merge_request.merge_params.merge!(params) + + return :failed unless merge_request.save + + strategy.to_sym + end + def cancel(merge_request) if cancel_auto_merge(merge_request) yield if block_given? diff --git a/app/services/auto_merge_service.rb b/app/services/auto_merge_service.rb index a3a780ff388..926d2f5fc66 100644 --- a/app/services/auto_merge_service.rb +++ b/app/services/auto_merge_service.rb @@ -24,6 +24,12 @@ class AutoMergeService < BaseService service.execute(merge_request) end + def update(merge_request) + return :failed unless merge_request.auto_merge_enabled? + + get_service_instance(merge_request.auto_merge_strategy).update(merge_request) + end + def process(merge_request) return unless merge_request.auto_merge_enabled? diff --git a/app/services/ci/pipeline_schedule_service.rb b/app/services/ci/pipeline_schedule_service.rb index 387d0351490..5b5e9a26520 100644 --- a/app/services/ci/pipeline_schedule_service.rb +++ b/app/services/ci/pipeline_schedule_service.rb @@ -7,7 +7,7 @@ module Ci # Otherwise, multiple pipelines could be created in a short interval. schedule.schedule_next_run! - RunPipelineScheduleWorker.perform_async(schedule.id, schedule.owner.id) + RunPipelineScheduleWorker.perform_async(schedule.id, schedule.owner&.id) end end end diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb index 8670b9ccf3d..87147d90c32 100644 --- a/app/services/merge_requests/merge_to_ref_service.rb +++ b/app/services/merge_requests/merge_to_ref_service.rb @@ -20,14 +20,20 @@ module MergeRequests raise_error('Conflicts detected during merge') unless commit_id - success(commit_id: commit_id) - rescue MergeError, ArgumentError => error + commit = project.commit(commit_id) + target_id, source_id = commit.parent_ids + + success(commit_id: commit.id, + target_id: target_id, + source_id: source_id) + rescue MergeError => error error(error.message) end private def validate! + authorization_check! error_check! end @@ -37,13 +43,21 @@ module MergeRequests error = if !hooks_validation_pass?(merge_request) hooks_validation_error(merge_request) - elsif source.blank? + elsif !@merge_request.mergeable_to_ref? + "Merge request is not mergeable to #{target_ref}" + elsif !source 'No source for merge' end raise_error(error) if error end + def authorization_check! + unless Ability.allowed?(current_user, :admin_merge_request, project) + raise_error("You are not allowed to merge to this ref") + end + end + def target_ref merge_request.merge_ref_path end diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb deleted file mode 100644 index ef833774e65..00000000000 --- a/app/services/merge_requests/mergeability_check_service.rb +++ /dev/null @@ -1,82 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class MergeabilityCheckService < ::BaseService - include Gitlab::Utils::StrongMemoize - - delegate :project, to: :@merge_request - delegate :repository, to: :project - - def initialize(merge_request) - @merge_request = merge_request - end - - # Updates the MR merge_status. Whenever it switches to a can_be_merged state, - # the merge-ref is refreshed. - # - # Returns a ServiceResponse indicating merge_status is/became can_be_merged - # and the merge-ref is synced. Success in case of being/becoming mergeable, - # error otherwise. - def execute - return ServiceResponse.error(message: 'Invalid argument') unless merge_request - return ServiceResponse.error(message: 'Unsupported operation') if Gitlab::Database.read_only? - - update_merge_status - - unless merge_request.can_be_merged? - return ServiceResponse.error(message: 'Merge request is not mergeable') - end - - unless payload.fetch(:merge_ref_head) - return ServiceResponse.error(message: 'Merge ref was not found') - end - - ServiceResponse.success(payload: payload) - end - - private - - attr_reader :merge_request - - def payload - strong_memoize(:payload) do - { - merge_ref_head: merge_ref_head_payload - } - end - end - - def merge_ref_head_payload - commit = merge_request.merge_ref_head - - return unless commit - - target_id, source_id = commit.parent_ids - - { - commit_id: commit.id, - source_id: source_id, - target_id: target_id - } - end - - def update_merge_status - return unless merge_request.recheck_merge_status? - - if can_git_merge? - merge_to_ref && merge_request.mark_as_mergeable - else - merge_request.mark_as_unmergeable - end - end - - def can_git_merge? - !merge_request.broken? && repository.can_be_merged?(merge_request.diff_head_sha, merge_request.target_branch) - end - - def merge_to_ref - result = MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request) - result[:status] == :success - end - end -end diff --git a/app/services/service_response.rb b/app/services/service_response.rb index f3437ba16de..1de30e68d87 100644 --- a/app/services/service_response.rb +++ b/app/services/service_response.rb @@ -1,20 +1,19 @@ # frozen_string_literal: true class ServiceResponse - def self.success(message: nil, payload: {}) - new(status: :success, message: message, payload: payload) + def self.success(message: nil) + new(status: :success, message: message) end - def self.error(message:, payload: {}, http_status: nil) - new(status: :error, message: message, payload: payload, http_status: http_status) + def self.error(message:, http_status: nil) + new(status: :error, message: message, http_status: http_status) end - attr_reader :status, :message, :http_status, :payload + attr_reader :status, :message, :http_status - def initialize(status:, message: nil, payload: {}, http_status: nil) + def initialize(status:, message: nil, http_status: nil) self.status = status self.message = message - self.payload = payload self.http_status = http_status end @@ -28,5 +27,5 @@ class ServiceResponse private - attr_writer :status, :message, :http_status, :payload + attr_writer :status, :message, :http_status end |