1 files changed, 91 insertions, 26 deletions

diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb
index 3cf45143594..8950bdd465f 100644
--- a/spec/services/issues/close_service_spec.rb
+++ b/spec/services/issues/close_service_spec.rb
@@ -3,24 +3,52 @@
 require 'spec_helper'
 
 RSpec.describe Issues::CloseService do
-  let(:project) { create(:project, :repository) }
-  let(:user) { create(:user, email: "user@example.com") }
-  let(:user2) { create(:user, email: "user2@example.com") }
-  let(:guest) { create(:user) }
-  let(:issue) { create(:issue, title: "My issue", project: project, assignees: [user2], author: create(:user)) }
+  subject(:close_issue) { described_class.new(project: project, current_user: user).close_issue(issue) }
+
+  let_it_be(:project, refind: true) { create(:project, :repository) }
+  let_it_be(:label1) { create(:label, project: project) }
+  let_it_be(:label2) { create(:label, project: project, remove_on_close: true) }
+  let_it_be(:author) { create(:user) }
+  let_it_be(:user) { create(:user, email: "user@example.com") }
+  let_it_be(:user2) { create(:user, email: "user2@example.com") }
+  let_it_be(:guest) { create(:user) }
+  let_it_be(:closing_merge_request) { create(:merge_request, source_project: project) }
+
   let(:external_issue) { ExternalIssue.new('JIRA-123', project) }
-  let(:closing_merge_request) { create(:merge_request, source_project: project) }
-  let(:closing_commit) { create(:commit, project: project) }
-  let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) }
+  let!(:issue) { create(:issue, title: "My issue", project: project, assignees: [user2], author: author) }
 
-  before do
+  before_all do
     project.add_maintainer(user)
     project.add_developer(user2)
     project.add_guest(guest)
   end
 
+  shared_examples 'removes labels marked for removal from issue when closed' do
+    before do
+      issue.update!(label_ids: [label1.id, label2.id])
+    end
+
+    it 'removes labels marked for removal' do
+      expect do
+        close_issue
+      end.to change { issue.reload.label_ids }.from(containing_exactly(label1.id, label2.id)).to(containing_exactly(label1.id))
+    end
+
+    it 'creates system notes for the removed labels' do
+      expect do
+        close_issue
+      end.to change(ResourceLabelEvent, :count).by(1)
+
+      expect(ResourceLabelEvent.last.slice(:action, :issue_id, :label_id)).to eq(
+        'action' => 'remove',
+        'issue_id' => issue.id,
+        'label_id' => label2.id
+      )
+    end
+  end
+
   describe '#execute' do
-    let(:service) { described_class.new(project, user) }
+    let(:service) { described_class.new(project: project, current_user: user) }
 
     it 'checks if the user is authorized to update the issue' do
       expect(service).to receive(:can?).with(user, :update_issue, issue)
@@ -87,18 +115,18 @@ RSpec.describe Issues::CloseService do
           project.reload
           expect(project.external_issue_tracker).to receive(:close_issue)
 
-          described_class.new(project, user).close_issue(external_issue)
+          described_class.new(project: project, current_user: user).close_issue(external_issue)
         end
       end
 
-      context 'with innactive external issue tracker supporting close_issue' do
+      context 'with inactive external issue tracker supporting close_issue' do
         let!(:external_issue_tracker) { create(:jira_service, project: project, active: false) }
 
         it 'does not close the issue on the external issue tracker' do
           project.reload
           expect(project.external_issue_tracker).not_to receive(:close_issue)
 
-          described_class.new(project, user).close_issue(external_issue)
+          described_class.new(project: project, current_user: user).close_issue(external_issue)
         end
       end
 
@@ -109,7 +137,7 @@ RSpec.describe Issues::CloseService do
           project.reload
           expect(project.external_issue_tracker).not_to receive(:close_issue)
 
-          described_class.new(project, user).close_issue(external_issue)
+          described_class.new(project: project, current_user: user).close_issue(external_issue)
         end
       end
     end
@@ -117,10 +145,12 @@ RSpec.describe Issues::CloseService do
     context "closed by a merge request", :sidekiq_might_not_need_inline do
       subject(:close_issue) do
         perform_enqueued_jobs do
