From 3bc30185180cbb5d7c8e026f6e7f1826617358a3 Mon Sep 17 00:00:00 2001
From: Jacopo <beschi.jacopo@gmail.com>
Date: Tue, 2 Apr 2019 17:58:52 +0200
Subject: [PATCH] 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.
---
 .../quick_actions/interpret_service.rb        | 19 +++++++--
 ...ddle-words-overlap-with-existing-label.yml |  5 +++
 doc/user/project/quick_actions.md             |  2 +-
 .../quick_actions/interpret_service_spec.rb   | 41 +++++++++++++++++++
 4 files changed, 63 insertions(+), 4 deletions(-)
 create mode 100644 changelogs/unreleased/53459-quick-action-adds-multiple-labels-to-issue-if-middle-words-overlap-with-existing-label.yml

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 }
-- 
2.30.9