1 files changed, 110 insertions, 60 deletions

diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 0313f424463..1638a46ed51 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -23,76 +23,123 @@ describe Issues::UpdateService, services: true do
 
   describe 'execute' do
     def find_note(starting_with)
-      @issue.notes.find do |note|
+      issue.notes.find do |note|
         note && note.note.start_with?(starting_with)
       end
     end
 
-    context "valid params" do
-      before do
-        opts = {
+    def update_issue(opts)
+      described_class.new(project, user, opts).execute(issue)
+    end
+
+    context 'valid params' do
+      let(:opts) do
+        {
           title: 'New title',
           description: 'Also please fix',
           assignee_id: user2.id,
           state_event: 'close',
           label_ids: [label.id],
-          confidential: true
+          due_date: Date.tomorrow
         }
+      end
 
-        perform_enqueued_jobs do
-          @issue = Issues::UpdateService.new(project, user, opts).execute(issue)
-        end
+      it 'updates the issue with the given params' do
+        update_issue(opts)
 
-        @issue.reload
+        expect(issue).to be_valid
+        expect(issue.title).to eq 'New title'
+        expect(issue.description).to eq 'Also please fix'
+        expect(issue.assignee).to eq user2
+        expect(issue).to be_closed
+        expect(issue.labels).to match_array [label]
+        expect(issue.due_date).to eq Date.tomorrow
       end
 
-      it { expect(@issue).to be_valid }
-      it { expect(@issue.title).to eq('New title') }
-      it { expect(@issue.assignee).to eq(user2) }
-      it { expect(@issue).to be_closed }
-      it { expect(@issue.labels.count).to eq(1) }
-      it { expect(@issue.labels.first.title).to eq(label.name) }
-
-      it 'sends email to user2 about assign of new issue and email to user3 about issue unassignment' do
-        deliveries = ActionMailer::Base.deliveries
-        email = deliveries.last
-        recipients = deliveries.last(2).map(&:to).flatten
-        expect(recipients).to include(user2.email, user3.email)
-        expect(email.subject).to include(issue.title)
-      end
+      context 'when current user cannot admin issues in the project' do
+        let(:guest) { create(:user) }
+        before do
+          project.team << [guest, :guest]
+        end
 
-      it 'creates system note about issue reassign' do
-        note = find_note('Reassigned to')
+        it 'filters out params that cannot be set without the :admin_issue permission' do
+          described_class.new(project, guest, opts).execute(issue)
 
-        expect(note).not_to be_nil
-        expect(note.note).to include "Reassigned to \@#{user2.username}"
+          expect(issue).to be_valid
+          expect(issue.title).to eq 'New title'
+          expect(issue.description).to eq 'Also please fix'
+          expect(issue.assignee).to eq user3
+          expect(issue.labels).to be_empty
+          expect(issue.milestone).to be_nil
+          expect(issue.due_date).to be_nil
+        end
       end
 
-      it 'creates system note about issue label edit' do
-        note = find_note('Added ~')
+      context 'with background jobs processed' do
+        before do
+          perform_enqueued_jobs do
+            update_issue(opts)
+          end
+        end
 
-        expect(note).not_to be_nil
-        expect(note.note).to include "Added ~#{label.id} label"
-      end
+        it 'sends email to user2 about assign of new issue and email to user3 about issue unassignment' do
+          deliveries = ActionMailer::Base.deliveries
+          email = deliveries.last
+          recipients = deliveries.last(2).map(&:to).flatten
+          expect(recipients).to include(user2.email, user3.email)
+          expect(email.subject).to include(issue.title)
+        end
 
-      it 'creates system note about title change' do
-        note = find_note('Changed title:')
+        it 'creates system note about issue reassign' do
+          note = find_note('Reassigned to')
 
-        expect(note).not_to be_nil
-        expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**'
+          expect(note).not_to be_nil
+          expect(note.note).to include "Reassigned to \@#{user2.username}"
+        end
+
+        it 'creates system note about issue label edit' do
+          note = find_note('Added ~')
+
+          expect(note).not_to be_nil
+          expect(note.note).to include "Added ~#{label.id} label"
+        end
+
+        it 'creates system note about title change' do
+          note = find_note('Changed title:')
+
+          expect(note).not_to be_nil
+          expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**'
+        end
+      end
+    end
+
+    context 'when issue turns confidential' do
+      let(:opts) do
+        {
+          title: 'New title',
+          description: 'Also please fix',
+          assignee_id: user2.id,
+          state_event: 'close',
+          label_ids: [label.id],
+          confidential: true
+        }
       end
 
       it 'creates system note about confidentiality change' do
+        update_issue(confidential: true)
+
         note = find_note('Made the issue confidential')
 
         expect(note).not_to be_nil
         expect(note.note).to eq 'Made the issue confidential'
       end
-    end
 
-    def update_issue(opts)
-      @issue = Issues::UpdateService.new(project, user, opts).execute(issue)
-      @issue.reload
+      it 'executes confidential issue hooks' do
+        expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks)
+        expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks)
+
+        update_issue(confidential: true)
+      end
     end
 
     context 'todos' do
@@ -100,7 +147,7 @@ describe Issues::UpdateService, services: true do
 
       context 'when the title change' do
         before do
-          update_issue({ title: 'New title' })
+          update_issue(title: 'New title')
         end
 
         it 'marks pending todos as done' do
@@ -110,7 +157,7 @@ describe Issues::UpdateService, services: true do
 
       context 'when the description change' do
         before do
