Merge branch '200011-move-error-tracking-incidents-to-core' into 'master'

Move Settings->Operations->Incidents to CE

Closes #200011

See merge request gitlab-org/gitlab!24600

app/controllers/projects/settings/operations_controller.rb

@@ -19,19 +19,36 @@ module Projects
 
       # overridden in EE
       def track_events(result)
+        if result[:status] == :success
+          ::Gitlab::Tracking::IncidentManagement.track_from_params(
+            update_params[:incident_management_setting_attributes]
+          )
+        end
       end
 
       private
 
-      # overridden in EE
       def render_update_response(result)
         respond_to do |format|
+          format.html do
+            render_update_html_response(result)
+          end
+
           format.json do
             render_update_json_response(result)
           end
         end
       end
 
+      def render_update_html_response(result)
+        if result[:status] == :success
+          flash[:notice] = _('Your changes have been saved')
+          redirect_to project_settings_operations_path(@project)
+        else
+          render 'show'
+        end
+      end
+
       def render_update_json_response(result)
         if result[:status] == :success
           flash[:notice] = _('Your changes have been saved')
@@ -61,6 +78,8 @@ module Projects
       # overridden in EE
       def permitted_project_params
         project_params = {
+          incident_management_setting_attributes: ::Gitlab::Tracking::IncidentManagement.tracking_keys.keys,
+
           metrics_setting_attributes: [:external_dashboard_url],
 
           error_tracking_setting_attributes: [

app/helpers/projects_helper.rb

@@ -3,6 +3,11 @@
 module ProjectsHelper
   prepend_if_ee('::EE::ProjectsHelper') # rubocop: disable Cop/InjectEnterpriseEditionModule
 
+  def project_incident_management_setting
+    @project_incident_management_setting ||= @project.incident_management_setting ||
+      @project.build_incident_management_setting
+  end
+
   def link_to_project(project)
     link_to namespace_project_path(namespace_id: project.namespace, id: project), title: h(project.name) do
       title = content_tag(:span, project.name, class: 'project-name')

app/services/projects/operations/update_service.rb

@@ -16,6 +16,7 @@ module Projects
           .merge(metrics_setting_params)
           .merge(grafana_integration_params)
           .merge(prometheus_integration_params)
+          .merge(incident_management_setting_params)
       end
 
       def metrics_setting_params
@@ -87,6 +88,10 @@ module Projects
 
         { prometheus_service_attributes: service.attributes.except(*%w(id project_id created_at updated_at)) }
       end
+
+      def incident_management_setting_params
+        params.slice(:incident_management_setting_attributes)
+      end
     end
   end
 end

ee/app/views/projects/settings/operations/_incidents.html.haml → app/views/projects/settings/operations/_incidents.html.haml

-- return unless incident_management_available?
-
 - templates = []
 - setting = project_incident_management_setting
 - templates = setting.available_issue_templates.map { |t| [t.name, t.key] }
 
-%section.settings.no-animate
+%section.settings.no-animate.js-incident-management-settings
   .settings-header
     %h4= _('Incidents')
     %button.btn.js-settings-toggle{ type: 'button' }

app/views/projects/settings/operations/show.html.haml

@@ -2,7 +2,7 @@
 - page_title _('Operations Settings')
 - breadcrumb_title _('Operations Settings')
 
-= render_if_exists 'projects/settings/operations/incidents'
+= render 'projects/settings/operations/incidents'
 = render 'projects/settings/operations/error_tracking'
 = render 'projects/settings/operations/external_dashboard'
 = render 'projects/settings/operations/grafana_integration'

changelogs/unreleased/200011-move-error-tracking-incidents-to-core.yml

+---
+title: Move Settings->Operations->Incidents to the Core
+merge_request: 24600
+author:
+type: changed

ee/app/controllers/ee/projects/settings/operations_controller.rb

@@ -25,19 +25,10 @@ module EE
             end
           end
 
-          helper_method :tracing_setting, :incident_management_available?
+          helper_method :tracing_setting
 
           private
 
-          def render_update_html_response(result)
-            if result[:status] == :success
-              flash[:notice] = _('Your changes have been saved')
-              redirect_to project_settings_operations_path(@project)
-            else
-              render 'show'
-            end
-          end
-
           def alerting_params
             { alerting_setting_attributes: { regenerate_token: true } }
           end
@@ -50,10 +41,6 @@ module EE
             project.feature_available?(:tracing, current_user)
           end
 
-          def incident_management_available?
-            project.feature_available?(:incident_management, current_user)
-          end
-
           def track_tracing_external_url
             external_url_previous_change = project&.tracing_setting&.external_url_previous_change
             return unless external_url_previous_change
@@ -71,37 +58,14 @@ module EE
             permitted_params[:tracing_setting_attributes] = [:external_url]
           end
 
-          if incident_management_available?
-            permitted_params[:incident_management_setting_attributes] = ::Gitlab::Tracking::IncidentManagement.tracking_keys.keys
-          end
-
           permitted_params
         end
 
-        override :render_update_response
-        def render_update_response(result)
-          respond_to do |format|
-            format.html do
-              render_update_html_response(result)
-            end
-
-            format.json do
-              render_update_json_response(result)
-            end
-          end
-        end
-
         override :track_events
         def track_events(result)
           super
 
-          if result[:status] == :success
-            ::Gitlab::Tracking::IncidentManagement.track_from_params(
-              update_params[:incident_management_setting_attributes]
-            )
-
-            track_tracing_external_url
-          end
+          track_tracing_external_url if result[:status] == :success
         end
       end
     end

ee/app/helpers/ee/projects_helper.rb

@@ -275,11 +275,6 @@ module EE
       @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
-
     override :can_import_members?
     def can_import_members?
       super && !membership_locked?

ee/spec/controllers/projects/settings/operations_controller_spec.rb

@@ -96,14 +96,6 @@ describe Projects::Settings::OperationsController do
     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)
@@ -125,47 +117,6 @@ describe Projects::Settings::OperationsController do
 
         expect(project.tracing_setting).to be_nil
       end
-
-      it 'does not create incident_management_setting' do
-        update_project(
-          project,
-          incident_management_params: incident_management_settings
-        )
-
-        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_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])
-          )
-        end
-      end
     end
 
     context 'format html' do
