From c919f9ec155a0e38f1ab0b6c74df0d9da272f7fb Mon Sep 17 00:00:00 2001
From: Peter Leitzen <pleitzen@gitlab.com>
Date: Tue, 26 Feb 2019 18:52:22 +0100
Subject: [PATCH 01/10] Expose and manage incident management settings

---
 .../settings/operations_controller.rb         | 19 ++++++++++++++++---
 .../ee/projects/operations/update_service.rb  |  5 +++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/ee/app/controllers/ee/projects/settings/operations_controller.rb b/ee/app/controllers/ee/projects/settings/operations_controller.rb
index d7d58c5872c1c2..38a5732f3c7401 100644
--- a/ee/app/controllers/ee/projects/settings/operations_controller.rb
+++ b/ee/app/controllers/ee/projects/settings/operations_controller.rb
@@ -8,6 +8,8 @@ module OperationsController
         extend ActiveSupport::Concern
 
         prepended do
+          include ::Gitlab::Utils::StrongMemoize
+
           before_action :authorize_read_prometheus_alerts!,
             only: [:reset_alerting_token]
 
@@ -25,7 +27,7 @@ def reset_alerting_token
             end
           end
 
-          helper_method :tracing_setting
+          helper_method :tracing_setting, :incident_management_setting
 
           private
 
@@ -43,13 +45,24 @@ def alerting_params
           end
 
           def tracing_setting
-            @tracing_setting ||= project.tracing_setting || project.build_tracing_setting
+            strong_memoize(:tracing_setting) do
+              project.tracing_setting || project.build_tracing_setting
+            end
+          end
+
+          def incident_management_setting
+            strong_memoize(:incident_management_setting) do
+              project.incident_management_setting || project.build_incident_management_setting
+            end
           end
         end
 
         override :permitted_project_params
         def permitted_project_params
-          super.merge(tracing_setting_attributes: [:external_url])
+          super.merge(
+            tracing_setting_attributes: [:external_url],
+            incident_management_setting_attributes: [:create_issue, :send_email, :issue_template_key]
+          )
         end
 
         override :render_update_response
diff --git a/ee/app/services/ee/projects/operations/update_service.rb b/ee/app/services/ee/projects/operations/update_service.rb
index f8d67c71eb852b..ef09c57d51aaed 100644
--- a/ee/app/services/ee/projects/operations/update_service.rb
+++ b/ee/app/services/ee/projects/operations/update_service.rb
@@ -12,6 +12,7 @@ def project_update_params
           super
             .merge(tracing_setting_params)
             .merge(alerting_setting_params)
+            .merge(incident_management_setting_params)
         end
 
         private
@@ -41,6 +42,10 @@ def alerting_setting_params
 
           { alerting_setting_attributes: attr }
         end
+
+        def incident_management_setting_params
+          params.slice(:incident_management_setting_attributes)
+        end
       end
     end
   end
-- 
GitLab


From 1c75d09098491b680c71bf2c945a2ff8c5ee3b22 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Fri, 1 Mar 2019 17:52:49 +0530
Subject: [PATCH 02/10] Add incident_management license

- Add a license check for incident management.
- Also refactor specs a bit.
---
 .../settings/operations_controller.rb         |  5 --
 .../settings/operations_controller.rb         | 23 ++++--
 ee/app/models/license.rb                      |  1 +
 .../settings/operations_controller_spec.rb    | 70 ++++++++++---------
 4 files changed, 56 insertions(+), 43 deletions(-)

diff --git a/app/controllers/projects/settings/operations_controller.rb b/app/controllers/projects/settings/operations_controller.rb
index 2953db764a0c73..15e565d5742d23 100644
--- a/app/controllers/projects/settings/operations_controller.rb
+++ b/app/controllers/projects/settings/operations_controller.rb
@@ -3,7 +3,6 @@
 module Projects
   module Settings
     class OperationsController < Projects::ApplicationController
-      before_action :check_license
       before_action :authorize_update_environment!
 
       helper_method :error_tracking_setting
@@ -65,10 +64,6 @@ def permitted_project_params
           ]
         }
       end
-
-      def check_license
-        render_404 unless helpers.settings_operations_available?
-      end
     end
   end
 end
diff --git a/ee/app/controllers/ee/projects/settings/operations_controller.rb b/ee/app/controllers/ee/projects/settings/operations_controller.rb
index 38a5732f3c7401..ac6072f4ffd862 100644
--- a/ee/app/controllers/ee/projects/settings/operations_controller.rb
+++ b/ee/app/controllers/ee/projects/settings/operations_controller.rb
@@ -55,14 +55,29 @@ def incident_management_setting
               project.incident_management_setting || project.build_incident_management_setting
             end
           end
+
+          def has_tracing_license?
+            project.feature_available?(:tracing, current_user)
+          end
+
+          def has_incident_management_license?
+            project.feature_available?(:incident_management, current_user)
+          end
         end
 
         override :permitted_project_params
         def permitted_project_params
-          super.merge(
-            tracing_setting_attributes: [:external_url],
-            incident_management_setting_attributes: [:create_issue, :send_email, :issue_template_key]
-          )
+          permitted_params = super
+
+          if has_tracing_license?
+            permitted_params[:tracing_setting_attributes] = [:external_url]
+          end
+
+          if has_incident_management_license?
+            permitted_params[:incident_management_setting_attributes] = [:create_issue, :send_email, :issue_template_key]
+          end
+
+          permitted_params
         end
 
         override :render_update_response
