Commit 921d3a8c authored by Eugenia Grieff's avatar Eugenia Grieff

Add parent arguments to bulk update service

Require issuables parent when initializing
BulkUpdateService. This parent then can be
used to verify that the collection of issuable ids
 belogs to the Group that is being bulk edited
parent 5cd237e8
......@@ -121,7 +121,7 @@ module IssuableActions
end
def bulk_update
result = Issuable::BulkUpdateService.new(current_user, bulk_update_params).execute(resource_name)
result = Issuable::BulkUpdateService.new(parent, current_user, bulk_update_params).execute(resource_name)
quantity = result[:count]
render json: { notice: "#{quantity} #{resource_name.pluralize(quantity)} updated" }
......
......@@ -4,10 +4,10 @@ module Issuable
class BulkUpdateService
include Gitlab::Allowable
attr_accessor :current_user, :params
attr_accessor :parent, :current_user, :params
def initialize(user = nil, params = {})
@current_user, @params = user, params.dup
def initialize(parent, user = nil, params = {})
@parent, @current_user, @params = parent, user, params.dup
end
# rubocop: disable CodeReuse/ActiveRecord
......@@ -27,7 +27,8 @@ module Issuable
end
items.each do |issuable|
next unless can?(current_user, :"update_#{type}", issuable)
next unless can?(current_user, :"update_#{type}", issuable) &&
valid_parent?(type, issuable.issuing_parent, parent)
update_class.new(issuable.issuing_parent, current_user, params).execute(issuable)
end
......@@ -50,5 +51,18 @@ module Issuable
attrs.push(:assignee_id)
end
end
def valid_parent?(type, issuing_parent, parent)
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
end
end
---
title: When bulk editing group issuables, verify that they belong to the correct group.
merge_request:
author:
type: fixed
......@@ -7,7 +7,7 @@ describe Issuable::BulkUpdateService do
let(:group) { create(:group) }
context 'with epics' do
subject { described_class.new(user, params).execute('epic') }
subject { described_class.new(group, user, params).execute('epic') }
let(:epic1) { create(:epic, group: group, labels: [label1]) }
let(:epic2) { create(:epic, group: group, labels: [label1]) }
......
......@@ -11,7 +11,7 @@ describe Issuable::BulkUpdateService do
.reverse_merge(issuable_ids: Array(issuables).map(&:id).join(','))
type = Array(issuables).first.model_name.param_key
Issuable::BulkUpdateService.new(user, bulk_update_params).execute(type)
Issuable::BulkUpdateService.new(parent, user, bulk_update_params).execute(type)
end
shared_examples 'updates milestones' do
......@@ -184,6 +184,8 @@ describe Issuable::BulkUpdateService do
end
context 'with issuables at a project level' do
let(:parent) { project }
describe 'close issues' do
let(:issues) { create_list(:issue, 2, project: project) }
......@@ -366,6 +368,7 @@ describe Issuable::BulkUpdateService do
context 'with issuables at a group level' do
let(:group) { create(:group) }
let(:parent) { group }
describe 'updating milestones' do
let(:milestone) { create(:milestone, group: 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