summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChristian Couder <chriscool@tuxfamily.org>2019-08-15 05:43:33 +0200
committerChristian Couder <chriscool@tuxfamily.org>2019-09-04 12:11:24 +0200
commitf00db0c342d01b33617f269447ff76140944a86e (patch)
treee1def576ec2f00ce18272d17ee913d2a60d09bed
parent60adc14473911fd9bd33feef2fbfd62a9824a11c (diff)
downloadgitlab-ce-f00db0c342d01b33617f269447ff76140944a86e.tar.gz
Support adding and removing labels w/ push opts
MergeRequests::PushOptionsHandlerService has been updated to allow adding and removing labels to a merge request using git push options. To create a new merge request and add 2 labels to it: git push -u origin -o merge_request.create \ -o merge_request.label="My label 1" \ -o merge_request.label="My label 2" To update an existing merge request and remove a label while adding a different label: git push -u origin -o merge_request.label="My added label" \ -o merge_request.unlabel="My removed label" Issue https://gitlab.com/gitlab-org/gitlab-ce/issues/64320
-rw-r--r--app/services/issuable_base_service.rb13
-rw-r--r--app/services/labels/available_labels_service.rb4
-rw-r--r--app/services/merge_requests/build_service.rb12
-rw-r--r--app/services/merge_requests/push_options_handler_service.rb10
-rw-r--r--changelogs/unreleased/add-label-push-opts.yml5
-rw-r--r--doc/user/project/merge_requests/index.md28
-rw-r--r--lib/gitlab/push_options.rb21
-rw-r--r--spec/services/merge_requests/push_options_handler_service_spec.rb145
8 files changed, 230 insertions, 8 deletions
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 2ab6e88599f..71c86106f48 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -76,8 +76,17 @@ class IssuableBaseService < BaseService
end
def filter_labels
- params[:add_label_ids] = labels_service.filter_labels_ids_in_param(:add_label_ids) if params[:add_label_ids]
- params[:remove_label_ids] = labels_service.filter_labels_ids_in_param(:remove_label_ids) if params[:remove_label_ids]
+ if params[:add_label_ids]
+ params[:add_label_ids] = labels_service.filter_labels_ids_in_param(:add_label_ids)
+ elsif params[:add_labels]
+ params[:add_label_ids] = labels_service.find_or_create_by_titles(:add_labels).map(&:id)
+ end
+
+ if params[:remove_label_ids]
+ params[:remove_label_ids] = labels_service.filter_labels_ids_in_param(:remove_label_ids)
+ elsif params[:remove_labels]
+ params[:remove_label_ids] = labels_service.find_or_create_by_titles(:remove_labels).map(&:id)
+ end
if params[:label_ids]
params[:label_ids] = labels_service.filter_labels_ids_in_param(:label_ids)
diff --git a/app/services/labels/available_labels_service.rb b/app/services/labels/available_labels_service.rb
index fe477d96970..bdd5f660576 100644
--- a/app/services/labels/available_labels_service.rb
+++ b/app/services/labels/available_labels_service.rb
@@ -9,8 +9,8 @@ module Labels
@params = params
end
- def find_or_create_by_titles
- labels = params.delete(:labels)
+ def find_or_create_by_titles(key = :labels)
+ labels = params.delete(key)
return [] unless labels
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index b28f80939ae..308a3a10d1a 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -18,6 +18,18 @@ module MergeRequests
merge_request.target_project = find_target_project
filter_params(merge_request)
+
+ # merge_request.assign_attributes(...) below is a Rails
+ # method that only work if all the params it is passed have
+ # corresponding fields in the database. As there are no fields
+ # in the database for :add_label_ids and :remove_label_ids, we
+ # need to remove them from the params before the call to
+ # merge_request.assign_attributes(...)
+ #
+ # IssuableBaseService#process_label_ids takes care
+ # of the removal.
+ params[:label_ids] = process_label_ids(params, extra_label_ids: merge_request.label_ids.to_a)
+
merge_request.assign_attributes(params.to_h.compact)
merge_request.compare_commits = []
diff --git a/app/services/merge_requests/push_options_handler_service.rb b/app/services/merge_requests/push_options_handler_service.rb
index b210004e6e1..0168b31005e 100644
--- a/app/services/merge_requests/push_options_handler_service.rb
+++ b/app/services/merge_requests/push_options_handler_service.rb
@@ -100,7 +100,8 @@ module MergeRequests
merge_request = ::MergeRequests::CreateService.new(
project,
current_user,
- merge_request.attributes.merge(assignees: merge_request.assignees)
+ merge_request.attributes.merge(assignees: merge_request.assignees,
+ label_ids: merge_request.label_ids)
).execute
end
@@ -122,7 +123,9 @@ module MergeRequests
title: push_options[:title],
description: push_options[:description],
target_branch: push_options[:target],
- force_remove_source_branch: push_options[:remove_source_branch]
+ force_remove_source_branch: push_options[:remove_source_branch],
+ label: push_options[:label],
+ unlabel: push_options[:unlabel]
}
params.compact!
@@ -134,6 +137,9 @@ module MergeRequests
)
end
+ params[:add_labels] = params.delete(:label).keys if params.has_key?(:label)
+ params[:remove_labels] = params.delete(:unlabel).keys if params.has_key?(:unlabel)
+
params
end
diff --git a/changelogs/unreleased/add-label-push-opts.yml b/changelogs/unreleased/add-label-push-opts.yml
new file mode 100644
index 00000000000..1289020e4e5
--- /dev/null
+++ b/changelogs/unreleased/add-label-push-opts.yml
@@ -0,0 +1,5 @@
+---
+title: Support adding and removing labels w/ push opts
+merge_request: 31831
+author:
+type: added
diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md
index d6da8cb99c7..9f31f38460a 100644
--- a/doc/user/project/merge_requests/index.md
+++ b/doc/user/project/merge_requests/index.md
@@ -289,6 +289,7 @@ as pushing changes:
- Set the merge request to remove the source branch when it's merged.
- Set the title of the merge request to a particular title.
- Set the description of the merge request to a particular description.
+- Add or remove labels from the merge request.
### Create a new merge request using git push options
@@ -375,6 +376,33 @@ git push -o merge_request.description="The description I want"
You can also use this push option in addition to the
`merge_request.create` push option.
+### Add or remove labels using git push options
+
+You can add or remove labels from merge requests using push options.
+
+For example, to add two labels to an existing merge request, use the
+`merge_request.label` push option:
+
+```sh
+git push -o merge_request.label="label1" -o merge_request.label="label2"
+```
+
+To remove two labels from an existing merge request, use
+the `merge_request.unlabel` push option:
+
+```sh
+git push -o merge_request.unlabel="label1" -o merge_request.unlabel="label2"
+```
+
+You can also use these push options in addition to the
+`merge_request.create` push option.
+
+To create a merge request and add two labels to it, use:
+
+```sh
+git push -o merge_request.create -o merge_request.label="label1" -o merge_request.label="label2"
+```
+
## Find the merge request that introduced a change
> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/2383) in GitLab 10.5.
diff --git a/lib/gitlab/push_options.rb b/lib/gitlab/push_options.rb
index 682edfc4259..a2296d265cd 100644
--- a/lib/gitlab/push_options.rb
+++ b/lib/gitlab/push_options.rb
@@ -7,10 +7,12 @@ module Gitlab
keys: [
:create,
:description,
+ :label,
:merge_when_pipeline_succeeds,
:remove_source_branch,
:target,
- :title
+ :title,
+ :unlabel
]
},
ci: {
@@ -18,6 +20,11 @@ module Gitlab
}
}).freeze
+ MULTI_VALUE_OPTIONS = [
+ %w[merge_request label],
+ %w[merge_request unlabel]
+ ].freeze
+
NAMESPACE_ALIASES = HashWithIndifferentAccess.new({
mr: :merge_request
}).freeze
@@ -50,12 +57,22 @@ module Gitlab
next if [namespace, key].any?(&:nil?)
options[namespace] ||= HashWithIndifferentAccess.new
- options[namespace][key] = value
+
+ if option_multi_value?(namespace, key)
+ options[namespace][key] ||= HashWithIndifferentAccess.new(0)
+ options[namespace][key][value] += 1
+ else
+ options[namespace][key] = value
+ end
end
options
end
+ def option_multi_value?(namespace, key)
+ MULTI_VALUE_OPTIONS.any? { |arr| arr == [namespace, key] }
+ end
+
def parse_option(option)
parts = OPTION_MATCHER.match(option)
return unless parts
diff --git a/spec/services/merge_requests/push_options_handler_service_spec.rb b/spec/services/merge_requests/push_options_handler_service_spec.rb
index a27fea0c90f..ff4cdd3e7e2 100644
--- a/spec/services/merge_requests/push_options_handler_service_spec.rb
+++ b/spec/services/merge_requests/push_options_handler_service_spec.rb
@@ -13,6 +13,9 @@ describe MergeRequests::PushOptionsHandlerService do
let(:target_branch) { 'feature' }
let(:title) { 'my title' }
let(:description) { 'my description' }
+ let(:label1) { 'mylabel1' }
+ let(:label2) { 'mylabel2' }
+ let(:label3) { 'mylabel3' }
let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" }
let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" }
let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::BLANK_SHA} refs/heads/#{source_branch}" }
@@ -122,6 +125,16 @@ describe MergeRequests::PushOptionsHandlerService do
end
end
+ shared_examples_for 'a service that can change labels of a merge request' do |count|
+ subject(:last_mr) { MergeRequest.last }
+
+ it 'changes label count' do
+ service.execute
+
+ expect(last_mr.label_ids.count).to eq(count)
+ end
+ end
+
shared_examples_for 'a service that does not create a merge request' do
it do
expect { service.execute }.not_to change { MergeRequest.count }
@@ -504,6 +517,138 @@ describe MergeRequests::PushOptionsHandlerService do
end
end
+ describe '`label` push option' do
+ let(:push_options) { { label: { label1 => 1, label2 => 1 } } }
+
+ context 'with a new branch' do
+ let(:changes) { new_branch_changes }
+
+ it_behaves_like 'a service that does not create a merge request'
+
+ it 'adds an error to the service' do
+ error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
+
+ service.execute
+
+ expect(service.errors).to include(error)
+ end
+
+ context 'when coupled with the `create` push option' do
+ let(:push_options) { { create: true, label: { label1 => 1, label2 => 1 } } }
+
+ it_behaves_like 'a service that can create a merge request'
+ it_behaves_like 'a service that can change labels of a merge request', 2
+ end
+ end
+
+ context 'with an existing branch but no open MR' do
+ let(:changes) { existing_branch_changes }
+
+ it_behaves_like 'a service that does not create a merge request'
+
+ it 'adds an error to the service' do
+ error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
+
+ service.execute
+
+ expect(service.errors).to include(error)
+ end
+
+ context 'when coupled with the `create` push option' do
+ let(:push_options) { { create: true, label: { label1 => 1, label2 => 1 } } }
+
+ it_behaves_like 'a service that can create a merge request'
+ it_behaves_like 'a service that can change labels of a merge request', 2
+ end
+ end
+
+ context 'with an existing branch that has a merge request open' do
+ let(:changes) { existing_branch_changes }
+ let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch)}
+
+ it_behaves_like 'a service that does not create a merge request'
+ it_behaves_like 'a service that can change labels of a merge request', 2
+ end
+
+ context 'with a deleted branch' do
+ let(:changes) { deleted_branch_changes }
+
+ it_behaves_like 'a service that does nothing'
+ end
+
+ context 'with the project default branch' do
+ let(:changes) { default_branch_changes }
+
+ it_behaves_like 'a service that does nothing'
+ end
+ end
+
+ describe '`unlabel` push option' do
+ let(:push_options) { { label: { label1 => 1, label2 => 1 }, unlabel: { label1 => 1, label3 => 1 } } }
+
+ context 'with a new branch' do
+ let(:changes) { new_branch_changes }
+
+ it_behaves_like 'a service that does not create a merge request'
+
+ it 'adds an error to the service' do
+ error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
+
+ service.execute
+
+ expect(service.errors).to include(error)
+ end
+
+ context 'when coupled with the `create` push option' do
+ let(:push_options) { { create: true, label: { label1 => 1, label2 => 1 }, unlabel: { label1 => 1, label3 => 1 } } }
+
+ it_behaves_like 'a service that can create a merge request'
+ it_behaves_like 'a service that can change labels of a merge request', 1
+ end
+ end
+
+ context 'with an existing branch but no open MR' do
+ let(:changes) { existing_branch_changes }
+
+ it_behaves_like 'a service that does not create a merge request'
+
+ it 'adds an error to the service' do
+ error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
+
+ service.execute
+
+ expect(service.errors).to include(error)
+ end
+
+ context 'when coupled with the `create` push option' do
+ let(:push_options) { { create: true, label: { label1 => 1, label2 => 1 }, unlabel: { label1 => 1, label3 => 1 } } }
+
+ it_behaves_like 'a service that can create a merge request'
+ it_behaves_like 'a service that can change labels of a merge request', 1
+ end
+ end
+
+ context 'with an existing branch that has a merge request open' do
+ let(:changes) { existing_branch_changes }
+ let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch)}
+
+ it_behaves_like 'a service that does not create a merge request'
+ it_behaves_like 'a service that can change labels of a merge request', 1
+ end
+
+ context 'with a deleted branch' do
+ let(:changes) { deleted_branch_changes }
+
+ it_behaves_like 'a service that does nothing'
+ end
+
+ context 'with the project default branch' do
+ let(:changes) { default_branch_changes }
+
+ it_behaves_like 'a service that does nothing'
+ end
+ end
+
describe 'multiple pushed branches' do
let(:push_options) { { create: true } }
let(:changes) do