-          update_issue({ description: 'Also please fix' })
+          update_issue(description: 'Also please fix')
         end
 
         it 'marks todos as done' do
@@ -120,7 +167,7 @@ describe Issues::UpdateService, services: true do
 
       context 'when is reassigned' do
         before do
-          update_issue({ assignee: user2 })
+          update_issue(assignee: user2)
         end
 
         it 'marks previous assignee todos as done' do
@@ -144,7 +191,7 @@ describe Issues::UpdateService, services: true do
 
       context 'when the milestone change' do
         before do
-          update_issue({ milestone: create(:milestone) })
+          update_issue(milestone: create(:milestone))
         end
 
         it 'marks todos as done' do
@@ -154,7 +201,7 @@ describe Issues::UpdateService, services: true do
 
       context 'when the labels change' do
         before do
-          update_issue({ label_ids: [label.id] })
+          update_issue(label_ids: [label.id])
         end
 
         it 'marks todos as done' do
@@ -165,6 +212,7 @@ describe Issues::UpdateService, services: true do
 
     context 'when the issue is relabeled' do
       let!(:non_subscriber) { create(:user) }
+
       let!(:subscriber) do
         create(:user).tap do |u|
           label.toggle_subscription(u)
@@ -176,7 +224,7 @@ describe Issues::UpdateService, services: true do
         opts = { label_ids: [label.id] }
 
         perform_enqueued_jobs do
-          @issue = Issues::UpdateService.new(project, user, opts).execute(issue)
+          @issue = described_class.new(project, user, opts).execute(issue)
         end
 
         should_email(subscriber)
@@ -190,7 +238,7 @@ describe Issues::UpdateService, services: true do
           opts = { label_ids: [label.id, label2.id] }
 
           perform_enqueued_jobs do
-            @issue = Issues::UpdateService.new(project, user, opts).execute(issue)
+            @issue = described_class.new(project, user, opts).execute(issue)
           end
 
           should_not_email(subscriber)
@@ -201,7 +249,7 @@ describe Issues::UpdateService, services: true do
           opts = { label_ids: [label2.id] }
 
           perform_enqueued_jobs do
-            @issue = Issues::UpdateService.new(project, user, opts).execute(issue)
+            @issue = described_class.new(project, user, opts).execute(issue)
           end
 
           should_not_email(subscriber)
@@ -210,13 +258,15 @@ describe Issues::UpdateService, services: true do
       end
     end
 
-    context 'when Issue has tasks' do
-      before { update_issue({ description: "- [ ] Task 1\n- [ ] Task 2" }) }
+    context 'when issue has tasks' do
+      before do
+        update_issue(description: "- [ ] Task 1\n- [ ] Task 2")
+      end
 
-      it { expect(@issue.tasks?).to eq(true) }
+      it { expect(issue.tasks?).to eq(true) }
 
       context 'when tasks are marked as completed' do
-        before { update_issue({ description: "- [x] Task 1\n- [X] Task 2" }) }
+        before { update_issue(description: "- [x] Task 1\n- [X] Task 2") }
 
         it 'creates system note about task status change' do
           note1 = find_note('Marked the task **Task 1** as completed')
@@ -229,8 +279,8 @@ describe Issues::UpdateService, services: true do
 
       context 'when tasks are marked as incomplete' do
         before do
-          update_issue({ description: "- [x] Task 1\n- [X] Task 2" })
-          update_issue({ description: "- [ ] Task 1\n- [ ] Task 2" })
+          update_issue(description: "- [x] Task 1\n- [X] Task 2")
+          update_issue(description: "- [ ] Task 1\n- [ ] Task 2")
         end
 
         it 'creates system note about task status change' do
@@ -244,8 +294,8 @@ describe Issues::UpdateService, services: true do
 
       context 'when tasks position has been modified' do
         before do
-          update_issue({ description: "- [x] Task 1\n- [X] Task 2" })
-          update_issue({ description: "- [x] Task 1\n- [ ] Task 3\n- [ ] Task 2" })
+          update_issue(description: "- [x] Task 1\n- [X] Task 2")
+          update_issue(description: "- [x] Task 1\n- [ ] Task 3\n- [ ] Task 2")
         end
 
         it 'does not create a system note' do
@@ -257,8 +307,8 @@ describe Issues::UpdateService, services: true do
 
       context 'when a Task list with a completed item is totally replaced' do
         before do
-          update_issue({ description: "- [ ] Task 1\n- [X] Task 2" })
-          update_issue({ description: "- [ ] One\n- [ ] Two\n- [ ] Three" })
+          update_issue(description: "- [ ] Task 1\n- [X] Task 2")
+          update_issue(description: "- [ ] One\n- [ ] Two\n- [ ] Three")
         end
 
         it 'does not create a system note referencing the position the old item' do
@@ -269,7 +319,7 @@ describe Issues::UpdateService, services: true do
 
         it 'does not generate a new note at all' do
           expect do
-            update_issue({ description: "- [ ] One\n- [ ] Two\n- [ ] Three" })
+            update_issue(description: "- [ ] One\n- [ ] Two\n- [ ] Three")
           end.not_to change { Note.count }
         end
       end
@@ -277,7 +327,7 @@ describe Issues::UpdateService, services: true do
 
     context 'updating labels' do
       let(:label3) { create(:label, project: project) }
-      let(:result) { Issues::UpdateService.new(project, user, params).execute(issue).reload }
+      let(:result) { described_class.new(project, user, params).execute(issue).reload }
 
       context 'when add_label_ids and label_ids are passed' do
         let(:params) { { label_ids: [label.id], add_label_ids: [label3.id] } }