Fixed specs and fixes based on failing specs

3 files changed, 57 insertions, 181 deletions

diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index 92dbccf0729..93885c84dc3 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -56,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
index 5632ec09834..9a262fcf32f 100644
--- a/spec/services/notes/slash_commands_service_spec.rb
+++ b/spec/services/notes/slash_commands_service_spec.rb
@@ -12,7 +12,6 @@ describe Notes::SlashCommandsService, services: true do
 
     before do
       note.note = note_text
-      described_class.new(project, master).execute(note)
     end
 
     describe 'note with only command' do
@@ -20,7 +19,10 @@ describe Notes::SlashCommandsService, services: true do
         let(:note_text) { %(/close\n/assign @#{assignee.username}") }
 
         it 'saves the note and does not alter the note text' do
-          expect(note.note).to eq note_text
+          content, command_params = service.extract_commands(note)
+
+          expect(content).to eq note_text
+          expect(command_params).to be_empty
         end
       end
     end
@@ -30,7 +32,10 @@ describe Notes::SlashCommandsService, services: true do
         let(:note_text) { %(HELLO\n/close\n/assign @#{assignee.username}\nWORLD) }
 
         it 'saves the note and does not alter the note text' do
-          expect(note.note).to eq note_text
+          content, command_params = service.extract_commands(note)
+
+          expect(content).to eq note_text
+          expect(command_params).to be_empty
         end
       end
     end
@@ -53,9 +58,10 @@ describe Notes::SlashCommandsService, services: true do
         end
 
         it 'closes noteable, sets labels, assigns, and sets milestone to noteable, and leave no note' do
-          described_class.new(project, master).execute(note)
+          content, command_params = service.extract_commands(note)
+          service.execute(command_params, note)
 
-          expect(note.note).to eq ''
+          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)
@@ -71,9 +77,10 @@ describe Notes::SlashCommandsService, services: true do
         let(:note_text) { '/open' }
 
         it 'opens the noteable, and leave no note' do
-          described_class.new(project, master).execute(note)
+          content, command_params = service.extract_commands(note)
+          service.execute(command_params, note)
 
-          expect(note.note).to eq ''
+          expect(content).to eq ''
           expect(note.noteable).to be_open
         end
       end
@@ -86,9 +93,10 @@ describe Notes::SlashCommandsService, services: true do
         end
 
         it 'closes noteable, sets labels, assigns, and sets milestone to noteable' do
-          described_class.new(project, master).execute(note)
+          content, command_params = service.extract_commands(note)
+          service.execute(command_params, note)
 
-          expect(note.note).to eq "HELLO\nWORLD"
+          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)
@@ -104,9 +112,10 @@ describe Notes::SlashCommandsService, services: true do
         let(:note_text) { "HELLO\n/open\nWORLD" }
 
         it 'opens the noteable' do
-          described_class.new(project, master).execute(note)
+          content, command_params = service.extract_commands(note)
+          service.execute(command_params, note)
 
-          expect(note.note).to eq "HELLO\nWORLD"
+          expect(content).to eq "HELLO\nWORLD"
           expect(note.noteable).to be_open
         end
       end
@@ -114,6 +123,8 @@ describe Notes::SlashCommandsService, services: true do
   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
diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb
index 0cf77e53435..c20aa90ddde 100644
--- a/spec/services/slash_commands/interpret_service_spec.rb
+++ b/spec/services/slash_commands/interpret_service_spec.rb
@@ -12,141 +12,6 @@ describe SlashCommands::InterpretService, services: true do
     project.team << [user, :developer]
   end
 
-  describe '#command_names' do
-    subject do
-      described_class.command_names(
-        project: project,
-        noteable: issue,
-        current_user: user
-      )
-    end
-
-    it 'returns the basic known commands' do
-      is_expected.to match_array([
-        :close,
-        :title,
-        :assign, :reassign,
-        :todo,
-        :subscribe,
-        :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(:merge_request) { create(:merge_request, source_project: project) }
@@ -154,60 +19,60 @@ describe SlashCommands::InterpretService, services: true do
     shared_examples 'open command' do
       it 'returns state_event: "open" if content contains /open' do
         issuable.close!
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(state_event: 'reopen')
+        expect(updates).to eq(state_event: 'reopen')
       end
     end
 
     shared_examples 'close command' do
       it 'returns state_event: "close" if content contains /open' do
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(state_event: 'close')
+        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
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(title: 'A brand new title')
+        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
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(assignee_id: user.id)
+        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)
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(assignee_id: nil)
+        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
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(milestone_id: milestone.id)
+        expect(updates).to eq(milestone_id: milestone.id)
       end
     end
 
     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)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(milestone_id: nil)
+        expect(updates).to eq(milestone_id: nil)
       end
     end
 
@@ -215,86 +80,86 @@ describe SlashCommands::InterpretService, services: true 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)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(add_label_ids: [bug.id, inprogress.id])
+        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
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(remove_label_ids: [inprogress.id])
+        expect(updates).to eq(remove_label_ids: [inprogress.id])
       end
     end
 
     shared_examples 'clear_labels command' do
       it 'populates label_ids: [] if content contains /clear_labels' do
         issuable.update(label_ids: [inprogress.id]) # populate the label
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(label_ids: [])
+        expect(updates).to eq(label_ids: [])
       end
     end
 
     shared_examples 'todo command' do
       it 'populates todo_event: "add" if content contains /todo' do
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(todo_event: 'add')
+        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)
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(todo_event: 'done')
+        expect(updates).to eq(todo_event: 'done')
       end
     end
 
     shared_examples 'subscribe command' do
       it 'populates subscription_event: "subscribe" if content contains /subscribe' do
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(subscription_event: 'subscribe')
+        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)
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(subscription_event: 'unsubscribe')
+        expect(updates).to eq(subscription_event: 'unsubscribe')
       end
     end
 
     shared_examples 'due_date command' do
       it 'populates due_date: Date.new(2016, 8, 28) if content contains /due_date 2016-08-28' do
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(due_date: defined?(expected_date) ? expected_date : Date.new(2016, 8, 28))
+        expect(updates).to eq(due_date: defined?(expected_date) ? expected_date : Date.new(2016, 8, 28))
       end
     end
 
     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)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to eq(due_date: nil)
+        expect(updates).to eq(due_date: nil)
       end
     end
 
     shared_examples 'empty command' do
       it 'populates {} if content contains an unsupported command' do
-        changes = service.execute(content, issuable)
+        _, updates = service.execute(content, issuable)
 
-        expect(changes).to be_empty
+        expect(updates).to be_empty
       end
     end