diff options
31 files changed, 1552 insertions, 58 deletions
diff --git a/CHANGELOG b/CHANGELOG index 96965a20f69..63ee7b91dcb 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 index 2e5b15f4b77..21639c7c084 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 9ece474d994..99bc1a640a8 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 96565da1bc9..edea4ad00eb 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 766b7e9cf22..f2422729364 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -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 = { diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 47efbd4a939..64d31e4a3a0 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -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| diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index ff866c2faa5..fd859e134e5 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 2d96efe1042..b365e19c4a8 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -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,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 @@ -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] diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 5e2de2ccf64..1b03d7f4c05 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 96a25330af1..0b592cd5620 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -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 diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 18971bd0be3..d7531a5d63b 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -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 diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index 23b6668e0d1..e943d2ffbcb 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -15,5 +15,9 @@ module Projects 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 00000000000..2c92a4f7de5 --- /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 49dec613716..17c04377b4c 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 00000000000..3bfc66309ba --- /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 b7ed11cb638..7cccf465334 100644 --- a/lib/gitlab/email/handler/base_handler.rb +++ b/lib/gitlab/email/handler/base_handler.rb @@ -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:" diff --git a/lib/gitlab/slash_commands/dsl.rb b/lib/gitlab/slash_commands/dsl.rb new file mode 100644 index 00000000000..3ded4109f2e --- /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 00000000000..1a854b81aca --- /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 00000000000..47c4ce306e9 --- /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 00000000000..890648f3860 --- /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 00000000000..b64d851a79c --- /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 a2119b0dadf..e2339c5e103 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 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do 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 00000000000..f8abb35674d --- /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 00000000000..fd1b30052ed --- /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 1ee9f3aae4d..fcc3c0a00bd 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -73,5 +73,7 @@ describe Issues::CreateService, services: true do 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 b84a580967a..c1e4f8bd96b 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -17,7 +17,7 @@ describe MergeRequests::CreateService, services: true do } 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 @@ describe MergeRequests::CreateService, services: true do 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 32753e84b31..36ca7d2bce8 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -4,22 +4,31 @@ describe Notes::CreateService, services: true do 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 00000000000..fa0f65495ce --- /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 00000000000..bd0201c866f --- /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 00000000000..0c8bd69add6 --- /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 00000000000..3f7ad8b2f91 --- /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 |