-          described_class.new(project, user).close_issue(issue, closed_via: closing_merge_request)
+          described_class.new(project: project, current_user: user).close_issue(issue, closed_via: closing_merge_request)
         end
       end
 
+      it_behaves_like 'removes labels marked for removal from issue when closed'
+
       it 'mentions closure via a merge request' do
         close_issue
 
@@ -184,10 +214,18 @@ RSpec.describe Issues::CloseService do
     end
 
     context "closed by a commit", :sidekiq_might_not_need_inline do
-      it 'mentions closure via a commit' do
+      subject(:close_issue) do
         perform_enqueued_jobs do
-          described_class.new(project, user).close_issue(issue, closed_via: closing_commit)
+          described_class.new(project: project, current_user: user).close_issue(issue, closed_via: closing_commit)
         end
+      end
+
+      let(:closing_commit) { create(:commit, project: project) }
+
+      it_behaves_like 'removes labels marked for removal from issue when closed'
+
+      it 'mentions closure via a commit' do
+        close_issue
 
         email = ActionMailer::Base.deliveries.last
 
@@ -199,9 +237,8 @@ RSpec.describe Issues::CloseService do
       context 'when user cannot read the commit' do
         it 'does not mention the commit id' do
           project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED)
-          perform_enqueued_jobs do
-            described_class.new(project, user).close_issue(issue, closed_via: closing_commit)
-          end
+
+          close_issue
 
           email = ActionMailer::Base.deliveries.last
           body_text = email.body.parts.map(&:body).join(" ")
@@ -216,10 +253,20 @@ RSpec.describe Issues::CloseService do
     context "valid params" do
       subject(:close_issue) do
         perform_enqueued_jobs do
-          described_class.new(project, user).close_issue(issue)
+          described_class.new(project: project, current_user: user).close_issue(issue)
         end
       end
 
+      it 'verifies the number of queries' do
+        recorded = ActiveRecord::QueryRecorder.new { close_issue }
+        expected_queries = 32
+
+        expect(recorded.count).to be <= expected_queries
+        expect(recorded.cached_count).to eq(0)
+      end
+
+      it_behaves_like 'removes labels marked for removal from issue when closed'
+
       it 'closes the issue' do
         close_issue
 
@@ -230,7 +277,7 @@ RSpec.describe Issues::CloseService do
       it 'records closed user' do
         close_issue
 
-        expect(issue.closed_by_id).to be(user.id)
+        expect(issue.reload.closed_by_id).to be(user.id)
       end
 
       it 'sends email to user2 about assign of new issue', :sidekiq_might_not_need_inline do
@@ -249,11 +296,23 @@ RSpec.describe Issues::CloseService do
       end
 
       it 'marks todos as done' do
+        todo = create(:todo, :assigned, user: user, project: project, target: issue, author: user2)
+
         close_issue
 
         expect(todo.reload).to be_done
       end
 
+      context 'when closing the issue fails' do
+        it 'does not assign a closed_by value for the issue' do
+          allow(issue).to receive(:close).and_return(false)
+
+          close_issue
+
+          expect(issue.closed_by_id).to be_nil
+        end
+      end
+
       context 'when there is an associated Alert Management Alert' do
         context 'when alert can be resolved' do
           let!(:alert) { create(:alert_management_alert, issue: issue, project: project) }
@@ -303,26 +362,32 @@ RSpec.describe Issues::CloseService do
     end
 
     context 'when issue is not confidential' do
+      it_behaves_like 'removes labels marked for removal from issue when closed'
+
       it 'executes issue hooks' do
         expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks)
         expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks)
 
-        described_class.new(project, user).close_issue(issue)
+        close_issue
       end
     end
 
     context 'when issue is confidential' do
-      it 'executes confidential issue hooks' do
-        issue = create(:issue, :confidential, project: project)
+      let(:issue) { create(:issue, :confidential, project: project) }
+
+      it_behaves_like 'removes labels marked for removal from issue when closed'
 
+      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)
 
-        described_class.new(project, user).close_issue(issue)
+        close_issue
       end
     end
 
     context 'internal issues disabled' do
+      let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) }
+
       before do
         project.issues_enabled = false
         project.save!