From 65349c22129fcdf2ae0c7103094bbf50ae73db61 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Wed, 10 Aug 2016 17:51:01 +0200
Subject: [PATCH] Make slash commands contextual
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Return only slash commands that make sense for the current noteable
- Allow slash commands decription to be dynamic

Other improvements:

- Add permission checks in slash commands definition
- Use IssuesFinder and MergeRequestsFinder
- Use next if instead of a unless block, and use splat operator instead of flatten

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 Gemfile                                       |   3 +-
 Gemfile.lock                                  |   2 +
 app/controllers/projects_controller.rb        |   2 +-
 app/services/projects/autocomplete_service.rb |  23 ++-
 .../slash_commands/interpret_service.rb       | 137 ++++++++++++---
 doc/workflow/slash_commands.md                |   2 +-
 lib/gitlab/slash_commands/dsl.rb              |  40 ++++-
 spec/fixtures/emails/commands_only_reply.eml  |   2 +-
 spec/lib/gitlab/slash_commands/dsl_spec.rb    |  82 +++++++--
 .../slash_commands/interpret_service_spec.rb  | 166 ++++++++++++++++--
 10 files changed, 394 insertions(+), 65 deletions(-)

diff --git a/Gemfile b/Gemfile
index 8b44b54e22c0c..7c7889fb946d6 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 3ba6048143cd1..ecce224adeb8a 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)
@@ -822,6 +823,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/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 64d31e4a3a030..af20984cbe7f8 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -146,7 +146,7 @@ def autocomplete_sources
       mergerequests: autocomplete.merge_requests,
       labels: autocomplete.labels,
       members: participants,
-      commands: autocomplete.commands
+      commands: autocomplete.commands(note_type, note_id)
     }
 
     respond_to do |format|
diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb
index e943d2ffbcb0f..779f64f584e62 100644
--- a/app/services/projects/autocomplete_service.rb
+++ b/app/services/projects/autocomplete_service.rb
@@ -16,8 +16,27 @@ def labels
       @project.labels.select([:title, :color])
     end
 
-    def commands
-      SlashCommands::InterpretService.command_definitions
+    def commands(noteable_type, noteable_id)
+      SlashCommands::InterpretService.command_definitions(
+        project: @project,
+        noteable: command_target(noteable_type, noteable_id),
+        current_user: current_user
+      )
+    end
+
+    private
+
+    def command_target(noteable_type, noteable_id)
+      case noteable_type
+      when 'Issue'
+        IssuesFinder.new(current_user, project_id: @project.id, state: 'all').
+          execute.find_or_initialize_by(iid: noteable_id)
+      when 'MergeRequest'
+        MergeRequestsFinder.new(current_user, project_id: @project.id, state: 'all').
+          execute.find_or_initialize_by(iid: noteable_id)
+      else
+        nil
+      end
     end
   end
 end
diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb
index 55b14f118d04a..e94ee83df8534 100644
--- a/app/services/slash_commands/interpret_service.rb
+++ b/app/services/slash_commands/interpret_service.rb
@@ -10,7 +10,7 @@ def execute(content, noteable)
       @noteable = noteable
       @updates = {}
 
-      commands = extractor.extract_commands!(content)
+      commands = extractor(noteable: noteable).extract_commands!(content)
       commands.each do |command|
         __send__(*command)
       end
@@ -20,28 +20,57 @@ def execute(content, noteable)
 
     private
 
-    def extractor
-      @extractor ||= Gitlab::SlashCommands::Extractor.new(self.class.command_names)
+    def extractor(opts = {})
+      opts.merge!(current_user: current_user, project: project)
+
+      Gitlab::SlashCommands::Extractor.new(self.class.command_names(opts))
     end
 
-    desc 'Close this issue or merge request'
+    desc ->(opts) { "Close this #{opts[:noteable].to_ability_name.humanize(capitalize: false)}" }
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:noteable].open? &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"update_#{opts[:noteable].to_ability_name}", opts[:project])
+    end
     command :close do
       @updates[:state_event] = 'close'
     end
 
