From 0eea8c885743575b0e93a98846b3663e9903aa66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me> Date: Thu, 30 Jun 2016 17:34:19 +0200 Subject: [PATCH] Support slash commands in noteable description and notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Rémy Coutable <remy@rymai.me> --- CHANGELOG | 1 + ...o_complete.js => gfm_auto_complete.js.es6} | 38 ++- app/assets/javascripts/notes.js | 7 +- .../stylesheets/framework/markdown_area.scss | 5 + app/controllers/projects/notes_controller.rb | 2 +- app/controllers/projects_controller.rb | 3 +- app/finders/todos_finder.rb | 2 +- app/services/issuable_base_service.rb | 79 +++++-- app/services/issues/create_service.rb | 20 +- app/services/merge_requests/create_service.rb | 25 +- app/services/notes/create_service.rb | 50 +++- app/services/projects/autocomplete_service.rb | 4 + .../slash_commands/interpret_service.rb | 133 +++++++++++ doc/workflow/README.md | 1 + doc/workflow/slash_commands.md | 28 +++ lib/gitlab/email/handler/base_handler.rb | 1 + lib/gitlab/slash_commands/dsl.rb | 76 ++++++ lib/gitlab/slash_commands/extractor.rb | 59 +++++ .../issues/user_uses_slash_commands_spec.rb | 59 +++++ .../user_uses_slash_commands_spec.rb | 59 +++++ spec/fixtures/emails/commands_only_reply.eml | 40 ++++ .../email/handler/create_note_handler_spec.rb | 9 + spec/lib/gitlab/slash_commands/dsl_spec.rb | 110 +++++++++ .../gitlab/slash_commands/extractor_spec.rb | 177 ++++++++++++++ spec/services/issues/create_service_spec.rb | 2 + .../merge_requests/create_service_spec.rb | 11 +- spec/services/notes/create_service_spec.rb | 23 +- .../slash_commands/interpret_service_spec.rb | 217 ++++++++++++++++++ ..._service_slash_commands_shared_examples.rb | 83 +++++++ ...issuable_slash_commands_shared_examples.rb | 170 ++++++++++++++ ..._service_slash_commands_shared_examples.rb | 116 ++++++++++ 31 files changed, 1552 insertions(+), 58 deletions(-) rename app/assets/javascripts/{gfm_auto_complete.js => gfm_auto_complete.js.es6} (86%) create mode 100644 app/services/slash_commands/interpret_service.rb create mode 100644 doc/workflow/slash_commands.md create mode 100644 lib/gitlab/slash_commands/dsl.rb create mode 100644 lib/gitlab/slash_commands/extractor.rb create mode 100644 spec/features/issues/user_uses_slash_commands_spec.rb create mode 100644 spec/features/merge_requests/user_uses_slash_commands_spec.rb create mode 100644 spec/fixtures/emails/commands_only_reply.eml create mode 100644 spec/lib/gitlab/slash_commands/dsl_spec.rb create mode 100644 spec/lib/gitlab/slash_commands/extractor_spec.rb create mode 100644 spec/services/slash_commands/interpret_service_spec.rb create mode 100644 spec/support/issuable_create_service_slash_commands_shared_examples.rb create mode 100644 spec/support/issuable_slash_commands_shared_examples.rb create mode 100644 spec/support/note_create_service_slash_commands_shared_examples.rb diff --git a/CHANGELOG b/CHANGELOG index 96965a20f6944..63ee7b91dcbe0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -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) diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js.es6 similarity index 86% rename from app/assets/javascripts/gfm_auto_complete.js rename to app/assets/javascripts/gfm_auto_complete.js.es6 index 2e5b15f4b77ea..21639c7c08431 100644 --- a/app/assets/javascripts/gfm_auto_complete.js +++ b/app/assets/javascripts/gfm_auto_complete.js.es6 @@ -223,7 +223,7 @@ } } }); - return this.input.atwho({ + this.input.atwho({ at: '~', alias: 'labels', searchKey: 'search', @@ -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'); @@ -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'); } }; diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 9ece474d9941e..99bc1a640a81c 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -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; } diff --git a/app/assets/stylesheets/framework/markdown_area.scss b/app/assets/stylesheets/framework/markdown_area.scss index 96565da1bc9ae..edea4ad00eb30 100644 --- a/app/assets/stylesheets/framework/markdown_area.scss +++ b/app/assets/stylesheets/framework/markdown_area.scss @@ -147,3 +147,8 @@ color: $gl-link-color; } } + +.atwho-view small.description { + float: right; + padding: 3px 5px; +} diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 766b7e9cf2228..f2422729364d6 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -125,7 +125,7 @@ def note_json(note) id: note.id, name: note.name } - elsif note.valid? + elsif note.persisted? Banzai::NoteRenderer.render([note], @project, current_user) attrs = { diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 47efbd4a93902..64d31e4a3a030 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -145,7 +145,8 @@ def autocomplete_sources milestones: autocomplete.milestones, mergerequests: autocomplete.merge_requests, labels: autocomplete.labels, - members: participants + members: participants, + commands: autocomplete.commands } respond_to do |format| diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index ff866c2faa502..fd859e134e5ac 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -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 diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 2d96efe1042c9..b365e19c4a883 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -69,14 +69,9 @@ def filter_milestone 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,23 +80,65 @@ def filter_labels_in_param(key) 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 @@ -134,6 +171,18 @@ def change_subscription(issuable) 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] diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 5e2de2ccf6452..1b03d7f4c052b 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -1,25 +1,19 @@ 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 diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 96a25330af100..0b592cd562033 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -7,26 +7,19 @@ def execute 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 diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 18971bd0be3ee..d7531a5d63bc2 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -11,13 +11,61 @@ def execute 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 diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index 23b6668e0d1c9..e943d2ffbcb0f 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -15,5 +15,9 @@ def merge_requests def labels @project.labels.select([:title, :color]) end + + def commands + SlashCommands::InterpretService.command_definitions + end end end diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb new file mode 100644 index 0000000000000..2c92a4f7de5d8 --- /dev/null +++ b/app/services/slash_commands/interpret_service.rb @@ -0,0 +1,133 @@ +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 diff --git a/doc/workflow/README.md b/doc/workflow/README.md index 49dec61371656..17c04377b4c75 100644 --- a/doc/workflow/README.md +++ b/doc/workflow/README.md @@ -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) diff --git a/doc/workflow/slash_commands.md b/doc/workflow/slash_commands.md new file mode 100644 index 0000000000000..3bfc66309baf0 --- /dev/null +++ b/doc/workflow/slash_commands.md @@ -0,0 +1,28 @@ +# 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 | diff --git a/lib/gitlab/email/handler/base_handler.rb b/lib/gitlab/email/handler/base_handler.rb index b7ed11cb6389e..7cccf465334f5 100644 --- a/lib/gitlab/email/handler/base_handler.rb +++ b/lib/gitlab/email/handler/base_handler.rb @@ -45,6 +45,7 @@ def add_attachments(reply) 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:" diff --git a/lib/gitlab/slash_commands/dsl.rb b/lib/gitlab/slash_commands/dsl.rb new file mode 100644 index 0000000000000..3ded4109f2e78 --- /dev/null +++ b/lib/gitlab/slash_commands/dsl.rb @@ -0,0 +1,76 @@ +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 diff --git a/lib/gitlab/slash_commands/extractor.rb b/lib/gitlab/slash_commands/extractor.rb new file mode 100644 index 0000000000000..1a854b81aca1b --- /dev/null +++ b/lib/gitlab/slash_commands/extractor.rb @@ -0,0 +1,59 @@ +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 diff --git a/spec/features/issues/user_uses_slash_commands_spec.rb b/spec/features/issues/user_uses_slash_commands_spec.rb new file mode 100644 index 0000000000000..47c4ce306e9f0 --- /dev/null +++ b/spec/features/issues/user_uses_slash_commands_spec.rb @@ -0,0 +1,59 @@ +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 diff --git a/spec/features/merge_requests/user_uses_slash_commands_spec.rb b/spec/features/merge_requests/user_uses_slash_commands_spec.rb new file mode 100644 index 0000000000000..890648f38602b --- /dev/null +++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb @@ -0,0 +1,59 @@ +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 diff --git a/spec/fixtures/emails/commands_only_reply.eml b/spec/fixtures/emails/commands_only_reply.eml new file mode 100644 index 0000000000000..b64d851a79c7a --- /dev/null +++ b/spec/fixtures/emails/commands_only_reply.eml @@ -0,0 +1,40 @@ +Return-Path: <jake@adventuretime.ooo> +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog <jake@adventuretime.ooo> +To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +In-Reply-To: <issue_1@localhost> +References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> +Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 + +/close +/unsubscribe + + +On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta +<reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo> wrote: +> +> +> +> eviltrout posted in 'Adventure Time Sux' on Discourse Meta: +> +> --- +> hey guys everyone knows adventure time sucks! +> +> --- +> Please visit this link to respond: http://localhost:3000/t/adventure-time-sux/1234/3 +> +> To unsubscribe from these emails, visit your [user preferences](http://localhost:3000/user_preferences). +> diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb index a2119b0dadfb1..e2339c5e10379 100644 --- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb @@ -60,6 +60,15 @@ it "raises an InvalidNoteError" do expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError) end + + context 'because the note was commands only' do + let!(:email_raw) { fixture_file("emails/commands_only_reply.eml") } + + it 'raises a CommandsOnlyNoteError' do + expect { receiver.execute }.not_to raise_error + end + + end end context "when the reply is blank" do diff --git a/spec/lib/gitlab/slash_commands/dsl_spec.rb b/spec/lib/gitlab/slash_commands/dsl_spec.rb new file mode 100644 index 0000000000000..f8abb35674dca --- /dev/null +++ b/spec/lib/gitlab/slash_commands/dsl_spec.rb @@ -0,0 +1,110 @@ +require 'spec_helper' + +describe Gitlab::SlashCommands::Dsl do + before :all do + DummyClass = Class.new do + include Gitlab::SlashCommands::Dsl + + desc 'A command with no args' + command :no_args, :none do + "Hello World!" + end + + desc 'A command returning a value' + command :returning do + return 42 + end + + params 'The first argument' + command :one_arg, :once, :first do |arg1| + arg1 + end + + desc 'A command with two args' + params 'The first argument', 'The second argument' + command :two_args do |arg1, arg2| + [arg1, arg2] + end + + command :wildcard do |*args| + args + end + end + end + let(:dummy) { DummyClass.new } + + describe '.command_definitions' do + it 'returns an array with commands definitions' do + expected = [ + { name: :no_args, aliases: [:none], description: 'A command with no args', params: [] }, + { name: :returning, aliases: [], description: 'A command returning a value', params: [] }, + { name: :one_arg, aliases: [:once, :first], description: '', params: ['The first argument'] }, + { name: :two_args, aliases: [], description: 'A command with two args', params: ['The first argument', 'The second argument'] }, + { name: :wildcard, aliases: [], description: '', params: [] } + ] + + expect(DummyClass.command_definitions).to eq expected + end + end + + describe '.command_names' do + it 'returns an array with commands definitions' do + expect(DummyClass.command_names).to eq [ + :no_args, :none, :returning, :one_arg, + :once, :first, :two_args, :wildcard + ] + end + end + + describe 'command with no args' do + context 'called with no args' do + it 'succeeds' do + expect(dummy.__send__(:no_args)).to eq 'Hello World!' + end + end + end + + describe 'command with an explicit return' do + context 'called with no args' do + it 'succeeds' do + expect(dummy.__send__(:returning)).to eq 42 + end + end + end + + describe 'command with one arg' do + context 'called with one arg' do + it 'succeeds' do + expect(dummy.__send__(:one_arg, 42)).to eq 42 + end + end + end + + describe 'command with two args' do + context 'called with two args' do + it 'succeeds' do + expect(dummy.__send__(:two_args, 42, 'foo')).to eq [42, 'foo'] + end + end + end + + describe 'command with wildcard' do + context 'called with no args' do + it 'succeeds' do + expect(dummy.__send__(:wildcard)).to eq [] + end + end + + context 'called with one arg' do + it 'succeeds' do + expect(dummy.__send__(:wildcard, 42)).to eq [42] + end + end + + context 'called with two args' do + it 'succeeds' do + expect(dummy.__send__(:wildcard, 42, 'foo')).to eq [42, 'foo'] + end + end + end +end diff --git a/spec/lib/gitlab/slash_commands/extractor_spec.rb b/spec/lib/gitlab/slash_commands/extractor_spec.rb new file mode 100644 index 0000000000000..fd1b30052ed96 --- /dev/null +++ b/spec/lib/gitlab/slash_commands/extractor_spec.rb @@ -0,0 +1,177 @@ +require 'spec_helper' + +describe Gitlab::SlashCommands::Extractor do + let(:extractor) { described_class.new([:open, :assign, :labels, :power]) } + + shared_examples 'command with no argument' do + it 'extracts command' do + commands = extractor.extract_commands!(original_msg) + + expect(commands).to eq [['open']] + expect(original_msg).to eq final_msg + end + end + + shared_examples 'command with a single argument' do + it 'extracts command' do + commands = extractor.extract_commands!(original_msg) + + expect(commands).to eq [['assign', '@joe']] + expect(original_msg).to eq final_msg + end + end + + shared_examples 'command with multiple arguments' do + it 'extracts command' do + commands = extractor.extract_commands!(original_msg) + + expect(commands).to eq [['labels', '~foo ~"bar baz" label']] + expect(original_msg).to eq final_msg + end + end + + describe '#extract_commands!' do + describe 'command with no argument' do + context 'at the start of content' do + it_behaves_like 'command with no argument' do + let(:original_msg) { "/open\nworld" } + let(:final_msg) { "world" } + end + end + + context 'in the middle of content' do + it_behaves_like 'command with no argument' do + let(:original_msg) { "hello\n/open\nworld" } + let(:final_msg) { "hello\nworld" } + end + end + + context 'in the middle of a line' do + it 'does not extract command' do + msg = "hello\nworld /open" + commands = extractor.extract_commands!(msg) + + expect(commands).to be_empty + expect(msg).to eq "hello\nworld /open" + end + end + + context 'at the end of content' do + it_behaves_like 'command with no argument' do + let(:original_msg) { "hello\n/open" } + let(:final_msg) { "hello\n" } + end + end + end + + describe 'command with a single argument' do + context 'at the start of content' do + it_behaves_like 'command with a single argument' do + let(:original_msg) { "/assign @joe\nworld" } + let(:final_msg) { "world" } + end + end + + context 'in the middle of content' do + it_behaves_like 'command with a single argument' do + let(:original_msg) { "hello\n/assign @joe\nworld" } + let(:final_msg) { "hello\nworld" } + end + end + + context 'in the middle of a line' do + it 'does not extract command' do + msg = "hello\nworld /assign @joe" + commands = extractor.extract_commands!(msg) + + expect(commands).to be_empty + expect(msg).to eq "hello\nworld /assign @joe" + end + end + + context 'at the end of content' do + it_behaves_like 'command with a single argument' do + let(:original_msg) { "hello\n/assign @joe" } + let(:final_msg) { "hello\n" } + end + end + + context 'when argument is not separated with a space' do + it 'does not extract command' do + msg = "hello\n/assign@joe\nworld" + commands = extractor.extract_commands!(msg) + + expect(commands).to be_empty + expect(msg).to eq "hello\n/assign@joe\nworld" + end + end + end + + describe 'command with multiple arguments' do + context 'at the start of content' do + it_behaves_like 'command with multiple arguments' do + let(:original_msg) { %(/labels ~foo ~"bar baz" label\nworld) } + let(:final_msg) { "world" } + end + end + + context 'in the middle of content' do + it_behaves_like 'command with multiple arguments' do + let(:original_msg) { %(hello\n/labels ~foo ~"bar baz" label\nworld) } + let(:final_msg) { "hello\nworld" } + end + end + + context 'in the middle of a line' do + it 'does not extract command' do + msg = %(hello\nworld /labels ~foo ~"bar baz" label) + commands = extractor.extract_commands!(msg) + + expect(commands).to be_empty + expect(msg).to eq %(hello\nworld /labels ~foo ~"bar baz" label) + end + end + + context 'at the end of content' do + it_behaves_like 'command with multiple arguments' do + let(:original_msg) { %(hello\n/labels ~foo ~"bar baz" label) } + let(:final_msg) { "hello\n" } + end + end + + context 'when argument is not separated with a space' do + it 'does not extract command' do + msg = %(hello\n/labels~foo ~"bar baz" label\nworld) + commands = extractor.extract_commands!(msg) + + expect(commands).to be_empty + expect(msg).to eq %(hello\n/labels~foo ~"bar baz" label\nworld) + end + end + end + + it 'extracts command with multiple arguments and various prefixes' do + msg = %(hello\n/power @user.name %9.10 ~"bar baz.2"\nworld) + commands = extractor.extract_commands!(msg) + + expect(commands).to eq [['power', '@user.name %9.10 ~"bar baz.2"']] + expect(msg).to eq "hello\nworld" + end + + it 'extracts multiple commands' do + msg = %(hello\n/power @user.name %9.10 ~"bar baz.2" label\nworld\n/open) + commands = extractor.extract_commands!(msg) + + expect(commands).to eq [['power', '@user.name %9.10 ~"bar baz.2" label'], ['open']] + expect(msg).to eq "hello\nworld\n" + end + + it 'does not alter original content if no command is found' do + msg = 'Fixes #123' + commands = extractor.extract_commands!(msg) + + expect(commands).to be_empty + expect(msg).to eq 'Fixes #123' + end + end +end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 1ee9f3aae4dcb..fcc3c0a00bd08 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -73,5 +73,7 @@ end end end + + it_behaves_like 'new issuable record that supports slash commands' end end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index b84a580967ad9..c1e4f8bd96b01 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -17,7 +17,7 @@ } end - let(:service) { MergeRequests::CreateService.new(project, user, opts) } + let(:service) { described_class.new(project, user, opts) } before do project.team << [user, :master] @@ -74,5 +74,14 @@ end end end + + it_behaves_like 'new issuable record that supports slash commands' do + let(:default_params) do + { + source_branch: 'feature', + target_branch: 'master' + } + end + end end end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 32753e84b314c..36ca7d2bce8e2 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -4,22 +4,31 @@ let(:project) { create(:empty_project) } let(:issue) { create(:issue, project: project) } let(:user) { create(:user) } + let(:opts) do + { note: 'Awesome comment', noteable_type: 'Issue', noteable_id: issue.id } + end describe '#execute' do context "valid params" do before do project.team << [user, :master] - opts = { - note: 'Awesome comment', - noteable_type: 'Issue', - noteable_id: issue.id - } - @note = Notes::CreateService.new(project, user, opts).execute end it { expect(@note).to be_valid } - it { expect(@note.note).to eq('Awesome comment') } + it { expect(@note.note).to eq(opts[:note]) } + + it_behaves_like 'note on noteable that supports slash commands' do + let(:noteable) { create(:issue, project: project) } + end + + it_behaves_like 'note on noteable that supports slash commands' do + let(:noteable) { create(:merge_request, source_project: project) } + end + + it_behaves_like 'note on noteable that does not support slash commands' do + let(:noteable) { create(:commit, project: project) } + end end end diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb new file mode 100644 index 0000000000000..fa0f65495ce92 --- /dev/null +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -0,0 +1,217 @@ +require 'spec_helper' + +describe SlashCommands::InterpretService, services: true do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project) } + let(:milestone) { create(:milestone, project: project, title: '9.10') } + let(:inprogress) { create(:label, project: project, title: 'In Progress') } + let(:bug) { create(:label, project: project, title: 'Bug') } + + describe '#command_names' do + subject { described_class.command_names } + + it 'returns the known commands' do + is_expected.to match_array([ + :open, :reopen, + :close, + :assign, :reassign, + :unassign, :remove_assignee, + :milestone, + :remove_milestone, + :clear_milestone, + :labels, :label, + :unlabel, :remove_labels, :remove_label, + :clear_labels, :clear_label, + :todo, + :done, + :subscribe, + :unsubscribe, + :due_date, + :clear_due_date + ]) + end + end + + describe '#execute' do + let(:service) { described_class.new(project, user) } + + shared_examples 'open command' do + it 'returns state_event: "open" if content contains /open' do + changes = service.execute(content) + + expect(changes).to eq(state_event: 'reopen') + end + end + + shared_examples 'close command' do + it 'returns state_event: "close" if content contains /open' do + changes = service.execute(content) + + expect(changes).to eq(state_event: 'close') + end + end + + shared_examples 'assign command' do + it 'fetches assignee and populates assignee_id if content contains /assign' do + changes = service.execute(content) + + expect(changes).to eq(assignee_id: user.id) + end + end + + shared_examples 'milestone command' do + it 'fetches milestone and populates milestone_id if content contains /milestone' do + changes = service.execute(content) + + expect(changes).to eq(milestone_id: milestone.id) + end + end + + shared_examples 'label command' do + it 'fetches label ids and populates add_label_ids if content contains /label' do + changes = service.execute(content) + + expect(changes).to eq(add_label_ids: [bug.id, inprogress.id]) + end + end + + shared_examples 'remove_labels command' do + it 'fetches label ids and populates remove_label_ids if content contains /label' do + changes = service.execute(content) + + expect(changes).to eq(remove_label_ids: [inprogress.id]) + end + end + + shared_examples 'clear_labels command' do + it 'populates label_ids: [] if content contains /clear_labels' do + changes = service.execute(content) + + expect(changes).to eq(label_ids: []) + end + end + + shared_examples 'command returning no changes' do + it 'returns an empty hash if content contains /open' do + changes = service.execute(content) + + expect(changes).to be_empty + end + end + + it_behaves_like 'open command' do + let(:content) { '/open' } + end + + it_behaves_like 'open command' do + let(:content) { '/reopen' } + end + + it_behaves_like 'close command' do + let(:content) { '/close' } + end + + it_behaves_like 'assign command' do + let(:content) { "/assign @#{user.username}" } + end + + it 'does not populate assignee_id if content contains /assign with an unknown user' do + changes = service.execute('/assign joe') + + expect(changes).to be_empty + end + + it 'does not populate assignee_id if content contains /assign without user' do + changes = service.execute('/assign') + + expect(changes).to be_empty + end + + it 'populates assignee_id: nil if content contains /unassign' do + changes = service.execute('/unassign') + + expect(changes).to eq(assignee_id: nil) + end + + it_behaves_like 'milestone command' do + let(:content) { "/milestone %#{milestone.title}" } + end + + it 'populates milestone_id: nil if content contains /clear_milestone' do + changes = service.execute('/clear_milestone') + + expect(changes).to eq(milestone_id: nil) + end + + it_behaves_like 'label command' do + let(:content) { %(/label ~"#{inprogress.title}" ~#{bug.title} ~unknown) } + end + + it_behaves_like 'label command' do + let(:content) { %(/labels ~"#{inprogress.title}" ~#{bug.title} ~unknown) } + end + + it_behaves_like 'remove_labels command' do + let(:content) { %(/unlabel ~"#{inprogress.title}") } + end + + it_behaves_like 'remove_labels command' do + let(:content) { %(/remove_labels ~"#{inprogress.title}") } + end + + it_behaves_like 'remove_labels command' do + let(:content) { %(/remove_label ~"#{inprogress.title}") } + end + + it_behaves_like 'clear_labels command' do + let(:content) { '/clear_labels' } + end + + it_behaves_like 'clear_labels command' do + let(:content) { '/clear_label' } + end + + it 'populates todo: :mark if content contains /todo' do + changes = service.execute('/todo') + + expect(changes).to eq(todo_event: 'mark') + end + + it 'populates todo: :done if content contains /done' do + changes = service.execute('/done') + + expect(changes).to eq(todo_event: 'done') + end + + it 'populates subscription: :subscribe if content contains /subscribe' do + changes = service.execute('/subscribe') + + expect(changes).to eq(subscription_event: 'subscribe') + end + + it 'populates subscription: :unsubscribe if content contains /unsubscribe' do + changes = service.execute('/unsubscribe') + + expect(changes).to eq(subscription_event: 'unsubscribe') + end + + it 'populates due_date: Time.now.tomorrow if content contains /due_date 2016-08-28' do + changes = service.execute('/due_date 2016-08-28') + + expect(changes).to eq(due_date: Date.new(2016, 8, 28)) + end + + it 'populates due_date: Time.now.tomorrow if content contains /due_date foo' do + changes = service.execute('/due_date foo') + + expect(changes).to be_empty + end + + it 'populates due_date: nil if content contains /clear_due_date' do + changes = service.execute('/clear_due_date') + + expect(changes).to eq(due_date: nil) + end + end +end diff --git a/spec/support/issuable_create_service_slash_commands_shared_examples.rb b/spec/support/issuable_create_service_slash_commands_shared_examples.rb new file mode 100644 index 0000000000000..bd0201c866f47 --- /dev/null +++ b/spec/support/issuable_create_service_slash_commands_shared_examples.rb @@ -0,0 +1,83 @@ +# Specifications for behavior common to all objects with executable attributes. +# It can take a `default_params`. + +shared_examples 'new issuable record that supports slash commands' do + let!(:project) { create(:project) } + let(:user) { create(:user).tap { |u| project.team << [u, :master] } } + let(:assignee) { create(:user) } + let!(:milestone) { create(:milestone, project: project) } + let!(:labels) { create_list(:label, 3, project: project) } + let(:base_params) { { title: FFaker::Lorem.sentence(3) } } + let(:params) { base_params.merge(defined?(default_params) ? default_params : {}).merge(example_params) } + let(:issuable) { described_class.new(project, user, params).execute } + + context 'with labels in command only' do + let(:example_params) do + { + description: "/label ~#{labels.first.name} ~#{labels.second.name}\n/remove_label ~#{labels.third.name}" + } + end + + it 'attaches labels to issuable' do + expect(issuable).to be_persisted + expect(issuable.label_ids).to match_array([labels.first.id, labels.second.id]) + end + end + + context 'with labels in params and command' do + let(:example_params) do + { + label_ids: [labels.second.id], + description: "/label ~#{labels.first.name}\n/remove_label ~#{labels.third.name}" + } + end + + it 'attaches all labels to issuable' do + expect(issuable).to be_persisted + expect(issuable.label_ids).to match_array([labels.first.id, labels.second.id]) + end + end + + context 'with assignee and milestone in command only' do + let(:example_params) do + { + description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}") + } + end + + it 'assigns and sets milestone to issuable' do + expect(issuable).to be_persisted + expect(issuable.assignee).to eq(assignee) + expect(issuable.milestone).to eq(milestone) + end + end + + context 'with assignee and milestone in params and command' do + let(:example_params) do + { + assignee: build_stubbed(:user), + milestone_id: double(:milestone), + description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}") + } + end + + it 'assigns and sets milestone to issuable from command' do + expect(issuable).to be_persisted + expect(issuable.assignee).to eq(assignee) + expect(issuable.milestone).to eq(milestone) + end + end + + describe '/close' do + let(:example_params) do + { + description: '/close' + } + end + + it 'returns an open issue' do + expect(issuable).to be_persisted + expect(issuable).to be_open + end + end +end diff --git a/spec/support/issuable_slash_commands_shared_examples.rb b/spec/support/issuable_slash_commands_shared_examples.rb new file mode 100644 index 0000000000000..0c8bd69add6e1 --- /dev/null +++ b/spec/support/issuable_slash_commands_shared_examples.rb @@ -0,0 +1,170 @@ +# Specifications for behavior common to all objects with executable attributes. +# It takes a `issuable_type`, and expect an `issuable`. + +shared_examples 'issuable record that supports slash commands in its description and notes' do |issuable_type| + let(:user) { create(:user) } + let(:assignee) { create(:user, username: 'bob') } + let(:project) { create(:project, :public) } + let!(:milestone) { create(:milestone, project: project, title: 'ASAP') } + let!(:label_bug) { create(:label, project: project, title: 'bug') } + let!(:label_feature) { create(:label, project: project, title: 'feature') } + let(:new_url_opts) { {} } + + before do + project.team << [user, :master] + project.team << [assignee, :developer] + login_with(user) + end + + describe "new #{issuable_type}" do + context 'with commands in the description' do + it "creates the #{issuable_type} and interpret commands accordingly" do + visit public_send("new_namespace_project_#{issuable_type}_path", project.namespace, project, new_url_opts) + fill_in "#{issuable_type}_title", with: 'bug 345' + fill_in "#{issuable_type}_description", with: "bug description\n/label ~bug\n/milestone %\"ASAP\"" + click_button "Submit #{issuable_type}".humanize + + issuable = project.public_send(issuable_type.to_s.pluralize).first + + expect(issuable.description).to eq "bug description\r\n" + expect(issuable.labels).to eq [label_bug] + expect(issuable.milestone).to eq milestone + expect(page).to have_content 'bug 345' + expect(page).to have_content 'bug description' + end + end + end + + describe "note on #{issuable_type}" do + before do + visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) + end + + context 'with a note containing commands' do + 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: "Awesome!\n/assign @bob\n/label ~bug\n/milestone %\"ASAP\"" + click_button 'Comment' + end + + expect(page).to have_content 'Awesome!' + expect(page).not_to have_content '/assign @bob' + expect(page).not_to have_content '/label ~bug' + expect(page).not_to have_content '/milestone %"ASAP"' + + issuable.reload + note = issuable.notes.user.first + + expect(note.note).to eq "Awesome!\r\n" + expect(issuable.assignee).to eq assignee + expect(issuable.labels).to eq [label_bug] + expect(issuable.milestone).to eq milestone + end + end + + context 'with a note containing only commands' do + it 'does not create a note but interpret the commands accordingly' do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: "/assign @bob\n/label ~bug\n/milestone %\"ASAP\"" + click_button 'Comment' + end + + expect(page).not_to have_content '/assign @bob' + expect(page).not_to have_content '/label ~bug' + expect(page).not_to have_content '/milestone %"ASAP"' + expect(page).to have_content 'Your commands are being executed.' + + issuable.reload + + expect(issuable.notes.user).to be_empty + expect(issuable.assignee).to eq assignee + expect(issuable.labels).to eq [label_bug] + expect(issuable.milestone).to eq milestone + end + end + + context "with a note marking the #{issuable_type} as todo" do + it "creates a new todo for the #{issuable_type}" do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: "/todo" + click_button 'Comment' + end + + expect(page).not_to have_content '/todo' + expect(page).to have_content 'Your commands are being executed.' + + todos = TodosFinder.new(user).execute + todo = todos.first + + expect(todos.size).to eq 1 + expect(todo).to be_pending + expect(todo.target).to eq issuable + expect(todo.author).to eq user + expect(todo.user).to eq user + end + end + + context "with a note marking the #{issuable_type} as done" do + before do + TodoService.new.mark_todo(issuable, user) + end + + it "creates a new todo for the #{issuable_type}" do + todos = TodosFinder.new(user).execute + todo = todos.first + + expect(todos.size).to eq 1 + expect(todos.first).to be_pending + expect(todo.target).to eq issuable + expect(todo.author).to eq user + expect(todo.user).to eq user + + page.within('.js-main-target-form') do + fill_in 'note[note]', with: "/done" + click_button 'Comment' + end + + expect(page).not_to have_content '/done' + expect(page).to have_content 'Your commands are being executed.' + + expect(todo.reload).to be_done + end + end + + context "with a note subscribing to the #{issuable_type}" do + it "creates a new todo for the #{issuable_type}" do + expect(issuable.subscribed?(user)).to be_falsy + + page.within('.js-main-target-form') do + fill_in 'note[note]', with: "/subscribe" + click_button 'Comment' + end + + expect(page).not_to have_content '/subscribe' + expect(page).to have_content 'Your commands are being executed.' + + expect(issuable.subscribed?(user)).to be_truthy + end + end + + context "with a note unsubscribing to the #{issuable_type} as done" do + before do + issuable.subscribe(user) + end + + it "creates a new todo for the #{issuable_type}" do + expect(issuable.subscribed?(user)).to be_truthy + + page.within('.js-main-target-form') do + fill_in 'note[note]', with: "/unsubscribe" + click_button 'Comment' + end + + expect(page).not_to have_content '/unsubscribe' + expect(page).to have_content 'Your commands are being executed.' + + expect(issuable.subscribed?(user)).to be_falsy + end + end + end +end diff --git a/spec/support/note_create_service_slash_commands_shared_examples.rb b/spec/support/note_create_service_slash_commands_shared_examples.rb new file mode 100644 index 0000000000000..3f7ad8b2f912d --- /dev/null +++ b/spec/support/note_create_service_slash_commands_shared_examples.rb @@ -0,0 +1,116 @@ +# Specifications for behavior common to all note objects with executable attributes. +# It expects a `noteable` object for which the note is posted. + +shared_context 'note on noteable' do + let!(:project) { create(:project) } + let(:user) { create(:user).tap { |u| project.team << [u, :master] } } + let(:assignee) { create(:user) } + let(:base_params) { { noteable: noteable } } + let(:params) { base_params.merge(example_params) } + let(:note) { described_class.new(project, user, params).execute } +end + +shared_examples 'note on noteable that does not support slash commands' do + include_context 'note on noteable' + + let(:params) { { commit_id: noteable.id, noteable_type: 'Commit' }.merge(example_params) } + + describe 'note with only command' do + describe '/close, /label, /assign & /milestone' do + let(:note_text) { %(/close\n/assign @#{assignee.username}") } + let(:example_params) { { note: note_text } } + + it 'saves the note and does not alter the note text' do + expect(note).to be_persisted + expect(note.note).to eq note_text + end + end + end + + describe 'note with command & text' do + describe '/close, /label, /assign & /milestone' do + let(:note_text) { %(HELLO\n/close\n/assign @#{assignee.username}\nWORLD) } + let(:example_params) { { note: note_text } } + + it 'saves the note and does not alter the note text' do + expect(note).to be_persisted + expect(note.note).to eq note_text + end + end + end +end + +shared_examples 'note on noteable that supports slash commands' do + include_context 'note on noteable' + + let!(:milestone) { create(:milestone, project: project) } + let!(:labels) { create_pair(:label, project: project) } + + describe 'note with only command' do + describe '/close, /label, /assign & /milestone' do + let(:example_params) do + { + note: %(/close\n/label ~#{labels.first.name} ~#{labels.last.name}\n/assign @#{assignee.username}\n/milestone %"#{milestone.name}") + } + end + + it 'closes noteable, sets labels, assigns, and sets milestone to noteable, and leave no note' do + expect(note).not_to be_persisted + expect(note.note).to eq '' + expect(noteable).to be_closed + expect(noteable.labels).to match_array(labels) + expect(noteable.assignee).to eq(assignee) + expect(noteable.milestone).to eq(milestone) + end + end + + describe '/open' do + let(:noteable) { create(:issue, project: project, state: :closed) } + let(:example_params) do + { + note: '/open' + } + end + + it 'opens the noteable, and leave no note' do + expect(note).not_to be_persisted + expect(note.note).to eq '' + expect(noteable).to be_open + end + end + end + + describe 'note with command & text' do + describe '/close, /label, /assign & /milestone' do + let(:example_params) do + { + note: %(HELLO\n/close\n/label ~#{labels.first.name} ~#{labels.last.name}\n/assign @#{assignee.username}\n/milestone %"#{milestone.name}"\nWORLD) + } + end + + it 'closes noteable, sets labels, assigns, and sets milestone to noteable' do + expect(note).to be_persisted + expect(note.note).to eq "HELLO\nWORLD" + expect(noteable).to be_closed + expect(noteable.labels).to match_array(labels) + expect(noteable.assignee).to eq(assignee) + expect(noteable.milestone).to eq(milestone) + end + end + + describe '/open' do + let(:noteable) { create(:issue, project: project, state: :closed) } + let(:example_params) do + { + note: "HELLO\n/open\nWORLD" + } + end + + it 'opens the noteable' do + expect(note).to be_persisted + expect(note.note).to eq "HELLO\nWORLD" + expect(noteable).to be_open + end + end + end +end -- GitLab