Fix specs and implement fixes based on failing specs
This commit is contained in:
parent
e07c27fee4
commit
8b8a4626c6
11 changed files with 75 additions and 84 deletions
|
@ -137,13 +137,13 @@ class ProjectsController < Projects::ApplicationController
|
|||
noteable =
|
||||
case params[:type]
|
||||
when 'Issue'
|
||||
IssuesFinder.new(current_user, project_id: project.id, state: 'all').
|
||||
IssuesFinder.new(current_user, project_id: @project.id, state: 'all').
|
||||
execute.find_by(iid: params[:type_id])
|
||||
when 'MergeRequest'
|
||||
MergeRequestsFinder.new(current_user, project_id: project.id, state: 'all').
|
||||
MergeRequestsFinder.new(current_user, project_id: @project.id, state: 'all').
|
||||
execute.find_by(iid: params[:type_id])
|
||||
when 'Commit'
|
||||
project.commit(params[:type_id])
|
||||
@project.commit(params[:type_id])
|
||||
else
|
||||
nil
|
||||
end
|
||||
|
|
|
@ -80,13 +80,16 @@ class IssuableBaseService < BaseService
|
|||
params[key] = project.labels.where(id: params[key]).pluck(:id)
|
||||
end
|
||||
|
||||
def process_label_ids(attributes, base_label_ids: [], merge_all: false)
|
||||
def process_label_ids(attributes, existing_label_ids: [])
|
||||
label_ids = attributes.delete(:label_ids)
|
||||
add_label_ids = attributes.delete(:add_label_ids)
|
||||
remove_label_ids = attributes.delete(:remove_label_ids)
|
||||
|
||||
new_label_ids = base_label_ids
|
||||
new_label_ids = label_ids if label_ids && (merge_all || (add_label_ids.blank? && remove_label_ids.blank?))
|
||||
new_label_ids = existing_label_ids
|
||||
|
||||
override_existing = new_label_ids.empty? || (add_label_ids.blank? && remove_label_ids.blank?)
|
||||
new_label_ids = label_ids if label_ids && override_existing
|
||||
|
||||
new_label_ids |= add_label_ids if add_label_ids
|
||||
new_label_ids -= remove_label_ids if remove_label_ids
|
||||
|
||||
|
@ -103,14 +106,8 @@ class IssuableBaseService < BaseService
|
|||
params.merge!(command_params)
|
||||
end
|
||||
|
||||
def create_issuable(issuable, attributes)
|
||||
def create_issuable(issuable, attributes, label_ids:)
|
||||
issuable.with_transaction_returning_status do
|
||||
attributes.delete(:state_event)
|
||||
params[:author] ||= current_user
|
||||
label_ids = process_label_ids(attributes, merge_all: true)
|
||||
|
||||
issuable.assign_attributes(attributes)
|
||||
|
||||
if issuable.save
|
||||
issuable.update_attributes(label_ids: label_ids)
|
||||
end
|
||||
|
@ -121,8 +118,16 @@ class IssuableBaseService < BaseService
|
|||
merge_slash_commands_into_params!(issuable)
|
||||
filter_params
|
||||
|
||||
if params.present? && create_issuable(issuable, params)
|
||||
handle_creation(issuable)
|
||||
params.delete(:state_event)
|
||||
params[:author] ||= current_user
|
||||
label_ids = process_label_ids(params)
|
||||
|
||||
issuable.assign_attributes(params)
|
||||
|
||||
before_create(issuable)
|
||||
|
||||
if params.present? && create_issuable(issuable, params, label_ids: label_ids)
|
||||
after_create(issuable)
|
||||
issuable.create_cross_references!(current_user)
|
||||
execute_hooks(issuable)
|
||||
end
|
||||
|
@ -130,10 +135,16 @@ class IssuableBaseService < BaseService
|
|||
issuable
|
||||
end
|
||||
|
||||
def before_create(issuable)
|
||||
# To be overridden by subclasses
|
||||
end
|
||||
|
||||
def after_create(issuable)
|
||||
# To be overridden by subclasses
|
||||
end
|
||||
|
||||
def update_issuable(issuable, attributes)
|
||||
issuable.with_transaction_returning_status do
|
||||
attributes[:label_ids] = process_label_ids(attributes, base_label_ids: issuable.label_ids)
|
||||
|
||||
issuable.update(attributes.merge(updated_by: current_user))
|
||||
end
|
||||
end
|
||||
|
@ -145,6 +156,8 @@ class IssuableBaseService < BaseService
|
|||
filter_params
|
||||
old_labels = issuable.labels.to_a
|
||||
|
||||
params[:label_ids] = process_label_ids(params, existing_label_ids: issuable.label_ids)
|
||||
|
||||
if params.present? && update_issuable(issuable, params)
|
||||
issuable.reset_events_cache
|
||||
handle_common_system_notes(issuable, old_labels: old_labels)
|
||||
|
|
|
@ -5,12 +5,15 @@ module Issues
|
|||
@api = params.delete(:api)
|
||||
|
||||
@issue = project.issues.new
|
||||
@issue.spam = spam_service.check(@api)
|
||||
|
||||
create(@issue)
|
||||
end
|
||||
|
||||
def handle_creation(issuable)
|
||||
def before_create(issuable)
|
||||
issuable.spam = spam_service.check(@api)
|
||||
end
|
||||
|
||||
def after_create(issuable)
|
||||
event_service.open_issue(issuable, current_user)
|
||||
notification_service.new_issue(issuable, current_user)
|
||||
todo_service.new_issue(issuable, current_user)
|
||||
|
|
|
@ -16,7 +16,7 @@ module MergeRequests
|
|||
create(merge_request)
|
||||
end
|
||||
|
||||
def handle_creation(issuable)
|
||||
def after_create(issuable)
|
||||
event_service.open_mr(issuable, current_user)
|
||||
notification_service.new_merge_request(issuable, current_user)
|
||||
todo_service.new_merge_request(issuable, current_user)
|
||||
|
|
|
@ -29,7 +29,7 @@ module Projects
|
|||
|
||||
opts = {
|
||||
project: project,
|
||||
noteable: noteable,
|
||||
issuable: noteable,
|
||||
current_user: current_user
|
||||
}
|
||||
SlashCommands::InterpretService.command_definitions.map do |definition|
|
||||
|
|
|
@ -4,8 +4,8 @@ module SlashCommands
|
|||
|
||||
attr_reader :issuable
|
||||
|
||||
# Takes a text and interpret the commands that are extracted from it.
|
||||
# Returns a hash of changes to be applied to a record.
|
||||
# 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)
|
||||
@issuable = issuable
|
||||
@updates = {}
|
||||
|
|
|
@ -4,7 +4,8 @@ Slash commands are textual shortcuts for common actions on issues or merge
|
|||
requests that are usually done by clicking buttons or dropdowns in GitLab's UI.
|
||||
You can enter these commands while creating a new issue or merge request, and
|
||||
in comments. Each command should be on a separate line in order to be properly
|
||||
detected and executed.
|
||||
detected and executed. The commands are removed from the issue, merge request or
|
||||
comment body before it is saved and will not be visible as such to anyone else.
|
||||
|
||||
Here is a list of all of the available commands and descriptions about what they
|
||||
do.
|
||||
|
|
|
@ -3,8 +3,14 @@ module Gitlab
|
|||
class CommandDefinition
|
||||
attr_accessor :name, :aliases, :description, :params, :condition_block, :action_block
|
||||
|
||||
def initialize(name)
|
||||
def initialize(name, attributes = {})
|
||||
@name = name
|
||||
|
||||
@aliases = attributes[:aliases] || []
|
||||
@description = attributes[:description] || ''
|
||||
@params = attributes[:params] || []
|
||||
@condition_block = attributes[:condition_block]
|
||||
@action_block = attributes[:action_block]
|
||||
end
|
||||
|
||||
def all_names
|
||||
|
|
|
@ -17,7 +17,7 @@ module Gitlab
|
|||
# Allows to give a description to the next slash command.
|
||||
# This description is shown in the autocomplete menu.
|
||||
# It accepts a block that will be evaluated with the context given to
|
||||
# `.command_definitions` or `.command_names`.
|
||||
# `CommandDefintion#to_h`.
|
||||
#
|
||||
# Example:
|
||||
#
|
||||
|
@ -47,7 +47,7 @@ module Gitlab
|
|||
# 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
|
||||
# `.command_definitions`, `.command_names`, and the actual command method.
|
||||
# `CommandDefintion#to_h`.
|
||||
#
|
||||
# Example:
|
||||
#
|
||||
|
@ -73,12 +73,14 @@ module Gitlab
|
|||
def command(*command_names, &block)
|
||||
name, *aliases = command_names
|
||||
|
||||
definition = CommandDefinition.new(name)
|
||||
definition.aliases = aliases
|
||||
definition.description = @description || ''
|
||||
definition.params = @params || []
|
||||
definition.condition_block = @condition_block
|
||||
definition.action_block = block
|
||||
definition = CommandDefinition.new(
|
||||
name,
|
||||
aliases: aliases,
|
||||
description: @description,
|
||||
params: @params,
|
||||
condition_block: @condition_block,
|
||||
action_block: block
|
||||
)
|
||||
|
||||
self.command_definitions << definition
|
||||
|
||||
|
|
|
@ -10,11 +10,6 @@ describe Gitlab::SlashCommands::Dsl do
|
|||
"Hello World!"
|
||||
end
|
||||
|
||||
desc { "A command with #{something}" }
|
||||
command :returning do
|
||||
42
|
||||
end
|
||||
|
||||
params 'The first argument'
|
||||
command :one_arg, :once, :first do |arg1|
|
||||
arg1
|
||||
|
@ -44,43 +39,8 @@ describe Gitlab::SlashCommands::Dsl do
|
|||
end
|
||||
|
||||
describe '.command_definitions' do
|
||||
let(:base_expected) do
|
||||
[
|
||||
{
|
||||
name: :no_args, aliases: [:none],
|
||||
description: 'A command with no args', params: [],
|
||||
condition_block: nil, action_block: a_kind_of(Proc)
|
||||
},
|
||||
{
|
||||
name: :returning, aliases: [],
|
||||
description: 'A command returning a value', params: [],
|
||||
condition_block: nil, action_block: a_kind_of(Proc)
|
||||
},
|
||||
{
|
||||
name: :one_arg, aliases: [:once, :first],
|
||||
description: '', params: ['The first argument'],
|
||||
condition_block: nil, action_block: a_kind_of(Proc)
|
||||
},
|
||||
{
|
||||
name: :two_args, aliases: [],
|
||||
description: '', params: ['The first argument', 'The second argument'],
|
||||
condition_block: nil, action_block: a_kind_of(Proc)
|
||||
},
|
||||
{
|
||||
name: :cc, aliases: [],
|
||||
description: '', params: [],
|
||||
condition_block: nil, action_block: nil
|
||||
},
|
||||
{
|
||||
name: :wildcard, aliases: [],
|
||||
description: '', params: [],
|
||||
condition_block: nil, action_block: a_kind_of(Proc)
|
||||
}
|
||||
]
|
||||
end
|
||||
|
||||
it 'returns an array with commands definitions' do
|
||||
no_args_def, returning_def, one_arg_def, two_args_def, cc_def, cond_action_def, wildcard_def = DummyClass.command_definitions
|
||||
no_args_def, one_arg_def, two_args_def, cc_def, cond_action_def, wildcard_def = DummyClass.command_definitions
|
||||
|
||||
expect(no_args_def.name).to eq(:no_args)
|
||||
expect(no_args_def.aliases).to eq([:none])
|
||||
|
@ -89,14 +49,6 @@ describe Gitlab::SlashCommands::Dsl do
|
|||
expect(no_args_def.condition_block).to be_nil
|
||||
expect(no_args_def.action_block).to be_a_kind_of(Proc)
|
||||
|
||||
expect(returning_def.name).to eq(:returning)
|
||||
expect(returning_def.aliases).to eq([])
|
||||
expect(returning_def.description).to be_a_kind_of(Proc)
|
||||
expect(returning_def.to_h(something: "a block description")[:description]).to eq('A command with a block description')
|
||||
expect(returning_def.params).to eq([])
|
||||
expect(returning_def.condition_block).to be_nil
|
||||
expect(returning_def.action_block).to be_a_kind_of(Proc)
|
||||
|
||||
expect(one_arg_def.name).to eq(:one_arg)
|
||||
expect(one_arg_def.aliases).to eq([:once, :first])
|
||||
expect(one_arg_def.description).to eq('')
|
||||
|
@ -104,6 +56,13 @@ describe Gitlab::SlashCommands::Dsl do
|
|||
expect(one_arg_def.condition_block).to be_nil
|
||||
expect(one_arg_def.action_block).to be_a_kind_of(Proc)
|
||||
|
||||
expect(two_args_def.name).to eq(:two_args)
|
||||
expect(two_args_def.aliases).to eq([])
|
||||
expect(two_args_def.to_h(noteable: "issue")[:description]).to eq('A dynamic description for ISSUE')
|
||||
expect(two_args_def.params).to eq(['The first argument', 'The second argument'])
|
||||
expect(two_args_def.condition_block).to be_nil
|
||||
expect(two_args_def.action_block).to be_a_kind_of(Proc)
|
||||
|
||||
expect(cc_def.name).to eq(:cc)
|
||||
expect(cc_def.aliases).to eq([])
|
||||
expect(cc_def.description).to eq('')
|
||||
|
@ -111,6 +70,13 @@ describe Gitlab::SlashCommands::Dsl do
|
|||
expect(cc_def.condition_block).to be_nil
|
||||
expect(cc_def.action_block).to be_nil
|
||||
|
||||
expect(cond_action_def.name).to eq(:cond_action)
|
||||
expect(cond_action_def.aliases).to eq([])
|
||||
expect(cond_action_def.description).to eq('')
|
||||
expect(cond_action_def.params).to eq([])
|
||||
expect(cond_action_def.condition_block).to be_a_kind_of(Proc)
|
||||
expect(cond_action_def.action_block).to be_a_kind_of(Proc)
|
||||
|
||||
expect(wildcard_def.name).to eq(:wildcard)
|
||||
expect(wildcard_def.aliases).to eq([])
|
||||
expect(wildcard_def.description).to eq('')
|
||||
|
|
|
@ -28,7 +28,7 @@ shared_examples 'issuable record that supports slash commands in its description
|
|||
|
||||
issuable = project.public_send(issuable_type.to_s.pluralize).first
|
||||
|
||||
expect(issuable.description).to eq "bug description\n"
|
||||
expect(issuable.description).to eq "bug description"
|
||||
expect(issuable.labels).to eq [label_bug]
|
||||
expect(issuable.milestone).to eq milestone
|
||||
expect(page).to have_content 'bug 345'
|
||||
|
@ -57,7 +57,7 @@ shared_examples 'issuable record that supports slash commands in its description
|
|||
issuable.reload
|
||||
note = issuable.notes.user.first
|
||||
|
||||
expect(note.note).to eq "Awesome!\n"
|
||||
expect(note.note).to eq "Awesome!"
|
||||
expect(issuable.assignee).to eq assignee
|
||||
expect(issuable.labels).to eq [label_bug]
|
||||
expect(issuable.milestone).to eq milestone
|
||||
|
|
Loading…
Reference in a new issue