-    desc 'Reopen this issue or merge request'
+    desc ->(opts) { "Reopen this #{opts[:noteable].to_ability_name.humanize(capitalize: false)}" }
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:noteable].closed? &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"update_#{opts[:noteable].to_ability_name}", opts[:project])
+    end
     command :open, :reopen do
       @updates[:state_event] = 'reopen'
     end
 
     desc 'Change title'
     params '<New title>'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:noteable].persisted? &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"update_#{opts[:noteable].to_ability_name}", opts[:project])
+    end
     command :title do |title_param|
       @updates[:title] = title_param
     end
 
     desc 'Assign'
     params '@user'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project])
+    end
     command :assign, :reassign do |assignee_param|
       user = extract_references(assignee_param, :user).first
       return unless user
@@ -50,12 +79,26 @@ def extractor
     end
 
     desc 'Remove assignee'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:noteable].assignee_id? &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project])
+    end
     command :unassign, :remove_assignee do
       @updates[:assignee_id] = nil
     end
 
     desc 'Set milestone'
     params '%"milestone"'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project]) &&
+      opts[:project].milestones.active.any?
+    end
     command :milestone do |milestone_param|
       milestone = extract_references(milestone_param, :milestone).first
       return unless milestone
@@ -64,12 +107,26 @@ def extractor
     end
 
     desc 'Remove milestone'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:noteable].milestone_id? &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project])
+    end
     command :clear_milestone, :remove_milestone do
       @updates[:milestone_id] = nil
     end
 
     desc 'Add label(s)'
     params '~label1 ~"label 2"'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project]) &&
+      opts[:project].labels.any?
+    end
     command :label, :labels do |labels_param|
       label_ids = find_label_ids(labels_param)
       return if label_ids.empty?
@@ -79,6 +136,13 @@ def extractor
 
     desc 'Remove label(s)'
     params '~label1 ~"label 2"'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:noteable].labels.any? &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project])
+    end
     command :unlabel, :remove_label, :remove_labels do |labels_param|
       label_ids = find_label_ids(labels_param)
       return if label_ids.empty?
@@ -87,52 +151,85 @@ def extractor
     end
 
     desc 'Remove all labels'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:noteable].labels.any? &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project])
+    end
     command :clear_labels, :clear_label do
       @updates[:label_ids] = []
     end
 
     desc 'Add a todo'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:noteable].persisted? &&
+      opts[:current_user] &&
+      !TodosFinder.new(opts[:current_user]).execute.exists?(target: opts[:noteable])
+    end
     command :todo do
       @updates[:todo_event] = 'add'
     end
 
     desc 'Mark todo as done'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:current_user] &&
+      TodosFinder.new(opts[:current_user]).execute.exists?(target: opts[:noteable])
+    end
     command :done do
       @updates[:todo_event] = 'done'
     end
 
     desc 'Subscribe'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:current_user] &&
+      opts[:noteable].persisted? &&
+      !opts[:noteable].subscribed?(opts[:current_user])
+    end
     command :subscribe do
       @updates[:subscription_event] = 'subscribe'
     end
 
     desc 'Unsubscribe'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:current_user] &&
+      opts[:noteable].persisted? &&
+      opts[:noteable].subscribed?(opts[:current_user])
+    end
     command :unsubscribe do
       @updates[:subscription_event] = 'unsubscribe'
     end
 
-    desc 'Set a due date'
-    params '<YYYY-MM-DD> | <N days>'
+    desc 'Set due date'
+    params 'a date in natural language'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:noteable].respond_to?(:due_date) &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"update_#{opts[:noteable].to_ability_name}", opts[:project])
+    end
     command :due_date, :due do |due_date_param|
-      return unless noteable.respond_to?(:due_date)
-
-      due_date = begin
-        if due_date_param.casecmp('tomorrow').zero?
-          Date.tomorrow
-        else
-          Time.now + ChronicDuration.parse(due_date_param)
-        end
-      rescue ChronicDuration::DurationParseError
-        Date.parse(due_date_param) rescue nil
-      end
+      due_date = Chronic.parse(due_date_param).try(:to_date)
 
       @updates[:due_date] = due_date if due_date
     end
 
     desc 'Remove due date'
