Commit ec9cb6ed authored by 🙈  jacopo beschi 🙉's avatar 🙈 jacopo beschi 🙉 Committed by Douwe Maan
Browse files

Resolve "Milestone Quick Action not displayed with no project milestones but with group milestones"

parent bb0fe96f
......@@ -50,16 +50,7 @@ def commands(noteable, type)
return [] unless noteable&.is_a?(Issuable)
opts = {
project: project,
issuable: noteable,
current_user: current_user
}
QuickActions::InterpretService.command_definitions.map do |definition|
next unless definition.available?(opts)
definition.to_h(opts)
end.compact
QuickActions::InterpretService.new(project, current_user).available_commands(noteable)
end
end
end
......@@ -7,6 +7,18 @@ class InterpretService < BaseService
SHRUG = '¯\\_(ツ)_/¯'.freeze
TABLEFLIP = '(╯°□°)╯︵ ┻━┻'.freeze
# Takes an issuable and returns an array of all the available commands
# represented with .to_h
def available_commands(issuable)
@issuable = issuable
self.class.command_definitions.map do |definition|
next unless definition.available?(self)
definition.to_h(self)
end.compact
end
# Takes a text and interprets the commands that are extracted from it.
# Returns the content without commands, and hash of changes to be applied to a record.
def execute(content, issuable)
......@@ -15,8 +27,8 @@ def execute(content, issuable)
@issuable = issuable
@updates = {}
content, commands = extractor.extract_commands(content, context)
extract_updates(commands, context)
content, commands = extractor.extract_commands(content)
extract_updates(commands)
[content, @updates]
end
......@@ -28,8 +40,8 @@ def explain(content, issuable)
@issuable = issuable
content, commands = extractor.extract_commands(content, context)
commands = explain_commands(commands, context)
content, commands = extractor.extract_commands(content)
commands = explain_commands(commands)
[content, commands]
end
......@@ -157,11 +169,11 @@ def extractor
params '%"milestone"'
condition do
current_user.can?(:"admin_#{issuable.to_ability_name}", project) &&
project.milestones.active.any?
find_milestones(project, state: 'active').any?
end
parse_params do |milestone_param|
extract_references(milestone_param, :milestone).first ||
project.milestones.find_by(title: milestone_param.strip)
find_milestones(project, title: milestone_param.strip).first
end
command :milestone do |milestone|
@updates[:milestone_id] = milestone.id if milestone
......@@ -544,6 +556,10 @@ def extract_users(params)
users
end
def find_milestones(project, params = {})
MilestonesFinder.new(params.merge(project_ids: [project.id], group_ids: [project.group&.id])).execute
end
def find_labels(labels_param)
extract_references(labels_param, :label) |
LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split).execute
......@@ -557,21 +573,21 @@ def find_label_ids(labels_param)
find_labels(labels_param).map(&:id)
end
def explain_commands(commands, opts)
def explain_commands(commands)
commands.map do |name, arg|
definition = self.class.definition_by_name(name)
next unless definition
definition.explain(self, opts, arg)
definition.explain(self, arg)
end.compact
end
def extract_updates(commands, opts)
def extract_updates(commands)
commands.each do |name, arg|
definition = self.class.definition_by_name(name)
next unless definition
definition.execute(self, opts, arg)
definition.execute(self, arg)
end
end
......@@ -581,14 +597,5 @@ def extract_references(arg, type)
ext.references(type)
end
def context
{
issuable: issuable,
current_user: current_user,
project: project,
params: params
}
end
end
end
---
title: Allows the usage of /milestone quick action for group milestones
merge_request: 17239
author: Jacopo Beschi @jacopo-beschi
type: fixed
......@@ -24,15 +24,14 @@ def noop?
action_block.nil?
end
def available?(opts)
def available?(context)
return true unless condition_block
context = OpenStruct.new(opts)
context.instance_exec(&condition_block)
end
def explain(context, opts, arg)
return unless available?(opts)
def explain(context, arg)
return unless available?(context)
if explanation.respond_to?(:call)
execute_block(explanation, context, arg)
......@@ -41,15 +40,13 @@ def explain(context, opts, arg)
end
end
def execute(context, opts, arg)
return if noop? || !available?(opts)
def execute(context, arg)
return if noop? || !available?(context)
execute_block(action_block, context, arg)
end
def to_h(opts)
context = OpenStruct.new(opts)
def to_h(context)
desc = description
if desc.respond_to?(:call)
desc = context.instance_exec(&desc) rescue ''
......
......@@ -62,9 +62,8 @@ def explanation(text = '', &block)
# Allows to define conditions that must be met in order for the command
# to be returned by `.command_names` & `.command_definitions`.
# It accepts a block that will be evaluated with the context given to
# `CommandDefintion#to_h`.
#
# It accepts a block that will be evaluated with the context
# of a QuickActions::InterpretService instance
# Example:
#
# condition do
......
......@@ -29,7 +29,7 @@ def initialize(command_definitions)
# commands = extractor.extract_commands(msg) #=> [['labels', '~foo ~"bar baz"']]
# msg #=> "hello\nworld"
# ```
def extract_commands(content, opts = {})
def extract_commands(content)
return [content, []] unless content
content = content.dup
......@@ -37,7 +37,7 @@ def extract_commands(content, opts = {})
commands = []
content.delete!("\r")
content.gsub!(commands_regex(opts)) do
content.gsub!(commands_regex) do
if $~[:cmd]
commands << [$~[:cmd], $~[:arg]].reject(&:blank?)
''
......@@ -60,8 +60,8 @@ def extract_commands(content, opts = {})
# It looks something like:
#
# /^\/(?<cmd>close|reopen|...)(?:( |$))(?<arg>[^\/\n]*)(?:\n|$)/
def commands_regex(opts)
names = command_names(opts).map(&:to_s)
def commands_regex
names = command_names.map(&:to_s)
@commands_regex ||= %r{
(?<code>
......@@ -133,7 +133,7 @@ def perform_substitutions(content, commands)
[content, commands]
end
def command_names(opts)
def command_names
command_definitions.flat_map do |command|
next if command.noop?
......
......@@ -40,7 +40,7 @@
end
describe "#available?" do
let(:opts) { { go: false } }
let(:opts) { OpenStruct.new(go: false) }
context "when the command has a condition block" do
before do
......@@ -78,7 +78,7 @@
it "doesn't execute the command" do
expect(context).not_to receive(:instance_exec)
subject.execute(context, {}, nil)
subject.execute(context, nil)
expect(context.run).to be false
end
......@@ -95,7 +95,7 @@
end
it "doesn't execute the command" do
subject.execute(context, {}, nil)
subject.execute(context, nil)
expect(context.run).to be false
end
......@@ -109,7 +109,7 @@
context "when the command is provided an argument" do
it "executes the command" do
subject.execute(context, {}, true)
subject.execute(context, true)
expect(context.run).to be true
end
......@@ -117,7 +117,7 @@
context "when the command is not provided an argument" do
it "executes the command" do
subject.execute(context, {}, nil)
subject.execute(context, nil)
expect(context.run).to be true
end
......@@ -131,7 +131,7 @@
context "when the command is provided an argument" do
it "executes the command" do
subject.execute(context, {}, true)
subject.execute(context, true)
expect(context.run).to be true
end
......@@ -139,7 +139,7 @@
context "when the command is not provided an argument" do
it "doesn't execute the command" do
subject.execute(context, {}, nil)
subject.execute(context, nil)
expect(context.run).to be false
end
......@@ -153,7 +153,7 @@
context "when the command is provided an argument" do
it "executes the command" do
subject.execute(context, {}, true)
subject.execute(context, true)
expect(context.run).to be true
end
......@@ -161,7 +161,7 @@
context "when the command is not provided an argument" do
it "executes the command" do
subject.execute(context, {}, nil)
subject.execute(context, nil)
expect(context.run).to be true
end
......@@ -175,7 +175,7 @@
end
it 'executes the command passing the parsed param' do
subject.execute(context, {}, 'something ')
subject.execute(context, 'something ')
expect(context.received_arg).to eq('something')
end
......@@ -192,7 +192,7 @@
end
it 'returns nil' do
result = subject.explain({}, {}, nil)
result = subject.explain({}, nil)
expect(result).to be_nil
end
......@@ -204,7 +204,7 @@
end
it 'returns this static string' do
result = subject.explain({}, {}, nil)
result = subject.explain({}, nil)
expect(result).to eq 'Explanation'
end
......@@ -216,7 +216,7 @@
end
it 'invokes the proc' do
result = subject.explain({}, {}, 'explanation')
result = subject.explain({}, 'explanation')
expect(result).to eq 'Dynamic explanation'
end
......
......@@ -76,7 +76,7 @@
expect(dynamic_description_def.name).to eq(:dynamic_description)
expect(dynamic_description_def.aliases).to eq([])
expect(dynamic_description_def.to_h(noteable: 'issue')[:description]).to eq('A dynamic description for ISSUE')
expect(dynamic_description_def.to_h(OpenStruct.new(noteable: 'issue'))[:description]).to eq('A dynamic description for ISSUE')
expect(dynamic_description_def.explanation).to eq('')
expect(dynamic_description_def.params).to eq(['The first argument', 'The second argument'])
expect(dynamic_description_def.condition_block).to be_nil
......
......@@ -522,6 +522,22 @@
let(:issuable) { merge_request }
end
context 'only group milestones available' do
let(:group) { create(:group) }
let(:project) { create(:project, :public, namespace: group) }
let(:milestone) { create(:milestone, group: group, title: '10.0') }
it_behaves_like 'milestone command' do
let(:content) { "/milestone %#{milestone.title}" }
let(:issuable) { issue }
end
it_behaves_like 'milestone command' do
let(:content) { "/milestone %#{milestone.title}" }
let(:issuable) { merge_request }
end
end
it_behaves_like 'remove_milestone command' do
let(:content) { '/remove_milestone' }
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