From 21f10af0956c69b6a5bad6b36b7d6e12e60e7867 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Tue, 30 Aug 2016 19:11:46 -0300
Subject: [PATCH] Scope hooks thal will run for confidential issues

---
 app/models/hooks/project_hook.rb            |  1 +
 app/models/service.rb                       |  1 +
 app/services/issues/base_service.rb         |  9 +++----
 spec/services/issues/close_service_spec.rb  | 15 ++++++++---
 spec/services/issues/create_service_spec.rb | 15 ++++++++---
 spec/services/issues/reopen_service_spec.rb | 30 +++++++++++++++------
 spec/services/issues/update_service_spec.rb |  6 ++---
 7 files changed, 55 insertions(+), 22 deletions(-)

diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb
index 836a75b0608..c631e7a7df5 100644
--- a/app/models/hooks/project_hook.rb
+++ b/app/models/hooks/project_hook.rb
@@ -2,6 +2,7 @@ class ProjectHook < WebHook
   belongs_to :project
 
   scope :issue_hooks, -> { where(issues_events: true) }
+  scope :confidential_issue_hooks, -> { where(confidential_issues_events: true) }
   scope :note_hooks, -> { where(note_events: true) }
   scope :merge_request_hooks, -> { where(merge_requests_events: true) }
   scope :build_hooks, -> { where(build_events: true) }
diff --git a/app/models/service.rb b/app/models/service.rb
index 7333f8d381b..198e7247838 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -34,6 +34,7 @@ class Service < ActiveRecord::Base
   scope :push_hooks, -> { where(push_events: true, active: true) }
   scope :tag_push_hooks, -> { where(tag_push_events: true, active: true) }
   scope :issue_hooks, -> { where(issues_events: true, active: true) }
+  scope :confidential_issue_hooks, -> { where(confidential_issues_events: true, active: true) }
   scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) }
   scope :note_hooks, -> { where(note_events: true, active: true) }
   scope :build_hooks, -> { where(build_events: true, active: true) }
diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb
index 241efc44d36..9ea3ce084ba 100644
--- a/app/services/issues/base_service.rb
+++ b/app/services/issues/base_service.rb
@@ -14,11 +14,10 @@ module Issues
     end
 
     def execute_hooks(issue, action = 'open')
-      return if issue.confidential?
-
-      issue_data = hook_data(issue, action)
-      issue.project.execute_hooks(issue_data, :issue_hooks)
-      issue.project.execute_services(issue_data, :issue_hooks)
+      issue_data  = hook_data(issue, action)
+      hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks
+      issue.project.execute_hooks(issue_data, hooks_scope)
+      issue.project.execute_services(issue_data, hooks_scope)
     end
   end
 end
diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb
index 4df99e41987..5dfb33f4b28 100644
--- a/spec/services/issues/close_service_spec.rb
+++ b/spec/services/issues/close_service_spec.rb
@@ -53,12 +53,21 @@ describe Issues::CloseService, services: true do
       end
     end
 
+    context 'when issue is not confidential' do
+      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).execute(issue)
+      end
+    end
+
     context 'when issue is confidential' do
-      it 'does not execute hooks' do
+      it 'executes confidential issue hooks' do
         issue = create(:issue, :confidential, project: project)
 
-        expect(project).not_to receive(:execute_hooks)
-        expect(project).not_to receive(:execute_services)
+        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).execute(issue)
       end
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index 81beca47738..58569ba96c3 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -73,11 +73,20 @@ describe Issues::CreateService, services: true do
         end
       end
 
-      it 'does not execute hooks when issue is confidential' do
+      it 'executes issue hooks when issue is not confidential' do
+        opts = { title: 'Title', description: 'Description', confidential: false }
+
+        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, opts).execute
+      end
+
+      it 'executes confidential issue hooks when issue is confidential' do
         opts = { title: 'Title', description: 'Description', confidential: true }
 
-        expect(project).not_to receive(:execute_hooks)
-        expect(project).not_to receive(:execute_services)
+        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, opts).execute
       end
diff --git a/spec/services/issues/reopen_service_spec.rb b/spec/services/issues/reopen_service_spec.rb
index 4549e2b395b..93a8270fd16 100644
--- a/spec/services/issues/reopen_service_spec.rb
+++ b/spec/services/issues/reopen_service_spec.rb
@@ -5,7 +5,7 @@ describe Issues::ReopenService, services: true do
   let(:issue) { create(:issue, :closed, project: project) }
 
   describe '#execute' do
-    context 'current user is not authorized to reopen issue' do
+    context 'when user is not authorized to reopen issue' do
       before do
         guest = create(:user)
         project.team << [guest, :guest]
@@ -20,17 +20,31 @@ describe Issues::ReopenService, services: true do
       end
     end
 
-    context 'when issue is confidential' do
-      it 'does not execute hooks' do
-        user = create(:user)
+    context 'when user is authrized to reopen issue' do
+      let(:user) { create(:user) }
+
+      before do
         project.team << [user, :master]
+      end
+
+      context 'when issue is not confidential' do
+        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)
 
-        issue = create(:issue, :confidential, :closed, project: project)
+          described_class.new(project, user).execute(issue)
+        end
+      end
 
-        expect(project).not_to receive(:execute_hooks)
-        expect(project).not_to receive(:execute_services)
+      context 'when issue is confidential' do
+        it 'executes confidential issue hooks' do
+          issue = create(:issue, :confidential, :closed, project: project)
 
-        described_class.new(project, user).execute(issue)
+          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).execute(issue)
+        end
       end
     end
   end
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 3c3a861b3af..4f5375a3583 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -105,9 +105,9 @@ describe Issues::UpdateService, services: true do
         expect(note.note).to eq 'Made the issue confidential'
       end
 
-      it 'does not execute hooks' do
-        expect(project).not_to receive(:execute_hooks)
-        expect(project).not_to receive(:execute_services)
+      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
-- 
GitLab