diff options
44 files changed, 880 insertions, 68 deletions
@@ -429,7 +429,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 1.32.0', require: 'gitaly' +gem 'gitaly-proto', '~> 1.36.0', require: 'gitaly' gem 'grpc', '~> 1.19.0' diff --git a/Gemfile.lock b/Gemfile.lock index 5b648d43137..5099167e03f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -303,7 +303,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (1.32.0) + gitaly-proto (1.36.0) grpc (~> 1.0) github-markup (1.7.0) gitlab-labkit (0.3.0) @@ -1092,7 +1092,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 1.32.0) + gitaly-proto (~> 1.36.0) github-markup (~> 1.7.0) gitlab-labkit (~> 0.3.0) gitlab-markup (~> 1.7.0) diff --git a/app/assets/javascripts/confidential_merge_request/components/dropdown.vue b/app/assets/javascripts/confidential_merge_request/components/dropdown.vue new file mode 100644 index 00000000000..444640980af --- /dev/null +++ b/app/assets/javascripts/confidential_merge_request/components/dropdown.vue @@ -0,0 +1,58 @@ +<script> +import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; +import { __ } from '~/locale'; +import Icon from '~/vue_shared/components/icon.vue'; + +export default { + components: { + GlDropdown, + GlDropdownItem, + Icon, + }, + props: { + projects: { + type: Array, + required: true, + }, + selectedProject: { + type: Object, + required: false, + default: () => ({}), + }, + }, + computed: { + dropdownText() { + if (Object.keys(this.selectedProject).length) { + return this.selectedProject.name; + } + + return __('Select private project'); + }, + }, + methods: { + selectProject(project) { + this.$emit('click', project); + }, + }, +}; +</script> + +<template> + <gl-dropdown toggle-class="d-flex align-items-center w-100" class="w-100"> + <template slot="button-content"> + <span class="str-truncated-100 mr-2"> + <icon name="lock" /> + {{ dropdownText }} + </span> + <icon name="chevron-down" class="ml-auto" /> + </template> + <gl-dropdown-item v-for="project in projects" :key="project.id" @click="selectProject(project)"> + <icon + name="mobile-issue-close" + :class="{ icon: project.id !== selectedProject.id }" + class="js-active-project-check" + /> + <span class="ml-1">{{ project.name }}</span> + </gl-dropdown-item> + </gl-dropdown> +</template> diff --git a/app/assets/javascripts/confidential_merge_request/components/project_form_group.vue b/app/assets/javascripts/confidential_merge_request/components/project_form_group.vue new file mode 100644 index 00000000000..b89729375be --- /dev/null +++ b/app/assets/javascripts/confidential_merge_request/components/project_form_group.vue @@ -0,0 +1,136 @@ +<script> +import { GlLink } from '@gitlab/ui'; +import { __, sprintf } from '../../locale'; +import createFlash from '../../flash'; +import Api from '../../api'; +import state from '../state'; +import Dropdown from './dropdown.vue'; + +export default { + components: { + GlLink, + Dropdown, + }, + props: { + namespacePath: { + type: String, + required: true, + }, + projectPath: { + type: String, + required: true, + }, + newForkPath: { + type: String, + required: true, + }, + helpPagePath: { + type: String, + required: true, + }, + }, + data() { + return { + projects: [], + }; + }, + computed: { + selectedProject() { + return state.selectedProject; + }, + noForkText() { + return sprintf( + __( + 'To protect this issues confidentiality, %{link_start}fork the project%{link_end} and set the forks visiblity to private.', + ), + { link_start: `<a href="${this.newForkPath}" class="help-link">`, link_end: '</a>' }, + false, + ); + }, + }, + mounted() { + this.fetchProjects(); + this.createBtn = document.querySelector('.js-create-target'); + this.warningText = document.querySelector('.js-exposed-info-warning'); + }, + methods: { + selectProject(project) { + if (project) { + Object.assign(state, { + selectedProject: project, + }); + + if (project.namespaceFullPath !== this.namespacePath) { + this.showWarning(); + } + } else if (this.createBtn) { + this.createBtn.setAttribute('disabled', 'disabled'); + } + }, + normalizeProjectData(data) { + return data.map(p => ({ + id: p.id, + name: p.name_with_namespace, + pathWithNamespace: p.path_with_namespace, + namespaceFullpath: p.namespace.full_path, + })); + }, + fetchProjects() { + Api.projectForks(this.projectPath, { + with_merge_requests_enabled: true, + min_access_level: 30, + visibility: 'private', + }) + .then(({ data }) => { + this.projects = this.normalizeProjectData(data); + this.selectProject(this.projects[0]); + }) + .catch(e => { + createFlash(__('Error fetching forked projects. Please try again.')); + throw e; + }); + }, + showWarning() { + if (this.warningText) { + this.warningText.classList.remove('hidden'); + } + + if (this.createBtn) { + this.createBtn.classList.add('btn-warning'); + this.createBtn.classList.remove('btn-success'); + } + }, + }, +}; +</script> + +<template> + <div class="form-group"> + <label>{{ __('Project') }}</label> + <div> + <dropdown + v-if="projects.length" + :projects="projects" + :selected-project="selectedProject" + @click="selectProject" + /> + <p class="text-muted mt-1 mb-0"> + <template v-if="projects.length"> + {{ + __( + 'To protect this issues confidentiality, a private fork of this project was selected.', + ) + }} + </template> + <template v-else> + {{ __('No forks available to you.') }}<br /> + <span v-html="noForkText"></span> + </template> + <gl-link :href="helpPagePath" class="help-link" target="_blank"> + <span class="sr-only">{{ __('Read more') }}</span> + <i class="fa fa-question-circle" aria-hidden="true"></i> + </gl-link> + </p> + </div> + </div> +</template> diff --git a/app/assets/javascripts/confidential_merge_request/index.js b/app/assets/javascripts/confidential_merge_request/index.js new file mode 100644 index 00000000000..9672821d30e --- /dev/null +++ b/app/assets/javascripts/confidential_merge_request/index.js @@ -0,0 +1,30 @@ +import Vue from 'vue'; +import { parseBoolean } from '../lib/utils/common_utils'; +import ProjectFormGroup from './components/project_form_group.vue'; +import state from './state'; + +export function isConfidentialIssue() { + return parseBoolean(document.querySelector('.js-create-mr').dataset.isConfidential); +} + +export function canCreateConfidentialMergeRequest() { + return isConfidentialIssue() && Object.keys(state.selectedProject).length > 0; +} + +export function init() { + const el = document.getElementById('js-forked-project'); + + return new Vue({ + el, + render(h) { + return h(ProjectFormGroup, { + props: { + namespacePath: el.dataset.namespacePath, + projectPath: el.dataset.projectPath, + newForkPath: el.dataset.newForkPath, + helpPagePath: el.dataset.helpPagePath, + }, + }); + }, + }); +} diff --git a/app/assets/javascripts/confidential_merge_request/state.js b/app/assets/javascripts/confidential_merge_request/state.js new file mode 100644 index 00000000000..95b0580f4b9 --- /dev/null +++ b/app/assets/javascripts/confidential_merge_request/state.js @@ -0,0 +1,5 @@ +import Vue from 'vue'; + +export default Vue.observable({ + selectedProject: {}, +}); diff --git a/app/assets/javascripts/create_merge_request_dropdown.js b/app/assets/javascripts/create_merge_request_dropdown.js index 8f5cece0788..052168bb21c 100644 --- a/app/assets/javascripts/create_merge_request_dropdown.js +++ b/app/assets/javascripts/create_merge_request_dropdown.js @@ -5,6 +5,12 @@ import Flash from './flash'; import DropLab from './droplab/drop_lab'; import ISetter from './droplab/plugins/input_setter'; import { __, sprintf } from './locale'; +import { + init as initConfidentialMergeRequest, + isConfidentialIssue, + canCreateConfidentialMergeRequest, +} from './confidential_merge_request'; +import confidentialMergeRequestState from './confidential_merge_request/state'; // Todo: Remove this when fixing issue in input_setter plugin const InputSetter = Object.assign({}, ISetter); @@ -12,6 +18,17 @@ const InputSetter = Object.assign({}, ISetter); const CREATE_MERGE_REQUEST = 'create-mr'; const CREATE_BRANCH = 'create-branch'; +function createEndpoint(projectPath, endpoint) { + if (canCreateConfidentialMergeRequest()) { + return endpoint.replace( + projectPath, + confidentialMergeRequestState.selectedProject.pathWithNamespace, + ); + } + + return endpoint; +} + export default class CreateMergeRequestDropdown { constructor(wrapperEl) { this.wrapperEl = wrapperEl; @@ -42,6 +59,8 @@ export default class CreateMergeRequestDropdown { this.refIsValid = true; this.refsPath = this.wrapperEl.dataset.refsPath; this.suggestedRef = this.refInput.value; + this.projectPath = this.wrapperEl.dataset.projectPath; + this.projectId = this.wrapperEl.dataset.projectId; // These regexps are used to replace // a backend generated new branch name and its source (ref) @@ -58,6 +77,14 @@ export default class CreateMergeRequestDropdown { }; this.init(); + + if (isConfidentialIssue()) { + this.createMergeRequestButton.setAttribute( + 'data-dropdown-trigger', + '#create-merge-request-dropdown', + ); + initConfidentialMergeRequest(); + } } available() { @@ -113,7 +140,9 @@ export default class CreateMergeRequestDropdown { this.isCreatingBranch = true; return axios - .post(this.createBranchPath) + .post(createEndpoint(this.projectPath, this.createBranchPath), { + confidential_issue_project_id: canCreateConfidentialMergeRequest() ? this.projectId : null, + }) .then(({ data }) => { this.branchCreated = true; window.location.href = data.url; @@ -125,7 +154,11 @@ export default class CreateMergeRequestDropdown { this.isCreatingMergeRequest = true; return axios - .post(this.createMrPath) + .post(this.createMrPath, { + target_project_id: canCreateConfidentialMergeRequest() + ? confidentialMergeRequestState.selectedProject.id + : null, + }) .then(({ data }) => { this.mergeRequestCreated = true; window.location.href = data.url; @@ -149,6 +182,8 @@ export default class CreateMergeRequestDropdown { } enable() { + if (!canCreateConfidentialMergeRequest()) return; + this.createMergeRequestButton.classList.remove('disabled'); this.createMergeRequestButton.removeAttribute('disabled'); @@ -205,7 +240,7 @@ export default class CreateMergeRequestDropdown { if (!ref) return false; return axios - .get(`${this.refsPath}${encodeURIComponent(ref)}`) + .get(`${createEndpoint(this.projectPath, this.refsPath)}${encodeURIComponent(ref)}`) .then(({ data }) => { const branches = data[Object.keys(data)[0]]; const tags = data[Object.keys(data)[1]]; @@ -325,6 +360,12 @@ export default class CreateMergeRequestDropdown { let xhr = null; event.preventDefault(); + if (isConfidentialIssue() && !event.target.classList.contains('js-create-target')) { + this.droplab.hooks.forEach(hook => hook.list.toggle()); + + return; + } + if (this.isBusy()) { return; } diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index cd951f67293..a12029d2419 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -287,8 +287,8 @@ list-style: none; padding: 0 1px; - a, - button, + a:not(.help-link), + button:not(.btn), .menu-item { @include dropdown-link; } diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index d77f64a84f5..141a7dfb923 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -169,7 +169,7 @@ class Projects::BranchesController < Projects::ApplicationController end def confidential_issue_project - return unless Feature.enabled?(:create_confidential_merge_request, @project) + return unless helpers.create_confidential_merge_request_enabled? return if params[:confidential_issue_project_id].blank? confidential_issue_project = Project.find(params[:confidential_issue_project_id]) diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index e275b417784..b866f574f67 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -172,7 +172,7 @@ class Projects::IssuesController < Projects::ApplicationController def create_merge_request create_params = params.slice(:branch_name, :ref).merge(issue_iid: issue.iid) - create_params[:target_project_id] = params[:target_project_id] if Feature.enabled?(:create_confidential_merge_request, @project) + create_params[:target_project_id] = params[:target_project_id] if helpers.create_confidential_merge_request_enabled? result = ::MergeRequests::CreateFromIssueService.new(project, current_user, create_params).execute if result[:status] == :success diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index dfadcfc33b2..5476a7cdff6 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -137,7 +137,7 @@ module IssuesHelper end def create_confidential_merge_request_enabled? - Feature.enabled?(:create_confidential_merge_request, @project) + Feature.enabled?(:create_confidential_merge_request, @project, default_enabled: true) end def show_new_branch_button? diff --git a/app/models/repository.rb b/app/models/repository.rb index d087a5a7bbd..a408db7ebbe 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -839,10 +839,10 @@ class Repository end end - def merge_to_ref(user, source_sha, merge_request, target_ref, message) + def merge_to_ref(user, source_sha, merge_request, target_ref, message, first_parent_ref) branch = merge_request.target_branch - raw.merge_to_ref(user, source_sha, branch, target_ref, message) + raw.merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref) end def ff_merge(user, source, target_branch, merge_request: nil) diff --git a/app/services/auto_merge/base_service.rb b/app/services/auto_merge/base_service.rb index d726085b89a..e06659a39cd 100644 --- a/app/services/auto_merge/base_service.rb +++ b/app/services/auto_merge/base_service.rb @@ -29,7 +29,7 @@ module AutoMerge end def cancel(merge_request) - if cancel_auto_merge(merge_request) + if clear_auto_merge_parameters(merge_request) yield if block_given? success @@ -38,6 +38,16 @@ module AutoMerge end end + def abort(merge_request, reason) + if clear_auto_merge_parameters(merge_request) + yield if block_given? + + success + else + error("Can't abort the automatic merge", 406) + end + end + private def strategy @@ -46,7 +56,7 @@ module AutoMerge end end - def cancel_auto_merge(merge_request) + def clear_auto_merge_parameters(merge_request) merge_request.auto_merge_enabled = false merge_request.merge_user = nil diff --git a/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb b/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb index cde8c19e8fc..6a33ec071db 100644 --- a/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb +++ b/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb @@ -19,7 +19,13 @@ module AutoMerge def cancel(merge_request) super do - SystemNoteService.cancel_merge_when_pipeline_succeeds(merge_request, @project, @current_user) + SystemNoteService.cancel_merge_when_pipeline_succeeds(merge_request, project, current_user) + end + end + + def abort(merge_request, reason) + super do + SystemNoteService.abort_merge_when_pipeline_succeeds(merge_request, project, current_user, reason) end end diff --git a/app/services/auto_merge_service.rb b/app/services/auto_merge_service.rb index 926d2f5fc66..95bf2db2018 100644 --- a/app/services/auto_merge_service.rb +++ b/app/services/auto_merge_service.rb @@ -42,6 +42,12 @@ class AutoMergeService < BaseService get_service_instance(merge_request.auto_merge_strategy).cancel(merge_request) end + def abort(merge_request, reason) + return error("Can't abort the automatic merge", 406) unless merge_request.auto_merge_enabled? + + get_service_instance(merge_request.auto_merge_strategy).abort(merge_request, reason) + end + def available_strategies(merge_request) self.class.all_strategies.select do |strategy| get_service_instance(strategy).available_for?(merge_request) diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index c34fbeb2adb..067510a8a0a 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -68,8 +68,8 @@ module MergeRequests !merge_request.for_fork? end - def cancel_auto_merge(merge_request) - AutoMergeService.new(project, current_user).cancel(merge_request) + def abort_auto_merge(merge_request, reason) + AutoMergeService.new(project, current_user).abort(merge_request, reason) end # Returns all origin and fork merge requests from `@project` satisfying passed arguments. diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index b81a4dd81d2..c2174d2a130 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -18,7 +18,7 @@ module MergeRequests invalidate_cache_counts(merge_request, users: merge_request.assignees) merge_request.update_project_counter_caches cleanup_environments(merge_request) - cancel_auto_merge(merge_request) + abort_auto_merge(merge_request, 'merge request was closed') end merge_request diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb index efe4dcd6255..0ea50a5dbf5 100644 --- a/app/services/merge_requests/merge_to_ref_service.rb +++ b/app/services/merge_requests/merge_to_ref_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module MergeRequests - # Performs the merge between source SHA and the target branch. Instead + # Performs the merge between source SHA and the target branch or the specified first parent ref. Instead # of writing the result to the MR target branch, it targets the `target_ref`. # # Ideally this should leave the `target_ref` state with the same state the @@ -56,12 +56,22 @@ module MergeRequests raise_error(error) if error end + ## + # The parameter `target_ref` is where the merge result will be written. + # Default is the merge ref i.e. `refs/merge-requests/:iid/merge`. def target_ref - merge_request.merge_ref_path + params[:target_ref] || merge_request.merge_ref_path + end + + ## + # The parameter `first_parent_ref` is the main line of the merge commit. + # Default is the target branch ref of the merge request. + def first_parent_ref + params[:first_parent_ref] || merge_request.target_branch_ref end def commit - repository.merge_to_ref(current_user, source, merge_request, target_ref, commit_message) + repository.merge_to_ref(current_user, source, merge_request, target_ref, commit_message, first_parent_ref) rescue Gitlab::Git::PreReceiveError => error raise MergeError, error.message end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 4b199bd8fa8..8961d2e1023 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -24,7 +24,7 @@ module MergeRequests reload_merge_requests outdate_suggestions refresh_pipelines_on_merge_requests - cancel_auto_merges + abort_auto_merges mark_pending_todos_done cache_merge_requests_closing_issues @@ -142,9 +142,9 @@ module MergeRequests end end - def cancel_auto_merges + def abort_auto_merges merge_requests_for_source_branch.each do |merge_request| - cancel_auto_merge(merge_request) + abort_auto_merge(merge_request, 'source branch was updated') end end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 0066cd0491f..d361e96babf 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -44,7 +44,7 @@ module MergeRequests merge_request.previous_changes['target_branch'].first, merge_request.target_branch) - cancel_auto_merge(merge_request) + abort_auto_merge(merge_request, 'target branch was changed') end if merge_request.assignees != old_assignees diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 1d02d7ed787..e4564bc9b00 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -234,6 +234,16 @@ module SystemNoteService create_note(NoteSummary.new(noteable, project, author, body, action: 'merge')) end + # Called when 'merge when pipeline succeeds' is aborted + def abort_merge_when_pipeline_succeeds(noteable, project, author, reason) + body = "aborted the automatic merge because #{reason}" + + ## + # TODO: Abort message should be sent by the system, not a particular user. + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63187. + create_note(NoteSummary.new(noteable, project, author, body, action: 'merge')) + end + def handle_merge_request_wip(noteable, project, author) prefix = noteable.work_in_progress? ? "marked" : "unmarked" diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 4ebfaff0860..d16e2dddbe0 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -92,21 +92,21 @@ .col-lg-8 .form-group %h5= s_('Preferences|Time format') - .checkbox-icon-inline-wrapper.form-check + .checkbox-icon-inline-wrapper - time_format_label = capture do = s_('Preferences|Display time in 24-hour format') - = f.check_box :time_format_in_24h, class: 'form-check-input' + = f.check_box :time_format_in_24h = f.label :time_format_in_24h do = time_format_label %h5= s_('Preferences|Time display') - .checkbox-icon-inline-wrapper.form-check + .checkbox-icon-inline-wrapper - time_display_label = capture do = s_('Preferences|Use relative times') - = f.check_box :time_display_relative, class: 'form-check-input' + = f.check_box :time_display_relative = f.label :time_display_relative do = time_display_label - .text-muted - = s_('Preferences|For example: 30 mins ago.') + .form-text.text-muted + = s_('Preferences|For example: 30 mins ago.') .col-lg-4.profile-settings-sidebar .col-lg-8 .form-group diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml index 52bb797b5b3..8d3e54dc455 100644 --- a/app/views/projects/issues/_new_branch.html.haml +++ b/app/views/projects/issues/_new_branch.html.haml @@ -3,13 +3,14 @@ - data_action = can_create_merge_request ? 'create-mr' : 'create-branch' - value = can_create_merge_request ? 'Create merge request' : 'Create branch' - value = can_create_confidential_merge_request? ? _('Create confidential merge request') : value + - create_mr_text = can_create_confidential_merge_request? ? _('Create confidential merge request') : _('Create merge request') - can_create_path = can_create_branch_project_issue_path(@project, @issue) - create_mr_path = create_merge_request_project_issue_path(@project, @issue, branch_name: @issue.to_branch_name, ref: @project.default_branch) - create_branch_path = project_branches_path(@project, branch_name: @issue.to_branch_name, ref: @project.default_branch, issue_iid: @issue.iid) - refs_path = refs_namespace_project_path(@project.namespace, @project, search: '') - .create-mr-dropdown-wrap.d-inline-block.full-width-mobile{ data: { can_create_path: can_create_path, create_mr_path: create_mr_path, create_branch_path: create_branch_path, refs_path: refs_path } } + .create-mr-dropdown-wrap.d-inline-block.full-width-mobile.js-create-mr{ data: { project_path: @project.full_path, project_id: @project.id, can_create_path: can_create_path, create_mr_path: create_mr_path, create_branch_path: create_branch_path, refs_path: refs_path, is_confidential: can_create_confidential_merge_request?.to_s } } .btn-group.btn-group-sm.unavailable %button.btn.btn-grouped{ type: 'button', disabled: 'disabled' } = icon('spinner', class: 'fa-spin') @@ -26,7 +27,7 @@ .droplab-dropdown %ul#create-merge-request-dropdown.create-merge-request-dropdown-menu.dropdown-menu.dropdown-menu-right.gl-show-field-errors{ class: ("create-confidential-merge-request-dropdown-menu" if can_create_confidential_merge_request?), data: { dropdown: true } } - if can_create_merge_request - %li.droplab-item-selected{ role: 'button', data: { value: 'create-mr', text: _('Create merge request') } } + %li.droplab-item-selected{ role: 'button', data: { value: 'create-mr', text: create_mr_text } } .menu-item = icon('check', class: 'icon') - if can_create_confidential_merge_request? @@ -41,6 +42,8 @@ %li.divider.droplab-item-ignore %li.droplab-item-ignore.prepend-left-8.append-right-8.prepend-top-16 + - if can_create_confidential_merge_request? + #js-forked-project{ data: { namespace_path: @project.namespace.full_path, project_path: @project.full_path, new_fork_path: new_project_fork_path(@project), help_page_path: help_page_path('user/project/merge_requests') } } .form-group %label{ for: 'new-branch-name' } = _('Branch name') @@ -55,4 +58,8 @@ .form-group %button.btn.btn-success.js-create-target{ type: 'button', data: { action: 'create-mr' } } - = _('Create merge request') + = create_mr_text + + - if can_create_confidential_merge_request? + %p.text-warning.js-exposed-info-warning.hidden + = _('This may expose confidential information as the selected fork is in another namespace that can have other members.') diff --git a/changelogs/unreleased/12553-preferences.yml b/changelogs/unreleased/12553-preferences.yml new file mode 100644 index 00000000000..c41a8c98e4e --- /dev/null +++ b/changelogs/unreleased/12553-preferences.yml @@ -0,0 +1,5 @@ +--- +title: Removes EE diff for app/views/profiles/preferences/show.html.haml +merge_request: +author: +type: other diff --git a/changelogs/unreleased/create-merge-train-ref-ce.yml b/changelogs/unreleased/create-merge-train-ref-ce.yml new file mode 100644 index 00000000000..b0b95275f58 --- /dev/null +++ b/changelogs/unreleased/create-merge-train-ref-ce.yml @@ -0,0 +1,5 @@ +--- +title: Extend `MergeToRefService` to create merge ref from an arbitrary ref +merge_request: 30361 +author: +type: added diff --git a/db/fixtures/development/17_cycle_analytics.rb b/db/fixtures/development/17_cycle_analytics.rb index 7a86fe2eb7c..78ceb74da65 100644 --- a/db/fixtures/development/17_cycle_analytics.rb +++ b/db/fixtures/development/17_cycle_analytics.rb @@ -146,11 +146,13 @@ class Gitlab::Seeder::CycleAnalytics commit_sha = issue.project.repository.create_file(@user, filename, "content", message: "Commit for #{issue.to_reference}", branch_name: branch_name) issue.project.repository.commit(commit_sha) - GitPushService.new(issue.project, - @user, - oldrev: issue.project.repository.commit("master").sha, - newrev: commit_sha, - ref: 'refs/heads/master').execute + Git::BranchPushService.new( + issue.project, + @user, + oldrev: issue.project.repository.commit("master").sha, + newrev: commit_sha, + ref: 'refs/heads/master' + ).execute branch_name end diff --git a/db/migrate/20190703130053_remove_gitaly_feature_flags.rb b/db/migrate/20190703130053_remove_gitaly_feature_flags.rb new file mode 100644 index 00000000000..13ac10a5e21 --- /dev/null +++ b/db/migrate/20190703130053_remove_gitaly_feature_flags.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +class RemoveGitalyFeatureFlags < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + FEATURES = %w[ + gitaly_batch_lfs_pointers gitaly_blame gitaly_blob_get_all_lfs_pointers gitaly_blob_get_new_lfs_pointers + gitaly_branch_names gitaly_branch_names_contains_sha gitaly_branches gitaly_bundle_to_disk + gitaly_calculate_checksum gitaly_can_be_merged gitaly_cherry_pick gitaly_commit_count + gitaly_commit_deltas gitaly_commit_languages gitaly_commit_messages gitaly_commit_patch + gitaly_commit_raw_diffs gitaly_commit_stats gitaly_commit_tree_entry gitaly_commits_between + gitaly_commits_by_message gitaly_conflicts_list_conflict_files gitaly_conflicts_resolve_conflicts gitaly_count_commits + gitaly_count_diverging_commits_no_max gitaly_create_branch gitaly_create_repo_from_bundle gitaly_create_repository + gitaly_delete_branch gitaly_delete_refs gitaly_delta_islands gitaly_deny_disk_acces + gitaly_diff_between gitaly_extract_commit_signature gitaly_fetch_ref gitaly_fetch_remote + gitaly_fetch_source_branch gitaly_filter_shas_with_signature gitaly_filter_shas_with_signatures gitaly_find_all_commits + gitaly_find_branch gitaly_find_commit gitaly_find_commits gitaly_find_ref_name + gitaly_force_push gitaly_fork_repository gitaly_garbage_collect gitaly_get_info_attributes + gitaly_git_blob_load_all_data gitaly_git_blob_raw gitaly_git_fsck gitaly_go-find-all-tags + gitaly_has_local_branches gitaly_import_repository gitaly_is_ancestor gitaly_last_commit_for_path + gitaly_license_short_name gitaly_list_blobs_by_sha_path gitaly_list_commits_by_oid gitaly_local_branches + gitaly_ls_files gitaly_merge_base gitaly_merged_branch_names gitaly_new_commits + gitaly_operation_user_add_tag gitaly_operation_user_commit_file gitaly_operation_user_commit_files gitaly_operation_user_create_branch + gitaly_operation_user_delete_branch gitaly_operation_user_delete_tag gitaly_operation_user_ff_branch gitaly_operation_user_merge_branch + gitaly_post_receive_pack gitaly_post_upload_pack gitaly_project_raw_show gitaly_raw_changes_between + gitaly_rebase gitaly_rebase_in_progress gitaly_ref_delete_refs gitaly_ref_exists + gitaly_ref_exists_branch gitaly_ref_exists_branches gitaly_ref_find_all_remote_branches gitaly_remote_add_remote + gitaly_remote_fetch_internal_remote gitaly_remote_remove_remote gitaly_remote_update_remote_mirror gitaly_remove_namespace + gitaly_repack_full gitaly_repack_incremental gitaly_repository_cleanup gitaly_repository_exists + gitaly_repository_size gitaly_root_ref gitaly_search_files_by_content gitaly_search_files_by_name + gitaly_squash gitaly_squash_in_progress gitaly_ssh_receive_pack gitaly_ssh_upload_pack + gitaly_submodule_url_for gitaly_tag_messages gitaly_tag_names gitaly_tag_names_contains_sha + gitaly_tags gitaly_tree_entries gitaly_wiki_delete_page gitaly_wiki_find_file + gitaly_wiki_find_page gitaly_wiki_get_all_pages gitaly_wiki_page_formatted_data gitaly_wiki_page_versions + gitaly_wiki_update_page gitaly_wiki_write_page gitaly_workhorse_archive gitaly_workhorse_raw_show + gitaly_workhorse_send_git_diff gitaly_workhorse_send_git_patch gitaly_write_config gitaly_write_ref + ] + + class Feature < ActiveRecord::Base + self.table_name = 'features' + end + + def up + Feature.where(key: FEATURES).delete_all + end +end diff --git a/db/schema.rb b/db/schema.rb index 56d49815166..9a8b64689bd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190628185004) do +ActiveRecord::Schema.define(version: 20190703130053) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 19b6aab1c4f..060a29be782 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -536,9 +536,9 @@ module Gitlab tags.find { |tag| tag.name == name } end - def merge_to_ref(user, source_sha, branch, target_ref, message) + def merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref) wrapped_gitaly_errors do - gitaly_operation_client.user_merge_to_ref(user, source_sha, branch, target_ref, message) + gitaly_operation_client.user_merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref) end end diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index b42e6cbad8d..783c2ff0915 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -100,14 +100,15 @@ module Gitlab end end - def user_merge_to_ref(user, source_sha, branch, target_ref, message) + def user_merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref) request = Gitaly::UserMergeToRefRequest.new( repository: @gitaly_repo, source_sha: source_sha, branch: encode_binary(branch), target_ref: encode_binary(target_ref), user: Gitlab::Git::User.from_gitlab(user).to_gitaly, - message: encode_binary(message) + message: encode_binary(message), + first_parent_ref: encode_binary(first_parent_ref) ) response = GitalyClient.call(@repository.storage, :operation_service, :user_merge_to_ref, request) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 01bf4949213..78bf48d66fc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4204,6 +4204,9 @@ msgstr "" msgid "Error fetching diverging counts for branches. Please try again." msgstr "" +msgid "Error fetching forked projects. Please try again." +msgstr "" + msgid "Error fetching labels." msgstr "" @@ -6869,6 +6872,9 @@ msgstr "" msgid "No files found." msgstr "" +msgid "No forks available to you." +msgstr "" + msgid "No job trace" msgstr "" @@ -9242,6 +9248,9 @@ msgstr "" msgid "Select members to invite" msgstr "" +msgid "Select private project" +msgstr "" + msgid "Select project" msgstr "" @@ -10815,6 +10824,9 @@ msgstr "" msgid "This job will automatically run after its timer finishes. Often they are used for incremental roll-out deploys to production environments. When unscheduled it converts into a manual action." msgstr "" +msgid "This may expose confidential information as the selected fork is in another namespace that can have other members." +msgstr "" + msgid "This means you can not push code until you create an empty repository or import existing one." msgstr "" @@ -11155,6 +11167,12 @@ msgstr "" msgid "To preserve performance only <strong>%{display_size} of %{real_size}</strong> files are displayed." msgstr "" +msgid "To protect this issues confidentiality, %{link_start}fork the project%{link_end} and set the forks visiblity to private." +msgstr "" + +msgid "To protect this issues confidentiality, a private fork of this project was selected." +msgstr "" + msgid "To see all the user's personal access tokens you must impersonate them first." msgstr "" diff --git a/spec/features/issues/user_creates_confidential_merge_request_spec.rb b/spec/features/issues/user_creates_confidential_merge_request_spec.rb new file mode 100644 index 00000000000..7ae4af4667b --- /dev/null +++ b/spec/features/issues/user_creates_confidential_merge_request_spec.rb @@ -0,0 +1,54 @@ +require 'rails_helper' + +describe 'User creates confidential merge request on issue page', :js do + include ProjectForksHelper + + let(:user) { create(:user) } + let(:project) { create(:project, :repository, :public) } + let(:issue) { create(:issue, project: project, confidential: true) } + + def visit_confidential_issue + sign_in(user) + visit project_issue_path(project, issue) + wait_for_requests + end + + before do + project.add_developer(user) + end + + context 'user has no private fork' do + before do + fork_project(project, user, repository: true) + visit_confidential_issue + end + + it 'shows that user has no fork available' do + click_button 'Create confidential merge request' + + page.within '.create-confidential-merge-request-dropdown-menu' do + expect(page).to have_content('No forks available to you') + end + end + end + + describe 'user has private fork' do + let(:forked_project) { fork_project(project, user, repository: true) } + + before do + forked_project.update(visibility: Gitlab::VisibilityLevel::PRIVATE) + visit_confidential_issue + end + + it 'create merge request in fork' do + click_button 'Create confidential merge request' + + page.within '.create-confidential-merge-request-dropdown-menu' do + expect(page).to have_button(forked_project.name_with_namespace) + click_button 'Create confidential merge request' + end + + expect(page).to have_content(forked_project.namespace.name) + end + end +end diff --git a/spec/frontend/confidential_merge_request/components/__snapshots__/project_form_group_spec.js.snap b/spec/frontend/confidential_merge_request/components/__snapshots__/project_form_group_spec.js.snap new file mode 100644 index 00000000000..a241c764df7 --- /dev/null +++ b/spec/frontend/confidential_merge_request/components/__snapshots__/project_form_group_spec.js.snap @@ -0,0 +1,101 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Confidential merge request project form group component renders empty state when response is empty 1`] = ` +<div + class="form-group" +> + <label> + Project + </label> + + <div> + <!----> + + <p + class="text-muted mt-1 mb-0" + > + + No forks available to you. + <br /> + + <span> + To protect this issues confidentiality, + <a + class="help-link" + href="https://test.com" + > + fork the project + </a> + and set the forks visiblity to private. + </span> + + <gllink-stub + class="help-link" + href="/help" + target="_blank" + > + <span + class="sr-only" + > + Read more + </span> + + <i + aria-hidden="true" + class="fa fa-question-circle" + /> + </gllink-stub> + </p> + </div> +</div> +`; + +exports[`Confidential merge request project form group component renders fork dropdown 1`] = ` +<div + class="form-group" +> + <label> + Project + </label> + + <div> + <!----> + + <p + class="text-muted mt-1 mb-0" + > + + No forks available to you. + <br /> + + <span> + To protect this issues confidentiality, + <a + class="help-link" + href="https://test.com" + > + fork the project + </a> + and set the forks visiblity to private. + </span> + + <gllink-stub + class="help-link" + href="/help" + target="_blank" + > + <span + class="sr-only" + > + Read more + </span> + + <i + aria-hidden="true" + class="fa fa-question-circle" + /> + </gllink-stub> + </p> + </div> +</div> +`; diff --git a/spec/frontend/confidential_merge_request/components/dropdown_spec.js b/spec/frontend/confidential_merge_request/components/dropdown_spec.js new file mode 100644 index 00000000000..69495f3c161 --- /dev/null +++ b/spec/frontend/confidential_merge_request/components/dropdown_spec.js @@ -0,0 +1,56 @@ +import { mount } from '@vue/test-utils'; +import { GlDropdownItem } from '@gitlab/ui'; +import Dropdown from '~/confidential_merge_request/components/dropdown.vue'; + +let vm; + +function factory(projects = []) { + vm = mount(Dropdown, { + propsData: { + projects, + selectedProject: projects[0], + }, + }); +} + +describe('Confidential merge request project dropdown component', () => { + afterEach(() => { + vm.destroy(); + }); + + it('renders dropdown items', () => { + factory([ + { + id: 1, + name: 'test', + }, + { + id: 2, + name: 'test', + }, + ]); + + expect(vm.findAll(GlDropdownItem).length).toBe(2); + }); + + it('renders selected project icon', () => { + factory([ + { + id: 1, + name: 'test', + }, + { + id: 2, + name: 'test 2', + }, + ]); + + expect(vm.find('.js-active-project-check').classes()).not.toContain('icon'); + expect( + vm + .findAll('.js-active-project-check') + .at(1) + .classes(), + ).toContain('icon'); + }); +}); diff --git a/spec/frontend/confidential_merge_request/components/project_form_group_spec.js b/spec/frontend/confidential_merge_request/components/project_form_group_spec.js new file mode 100644 index 00000000000..3001363f7b9 --- /dev/null +++ b/spec/frontend/confidential_merge_request/components/project_form_group_spec.js @@ -0,0 +1,77 @@ +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; +import ProjectFormGroup from '~/confidential_merge_request/components/project_form_group.vue'; + +const localVue = createLocalVue(); +const mockData = [ + { + id: 1, + name_with_namespace: 'root / gitlab-ce', + path_with_namespace: 'root/gitlab-ce', + namespace: { + full_path: 'root', + }, + }, + { + id: 2, + name_with_namespace: 'test / gitlab-ce', + path_with_namespace: 'test/gitlab-ce', + namespace: { + full_path: 'test', + }, + }, +]; +let vm; +let mock; + +function factory(projects = mockData) { + mock = new MockAdapter(axios); + mock.onGet(/api\/(.*)\/projects\/gitlab-org%2Fgitlab-ce\/forks/).reply(200, projects); + + vm = shallowMount(ProjectFormGroup, { + localVue, + propsData: { + namespacePath: 'gitlab-org', + projectPath: 'gitlab-org/gitlab-ce', + newForkPath: 'https://test.com', + helpPagePath: '/help', + }, + }); +} + +describe('Confidential merge request project form group component', () => { + afterEach(() => { + mock.restore(); + vm.destroy(); + }); + + it('renders fork dropdown', () => { + factory(); + + return localVue.nextTick(() => { + expect(vm.element).toMatchSnapshot(); + }); + }); + + it('sets selected project as first fork', () => { + factory(); + + return localVue.nextTick(() => { + expect(vm.vm.selectedProject).toEqual({ + id: 1, + name: 'root / gitlab-ce', + pathWithNamespace: 'root/gitlab-ce', + namespaceFullpath: 'root', + }); + }); + }); + + it('renders empty state when response is empty', () => { + factory([]); + + return localVue.nextTick(() => { + expect(vm.element).toMatchSnapshot(); + }); + }); +}); diff --git a/spec/javascripts/create_merge_request_dropdown_spec.js b/spec/frontend/create_merge_request_dropdown_spec.js index 00fe3f451f5..6e41fdabdce 100644 --- a/spec/javascripts/create_merge_request_dropdown_spec.js +++ b/spec/frontend/create_merge_request_dropdown_spec.js @@ -1,7 +1,7 @@ import axios from '~/lib/utils/axios_utils'; import MockAdapter from 'axios-mock-adapter'; import CreateMergeRequestDropdown from '~/create_merge_request_dropdown'; -import { TEST_HOST } from 'spec/test_constants'; +import { TEST_HOST } from './helpers/test_constants'; describe('CreateMergeRequestDropdown', () => { let axiosMock; @@ -10,7 +10,7 @@ describe('CreateMergeRequestDropdown', () => { beforeEach(() => { axiosMock = new MockAdapter(axios); - setFixtures(` + document.body.innerHTML = ` <div id="dummy-wrapper-element"> <div class="available"></div> <div class="unavailable"> @@ -18,11 +18,12 @@ describe('CreateMergeRequestDropdown', () => { <div class="text"></div> </div> <div class="js-ref"></div> + <div class="js-create-mr"></div> <div class="js-create-merge-request"></div> <div class="js-create-target"></div> <div class="js-dropdown-toggle"></div> </div> - `); + `; const dummyElement = document.getElementById('dummy-wrapper-element'); dropdown = new CreateMergeRequestDropdown(dummyElement); @@ -36,7 +37,7 @@ describe('CreateMergeRequestDropdown', () => { describe('getRef', () => { it('escapes branch names correctly', done => { const endpoint = `${dropdown.refsPath}contains%23hash`; - spyOn(axios, 'get').and.callThrough(); + jest.spyOn(axios, 'get'); axiosMock.onGet(endpoint).replyOnce({}); dropdown diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index cceeae8afe6..a28b95e5bff 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1694,14 +1694,15 @@ describe Gitlab::Git::Repository, :seed_helper do let(:branch_head) { '6d394385cf567f80a8fd85055db1ab4c5295806f' } let(:left_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } let(:right_branch) { 'test-master' } + let(:first_parent_ref) { 'refs/heads/test-master' } let(:target_ref) { 'refs/merge-requests/999/merge' } before do - repository.create_branch(right_branch, branch_head) unless repository.branch_exists?(right_branch) + repository.create_branch(right_branch, branch_head) unless repository.ref_exists?(first_parent_ref) end def merge_to_ref - repository.merge_to_ref(user, left_sha, right_branch, target_ref, 'Merge message') + repository.merge_to_ref(user, left_sha, right_branch, target_ref, 'Merge message', first_parent_ref) end it 'generates a commit in the target_ref' do @@ -1716,7 +1717,7 @@ describe Gitlab::Git::Repository, :seed_helper do end it 'does not change the right branch HEAD' do - expect { merge_to_ref }.not_to change { repository.find_branch(right_branch).target } + expect { merge_to_ref }.not_to change { repository.commit(first_parent_ref).sha } end end diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index 18663a72fcd..f38b8d31237 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -79,13 +79,13 @@ describe Gitlab::GitalyClient::OperationService do end describe '#user_merge_to_ref' do - let(:branch) { 'my-branch' } + let(:first_parent_ref) { 'refs/heads/my-branch' } let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } let(:ref) { 'refs/merge-requests/x/merge' } let(:message) { 'validación' } let(:response) { Gitaly::UserMergeToRefResponse.new(commit_id: 'new-commit-id') } - subject { client.user_merge_to_ref(user, source_sha, branch, ref, message) } + subject { client.user_merge_to_ref(user, source_sha, nil, ref, message, first_parent_ref) } it 'sends a user_merge_to_ref message' do expect_any_instance_of(Gitaly::OperationService::Stub) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 13da7bd7407..3d967aa4ab8 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1420,12 +1420,13 @@ describe Repository do source_project: project) end - it 'writes merge of source and target to MR merge_ref_path' do + it 'writes merge of source SHA and first parent ref to MR merge_ref_path' do merge_commit_id = repository.merge_to_ref(user, merge_request.diff_head_sha, merge_request, merge_request.merge_ref_path, - 'Custom message') + 'Custom message', + merge_request.target_branch_ref) merge_commit = repository.commit(merge_commit_id) diff --git a/spec/services/auto_merge/base_service_spec.rb b/spec/services/auto_merge/base_service_spec.rb index 24cb63a0d61..a409f21a7e4 100644 --- a/spec/services/auto_merge/base_service_spec.rb +++ b/spec/services/auto_merge/base_service_spec.rb @@ -121,11 +121,7 @@ describe AutoMerge::BaseService do end end - describe '#cancel' do - subject { service.cancel(merge_request) } - - let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } - + shared_examples_for 'Canceled or Dropped' do it 'removes properies from the merge request' do subject @@ -173,6 +169,20 @@ describe AutoMerge::BaseService do it 'does not yield block' do expect { |b| service.execute(merge_request, &b) }.not_to yield_control end + end + end + + describe '#cancel' do + subject { service.cancel(merge_request) } + + let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + + it_behaves_like 'Canceled or Dropped' + + context 'when failed to save' do + before do + allow(merge_request).to receive(:save) { false } + end it 'returns error status' do expect(subject[:status]).to eq(:error) @@ -180,4 +190,24 @@ describe AutoMerge::BaseService do end end end + + describe '#abort' do + subject { service.abort(merge_request, reason) } + + let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + let(:reason) { 'an error'} + + it_behaves_like 'Canceled or Dropped' + + context 'when failed to save' do + before do + allow(merge_request).to receive(:save) { false } + end + + it 'returns error status' do + expect(subject[:status]).to eq(:error) + expect(subject[:message]).to eq("Can't abort the automatic merge") + end + end + end end diff --git a/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb index 5e84ef052ce..931b52470c4 100644 --- a/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb +++ b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb @@ -177,6 +177,17 @@ describe AutoMerge::MergeWhenPipelineSucceedsService do end end + describe "#abort" do + before do + service.abort(mr_merge_if_green_enabled, 'an error') + end + + it 'posts a system note' do + note = mr_merge_if_green_enabled.notes.last + expect(note.note).to include 'aborted the automatic merge' + end + end + describe 'pipeline integration' do context 'when there are multiple stages in the pipeline' do let(:ref) { mr_merge_if_green_enabled.source_branch } diff --git a/spec/services/auto_merge_service_spec.rb b/spec/services/auto_merge_service_spec.rb index 93a22e60498..50dfc49a59c 100644 --- a/spec/services/auto_merge_service_spec.rb +++ b/spec/services/auto_merge_service_spec.rb @@ -161,4 +161,29 @@ describe AutoMergeService do end end end + + describe '#abort' do + subject { service.abort(merge_request, error) } + + let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + let(:error) { 'an error' } + + it 'delegates to a relevant service instance' do + expect_next_instance_of(AutoMerge::MergeWhenPipelineSucceedsService) do |service| + expect(service).to receive(:abort).with(merge_request, error) + end + + subject + end + + context 'when auto merge is not enabled' do + let(:merge_request) { create(:merge_request) } + + it 'returns error' do + expect(subject[:message]).to eq("Can't abort the automatic merge") + expect(subject[:status]).to eq(:error) + expect(subject[:http_status]).to eq(406) + end + end + end end diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb index 61f99f82a76..e2f201677fa 100644 --- a/spec/services/merge_requests/merge_to_ref_service_spec.rb +++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb @@ -22,7 +22,6 @@ describe MergeRequests::MergeToRefService do shared_examples_for 'successfully merges to ref with merge method' do it 'writes commit to merge ref' do repository = project.repository - target_ref = merge_request.merge_ref_path expect(repository.ref_exists?(target_ref)).to be(false) @@ -33,7 +32,7 @@ describe MergeRequests::MergeToRefService do expect(result[:status]).to eq(:success) expect(result[:commit_id]).to be_present expect(result[:source_id]).to eq(merge_request.source_branch_sha) - expect(result[:target_id]).to eq(merge_request.target_branch_sha) + expect(result[:target_id]).to eq(repository.commit(first_parent_ref).sha) expect(repository.ref_exists?(target_ref)).to be(true) expect(ref_head.id).to eq(result[:commit_id]) end @@ -74,17 +73,22 @@ describe MergeRequests::MergeToRefService do describe '#execute' do let(:service) do - described_class.new(project, user, commit_message: 'Awesome message', - should_remove_source_branch: true) + described_class.new(project, user, **params) end + let(:params) { { commit_message: 'Awesome message', should_remove_source_branch: true } } + def process_merge_to_ref perform_enqueued_jobs do service.execute(merge_request) end end - it_behaves_like 'successfully merges to ref with merge method' + it_behaves_like 'successfully merges to ref with merge method' do + let(:first_parent_ref) { 'refs/heads/master' } + let(:target_ref) { merge_request.merge_ref_path } + end + it_behaves_like 'successfully evaluates pre-condition checks' context 'commit history comparison with regular MergeService' do @@ -129,14 +133,22 @@ describe MergeRequests::MergeToRefService do context 'when semi-linear merge method' do let(:merge_method) { :rebase_merge } - it_behaves_like 'successfully merges to ref with merge method' + it_behaves_like 'successfully merges to ref with merge method' do + let(:first_parent_ref) { 'refs/heads/master' } + let(:target_ref) { merge_request.merge_ref_path } + end + it_behaves_like 'successfully evaluates pre-condition checks' end context 'when fast-forward merge method' do let(:merge_method) { :ff } - it_behaves_like 'successfully merges to ref with merge method' + it_behaves_like 'successfully merges to ref with merge method' do + let(:first_parent_ref) { 'refs/heads/master' } + let(:target_ref) { merge_request.merge_ref_path } + end + it_behaves_like 'successfully evaluates pre-condition checks' end @@ -178,5 +190,34 @@ describe MergeRequests::MergeToRefService do it { expect(todo).not_to be_done } end + + describe 'cascading merge refs' do + set(:project) { create(:project, :repository) } + let(:params) { { commit_message: 'Cascading merge', first_parent_ref: first_parent_ref, target_ref: target_ref } } + + context 'when first merge happens' do + let(:merge_request) do + create(:merge_request, source_project: project, source_branch: 'feature', + target_project: project, target_branch: 'master') + end + + it_behaves_like 'successfully merges to ref with merge method' do + let(:first_parent_ref) { 'refs/heads/master' } + let(:target_ref) { 'refs/merge-requests/1/train' } + end + + context 'when second merge happens' do + let(:merge_request) do + create(:merge_request, source_project: project, source_branch: 'improve/awesome', + target_project: project, target_branch: 'master') + end + + it_behaves_like 'successfully merges to ref with merge method' do + let(:first_parent_ref) { 'refs/merge-requests/1/train' } + let(:target_ref) { 'refs/merge-requests/2/train' } + end + end + end + end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index b934533b1ab..157cfc46e69 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -359,6 +359,22 @@ describe SystemNoteService do end end + describe '.abort_merge_when_pipeline_succeeds' do + let(:noteable) do + create(:merge_request, source_project: project, target_project: project) + end + + subject { described_class.abort_merge_when_pipeline_succeeds(noteable, project, author, 'merge request was closed') } + + it_behaves_like 'a system note' do + let(:action) { 'merge' } + end + + it "posts the 'merge when pipeline succeeds' system note" do + expect(subject.note).to eq "aborted the automatic merge because merge request was closed" + end + end + describe '.change_title' do let(:noteable) { create(:issue, project: project, title: 'Lorem ipsum') } |