From 3bc30185180cbb5d7c8e026f6e7f1826617358a3 Mon Sep 17 00:00:00 2001 From: Jacopo 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 - extract_references(labels_params, :label) | result + def label_with_tilde?(labels_params) + labels_params&.include?('~') + end + + 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 | ✓ | ✓ | | /copy_metadata #issue | !merge_request | 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 }