Commit 16884719 authored by Robert Speicher's avatar Robert Speicher Committed by Ruben Davila

Merge branch '4273-slash-commands' into 'master'

Support slash commands in issues / MR description & comments

See merge request !5021
parent abb1a595
......@@ -72,6 +72,7 @@ v 8.11.0 (unreleased)
- 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
- Fix syntax highlighting in file editor
- Support slash commands in issue and merge request descriptions as well as comments. !5021
- Nokogiri's various parsing methods are now instrumented
- Add archived badge to project list !5798
- Add simple identifier to public SSH keys (muteor)
......
......@@ -209,7 +209,8 @@ gem 'mousetrap-rails', '~> 1.4.6'
# Detect and convert string character encoding
gem 'charlock_holmes', '~> 0.7.3'
# Parse duration
# Parse time & duration
gem 'chronic', '~> 0.10.2'
gem 'chronic_duration', '~> 0.10.6'
gem 'sass-rails', '~> 5.0.0'
......
......@@ -128,6 +128,7 @@ GEM
mime-types (>= 1.16)
cause (0.1)
charlock_holmes (0.7.3)
chronic (0.10.2)
chronic_duration (0.10.6)
numerizer (~> 0.1.1)
chunky_png (1.3.5)
......@@ -824,6 +825,7 @@ DEPENDENCIES
capybara-screenshot (~> 1.0.0)
carrierwave (~> 0.10.0)
charlock_holmes (~> 0.7.3)
chronic (~> 0.10.2)
chronic_duration (~> 0.10.6)
coffee-rails (~> 4.1.0)
connection_pool (~> 2.0)
......
......@@ -223,7 +223,7 @@
}
}
});
return this.input.atwho({
this.input.atwho({
at: '~',
alias: 'labels',
searchKey: 'search',
......@@ -249,6 +249,68 @@
}
}
});
// We don't instantiate the slash commands autocomplete for note and issue/MR edit forms
this.input.filter('[data-supports-slash-commands="true"]').atwho({
at: '/',
alias: 'commands',
searchKey: 'search',
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 = "/${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,
beforeSave: function(commands) {
return $.map(commands, function(c) {
var search = c.name;
if (c.aliases.length > 0) {
search = search + " " + c.aliases.join(" ");
}
return {
name: c.name,
aliases: c.aliases,
params: c.params,
description: c.description,
search: search
};
});
},
matcher: function(flag, subtext, should_startWithSpace, acceptSpaceBar) {
var regexp = /(?:^|\n)\/([A-Za-z_]*)$/gi
var match = regexp.exec(subtext);
if (match) {
return match[1];
} else {
return null;
}
}
}
});
return;
},
destroyAtWho: function() {
return this.input.atwho('destroy');
......@@ -265,6 +327,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');
}
};
......
......@@ -201,7 +201,7 @@
Increase @pollingInterval up to 120 seconds on every function call,
if `shouldReset` has a truthy value, 'null' or 'undefined' the variable
will reset to @basePollingInterval.
Note: this function is used to gradually increase the polling interval
if there aren't new notes coming from the server
*/
......@@ -223,7 +223,7 @@
/*
Render note in main comments area.
Note: for rendering inline notes use renderDiscussionNote
*/
......@@ -231,7 +231,13 @@
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);
this.refresh();
}
}
return;
}
......@@ -245,6 +251,7 @@
$notesList.append(note.html).syntaxHighlight();
gl.utils.localTimeAgo($notesList.find("#note_" + note.id + " .js-timeago"), false);
this.initTaskList();
this.refresh();
return this.updateNotesCount(1);
}
};
......@@ -265,7 +272,7 @@
/*
Render note in discussion area.
Note: for rendering inline notes use renderDiscussionNote
*/
......@@ -304,7 +311,7 @@
/*
Called in response the main target form has been successfully submitted.
Removes any errors.
Resets text and preview.
Resets buttons.
......@@ -329,7 +336,7 @@
/*
Shows the main form and does some setup on it.
Sets some hidden fields in the form.
*/
......@@ -349,7 +356,7 @@
/*
General note form setup.
deactivates the submit button when text is empty
hides the preview button when text is empty
setup GFM auto complete
......@@ -366,7 +373,7 @@
/*
Called in response to the new note form being submitted
Adds new note to list.
*/
......@@ -381,7 +388,7 @@
/*
Called in response to the new note form being submitted
Adds new note to list.
*/
......@@ -393,7 +400,7 @@
/*
Called in response to the edit note form being submitted
Updates the current note field.
*/
......@@ -410,7 +417,7 @@
/*
Called in response to clicking the edit note link
Replaces the note text with the note edit form
Adds a data attribute to the form with the original content of the note for cancellations
*/
......@@ -450,7 +457,7 @@
/*
Called in response to clicking the edit note link
Hides edit form and restores the original note text to the editor textarea.
*/
......@@ -472,7 +479,7 @@
/*
Called in response to deleting a note of any kind.
Removes the actual note from view.
Removes the whole discussion if the last note is being removed.
*/
......@@ -498,7 +505,7 @@
/*
Called in response to clicking the delete attachment link
Removes the attachment wrapper view, including image tag if it exists
Resets the note editing form
*/
......@@ -515,7 +522,7 @@
/*
Called when clicking on the "reply" button for a diff line.
Shows the note form below the notes.
*/
......@@ -531,9 +538,9 @@
/*
Shows the diff or discussion form and does some setup on it.
Sets some hidden fields in the form.
Note: dataHolder must have the "discussionId", "lineCode", "noteableType"
and "noteableId" data attributes set.
*/
......@@ -557,7 +564,7 @@
/*
Called when clicking on the "add a comment" button on the side of a diff line.
Inserts a temporary row for the form below the line.
Sets up the form and shows it.
*/
......@@ -605,7 +612,7 @@
/*
Called in response to "cancel" on a diff note form.
Shows the reply button again.
Removes the form and if necessary it's temporary row.
*/
......@@ -634,7 +641,7 @@
/*
Called after an attachment file has been selected.
Updates the file name for the selected attachment.
*/
......
......@@ -147,3 +147,8 @@
color: $gl-link-color;
}
}
.atwho-view small.description {
float: right;
padding: 3px 5px;
}
......@@ -93,7 +93,7 @@ class Projects::CommitController < Projects::ApplicationController
end
def commit
@commit ||= @project.commit(params[:id])
@noteable = @commit ||= @project.commit(params[:id])
end
def pipelines
......
......@@ -177,11 +177,7 @@ class Projects::IssuesController < Projects::ApplicationController
protected
def issue
@issue ||= begin
@project.issues.find_by!(iid: params[:id])
rescue ActiveRecord::RecordNotFound
redirect_old
end
@noteable = @issue ||= @project.issues.find_by(iid: params[:id]) || redirect_old
end
alias_method :subscribable_resource, :issue
alias_method :issuable, :issue
......@@ -226,7 +222,6 @@ class Projects::IssuesController < Projects::ApplicationController
if issue
redirect_to issue_path(issue)
return
else
raise ActiveRecord::RecordNotFound.new
end
......
......@@ -381,7 +381,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end
def merge_request
@merge_request ||= @project.merge_requests.find_by!(iid: params[:id])
@issuable = @merge_request ||= @project.merge_requests.find_by!(iid: params[:id])
end
alias_method :subscribable_resource, :merge_request
alias_method :issuable, :merge_request
......
......@@ -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 = {
......
......@@ -134,10 +134,22 @@ class ProjectsController < Projects::ApplicationController
end
def autocomplete_sources
note_type = params['type']
note_id = params['type_id']
noteable =
case params[:type]
when 'Issue'
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').
execute.find_by(iid: params[:type_id])
when 'Commit'
@project.commit(params[:type_id])
else
nil
end
autocomplete = ::Projects::AutocompleteService.new(@project, current_user)
participants = ::Projects::ParticipantsService.new(@project, current_user).execute(note_type, note_id)
participants = ::Projects::ParticipantsService.new(@project, current_user).execute(noteable)
@suggestions = {
emojis: Gitlab::AwardEmoji.urls,
......@@ -145,7 +157,8 @@ class ProjectsController < Projects::ApplicationController
milestones: autocomplete.milestones,
mergerequests: autocomplete.merge_requests,
labels: autocomplete.labels,
members: participants
members: participants,
commands: autocomplete.commands(noteable, params[:type])
}
respond_to do |format|
......
......@@ -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
......
......@@ -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)
......@@ -85,27 +80,86 @@ class IssuableBaseService < BaseService
params[key] = project.labels.where(id: params[key]).pluck(:id)
end
def update_issuable(issuable, attributes)
def process_label_ids(attributes, existing_label_ids: nil)
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 = existing_label_ids || label_ids || []
if add_label_ids.blank? && remove_label_ids.blank?
new_label_ids = label_ids if label_ids
else
new_label_ids |= add_label_ids if add_label_ids
new_label_ids -= remove_label_ids if remove_label_ids
end
new_label_ids
end
def merge_slash_commands_into_params!(issuable)
description, command_params =
SlashCommands::InterpretService.new(project, current_user).
execute(params[:description], issuable)
params[:description] = description
params.merge!(command_params)
end
def create_issuable(issuable, attributes, label_ids:)
issuable.with_transaction_returning_status do
add_label_ids = attributes.delete(:add_label_ids)
remove_label_ids = attributes.delete(:remove_label_ids)
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!(issuable)
filter_params
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
issuable
end
issuable.assign_attributes(attributes.merge(updated_by: current_user))
def before_create(issuable)
# To be overridden by subclasses
end
def after_create(issuable)
# To be overridden by subclasses
end
issuable.save
def update_issuable(issuable, attributes)
issuable.with_transaction_returning_status do
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
old_mentioned_users = issuable.mentioned_users.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)
......@@ -135,6 +189,16 @@ class IssuableBaseService < BaseService
end
end
def change_todo(issuable)
case params.delete(:todo_event)
when 'add'
todo_service.mark_todo(issuable, current_user)
when 'done'
todo = TodosFinder.new(current_user).execute.find_by(target: issuable)
todo_service.mark_todos_as_done([todo], current_user) if todo
end
end
def has_changes?(issuable, old_labels: [])
valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch]
......
module Issues
class CloseService < Issues::BaseService
def execute(issue, commit: nil, notifications: true, system_note: true)
return issue unless can?(current_user, :update_issue, issue)
if project.jira_tracker? && project.jira_service.active
project.jira_service.execute(commit, issue)
todo_service.close_issue(issue, current_user)
......
module Issues
class CreateService < Issues::BaseService
def execute
filter_params
label_params = params.delete(:label_ids)
@request = params.delete(:request)
@api = params.delete(:api)
@issue = project.issues.new(params)
@issue.author = params[:author] || current_user
@issue.spam = spam_service.check(@api)
@issue = project.issues.new
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)
user_agent_detail_service.create
@issue.create_cross_references!(current_user)
execute_hooks(@issue, 'open')
end
create(@issue)
end
def before_create(issuable)
issuable.spam = spam_service.check(@api)
end
@issue
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)
user_agent_detail_service.create
end
private
......
module Issues
class ReopenService < Issues::BaseService
def execute(issue)
return issue unless can?(current_user, :update_issue, issue)
if issue.reopen
event_service.reopen_issue(issue, current_user)
create_note(issue)
......
module MergeRequests
class CloseService < MergeRequests::BaseService
def execute(merge_request, commit = nil)
return merge_request unless can?(current_user, :update_merge_request, merge_request)
# If we close MergeRequest we want to ignore validation
# so we can close broken one (Ex. fork project removed)
merge_request.allow_broken = true
......
......@@ -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 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)
end
end
end
module MergeRequests
class ReopenService < MergeRequests::BaseService
def execute(merge_request)
return merge_request unless can?(current_user, :update_merge_request, merge_request)
if merge_request.reopen
event_service.reopen_mr(merge_request, current_user)
create_note(merge_request)
......
......@@ -11,10 +11,33 @@ module Notes
return noteable.create_award_emoji(note.award_emoji_name, current_user)
end
if note.save
# 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!
slash_commands_service = SlashCommandsService.new(project, current_user)
if slash_commands_service.supported?(note)
content, command_params = slash_commands_service.extract_commands(note)
only_commands = content.empty?
note.note = content
end
if !only_commands && 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 command_params && command_params.any?
slash_commands_service.execute(command_params, note)
# We must add the error after we call #save because errors are reset
# when #save is called
if only_commands
note.errors.add(:commands_only, 'Your commands have been executed!')
end
end
note
......
module Notes
class SlashCommandsService < BaseService
UPDATE_SERVICES = {
'Issue' => Issues::UpdateService,
'MergeRequest' => MergeRequests::UpdateService
}
def supported?(note)
noteable_update_service(note) &&
can?(current_user, :"update_#{note.noteable_type.underscore}", note.noteable)
end
def extract_commands(note)
return [note.note, {}] unless supported?(note)
SlashCommands::InterpretService.new(project, current_user).
execute(note.note, note.noteable)
end
def execute(command_params, note)