summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJacopo <beschi.jacopo@gmail.com>2019-04-02 17:58:52 +0200
committerJacopo <beschi.jacopo@gmail.com>2019-04-02 18:28:25 +0200
commit3bc30185180cbb5d7c8e026f6e7f1826617358a3 (patch)
treeb3fc099c17666a35089479103f1e5f9a30d2126d
parent23c353515f9d2472a944a21cc1d9a3c89556dd48 (diff)
downloadgitlab-ce-53459-quick-action-adds-multiple-labels-to-issue-if-middle-words-overlap-with-existing-label.tar.gz
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.
-rw-r--r--app/services/quick_actions/interpret_service.rb19
-rw-r--r--changelogs/unreleased/53459-quick-action-adds-multiple-labels-to-issue-if-middle-words-overlap-with-existing-label.yml5
-rw-r--r--doc/user/project/quick_actions.md2
-rw-r--r--spec/services/quick_actions/interpret_service_spec.rb41
4 files changed, 63 insertions, 4 deletions
diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb
index f463e08ee7e..8ff73522e5f 100644
--- a/app/services/quick_actions/interpret_service.rb
+++ b/app/services/quick_actions/interpret_service.rb
@@ -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)
diff --git a/changelogs/unreleased/53459-quick-action-adds-multiple-labels-to-issue-if-middle-words-overlap-with-existing-label.yml b/changelogs/unreleased/53459-quick-action-adds-multiple-labels-to-issue-if-middle-words-overlap-with-existing-label.yml
new file mode 100644
index 00000000000..30d8c0e95d7
--- /dev/null
+++ b/changelogs/unreleased/53459-quick-action-adds-multiple-labels-to-issue-if-middle-words-overlap-with-existing-label.yml
@@ -0,0 +1,5 @@
+---
+title: Fix quick actions add label name middle word overlaps
+merge_request: 26602
+author: Jacopo Beschi @jacopo-beschi
+type: fixed
diff --git a/doc/user/project/quick_actions.md b/doc/user/project/quick_actions.md
index 392e72dcc5c..88f4de891a1 100644
--- a/doc/user/project/quick_actions.md
+++ b/doc/user/project/quick_actions.md
@@ -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 | ✓ | ✓ |
diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb
index 8b0f9c8ade2..c7e5cca324f 100644
--- a/spec/services/quick_actions/interpret_service_spec.rb
+++ b/spec/services/quick_actions/interpret_service_spec.rb
@@ -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 }