Support slash commands in issues / MR description & comments
What does this MR do?
It brings support for slash commands (I think "slash commands" is a good name for that, inspired by https://api.slack.com/slash-commands).
The UI does not update accordingly when leaving a comment with commands in it. This will require further frontend work but I think we can already ship this first iteration.
For the moment actions are interpreted in a hash of changes.
- For new issuable, this hash of changes is then merged to the
params
used to create issuable in their respective creation service. - For new notes, the hash is passed to the
noteable
update service so that it updates thenoteable
first, and then tries to save the note. The good thing about this workflow is that since commands are extracted from the original note, no note is created in the case the note consisted of commands only.
For instance if I leave the following note:
/milestone %9.10
/label ~"Pick into Stable"
These commands will actually trigger an update of the noteable, and no actual note will be created (of course the issuable update will create new system notes).
I think this is the right approach as a first step, then we can build on this, by returning "special value" in the hash to trigger advanced actions.
For instance, we could add /todo
& /done
actions that would translate in { todo: :mark }
or { todo: :done }
that could easily trigger a todo_service.mark_todo(note.noteable, current_user)
/ todo_service.mark_todos_as_done(note.noteable, current_user)
.
There's no need to make our models bigger with a new concern (as this was originally implemented) since:
- all the creation/update logic is handled in services
- advanced workflow like this one shouldn't be part of the model
Are there points in the code the reviewer needs to double check?
One important thing I wanted was to be sure that we don't bypass the services to create/update issuable so I don't think this MR can create security issues.
Why was this MR needed?
Because this is awesome and will allow many people to automate more. For instance, as a release manager, when I'm preparing a patch release, the workflow is the following:
- pick a commit into the stable branch
- leave a comment to say that it's been picked
- remove the Pick into Stable label
Now, I will be able to have only two steps:
- pick a commit into the stable branch
- leave a comment to say that it's been picked with the
/remove_label ~"Pick into Stable" label
command, and voila!
Also, since reply by email is now enabled on .com, you will be able to actually update an issue or MR from your email client!
What are the relevant issue numbers?
Closes #4273 (closed).
Screenshots
Slash commands when creating an issue
Slash commands in a note
Does this MR meet the acceptance criteria?
Todo:
-
Initial slash commands -
/todo
: Mark issuable as todo -
/done
: Mark todo as done for the issuable -
/subscribe
: Subscribe to the issuable without leaving a message -
/unsubscribe
: Unsubscribe from the issuable (useful for release managers that leave many messages but don't want to be subscribed!) -
/due_date
: Set a due date for the issue (not for the MR since it's not supported) -
/clear_due_date
: Set a due date for the issue (not for the MR since it's not supported) -
We should also bring autocompletion for slash commands themselves once you type /
! -
Improve autocompletion by adding a description of the command -
Check that the feature works well with the reply-by-email feature -
The right thing happens when the reply contained nothing but commands (no comment is created, no error email is sent out)
To be done in EE:
-
/weight
: Set a weight for the issuable => gitlab-org/gitlab-ee#852
To be done in a new MR (mote complex):
-
/merge
: Merge the MR => gitlab-org/gitlab-ce#20741
Feature improvement:
-
Ensure that slash commands are not detected inside code blocks (see !3954 (merged)) -
Support /title <New title>
-
Improve CHANGELOG -
Add a /cc
command which would be a no-op but would appear in the auto-complete to not confuse people -
Don't return actions that would have no effect (no /remove_milestone
if the noteable has no milestone, or no/milestone
if the project has no milestone, etc.) -
Use a contextual description for commands: "Close this issue" or "Close this merge request" based on context?
Backend code improvement:
-
Improve the attributes.delete(:remove_label_ids)
mess inIssuableBaseService
-
Improve the wording of the “You entered only commands” message -
Extract #execute_slash_commands!
to a newNotes::SlashCommandsService
class -
Extract the body of the command_params.reduce
block to its own method -
Use :subscription_event
instead of manually trigger subscription! -
Create a new :todo_event
param that would work similarly to:state_event
/:subscription_event
? -
Let the commands have their own block to find out if the noteable supports that command -
Don’t noteable_updated_at != noteable.updated_at
, returntrue
if there’s any command -
Use TodoFinder
instead ofcurrent_user.todos.pending.find_by(target: note.noteable)
-
Create a new Notes::SlashCommandsService.UPDATE_SERVICES
, as a hash from class name to service. -
Get rid of a useless begin/end
-
Improve the command descriptions -
Document that each command should be on a separate line -
Let the user know that we support ChronicDuration style? -
Make Extractor
aclass
instead of aStruct
-
Ensure we don’t detect commands with no space between the command and the first argument -
/remove_milestone
->/clear_milestone
and set/remove_milestone
as an alias -
Ensure unauthorized user cannot perform unauthorized actions. Could be done in IssuableBaseService#filter_params
. -
If the user doesn't have permissions to perform an action, we should not even show that action in autocomplete
Frontend code:
-
Remove useless auto complete template -
Ensure we add the reference prefix only for commands that need one -
Insert a new line before inserting a slash command -
Use !==
instead of!=
in JS (don't use it for most of presence checks, i.e. use!= null
and not!== null
) -
Do not useNo need for<b>
,font-weight
is more appropriate<b>
after all -
Use _.template
-
UseWe don't even needbind
instead of(function(_this){})(this)
bind
-
CHANGELOG entry added -
Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the style guides -
Branch has no merge conflicts with master
(if you do - rebase it please)
Merge request reports
Activity
mentioned in merge request !3242 (closed)
Added 124 commits:
- 7feeef23...f60b48bd - 122 commits from branch
master
- 25232e25 - Support slash commands in noteable description and notes
- ba6c5243 - Add documentation for slash commands
- 7feeef23...f60b48bd - 122 commits from branch
Reassigned to @DouweM
@rymai About those "Here are a few ideas of possible future actions:" and "We should also bring autocompletion for slash commands themselves once you type /!", we should definitely do the autocomplete and some of those actions for 8.10 if we want to make users excited about this feature :) If it's just in documentation, it practically doesn't exist.
Reassigned to @rymai