Commit 4f1a4b8f authored by Eugenia Grieff's avatar Eugenia Grieff

Remove unused mothod in BulkUpdateService

Remove needless changelog file
Refactor specs to remove unnecessary assigment
parent 7401c7a5
...@@ -15,7 +15,7 @@ module Issuable ...@@ -15,7 +15,7 @@ module Issuable
update_class = type.classify.pluralize.constantize::UpdateService update_class = type.classify.pluralize.constantize::UpdateService
ids = params.delete(:issuable_ids).split(",") ids = params.delete(:issuable_ids).split(",")
items = find_issuables(model_class, ids, parent) items = find_issuables(parent, model_class, ids)
permitted_attrs(type).each do |key| permitted_attrs(type).each do |key|
params.delete(key) unless params[key].present? params.delete(key) unless params[key].present?
...@@ -49,27 +49,14 @@ module Issuable ...@@ -49,27 +49,14 @@ module Issuable
end end
end end
def valid_parent?(type, issuing_parent, parent) def find_issuables(parent, model_class, ids)
return true unless parent.class.name == 'Group'
issuing_parents =
if type == "issue" || type == "merge_request"
issuing_parent&.group&.self_and_descendants
else
issuing_parent&.self_and_descendants
end
issuing_parents.include?(parent)
end
# rubocop: disable CodeReuse/ActiveRecord
def find_issuables(model_class, ids, parent)
if parent.is_a?(Project) if parent.is_a?(Project)
model_class.where(id: ids).where(project_id: parent) model_class.id_in(ids).of_projects(parent)
elsif parent.is_a?(Group) elsif parent.is_a?(Group)
model_class.where(id: ids).where(project_id: parent.all_projects) model_class.id_in(ids).of_projects(parent.all_projects)
end end
end end
# rubocop: enable CodeReuse/ActiveRecord
end end
end end
Issuable::BulkUpdateService.prepend_if_ee('EE::Issuable::BulkUpdateService')
---
title: When bulk editing group issuables, verify that they belong to the correct group.
merge_request:
author:
type: fixed
...@@ -7,18 +7,12 @@ module EE ...@@ -7,18 +7,12 @@ module EE
private private
# rubocop: disable CodeReuse/ActiveRecord
override :find_issuables override :find_issuables
def find_issuables(model_class, ids, parent) def find_issuables(parent, model_class, ids)
if model_class.method_defined?("group") && parent.is_a?(Group) return model_class.for_ids(ids).in_selected_groups(parent.self_and_descendants) if model_class == ::Epic
model_class.where(id: ids).where(group_id: parent.self_and_descendants)
else
super super
end end
end end
# rubocop: enable CodeReuse/ActiveRecord
end
end end
end end
Issuable::BulkUpdateService.prepend_if_ee('EE::Issuable::BulkUpdateService')
...@@ -33,10 +33,8 @@ describe Issuable::BulkUpdateService do ...@@ -33,10 +33,8 @@ describe Issuable::BulkUpdateService do
context 'when epics are enabled' do context 'when epics are enabled' do
it 'updates epic labels' do it 'updates epic labels' do
result = subject expect(subject[:success]).to be_truthy
expect(subject[:count]).to eq(issuables.count)
expect(result[:success]).to be_truthy
expect(result[:count]).to eq(issuables.count)
issuables.each do |issuable| issuables.each do |issuable|
expect(issuable.reload.labels).to eq([label2, label3]) expect(issuable.reload.labels).to eq([label2, label3])
...@@ -57,18 +55,17 @@ describe Issuable::BulkUpdateService do ...@@ -57,18 +55,17 @@ describe Issuable::BulkUpdateService do
end end
context 'when issuable_ids contain external epics' do context 'when issuable_ids contain external epics' do
it 'updates epics that belong to the parent group or descendants' do let(:epic3) { create(:epic, group: create(:group, parent: group), labels: [label1]) }
epic3 = create(:epic, labels: [label1]) let(:outer_epic) { create(:epic, labels: [label1]) }
epic4 = create(:epic, group: create(:group, parent: group), labels: [label1]) let(:params) { { issuable_ids: [epic1.id, epic3.id, outer_epic.id], add_label_ids: [label3.id] } }
params = { issuable_ids: [epic1.id, epic3.id, epic4.id], add_label_ids: [label3.id] }
result = described_class.new(group, user, params).execute('epic')
expect(result[:success]).to be_truthy it 'updates epics that belong to the parent group or descendants' do
expect(result[:count]).to eq(2) expect(subject[:success]).to be_truthy
expect(subject[:count]).to eq(2)
expect(epic1.reload.labels).to eq([label1, label3]) expect(epic1.reload.labels).to eq([label1, label3])
expect(epic3.reload.labels).to eq([label1]) expect(epic3.reload.labels).to eq([label1, label3])
expect(epic4.reload.labels).to eq([label1, label3]) expect(outer_epic.reload.labels).to eq([label1])
end end
end end
end end
......
...@@ -338,8 +338,8 @@ describe Issuable::BulkUpdateService do ...@@ -338,8 +338,8 @@ describe Issuable::BulkUpdateService do
end end
end end
describe 'updating issuables from external project' do describe 'updating issues from external project' do
it 'updates issuables that belong to the parent project' do it 'updates only issues that belong to the parent project' do
issue1 = create(:issue, project: project) issue1 = create(:issue, project: project)
issue2 = create(:issue, project: create(:project)) issue2 = create(:issue, project: create(:project))
result = bulk_update([issue1, issue2], assignee_ids: [user.id]) result = bulk_update([issue1, issue2], assignee_ids: [user.id])
...@@ -395,8 +395,8 @@ describe Issuable::BulkUpdateService do ...@@ -395,8 +395,8 @@ describe Issuable::BulkUpdateService do
it_behaves_like 'updating labels' it_behaves_like 'updating labels'
end end
describe 'updating issuables from external group' do describe 'with issues from external group' do
it 'updates issuables that belong to the parent group or descendants' do it 'updates issues that belong to the parent group or descendants' do
issue1 = create(:issue, project: create(:project, group: group)) issue1 = create(:issue, project: create(:project, group: group))
issue2 = create(:issue, project: create(:project, group: create(:group))) issue2 = create(:issue, project: create(:project, group: create(:group)))
issue3 = create(:issue, project: create(:project, group: create(:group, parent: group))) issue3 = create(:issue, project: create(:project, group: create(:group, parent: group)))
......
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