@@ -226,42 +177,6 @@ describe Projects::Settings::OperationsController do
 
           expect(project.tracing_setting.external_url).to eq(tracing_url)
         end
-
-        it 'creates incident management settings' do
-          update_project(
-            project,
-            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))
-          )
-        end
-
-        it 'creates tracing and incident management settings' do
-          update_project(
-            project,
-            tracing_params: { external_url: tracing_url },
-            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))
-          )
-        end
       end
 
       context 'with maintainer role' do
@@ -343,72 +258,6 @@ describe Projects::Settings::OperationsController do
         end
       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 'updates incident management setting' do
-          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])
-          )
-        end
-      end
-
-      context 'updating each incident management setting' do
-        let(:project) { create(:project) }
-        let(:new_incident_management_settings) { {} }
-
-        before do
-          project.add_maintainer(user)
-        end
-
-        shared_examples 'a gitlab tracking event' do |params, event_key|
-          it "creates a gitlab tracking event #{event_key}" do
-            new_incident_management_settings = params
-
-            expect(Gitlab::Tracking).to receive(:event)
-              .with('IncidentManagement::Settings', event_key, kind_of(Hash))
-
-            update_project(project,
-              incident_management_params: new_incident_management_settings)
-          end
-        end
-
-        it_behaves_like 'a gitlab tracking event', { create_issue: '1' }, 'enabled_issue_auto_creation_on_alerts'
-        it_behaves_like 'a gitlab tracking event', { create_issue: '0' }, 'disabled_issue_auto_creation_on_alerts'
-        it_behaves_like 'a gitlab tracking event', { issue_template_key: 'template' }, 'enabled_issue_template_on_alerts'
-        it_behaves_like 'a gitlab tracking event', { issue_template_key: nil }, 'disabled_issue_template_on_alerts'
-        it_behaves_like 'a gitlab tracking event', { send_email: '1' }, 'enabled_sending_emails'
-        it_behaves_like 'a gitlab tracking event', { send_email: '0' }, 'disabled_sending_emails'
-      end
-
       context 'updating tracing settings' do
         let(:project) { create(:project) }
         let(:new_tracing_settings) { {} }

ee/spec/features/projects/settings/operations_settings_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe 'Projects > Settings' do
-  let(:user) { create(:user) }
-  let(:project) { create(:project, :repository, create_templates: :issue) }
-  let(:role) { :maintainer }
-  let(:create_issue) { 'Create an issue. Issues are created for each alert triggered.' }
-  let(:send_email) { 'Send a separate email notification to Developers.' }
-
-  before do
-    create(:project_incident_management_setting, send_email: true, project: project)
-    sign_in(user)
-    project.add_role(user, role)
-  end
-
-  describe 'Incidents' do
-    context 'with license' do
-      before do
-        stub_licensed_features(incident_management: true)
-        visit project_settings_operations_path(project)
-      end
-
-      it 'renders form for incident management' do
-        expect(page).to have_selector('h4', text: 'Incidents')
-      end
-
-      it 'sets correct default values' do
-        expect(find_field(create_issue)).not_to be_checked
-        expect(find_field(send_email)).to be_checked
-      end
-
-      it 'updates form values' do
-        check(create_issue)
-        template_select = find_field('Issue template')
-        template_select.find(:xpath, 'option[2]').select_option
-        uncheck(send_email)
-        save_form
-
-        expect(find_field(create_issue)).to be_checked
-        expect(page).to have_select('Issue template', selected: 'bug')
-        expect(find_field(send_email)).not_to be_checked
-      end
-
-      def save_form
-        page.within "#edit_project_#{project.id}" do
-          click_on 'Save changes'
-        end
-      end
-    end
-
-    context 'without license' do
-      before do
-        stub_licensed_features(incident_management: false)
-        visit project_settings_operations_path(project)
-      end
-
-      it 'renders form for incident management' do
-        expect(page).not_to have_selector('h4', text: 'Incidents')
-      end
-    end
-  end
-end

ee/spec/helpers/projects_helper_spec.rb

@@ -9,30 +9,6 @@ describe ProjectsHelper do
     helper.instance_variable_set(:@project, project)
   end
 
-  describe '#project_incident_management_setting' do
-    context 'when incident_management_setting exists' do
-      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)
-        )
-      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(false)
-        expect(helper.project_incident_management_setting.create_issue).to be(true)
-        expect(helper.project_incident_management_setting.issue_template_key).to be(nil)
-      end
-    end
-  end
-
   describe 'default_clone_protocol' do
     context 'when gitlab.config.kerberos is enabled and user is logged in' do
       it 'returns krb5 as default protocol' do

spec/controllers/projects/settings/operations_controller_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 describe Projects::Settings::OperationsController do
   let_it_be(:user) { create(:user) }
-  let_it_be(:project) { create(:project) }
+  let_it_be(:project, reload: true) { create(:project) }
 
   before do
     sign_in(user)
@@ -121,6 +121,74 @@ describe Projects::Settings::OperationsController do
     end
   end
 
+  context 'incident management' do
+    describe 'GET #show' do
+      context 'with existing setting' do
+        let!(:incident_management_setting) do
+          create(:project_incident_management_setting, project: project)
+        end
+
+        it 'loads existing setting' do
+          get :show, params: project_params(project)
+
+          expect(controller.helpers.project_incident_management_setting)
+            .to eq(incident_management_setting)
+        end
+      end
+
+      context 'without an existing setting' do
+        it 'builds a new setting' do
+          get :show, params: project_params(project)
+
+          expect(controller.helpers.project_incident_management_setting).to be_new_record
+        end
+      end
+    end
+
+    describe 'PATCH #update' do
+      let(:params) do
+        {
+          incident_management_setting_attributes: {
+            create_issue: 'false',
+            send_email: 'false',
+            issue_template_key: 'some-other-template'
+          }
+        }
+      end
+
+      it_behaves_like 'PATCHable'
+
+      context 'updating each incident management setting' do
+        let(:project) { create(:project) }
+        let(:new_incident_management_settings) { {} }
+
+        before do
+          project.add_maintainer(user)
+        end
+
+        shared_examples 'a gitlab tracking event' do |params, event_key|
+          it "creates a gitlab tracking event #{event_key}" do
+            new_incident_management_settings = params
+
+            expect(Gitlab::Tracking).to receive(:event)
+              .with('IncidentManagement::Settings', event_key, kind_of(Hash))
+
+            patch :update, params: project_params(project, incident_management_setting_attributes: new_incident_management_settings)
+
+            project.reload
+          end
+        end
+
+        it_behaves_like 'a gitlab tracking event', { create_issue: '1' }, 'enabled_issue_auto_creation_on_alerts'
+        it_behaves_like 'a gitlab tracking event', { create_issue: '0' }, 'disabled_issue_auto_creation_on_alerts'
+        it_behaves_like 'a gitlab tracking event', { issue_template_key: 'template' }, 'enabled_issue_template_on_alerts'
+        it_behaves_like 'a gitlab tracking event', { issue_template_key: nil }, 'disabled_issue_template_on_alerts'
+        it_behaves_like 'a gitlab tracking event', { send_email: '1' }, 'enabled_sending_emails'
+        it_behaves_like 'a gitlab tracking event', { send_email: '0' }, 'disabled_sending_emails'
+      end
+    end
+  end
+
   context 'error tracking' do
     describe 'GET #show' do
       context 'with existing setting' do

spec/features/projects/settings/operations_settings_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 describe 'Projects > Settings > For a forked project', :js do
   let(:user) { create(:user) }
-  let(:project) { create(:project, :repository) }
+  let(:project) { create(:project, :repository, create_templates: :issue) }
   let(:role) { :maintainer }
 
   before do
@@ -22,6 +22,54 @@ describe 'Projects > Settings > For a forked project', :js do
   end
 
   describe 'Settings > Operations' do
+    describe 'Incidents' do
+      let(:create_issue) { 'Create an issue. Issues are created for each alert triggered.' }
+      let(:send_email) { 'Send a separate email notification to Developers.' }
+
+      before do
+        create(:project_incident_management_setting, send_email: true, project: project)
+        visit project_settings_operations_path(project)
+
+        wait_for_requests
+        click_expand_incident_management_button
+      end
+
+      it 'renders form for incident management' do
+        expect(page).to have_selector('h4', text: 'Incidents')
+      end
+
+      it 'sets correct default values' do
+        expect(find_field(create_issue)).not_to be_checked
+        expect(find_field(send_email)).to be_checked
+      end
+
+      it 'updates form values' do
+        check(create_issue)
+        template_select = find_field('Issue template')
+        template_select.find(:xpath, 'option[2]').select_option
+        uncheck(send_email)
+
+        save_form
+        click_expand_incident_management_button
+
+        expect(find_field(create_issue)).to be_checked
+        expect(page).to have_select('Issue template', selected: 'bug')
+        expect(find_field(send_email)).not_to be_checked
+      end
+
+      def click_expand_incident_management_button
+        within '.js-incident-management-settings' do
+          click_button('Expand')
+        end
+      end
+
+      def save_form
+        page.within "#edit_project_#{project.id}" do
+          click_on 'Save changes'
+        end
+      end
+    end
+
     context 'error tracking settings form' do
       let(:sentry_list_projects_url) { 'http://sentry.example.com/api/0/projects/' }
 

spec/helpers/projects_helper_spec.rb

@@ -5,6 +5,37 @@ require 'spec_helper'
 describe ProjectsHelper do
   include ProjectForksHelper
 
+  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) 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)
+        )
+      end
+    end
+
+    context 'when incident_management_setting does not exist' do
+      it 'builds incident_management_setting' do
+        setting = helper.project_incident_management_setting
+
+        expect(setting).not_to be_persisted
+        expect(setting.send_email).to be_falsey
+        expect(setting.create_issue).to be_truthy
+        expect(setting.issue_template_key).to be_nil
+      end
+    end
+  end
+
   describe '#error_tracking_setting_project_json' do
     let(:project) { create(:project) }