Commit c5db79d6 authored by Stan Hu's avatar Stan Hu

Ignore nil labels with issuable creation

If a user provides `nil` as a parameter for `labels`, creating an issue,
merge request, or epic would fail with a 500 error. This happened
because Rails would attempt to iterate on the association.

This issue was exposed when we upgraded Grape in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/27276 because
`nil` values were not coerced to empty Arrays automatically.

`IssuableBaseService#label_ids_to_filter` attempts to map the `labels`
value to `label_ids` and clears out the `labels` key. However, this
isn't done if the `labels` value is `nil`.

To avoid this, we should always clear out the key after
`label_ids_to_filter` completes.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/215936
parent e48d161f
......@@ -91,6 +91,8 @@ class IssuableBaseService < BaseService
elsif params[label_key]
params[label_id_key] = labels_service.find_or_create_by_titles(label_key, find_only: find_only).map(&:id)
end
params.delete(label_key) if params[label_key].nil?
end
def filter_labels_in_param(key)
......
......@@ -110,6 +110,31 @@ describe Issues::CreateService do
end
end
context 'when labels is nil' do
let(:opts) do
{ title: 'Title',
description: 'Description',
labels: nil }
end
it 'does not assign label' do
expect(issue.labels).to be_empty
end
end
context 'when labels is nil and label_ids is present' do
let(:opts) do
{ title: 'Title',
description: 'Description',
labels: nil,
label_ids: labels.map(&:id) }
end
it 'assigns group labels' do
expect(issue.labels).to match_array labels
end
end
context 'when milestone belongs to different project' do
let(:milestone) { create(:milestone) }
......
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