Make slash commands contextual

- 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>

1 files changed, 151 insertions, 15 deletions

diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb
index 620687e3212..0cf77e53435 100644
--- a/spec/services/slash_commands/interpret_service_spec.rb
+++ b/spec/services/slash_commands/interpret_service_spec.rb
@@ -8,38 +8,152 @@ describe SlashCommands::InterpretService, services: true do
   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 @@ describe SlashCommands::InterpretService, services: true do
 
     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 @@ describe SlashCommands::InterpretService, services: true do
 
     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 @@ describe SlashCommands::InterpretService, services: true do
 
     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 @@ describe SlashCommands::InterpretService, services: true do
 
     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 @@ describe SlashCommands::InterpretService, services: true do
 
     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 @@ describe SlashCommands::InterpretService, services: true do
 
     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 @@ describe SlashCommands::InterpretService, services: true do
 
     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 @@ describe SlashCommands::InterpretService, services: true do
 
     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 @@ describe SlashCommands::InterpretService, services: true do
 
     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 @@ describe SlashCommands::InterpretService, services: true do
       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 }