+    condition ->(opts) do
+      opts[:noteable] &&
+      opts[:noteable].respond_to?(:due_date) &&
+      opts[:noteable].due_date? &&
+      opts[:current_user] &&
+      opts[:project] &&
+      opts[:current_user].can?(:"update_#{opts[:noteable].to_ability_name}", opts[:project])
+    end
     command :clear_due_date do
-      return unless noteable.respond_to?(:due_date)
-
       @updates[:due_date] = nil
     end
 
diff --git a/doc/workflow/slash_commands.md b/doc/workflow/slash_commands.md
index 46f291561d790..c4edbeddd40fe 100644
--- a/doc/workflow/slash_commands.md
+++ b/doc/workflow/slash_commands.md
@@ -25,5 +25,5 @@ do.
 | `/done`                    | None                | Mark todo as done |
 | `/subscribe`               | None                | Subscribe |
 | `/unsubscribe`             | None                | Unsubscribe |
-| `/due_date <YYYY-MM-DD> | <N days>` | `/due`     | Set a due date |
+| `/due_date a date in natural language` | `/due`  | Set due date |
 | `/clear_due_date`          | None                | Remove due date |
diff --git a/lib/gitlab/slash_commands/dsl.rb b/lib/gitlab/slash_commands/dsl.rb
index edfe840587673..20e1d071d06f0 100644
--- a/lib/gitlab/slash_commands/dsl.rb
+++ b/lib/gitlab/slash_commands/dsl.rb
@@ -8,15 +8,25 @@ module Dsl
       end
 
       module ClassMethods
-        def command_definitions
-          @command_definitions
-        end
+        def command_definitions(opts = {})
+          @command_definitions.map do |cmd_def|
+            next if cmd_def[:cond_lambda] && !cmd_def[:cond_lambda].call(opts)
+
+            cmd_def = cmd_def.dup
 
-        def command_names
-          command_definitions.flat_map do |command_definition|
-            unless command_definition[:noop]
-              [command_definition[:name], command_definition[:aliases]].flatten
+            if cmd_def[:description].present? && cmd_def[:description].respond_to?(:call)
+              cmd_def[:description] = cmd_def[:description].call(opts) rescue ''
             end
+
+            cmd_def
+          end.compact
+        end
+
+        def command_names(opts = {})
+          command_definitions(opts).flat_map do |command_definition|
+            next if command_definition[:noop]
+
+            [command_definition[:name], *command_definition[:aliases]]
           end.compact
         end
 
@@ -35,6 +45,11 @@ def noop(noop)
           @noop = noop
         end
 
+        # Allows to define if a lambda to conditionally return an action
+        def condition(cond_lambda)
+          @cond_lambda = cond_lambda
+        end
+
         # Registers a new command which is recognizeable
         # from body of email or comment.
         # Example:
@@ -53,6 +68,10 @@ def command(*command_names, &block)
           define_method(proxy_method_name, &block)
 
           define_method(command_name) do |*args|
+            unless @cond_lambda.nil? || @cond_lambda.call(project: project, current_user: current_user, noteable: noteable)
+              return
+            end
+
             proxy_method = method(proxy_method_name)
 
             if proxy_method.arity == -1 || proxy_method.arity == args.size
@@ -70,13 +89,16 @@ def command(*command_names, &block)
             name: command_name,
             aliases: aliases,
             description: @description || '',
-            params: @params || [],
-            noop: @noop || false
+            params: @params || []
           }
+          command_definition[:noop] = @noop unless @noop.nil?
+          command_definition[:cond_lambda] = @cond_lambda unless @cond_lambda.nil?
           @command_definitions << command_definition
 
           @description = nil
           @params = nil
+          @noop = nil
+          @cond_lambda = nil
         end
       end
     end
