Skip to content
Snippets Groups Projects
Commit 0eea8c88 authored by Rémy Coutable's avatar Rémy Coutable
Browse files

Support slash commands in noteable description and notes


Some important things to note:

- commands are removed from noteable.description / note.note
- commands are translated to params so that they are treated as normal
  params in noteable Creation services
- the logic is not in the models but in the Creation services, which is
  the right place for advanced logic that has nothing to do with what
  models should be responsible of!
- UI/JS needs to be updated to handle notes which consist of commands
  only
- the `/merge` command is not handled yet

Other improvements:

- Don't process commands in commit notes and display a flash is note is only commands
- Add autocomplete for slash commands
- Add description and params to slash command DSL methods
- Ensure replying by email with a commands-only note works
- Use :subscription_event instead of calling noteable.subscribe
- Support :todo_event in IssuableBaseService

Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 11eefba8
No related branches found
No related tags found
No related merge requests found
Showing
with 602 additions and 50 deletions
Loading
Loading
@@ -53,6 +53,7 @@ v 8.11.0 (unreleased)
- Update version_sorter and use new interface for faster tag sorting
- Optimize checking if a user has read access to a list of issues !5370
- Store all DB secrets in secrets.yml, under descriptive names !5274
- Support slash commands in issue and merge request descriptions as well as comments. !5021
- Nokogiri's various parsing methods are now instrumented
- Add simple identifier to public SSH keys (muteor)
- Admin page now references docs instead of a specific file !5600 (AnAverageHuman)
Loading
Loading
Loading
Loading
@@ -223,7 +223,7 @@
}
}
});
return this.input.atwho({
this.input.atwho({
at: '~',
alias: 'labels',
searchKey: 'search',
Loading
Loading
@@ -249,6 +249,41 @@
}
}
});
return this.input.atwho({
at: '/',
alias: 'commands',
displayTpl: function(value) {
var tpl = '<li>/${name}';
if (value.aliases.length > 0) {
tpl += ' <small>(or /<%- aliases.join(", /") %>)</small>';
}
if (value.params.length > 0) {
tpl += ' <small><%- params.join(" ") %></small>';
}
if (value.description !== '') {
tpl += '<small class="description"><i><%- description %></i></small>';
}
tpl += '</li>';
return _.template(tpl)(value);
},
insertTpl: function(value) {
var tpl = "\n/${name} ";
var reference_prefix = null;
if (value.params.length > 0) {
reference_prefix = value.params[0][0];
if (/^[@%~]/.test(reference_prefix)) {
tpl += '<%- reference_prefix %>';
}
}
return _.template(tpl)({ reference_prefix: reference_prefix });
},
suffix: '',
callbacks: {
sorter: this.DefaultOptions.sorter,
filter: this.DefaultOptions.filter,
beforeInsert: this.DefaultOptions.beforeInsert
}
});
},
destroyAtWho: function() {
return this.input.atwho('destroy');
Loading
Loading
@@ -265,6 +300,7 @@
this.input.atwho('load', 'mergerequests', data.mergerequests);
this.input.atwho('load', ':', data.emojis);
this.input.atwho('load', '~', data.labels);
this.input.atwho('load', '/', data.commands);
return $(':focus').trigger('keyup');
}
};
Loading
Loading
Loading
Loading
@@ -231,7 +231,12 @@
var $notesList, votesBlock;
if (!note.valid) {
if (note.award) {
new Flash('You have already awarded this emoji!', 'alert');
new Flash('You have already awarded this emoji!', 'alert', this.parentTimeline);
}
else {
if (note.errors.commands_only) {
new Flash(note.errors.commands_only, 'notice', this.parentTimeline);
}
}
return;
}
Loading
Loading
Loading
Loading
@@ -147,3 +147,8 @@
color: $gl-link-color;
}
}
.atwho-view small.description {
float: right;
padding: 3px 5px;
}
Loading
Loading
@@ -125,7 +125,7 @@ class Projects::NotesController < Projects::ApplicationController
id: note.id,
name: note.name
}
elsif note.valid?
elsif note.persisted?
Banzai::NoteRenderer.render([note], @project, current_user)
 
attrs = {
Loading
Loading
Loading
Loading
@@ -145,7 +145,8 @@ class ProjectsController < Projects::ApplicationController
milestones: autocomplete.milestones,
mergerequests: autocomplete.merge_requests,
labels: autocomplete.labels,
members: participants
members: participants,
commands: autocomplete.commands
}
 