diff --git a/ee/app/models/license.rb b/ee/app/models/license.rb
index 0104ea9becd661..c072c6144ea19f 100644
--- a/ee/app/models/license.rb
+++ b/ee/app/models/license.rb
@@ -95,6 +95,7 @@ class License < ActiveRecord::Base
     tracing
     insights
     web_ide_terminal
+    incident_management
   ]
   EEU_FEATURES.freeze
 
diff --git a/ee/spec/controllers/projects/settings/operations_controller_spec.rb b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
index 003889797e26b6..483322ae5d8e25 100644
--- a/ee/spec/controllers/projects/settings/operations_controller_spec.rb
+++ b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
@@ -10,19 +10,17 @@
   end
 
   describe 'GET show' do
-    shared_examples 'user without read access' do |project_visibility|
+    shared_examples 'user without read access' do |project_visibility, project_role|
       let(:project) { create(:project, project_visibility) }
 
-      %w[guest reporter developer].each do |role|
-        before do
-          project.public_send("add_#{role}", user)
-        end
+      before do
+        project.public_send("add_#{project_role}", user)
+      end
 
-        it 'returns 404' do
-          get :show, params: project_params(project)
+      it 'returns 404' do
+        get :show, params: project_params(project)
 
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
+        expect(response).to have_gitlab_http_status(:not_found)
       end
     end
 
@@ -63,9 +61,11 @@
       end
 
       context 'without maintainer role' do
-        it_behaves_like 'user without read access', :public
-        it_behaves_like 'user without read access', :private
-        it_behaves_like 'user without read access', :internal
+        %w[guest reporter developer].each do |role|
+          it_behaves_like 'user without read access', :public, role
+          it_behaves_like 'user without read access', :private, role
+          it_behaves_like 'user without read access', :internal, role
+        end
       end
 
       context 'when user not logged in' do
@@ -73,7 +73,7 @@
           sign_out(user)
         end
 
-        it_behaves_like 'user without read access', :public
+        it_behaves_like 'user without read access', :public, :maintainer
 
         it_behaves_like 'user needs to login', :private
         it_behaves_like 'user needs to login', :internal
@@ -85,9 +85,9 @@
         stub_licensed_features(tracing: false)
       end
 
-      it_behaves_like 'user without read access', :public
-      it_behaves_like 'user without read access', :private
-      it_behaves_like 'user without read access', :internal
+      it_behaves_like 'user with read access', :public
+      it_behaves_like 'user with read access', :private
+      it_behaves_like 'user with read access', :internal
     end
   end
 
@@ -102,19 +102,17 @@
       internal_project.add_maintainer(user)
     end
 
-    shared_examples 'user without write access' do |project_visibility|
+    shared_examples 'user without write access' do |project_visibility, project_role|
       let(:project) { create(:project, project_visibility) }
 
-      %w[guest reporter developer].each do |role|
-        before do
-          project.public_send("add_#{role}", user)
-        end
+      before do
+        project.public_send("add_#{project_role}", user)
+      end
 
-        it 'does not update tracing external_url' do
-          update_project(project, external_url: 'https://gitlab.com')
+      it 'does not update tracing external_url' do
+        update_project(project, external_url: 'https://gitlab.com')
 
-          expect(project.tracing_setting).to be_nil
-        end
+        expect(project.tracing_setting).to be_nil
       end
     end
 
@@ -181,9 +179,13 @@
       end
 
       context 'with non maintainer roles' do
-        it_behaves_like 'user without write access', :public
-        it_behaves_like 'user without write access', :private
-        it_behaves_like 'user without write access', :internal
+        %w[guest reporter developer].each do |role|
+          context "with #{role} role" do
+            it_behaves_like 'user without write access', :public, role
+            it_behaves_like 'user without write access', :private, role
+            it_behaves_like 'user without write access', :internal, role
+          end
+        end
       end
 
       context 'with anonymous user' do
@@ -191,9 +193,9 @@
           sign_out(user)
         end
 
-        it_behaves_like 'user without write access', :public
-        it_behaves_like 'user without write access', :private
-        it_behaves_like 'user without write access', :internal
+        it_behaves_like 'user without write access', :public, :maintainer
+        it_behaves_like 'user without write access', :private, :maintainer
+        it_behaves_like 'user without write access', :internal, :maintainer
       end
 
       context 'with existing tracing_setting' do
@@ -255,9 +257,9 @@
         stub_licensed_features(tracing: false)
       end
 
-      it_behaves_like 'user without write access', :public
-      it_behaves_like 'user without write access', :private
-      it_behaves_like 'user without write access', :internal
+      it_behaves_like 'user without write access', :public, :maintainer
+      it_behaves_like 'user without write access', :private, :maintainer
+      it_behaves_like 'user without write access', :internal, :maintainer
     end
 
     private
-- 
GitLab


From 2cdec7f55b277fa582f1e6b26b4c127576c4b1a1 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Fri, 1 Mar 2019 20:05:36 +0530
Subject: [PATCH 03/10] Add specs for incident_management_settings

---
 ee/app/models/ee/project.rb                   |  1 +
 .../settings/operations_controller_spec.rb    | 84 ++++++++++++++-----
 2 files changed, 64 insertions(+), 21 deletions(-)

diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb
index 0fc6bd7a04a14c..734b0baca9f732 100644
--- a/ee/app/models/ee/project.rb
+++ b/ee/app/models/ee/project.rb
@@ -123,6 +123,7 @@ module Project
 
       accepts_nested_attributes_for :tracing_setting, update_only: true, allow_destroy: true
       accepts_nested_attributes_for :alerting_setting, update_only: true
+      accepts_nested_attributes_for :incident_management_setting, update_only: true
 
       alias_attribute :fallback_approvals_required, :approvals_before_merge
     end
diff --git a/ee/spec/controllers/projects/settings/operations_controller_spec.rb b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
index 483322ae5d8e25..813a8b62cc9a14 100644
--- a/ee/spec/controllers/projects/settings/operations_controller_spec.rb
+++ b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
@@ -96,6 +96,14 @@
     let(:private_project) { create(:project, :private) }
     let(:internal_project) { create(:project, :internal) }
 
+    let(:incident_management_settings) do
+      {
+        create_issue: true,
+        send_email: true,
+        issue_template_key: 'some-key'
+      }
+    end
+
     before do
       public_project.add_maintainer(user)
       private_project.add_maintainer(user)
@@ -109,10 +117,15 @@
         project.public_send("add_#{project_role}", user)
       end
 
-      it 'does not update tracing external_url' do
-        update_project(project, external_url: 'https://gitlab.com')
+      it 'does not update' do
+        update_project(
+          project,
+          tracing_params: { external_url: 'https://gitlab.com' },
+          incident_mgmt_params: incident_management_settings
+        )
 
         expect(project.tracing_setting).to be_nil
+        expect(project.incident_management_setting).to be_nil
       end
     end
 
@@ -132,7 +145,7 @@
         end
 
         it 'shows a notice' do
-          update_project(project, { external_url: 'http://gitlab.com' })
+          update_project(project, tracing_params: { external_url: 'http://gitlab.com' })
 
           expect(response).to redirect_to(project_settings_operations_url(project))
           expect(flash[:notice]).to eq _('Your changes have been saved')
@@ -145,7 +158,7 @@
         end
 
         it 'renders show page' do
-          update_project(project, { external_url: 'http://gitlab.com' })
+          update_project(project, tracing_params: { external_url: 'http://gitlab.com' })
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(response).to render_template(:show)
@@ -155,27 +168,55 @@
 
     context 'with a license' do
       before do
-        stub_licensed_features(tracing: true)
+        stub_licensed_features(tracing: true, incident_management: true)
       end
 
-      shared_examples 'user with write access' do |project_visibility, value_to_set, value_to_check|
+      shared_examples 'user with write access' do |project_visibility|
         let(:project) { create(:project, project_visibility) }
+        let(:tracing_url) { 'https://gitlab.com' }
 
         before do
           project.add_maintainer(user)
         end
 
         it 'updates tracing external_url' do
-          update_project(project, external_url: value_to_set)
+          update_project(
+            project,
+            tracing_params: { external_url: tracing_url }
+          )
+
+          expect(project.tracing_setting.external_url).to eq(tracing_url)
+        end
+
+        it 'updates incident management settings' do
+          update_project(
+            project,
+            incident_mgmt_params: incident_management_settings
+          )
+
+          expect(project.incident_management_setting.create_issue).to eq(incident_management_settings.dig(:create_issue))
+          expect(project.incident_management_setting.send_email).to eq(incident_management_settings.dig(:send_email))
+          expect(project.incident_management_setting.issue_template_key).to eq(incident_management_settings.dig(:issue_template_key))
+        end
 
-          expect(project.tracing_setting.external_url).to eq(value_to_check)
+        it 'updates tracing and incident management settings' do
+          update_project(
+            project,
+            tracing_params: { external_url: tracing_url },
+            incident_mgmt_params: incident_management_settings
+          )
+
+          expect(project.tracing_setting.external_url).to eq(tracing_url)
+          expect(project.incident_management_setting.create_issue).to eq(incident_management_settings.dig(:create_issue))
+          expect(project.incident_management_setting.send_email).to eq(incident_management_settings.dig(:send_email))
+          expect(project.incident_management_setting.issue_template_key).to eq(incident_management_settings.dig(:issue_template_key))
         end
       end
 
       context 'with maintainer role' do
-        it_behaves_like 'user with write access', :public, 'https://gitlab.com', 'https://gitlab.com'
-        it_behaves_like 'user with write access', :private, 'https://gitlab.com', 'https://gitlab.com'
-        it_behaves_like 'user with write access', :internal, 'https://gitlab.com', 'https://gitlab.com'
+        it_behaves_like 'user with write access', :public
+        it_behaves_like 'user with write access', :private
+        it_behaves_like 'user with write access', :internal
       end
 
       context 'with non maintainer roles' do
@@ -207,26 +248,26 @@
         end
 
         it 'unsets external_url with nil' do
-          update_project(project, external_url: nil)
+          update_project(project, tracing_params: { external_url: nil })
 
           expect(project.tracing_setting).to be_nil
         end
 
         it 'unsets external_url with empty string' do
-          update_project(project, external_url: '')
+          update_project(project, tracing_params: { external_url: '' })
 
           expect(project.tracing_setting).to be_nil
         end
 
         it 'fails validation with invalid url' do
           expect do
-            update_project(project, external_url: "invalid")
+            update_project(project, tracing_params: { external_url: "invalid" })
           end.not_to change(project.tracing_setting, :external_url)
         end
 
         it 'does not set external_url if not present in params' do
           expect do
-            update_project(project, some_param: 'some_value')
+            update_project(project, tracing_params: { some_param: 'some_value' })
           end.not_to change(project.tracing_setting, :external_url)
         end
       end
@@ -239,13 +280,13 @@
         end
 
         it 'fails validation with invalid url' do
-          update_project(project, external_url: "invalid")
+          update_project(project, tracing_params: { external_url: "invalid" })
 
           expect(project.tracing_setting).to be_nil
         end
 
         it 'does not set external_url if not present in params' do
-          update_project(project, some_param: 'some_value')
+          update_project(project, tracing_params: { some_param: 'some_value' })
 
           expect(project.tracing_setting).to be_nil
         end
@@ -264,8 +305,8 @@
 
     private
 
-    def update_project(project, params)
-      patch :update, params: project_params(project, params)
+    def update_project(project, tracing_params: nil, incident_mgmt_params: nil)
+      patch :update, params: project_params(project, tracing_params: tracing_params, incident_mgmt_params: incident_mgmt_params)
 
       project.reload
     end
@@ -374,12 +415,13 @@ def reset_alerting_token
 
   private
 
-  def project_params(project, params = {})
+  def project_params(project, tracing_params: nil, incident_mgmt_params: nil)
     {
       namespace_id: project.namespace,
       project_id: project,
       project: {
-        tracing_setting_attributes: params
+        tracing_setting_attributes: tracing_params,
+        incident_management_setting_attributes: incident_mgmt_params
       }
     }
   end
-- 
GitLab


From 67afa7efd0a2963a55d39cf277312105d6f7fff9 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Mon, 4 Mar 2019 11:23:38 +0530
Subject: [PATCH 04/10] Add spec for updating existing settings

- Spec for updating existing incident management settings.
---
 .../settings/operations_controller_spec.rb    | 37 +++++++++++++++++--
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/ee/spec/controllers/projects/settings/operations_controller_spec.rb b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
index 813a8b62cc9a14..16f38abd8a95b7 100644
--- a/ee/spec/controllers/projects/settings/operations_controller_spec.rb
+++ b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
@@ -117,7 +117,7 @@
         project.public_send("add_#{project_role}", user)
       end
 
-      it 'does not update' do
+      it 'does not create' do
         update_project(
           project,
           tracing_params: { external_url: 'https://gitlab.com' },
@@ -179,7 +179,7 @@
           project.add_maintainer(user)
         end
 
-        it 'updates tracing external_url' do
+        it 'creates tracing setting' do
           update_project(
             project,
             tracing_params: { external_url: tracing_url }
@@ -188,7 +188,7 @@
           expect(project.tracing_setting.external_url).to eq(tracing_url)
         end
 
-        it 'updates incident management settings' do
+        it 'creates incident management settings' do
           update_project(
             project,
             incident_mgmt_params: incident_management_settings
@@ -199,7 +199,7 @@
           expect(project.incident_management_setting.issue_template_key).to eq(incident_management_settings.dig(:issue_template_key))
         end
 
-        it 'updates tracing and incident management settings' do
+        it 'creates tracing and incident management settings' do
           update_project(
             project,
             tracing_params: { external_url: tracing_url },
@@ -291,6 +291,35 @@
           expect(project.tracing_setting).to be_nil
         end
       end
+
+      context 'with existing incident management setting' do
+        let(:project) { create(:project) }
+
+        let(:incident_management_settings) do
+          create(:project_incident_management_setting, **incident_management_settings)
+        end
+
+        let(:new_incident_management_settings) do
+          {
+            create_issue: false,
+            send_email: false,
+            issue_template_key: 'some-other-template'
+          }
+        end
+
+        before do
+          project.add_maintainer(user)
+        end
+
+        it 'updates incident management setting' do
+          update_project(project, incident_mgmt_params: new_incident_management_settings)
+
+          setting = project.incident_management_setting
+          expect(setting.create_issue).to eq(new_incident_management_settings[:create_issue])
+          expect(setting.send_email).to eq(new_incident_management_settings[:send_email])
+          expect(setting.issue_template_key).to eq(new_incident_management_settings[:issue_template_key])
+        end
+      end
     end
 
     context 'without a license' do
-- 
GitLab


From 050718152b1f6080e94b3c74463265d1e0cfde96 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Tue, 5 Mar 2019 18:33:06 +0530
Subject: [PATCH 05/10] Add specs for no incident_management license

---
 .../settings/operations_controller_spec.rb    | 60 +++++++++++++++----
 1 file changed, 48 insertions(+), 12 deletions(-)

diff --git a/ee/spec/controllers/projects/settings/operations_controller_spec.rb b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
index 16f38abd8a95b7..d26ae0d0117613 100644
--- a/ee/spec/controllers/projects/settings/operations_controller_spec.rb
+++ b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
@@ -14,7 +14,7 @@
       let(:project) { create(:project, project_visibility) }
 
       before do
-        project.public_send("add_#{project_role}", user)
+        project.add_role(user, project_role)
       end
 
       it 'returns 404' do
@@ -51,7 +51,7 @@
 
     context 'with a license' do
       before do
-        stub_licensed_features(tracing: true)
+        stub_licensed_features(tracing: true, incident_management: true)
       end
 
       context 'with maintainer role' do
@@ -82,7 +82,7 @@
 
     context 'without license' do
       before do
-        stub_licensed_features(tracing: false)
+        stub_licensed_features(tracing: false, incident_management: false)
       end
 
       it_behaves_like 'user with read access', :public
@@ -114,17 +114,24 @@
       let(:project) { create(:project, project_visibility) }
 
       before do
-        project.public_send("add_#{project_role}", user)
+        project.add_role(user, project_role)
       end
 
-      it 'does not create' do
+      it 'does not create tracing_setting' do
         update_project(
           project,
-          tracing_params: { external_url: 'https://gitlab.com' },
-          incident_mgmt_params: incident_management_settings
+          tracing_params: { external_url: 'https://gitlab.com' }
         )
 
         expect(project.tracing_setting).to be_nil
+      end
+
+      it 'does not create incident_management_setting' do
+        update_project(
+          project,
+          incident_mgmt_params: incident_management_settings
+        )
+
         expect(project.incident_management_setting).to be_nil
       end
     end
@@ -295,10 +302,6 @@
       context 'with existing incident management setting' do
         let(:project) { create(:project) }
 
-        let(:incident_management_settings) do
-          create(:project_incident_management_setting, **incident_management_settings)
-        end
-
         let(:new_incident_management_settings) do
           {
             create_issue: false,
@@ -307,6 +310,10 @@
           }
         end
 
+        let!(:incident_management_setting) do
+          create(:project_incident_management_setting, project: project, **incident_management_settings)
+        end
+
         before do
           project.add_maintainer(user)
         end
@@ -324,12 +331,41 @@
 
     context 'without a license' do
       before do
-        stub_licensed_features(tracing: false)
+        stub_licensed_features(tracing: false, incident_management: false)
       end
 
       it_behaves_like 'user without write access', :public, :maintainer
       it_behaves_like 'user without write access', :private, :maintainer
       it_behaves_like 'user without write access', :internal, :maintainer
+
+      context 'with existing incident_management_setting' do
+        let(:project) { create(:project) }
+
+        let(:new_incident_management_settings) do
+          {
+            create_issue: false,
+            send_email: false,
+            issue_template_key: 'some-other-template'
+          }
+        end
+
+        let!(:incident_management_setting) do
+          create(:project_incident_management_setting, project: project, **incident_management_settings)
+        end
+
+        before do
+          project.add_maintainer(user)
+        end
+
+        it 'does not update incident_management_setting' do
+          update_project(project, incident_mgmt_params: new_incident_management_settings)
+
+          setting = project.incident_management_setting
+          expect(setting.create_issue).to eq(incident_management_settings[:create_issue])
+          expect(setting.send_email).to eq(incident_management_settings[:send_email])
+          expect(setting.issue_template_key).to eq(incident_management_settings[:issue_template_key])
+        end
+      end
     end
 
     private
-- 
GitLab


From 56da74ebcb060e7f5621270cb207eb46313acc54 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Tue, 5 Mar 2019 20:42:13 +0530
Subject: [PATCH 06/10] Move helper to projects_helper.rb

- Also add a spec for ee projects helpers.
---
 .../settings/operations_controller.rb         | 14 ++-------
 ee/app/helpers/ee/projects_helper.rb          |  5 +++
 .../project_incident_management_setting.rb    |  7 +++++
 ee/spec/helpers/ee/projects_helper_spec.rb    | 31 +++++++++++++++++++
 4 files changed, 45 insertions(+), 12 deletions(-)
 create mode 100644 ee/spec/helpers/ee/projects_helper_spec.rb

diff --git a/ee/app/controllers/ee/projects/settings/operations_controller.rb b/ee/app/controllers/ee/projects/settings/operations_controller.rb
index ac6072f4ffd862..7e8bced93709d2 100644
--- a/ee/app/controllers/ee/projects/settings/operations_controller.rb
+++ b/ee/app/controllers/ee/projects/settings/operations_controller.rb
@@ -8,8 +8,6 @@ module OperationsController
         extend ActiveSupport::Concern
 
         prepended do
-          include ::Gitlab::Utils::StrongMemoize
-
           before_action :authorize_read_prometheus_alerts!,
             only: [:reset_alerting_token]
 
@@ -27,7 +25,7 @@ def reset_alerting_token
             end
           end
 
-          helper_method :tracing_setting, :incident_management_setting
+          helper_method :tracing_setting
 
           private
 
@@ -45,15 +43,7 @@ def alerting_params
           end
 
           def tracing_setting
-            strong_memoize(:tracing_setting) do
-              project.tracing_setting || project.build_tracing_setting
-            end
-          end
-
-          def incident_management_setting
-            strong_memoize(:incident_management_setting) do
-              project.incident_management_setting || project.build_incident_management_setting
-            end
+            @tracing_setting ||= project.tracing_setting || project.build_tracing_setting
           end
 
           def has_tracing_license?
diff --git a/ee/app/helpers/ee/projects_helper.rb b/ee/app/helpers/ee/projects_helper.rb
index 9989d9a290b491..61da461b258bb5 100644
--- a/ee/app/helpers/ee/projects_helper.rb
+++ b/ee/app/helpers/ee/projects_helper.rb
@@ -214,5 +214,10 @@ def settings_operations_available?
 
       @project.feature_available?(:tracing, current_user) && can?(current_user, :read_environment, @project)
     end
+
+    def project_incident_management_setting
+      @project_incident_management_setting ||= @project.incident_management_setting ||
+        @project.build_incident_management_setting
+    end
   end
 end
diff --git a/ee/app/models/incident_management/project_incident_management_setting.rb b/ee/app/models/incident_management/project_incident_management_setting.rb
index 0aa75ecf4f7e91..730be1abe976e7 100644
--- a/ee/app/models/incident_management/project_incident_management_setting.rb
+++ b/ee/app/models/incident_management/project_incident_management_setting.rb
@@ -6,8 +6,15 @@ class ProjectIncidentManagementSetting < ApplicationRecord
 
     validate :issue_template_exists, if: :create_issue?
 
+    after_initialize :set_defaults
+
     private
 
+    def set_defaults
+      self.send_email = true
+      self.create_issue = false
+    end
+
     def issue_template_exists
       return unless issue_template_key.present?
 
diff --git a/ee/spec/helpers/ee/projects_helper_spec.rb b/ee/spec/helpers/ee/projects_helper_spec.rb
new file mode 100644
index 00000000000000..23ebf50e11b2af
--- /dev/null
+++ b/ee/spec/helpers/ee/projects_helper_spec.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProjectsHelper do
+  describe '#project_incident_management_setting' do
+    let(:project) { create(:project) }
+
+    before do
+      helper.instance_variable_set(:@project, project)
+    end
+
+    context 'when incident_management_setting exists' do
+      let(:project_incident_management_setting) { create(:project_incident_management_setting, project: project) }
+
+      it 'return project_incident_management_setting' do
+        expect(helper.project_incident_management_setting).to eq(project_incident_management_setting)
+      end
+    end
+
+    context 'when incident_management_setting does not exist' do
+      it 'builds incident_management_setting' do
+        expect(helper.project_incident_management_setting.persisted?).to be false
+
+        expect(helper.project_incident_management_setting.send_email).to be true
+        expect(helper.project_incident_management_setting.create_issue).to be false
+        expect(helper.project_incident_management_setting.issue_template_key).to be nil
+      end
+    end
+  end
+end
-- 
GitLab


From 9b215e4e6cf1acec6b6b799fc4986ac867ed4aaa Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Wed, 6 Mar 2019 13:38:07 +0530
Subject: [PATCH 07/10] Remove after_initialize callback

---
 .../project_incident_management_setting.rb                 | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/ee/app/models/incident_management/project_incident_management_setting.rb b/ee/app/models/incident_management/project_incident_management_setting.rb
index 730be1abe976e7..0aa75ecf4f7e91 100644
--- a/ee/app/models/incident_management/project_incident_management_setting.rb
+++ b/ee/app/models/incident_management/project_incident_management_setting.rb
@@ -6,15 +6,8 @@ class ProjectIncidentManagementSetting < ApplicationRecord
 
     validate :issue_template_exists, if: :create_issue?
 
-    after_initialize :set_defaults
-
     private
 
-    def set_defaults
-      self.send_email = true
-      self.create_issue = false
-    end
-
     def issue_template_exists
       return unless issue_template_key.present?
 
-- 
GitLab


From 272c4bcf5e5398c221ac2d71756f578e85d389c4 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Thu, 7 Mar 2019 13:02:27 +0530
Subject: [PATCH 08/10] Add explicit feature flag check

This is because implicit feature flag checks default to enabled. We
would like to default to disabled.
---
 .../settings/operations_controller.rb         | 12 +++-
 .../settings/operations_controller_spec.rb    | 58 ++++++++++---------
 2 files changed, 41 insertions(+), 29 deletions(-)

diff --git a/ee/app/controllers/ee/projects/settings/operations_controller.rb b/ee/app/controllers/ee/projects/settings/operations_controller.rb
index 7e8bced93709d2..aaceb3819a6c70 100644
--- a/ee/app/controllers/ee/projects/settings/operations_controller.rb
+++ b/ee/app/controllers/ee/projects/settings/operations_controller.rb
@@ -25,7 +25,7 @@ def reset_alerting_token
             end
           end
 
-          helper_method :tracing_setting
+          helper_method :tracing_setting, :incident_management_available?
 
           private
 
@@ -53,6 +53,14 @@ def has_tracing_license?
           def has_incident_management_license?
             project.feature_available?(:incident_management, current_user)
           end
+
+          def incident_management_feature_enabled?
+            ::Feature.enabled?(:incident_management)
+          end
+
+          def incident_management_available?
+            incident_management_feature_enabled? && has_incident_management_license?
+          end
         end
 
         override :permitted_project_params
@@ -63,7 +71,7 @@ def permitted_project_params
             permitted_params[:tracing_setting_attributes] = [:external_url]
           end
 
-          if has_incident_management_license?
+          if incident_management_available?
             permitted_params[:incident_management_setting_attributes] = [:create_issue, :send_email, :issue_template_key]
           end
 
diff --git a/ee/spec/controllers/projects/settings/operations_controller_spec.rb b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
index d26ae0d0117613..ee9694cc8dd164 100644
--- a/ee/spec/controllers/projects/settings/operations_controller_spec.rb
+++ b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
@@ -134,6 +134,29 @@
 
         expect(project.incident_management_setting).to be_nil
       end
+
+      context 'with existing incident_management_setting' do
+        let(:new_incident_management_settings) do
+          {
+            create_issue: false,
+            send_email: false,
+            issue_template_key: 'some-other-template'
+          }
+        end
+
+        let!(:incident_management_setting) do
+          create(:project_incident_management_setting, project: project, **incident_management_settings)
+        end
+
+        it 'does not update incident_management_setting' do
+          update_project(project, incident_mgmt_params: new_incident_management_settings)
+
+          setting = project.incident_management_setting
+          expect(setting.create_issue).to eq(incident_management_settings[:create_issue])
+          expect(setting.send_email).to eq(incident_management_settings[:send_email])
+          expect(setting.issue_template_key).to eq(incident_management_settings[:issue_template_key])
+        end
+      end
     end
 
     context 'format html' do
@@ -337,35 +360,16 @@
       it_behaves_like 'user without write access', :public, :maintainer
       it_behaves_like 'user without write access', :private, :maintainer
       it_behaves_like 'user without write access', :internal, :maintainer
+    end
 
-      context 'with existing incident_management_setting' do
-        let(:project) { create(:project) }
-
-        let(:new_incident_management_settings) do
-          {
-            create_issue: false,
-            send_email: false,
-            issue_template_key: 'some-other-template'
-          }
-        end
-
-        let!(:incident_management_setting) do
-          create(:project_incident_management_setting, project: project, **incident_management_settings)
-        end
-
-        before do
-          project.add_maintainer(user)
-        end
-
-        it 'does not update incident_management_setting' do
-          update_project(project, incident_mgmt_params: new_incident_management_settings)
-
-          setting = project.incident_management_setting
-          expect(setting.create_issue).to eq(incident_management_settings[:create_issue])
-          expect(setting.send_email).to eq(incident_management_settings[:send_email])
-          expect(setting.issue_template_key).to eq(incident_management_settings[:issue_template_key])
-        end
+    context 'with incident_management feature flag disabled' do
+      before do
+        stub_feature_flags(incident_management: false)
       end
+
+      it_behaves_like 'user without write access', :public, :maintainer
+      it_behaves_like 'user without write access', :private, :maintainer
+      it_behaves_like 'user without write access', :internal, :maintainer
     end
 
     private
-- 
GitLab


From a4d60d1f80d857e22af2634db23d57884f734ae2 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Thu, 7 Mar 2019 17:03:36 +0530
Subject: [PATCH 09/10] Refactor spec not to use abbreviations

- Also use set instead of let.
- Use () for consistency.
---
 .../settings/operations_controller_spec.rb    | 80 +++++++++++++------
 ee/spec/helpers/ee/projects_helper_spec.rb    | 18 +++--
 2 files changed, 68 insertions(+), 30 deletions(-)

diff --git a/ee/spec/controllers/projects/settings/operations_controller_spec.rb b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
index ee9694cc8dd164..4cd403895eb2a3 100644
--- a/ee/spec/controllers/projects/settings/operations_controller_spec.rb
+++ b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
@@ -129,7 +129,7 @@
       it 'does not create incident_management_setting' do
         update_project(
           project,
-          incident_mgmt_params: incident_management_settings
+          incident_management_params: incident_management_settings
         )
 
         expect(project.incident_management_setting).to be_nil
@@ -145,16 +145,25 @@
         end
 
         let!(:incident_management_setting) do
-          create(:project_incident_management_setting, project: project, **incident_management_settings)
+          create(:project_incident_management_setting,
+            project: project,
+            **incident_management_settings)
         end
 
         it 'does not update incident_management_setting' do
-          update_project(project, incident_mgmt_params: new_incident_management_settings)
+          update_project(project,
+            incident_management_params: new_incident_management_settings)
 
           setting = project.incident_management_setting
-          expect(setting.create_issue).to eq(incident_management_settings[:create_issue])
-          expect(setting.send_email).to eq(incident_management_settings[:send_email])
-          expect(setting.issue_template_key).to eq(incident_management_settings[:issue_template_key])
+          expect(setting.create_issue).to(
+            eq(incident_management_settings[:create_issue])
+          )
+          expect(setting.send_email).to(
+            eq(incident_management_settings[:send_email])
+          )
+          expect(setting.issue_template_key).to(
+            eq(incident_management_settings[:issue_template_key])
+          )
         end
       end
     end
@@ -221,25 +230,37 @@
         it 'creates incident management settings' do
           update_project(
             project,
-            incident_mgmt_params: incident_management_settings
+            incident_management_params: incident_management_settings
           )
 
-          expect(project.incident_management_setting.create_issue).to eq(incident_management_settings.dig(:create_issue))
-          expect(project.incident_management_setting.send_email).to eq(incident_management_settings.dig(:send_email))
-          expect(project.incident_management_setting.issue_template_key).to eq(incident_management_settings.dig(:issue_template_key))
+          expect(project.incident_management_setting.create_issue).to(
+            eq(incident_management_settings.dig(:create_issue))
+          )
+          expect(project.incident_management_setting.send_email).to(
+            eq(incident_management_settings.dig(:send_email))
+          )
+          expect(project.incident_management_setting.issue_template_key).to(
+            eq(incident_management_settings.dig(:issue_template_key))
+          )
         end
 
         it 'creates tracing and incident management settings' do
           update_project(
             project,
             tracing_params: { external_url: tracing_url },
-            incident_mgmt_params: incident_management_settings
+            incident_management_params: incident_management_settings
           )
 
           expect(project.tracing_setting.external_url).to eq(tracing_url)
-          expect(project.incident_management_setting.create_issue).to eq(incident_management_settings.dig(:create_issue))
-          expect(project.incident_management_setting.send_email).to eq(incident_management_settings.dig(:send_email))
-          expect(project.incident_management_setting.issue_template_key).to eq(incident_management_settings.dig(:issue_template_key))
+          expect(project.incident_management_setting.create_issue).to(
+            eq(incident_management_settings.dig(:create_issue))
+          )
+          expect(project.incident_management_setting.send_email).to(
+            eq(incident_management_settings.dig(:send_email))
+          )
+          expect(project.incident_management_setting.issue_template_key).to(
+            eq(incident_management_settings.dig(:issue_template_key))
+          )
         end
       end
 
@@ -334,7 +355,9 @@
         end
 
         let!(:incident_management_setting) do
-          create(:project_incident_management_setting, project: project, **incident_management_settings)
+          create(:project_incident_management_setting,
+            project: project,
+            **incident_management_settings)
         end
 
         before do
@@ -342,12 +365,19 @@
         end
 
         it 'updates incident management setting' do
-          update_project(project, incident_mgmt_params: new_incident_management_settings)
+          update_project(project,
+            incident_management_params: new_incident_management_settings)
 
           setting = project.incident_management_setting
-          expect(setting.create_issue).to eq(new_incident_management_settings[:create_issue])
-          expect(setting.send_email).to eq(new_incident_management_settings[:send_email])
-          expect(setting.issue_template_key).to eq(new_incident_management_settings[:issue_template_key])
+          expect(setting.create_issue).to(
+            eq(new_incident_management_settings[:create_issue])
+          )
+          expect(setting.send_email).to(
+            eq(new_incident_management_settings[:send_email])
+          )
+          expect(setting.issue_template_key).to(
+            eq(new_incident_management_settings[:issue_template_key])
+          )
         end
       end
     end
@@ -374,8 +404,12 @@
 
     private
 
-    def update_project(project, tracing_params: nil, incident_mgmt_params: nil)
-      patch :update, params: project_params(project, tracing_params: tracing_params, incident_mgmt_params: incident_mgmt_params)
+    def update_project(project, tracing_params: nil, incident_management_params: nil)
+      patch :update, params: project_params(
+        project,
+        tracing_params: tracing_params,
+        incident_management_params: incident_management_params
+      )
 
       project.reload
     end
@@ -484,13 +518,13 @@ def reset_alerting_token
 
   private
 
-  def project_params(project, tracing_params: nil, incident_mgmt_params: nil)
+  def project_params(project, tracing_params: nil, incident_management_params: nil)
     {
       namespace_id: project.namespace,
       project_id: project,
       project: {
         tracing_setting_attributes: tracing_params,
-        incident_management_setting_attributes: incident_mgmt_params
+        incident_management_setting_attributes: incident_management_params
       }
     }
   end
diff --git a/ee/spec/helpers/ee/projects_helper_spec.rb b/ee/spec/helpers/ee/projects_helper_spec.rb
index 23ebf50e11b2af..168b3d99fe855f 100644
--- a/ee/spec/helpers/ee/projects_helper_spec.rb
+++ b/ee/spec/helpers/ee/projects_helper_spec.rb
@@ -4,27 +4,31 @@
 
 describe ProjectsHelper do
   describe '#project_incident_management_setting' do
-    let(:project) { create(:project) }
+    set(:project) { create(:project) }
 
     before do
       helper.instance_variable_set(:@project, project)
     end
 
     context 'when incident_management_setting exists' do
-      let(:project_incident_management_setting) { create(:project_incident_management_setting, project: project) }
+      let(:project_incident_management_setting) do
+        create(:project_incident_management_setting, project: project)
+      end
 
       it 'return project_incident_management_setting' do
-        expect(helper.project_incident_management_setting).to eq(project_incident_management_setting)
+        expect(helper.project_incident_management_setting).to(
+          eq(project_incident_management_setting)
+        )
       end
     end
 
     context 'when incident_management_setting does not exist' do
       it 'builds incident_management_setting' do
-        expect(helper.project_incident_management_setting.persisted?).to be false
+        expect(helper.project_incident_management_setting.persisted?).to be(false)
 
-        expect(helper.project_incident_management_setting.send_email).to be true
-        expect(helper.project_incident_management_setting.create_issue).to be false
-        expect(helper.project_incident_management_setting.issue_template_key).to be nil
+        expect(helper.project_incident_management_setting.send_email).to be(true)
+        expect(helper.project_incident_management_setting.create_issue).to be(false)
+        expect(helper.project_incident_management_setting.issue_template_key).to be(nil)
       end
     end
   end
-- 
GitLab


From c92544c9b9ce8ce7aaa3b81b6530610f9e9654df Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Thu, 7 Mar 2019 19:35:53 +0530
Subject: [PATCH 10/10] Add changelog entry

---
 ee/changelogs/unreleased/4925-use-send-email-setting.yml | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 ee/changelogs/unreleased/4925-use-send-email-setting.yml

diff --git a/ee/changelogs/unreleased/4925-use-send-email-setting.yml b/ee/changelogs/unreleased/4925-use-send-email-setting.yml
new file mode 100644
index 00000000000000..8d7c04ba769cae
--- /dev/null
+++ b/ee/changelogs/unreleased/4925-use-send-email-setting.yml
@@ -0,0 +1,6 @@
+---
+title: Add an incident management settings form and create issues from alertmanager
+  alerts
+merge_request: 9773
+author:
+type: added
-- 
GitLab