diff --git a/spec/fixtures/emails/commands_only_reply.eml b/spec/fixtures/emails/commands_only_reply.eml
index b64d851a79c7a..ccd92e406c497 100644
--- a/spec/fixtures/emails/commands_only_reply.eml
+++ b/spec/fixtures/emails/commands_only_reply.eml
@@ -20,7 +20,7 @@ X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu,
 X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1
 
 /close
-/unsubscribe
+/todo
 
 
 On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta
diff --git a/spec/lib/gitlab/slash_commands/dsl_spec.rb b/spec/lib/gitlab/slash_commands/dsl_spec.rb
index 39e1996c8916b..893a7692f111a 100644
--- a/spec/lib/gitlab/slash_commands/dsl_spec.rb
+++ b/spec/lib/gitlab/slash_commands/dsl_spec.rb
@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::SlashCommands::Dsl do
+  COND_LAMBDA = ->(opts) { opts[:project] == 'foo' }
   before :all do
     DummyClass = Class.new do
       include Gitlab::SlashCommands::Dsl
@@ -20,18 +21,23 @@
         arg1
       end
 
-      desc 'A command with two args'
+      desc ->(opts) { "A dynamic description for #{opts.fetch(:noteable)}" }
       params 'The first argument', 'The second argument'
       command :two_args do |arg1, arg2|
         [arg1, arg2]
       end
 
-      command :wildcard do |*args|
+      noop true
+      command :cc do |*args|
         args
       end
 
-      noop true
-      command :cc do |*args|
+      condition COND_LAMBDA
+      command :cond_action do |*args|
+        args
+      end
+
+      command :wildcard do |*args|
         args
       end
     end
@@ -39,27 +45,73 @@
   let(:dummy) { DummyClass.new }
 
   describe '.command_definitions' do