respond_to do |format|
Loading
Loading
Loading
Loading
@@ -17,7 +17,7 @@ class TodosFinder
 
attr_accessor :current_user, :params
 
def initialize(current_user, params)
def initialize(current_user, params = {})
@current_user = current_user
@params = params
end
Loading
Loading
Loading
Loading
@@ -69,14 +69,9 @@ class IssuableBaseService < BaseService
end
 
def filter_labels
if params[:add_label_ids].present? || params[:remove_label_ids].present?
params.delete(:label_ids)
filter_labels_in_param(:add_label_ids)
filter_labels_in_param(:remove_label_ids)
else
filter_labels_in_param(:label_ids)
end
filter_labels_in_param(:add_label_ids)
filter_labels_in_param(:remove_label_ids)
filter_labels_in_param(:label_ids)
end
 
def filter_labels_in_param(key)
Loading
Loading
@@ -85,23 +80,65 @@ class IssuableBaseService < BaseService
params[key] = project.labels.where(id: params[key]).pluck(:id)
end
 
def update_issuable(issuable, attributes)
def process_label_ids(attributes, base_label_ids: [], merge_all: false)
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 merge_all || (add_label_ids.empty? && remove_label_ids.empty?)
new_label_ids |= add_label_ids
new_label_ids -= remove_label_ids
new_label_ids
end
def merge_slash_commands_into_params!
command_params = SlashCommands::InterpretService.new(project, current_user).
execute(params[:description])
params.merge!(command_params)
end
def create_issuable(issuable, attributes)
issuable.with_transaction_returning_status do
add_label_ids = attributes.delete(:add_label_ids)
remove_label_ids = attributes.delete(:remove_label_ids)
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
end
end
 
issuable.label_ids |= add_label_ids if add_label_ids
issuable.label_ids -= remove_label_ids if remove_label_ids
def create(issuable)
merge_slash_commands_into_params!
filter_params
 
issuable.assign_attributes(attributes.merge(updated_by: current_user))
if params.present? && create_issuable(issuable, params)
handle_creation(issuable)
issuable.create_cross_references!(current_user)
execute_hooks(issuable)
end
issuable
end
 
issuable.save
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
 
def update(issuable)
change_state(issuable)
change_subscription(issuable)
change_todo(issuable)
filter_params
old_labels = issuable.labels.to_a
 
Loading
Loading
@@ -134,6 +171,18 @@ class IssuableBaseService < BaseService
end
end
 
def change_todo(issuable)
case params.delete(:todo_event)
when 'mark'
todo_service.mark_todo(issuable, current_user)
when 'done'
todo = TodosFinder.new(current_user).execute.find_by(target: issuable)
if todo
todo_service.mark_todos_as_done([todo], current_user)
end
end
end
def has_changes?(issuable, old_labels: [])
valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch]
 
Loading
Loading
module Issues
class CreateService < Issues::BaseService
def execute
filter_params
label_params = params.delete(:label_ids)
issue = project.issues.new
request = params.delete(:request)
api = params.delete(:api)
issue = project.issues.new(params)
issue.author = params[:author] || current_user
 
issue.spam = spam_check_service.execute(request, api)
 
if issue.save
issue.update_attributes(label_ids: label_params)
notification_service.new_issue(issue, current_user)
todo_service.new_issue(issue, current_user)
event_service.open_issue(issue, current_user)
issue.create_cross_references!(current_user)
execute_hooks(issue, 'open')
end
create(issue)
end
 
issue
def handle_creation(issuable)
event_service.open_issue(issuable, current_user)
notification_service.new_issue(issuable, current_user)
todo_service.new_issue(issuable, current_user)
end
 
private
Loading
Loading
Loading
Loading
@@ -7,26 +7,19 @@ module MergeRequests
source_project = @project
@project = Project.find(params[:target_project_id]) if params[:target_project_id]
 
filter_params
label_params = params.delete(:label_ids)
force_remove_source_branch = params.delete(:force_remove_source_branch)
params[:target_project_id] ||= source_project.id
 
merge_request = MergeRequest.new(params)
merge_request = MergeRequest.new
merge_request.source_project = source_project
merge_request.target_project ||= source_project
merge_request.author = current_user
merge_request.merge_params['force_remove_source_branch'] = force_remove_source_branch
merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch)
 
