Commit 1454aa9f authored by Jarka Košanová's avatar Jarka Košanová Committed by Lin Jen-Shin

Fix adding labels to epics using quick actions

- check if an entity is project or group
- fetch labels based on that
parent 9b85d7c0
...@@ -456,6 +456,10 @@ class Note < ActiveRecord::Base ...@@ -456,6 +456,10 @@ class Note < ActiveRecord::Base
Upload.find_by(model: self, path: paths) Upload.find_by(model: self, path: paths)
end end
def parent
project
end
private private
def keep_around_commit def keep_around_commit
......
...@@ -32,7 +32,7 @@ module Notes ...@@ -32,7 +32,7 @@ module Notes
return if command_params.empty? return if command_params.empty?
return unless supported?(note) return unless supported?(note)
self.class.noteable_update_service(note).new(project, current_user, command_params).execute(note.noteable) self.class.noteable_update_service(note).new(note.parent, current_user, command_params).execute(note.noteable)
end end
end end
end end
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
module QuickActions module QuickActions
class InterpretService < BaseService class InterpretService < BaseService
include Gitlab::Utils::StrongMemoize
include Gitlab::QuickActions::Dsl include Gitlab::QuickActions::Dsl
attr_reader :issuable attr_reader :issuable
...@@ -209,15 +210,11 @@ module QuickActions ...@@ -209,15 +210,11 @@ module QuickActions
end end
params '~label1 ~"label 2"' params '~label1 ~"label 2"'
condition do condition do
if project parent = project || issuable_group
available_labels = LabelsFinder
.new(current_user, project_id: project.id, include_ancestor_groups: true)
.execute
end
project && parent &&
current_user.can?(:"admin_#{issuable.to_ability_name}", project) && current_user.can?(:"admin_#{issuable.to_ability_name}", parent) &&
available_labels.any? find_labels.any?
end end
command :label do |labels_param| command :label do |labels_param|
label_ids = find_label_ids(labels_param) label_ids = find_label_ids(labels_param)
...@@ -673,9 +670,23 @@ module QuickActions ...@@ -673,9 +670,23 @@ module QuickActions
MilestonesFinder.new(params.merge(project_ids: [project.id], group_ids: [project.group&.id])).execute MilestonesFinder.new(params.merge(project_ids: [project.id], group_ids: [project.group&.id])).execute
end end
def find_labels(labels_param) def issuable_group
extract_references(labels_param, :label) | strong_memoize(:issuable_group) do
LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split, include_ancestor_groups: true).execute issuable.group if issuable.respond_to?(:group)
end
end
def find_labels(label_references = nil)
labels_params = { include_ancestor_groups: true }
labels_params[:project_id] = project.id if project
labels_params[:group_id] = issuable_group.id if issuable_group
labels_params[:name] = label_references.split if label_references
result = LabelsFinder.new(current_user, labels_params).execute
return result unless label_references
extract_references(label_references, :label) | result
end end
def find_label_references(labels_param) def find_label_references(labels_param)
...@@ -708,7 +719,7 @@ module QuickActions ...@@ -708,7 +719,7 @@ module QuickActions
def extract_references(arg, type) def extract_references(arg, type)
ext = Gitlab::ReferenceExtractor.new(project, current_user) ext = Gitlab::ReferenceExtractor.new(project, current_user)
ext.analyze(arg, author: current_user) ext.analyze(arg, author: current_user, group: issuable_group)
ext.references(type) ext.references(type)
end end
......
...@@ -46,6 +46,11 @@ module EE ...@@ -46,6 +46,11 @@ module EE
for_epic? || super for_epic? || super
end end
override :parent
def parent
for_epic? ? noteable.group : super
end
private private
def banzai_context_params def banzai_context_params
......
---
title: Fix adding labels to epics using quick actions
merge_request: 8772
author:
type: fixed
...@@ -29,4 +29,13 @@ describe Note do ...@@ -29,4 +29,13 @@ describe Note do
end end
end end
end end
describe '#parent' do
it 'returns group for epic notes' do
group = create(:group)
note = create(:note_on_epic, noteable: create(:epic, group: group))
expect(note.parent).to eq(group)
end
end
end end
...@@ -10,10 +10,6 @@ describe Notes::QuickActionsService do ...@@ -10,10 +10,6 @@ describe Notes::QuickActionsService do
let(:service) { described_class.new(project, user) } let(:service) { described_class.new(project, user) }
before do
project.add_maintainer(user)
end
def execute(note) def execute(note)
content, command_params = service.extract_commands(note) content, command_params = service.extract_commands(note)
service.execute(command_params, note) service.execute(command_params, note)
...@@ -169,5 +165,37 @@ describe Notes::QuickActionsService do ...@@ -169,5 +165,37 @@ describe Notes::QuickActionsService do
end end
end end
end end
describe '/label' do
let(:project) { nil }
let!(:bug) { create(:group_label, title: 'bug', group: group)}
let!(:project_label) { create(:label, title: 'project_label', project: create(:project, group: group))}
let(:note_text) { "/label ~bug ~project_label" }
let(:note) { create(:note, noteable: epic, note: note_text) }
before do
group.add_developer(user)
end
context 'when epics are not enabled' do
it 'does not add a label to the epic' do
expect { execute(note) }.not_to change { epic.reload.labels }
end
end
context 'when epics are enabled' do
before do
stub_licensed_features(epics: true)
end
it 'adds a group label to the epic' do
expect { execute(note) }.to change { epic.reload.labels.map(&:title) }.to(['bug'])
end
it 'leaves the note empty' do
expect(execute(note)).to eq('')
end
end
end
end end
end end
...@@ -199,6 +199,51 @@ describe QuickActions::InterpretService do ...@@ -199,6 +199,51 @@ describe QuickActions::InterpretService do
end end
end end
context 'label command for epics' do
let(:epic) { create(:epic, group: group)}
let(:label) { create(:group_label, title: 'bug', group: group) }
let(:project_label) { create(:label, title: 'project_label') }
let(:content) { "/label ~#{label.title} ~#{project_label.title}" }
let(:service) { described_class.new(nil, current_user) }
context 'when epics are enabled' do
before do
stub_licensed_features(epics: true)
end
context 'when a user has permissions to label an epic' do
before do
group.add_developer(current_user)
end
it 'populates valid label ids' do
_, updates = service.execute(content, epic)
expect(updates).to eq(add_label_ids: [label.id])
end
end
context 'when a user does not have permissions to label an epic' do
it 'does not populate any lables' do
_, updates = service.execute(content, epic)
expect(updates).to be_empty
end
end
end
context 'when epics are disabled' do
it 'does not populate any lables' do
group.add_developer(current_user)
_, updates = service.execute(content, epic)
expect(updates).to be_empty
end
end
end
context 'remove_epic command' do context 'remove_epic command' do
let(:epic) { create(:epic, group: group)} let(:epic) { create(:epic, group: group)}
let(:content) { "/remove_epic #{epic.to_reference(project)}" } let(:content) { "/remove_epic #{epic.to_reference(project)}" }
......
...@@ -14,6 +14,7 @@ FactoryBot.define do ...@@ -14,6 +14,7 @@ FactoryBot.define do
factory :note_on_merge_request, traits: [:on_merge_request] factory :note_on_merge_request, traits: [:on_merge_request]
factory :note_on_project_snippet, traits: [:on_project_snippet] factory :note_on_project_snippet, traits: [:on_project_snippet]
factory :note_on_personal_snippet, traits: [:on_personal_snippet] factory :note_on_personal_snippet, traits: [:on_personal_snippet]
factory :note_on_epic, traits: [:on_epic]
factory :system_note, traits: [:system] factory :system_note, traits: [:system]
factory :discussion_note, class: DiscussionNote factory :discussion_note, class: DiscussionNote
...@@ -133,6 +134,11 @@ FactoryBot.define do ...@@ -133,6 +134,11 @@ FactoryBot.define do
project nil project nil
end end
trait :on_epic do
noteable { create(:epic) }
project nil
end
trait :system do trait :system do
system true system true
end end
......
...@@ -890,4 +890,19 @@ describe Note do ...@@ -890,4 +890,19 @@ describe Note do
end end
end end
end end
describe '#parent' do
it 'returns project for project notes' do
project = create(:project)
note = create(:note_on_issue, project: project)
expect(note.parent).to eq(project)
end
it 'returns nil for personal snippet note' do
note = create(:note_on_personal_snippet)
expect(note.parent).to be_nil
end
end
end end
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