Skip to content

G
gitlab-ce
Commit 262575ce authored by Markus Koller's avatar Markus Koller
Browse files
Options

Merge branch 'support-label_ids-with-add_label_ids' into 'master' 

Add support for updating labels with quick actions

See merge request gitlab-org/gitlab!33699
parents dd27e4a9 57d0678b
Hide whitespace changes
Inline Side-by-side
Showing with 46 additions and 127 deletions
app/controllers/concerns/issuable_actions.rb
View file @ 262575ce
......@@ -16,19 +16,6 @@ module IssuableActions
end
end
def permitted_keys
[
:issuable_ids,
:assignee_id,
:milestone_id,
:state_event,
:subscription_event,
label_ids: [],
add_label_ids: [],
remove_label_ids: []
]
end
def show
respond_to do |format|
format.html do
......@@ -221,10 +208,20 @@ module IssuableActions
end
def bulk_update_params
permitted_keys_array = permitted_keys.dup
permitted_keys_array << { assignee_ids: [] }
params.require(:update).permit(bulk_update_permitted_keys)
end
params.require(:update).permit(permitted_keys_array)
def bulk_update_permitted_keys
[
:issuable_ids,
:assignee_id,
:milestone_id,
:state_event,
:subscription_event,
assignee_ids: [],
add_label_ids: [],
remove_label_ids: []
]
end
def resource_name
......
app/services/issuable/bulk_update_service.rb
View file @ 262575ce
......@@ -17,9 +17,8 @@ module Issuable
ids = params.delete(:issuable_ids).split(",")
items = find_issuables(parent, model_class, ids)
permitted_attrs(type).each do |key|
params.delete(key) unless params[key].present?
end
params.slice!(*permitted_attrs(type))
params.delete_if { |k, v| v.blank? }
if params[:assignee_ids] == [IssuableFinder::Params::NONE.to_s]
params[:assignee_ids] = []
......
app/services/issuable_base_service.rb
View file @ 262575ce
......@@ -129,15 +129,11 @@ class IssuableBaseService < BaseService
add_label_ids = attributes.delete(:add_label_ids)
remove_label_ids = attributes.delete(:remove_label_ids)
new_label_ids = existing_label_ids || label_ids || []
new_label_ids = label_ids || existing_label_ids || []
new_label_ids |= extra_label_ids
if add_label_ids.blank? && remove_label_ids.blank?
new_label_ids = label_ids if label_ids
else
new_label_ids |= add_label_ids if add_label_ids
new_label_ids -= remove_label_ids if remove_label_ids
end
new_label_ids |= add_label_ids if add_label_ids
new_label_ids -= remove_label_ids if remove_label_ids
new_label_ids.uniq
end
......
ee/app/controllers/concerns/ee/issuable_actions.rb
View file @ 262575ce
......@@ -9,8 +9,10 @@ module EE
weight
].freeze
override :permitted_keys
def permitted_keys
private
override :bulk_update_permitted_keys
def bulk_update_permitted_keys
@permitted_keys ||= (super + EE_PERMITTED_KEYS).freeze
end
end
......
spec/services/issuable/bulk_update_service_spec.rb
View file @ 262575ce
......@@ -42,13 +42,11 @@ describe Issuable::BulkUpdateService do
let(:issue_no_labels) { create(:issue, project: project) }
let(:issues) { [issue_all_labels, issue_bug_and_regression, issue_bug_and_merge_requests, issue_no_labels] }
let(:labels) { [] }
let(:add_labels) { [] }
let(:remove_labels) { [] }
let(:bulk_update_params) do
{
label_ids: labels.map(&:id),
add_label_ids: add_labels.map(&:id),
remove_label_ids: remove_labels.map(&:id)
}
......@@ -58,27 +56,6 @@ describe Issuable::BulkUpdateService do
bulk_update(issues, bulk_update_params)
end
context 'when label_ids are passed' do
let(:issues) { [issue_all_labels, issue_no_labels] }
let(:labels) { [bug, regression] }
it 'updates the labels of all issues passed to the labels passed' do
expect(issues.map(&:reload).map(&:label_ids)).to all(match_array(labels.map(&:id)))
end
it 'does not update issues not passed in' do
expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id)
end
context 'when those label IDs are empty' do
let(:labels) { [] }
it 'updates the issues passed to have no labels' do
expect(issues.map(&:reload).map(&:label_ids)).to all(be_empty)
end
end
end
context 'when add_label_ids are passed' do
let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] }
let(:add_labels) { [bug, regression, merge_requests] }
......@@ -122,69 +99,21 @@ describe Issuable::BulkUpdateService do
expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id)
end
end
end
context 'when add_label_ids and label_ids are passed' do
let(:issues) { [issue_all_labels, issue_bug_and_regression, issue_bug_and_merge_requests] }
let(:labels) { [merge_requests] }
let(:add_labels) { [regression] }
it 'adds the label IDs to all issues passed' do
expect(issues.map(&:reload).map(&:label_ids)).to all(include(regression.id))
end
it 'ignores the label IDs parameter' do
expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id))
end
it 'does not update issues not passed in' do
expect(issue_no_labels.label_ids).to be_empty
end
end
context 'when remove_label_ids and label_ids are passed' do
let(:issues) { [issue_no_labels, issue_bug_and_regression] }
let(:labels) { [merge_requests] }
let(:remove_labels) { [regression] }
it 'removes the label IDs from all issues passed' do
expect(issues.map(&:reload).flat_map(&:label_ids)).not_to include(regression.id)
end
it 'ignores the label IDs parameter' do
expect(issues.map(&:reload).flat_map(&:label_ids)).not_to include(merge_requests.id)
end
it 'does not update issues not passed in' do
expect(issue_all_labels.label_ids).to contain_exactly(bug.id, regression.id, merge_requests.id)
end
end
context 'when add_label_ids, remove_label_ids, and label_ids are passed' do
let(:issues) { [issue_bug_and_merge_requests, issue_no_labels] }
let(:labels) { [regression] }
let(:add_labels) { [bug] }
let(:remove_labels) { [merge_requests] }
it 'adds the label IDs to all issues passed' do
expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id))
end
context 'with issuables at a project level' do
let(:parent) { project }
it 'removes the label IDs from all issues passed' do
expect(issues.map(&:reload).flat_map(&:label_ids)).not_to include(merge_requests.id)
end
context 'with unpermitted attributes' do
let(:issues) { create_list(:issue, 2, project: project) }
let(:label) { create(:label, project: project) }
it 'ignores the label IDs parameter' do
expect(issues.map(&:reload).flat_map(&:label_ids)).not_to include(regression.id)
end
it 'does not update the issues' do
bulk_update(issues, label_ids: [label.id])
it 'does not update issues not passed in' do
expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id)
expect(issues.map(&:reload).map(&:label_ids)).not_to include(label.id)
end
end
end
context 'with issuables at a project level' do
let(:parent) { project }
describe 'close issues' do
let(:issues) { create_list(:issue, 2, project: project) }
......
spec/services/issues/update_service_spec.rb
View file @ 262575ce
......@@ -3,13 +3,13 @@
require 'spec_helper'
describe Issues::UpdateService, :mailer do
let(:user) { create(:user) }
let(:user2) { create(:user) }
let(:user3) { create(:user) }
let(:group) { create(:group, :public) }
let(:project) { create(:project, :repository, group: group) }
let(:label) { create(:label, project: project) }
let(:label2) { create(:label) }
let_it_be(:user) { create(:user) }
let_it_be(:user2) { create(:user) }
let_it_be(:user3) { create(:user) }
let_it_be(:group) { create(:group, :public) }
let_it_be(:project, reload: true) { create(:project, :repository, group: group) }
let_it_be(:label) { create(:label, project: project) }
let_it_be(:label2) { create(:label, project: project) }
let(:issue) do
create(:issue, title: 'Old title',
......@@ -19,7 +19,7 @@ describe Issues::UpdateService, :mailer do
author: create(:user))
end
before do
before_all do
project.add_maintainer(user)
project.add_developer(user2)
project.add_developer(user3)
......@@ -669,28 +669,24 @@ describe Issues::UpdateService, :mailer do
context 'when add_label_ids and label_ids are passed' do
let(:params) { { label_ids: [label.id], add_label_ids: [label3.id] } }
it 'ignores the label_ids parameter' do
expect(result.label_ids).not_to include(label.id)
before do
issue.update(labels: [label2])
end
it 'adds the passed labels' do
expect(result.label_ids).to include(label3.id)
it 'replaces the labels with the ones in label_ids and adds those in add_label_ids' do
expect(result.label_ids).to contain_exactly(label.id, label3.id)
end
end
context 'when remove_label_ids and label_ids are passed' do
let(:params) { { label_ids: [], remove_label_ids: [label.id] } }
let(:params) { { label_ids: [label.id, label2.id, label3.id], remove_label_ids: [label.id] } }
before do
issue.update(labels: [label, label3])
end
it 'ignores the label_ids parameter' do
expect(result.label_ids).not_to be_empty
end
it 'removes the passed labels' do
expect(result.label_ids).not_to include(label.id)
it 'replaces the labels with the ones in label_ids and removes those in remove_label_ids' do
expect(result.label_ids).to contain_exactly(label2.id, label3.id)
end
end
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7