if merge_request.save
merge_request.update_attributes(label_ids: label_params)
event_service.open_mr(merge_request, current_user)
notification_service.new_merge_request(merge_request, current_user)
todo_service.new_merge_request(merge_request, current_user)
merge_request.create_cross_references!(current_user)
execute_hooks(merge_request)
end
create(merge_request)
end
 
merge_request
def handle_creation(issuable)
event_service.open_mr(issuable, current_user)
notification_service.new_merge_request(issuable, current_user)
todo_service.new_merge_request(issuable, current_user)
end
end
end
Loading
Loading
@@ -11,13 +11,61 @@ module Notes
return noteable.create_award_emoji(note.award_emoji_name, current_user)
end
 
# We execute commands (extracted from `params[:note]`) on the noteable
# **before** we save the note because if the note consists of commands
# only, there is no need be create a note!
commands_executed = execute_slash_commands!(note)
if note.save
# Finish the harder work in the background
NewNoteWorker.perform_in(2.seconds, note.id, params)
TodoService.new.new_note(note, current_user)
todo_service.new_note(note, current_user)
end
if commands_executed && note.note.blank?
note.errors.add(:commands_only, 'Your commands are being executed.')
end
 
note
end
private
def execute_slash_commands!(note)
noteable_update_service = noteable_update_service(note.noteable_type)
return unless noteable_update_service
command_params = SlashCommands::InterpretService.new(project, current_user).
execute(note.note)
commands = execute_or_filter_commands(command_params, note)
if commands.any?
noteable_update_service.new(project, current_user, commands).execute(note.noteable)
end
end
def execute_or_filter_commands(commands, note)
final_commands = commands.reduce({}) do |memo, (command_key, command_value)|
if command_key != :due_date || note.noteable.respond_to?(:due_date)
memo[command_key] = command_value
end
memo
end
final_commands
end
def noteable_update_service(noteable_type)
case noteable_type
when 'Issue'
Issues::UpdateService
when 'MergeRequest'
MergeRequests::UpdateService
else
nil
end
end
end
end
Loading
Loading
@@ -15,5 +15,9 @@ module Projects
def labels
@project.labels.select([:title, :color])
end
def commands
SlashCommands::InterpretService.command_definitions
end
end
end
module SlashCommands
class InterpretService < BaseService
include Gitlab::SlashCommands::Dsl
# Takes a text and interpret the commands that are extracted from it.
# Returns a hash of changes to be applied to a record.
def execute(content)
@updates = {}
commands = extractor.extract_commands!(content)
commands.each do |command|
__send__(*command)
end
@updates
end
private
def extractor
@extractor ||= Gitlab::SlashCommands::Extractor.new(self.class.command_names)
end
desc 'Close this issue or merge request'
command :close do
@updates[:state_event] = 'close'
end
desc 'Reopen this issue or merge request'
command :open, :reopen do
@updates[:state_event] = 'reopen'
end
desc 'Reassign'
params '@user'
command :assign, :reassign do |assignee_param|
user = extract_references(assignee_param, :user).first
return unless user
@updates[:assignee_id] = user.id
end
desc 'Remove assignee'
command :unassign, :remove_assignee do
@updates[:assignee_id] = nil
end
desc 'Change milestone'
params '%"milestone"'
command :milestone do |milestone_param|
milestone = extract_references(milestone_param, :milestone).first
return unless milestone
@updates[:milestone_id] = milestone.id
end
desc 'Remove milestone'
command :clear_milestone, :remove_milestone do
@updates[:milestone_id] = nil
end
desc 'Add label(s)'
params '~label1 ~"label 2"'
command :label, :labels do |labels_param|
label_ids = find_label_ids(labels_param)
return if label_ids.empty?
@updates[:add_label_ids] = label_ids
end
desc 'Remove label(s)'
params '~label1 ~"label 2"'
command :unlabel, :remove_label, :remove_labels do |labels_param|
label_ids = find_label_ids(labels_param)
return if label_ids.empty?
@updates[:remove_label_ids] = label_ids
end
desc 'Remove all labels'
command :clear_labels, :clear_label do
@updates[:label_ids] = []
end
desc 'Add a todo'
command :todo do
@updates[:todo_event] = 'mark'
end
desc 'Mark todo as done'
command :done do
@updates[:todo_event] = 'done'
end
desc 'Subscribe'
command :subscribe do
@updates[:subscription_event] = 'subscribe'
end
desc 'Unsubscribe'
command :unsubscribe do
@updates[:subscription_event] = 'unsubscribe'
end
desc 'Set a due date'
params '<YYYY-MM-DD> | <N days>'
command :due_date do |due_date_param|
due_date = begin
Time.now + ChronicDuration.parse(due_date_param)
rescue ChronicDuration::DurationParseError
Date.parse(due_date_param) rescue nil
end
@updates[:due_date] = due_date if due_date
end
desc 'Remove due date'
command :clear_due_date do
@updates[:due_date] = nil
end
def find_label_ids(labels_param)
extract_references(labels_param, :label).map(&:id)
end
def extract_references(cmd_arg, type)
ext = Gitlab::ReferenceExtractor.new(project, current_user)
ext.analyze(cmd_arg, author: current_user)
ext.references(type)
end
end
end
Loading
Loading
@@ -6,6 +6,7 @@
- [GitLab Flow](gitlab_flow.md)
- [Groups](groups.md)
- [Keyboard shortcuts](shortcuts.md)
- [Slash commands](slash_commands.md)
- [File finder](file_finder.md)
- [Labels](../user/project/labels.md)
- [Notification emails](notifications.md)
Loading
Loading
# GitLab slash commands
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.
Here is a list of all of the available commands and descriptions about what they
do.
| Command | Aliases | Action |
|:---------------------------|:--------------------|:-------------|
| `/close` | None | Close the issue or merge request |
| `/open` | `/reopen` | Reopen the issue or merge request |
| `/assign @username` | `/reassign` | Reassign |
| `/unassign` | `/remove_assignee` | Remove assignee |
| `/milestone %milestone` | None | Change milestone |
| `/clear_milestone` | `/remove_milestone` | Remove milestone |
| `/label ~foo ~"bar baz"` | `/labels` | Add label(s) |
| `/unlabel ~foo ~"bar baz"` | `/remove_label`, `remove_labels` | Remove label(s) |
| `/clear_labels` | `/clear_label` | Clear all labels |
| `/todo` | None | Add a todo |
| `/done` | None | Mark todo as done |
| `/subscribe` | None | Subscribe |
| `/unsubscribe` | None | Unsubscribe |
| `/due_date` | None | Set a due date |
| `/clear_due_date` | None | Remove due date |
Loading
Loading
@@ -45,6 +45,7 @@ module Gitlab
 
