Commit 3bc30185 authored by Jacopo's avatar Jacopo

Fix quick actions add label name middle word overlaps

Fixes quick actions add label when adding a label which name middle
word overlaps with another label name: for example adding "A B C" when
also label "B" exists.
With the fix only the label "A B C" is correctly added, previously
also the label "B" was added due to the middle word overlaps.
parent 23c35351
......@@ -96,14 +96,27 @@ module QuickActions
end
def find_labels(labels_params = nil)
extract_references(labels_params, :label) | find_labels_by_name_no_tilde(labels_params)
end
def find_labels_by_name_no_tilde(labels_params)
return Label.none if label_with_tilde?(labels_params)
finder_params = { include_ancestor_groups: true }
finder_params[:project_id] = project.id if project
finder_params[:group_id] = group.id if group
finder_params[:name] = labels_params.split if labels_params
finder_params[:name] = extract_label_names(labels_params) if labels_params
result = LabelsFinder.new(current_user, finder_params).execute
LabelsFinder.new(current_user, finder_params).execute
end
def label_with_tilde?(labels_params)
labels_params&.include?('~')
end
extract_references(labels_params, :label) | result
def extract_label_names(labels_params)
# '"A" "A B C" A B' => ["A", "A B C", "A", "B"]
labels_params.scan(/"([^"]+)"|([^ ]+)/).flatten.compact
end
def find_label_references(labels_param)
......
---
title: Fix quick actions add label name middle word overlaps
merge_request: 26602
author: Jacopo Beschi @jacopo-beschi
type: fixed
......@@ -31,7 +31,7 @@ discussions, and descriptions:
| `/reassign @user1 @user2` | Change assignee | ✓ | ✓ |
| `/milestone %milestone` | Set milestone | ✓ | ✓ |
| `/remove_milestone` | Remove milestone | ✓ | ✓ |
| `/label ~label1 ~label2` | Add label(s) | ✓ | ✓ |
| `/label ~label1 ~label2` | Add label(s). Label names can also start without ~ but mixed syntax is not supported. | ✓ | ✓ |
| `/unlabel ~label1 ~label2` | Remove all or specific label(s)| ✓ | ✓ |
| `/relabel ~label1 ~label2` | Replace label | ✓ | ✓ |
| <code>/copy_metadata #issue &#124; !merge_request</code> | Copy labels and milestone from other issue or merge request | ✓ | ✓ |
......
......@@ -10,6 +10,7 @@ describe QuickActions::InterpretService do
let(:milestone) { create(:milestone, project: project, title: '9.10') }
let(:commit) { create(:commit, project: project) }
let(:inprogress) { create(:label, project: project, title: 'In Progress') }
let(:helmchart) { create(:label, project: project, title: 'Helm Chart Registry') }
let(:bug) { create(:label, project: project, title: 'Bug') }
let(:note) { build(:note, commit_id: merge_request.diff_head_sha) }
let(:service) { described_class.new(project, developer) }
......@@ -94,6 +95,26 @@ describe QuickActions::InterpretService do
end
end
shared_examples 'multiword label name starting without ~' do
it 'fetches label ids and populates add_label_ids if content contains /label' do
helmchart # populate the label
_, updates = service.execute(content, issuable)
expect(updates).to eq(add_label_ids: [helmchart.id])
end
end
shared_examples 'label name is included in the middle of another label name' do
it 'ignores the sublabel when the content contains the includer label name' do
helmchart # populate the label
create(:label, project: project, title: 'Chart')
_, updates = service.execute(content, issuable)
expect(updates).to eq(add_label_ids: [helmchart.id])
end
end
shared_examples 'unlabel command' do
it 'fetches label ids and populates remove_label_ids if content contains /unlabel' do
issuable.update!(label_ids: [inprogress.id]) # populate the label
......@@ -624,6 +645,26 @@ describe QuickActions::InterpretService do
let(:issuable) { issue }
end
it_behaves_like 'multiword label name starting without ~' do
let(:content) { %(/label "#{helmchart.title}") }
let(:issuable) { issue }
end
it_behaves_like 'multiword label name starting without ~' do
let(:content) { %(/label "#{helmchart.title}") }
let(:issuable) { merge_request }
end
it_behaves_like 'label name is included in the middle of another label name' do
let(:content) { %(/label ~"#{helmchart.title}") }
let(:issuable) { issue }
end
it_behaves_like 'label name is included in the middle of another label name' do
let(:content) { %(/label ~"#{helmchart.title}") }
let(:issuable) { merge_request }
end
it_behaves_like 'unlabel command' do
let(:content) { %(/unlabel ~"#{inprogress.title}") }
let(:issuable) { issue }
......
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