-    it 'returns an array with commands definitions' do
-      expected = [
-        { name: :no_args, aliases: [:none], description: 'A command with no args', params: [], noop: false },
-        { name: :returning, aliases: [], description: 'A command returning a value', params: [], noop: false },
-        { name: :one_arg, aliases: [:once, :first], description: '', params: ['The first argument'], noop: false },
-        { name: :two_args, aliases: [], description: 'A command with two args', params: ['The first argument', 'The second argument'], noop: false },
-        { name: :wildcard, aliases: [], description: '', params: [], noop: false },
-        { name: :cc, aliases: [], description: '', params: [], noop: true }
+    let(:base_expected) do
+      [
+        { name: :no_args, aliases: [:none], description: 'A command with no args', params: [] },
+        { name: :returning, aliases: [], description: 'A command returning a value', params: [] },
+        { name: :one_arg, aliases: [:once, :first], description: '', params: ['The first argument'] },
+        { name: :two_args, aliases: [], description: '', params: ['The first argument', 'The second argument'] },
+        { name: :cc, aliases: [], description: '', params: [], noop: true },
+        { name: :wildcard, aliases: [], description: '', params: [] }
       ]
+    end
 
-      expect(DummyClass.command_definitions).to eq expected
+    it 'returns an array with commands definitions' do
+      expect(DummyClass.command_definitions).to match_array base_expected
+    end
+
+    context 'with options passed' do
+      context 'when condition is met' do
+        let(:expected) { base_expected << { name: :cond_action, aliases: [], description: '', params: [], cond_lambda: COND_LAMBDA } }
+
+        it 'returns an array with commands definitions' do
+          expect(DummyClass.command_definitions(project: 'foo')).to match_array expected
+        end
+      end
+
+      context 'when condition is not met' do
+        it 'returns an array with commands definitions without actions that did not met conditions' do
+          expect(DummyClass.command_definitions(project: 'bar')).to match_array base_expected
+        end
+      end
+
+      context 'when description can be generated dynamically' do
+        it 'returns an array with commands definitions with dynamic descriptions' do
+          base_expected[3][:description] = 'A dynamic description for merge request'
+
+          expect(DummyClass.command_definitions(noteable: 'merge request')).to match_array base_expected
+        end
+      end
     end
   end
 
   describe '.command_names' do
-    it 'returns an array with commands definitions' do
-      expect(DummyClass.command_names).to eq [
+    let(:base_expected) do
+      [
         :no_args, :none, :returning, :one_arg,
         :once, :first, :two_args, :wildcard
       ]
     end
+
+    it 'returns an array with commands definitions' do
+      expect(DummyClass.command_names).to eq base_expected
+    end
+
+    context 'with options passed' do
+      context 'when condition is met' do
+        let(:expected) { base_expected << :cond_action }
+
+        it 'returns an array with commands definitions' do
+          expect(DummyClass.command_names(project: 'foo')).to match_array expected
+        end
+      end
+
+      context 'when condition is not met' do
+        it 'returns an array with commands definitions without action that did not met conditions' do
+          expect(DummyClass.command_names(project: 'bar')).to match_array base_expected
+        end
+      end
+    end
   end
 
   describe 'command with no args' do
diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb
index 620687e321240..0cf77e53435db 100644
--- a/spec/services/slash_commands/interpret_service_spec.rb
+++ b/spec/services/slash_commands/interpret_service_spec.rb
@@ -8,38 +8,152 @@
   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 '#command_names' do
-    subject { described_class.command_names }
+    subject do
+      described_class.command_names(
+        project: project,
+        noteable: issue,
+        current_user: user
+      )
+    end
 
-    it 'returns the known commands' do
+    it 'returns the basic known commands' do
       is_expected.to match_array([
-        :open, :reopen,
         :close,
         :title,
         :assign, :reassign,
-        :unassign, :remove_assignee,
-        :milestone,
-        :clear_milestone, :remove_milestone,
-        :labels, :label,
-        :unlabel, :remove_labels, :remove_label,
-        :clear_labels, :clear_label,
         :todo,
-        :done,
         :subscribe,
-        :unsubscribe,
-        :due_date, :due,
-        :clear_due_date
+        :due_date, :due
       ])
     end
+
+    context 'when noteable is open' do
+      it 'includes the :close command' do
+        is_expected.to include(*[:close])
+      end
+    end
+
+    context 'when noteable is closed' do
+      before do
+        issue.close!
+      end
+
+      it 'includes the :open, :reopen commands' do
+        is_expected.to include(*[:open, :reopen])
+      end
+    end
+
+    context 'when noteable has an assignee' do
+      before do
+        issue.update(assignee_id: user.id)
+      end
+
+      it 'includes the :unassign, :remove_assignee commands' do
+        is_expected.to include(*[:unassign, :remove_assignee])
+      end
+    end
+
+    context 'when noteable has a milestone' do
+      before do
+        issue.update(milestone: milestone)
+      end
+
+      it 'includes the :clear_milestone, :remove_milestone commands' do
+        is_expected.to include(*[:milestone, :clear_milestone, :remove_milestone])
+      end
+    end
+
+    context 'when project has a milestone' do
+      before do
+        milestone
+      end
+
+      it 'includes the :milestone command' do
+        is_expected.to include(*[:milestone])
+      end
+    end
+
+    context 'when noteable has a label' do
+      before do
+        issue.update(label_ids: [bug.id])
+      end
+
+      it 'includes the :unlabel, :remove_labels, :remove_label, :clear_labels, :clear_label commands' do
+        is_expected.to include(*[:unlabel, :remove_labels, :remove_label, :clear_labels, :clear_label])
+      end
+    end
+
+    context 'when project has a label' do
+      before do
+        inprogress
+      end
+
+      it 'includes the :labels, :label commands' do
+        is_expected.to include(*[:labels, :label])
+      end
+    end
+
+    context 'when user has no todo' do
+      it 'includes the :todo command' do
+        is_expected.to include(*[:todo])
+      end
+    end
+
+    context 'when user has a todo' do
+      before do
+        TodoService.new.mark_todo(issue, user)
+      end
+
+      it 'includes the :done command' do
+        is_expected.to include(*[:done])
+      end
+    end
+
+    context 'when user is not subscribed' do
+      it 'includes the :subscribe command' do
+        is_expected.to include(*[:subscribe])
+      end
+    end
+
+    context 'when user is subscribed' do
+      before do
+        issue.subscribe(user)
+      end
+
+      it 'includes the :unsubscribe command' do
+        is_expected.to include(*[:unsubscribe])
+      end
+    end
+
+    context 'when noteable has a no due date' do
+      it 'includes the :due_date, :due commands' do
+        is_expected.to include(*[:due_date, :due])
+      end
+    end
+
+    context 'when noteable has a due date' do
+      before do
+        issue.update(due_date: Date.today)
+      end
+
+      it 'includes the :clear_due_date command' do
+        is_expected.to include(*[:due_date, :due, :clear_due_date])
+      end
+    end
   end
 
   describe '#execute' do
     let(:service) { described_class.new(project, user) }
-    let(:issue) { create(:issue) }
-    let(:merge_request) { create(:merge_request) }
+    let(:merge_request) { create(:merge_request, source_project: project) }
 
     shared_examples 'open command' do
       it 'returns state_event: "open" if content contains /open' do
+        issuable.close!
         changes = service.execute(content, issuable)
 
         expect(changes).to eq(state_event: 'reopen')
@@ -72,6 +186,7 @@
 
     shared_examples 'unassign command' do
       it 'populates assignee_id: nil if content contains /unassign' do
+        issuable.update(assignee_id: user.id)
         changes = service.execute(content, issuable)
 
         expect(changes).to eq(assignee_id: nil)
@@ -80,6 +195,7 @@
 
     shared_examples 'milestone command' do
       it 'fetches milestone and populates milestone_id if content contains /milestone' do
+        milestone # populate the milestone
         changes = service.execute(content, issuable)
 
         expect(changes).to eq(milestone_id: milestone.id)
@@ -88,6 +204,7 @@
 
     shared_examples 'clear_milestone command' do
       it 'populates milestone_id: nil if content contains /clear_milestone' do
+        issuable.update(milestone_id: milestone.id)
         changes = service.execute(content, issuable)
 
         expect(changes).to eq(milestone_id: nil)
@@ -96,6 +213,8 @@
 
     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
         changes = service.execute(content, issuable)
 
         expect(changes).to eq(add_label_ids: [bug.id, inprogress.id])
@@ -104,6 +223,7 @@
 
     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
         changes = service.execute(content, issuable)
 
         expect(changes).to eq(remove_label_ids: [inprogress.id])
@@ -112,6 +232,7 @@
 
     shared_examples 'clear_labels command' do
       it 'populates label_ids: [] if content contains /clear_labels' do
+        issuable.update(label_ids: [inprogress.id]) # populate the label
         changes = service.execute(content, issuable)
 
         expect(changes).to eq(label_ids: [])
@@ -128,6 +249,7 @@
 
     shared_examples 'done command' do
       it 'populates todo_event: "done" if content contains /done' do
+        TodoService.new.mark_todo(issuable, user)
         changes = service.execute(content, issuable)
 
         expect(changes).to eq(todo_event: 'done')
@@ -144,6 +266,7 @@
 
     shared_examples 'unsubscribe command' do
       it 'populates subscription_event: "unsubscribe" if content contains /unsubscribe' do
+        issuable.subscribe(user)
         changes = service.execute(content, issuable)
 
         expect(changes).to eq(subscription_event: 'unsubscribe')
@@ -160,6 +283,7 @@
 
     shared_examples 'clear_due_date command' do
       it 'populates due_date: nil if content contains /clear_due_date' do
+        issuable.update(due_date: Date.today)
         changes = service.execute(content, issuable)
 
         expect(changes).to eq(due_date: nil)
@@ -375,6 +499,18 @@
       let(:expected_date) { Date.tomorrow }
     end
 
+    it_behaves_like 'due_date 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_date 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_date foo bar' }
       let(:issuable) { issue }
-- 
GitLab