def verify_record!(record:, invalid_exception:, record_name:)
return if record.persisted?
return if record.errors.key?(:commands_only)
 
error_title = "The #{record_name} could not be created for the following reasons:"
 
Loading
Loading
module Gitlab
module SlashCommands
module Dsl
extend ActiveSupport::Concern
included do
@command_definitions = []
end
module ClassMethods
def command_definitions
@command_definitions
end
def command_names
command_definitions.flat_map do |command_definition|
[command_definition[:name], command_definition[:aliases]].flatten
end
end
# Allows to give a description to the next slash command
def desc(text)
@description = text
end
# Allows to define params for the next slash command
def params(*params)
@params = params
end
# Registers a new command which is recognizeable
# from body of email or comment.
# Example:
#
# command :command_key do |arguments|
# # Awesome code block
# end
#
def command(*command_names, &block)
command_name, *aliases = command_names
proxy_method_name = "__#{command_name}__"
# This proxy method is needed because calling `return` from inside a
# block/proc, causes a `return` from the enclosing method or lambda,
# otherwise a LocalJumpError error is raised.
define_method(proxy_method_name, &block)
define_method(command_name) do |*args|
proxy_method = method(proxy_method_name)
if proxy_method.arity == -1 || proxy_method.arity == args.size
instance_exec(*args, &proxy_method)
end
end
private command_name
aliases.each do |alias_command|
alias_method alias_command, command_name
private alias_command
end
command_definition = {
name: command_name,
aliases: aliases,
description: @description || '',
params: @params || []
}
@command_definitions << command_definition
@description = nil
@params = nil
end
end
end
end
end
module Gitlab
module SlashCommands
# This class takes an array of commands that should be extracted from a
# given text.
#
# ```
# extractor = Gitlab::SlashCommands::Extractor.new([:open, :assign, :labels])
# ```
class Extractor
attr_reader :command_names
def initialize(command_names)
@command_names = command_names
end
# Extracts commands from content and return an array of commands.
# The array looks like the following:
# [
# ['command1'],
# ['command3', 'arg1 arg2'],
# ]
# The command and the arguments are stripped.
# The original command text is removed from the given `content`.
#
# Usage:
# ```
# extractor = Gitlab::SlashCommands::Extractor.new([:open, :assign, :labels])
# msg = %(hello\n/labels ~foo ~"bar baz"\nworld)
# commands = extractor.extract_commands! #=> [['labels', '~foo ~"bar baz"']]
# msg #=> "hello\nworld"
# ```
def extract_commands!(content)
return [] unless content
commands = []
content.gsub!(commands_regex) do
commands << [$1, $2].flatten.reject(&:blank?)
''
end
commands
end
private
# Builds a regular expression to match known commands.
# First match group captures the command name and
# second match group captures its arguments.
#
# It looks something like:
#
# /^\/(?<cmd>close|reopen|...)(?:( |$))(?<args>[^\/\n]*)(?:\n|$)/
def commands_regex
/^\/(?<cmd>#{command_names.join('|')})(?:( |$))(?<args>[^\/\n]*)(?:\n|$)/
end
end
end
end
require 'rails_helper'
feature 'Issues > User uses slash commands', feature: true, js: true do
include WaitForAjax
it_behaves_like 'issuable record that supports slash commands in its description and notes', :issue do
let(:issuable) { create(:issue, project: project) }
end
describe 'issue-only commands' do
let(:user) { create(:user) }
let(:project) { create(:project, :public) }
before do
project.team << [user, :master]
login_with(user)
visit namespace_project_issue_path(project.namespace, project, issue)
end
describe 'adding a due date from note' do
let(:issue) { create(:issue, project: project) }
it 'does not create a note, and sets the due date accordingly' do
page.within('.js-main-target-form') do
fill_in 'note[note]', with: "/due_date 2016-08-28"
click_button 'Comment'
end
expect(page).not_to have_content '/due_date 2016-08-28'
expect(page).to have_content 'Your commands are being executed.'
issue.reload
expect(issue.due_date).to eq Date.new(2016, 8, 28)
end
end
describe 'removing a due date from note' do
let(:issue) { create(:issue, project: project, due_date: Date.new(2016, 8, 28)) }
it 'does not create a note, and removes the due date accordingly' do
expect(issue.due_date).to eq Date.new(2016, 8, 28)
page.within('.js-main-target-form') do
fill_in 'note[note]', with: "/clear_due_date"
click_button 'Comment'
end
expect(page).not_to have_content '/clear_due_date'
expect(page).to have_content 'Your commands are being executed.'
issue.reload
expect(issue.due_date).to be_nil
end
end
end
end
require 'rails_helper'
feature 'Merge Requests > User uses slash commands', feature: true, js: true do
include WaitForAjax
let(:user) { create(:user) }
let(:project) { create(:project, :public) }
let(:merge_request) { create(:merge_request, source_project: project) }
let!(:milestone) { create(:milestone, project: project, title: 'ASAP') }
it_behaves_like 'issuable record that supports slash commands in its description and notes', :merge_request do
let(:issuable) { create(:merge_request, source_project: project) }
let(:new_url_opts) { { merge_request: { source_branch: 'feature' } } }
end
describe 'adding a due date from note' do
before do
project.team << [user, :master]
login_with(user)
visit namespace_project_merge_request_path(project.namespace, project, merge_request)
end
it 'does not recognize the command nor create a note' do
page.within('.js-main-target-form') do
fill_in 'note[note]', with: "/due_date 2016-08-28"
click_button 'Comment'
end
expect(page).not_to have_content '/due_date 2016-08-28'
end
end
# Postponed because of high complexity
xdescribe 'merging from note' do
before do
project.team << [user, :master]
login_with(user)
visit namespace_project_merge_request_path(project.namespace, project, merge_request)
end
it 'creates a note without the commands and interpret the commands accordingly' do
page.within('.js-main-target-form') do
fill_in 'note[note]', with: "Let's merge this!\n/merge\n/milestone %ASAP"
click_button 'Comment'
end
expect(page).to have_content("Let's merge this!")
expect(page).not_to have_content('/merge')
expect(page).not_to have_content('/milestone %ASAP')
merge_request.reload
note = merge_request.notes.user.first
expect(note.note).to eq "Let's merge this!\r\n"
expect(merge_request).to be_merged
expect(merge_request.milestone).to eq milestone
end
end
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment