From 16884719d654943785864aef7b4518b5382e7265 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 18 Aug 2016 16:38:42 -0500 Subject: Merge branch '4273-slash-commands' into 'master' Support slash commands in issues / MR description & comments See merge request !5021 --- CHANGELOG | 1 + Gemfile | 3 +- Gemfile.lock | 2 + app/assets/javascripts/gfm_auto_complete.js | 272 --------------- app/assets/javascripts/gfm_auto_complete.js.es6 | 335 ++++++++++++++++++ app/assets/javascripts/notes.js | 47 +-- .../stylesheets/framework/markdown_area.scss | 5 + app/controllers/projects/commit_controller.rb | 2 +- app/controllers/projects/issues_controller.rb | 7 +- .../projects/merge_requests_controller.rb | 2 +- app/controllers/projects/notes_controller.rb | 2 +- app/controllers/projects_controller.rb | 21 +- app/finders/todos_finder.rb | 2 +- app/services/issuable_base_service.rb | 94 ++++- app/services/issues/close_service.rb | 2 + app/services/issues/create_service.rb | 27 +- app/services/issues/reopen_service.rb | 2 + app/services/merge_requests/close_service.rb | 2 + app/services/merge_requests/create_service.rb | 25 +- app/services/merge_requests/reopen_service.rb | 2 + app/services/notes/create_service.rb | 27 +- app/services/notes/slash_commands_service.rb | 33 ++ app/services/projects/autocomplete_service.rb | 27 +- app/services/projects/participants_service.rb | 38 +- app/services/slash_commands/interpret_service.rb | 236 +++++++++++++ app/services/todo_service.rb | 4 + app/views/layouts/_init_auto_complete.html.haml | 4 +- app/views/projects/_zen.html.haml | 3 +- app/views/projects/notes/_form.html.haml | 8 +- app/views/projects/notes/_hints.html.haml | 11 +- app/views/shared/issuable/_form.html.haml | 5 +- doc/workflow/README.md | 1 + doc/workflow/slash_commands.md | 30 ++ lib/gitlab/email/handler/base_handler.rb | 1 + lib/gitlab/slash_commands/command_definition.rb | 57 +++ lib/gitlab/slash_commands/dsl.rb | 98 ++++++ lib/gitlab/slash_commands/extractor.rb | 122 +++++++ .../issues/user_uses_slash_commands_spec.rb | 58 ++++ .../user_uses_slash_commands_spec.rb | 32 ++ spec/fixtures/emails/commands_in_reply.eml | 43 +++ spec/fixtures/emails/commands_only_reply.eml | 41 +++ .../email/handler/create_note_handler_spec.rb | 61 ++++ .../slash_commands/command_definition_spec.rb | 173 ++++++++++ spec/lib/gitlab/slash_commands/dsl_spec.rb | 77 +++++ spec/lib/gitlab/slash_commands/extractor_spec.rb | 215 ++++++++++++ spec/services/issues/close_service_spec.rb | 18 +- spec/services/issues/create_service_spec.rb | 2 + spec/services/issues/reopen_service_spec.rb | 25 ++ spec/services/merge_requests/close_service_spec.rb | 16 +- .../services/merge_requests/create_service_spec.rb | 11 +- .../services/merge_requests/reopen_service_spec.rb | 19 +- spec/services/notes/create_service_spec.rb | 32 +- spec/services/notes/slash_commands_service_spec.rb | 140 ++++++++ .../slash_commands/interpret_service_spec.rb | 384 +++++++++++++++++++++ spec/services/todo_service_spec.rb | 12 + ...reate_service_slash_commands_shared_examples.rb | 83 +++++ .../issuable_slash_commands_shared_examples.rb | 289 ++++++++++++++++ 57 files changed, 2884 insertions(+), 407 deletions(-) delete mode 100644 app/assets/javascripts/gfm_auto_complete.js create mode 100644 app/assets/javascripts/gfm_auto_complete.js.es6 create mode 100644 app/services/notes/slash_commands_service.rb 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/command_definition.rb 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_in_reply.eml create mode 100644 spec/fixtures/emails/commands_only_reply.eml create mode 100644 spec/lib/gitlab/slash_commands/command_definition_spec.rb 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/issues/reopen_service_spec.rb create mode 100644 spec/services/notes/slash_commands_service_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 diff --git a/CHANGELOG b/CHANGELOG index f38057793fb..21737738558 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -72,6 +72,7 @@ v 8.11.0 (unreleased) - Optimize checking if a user has read access to a list of issues !5370 - Store all DB secrets in secrets.yml, under descriptive names !5274 - Fix syntax highlighting in file editor + - Support slash commands in issue and merge request descriptions as well as comments. !5021 - Nokogiri's various parsing methods are now instrumented - Add archived badge to project list !5798 - Add simple identifier to public SSH keys (muteor) diff --git a/Gemfile b/Gemfile index 5d02a147cff..68547b6fac8 100644 --- a/Gemfile +++ b/Gemfile @@ -209,7 +209,8 @@ gem 'mousetrap-rails', '~> 1.4.6' # Detect and convert string character encoding gem 'charlock_holmes', '~> 0.7.3' -# Parse duration +# Parse time & duration +gem 'chronic', '~> 0.10.2' gem 'chronic_duration', '~> 0.10.6' gem 'sass-rails', '~> 5.0.0' diff --git a/Gemfile.lock b/Gemfile.lock index 03616c377fa..5511d718938 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -128,6 +128,7 @@ GEM mime-types (>= 1.16) cause (0.1) charlock_holmes (0.7.3) + chronic (0.10.2) chronic_duration (0.10.6) numerizer (~> 0.1.1) chunky_png (1.3.5) @@ -824,6 +825,7 @@ DEPENDENCIES capybara-screenshot (~> 1.0.0) carrierwave (~> 0.10.0) charlock_holmes (~> 0.7.3) + chronic (~> 0.10.2) chronic_duration (~> 0.10.6) coffee-rails (~> 4.1.0) connection_pool (~> 2.0) diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js deleted file mode 100644 index 2e5b15f4b77..00000000000 --- a/app/assets/javascripts/gfm_auto_complete.js +++ /dev/null @@ -1,272 +0,0 @@ -(function() { - if (window.GitLab == null) { - window.GitLab = {}; - } - - GitLab.GfmAutoComplete = { - dataLoading: false, - dataLoaded: false, - cachedData: {}, - dataSource: '', - Emoji: { - template: '
  • ${name} ${name}
  • ' - }, - Members: { - template: '
  • ${username} ${title}
  • ' - }, - Labels: { - template: '
  • ${title}
  • ' - }, - Issues: { - template: '
  • ${id} ${title}
  • ' - }, - Milestones: { - template: '
  • ${title}
  • ' - }, - Loading: { - template: '
  • Loading...
  • ' - }, - DefaultOptions: { - sorter: function(query, items, searchKey) { - if ((items[0].name != null) && items[0].name === 'loading') { - return items; - } - return $.fn.atwho["default"].callbacks.sorter(query, items, searchKey); - }, - filter: function(query, data, searchKey) { - if (data[0] === 'loading') { - return data; - } - return $.fn.atwho["default"].callbacks.filter(query, data, searchKey); - }, - beforeInsert: function(value) { - if (!GitLab.GfmAutoComplete.dataLoaded) { - return this.at; - } else { - return value; - } - } - }, - setup: function(input) { - this.input = input || $('.js-gfm-input'); - this.destroyAtWho(); - this.setupAtWho(); - if (this.dataSource) { - if (!this.dataLoading && !this.cachedData) { - this.dataLoading = true; - setTimeout((function(_this) { - return function() { - var fetch; - fetch = _this.fetchData(_this.dataSource); - return fetch.done(function(data) { - _this.dataLoading = false; - return _this.loadData(data); - }); - }; - })(this), 1000); - } - if (this.cachedData != null) { - return this.loadData(this.cachedData); - } - } - }, - setupAtWho: function() { - this.input.atwho({ - at: ':', - displayTpl: (function(_this) { - return function(value) { - if (value.path != null) { - return _this.Emoji.template; - } else { - return _this.Loading.template; - } - }; - })(this), - insertTpl: ':${name}:', - data: ['loading'], - callbacks: { - sorter: this.DefaultOptions.sorter, - filter: this.DefaultOptions.filter, - beforeInsert: this.DefaultOptions.beforeInsert - } - }); - this.input.atwho({ - at: '@', - displayTpl: (function(_this) { - return function(value) { - if (value.username != null) { - return _this.Members.template; - } else { - return _this.Loading.template; - } - }; - })(this), - insertTpl: '${atwho-at}${username}', - searchKey: 'search', - data: ['loading'], - callbacks: { - sorter: this.DefaultOptions.sorter, - filter: this.DefaultOptions.filter, - beforeInsert: this.DefaultOptions.beforeInsert, - beforeSave: function(members) { - return $.map(members, function(m) { - var title; - if (m.username == null) { - return m; - } - title = m.name; - if (m.count) { - title += " (" + m.count + ")"; - } - return { - username: m.username, - title: sanitize(title), - search: sanitize(m.username + " " + m.name) - }; - }); - } - } - }); - this.input.atwho({ - at: '#', - alias: 'issues', - searchKey: 'search', - displayTpl: (function(_this) { - return function(value) { - if (value.title != null) { - return _this.Issues.template; - } else { - return _this.Loading.template; - } - }; - })(this), - data: ['loading'], - insertTpl: '${atwho-at}${id}', - callbacks: { - sorter: this.DefaultOptions.sorter, - filter: this.DefaultOptions.filter, - beforeInsert: this.DefaultOptions.beforeInsert, - beforeSave: function(issues) { - return $.map(issues, function(i) { - if (i.title == null) { - return i; - } - return { - id: i.iid, - title: sanitize(i.title), - search: i.iid + " " + i.title - }; - }); - } - } - }); - this.input.atwho({ - at: '%', - alias: 'milestones', - searchKey: 'search', - displayTpl: (function(_this) { - return function(value) { - if (value.title != null) { - return _this.Milestones.template; - } else { - return _this.Loading.template; - } - }; - })(this), - insertTpl: '${atwho-at}"${title}"', - data: ['loading'], - callbacks: { - beforeSave: function(milestones) { - return $.map(milestones, function(m) { - if (m.title == null) { - return m; - } - return { - id: m.iid, - title: sanitize(m.title), - search: "" + m.title - }; - }); - } - } - }); - this.input.atwho({ - at: '!', - alias: 'mergerequests', - searchKey: 'search', - displayTpl: (function(_this) { - return function(value) { - if (value.title != null) { - return _this.Issues.template; - } else { - return _this.Loading.template; - } - }; - })(this), - data: ['loading'], - insertTpl: '${atwho-at}${id}', - callbacks: { - sorter: this.DefaultOptions.sorter, - filter: this.DefaultOptions.filter, - beforeInsert: this.DefaultOptions.beforeInsert, - beforeSave: function(merges) { - return $.map(merges, function(m) { - if (m.title == null) { - return m; - } - return { - id: m.iid, - title: sanitize(m.title), - search: m.iid + " " + m.title - }; - }); - } - } - }); - return this.input.atwho({ - at: '~', - alias: 'labels', - searchKey: 'search', - displayTpl: this.Labels.template, - insertTpl: '${atwho-at}${title}', - callbacks: { - beforeSave: function(merges) { - var sanitizeLabelTitle; - sanitizeLabelTitle = function(title) { - if (/[\w\?&]+\s+[\w\?&]+/g.test(title)) { - return "\"" + (sanitize(title)) + "\""; - } else { - return sanitize(title); - } - }; - return $.map(merges, function(m) { - return { - title: sanitizeLabelTitle(m.title), - color: m.color, - search: "" + m.title - }; - }); - } - } - }); - }, - destroyAtWho: function() { - return this.input.atwho('destroy'); - }, - fetchData: function(dataSource) { - return $.getJSON(dataSource); - }, - loadData: function(data) { - this.cachedData = data; - this.dataLoaded = true; - this.input.atwho('load', '@', data.members); - this.input.atwho('load', 'issues', data.issues); - this.input.atwho('load', 'milestones', data.milestones); - this.input.atwho('load', 'mergerequests', data.mergerequests); - this.input.atwho('load', ':', data.emojis); - this.input.atwho('load', '~', data.labels); - return $(':focus').trigger('keyup'); - } - }; - -}).call(this); diff --git a/app/assets/javascripts/gfm_auto_complete.js.es6 b/app/assets/javascripts/gfm_auto_complete.js.es6 new file mode 100644 index 00000000000..3dca06d36b1 --- /dev/null +++ b/app/assets/javascripts/gfm_auto_complete.js.es6 @@ -0,0 +1,335 @@ +(function() { + if (window.GitLab == null) { + window.GitLab = {}; + } + + GitLab.GfmAutoComplete = { + dataLoading: false, + dataLoaded: false, + cachedData: {}, + dataSource: '', + Emoji: { + template: '
  • ${name} ${name}
  • ' + }, + Members: { + template: '
  • ${username} ${title}
  • ' + }, + Labels: { + template: '
  • ${title}
  • ' + }, + Issues: { + template: '
  • ${id} ${title}
  • ' + }, + Milestones: { + template: '
  • ${title}
  • ' + }, + Loading: { + template: '
  • Loading...
  • ' + }, + DefaultOptions: { + sorter: function(query, items, searchKey) { + if ((items[0].name != null) && items[0].name === 'loading') { + return items; + } + return $.fn.atwho["default"].callbacks.sorter(query, items, searchKey); + }, + filter: function(query, data, searchKey) { + if (data[0] === 'loading') { + return data; + } + return $.fn.atwho["default"].callbacks.filter(query, data, searchKey); + }, + beforeInsert: function(value) { + if (!GitLab.GfmAutoComplete.dataLoaded) { + return this.at; + } else { + return value; + } + } + }, + setup: function(input) { + this.input = input || $('.js-gfm-input'); + this.destroyAtWho(); + this.setupAtWho(); + if (this.dataSource) { + if (!this.dataLoading && !this.cachedData) { + this.dataLoading = true; + setTimeout((function(_this) { + return function() { + var fetch; + fetch = _this.fetchData(_this.dataSource); + return fetch.done(function(data) { + _this.dataLoading = false; + return _this.loadData(data); + }); + }; + })(this), 1000); + } + if (this.cachedData != null) { + return this.loadData(this.cachedData); + } + } + }, + setupAtWho: function() { + this.input.atwho({ + at: ':', + displayTpl: (function(_this) { + return function(value) { + if (value.path != null) { + return _this.Emoji.template; + } else { + return _this.Loading.template; + } + }; + })(this), + insertTpl: ':${name}:', + data: ['loading'], + callbacks: { + sorter: this.DefaultOptions.sorter, + filter: this.DefaultOptions.filter, + beforeInsert: this.DefaultOptions.beforeInsert + } + }); + this.input.atwho({ + at: '@', + displayTpl: (function(_this) { + return function(value) { + if (value.username != null) { + return _this.Members.template; + } else { + return _this.Loading.template; + } + }; + })(this), + insertTpl: '${atwho-at}${username}', + searchKey: 'search', + data: ['loading'], + callbacks: { + sorter: this.DefaultOptions.sorter, + filter: this.DefaultOptions.filter, + beforeInsert: this.DefaultOptions.beforeInsert, + beforeSave: function(members) { + return $.map(members, function(m) { + var title; + if (m.username == null) { + return m; + } + title = m.name; + if (m.count) { + title += " (" + m.count + ")"; + } + return { + username: m.username, + title: sanitize(title), + search: sanitize(m.username + " " + m.name) + }; + }); + } + } + }); + this.input.atwho({ + at: '#', + alias: 'issues', + searchKey: 'search', + displayTpl: (function(_this) { + return function(value) { + if (value.title != null) { + return _this.Issues.template; + } else { + return _this.Loading.template; + } + }; + })(this), + data: ['loading'], + insertTpl: '${atwho-at}${id}', + callbacks: { + sorter: this.DefaultOptions.sorter, + filter: this.DefaultOptions.filter, + beforeInsert: this.DefaultOptions.beforeInsert, + beforeSave: function(issues) { + return $.map(issues, function(i) { + if (i.title == null) { + return i; + } + return { + id: i.iid, + title: sanitize(i.title), + search: i.iid + " " + i.title + }; + }); + } + } + }); + this.input.atwho({ + at: '%', + alias: 'milestones', + searchKey: 'search', + displayTpl: (function(_this) { + return function(value) { + if (value.title != null) { + return _this.Milestones.template; + } else { + return _this.Loading.template; + } + }; + })(this), + insertTpl: '${atwho-at}"${title}"', + data: ['loading'], + callbacks: { + beforeSave: function(milestones) { + return $.map(milestones, function(m) { + if (m.title == null) { + return m; + } + return { + id: m.iid, + title: sanitize(m.title), + search: "" + m.title + }; + }); + } + } + }); + this.input.atwho({ + at: '!', + alias: 'mergerequests', + searchKey: 'search', + displayTpl: (function(_this) { + return function(value) { + if (value.title != null) { + return _this.Issues.template; + } else { + return _this.Loading.template; + } + }; + })(this), + data: ['loading'], + insertTpl: '${atwho-at}${id}', + callbacks: { + sorter: this.DefaultOptions.sorter, + filter: this.DefaultOptions.filter, + beforeInsert: this.DefaultOptions.beforeInsert, + beforeSave: function(merges) { + return $.map(merges, function(m) { + if (m.title == null) { + return m; + } + return { + id: m.iid, + title: sanitize(m.title), + search: m.iid + " " + m.title + }; + }); + } + } + }); + this.input.atwho({ + at: '~', + alias: 'labels', + searchKey: 'search', + displayTpl: this.Labels.template, + insertTpl: '${atwho-at}${title}', + callbacks: { + beforeSave: function(merges) { + var sanitizeLabelTitle; + sanitizeLabelTitle = function(title) { + if (/[\w\?&]+\s+[\w\?&]+/g.test(title)) { + return "\"" + (sanitize(title)) + "\""; + } else { + return sanitize(title); + } + }; + return $.map(merges, function(m) { + return { + title: sanitizeLabelTitle(m.title), + color: m.color, + search: "" + m.title + }; + }); + } + } + }); + // We don't instantiate the slash commands autocomplete for note and issue/MR edit forms + this.input.filter('[data-supports-slash-commands="true"]').atwho({ + at: '/', + alias: 'commands', + searchKey: 'search', + displayTpl: function(value) { + var tpl = '
  • /${name}'; + if (value.aliases.length > 0) { + tpl += ' (or /<%- aliases.join(", /") %>)'; + } + if (value.params.length > 0) { + tpl += ' <%- params.join(" ") %>'; + } + if (value.description !== '') { + tpl += '<%- description %>'; + } + tpl += '
  • '; + return _.template(tpl)(value); + }, + insertTpl: function(value) { + var tpl = "/${name} "; + var reference_prefix = null; + if (value.params.length > 0) { + reference_prefix = value.params[0][0]; + if (/^[@%~]/.test(reference_prefix)) { + tpl += '<%- reference_prefix %>'; + } + } + return _.template(tpl)({ reference_prefix: reference_prefix }); + }, + suffix: '', + callbacks: { + sorter: this.DefaultOptions.sorter, + filter: this.DefaultOptions.filter, + beforeInsert: this.DefaultOptions.beforeInsert, + beforeSave: function(commands) { + return $.map(commands, function(c) { + var search = c.name; + if (c.aliases.length > 0) { + search = search + " " + c.aliases.join(" "); + } + return { + name: c.name, + aliases: c.aliases, + params: c.params, + description: c.description, + search: search + }; + }); + }, + matcher: function(flag, subtext, should_startWithSpace, acceptSpaceBar) { + var regexp = /(?:^|\n)\/([A-Za-z_]*)$/gi + var match = regexp.exec(subtext); + if (match) { + return match[1]; + } else { + return null; + } + } + } + }); + return; + }, + destroyAtWho: function() { + return this.input.atwho('destroy'); + }, + fetchData: function(dataSource) { + return $.getJSON(dataSource); + }, + loadData: function(data) { + this.cachedData = data; + this.dataLoaded = true; + this.input.atwho('load', '@', data.members); + this.input.atwho('load', 'issues', data.issues); + this.input.atwho('load', 'milestones', data.milestones); + 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'); + } + }; + +}).call(this); diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 9ece474d994..2484a07f363 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -201,7 +201,7 @@ Increase @pollingInterval up to 120 seconds on every function call, if `shouldReset` has a truthy value, 'null' or 'undefined' the variable will reset to @basePollingInterval. - + Note: this function is used to gradually increase the polling interval if there aren't new notes coming from the server */ @@ -223,7 +223,7 @@ /* Render note in main comments area. - + Note: for rendering inline notes use renderDiscussionNote */ @@ -231,7 +231,13 @@ var $notesList, votesBlock; if (!note.valid) { if (note.award) { - new Flash('You have already awarded this emoji!', 'alert'); + new Flash('You have already awarded this emoji!', 'alert', this.parentTimeline); + } + else { + if (note.errors.commands_only) { + new Flash(note.errors.commands_only, 'notice', this.parentTimeline); + this.refresh(); + } } return; } @@ -245,6 +251,7 @@ $notesList.append(note.html).syntaxHighlight(); gl.utils.localTimeAgo($notesList.find("#note_" + note.id + " .js-timeago"), false); this.initTaskList(); + this.refresh(); return this.updateNotesCount(1); } }; @@ -265,7 +272,7 @@ /* Render note in discussion area. - + Note: for rendering inline notes use renderDiscussionNote */ @@ -304,7 +311,7 @@ /* Called in response the main target form has been successfully submitted. - + Removes any errors. Resets text and preview. Resets buttons. @@ -329,7 +336,7 @@ /* Shows the main form and does some setup on it. - + Sets some hidden fields in the form. */ @@ -349,7 +356,7 @@ /* General note form setup. - + deactivates the submit button when text is empty hides the preview button when text is empty setup GFM auto complete @@ -366,7 +373,7 @@ /* Called in response to the new note form being submitted - + Adds new note to list. */ @@ -381,7 +388,7 @@ /* Called in response to the new note form being submitted - + Adds new note to list. */ @@ -393,7 +400,7 @@ /* Called in response to the edit note form being submitted - + Updates the current note field. */ @@ -410,7 +417,7 @@ /* Called in response to clicking the edit note link - + Replaces the note text with the note edit form Adds a data attribute to the form with the original content of the note for cancellations */ @@ -450,7 +457,7 @@ /* Called in response to clicking the edit note link - + Hides edit form and restores the original note text to the editor textarea. */ @@ -472,7 +479,7 @@ /* Called in response to deleting a note of any kind. - + Removes the actual note from view. Removes the whole discussion if the last note is being removed. */ @@ -498,7 +505,7 @@ /* Called in response to clicking the delete attachment link - + Removes the attachment wrapper view, including image tag if it exists Resets the note editing form */ @@ -515,7 +522,7 @@ /* Called when clicking on the "reply" button for a diff line. - + Shows the note form below the notes. */ @@ -531,9 +538,9 @@ /* Shows the diff or discussion form and does some setup on it. - + Sets some hidden fields in the form. - + Note: dataHolder must have the "discussionId", "lineCode", "noteableType" and "noteableId" data attributes set. */ @@ -557,7 +564,7 @@ /* Called when clicking on the "add a comment" button on the side of a diff line. - + Inserts a temporary row for the form below the line. Sets up the form and shows it. */ @@ -605,7 +612,7 @@ /* Called in response to "cancel" on a diff note form. - + Shows the reply button again. Removes the form and if necessary it's temporary row. */ @@ -634,7 +641,7 @@ /* Called after an attachment file has been selected. - + Updates the file name for the selected attachment. */ 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/commit_controller.rb b/app/controllers/projects/commit_controller.rb index f44e9bb3fd7..02fb3f56890 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -93,7 +93,7 @@ class Projects::CommitController < Projects::ApplicationController end def commit - @commit ||= @project.commit(params[:id]) + @noteable = @commit ||= @project.commit(params[:id]) end def pipelines diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index e9fb11e8f94..639cf4c0ef2 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -177,11 +177,7 @@ class Projects::IssuesController < Projects::ApplicationController protected def issue - @issue ||= begin - @project.issues.find_by!(iid: params[:id]) - rescue ActiveRecord::RecordNotFound - redirect_old - end + @noteable = @issue ||= @project.issues.find_by(iid: params[:id]) || redirect_old end alias_method :subscribable_resource, :issue alias_method :issuable, :issue @@ -226,7 +222,6 @@ class Projects::IssuesController < Projects::ApplicationController if issue redirect_to issue_path(issue) - return else raise ActiveRecord::RecordNotFound.new end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 4c1f38be9f3..0f52a5f96a3 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -381,7 +381,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def merge_request - @merge_request ||= @project.merge_requests.find_by!(iid: params[:id]) + @issuable = @merge_request ||= @project.merge_requests.find_by!(iid: params[:id]) end alias_method :subscribable_resource, :merge_request alias_method :issuable, :merge_request 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..fc52cd2f367 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -134,10 +134,22 @@ class ProjectsController < Projects::ApplicationController end def autocomplete_sources - note_type = params['type'] - note_id = params['type_id'] + noteable = + case params[:type] + when 'Issue' + IssuesFinder.new(current_user, project_id: @project.id, state: 'all'). + execute.find_by(iid: params[:type_id]) + when 'MergeRequest' + MergeRequestsFinder.new(current_user, project_id: @project.id, state: 'all'). + execute.find_by(iid: params[:type_id]) + when 'Commit' + @project.commit(params[:type_id]) + else + nil + end + autocomplete = ::Projects::AutocompleteService.new(@project, current_user) - participants = ::Projects::ParticipantsService.new(@project, current_user).execute(note_type, note_id) + participants = ::Projects::ParticipantsService.new(@project, current_user).execute(noteable) @suggestions = { emojis: Gitlab::AwardEmoji.urls, @@ -145,7 +157,8 @@ class ProjectsController < Projects::ApplicationController milestones: autocomplete.milestones, mergerequests: autocomplete.merge_requests, labels: autocomplete.labels, - members: participants + members: participants, + commands: autocomplete.commands(noteable, params[:type]) } respond_to do |format| diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 4fe0070552e..37bad596a16 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 b0ea7c905f8..e06c37c323e 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,27 +80,86 @@ class IssuableBaseService < BaseService params[key] = project.labels.where(id: params[key]).pluck(:id) end - def update_issuable(issuable, attributes) + def process_label_ids(attributes, existing_label_ids: nil) + label_ids = attributes.delete(:label_ids) + add_label_ids = attributes.delete(:add_label_ids) + remove_label_ids = attributes.delete(:remove_label_ids) + + new_label_ids = existing_label_ids || label_ids || [] + + if add_label_ids.blank? && remove_label_ids.blank? + new_label_ids = label_ids if label_ids + else + new_label_ids |= add_label_ids if add_label_ids + new_label_ids -= remove_label_ids if remove_label_ids + end + + new_label_ids + end + + def merge_slash_commands_into_params!(issuable) + description, command_params = + SlashCommands::InterpretService.new(project, current_user). + execute(params[:description], issuable) + + params[:description] = description + + params.merge!(command_params) + end + + def create_issuable(issuable, attributes, label_ids:) issuable.with_transaction_returning_status do - add_label_ids = attributes.delete(:add_label_ids) - remove_label_ids = attributes.delete(:remove_label_ids) + if issuable.save + issuable.update_attributes(label_ids: label_ids) + end + end + end - issuable.label_ids |= add_label_ids if add_label_ids - issuable.label_ids -= remove_label_ids if remove_label_ids + def create(issuable) + merge_slash_commands_into_params!(issuable) + filter_params + + params.delete(:state_event) + params[:author] ||= current_user + label_ids = process_label_ids(params) + + issuable.assign_attributes(params) + + before_create(issuable) + + if params.present? && create_issuable(issuable, params, label_ids: label_ids) + after_create(issuable) + issuable.create_cross_references!(current_user) + execute_hooks(issuable) + end + + issuable + end - issuable.assign_attributes(attributes.merge(updated_by: current_user)) + def before_create(issuable) + # To be overridden by subclasses + end + + def after_create(issuable) + # To be overridden by subclasses + end - issuable.save + def update_issuable(issuable, attributes) + issuable.with_transaction_returning_status do + issuable.update(attributes.merge(updated_by: current_user)) end end def update(issuable) change_state(issuable) change_subscription(issuable) + change_todo(issuable) filter_params old_labels = issuable.labels.to_a old_mentioned_users = issuable.mentioned_users.to_a + params[:label_ids] = process_label_ids(params, existing_label_ids: issuable.label_ids) + if params.present? && update_issuable(issuable, params) issuable.reset_events_cache handle_common_system_notes(issuable, old_labels: old_labels) @@ -135,6 +189,16 @@ class IssuableBaseService < BaseService end end + def change_todo(issuable) + case params.delete(:todo_event) + when 'add' + todo_service.mark_todo(issuable, current_user) + when 'done' + todo = TodosFinder.new(current_user).execute.find_by(target: issuable) + todo_service.mark_todos_as_done([todo], current_user) if todo + end + end + def has_changes?(issuable, old_labels: []) valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch] diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 859c934ea3b..45cca216ccc 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -1,6 +1,8 @@ module Issues class CloseService < Issues::BaseService def execute(issue, commit: nil, notifications: true, system_note: true) + return issue unless can?(current_user, :update_issue, issue) + if project.jira_tracker? && project.jira_service.active project.jira_service.execute(commit, issue) todo_service.close_issue(issue, current_user) diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 65550ab8ec6..ea1690f3e38 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -1,26 +1,23 @@ module Issues class CreateService < Issues::BaseService def execute - filter_params - label_params = params.delete(:label_ids) @request = params.delete(:request) @api = params.delete(:api) - @issue = project.issues.new(params) - @issue.author = params[:author] || current_user - @issue.spam = spam_service.check(@api) + @issue = project.issues.new - if @issue.save - @issue.update_attributes(label_ids: label_params) - notification_service.new_issue(@issue, current_user) - todo_service.new_issue(@issue, current_user) - event_service.open_issue(@issue, current_user) - user_agent_detail_service.create - @issue.create_cross_references!(current_user) - execute_hooks(@issue, 'open') - end + create(@issue) + end + + def before_create(issuable) + issuable.spam = spam_service.check(@api) + end - @issue + def after_create(issuable) + event_service.open_issue(issuable, current_user) + notification_service.new_issue(issuable, current_user) + todo_service.new_issue(issuable, current_user) + user_agent_detail_service.create end private diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index e48ca359f4f..40fbe354492 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -1,6 +1,8 @@ module Issues class ReopenService < Issues::BaseService def execute(issue) + return issue unless can?(current_user, :update_issue, issue) + if issue.reopen event_service.reopen_issue(issue, current_user) create_note(issue) diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index 27ee81fe3e7..f2053bda83a 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -1,6 +1,8 @@ module MergeRequests class CloseService < MergeRequests::BaseService def execute(merge_request, commit = nil) + return merge_request unless can?(current_user, :update_merge_request, merge_request) + # If we close MergeRequest we want to ignore validation # so we can close broken one (Ex. fork project removed) merge_request.allow_broken = true diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 96a25330af1..73247e62421 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 after_create(issuable) + event_service.open_mr(issuable, current_user) + notification_service.new_merge_request(issuable, current_user) + todo_service.new_merge_request(issuable, current_user) end end end diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb index eb88ae9d11c..fadcce5d9b6 100644 --- a/app/services/merge_requests/reopen_service.rb +++ b/app/services/merge_requests/reopen_service.rb @@ -1,6 +1,8 @@ module MergeRequests class ReopenService < MergeRequests::BaseService def execute(merge_request) + return merge_request unless can?(current_user, :update_merge_request, merge_request) + if merge_request.reopen event_service.reopen_mr(merge_request, current_user) create_note(merge_request) diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 18971bd0be3..a36008c3ef5 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -11,10 +11,33 @@ module Notes return noteable.create_award_emoji(note.award_emoji_name, current_user) end - if note.save + # We execute commands (extracted from `params[:note]`) on the noteable + # **before** we save the note because if the note consists of commands + # only, there is no need be create a note! + slash_commands_service = SlashCommandsService.new(project, current_user) + + if slash_commands_service.supported?(note) + content, command_params = slash_commands_service.extract_commands(note) + + only_commands = content.empty? + + note.note = content + end + + if !only_commands && note.save # Finish the harder work in the background NewNoteWorker.perform_in(2.seconds, note.id, params) - TodoService.new.new_note(note, current_user) + todo_service.new_note(note, current_user) + end + + if command_params && command_params.any? + slash_commands_service.execute(command_params, note) + + # We must add the error after we call #save because errors are reset + # when #save is called + if only_commands + note.errors.add(:commands_only, 'Your commands have been executed!') + end end note diff --git a/app/services/notes/slash_commands_service.rb b/app/services/notes/slash_commands_service.rb new file mode 100644 index 00000000000..4a9a8a64653 --- /dev/null +++ b/app/services/notes/slash_commands_service.rb @@ -0,0 +1,33 @@ +module Notes + class SlashCommandsService < BaseService + UPDATE_SERVICES = { + 'Issue' => Issues::UpdateService, + 'MergeRequest' => MergeRequests::UpdateService + } + + def supported?(note) + noteable_update_service(note) && + can?(current_user, :"update_#{note.noteable_type.underscore}", note.noteable) + end + + def extract_commands(note) + return [note.note, {}] unless supported?(note) + + SlashCommands::InterpretService.new(project, current_user). + execute(note.note, note.noteable) + end + + def execute(command_params, note) + return if command_params.empty? + return unless supported?(note) + + noteable_update_service(note).new(project, current_user, command_params).execute(note.noteable) + end + + private + + def noteable_update_service(note) + UPDATE_SERVICES[note.noteable_type] + end + end +end diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index 23b6668e0d1..f578f8dbea2 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -1,7 +1,7 @@ module Projects class AutocompleteService < BaseService def issues - @project.issues.visible_to_user(current_user).opened.select([:iid, :title]) + IssuesFinder.new(current_user, project_id: project.id, state: 'opened').execute.select([:iid, :title]) end def milestones @@ -9,11 +9,34 @@ module Projects end def merge_requests - @project.merge_requests.opened.select([:iid, :title]) + MergeRequestsFinder.new(current_user, project_id: project.id, state: 'opened').execute.select([:iid, :title]) end def labels @project.labels.select([:title, :color]) end + + def commands(noteable, type) + noteable ||= + case type + when 'Issue' + @project.issues.build + when 'MergeRequest' + @project.merge_requests.build + end + + return [] unless noteable && noteable.is_a?(Issuable) + + opts = { + project: project, + issuable: noteable, + current_user: current_user + } + SlashCommands::InterpretService.command_definitions.map do |definition| + next unless definition.available?(opts) + + definition.to_h(opts) + end.compact + end end end diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index 02c4eee3d02..d38328403c1 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -1,40 +1,28 @@ module Projects class ParticipantsService < BaseService - def execute(noteable_type, noteable_id) - @noteable_type = noteable_type - @noteable_id = noteable_id + attr_reader :noteable + + def execute(noteable) + @noteable = noteable + project_members = sorted(project.team.members) - participants = target_owner + participants_in_target + all_members + groups + project_members + participants = noteable_owner + participants_in_noteable + all_members + groups + project_members participants.uniq end - def target - @target ||= - case @noteable_type - when "Issue" - project.issues.find_by_iid(@noteable_id) - when "MergeRequest" - project.merge_requests.find_by_iid(@noteable_id) - when "Commit" - project.commit(@noteable_id) - else - nil - end - end - - def target_owner - return [] unless target && target.author.present? + def noteable_owner + return [] unless noteable && noteable.author.present? [{ - name: target.author.name, - username: target.author.username + name: noteable.author.name, + username: noteable.author.username }] end - def participants_in_target - return [] unless target + def participants_in_noteable + return [] unless noteable - users = target.participants(current_user) + users = noteable.participants(current_user) sorted(users) 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..9ac1124abc1 --- /dev/null +++ b/app/services/slash_commands/interpret_service.rb @@ -0,0 +1,236 @@ +module SlashCommands + class InterpretService < BaseService + include Gitlab::SlashCommands::Dsl + + attr_reader :issuable + + # Takes a text and interprets the commands that are extracted from it. + # Returns the content without commands, and hash of changes to be applied to a record. + def execute(content, issuable) + @issuable = issuable + @updates = {} + + opts = { + issuable: issuable, + current_user: current_user, + project: project + } + + content, commands = extractor.extract_commands(content, opts) + + commands.each do |name, arg| + definition = self.class.command_definitions_by_name[name.to_sym] + next unless definition + + definition.execute(self, opts, arg) + end + + [content, @updates] + end + + private + + def extractor + Gitlab::SlashCommands::Extractor.new(self.class.command_definitions) + end + + desc do + "Close this #{issuable.to_ability_name.humanize(capitalize: false)}" + end + condition do + issuable.persisted? && + issuable.open? && + current_user.can?(:"update_#{issuable.to_ability_name}", issuable) + end + command :close do + @updates[:state_event] = 'close' + end + + desc do + "Reopen this #{issuable.to_ability_name.humanize(capitalize: false)}" + end + condition do + issuable.persisted? && + issuable.closed? && + current_user.can?(:"update_#{issuable.to_ability_name}", issuable) + end + command :reopen do + @updates[:state_event] = 'reopen' + end + + desc 'Change title' + params '' + condition do + issuable.persisted? && + current_user.can?(:"update_#{issuable.to_ability_name}", issuable) + end + command :title do |title_param| + @updates[:title] = title_param + end + + desc 'Assign' + params '@user' + condition do + current_user.can?(:"admin_#{issuable.to_ability_name}", project) + end + command :assign do |assignee_param| + user = extract_references(assignee_param, :user).first + user ||= User.find_by(username: assignee_param) + + @updates[:assignee_id] = user.id if user + end + + desc 'Remove assignee' + condition do + issuable.persisted? && + issuable.assignee_id? && + current_user.can?(:"admin_#{issuable.to_ability_name}", project) + end + command :unassign do + @updates[:assignee_id] = nil + end + + desc 'Set milestone' + params '%"milestone"' + condition do + current_user.can?(:"admin_#{issuable.to_ability_name}", project) && + project.milestones.active.any? + end + command :milestone do |milestone_param| + milestone = extract_references(milestone_param, :milestone).first + milestone ||= project.milestones.find_by(title: milestone_param.strip) + + @updates[:milestone_id] = milestone.id if milestone + end + + desc 'Remove milestone' + condition do + issuable.persisted? && + issuable.milestone_id? && + current_user.can?(:"admin_#{issuable.to_ability_name}", project) + end + command :remove_milestone do + @updates[:milestone_id] = nil + end + + desc 'Add label(s)' + params '~label1 ~"label 2"' + condition do + current_user.can?(:"admin_#{issuable.to_ability_name}", project) && + project.labels.any? + end + command :label do |labels_param| + label_ids = find_label_ids(labels_param) + + @updates[:add_label_ids] = label_ids unless label_ids.empty? + end + + desc 'Remove all or specific label(s)' + params '~label1 ~"label 2"' + condition do + issuable.persisted? && + issuable.labels.any? && + current_user.can?(:"admin_#{issuable.to_ability_name}", project) + end + command :unlabel do |labels_param = nil| + if labels_param.present? + label_ids = find_label_ids(labels_param) + + @updates[:remove_label_ids] = label_ids unless label_ids.empty? + else + @updates[:label_ids] = [] + end + end + + desc 'Replace all label(s)' + params '~label1 ~"label 2"' + condition do + issuable.persisted? && + issuable.labels.any? && + current_user.can?(:"admin_#{issuable.to_ability_name}", project) + end + command :relabel do |labels_param| + label_ids = find_label_ids(labels_param) + + @updates[:label_ids] = label_ids unless label_ids.empty? + end + + desc 'Add a todo' + condition do + issuable.persisted? && + !TodoService.new.todo_exist?(issuable, current_user) + end + command :todo do + @updates[:todo_event] = 'add' + end + + desc 'Mark todo as done' + condition do + issuable.persisted? && + TodoService.new.todo_exist?(issuable, current_user) + end + command :done do + @updates[:todo_event] = 'done' + end + + desc 'Subscribe' + condition do + issuable.persisted? && + !issuable.subscribed?(current_user) + end + command :subscribe do + @updates[:subscription_event] = 'subscribe' + end + + desc 'Unsubscribe' + condition do + issuable.persisted? && + issuable.subscribed?(current_user) + end + command :unsubscribe do + @updates[:subscription_event] = 'unsubscribe' + end + + desc 'Set due date' + params '' + condition do + issuable.respond_to?(:due_date) && + current_user.can?(:"update_#{issuable.to_ability_name}", issuable) + end + command :due do |due_date_param| + due_date = Chronic.parse(due_date_param).try(:to_date) + + @updates[:due_date] = due_date if due_date + end + + desc 'Remove due date' + condition do + issuable.persisted? && + issuable.respond_to?(:due_date) && + issuable.due_date? && + current_user.can?(:"update_#{issuable.to_ability_name}", issuable) + end + command :remove_due_date do + @updates[:due_date] = nil + end + + # This is a dummy command, so that it appears in the autocomplete commands + desc 'CC' + params '@user' + command :cc + + def find_label_ids(labels_param) + label_ids_by_reference = extract_references(labels_param, :label).map(&:id) + labels_ids_by_name = @project.labels.where(name: labels_param.split).select(:id) + + label_ids_by_reference | labels_ids_by_name + end + + def extract_references(arg, type) + ext = Gitlab::ReferenceExtractor.new(project, current_user) + ext.analyze(arg, author: current_user) + + ext.references(type) + end + end +end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index daf4339cb48..e0ccb654590 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -159,6 +159,10 @@ class TodoService create_todos(current_user, attributes) end + def todo_exist?(issuable, current_user) + TodosFinder.new(current_user).execute.exists?(target: issuable) + end + private def create_todos(users, attributes) diff --git a/app/views/layouts/_init_auto_complete.html.haml b/app/views/layouts/_init_auto_complete.html.haml index 351100f3523..67ff4b272b9 100644 --- a/app/views/layouts/_init_auto_complete.html.haml +++ b/app/views/layouts/_init_auto_complete.html.haml @@ -1,7 +1,7 @@ - project = @target_project || @project -- noteable_class = @noteable.class if @noteable.present? +- noteable_type = @noteable.class if @noteable.present? :javascript - GitLab.GfmAutoComplete.dataSource = "#{autocomplete_sources_namespace_project_path(project.namespace, project, type: noteable_class, type_id: params[:id])}" + GitLab.GfmAutoComplete.dataSource = "#{autocomplete_sources_namespace_project_path(project.namespace, project, type: noteable_type, type_id: params[:id])}" GitLab.GfmAutoComplete.cachedData = undefined; GitLab.GfmAutoComplete.setup(); diff --git a/app/views/projects/_zen.html.haml b/app/views/projects/_zen.html.haml index 413477a2d3a..3978fa60d66 100644 --- a/app/views/projects/_zen.html.haml +++ b/app/views/projects/_zen.html.haml @@ -1,7 +1,8 @@ +- supports_slash_commands = local_assigns.fetch(:supports_slash_commands, false) .zen-backdrop - classes << ' js-gfm-input js-autosize markdown-area' - if defined?(f) && f - = f.text_area attr, class: classes, placeholder: placeholder + = f.text_area attr, class: classes, placeholder: placeholder, data: { supports_slash_commands: supports_slash_commands } - else = text_area_tag attr, nil, class: classes, placeholder: placeholder %a.zen-control.zen-control-leave.js-zen-leave{ href: "#" } diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index 7c61ba750fe..759c72b2477 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -10,8 +10,12 @@ = f.hidden_field :position = render layout: 'projects/md_preview', locals: { preview_class: "md-preview", referenced_users: true } do - = render 'projects/zen', f: f, attr: :note, classes: 'note-textarea js-note-text', placeholder: "Write a comment or drag your files here..." - = render 'projects/notes/hints' + = render 'projects/zen', f: f, + attr: :note, + classes: 'note-textarea js-note-text', + placeholder: "Write a comment or drag your files here...", + supports_slash_commands: true + = render 'projects/notes/hints', supports_slash_commands: true .error-alert .note-form-actions.clearfix diff --git a/app/views/projects/notes/_hints.html.haml b/app/views/projects/notes/_hints.html.haml index 25466e7562e..cf6e14648cc 100644 --- a/app/views/projects/notes/_hints.html.haml +++ b/app/views/projects/notes/_hints.html.haml @@ -1,8 +1,15 @@ +- supports_slash_commands = local_assigns.fetch(:supports_slash_commands, false) .comment-toolbar.clearfix .toolbar-text Styling with = link_to 'Markdown', help_page_path('markdown/markdown'), target: '_blank', tabindex: -1 - is supported + - if supports_slash_commands + and + = link_to 'slash commands', help_page_path('workflow/slash_commands'), target: '_blank', tabindex: -1 + are + - else + is + supported %button.toolbar-button.markdown-selector{ type: 'button', tabindex: '-1' } = icon('file-image-o', class: 'toolbar-button-icon') - Attach a file \ No newline at end of file + Attach a file diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index c30bdb0ae91..25b29ccf70d 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -30,8 +30,9 @@ = render layout: 'projects/md_preview', locals: { preview_class: "md-preview", referenced_users: true } do = render 'projects/zen', f: f, attr: :description, classes: 'note-textarea', - placeholder: "Write a comment or drag your files here..." - = render 'projects/notes/hints' + placeholder: "Write a comment or drag your files here...", + supports_slash_commands: !issuable.persisted? + = render 'projects/notes/hints', supports_slash_commands: !issuable.persisted? .clearfix .error-alert 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..91d69d4e77e --- /dev/null +++ b/doc/workflow/slash_commands.md @@ -0,0 +1,30 @@ +# 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. The commands are removed from the issue, merge request or +comment body before it is saved and will not be visible to anyone else. + +Below is a list of all of the available commands and descriptions about what they +do. + +| Command | Action | +|:---------------------------|:-------------| +| `/close` | Close the issue or merge request | +| `/reopen` | Reopen the issue or merge request | +| `/title ` | Change title | +| `/assign @username` | Assign | +| `/unassign` | Remove assignee | +| `/milestone %milestone` | Set milestone | +| `/remove_milestone` | Remove milestone | +| `/label ~foo ~"bar baz"` | Add label(s) | +| `/unlabel ~foo ~"bar baz"` | Remove all or specific label(s) | +| `/relabel ~foo ~"bar baz"` | Replace all label(s) | +| `/todo` | Add a todo | +| `/done` | Mark todo as done | +| `/subscribe` | Subscribe | +| `/unsubscribe` | Unsubscribe | +| `/due ` | Set due date | +| `/remove_due_date` | 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/command_definition.rb b/lib/gitlab/slash_commands/command_definition.rb new file mode 100644 index 00000000000..60d35be2599 --- /dev/null +++ b/lib/gitlab/slash_commands/command_definition.rb @@ -0,0 +1,57 @@ +module Gitlab + module SlashCommands + class CommandDefinition + attr_accessor :name, :aliases, :description, :params, :condition_block, :action_block + + def initialize(name, attributes = {}) + @name = name + + @aliases = attributes[:aliases] || [] + @description = attributes[:description] || '' + @params = attributes[:params] || [] + @condition_block = attributes[:condition_block] + @action_block = attributes[:action_block] + end + + def all_names + [name, *aliases] + end + + def noop? + action_block.nil? + end + + def available?(opts) + return true unless condition_block + + context = OpenStruct.new(opts) + context.instance_exec(&condition_block) + end + + def execute(context, opts, arg) + return if noop? || !available?(opts) + + if arg.present? + context.instance_exec(arg, &action_block) + elsif action_block.arity == 0 + context.instance_exec(&action_block) + end + end + + def to_h(opts) + desc = description + if desc.respond_to?(:call) + context = OpenStruct.new(opts) + desc = context.instance_exec(&desc) rescue '' + end + + { + name: name, + aliases: aliases, + description: desc, + params: params + } + end + end + end +end diff --git a/lib/gitlab/slash_commands/dsl.rb b/lib/gitlab/slash_commands/dsl.rb new file mode 100644 index 00000000000..50b0937d267 --- /dev/null +++ b/lib/gitlab/slash_commands/dsl.rb @@ -0,0 +1,98 @@ +module Gitlab + module SlashCommands + module Dsl + extend ActiveSupport::Concern + + included do + cattr_accessor :command_definitions, instance_accessor: false do + [] + end + + cattr_accessor :command_definitions_by_name, instance_accessor: false do + {} + end + end + + class_methods do + # Allows to give a description to the next slash command. + # This description is shown in the autocomplete menu. + # It accepts a block that will be evaluated with the context given to + # `CommandDefintion#to_h`. + # + # Example: + # + # desc do + # "This is a dynamic description for #{noteable.to_ability_name}" + # end + # command :command_key do |arguments| + # # Awesome code block + # end + def desc(text = '', &block) + @description = block_given? ? block : text + end + + # Allows to define params for the next slash command. + # These params are shown in the autocomplete menu. + # + # Example: + # + # params "~label ~label2" + # command :command_key do |arguments| + # # Awesome code block + # end + def params(*params) + @params = params + end + + # Allows to define conditions that must be met in order for the command + # to be returned by `.command_names` & `.command_definitions`. + # It accepts a block that will be evaluated with the context given to + # `CommandDefintion#to_h`. + # + # Example: + # + # condition do + # project.public? + # end + # command :command_key do |arguments| + # # Awesome code block + # end + def condition(&block) + @condition_block = block + end + + # Registers a new command which is recognizeable from body of email or + # comment. + # It accepts aliases and takes a block. + # + # Example: + # + # command :my_command, :alias_for_my_command do |arguments| + # # Awesome code block + # end + def command(*command_names, &block) + name, *aliases = command_names + + definition = CommandDefinition.new( + name, + aliases: aliases, + description: @description, + params: @params, + condition_block: @condition_block, + action_block: block + ) + + self.command_definitions << definition + + definition.all_names.each do |name| + self.command_definitions_by_name[name] = definition + end + + @description = nil + @params = nil + @condition_block = 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..a672e5e4855 --- /dev/null +++ b/lib/gitlab/slash_commands/extractor.rb @@ -0,0 +1,122 @@ +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_definitions + + def initialize(command_definitions) + @command_definitions = command_definitions + 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(msg) #=> [['labels', '~foo ~"bar baz"']] + # msg #=> "hello\nworld" + # ``` + def extract_commands(content, opts = {}) + return [content, []] unless content + + content = content.dup + + commands = [] + + content.delete!("\r") + content.gsub!(commands_regex(opts)) do + if $~[:cmd] + commands << [$~[:cmd], $~[:arg]].reject(&:blank?) + '' + else + $~[0] + end + end + + [content.strip, 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: + # + # /^\/(?close|reopen|...)(?:( |$))(?[^\/\n]*)(?:\n|$)/ + def commands_regex(opts) + names = command_names(opts).map(&:to_s) + + @commands_regex ||= %r{ + (? + # Code blocks: + # ``` + # Anything, including `/cmd arg` which are ignored by this filter + # ``` + + ^``` + .+? + \n```$ + ) + | + (? + # HTML block: + # + # Anything, including `/cmd arg` which are ignored by this filter + # + + ^<[^>]+?>\n + .+? + \n<\/[^>]+?>$ + ) + | + (? + # Quote block: + # >>> + # Anything, including `/cmd arg` which are ignored by this filter + # >>> + + ^>>> + .+? + \n>>>$ + ) + | + (?: + # Command not in a blockquote, blockcode, or HTML tag: + # /close + + ^\/ + (?#{Regexp.union(names)}) + (?: + [ ] + (?[^\/\n]*) + )? + (?:\n|$) + ) + }mx + end + + def command_names(opts) + command_definitions.flat_map do |command| + next if command.noop? + + command.all_names + end.compact + 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..2883e392694 --- /dev/null +++ b/spec/features/issues/user_uses_slash_commands_spec.rb @@ -0,0 +1,58 @@ +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 2016-08-28" + click_button 'Comment' + end + + expect(page).not_to have_content '/due 2016-08-28' + expect(page).to have_content 'Your commands have been 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: "/remove_due_date" + click_button 'Comment' + end + + expect(page).not_to have_content '/remove_due_date' + expect(page).to have_content 'Your commands have been 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..d9ef0d18074 --- /dev/null +++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb @@ -0,0 +1,32 @@ +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 2016-08-28" + click_button 'Comment' + end + + expect(page).not_to have_content '/due 2016-08-28' + end + end +end diff --git a/spec/fixtures/emails/commands_in_reply.eml b/spec/fixtures/emails/commands_in_reply.eml new file mode 100644 index 00000000000..06bf60ab734 --- /dev/null +++ b/spec/fixtures/emails/commands_in_reply.eml @@ -0,0 +1,43 @@ +Return-Path: +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 ; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; 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 +To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo +Message-ID: +In-Reply-To: +References: +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 + +Cool! + +/close +/todo +/due tomorrow + + +On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta + 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/fixtures/emails/commands_only_reply.eml b/spec/fixtures/emails/commands_only_reply.eml new file mode 100644 index 00000000000..aed64224b06 --- /dev/null +++ b/spec/fixtures/emails/commands_only_reply.eml @@ -0,0 +1,41 @@ +Return-Path: +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 ; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; 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 +To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo +Message-ID: +In-Reply-To: +References: +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 +/todo +/due tomorrow + + +On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta + 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..4909fed6b77 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,67 @@ 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") } + + context 'and current user cannot update noteable' do + it 'raises a CommandsOnlyNoteError' do + expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError) + end + end + + context 'and current user can update noteable' do + before do + project.team << [user, :developer] + end + + it 'does not raise an error' do + expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy + + # One system note is created for the 'close' event + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + + expect(noteable.reload).to be_closed + expect(noteable.due_date).to eq(Date.tomorrow) + expect(TodoService.new.todo_exist?(noteable, user)).to be_truthy + end + end + end + end + + context 'when the note contains slash commands' do + let!(:email_raw) { fixture_file("emails/commands_in_reply.eml") } + + context 'and current user cannot update noteable' do + it 'post a note and does not update the noteable' do + expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy + + # One system note is created for the new note + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + + expect(noteable.reload).to be_open + expect(noteable.due_date).to be_nil + expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy + end + end + + context 'and current user can update noteable' do + before do + project.team << [user, :developer] + end + + it 'post a note and updates the noteable' do + expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy + + # One system note is created for the new note, one for the 'close' event + expect { receiver.execute }.to change { noteable.notes.count }.by(2) + + expect(noteable.reload).to be_closed + expect(noteable.due_date).to eq(Date.tomorrow) + expect(TodoService.new.todo_exist?(noteable, user)).to be_truthy + end + end end context "when the reply is blank" do diff --git a/spec/lib/gitlab/slash_commands/command_definition_spec.rb b/spec/lib/gitlab/slash_commands/command_definition_spec.rb new file mode 100644 index 00000000000..c9c2f314e57 --- /dev/null +++ b/spec/lib/gitlab/slash_commands/command_definition_spec.rb @@ -0,0 +1,173 @@ +require 'spec_helper' + +describe Gitlab::SlashCommands::CommandDefinition do + subject { described_class.new(:command) } + + describe "#all_names" do + context "when the command has aliases" do + before do + subject.aliases = [:alias1, :alias2] + end + + it "returns an array with the name and aliases" do + expect(subject.all_names).to eq([:command, :alias1, :alias2]) + end + end + + context "when the command doesn't have aliases" do + it "returns an array with the name" do + expect(subject.all_names).to eq([:command]) + end + end + end + + describe "#noop?" do + context "when the command has an action block" do + before do + subject.action_block = proc { } + end + + it "returns false" do + expect(subject.noop?).to be false + end + end + + context "when the command doesn't have an action block" do + it "returns true" do + expect(subject.noop?).to be true + end + end + end + + describe "#available?" do + let(:opts) { { go: false } } + + context "when the command has a condition block" do + before do + subject.condition_block = proc { go } + end + + context "when the condition block returns true" do + before do + opts[:go] = true + end + + it "returns true" do + expect(subject.available?(opts)).to be true + end + end + + context "when the condition block returns false" do + it "returns false" do + expect(subject.available?(opts)).to be false + end + end + end + + context "when the command doesn't have a condition block" do + it "returns true" do + expect(subject.available?(opts)).to be true + end + end + end + + describe "#execute" do + let(:context) { OpenStruct.new(run: false) } + + context "when the command is a noop" do + it "doesn't execute the command" do + expect(context).not_to receive(:instance_exec) + + subject.execute(context, {}, nil) + + expect(context.run).to be false + end + end + + context "when the command is not a noop" do + before do + subject.action_block = proc { self.run = true } + end + + context "when the command is not available" do + before do + subject.condition_block = proc { false } + end + + it "doesn't execute the command" do + subject.execute(context, {}, nil) + + expect(context.run).to be false + end + end + + context "when the command is available" do + context "when the commnd has no arguments" do + before do + subject.action_block = proc { self.run = true } + end + + context "when the command is provided an argument" do + it "executes the command" do + subject.execute(context, {}, true) + + expect(context.run).to be true + end + end + + context "when the command is not provided an argument" do + it "executes the command" do + subject.execute(context, {}, nil) + + expect(context.run).to be true + end + end + end + + context "when the command has 1 required argument" do + before do + subject.action_block = ->(arg) { self.run = arg } + end + + context "when the command is provided an argument" do + it "executes the command" do + subject.execute(context, {}, true) + + expect(context.run).to be true + end + end + + context "when the command is not provided an argument" do + it "doesn't execute the command" do + subject.execute(context, {}, nil) + + expect(context.run).to be false + end + end + end + + context "when the command has 1 optional argument" do + before do + subject.action_block = proc { |arg = nil| self.run = arg || true } + end + + context "when the command is provided an argument" do + it "executes the command" do + subject.execute(context, {}, true) + + expect(context.run).to be true + end + end + + context "when the command is not provided an argument" do + it "executes the command" do + subject.execute(context, {}, nil) + + expect(context.run).to be true + end + end + end + end + end + end +end 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..26217a0e3b2 --- /dev/null +++ b/spec/lib/gitlab/slash_commands/dsl_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe Gitlab::SlashCommands::Dsl do + before :all do + DummyClass = Struct.new(:project) do + include Gitlab::SlashCommands::Dsl + + desc 'A command with no args' + command :no_args, :none do + "Hello World!" + end + + params 'The first argument' + command :one_arg, :once, :first do |arg1| + arg1 + end + + desc do + "A dynamic description for #{noteable.upcase}" + end + params 'The first argument', 'The second argument' + command :two_args do |arg1, arg2| + [arg1, arg2] + end + + command :cc + + condition do + project == 'foo' + end + command :cond_action do |arg| + arg + end + end + end + + describe '.command_definitions' do + it 'returns an array with commands definitions' do + no_args_def, one_arg_def, two_args_def, cc_def, cond_action_def = DummyClass.command_definitions + + expect(no_args_def.name).to eq(:no_args) + expect(no_args_def.aliases).to eq([:none]) + expect(no_args_def.description).to eq('A command with no args') + expect(no_args_def.params).to eq([]) + expect(no_args_def.condition_block).to be_nil + expect(no_args_def.action_block).to be_a_kind_of(Proc) + + expect(one_arg_def.name).to eq(:one_arg) + expect(one_arg_def.aliases).to eq([:once, :first]) + expect(one_arg_def.description).to eq('') + expect(one_arg_def.params).to eq(['The first argument']) + expect(one_arg_def.condition_block).to be_nil + expect(one_arg_def.action_block).to be_a_kind_of(Proc) + + expect(two_args_def.name).to eq(:two_args) + expect(two_args_def.aliases).to eq([]) + expect(two_args_def.to_h(noteable: "issue")[:description]).to eq('A dynamic description for ISSUE') + expect(two_args_def.params).to eq(['The first argument', 'The second argument']) + expect(two_args_def.condition_block).to be_nil + expect(two_args_def.action_block).to be_a_kind_of(Proc) + + expect(cc_def.name).to eq(:cc) + expect(cc_def.aliases).to eq([]) + expect(cc_def.description).to eq('') + expect(cc_def.params).to eq([]) + expect(cc_def.condition_block).to be_nil + expect(cc_def.action_block).to be_nil + + expect(cond_action_def.name).to eq(:cond_action) + expect(cond_action_def.aliases).to eq([]) + expect(cond_action_def.description).to eq('') + expect(cond_action_def.params).to eq([]) + expect(cond_action_def.condition_block).to be_a_kind_of(Proc) + expect(cond_action_def.action_block).to be_a_kind_of(Proc) + 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..1e4954c4af8 --- /dev/null +++ b/spec/lib/gitlab/slash_commands/extractor_spec.rb @@ -0,0 +1,215 @@ +require 'spec_helper' + +describe Gitlab::SlashCommands::Extractor do + let(:definitions) do + Class.new do + include Gitlab::SlashCommands::Dsl + + command(:reopen, :open) { } + command(:assign) { } + command(:labels) { } + command(:power) { } + end.command_definitions + end + + let(:extractor) { described_class.new(definitions) } + + shared_examples 'command with no argument' do + it 'extracts command' do + msg, commands = extractor.extract_commands(original_msg) + + expect(commands).to eq [['reopen']] + expect(msg).to eq final_msg + end + end + + shared_examples 'command with a single argument' do + it 'extracts command' do + msg, commands = extractor.extract_commands(original_msg) + + expect(commands).to eq [['assign', '@joe']] + expect(msg).to eq final_msg + end + end + + shared_examples 'command with multiple arguments' do + it 'extracts command' do + msg, commands = extractor.extract_commands(original_msg) + + expect(commands).to eq [['labels', '~foo ~"bar baz" label']] + expect(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) { "/reopen\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/reopen\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 /reopen" + msg, commands = extractor.extract_commands(msg) + + expect(commands).to be_empty + expect(msg).to eq "hello\nworld /reopen" + end + end + + context 'at the end of content' do + it_behaves_like 'command with no argument' do + let(:original_msg) { "hello\n/reopen" } + let(:final_msg) { "hello" } + 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" + msg, 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" } + end + end + + context 'when argument is not separated with a space' do + it 'does not extract command' do + msg = "hello\n/assign@joe\nworld" + msg, 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) + msg, 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" } + 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) + msg, 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) + msg, 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/reopen) + msg, commands = extractor.extract_commands(msg) + + expect(commands).to eq [['power', '@user.name %9.10 ~"bar baz.2" label'], ['reopen']] + expect(msg).to eq "hello\nworld" + end + + it 'does not alter original content if no command is found' do + msg = 'Fixes #123' + msg, commands = extractor.extract_commands(msg) + + expect(commands).to be_empty + expect(msg).to eq 'Fixes #123' + end + + it 'does not extract commands inside a blockcode' do + msg = "Hello\r\n```\r\nThis is some text\r\n/close\r\n/assign @user\r\n```\r\n\r\nWorld" + expected = msg.delete("\r") + msg, commands = extractor.extract_commands(msg) + + expect(commands).to be_empty + expect(msg).to eq expected + end + + it 'does not extract commands inside a blockquote' do + msg = "Hello\r\n>>>\r\nThis is some text\r\n/close\r\n/assign @user\r\n>>>\r\n\r\nWorld" + expected = msg.delete("\r") + msg, commands = extractor.extract_commands(msg) + + expect(commands).to be_empty + expect(msg).to eq expected + end + + it 'does not extract commands inside a HTML tag' do + msg = "Hello\r\n
    \r\nThis is some text\r\n/close\r\n/assign @user\r\n
    \r\n\r\nWorld" + expected = msg.delete("\r") + msg, commands = extractor.extract_commands(msg) + + expect(commands).to be_empty + expect(msg).to eq expected + end + end +end diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 1318607a388..aff022a573e 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe Issues::CloseService, services: true do let(:user) { create(:user) } let(:user2) { create(:user) } + let(:guest) { create(:user) } let(:issue) { create(:issue, assignee: user2) } let(:project) { issue.project } let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } @@ -10,13 +11,14 @@ describe Issues::CloseService, services: true do before do project.team << [user, :master] project.team << [user2, :developer] + project.team << [guest, :guest] end describe '#execute' do context "valid params" do before do perform_enqueued_jobs do - @issue = Issues::CloseService.new(project, user, {}).execute(issue) + @issue = described_class.new(project, user, {}).execute(issue) end end @@ -39,10 +41,22 @@ describe Issues::CloseService, services: true do end end + context 'current user is not authorized to close issue' do + before do + perform_enqueued_jobs do + @issue = described_class.new(project, guest).execute(issue) + end + end + + it 'does not close the issue' do + expect(@issue).to be_open + end + end + context "external issue tracker" do before do allow(project).to receive(:default_issues_tracker?).and_return(false) - @issue = Issues::CloseService.new(project, user, {}).execute(issue) + @issue = described_class.new(project, user, {}).execute(issue) end it { expect(@issue).to be_valid } 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/issues/reopen_service_spec.rb b/spec/services/issues/reopen_service_spec.rb new file mode 100644 index 00000000000..34a89fcd4e1 --- /dev/null +++ b/spec/services/issues/reopen_service_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe Issues::ReopenService, services: true do + let(:guest) { create(:user) } + let(:issue) { create(:issue, :closed) } + let(:project) { issue.project } + + before do + project.team << [guest, :guest] + end + + describe '#execute' do + context 'current user is not authorized to reopen issue' do + before do + perform_enqueued_jobs do + @issue = described_class.new(project, guest).execute(issue) + end + end + + it 'does not reopen the issue' do + expect(@issue).to be_closed + end + end + end +end diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 403533be5d9..24c25e4350f 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe MergeRequests::CloseService, services: true do let(:user) { create(:user) } let(:user2) { create(:user) } + let(:guest) { create(:user) } let(:merge_request) { create(:merge_request, assignee: user2) } let(:project) { merge_request.project } let!(:todo) { create(:todo, :assigned, user: user, project: project, target: merge_request, author: user2) } @@ -10,11 +11,12 @@ describe MergeRequests::CloseService, services: true do before do project.team << [user, :master] project.team << [user2, :developer] + project.team << [guest, :guest] end describe '#execute' do context 'valid params' do - let(:service) { MergeRequests::CloseService.new(project, user, {}) } + let(:service) { described_class.new(project, user, {}) } before do allow(service).to receive(:execute_hooks) @@ -47,5 +49,17 @@ describe MergeRequests::CloseService, services: true do expect(todo.reload).to be_done end end + + context 'current user is not authorized to close merge request' do + before do + perform_enqueued_jobs do + @merge_request = described_class.new(project, guest).execute(merge_request) + end + end + + it 'does not close the merge request' do + expect(@merge_request).to be_open + end + end 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/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index 3419b8bf5e6..af7424a76a9 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -3,22 +3,23 @@ require 'spec_helper' describe MergeRequests::ReopenService, services: true do let(:user) { create(:user) } let(:user2) { create(:user) } - let(:merge_request) { create(:merge_request, assignee: user2) } + let(:guest) { create(:user) } + let(:merge_request) { create(:merge_request, :closed, assignee: user2) } let(:project) { merge_request.project } before do project.team << [user, :master] project.team << [user2, :developer] + project.team << [guest, :guest] end describe '#execute' do context 'valid params' do - let(:service) { MergeRequests::ReopenService.new(project, user, {}) } + let(:service) { described_class.new(project, user, {}) } before do allow(service).to receive(:execute_hooks) - merge_request.state = :closed perform_enqueued_jobs do service.execute(merge_request) end @@ -43,5 +44,17 @@ describe MergeRequests::ReopenService, services: true do expect(note.note).to include 'Status changed to reopened' end end + + context 'current user is not authorized to reopen merge request' do + before do + perform_enqueued_jobs do + @merge_request = described_class.new(project, guest).execute(merge_request) + end + end + + it 'does not reopen the merge request' do + expect(@merge_request).to be_closed + end + end end end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 32753e84b31..93885c84dc3 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -4,22 +4,36 @@ 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 + before do + project.team << [user, :master] + end + 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]) } + end + + describe 'note with commands' do + describe '/close, /label, /assign & /milestone' do + let(:note_text) { %(HELLO\n/close\n/assign @#{user.username}\nWORLD) } + + it 'saves the note and does not alter the note text' do + expect_any_instance_of(Issues::UpdateService).to receive(:execute).and_call_original + + note = described_class.new(project, user, opts.merge(note: note_text)).execute + + expect(note.note).to eq "HELLO\nWORLD" + end + end end end @@ -42,7 +56,7 @@ describe Notes::CreateService, services: true do it "creates regular note if emoji name is invalid" do opts = { - note: ':smile: moretext: ', + note: ':smile: moretext:', noteable_type: 'Issue', noteable_id: issue.id } diff --git a/spec/services/notes/slash_commands_service_spec.rb b/spec/services/notes/slash_commands_service_spec.rb new file mode 100644 index 00000000000..4f231aab161 --- /dev/null +++ b/spec/services/notes/slash_commands_service_spec.rb @@ -0,0 +1,140 @@ +require 'spec_helper' + +describe Notes::SlashCommandsService, services: true do + shared_context 'note on noteable' do + let(:project) { create(:empty_project) } + let(:master) { create(:user).tap { |u| project.team << [u, :master] } } + let(:assignee) { create(:user) } + end + + shared_examples 'note on noteable that does not support slash commands' do + include_context 'note on noteable' + + before do + note.note = note_text + end + + describe 'note with only command' do + describe '/close, /label, /assign & /milestone' do + let(:note_text) { %(/close\n/assign @#{assignee.username}") } + + it 'saves the note and does not alter the note text' do + content, command_params = service.extract_commands(note) + + expect(content).to eq note_text + expect(command_params).to be_empty + 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) } + + it 'saves the note and does not alter the note text' do + content, command_params = service.extract_commands(note) + + expect(content).to eq note_text + expect(command_params).to be_empty + end + end + end + end + + shared_examples 'note on noteable that supports slash commands' do + include_context 'note on noteable' + + before do + note.note = note_text + end + + 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(:note_text) do + %(/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 + content, command_params = service.extract_commands(note) + service.execute(command_params, note) + + expect(content).to eq '' + expect(note.noteable).to be_closed + expect(note.noteable.labels).to match_array(labels) + expect(note.noteable.assignee).to eq(assignee) + expect(note.noteable.milestone).to eq(milestone) + end + end + + describe '/reopen' do + before do + note.noteable.close! + expect(note.noteable).to be_closed + end + let(:note_text) { '/reopen' } + + it 'opens the noteable, and leave no note' do + content, command_params = service.extract_commands(note) + service.execute(command_params, note) + + expect(content).to eq '' + expect(note.noteable).to be_open + end + end + end + + describe 'note with command & text' do + describe '/close, /label, /assign & /milestone' do + let(:note_text) do + %(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 + content, command_params = service.extract_commands(note) + service.execute(command_params, note) + + expect(content).to eq "HELLO\nWORLD" + expect(note.noteable).to be_closed + expect(note.noteable.labels).to match_array(labels) + expect(note.noteable.assignee).to eq(assignee) + expect(note.noteable.milestone).to eq(milestone) + end + end + + describe '/reopen' do + before do + note.noteable.close + expect(note.noteable).to be_closed + end + let(:note_text) { "HELLO\n/reopen\nWORLD" } + + it 'opens the noteable' do + content, command_params = service.extract_commands(note) + service.execute(command_params, note) + + expect(content).to eq "HELLO\nWORLD" + expect(note.noteable).to be_open + end + end + end + end + + describe '#execute' do + let(:service) { described_class.new(project, master) } + + it_behaves_like 'note on noteable that supports slash commands' do + let(:note) { build(:note_on_issue, project: project) } + end + + it_behaves_like 'note on noteable that supports slash commands' do + let(:note) { build(:note_on_merge_request, project: project) } + end + + it_behaves_like 'note on noteable that does not support slash commands' do + let(:note) { build(:note_on_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..a616275e883 --- /dev/null +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -0,0 +1,384 @@ +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') } + + before do + project.team << [user, :developer] + end + + describe '#execute' do + let(:service) { described_class.new(project, user) } + let(:merge_request) { create(:merge_request, source_project: project) } + + shared_examples 'reopen command' do + it 'returns state_event: "reopen" if content contains /reopen' do + issuable.close! + _, updates = service.execute(content, issuable) + + expect(updates).to eq(state_event: 'reopen') + end + end + + shared_examples 'close command' do + it 'returns state_event: "close" if content contains /close' do + _, updates = service.execute(content, issuable) + + expect(updates).to eq(state_event: 'close') + end + end + + shared_examples 'title command' do + it 'populates title: "A brand new title" if content contains /title A brand new title' do + _, updates = service.execute(content, issuable) + + expect(updates).to eq(title: 'A brand new title') + end + end + + shared_examples 'assign command' do + it 'fetches assignee and populates assignee_id if content contains /assign' do + _, updates = service.execute(content, issuable) + + expect(updates).to eq(assignee_id: user.id) + end + end + + shared_examples 'unassign command' do + it 'populates assignee_id: nil if content contains /unassign' do + issuable.update(assignee_id: user.id) + _, updates = service.execute(content, issuable) + + expect(updates).to eq(assignee_id: nil) + end + end + + shared_examples 'milestone command' do + it 'fetches milestone and populates milestone_id if content contains /milestone' do + milestone # populate the milestone + _, updates = service.execute(content, issuable) + + expect(updates).to eq(milestone_id: milestone.id) + end + end + + shared_examples 'remove_milestone command' do + it 'populates milestone_id: nil if content contains /remove_milestone' do + issuable.update(milestone_id: milestone.id) + _, updates = service.execute(content, issuable) + + expect(updates).to eq(milestone_id: nil) + end + end + + shared_examples 'label command' do + it 'fetches label ids and populates add_label_ids if content contains /label' do + bug # populate the label + inprogress # populate the label + _, updates = service.execute(content, issuable) + + expect(updates).to eq(add_label_ids: [bug.id, inprogress.id]) + end + end + + shared_examples 'unlabel command' do + it 'fetches label ids and populates remove_label_ids if content contains /unlabel' do + issuable.update(label_ids: [inprogress.id]) # populate the label + _, updates = service.execute(content, issuable) + + expect(updates).to eq(remove_label_ids: [inprogress.id]) + end + end + + shared_examples 'unlabel command with no argument' do + it 'populates label_ids: [] if content contains /unlabel with no arguments' do + issuable.update(label_ids: [inprogress.id]) # populate the label + _, updates = service.execute(content, issuable) + + expect(updates).to eq(label_ids: []) + end + end + + shared_examples 'relabel command' do + it 'populates label_ids: [] if content contains /relabel' do + issuable.update(label_ids: [bug.id]) # populate the label + inprogress # populate the label + _, updates = service.execute(content, issuable) + + expect(updates).to eq(label_ids: [inprogress.id]) + end + end + + shared_examples 'todo command' do + it 'populates todo_event: "add" if content contains /todo' do + _, updates = service.execute(content, issuable) + + expect(updates).to eq(todo_event: 'add') + end + end + + shared_examples 'done command' do + it 'populates todo_event: "done" if content contains /done' do + TodoService.new.mark_todo(issuable, user) + _, updates = service.execute(content, issuable) + + expect(updates).to eq(todo_event: 'done') + end + end + + shared_examples 'subscribe command' do + it 'populates subscription_event: "subscribe" if content contains /subscribe' do + _, updates = service.execute(content, issuable) + + expect(updates).to eq(subscription_event: 'subscribe') + end + end + + shared_examples 'unsubscribe command' do + it 'populates subscription_event: "unsubscribe" if content contains /unsubscribe' do + issuable.subscribe(user) + _, updates = service.execute(content, issuable) + + expect(updates).to eq(subscription_event: 'unsubscribe') + end + end + + shared_examples 'due command' do + it 'populates due_date: Date.new(2016, 8, 28) if content contains /due 2016-08-28' do + _, updates = service.execute(content, issuable) + + expect(updates).to eq(due_date: defined?(expected_date) ? expected_date : Date.new(2016, 8, 28)) + end + end + + shared_examples 'remove_due_date command' do + it 'populates due_date: nil if content contains /remove_due_date' do + issuable.update(due_date: Date.today) + _, updates = service.execute(content, issuable) + + expect(updates).to eq(due_date: nil) + end + end + + shared_examples 'empty command' do + it 'populates {} if content contains an unsupported command' do + _, updates = service.execute(content, issuable) + + expect(updates).to be_empty + end + end + + it_behaves_like 'reopen command' do + let(:content) { '/reopen' } + let(:issuable) { issue } + end + + it_behaves_like 'reopen command' do + let(:content) { '/reopen' } + let(:issuable) { merge_request } + end + + it_behaves_like 'close command' do + let(:content) { '/close' } + let(:issuable) { issue } + end + + it_behaves_like 'close command' do + let(:content) { '/close' } + let(:issuable) { merge_request } + end + + it_behaves_like 'title command' do + let(:content) { '/title A brand new title' } + let(:issuable) { issue } + end + + it_behaves_like 'title command' do + let(:content) { '/title A brand new title' } + let(:issuable) { merge_request } + end + + it_behaves_like 'empty command' do + let(:content) { '/title' } + let(:issuable) { issue } + end + + it_behaves_like 'assign command' do + let(:content) { "/assign @#{user.username}" } + let(:issuable) { issue } + end + + it_behaves_like 'assign command' do + let(:content) { "/assign @#{user.username}" } + let(:issuable) { merge_request } + end + + it_behaves_like 'empty command' do + let(:content) { '/assign @abcd1234' } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/assign' } + let(:issuable) { issue } + end + + it_behaves_like 'unassign command' do + let(:content) { '/unassign' } + let(:issuable) { issue } + end + + it_behaves_like 'unassign command' do + let(:content) { '/unassign' } + let(:issuable) { merge_request } + end + + it_behaves_like 'milestone command' do + let(:content) { "/milestone %#{milestone.title}" } + let(:issuable) { issue } + end + + it_behaves_like 'milestone command' do + let(:content) { "/milestone %#{milestone.title}" } + let(:issuable) { merge_request } + end + + it_behaves_like 'remove_milestone command' do + let(:content) { '/remove_milestone' } + let(:issuable) { issue } + end + + it_behaves_like 'remove_milestone command' do + let(:content) { '/remove_milestone' } + let(:issuable) { merge_request } + end + + it_behaves_like 'label command' do + let(:content) { %(/label ~"#{inprogress.title}" ~#{bug.title} ~unknown) } + let(:issuable) { issue } + end + + it_behaves_like 'label command' do + let(:content) { %(/label ~"#{inprogress.title}" ~#{bug.title} ~unknown) } + let(:issuable) { merge_request } + end + + it_behaves_like 'unlabel command' do + let(:content) { %(/unlabel ~"#{inprogress.title}") } + let(:issuable) { issue } + end + + it_behaves_like 'unlabel command' do + let(:content) { %(/unlabel ~"#{inprogress.title}") } + let(:issuable) { merge_request } + end + + it_behaves_like 'unlabel command with no argument' do + let(:content) { %(/unlabel) } + let(:issuable) { issue } + end + + it_behaves_like 'unlabel command with no argument' do + let(:content) { %(/unlabel) } + let(:issuable) { merge_request } + end + + it_behaves_like 'relabel command' do + let(:content) { %(/relabel ~"#{inprogress.title}") } + let(:issuable) { issue } + end + + it_behaves_like 'relabel command' do + let(:content) { %(/relabel ~"#{inprogress.title}") } + let(:issuable) { merge_request } + end + + it_behaves_like 'todo command' do + let(:content) { '/todo' } + let(:issuable) { issue } + end + + it_behaves_like 'todo command' do + let(:content) { '/todo' } + let(:issuable) { merge_request } + end + + it_behaves_like 'done command' do + let(:content) { '/done' } + let(:issuable) { issue } + end + + it_behaves_like 'done command' do + let(:content) { '/done' } + let(:issuable) { merge_request } + end + + it_behaves_like 'subscribe command' do + let(:content) { '/subscribe' } + let(:issuable) { issue } + end + + it_behaves_like 'subscribe command' do + let(:content) { '/subscribe' } + let(:issuable) { merge_request } + end + + it_behaves_like 'unsubscribe command' do + let(:content) { '/unsubscribe' } + let(:issuable) { issue } + end + + it_behaves_like 'unsubscribe command' do + let(:content) { '/unsubscribe' } + let(:issuable) { merge_request } + end + + it_behaves_like 'due command' do + let(:content) { '/due 2016-08-28' } + let(:issuable) { issue } + end + + it_behaves_like 'due command' do + let(:content) { '/due tomorrow' } + let(:issuable) { issue } + let(:expected_date) { Date.tomorrow } + end + + it_behaves_like 'due command' do + let(:content) { '/due 5 days from now' } + let(:issuable) { issue } + let(:expected_date) { 5.days.from_now.to_date } + end + + it_behaves_like 'due command' do + let(:content) { '/due in 2 days' } + let(:issuable) { issue } + let(:expected_date) { 2.days.from_now.to_date } + end + + it_behaves_like 'empty command' do + let(:content) { '/due foo bar' } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/due 2016-08-28' } + let(:issuable) { merge_request } + end + + it_behaves_like 'remove_due_date command' do + let(:content) { '/remove_due_date' } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/remove_due_date' } + let(:issuable) { merge_request } + end + end +end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index a2a1d5e6d30..296fd1bd5a4 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -300,6 +300,18 @@ describe TodoService, services: true do should_create_todo(user: author, target: unassigned_issue, action: Todo::MARKED) end end + + describe '#todo_exists?' do + it 'returns false when no todo exist for the given issuable' do + expect(service.todo_exist?(unassigned_issue, author)).to be_falsy + end + + it 'returns true when a todo exist for the given issuable' do + service.mark_todo(unassigned_issue, author) + + expect(service.todo_exist?(unassigned_issue, author)).to be_truthy + end + end end describe 'Merge Requests' do 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..5f9645ed44f --- /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/unlabel ~#{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/unlabel ~#{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..d2a49ea5c5e --- /dev/null +++ b/spec/support/issuable_slash_commands_shared_examples.rb @@ -0,0 +1,289 @@ +# 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(:master) { create(:user) } + let(:assignee) { create(:user, username: 'bob') } + let(:guest) { create(:user) } + 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 << [master, :master] + project.team << [assignee, :developer] + project.team << [guest, :guest] + login_with(master) + 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" + 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!" + 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 have been 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 closing the #{issuable_type}" do + before do + expect(issuable).to be_open + end + + context "when current user can close #{issuable_type}" do + it "closes the #{issuable_type}" do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: "/close" + click_button 'Comment' + end + + expect(page).not_to have_content '/close' + expect(page).to have_content 'Your commands have been executed!' + + expect(issuable.reload).to be_closed + end + end + + context "when current user cannot close #{issuable_type}" do + before do + logout + login_with(guest) + visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) + end + + it "does not close the #{issuable_type}" do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: "/close" + click_button 'Comment' + end + + expect(page).not_to have_content '/close' + expect(page).not_to have_content 'Your commands have been executed!' + + expect(issuable).to be_open + end + end + end + + context "with a note reopening the #{issuable_type}" do + before do + issuable.close + expect(issuable).to be_closed + end + + context "when current user can reopen #{issuable_type}" do + it "reopens the #{issuable_type}" do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: "/reopen" + click_button 'Comment' + end + + expect(page).not_to have_content '/reopen' + expect(page).to have_content 'Your commands have been executed!' + + expect(issuable.reload).to be_open + end + end + + context "when current user cannot reopen #{issuable_type}" do + before do + logout + login_with(guest) + visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) + end + + it "does not reopen the #{issuable_type}" do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: "/reopen" + click_button 'Comment' + end + + expect(page).not_to have_content '/reopen' + expect(page).not_to have_content 'Your commands have been executed!' + + expect(issuable).to be_closed + end + end + end + + context "with a note changing the #{issuable_type}'s title" do + context "when current user can change title of #{issuable_type}" do + it "reopens the #{issuable_type}" do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: "/title Awesome new title" + click_button 'Comment' + end + + expect(page).not_to have_content '/title' + expect(page).to have_content 'Your commands have been executed!' + + expect(issuable.reload.title).to eq 'Awesome new title' + end + end + + context "when current user cannot change title of #{issuable_type}" do + before do + logout + login_with(guest) + visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) + end + + it "does not reopen the #{issuable_type}" do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: "/title Awesome new title" + click_button 'Comment' + end + + expect(page).not_to have_content '/title' + expect(page).not_to have_content 'Your commands have been executed!' + + expect(issuable.reload.title).not_to eq 'Awesome new title' + end + 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 have been executed!' + + todos = TodosFinder.new(master).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 master + expect(todo.user).to eq master + end + end + + context "with a note marking the #{issuable_type} as done" do + before do + TodoService.new.mark_todo(issuable, master) + end + + it "creates a new todo for the #{issuable_type}" do + todos = TodosFinder.new(master).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 master + expect(todo.user).to eq master + + 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 have been 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?(master)).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 have been executed!' + + expect(issuable.subscribed?(master)).to be_truthy + end + end + + context "with a note unsubscribing to the #{issuable_type} as done" do + before do + issuable.subscribe(master) + end + + it "creates a new todo for the #{issuable_type}" do + expect(issuable.subscribed?(master)).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 have been executed!' + + expect(issuable.subscribed?(master)).to be_falsy + end + end + end +end -- cgit v1.2.3