Commit fd2d1088 authored by Sean McGivern's avatar Sean McGivern

Merge branch '9688-add-remove-blocking-mrs' into 'master'

Add or remove blocking merge requests

See merge request gitlab-org/gitlab-ee!13506
parents e292cc74 32661e5b
......@@ -33,6 +33,8 @@
= render_if_exists 'shared/issuable/approvals', issuable: issuable, presenter: presenter, form: form
= render_if_exists "shared/issuable/form/merge_request_blocks", issuable: issuable, form: form
= render 'shared/issuable/form/branch_chooser', issuable: issuable, form: form
= render 'shared/issuable/form/merge_params', issuable: issuable
......
---
type: reference, concepts
---
# Blocking merge requests **(PREMIUM)**
> Introduced in GitLab Premium 12.2
Blocking merge requests allow dependencies between MRs to be expressed. If a
merge request is blocked by another MR, it cannot be merged until that blocking
MR is itself merged.
NOTE: **Note:**
Blocking merge requests are a **PREMIUM** feature, but this restriction is only
enforced for the blocked merge request. A merge request in a **CORE** or
**STARTER** project can block a **PREMIUM** merge request, but not vice-versa.
## Use cases
* Ensure changes to a library are merged before changes to a project that
imports the library
* Prevent a documentation-only merge request from being merged before the MR
implementing the feature to be documented
* Require an MR updating a permissions matrix to be merged before merging an
MR from someone who hasn't yet been granted permissions
It is common for a single logical change to span several merge requests. These
MRs may all be in a single project, or they may be spread out across multiple
projects, and the order in which they are merged can be significant.
For example, given a project `mycorp/awesome-project` that imports a library
at `myfriend/awesome-lib`, adding a feature in `awesome-project` may **also**
require changes to `awesome-lib`, and so necessitate two merge requests. Merging
the `awesome-project` MR before the `awesome-lib` one would break the `master`
branch.
The `awesome-project` MR could be [marked as WIP](work_in_progress_merge_requests.md),
and the reason for the WIP stated included in the comments. However, this
requires the state of the `awesome-lib` MR to be manually tracked, and doesn't
scale well if the `awesome-project` MR depends on changes to **several** other
projects.
By marking the `awesome-project` MR as blocked on the `awesome-lib` MR instead,
the status of the dependency is automatically tracked by GitLab, and the WIP
state can be used to communicate the readiness of the code in each individual
MR instead.
## Configuration
To continue the above example, you can configure a block when creating the
new MR in `awesome-project` (or by editing it, if it already exists). The block
needs to be configured on the MR that will be **blocked**, rather than on the
**blocking** MR. There is a "Blocking merge requests" section in the form:
![Blocking merge requests form control](img/edit_blocking_merge_requests.png)
Anyone who can edit a merge request can change the list of blocking merge
requests.
New blocks can be added by reference, by URL, or by using autcompletion. To
remove a block, press the "X" by its reference.
As blocks can be specified across projects, it's possible that someone else has
added a block for a merge request in a project you don't have access to. These
are shown as a simple count:
![Blocking merge requests form control with inaccessible MRs](img/edit_blocking_merge_requests_inaccessible.png)
If necessary, you can remove all the blocks like this by pressing the "X", just
as you would for a single, visible block.
Once you're finished, press the "Save changes" button to submit the request, or
"Cancel" to return without making any changes.
The list of configured blocks, and the status of each one, is shown in the merge
request widget:
![Blocking merge requests in merge request widget](img/show_blocking_merge_requests_in_mr_widget.png)
Until all blocking merge requests have, themselves, been merged, the "Merge"
button will be disabled. In particular, note that **closed** merge requests
still block their dependents - it is impossible to automatically determine if
merge requests that were blocked by that MR when it was open, are still blocked
when it is closed.
If a merge request has been closed **and** the block is no longer relevant, it
must be removed as a blocking MR, following the instructions above, before
merge.
## Limitations
* API support: [gitlab-ee#12551](https://gitlab.com/gitlab-org/gitlab-ee/issues/12551)
* Blocking relationships are not preserved across project export/import: [gitlab-ee#12549](https://gitlab.com/gitlab-org/gitlab-ee/issues/12549)
* Complex merge order dependencies are not supported: [gitlab-ee#11393](https://gitlab.com/gitlab-org/gitlab-ee/issues/11393)
The last item merits a little more explanation. Blocking merge requests can be
described as a graph of dependencies. The simplest possible graph has one
merge request blocking another:
```mermaid
graph LR;
myfriend/awesome-lib!10-->mycorp/awesome-project!100;
```
A more complex (and still supported) graph might have several MRs blocking
another from being merged:
```mermaid
graph LR;
myfriend/awesome-lib!10-->mycorp/awesome-project!100;
herfriend/another-lib!1-->mycorp/awesome-project!100;
```
We also support one MR blocking several others from being merged:
```mermaid
graph LR;
herfriend/another-lib!1-->myfriend/awesome-lib!10;
herfriend/another-lib!1-->mycorp/awesome-project!100;
```
What is **not** supported is a "deep", or "nested" graph of dependencies, e.g.:
```mermaid
graph LR;
herfriend/another-lib!1-->myfriend/awesome-lib!10;
myfriend/awesome-lib!10-->mycorp/awesome-project!100;
```
In this example, `myfriend/awesome-lib!10` would be blocked from being merged by
`herfriend/another-lib!1`, and would also block `mycorp/awesome-project!100`
from being merged. This is **not** yet supported.
......@@ -47,6 +47,7 @@ With **[GitLab Enterprise Edition][ee]**, you can also:
- Analyze your dependencies for vulnerabilities with [Dependency Scanning](../../application_security/dependency_scanning/index.md) **(ULTIMATE)**
- Analyze your Docker images for vulnerabilities with [Container Scanning](../../application_security/container_scanning/index.md) **(ULTIMATE)**
- Determine the performance impact of changes with [Browser Performance Testing](#browser-performance-testing-premium) **(PREMIUM)**
- Specify merge order dependencies with [Blocking Merge Requests](#blocking-merge-requests-premium) **(PREMIUM)**
## Use cases
......@@ -412,6 +413,21 @@ GitLab runs the [Sitespeed.io container][sitespeed-container] and displays the d
[Read more about Browser Performance Testing.](browser_performance_testing.md)
## Blocking Merge Requests **(PREMIUM)**
> Introduced in [GitLab Premium][products] 12.2.
A single logical change may be split across several merge requests, and perhaps
even across several projects. When this happens, the order in which MRs are
merged is important.
GitLab allows you to specify that a merge request is blocked by other MRs. With
this relationship in place, the merge request cannot be merged until all of its
blockers have also been merged, helping to maintain the consistency of a single
logical change.
[Read more about Blocking Merge Requests.](blocking_merge_requests.md)
## Security reports **(ULTIMATE)**
GitLab can scan and report any vulnerabilities found in your project.
......
import mountApprovals from 'ee/approvals/mount_mr_edit';
import mountBlockingMergeRequestsInput from 'ee/projects/merge_requests/blocking_mr_input';
export default () => {
mountApprovals(document.getElementById('js-mr-approvals-input'));
mountBlockingMergeRequestsInput(document.getElementById('js-blocking-merge-requests-input'));
};
import Vue from 'vue';
import BlockingMrInput from 'ee/projects/merge_requests/blocking_mr_input_root.vue';
import { n__ } from '~/locale';
export default el => {
if (!el) {
return null;
}
const { hiddenBlockingMrsCount, visibleBlockingMrRefs } = el.dataset;
const parsedVisibleBlockingMrRefs = JSON.parse(visibleBlockingMrRefs);
const containsHiddenBlockingMrs = hiddenBlockingMrsCount > 0;
const references = containsHiddenBlockingMrs
? [
...parsedVisibleBlockingMrRefs,
{
text: n__(
'%d inaccessible merge request',
'%d inaccessible merge requests',
hiddenBlockingMrsCount,
),
isHiddenRef: true,
},
]
: parsedVisibleBlockingMrRefs;
return new Vue({
el,
render(h) {
return h(BlockingMrInput, {
props: {
existingRefs: references,
containsHiddenBlockingMrs,
},
});
},
});
};
<script>
import RelatedIssuableInput from 'ee/related_issues/components/related_issuable_input.vue';
import { issuableTypesMap } from 'ee/related_issues/constants';
export default {
components: {
RelatedIssuableInput,
},
props: {
existingRefs: {
type: Array,
required: false,
default: () => [],
},
containsHiddenBlockingMrs: {
type: Boolean,
required: false,
default: false,
},
},
data() {
return {
references: this.existingRefs,
inputValue: '',
shouldRemoveHiddenBlockingMrs: false,
hasFieldBeenTouched: false,
};
},
computed: {
autoCompleteSources() {
return gl.GfmAutoComplete && gl.GfmAutoComplete.dataSources;
},
},
methods: {
onAddIssuable({ untouchedRawReferences, touchedReference }) {
this.hasFieldBeenTouched = true;
this.setReferences(this.references.concat(untouchedRawReferences));
this.inputValue = touchedReference;
},
onPendingIssuableRemoveRequest(index) {
this.references.splice(index, 1);
},
setReferences(refs) {
// Remove duplicates but retain order.
// If you don't do this, Vue will be confused by duplicates and refuse to delete them all.
this.references = refs.filter((ref, idx) => refs.indexOf(ref) === idx);
},
removeReference(idToRemove) {
this.hasFieldBeenTouched = true;
// With this `if` statement we should only ever have to actually check the `includes` statement once.
if (
this.containsHiddenBlockingMrs &&
!this.shouldRemoveHiddenBlockingMrs &&
this.references[idToRemove].isHiddenRef
) {
this.shouldRemoveHiddenBlockingMrs = true;
}
this.references.splice(idToRemove, 1);
},
onBlur(val) {
if (val) {
this.setReferences(this.references.concat(val));
this.inputValue = '';
}
},
},
issuableTypesMap,
};
</script>
<template>
<div>
<related-issuable-input
path-id-separator="!"
input-id="merge_request_blocking_merge_request_references"
:references="references"
:input-value="inputValue"
:issuable-type="$options.issuableTypesMap.MERGE_REQUEST"
:auto-complete-options="{ mergeRequests: true }"
:auto-complete-sources="autoCompleteSources"
@addIssuableFormInput="onAddIssuable"
@pendingIssuableRemoveRequest="removeReference"
@addIssuableFormBlur="onBlur"
/>
<input
v-for="ref in references"
:key="ref"
:value="ref"
type="hidden"
name="merge_request[blocking_merge_request_references][]"
/>
<input
v-if="containsHiddenBlockingMrs"
type="hidden"
name="merge_request[remove_hidden_blocking_merge_requests]"
:value="shouldRemoveHiddenBlockingMrs"
/>
<input
type="hidden"
name="merge_request[update_blocking_merge_request_refs]"
:value="hasFieldBeenTouched"
/>
</div>
</template>
......@@ -12,6 +12,11 @@ export default {
issueToken,
},
props: {
inputId: {
type: String,
required: false,
default: '',
},
references: {
type: Array,
required: false,
......@@ -162,7 +167,7 @@ export default {
>
<issue-token
:id-key="index"
:display-reference="reference"
:display-reference="reference.text || reference"
:can-remove="true"
:is-condensed="true"
:path-id-separator="pathIdSeparator"
......@@ -176,6 +181,7 @@ export default {
</li>
<li class="add-issuable-form-input-list-item">
<input
:id="inputId"
ref="input"
:value="inputValue"
:placeholder="inputPlaceholder"
......
......@@ -3,22 +3,26 @@ import { __ } from '~/locale';
export const issuableTypesMap = {
ISSUE: 'issue',
EPIC: 'epic',
MERGE_REQUEST: 'merge_request',
};
export const autoCompleteTextMap = {
true: {
[issuableTypesMap.ISSUE]: __(' or <#issue id>'),
[issuableTypesMap.EPIC]: __(' or <#epic id>'),
[issuableTypesMap.MERGE_REQUEST]: __(' or <#merge request id>'),
},
false: {
[issuableTypesMap.ISSUE]: '',
[issuableTypesMap.EPIC]: '',
[issuableTypesMap.MERGE_REQUEST]: '',
},
};
export const inputPlaceholderTextMap = {
[issuableTypesMap.ISSUE]: __('Paste issue link'),
[issuableTypesMap.EPIC]: __('Paste epic link'),
[issuableTypesMap.MERGE_REQUEST]: __('Paste a merge request link'),
};
export const relatedIssuesRemoveErrorMap = {
......
......@@ -23,6 +23,9 @@ module EE
def merge_request_params_attributes
attrs = super.push(
{ blocking_merge_request_references: [] },
:update_blocking_merge_request_refs,
:remove_hidden_blocking_merge_requests,
approval_rule_attributes,
:approvals_before_merge,
:approver_group_ids,
......
......@@ -6,6 +6,7 @@ module EE
extend ::Gitlab::Utils::Override
include ::Approvable
include ::Gitlab::Allowable
include ::Gitlab::Utils::StrongMemoize
include FromUnion
......@@ -83,6 +84,28 @@ module EE
false
end
def visible_blocking_merge_requests(user)
Ability.merge_requests_readable_by_user(blocking_merge_requests, user)
end
def visible_blocking_merge_request_refs(user)
visible_blocking_merge_requests(user).map do |mr|
mr.to_reference(target_project)
end
end
# Unlike +visible_blocking_merge_requests+, this method doesn't include
# blocking MRs that have been merged. This simplifies output, since we don't
# need to tell the user that there are X hidden blocking MRs, of which only
# Y are an obstacle. Pass include_merged: true to override this behaviour.
def hidden_blocking_merge_requests_count(user, include_merged: false)
hidden = blocking_merge_requests - visible_blocking_merge_requests(user)
hidden.delete_if(&:merged?) unless include_merged
hidden.count
end
def validate_approval_rule_source
return unless approval_rules.any?
......
......@@ -10,6 +10,27 @@ class MergeRequestBlock < ApplicationRecord
validate :check_block_constraints
scope :with_blocking_mr_ids, -> (ids) do
where(blocking_merge_request_id: ids).includes(:blocking_merge_request)
end
# Add a number of blocks at once. Pass a hash of blocking_id => blocked_id, or
# an Array of [blocking_id, blocked_id] tuples
def self.bulk_insert(pairs)
now = Time.current
rows = pairs.map do |blocking, blocked|
{
blocking_merge_request_id: blocking,
blocked_merge_request_id: blocked,
created_at: now,
updated_at: now
}
end
::Gitlab::Database.bulk_insert(table_name, rows)
end
private
def check_block_constraints
......
......@@ -173,21 +173,14 @@ module EE
private
def blocking_merge_requests
visible_mrs_by_state = Hash.new { |h, k| h[k] = [] }
visible_count = 0
hidden_blocking_count = 0
object.blocking_merge_requests.each do |mr|
if can?(current_user, :read_merge_request, mr)
visible_mrs_by_state[mr.state_name] << represent_blocking_mr(mr)
visible_count += 1
elsif !mr.merged? # Ignore merged hidden MRs to make display simpler
hidden_blocking_count += 1
end
end
hidden_blocking_count = object.hidden_blocking_merge_requests_count(current_user)
visible_mrs = object.visible_blocking_merge_requests(current_user)
visible_mrs_by_state = visible_mrs
.map { |mr| represent_blocking_mr(mr) }
.group_by { |blocking_mr| blocking_mr.object.state_name }
{
total_count: visible_count + hidden_blocking_count,
total_count: visible_mrs.count + hidden_blocking_count,
hidden_count: hidden_blocking_count,
visible_merge_requests: visible_mrs_by_state
}
......
......@@ -5,6 +5,8 @@ module EE
module BaseService
private
attr_accessor :blocking_merge_requests_params
def filter_params(merge_request)
unless current_user.can?(:update_approvers, merge_request)
params.delete(:approvals_before_merge)
......@@ -14,6 +16,9 @@ module EE
self.params = ApprovalRules::ParamsFilteringService.new(merge_request, current_user, params).execute
self.blocking_merge_requests_params =
::MergeRequests::UpdateBlocksService.extract_params!(params)
super
end
end
......
......@@ -17,6 +17,10 @@ module EE
if pipeline
::SyncSecurityReportsToReportApprovalRulesWorker.perform_async(pipeline.id)
end
::MergeRequests::UpdateBlocksService
.new(issuable, current_user, blocking_merge_requests_params)
.execute
end
end
end
......
......@@ -31,6 +31,10 @@ module EE
notification_service.add_merge_request_approvers(merge_request, new_approvers, current_user)
end
::MergeRequests::UpdateBlocksService
.new(merge_request, current_user, blocking_merge_requests_params)
.execute
merge_request
end
......
# frozen_string_literal: true
module MergeRequests
class UpdateBlocksService
include ::Gitlab::Allowable
include ::Gitlab::Utils::StrongMemoize
class << self
def extract_params!(mutable_params)
{
update: mutable_params.delete(:update_blocking_merge_request_refs),
remove_hidden: mutable_params.delete(:remove_hidden_blocking_merge_requests),
references: mutable_params.delete(:blocking_merge_request_references)
}
end
end
attr_reader :merge_request, :current_user, :params
def initialize(merge_request, current_user, params = {})
@merge_request = merge_request
@current_user = current_user
@params = params
DeclarativePolicy.user_scope do
@visible_blocks, @hidden_blocks = merge_request.blocks_as_blockee.partition do |block|
can?(current_user, :read_merge_request, block.blocking_merge_request)
end
end
end
def execute
return unless update?
return unless merge_request.target_project.feature_available?(:blocking_merge_requests)
merge_request
.blocks_as_blockee
.with_blocking_mr_ids(ids_to_del)
.delete_all
::MergeRequestBlock.bulk_insert(
ids_to_add.map { |blocking_id| [blocking_id, merge_request.id] }
)
true
end
private
attr_reader :visible_blocks, :hidden_blocks
def update?
params.fetch(:update, false)
end
def remove_hidden?
params.fetch(:remove_hidden, false)
end
def references
params.fetch(:references, [])
end
def requested_ids
strong_memoize(:requested_ids) do
next [] unless references.present?
# The analyzer will only return references the current user can see
analyzer = ::Gitlab::ReferenceExtractor.new(merge_request.target_project, current_user)
analyzer.analyze(references.join(" "))
analyzer.merge_requests.map(&:id)
end
end
def visible_ids
strong_memoize(:visible_ids) { visible_blocks.map(&:blocking_merge_request_id) }
end
def hidden_ids
strong_memoize(:hidden_ids) { hidden_blocks.map(&:blocking_merge_request_id) }
end
def ids_to_add
strong_memoize(:ids_to_add) { requested_ids - visible_ids }
end
def ids_to_del
strong_memoize(:ids_to_del) do
(visible_ids - requested_ids).tap do |ary|
ary.push(*hidden_ids) if remove_hidden?
end
end
end
end
end
- merge_request = local_assigns.fetch(:issuable)
- return unless merge_request.is_a?(MergeRequest)
- form = local_assigns.fetch(:form)
- project = merge_request.target_project
- return unless project&.feature_available?(:blocking_merge_requests)
.form-group.row.blocking-merge-requests
= form.label :blocking_merge_request_references, _('Blocking merge requests'), class: 'col-form-label col-sm-2'
.col-sm-10
= text_field_tag 'blocking_merge_request_refs', nil,
class: "form-control ",
id: "js-blocking-merge-requests-input",
data: { hidden_blocking_mrs_count: merge_request.hidden_blocking_merge_requests_count(current_user),
visible_blocking_mr_refs: merge_request.visible_blocking_merge_request_refs(current_user) }
---
title: Support for blocking merge requests
merge_request: 13506
author:
type: added
# frozen_string_literal: true
require 'spec_helper'
describe 'User creates a merge request with blocking MRs', :js do
let(:project) { create(:project, :repository) }
let(:user) { project.owner }
let(:mr_params) { { title: 'Some feature', source_branch: 'fix', target_branch: 'feature' } }
before do
sign_in(user)
end
context 'feature is enabled' do
before do
stub_licensed_features(blocking_merge_requests: true)
end
it 'creates a merge request with a blocking MR' do
other_mr = create(:merge_request)
other_mr.target_project.team.add_maintainer(user)
visit(project_new_merge_request_path(project, merge_request: mr_params))
fill_in 'Blocking merge requests', with: other_mr.to_reference(full: true)
click_button('Submit merge request')
expect(page).to have_content('Blocked by 1 merge request')
end
end
context 'feature is disabled' do
before do
stub_licensed_features(blocking_merge_requests: false)
end
it 'does not show blocking MRs controls' do
visit(project_new_merge_request_path(project, merge_request: mr_params))
expect(page).not_to have_content('Blocking merge requests')
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe "User edits merge request with blocking MRs", :js do
let(:merge_request) { create(:merge_request) }
let(:project) { merge_request.target_project }
let(:user) { merge_request.target_project.owner }
let(:other_mr) { create(:merge_request) }
before do
sign_in(user)
end
context 'feature is enabled' do
before do
stub_licensed_features(blocking_merge_requests: true)
end
context 'user can see the other MR' do
before do
other_mr.target_project.team.add_developer(user)
end
it 'can add the other MR' do
visit edit_project_merge_request_path(project, merge_request)
fill_in 'Blocking merge requests', with: other_mr.to_reference(full: true)
click_button 'Save changes'
expect(page).to have_content('Blocked by 1 merge request')
end
it 'can see and remove an existing blocking MR' do
create(:merge_request_block, blocking_merge_request: other_mr, blocked_merge_request: merge_request)
visit edit_project_merge_request_path(project, merge_request)
expect(page).to have_content(other_mr.to_reference(full: true))
click_button "Remove #{other_mr.to_reference(full: true)}"
click_button 'Save changes'
expect(page).not_to have_content('Blocked by 1 merge request')
expect(page).not_to have_content(other_mr.to_reference(full: true))
end
end
context 'user cannot see the other MR' do
it 'cannot add the other MR' do
visit edit_project_merge_request_path(project, merge_request)
fill_in 'Blocking merge requests', with: other_mr.to_reference(full: true)
click_button 'Save changes'
expect(page).not_to have_content('Blocked by 1 merge request')
end
it 'sees the existing MR as hidden and can remove it' do
create(:merge_request_block, blocking_merge_request: other_mr, blocked_merge_request: merge_request)
visit edit_project_merge_request_path(project, merge_request)
expect(page).to have_content('1 inaccessible merge request')
click_button 'Remove 1 inaccessible merge request'
click_button 'Save changes'
expect(page).not_to have_content('Blocked by 1 merge request')
expect(page).not_to have_content(other_mr.to_reference(full: true))
end
end
end
context 'feature is disabled' do
before do
stub_licensed_features(blocking_merge_requests: false)
end
it 'cannot see the blocking MR controls' do
visit edit_project_merge_request_path(project, merge_request)
expect(page).not_to have_content('Blocking merge requests')
end
end
end
import { shallowMount } from '@vue/test-utils';
import BlockingMrInputRoot from 'ee/projects/merge_requests/blocking_mr_input_root.vue';
import RelatedIssuableInput from 'ee/related_issues/components/related_issuable_input.vue';
describe('blocking mr input root', () => {
let wrapper;
const getInput = () => wrapper.find(RelatedIssuableInput);
const addTokenizedInput = input => {
getInput().vm.$emit('addIssuableFormInput', {
untouchedRawReferences: [input],
touchedReference: '',
});
};
const addInput = input => {
getInput().vm.$emit('addIssuableFormInput', {
untouchedRawReferences: [],
touchedReference: input,
});
};
const removeRef = index => {
getInput().vm.$emit('pendingIssuableRemoveRequest', index);
};
const createComponent = (propsData = {}) => {
wrapper = shallowMount(BlockingMrInputRoot, { propsData });
};
it('does not keep duplicate references', () => {
createComponent();
const input = '!1';
addTokenizedInput(input);
addTokenizedInput(input);
expect(wrapper.vm.references).toEqual(['!1']);
});
it('updates input value to empty string when adding a tokenized input', () => {
createComponent();
addTokenizedInput('foo');
expect(wrapper.vm.inputValue).toBe('');
});
it('updates input value to ref when typing into input (before adding whitespace)', () => {
createComponent();
addInput('foo');
expect(wrapper.vm.inputValue).toBe('foo');
});
it('does not reorder when adding a ref that already exists', () => {
const input = '!1';
createComponent({
existingRefs: [input, '!2'],
});
addTokenizedInput(input, wrapper);
expect(wrapper.vm.references).toEqual(['!1', '!2']);
});
it('does not add empty reference on blur', () => {
createComponent();
getInput().vm.$emit('addIssuableFormBlur', '');
expect(wrapper.vm.references).toHaveLength(0);
});
describe('hidden inputs', () => {
const createHiddenInputExpectation = selector => bool => {
expect(wrapper.find(selector).element.value).toBe(`${bool}`);
};
describe('update_blocking_merge_request_refs', () => {
const expectShouldUpdateRefsToBe = createHiddenInputExpectation(
'input[name="merge_request[update_blocking_merge_request_refs]"]',
);
it('is false when nothing happens', () => {
createComponent();
expectShouldUpdateRefsToBe(false);
});
it('is true after a ref is removed', () => {
createComponent({ existingRefs: ['!1'] });
removeRef(0);
expectShouldUpdateRefsToBe(true);
});
it('is true after a ref is added', () => {
createComponent();
addTokenizedInput('foo');
expectShouldUpdateRefsToBe(true);
});
});
describe('remove_hidden_blocking_merge_requests', () => {
const expectRemoveHiddenBlockingMergeRequestsToBe = createHiddenInputExpectation(
'input[name="merge_request[update_blocking_merge_request_refs]"]',
);
const makeComponentWithHiddenMrs = () => {
const hiddenMrsRef = '2 inaccessible merge requests';
createComponent({
containsHiddenBlockingMrs: true,
existingRefs: ['!1', '!2', hiddenMrsRef],
});
};
it('is true when nothing has happened', () => {
makeComponentWithHiddenMrs();
expectRemoveHiddenBlockingMergeRequestsToBe(false);
});
it('is false when removing any other MRs', () => {
makeComponentWithHiddenMrs();
expectRemoveHiddenBlockingMergeRequestsToBe(false);
});
it('is false when ref has been removed', () => {
makeComponentWithHiddenMrs();
removeRef(2);
expectRemoveHiddenBlockingMergeRequestsToBe(true);
});
});
});
});
import Vue from 'vue';
import initBlockingMrInput from 'ee/projects/merge_requests/blocking_mr_input';
jest.mock('vue');
describe('BlockingMrInput', () => {
let h;
const refs = ['!1'];
const getProps = () => h.mock.calls[0][1].props;
const callRender = () => {
Vue.mock.calls[0][0].render(h);
};
const setInnerHtml = (visibleMrs = refs, hiddenCount = 2) => {
document.body.innerHTML += `<div id="test" data-hidden-blocking-mrs-count="${hiddenCount}" data-visible-blocking-mr-refs='${JSON.stringify(
visibleMrs,
)}'></div>`;
};
beforeEach(() => {
h = jest.fn();
});
afterEach(() => {
document.querySelector('#test').remove();
jest.clearAllMocks();
});
it('adds hidden references block when hidden count is greater than 0', () => {
setInnerHtml();
initBlockingMrInput(document.querySelector('#test'));
callRender();
expect(getProps().existingRefs[refs.length].text).toBe('2 inaccessible merge requests');
});
it('containsHiddenBlockingMrs is true when count is greater than one', () => {
setInnerHtml();
initBlockingMrInput(document.querySelector('#test'));
callRender();
expect(getProps().containsHiddenBlockingMrs).toBe(true);
});
it('containsHiddenBlockingMrs is false when count is zero', () => {
setInnerHtml(refs, 0);
initBlockingMrInput(document.querySelector('#test'));
callRender();
expect(getProps().containsHiddenBlockingMrs).toBe(false);
});
});
......@@ -68,4 +68,80 @@ describe MergeRequest do
end
end
end
describe '#visible_blocking_merge_requests' do
let(:block) { create(:merge_request_block) }
let(:blocking_mr) { block.blocking_merge_request }
let(:blocked_mr) { block.blocked_merge_request }
let(:user) { create(:user) }
it 'shows blocking MR to developer' do
blocking_mr.target_project.team.add_developer(user)
expect(blocked_mr.visible_blocking_merge_requests(user)).to contain_exactly(blocking_mr)
end
it 'hides block from guest' do
blocking_mr.target_project.team.add_guest(user)
expect(blocked_mr.visible_blocking_merge_requests(user)).to be_empty
end
it 'hides block from anonymous user' do
expect(blocked_mr.visible_blocking_merge_requests(nil)).to be_empty
end
end
describe '#visible_blocking_merge_request_refs' do
let(:merge_request) { create(:merge_request) }
let(:other_mr) { create(:merge_request) }
let(:user) { create(:user) }
it 'returns the references for the result of #visible_blocking_merge_requests' do
expect(merge_request)
.to receive(:visible_blocking_merge_requests)
.with(user)
.and_return([other_mr])
expect(merge_request.visible_blocking_merge_request_refs(user))
.to eq([other_mr.to_reference(full: true)])
end
end
describe '#hidden_blocking_merge_requests_count' do
let(:block) { create(:merge_request_block) }
let(:blocking_mr) { block.blocking_merge_request }
let(:blocked_mr) { block.blocked_merge_request }
let(:user) { create(:user) }
it 'returns 0 when all MRs are visible' do
blocking_mr.target_project.team.add_developer(user)
expect(blocked_mr.hidden_blocking_merge_requests_count(user)).to eq(0)
end
context 'MR is hidden' do
before do
blocking_mr.target_project.team.add_guest(user)
end
it 'returns 1 when MR is unmerged by default' do
expect(blocked_mr.hidden_blocking_merge_requests_count(user)).to eq(1)
end
context 'MR is merged' do
before do
blocking_mr.update_columns(state: 'merged')
end
it 'returns 0 by default' do
expect(blocked_mr.hidden_blocking_merge_requests_count(user)).to eq(0)
end
it 'returns 1 when include_merged: true' do
expect(blocked_mr.hidden_blocking_merge_requests_count(user, include_merged: true)).to eq(1)
end
end
end
end
end
......@@ -60,4 +60,45 @@ describe MergeRequestBlock do
expect(new_block).not_to be_valid
end
end
describe '.bulk_insert' do
let(:mrs) { create_list(:merge_request, 4) }
it 'inserts multiple blocks specified as a Hash' do
described_class.bulk_insert(
mrs[0].id => mrs[1].id,
mrs[2].id => mrs[3].id
)
expect(described_class.all).to contain_exactly(
having_attributes(blocking_merge_request_id: mrs[0].id, blocked_merge_request_id: mrs[1].id),
having_attributes(blocking_merge_request_id: mrs[2].id, blocked_merge_request_id: mrs[3].id)
)
end
it 'inserts multiple blocks specified as an Array' do
described_class.bulk_insert([[mrs[0].id, mrs[1].id], [mrs[2].id, mrs[3].id]])
expect(described_class.all).to contain_exactly(
having_attributes(blocking_merge_request_id: mrs[0].id, blocked_merge_request_id: mrs[1].id),
having_attributes(blocking_merge_request_id: mrs[2].id, blocked_merge_request_id: mrs[3].id)
)
end
end
describe '.with_blocking_mr_ids' do
let!(:block) { create(:merge_request_block) }
let!(:other_block) { create(:merge_request_block) }
subject(:result) { described_class.with_blocking_mr_ids([block.blocking_merge_request_id]) }
it 'returns blocks with a matching blocking_merge_request_id' do
is_expected.to contain_exactly(block)
end
it 'eager-loads the blocking MRs' do
association = result.first.association(:blocking_merge_request)
expect(association.loaded?).to be(true)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe MergeRequests::CreateService do
let(:project) { create(:project, :repository) }
let(:user) { project.owner }
subject(:service) { described_class.new(project, user, params) }
describe '#execute' do
context 'with blocking merge requests' do
let(:params) { { title: 'Blocked MR', source_branch: 'feature', target_branch: 'master' } }
it 'delegates to MergeRequests::UpdateBlocksService' do
expect(MergeRequests::UpdateBlocksService)
.to receive(:extract_params!)
.and_return(:extracted_params)
expect_next_instance_of(MergeRequests::UpdateBlocksService) do |block_service|
expect(block_service.merge_request.title).to eq('Blocked MR')
expect(block_service.current_user).to eq(user)
expect(block_service.params).to eq(:extracted_params)
expect(block_service).to receive(:execute)
end
service.execute
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe MergeRequests::UpdateBlocksService do
describe '.extract_params!' do
it 'removes and reformats merge request params' do
mr_params = {
unrelated: true,
update_blocking_merge_request_refs: true,
remove_hidden_blocking_merge_requests: true,
blocking_merge_request_references: ['!1']
}
block_params = described_class.extract_params!(mr_params)
expect(block_params).to eq(
update: true,
remove_hidden: true,
references: ['!1']
)
expect(mr_params).to eq(unrelated: true)
end
end
describe '#execute' do
let(:merge_request) { create(:merge_request) }
let(:user) { merge_request.target_project.owner }
let(:mr_to_ignore) { create(:merge_request) }
let(:mr_to_add) { create(:merge_request) }
let(:mr_to_keep) { create(:merge_request) }
let(:mr_to_del) { create(:merge_request) }
let(:hidden_mr) { create(:merge_request) }
let(:refs) do
[mr_to_ignore, mr_to_add, mr_to_keep].map { |mr| mr.to_reference(full: true) }
end
let(:params) do
{
remove_hidden: remove_hidden,
references: refs,
update: update
}
end
subject(:service) { described_class.new(merge_request, user, params) }
before do
[mr_to_add, mr_to_keep, mr_to_del].each do |mr|
mr.target_project.team.add_maintainer(user)
end
create(:merge_request_block, blocking_merge_request: mr_to_keep, blocked_merge_request: merge_request)
create(:merge_request_block, blocking_merge_request: mr_to_del, blocked_merge_request: merge_request)
create(:merge_request_block, blocking_merge_request: hidden_mr, blocked_merge_request: merge_request)
end
context 'licensed' do
before do
stub_licensed_features(blocking_merge_requests: true)
end
context 'with update: false' do
let(:update) { false }
let(:remove_hidden) { true }
it 'does nothing' do
expect { service.execute }.not_to change { MergeRequestBlock.count }
end
end
context 'with update: true' do
let(:update) { true }
context 'with remove_hidden: false' do
let(:remove_hidden) { false }
it 'adds only the requested MRs the user can see' do
service.execute
expect(merge_request.blocking_merge_requests)
.to contain_exactly(mr_to_add, mr_to_keep, hidden_mr)
end
end
context 'with remove_hidden: true' do
let(:remove_hidden) { true }
it 'adds visible MRs and removes the hidden MR' do
service.execute
expect(merge_request.blocking_merge_requests)
.to contain_exactly(mr_to_add, mr_to_keep)
end
end
end
end
context 'unlicensed' do
let(:update) { true }
let(:remove_hidden) { true }
before do
stub_licensed_features(blocking_merge_requests: false)
end
it 'does nothing' do
expect { service.execute }.not_to change { MergeRequestBlock.count }
end
end
end
end
......@@ -179,5 +179,23 @@ describe MergeRequests::UpdateService, :mailer do
expect(merge_request.reload.approvals).to be_empty
end
end
context 'updating blocking merge requests' do
it 'delegates to MergeRequests::UpdateBlocksService' do
expect(MergeRequests::UpdateBlocksService)
.to receive(:extract_params!)
.and_return(:extracted_params)
expect_next_instance_of(MergeRequests::UpdateBlocksService) do |service|
expect(service.merge_request).to eq(merge_request)
expect(service.current_user).to eq(user)
expect(service.params).to eq(:extracted_params)
expect(service).to receive(:execute)
end
update_merge_request({})
end
end
end
end
......@@ -56,6 +56,9 @@ msgstr ""
msgid " or <#issue id>"
msgstr ""
msgid " or <#merge request id>"
msgstr ""
msgid "%d comment"
msgid_plural "%d comments"
msgstr[0] ""
......@@ -99,6 +102,11 @@ msgid_plural "%d fixed test results"
msgstr[0] ""
msgstr[1] ""
msgid "%d inaccessible merge request"
msgid_plural "%d inaccessible merge requests"
msgstr[0] ""
msgstr[1] ""
msgid "%d issue"
msgid_plural "%d issues"
msgstr[0] ""
......@@ -2117,6 +2125,9 @@ msgid_plural "Blocked by <strong>%d closed</strong> merge requests."
msgstr[0] ""
msgstr[1] ""
msgid "Blocking merge requests"
msgstr ""
msgid "Blog"
msgstr ""
......@@ -9750,6 +9761,9 @@ msgstr ""
msgid "Paste a machine public key here. Read more about how to generate it %{link_start}here%{link_end}"
msgstr ""
msgid "Paste a merge request link"
msgstr ""
msgid "Paste epic link"
msgstr ""
......
......@@ -47,7 +47,9 @@ describe 'Multi-file editor new directory', :js do
fill_in('commit-message', with: 'commit message ide')
click_button('Commit')
page.within '.multi-file-commit-form' do
click_button('Commit')
end
find('.js-ide-edit-mode').click
......
......@@ -39,7 +39,9 @@ describe 'Multi-file editor new file', :js do
fill_in('commit-message', with: 'commit message ide')
click_button('Commit')
page.within '.multi-file-commit-form' do
click_button('Commit')
end
expect(page).to have_content('file name')
end
......
......@@ -58,6 +58,7 @@ Capybara.javascript_driver = :chrome
Capybara.default_max_wait_time = timeout
Capybara.ignore_hidden_elements = true
Capybara.default_normalize_ws = true
Capybara.enable_aria_label = true
# Keep only the screenshots generated from the last failing test suite
Capybara::Screenshot.prune_strategy = :keep_last_run
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment