Merge branch '229158-remove-pagerduty_webhook-feature-flag' into 'master'

Remove pagerduty_webhook feature flag

See merge request gitlab-org/gitlab!37193

app/assets/javascripts/incidents_settings/components/incidents_settings_tabs.vue

@@ -2,7 +2,6 @@
 import { GlButton, GlTabs, GlTab } from '@gitlab/ui';
 import AlertsSettingsForm from './alerts_form.vue';
 import PagerDutySettingsForm from './pagerduty_form.vue';
-import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { INTEGRATION_TABS_CONFIG, I18N_INTEGRATION_TABS } from '../constants';
 
 export default {
@@ -13,17 +12,8 @@ export default {
     AlertsSettingsForm,
     PagerDutySettingsForm,
   },
-  mixins: [glFeatureFlagMixin()],
   tabs: INTEGRATION_TABS_CONFIG,
   i18n: I18N_INTEGRATION_TABS,
-  methods: {
-    isFeatureFlagEnabled(tab) {
-      if (tab.featureFlag) {
-        return this.glFeatures[tab.featureFlag];
-      }
-      return true;
-    },
-  },
 };
 </script>
 
@@ -49,7 +39,7 @@ export default {
       <gl-tabs>
         <gl-tab
           v-for="(tab, index) in $options.tabs"
-          v-if="tab.active && isFeatureFlagEnabled(tab)"
+          v-if="tab.active"
           :key="`${tab.title}_${index}`"
           :title="tab.title"
         >

app/assets/javascripts/incidents_settings/constants.js

@@ -11,7 +11,6 @@ export const INTEGRATION_TABS_CONFIG = [
     title: s__('IncidentSettings|PagerDuty integration'),
     component: 'PagerDutySettingsForm',
     active: true,
-    featureFlag: 'pagerdutyWebhook',
   },
   {
     title: s__('IncidentSettings|Grafana integration'),

app/controllers/projects/settings/operations_controller.rb

@@ -6,10 +6,6 @@ module Projects
       before_action :authorize_admin_operations!
       before_action :authorize_read_prometheus_alerts!, only: [:reset_alerting_token]
 
-      before_action do
-        push_frontend_feature_flag(:pagerduty_webhook, project)
-      end
-
       respond_to :json, only: [:reset_alerting_token, :reset_pagerduty_token]
 
       helper_method :error_tracking_setting

app/services/incident_management/pager_duty/create_incident_issue_service.rb

@@ -40,8 +40,7 @@ module IncidentManagement
       end
 
       def webhook_available?
-        Feature.enabled?(:pagerduty_webhook, project) &&
-          incident_management_setting.pagerduty_active?
+        incident_management_setting.pagerduty_active?
       end
 
       def forbidden

app/services/incident_management/pager_duty/process_webhook_service.rb

@@ -39,8 +39,7 @@ module IncidentManagement
       end
 
       def webhook_setting_active?
-        Feature.enabled?(:pagerduty_webhook, project) &&
-          incident_management_setting.pagerduty_active?
+        incident_management_setting.pagerduty_active?
       end
 
       def valid_token?(token)

changelogs/unreleased/229158-remove-pagerduty_webhook-feature-flag.yml

+---
+title: Add PagerDuty incident integration.
+merge_request: 37193
+author:
+type: added

spec/frontend/incidents_settings/components/incidents_settings_tabs_spec.js

@@ -6,9 +6,7 @@ describe('IncidentsSettingTabs', () => {
   let wrapper;
 
   beforeEach(() => {
-    wrapper = shallowMount(IncidentsSettingTabs, {
-      provide: { glFeatures: { pagerdutyWebhook: true } },
-    });
+    wrapper = shallowMount(IncidentsSettingTabs);
   });
 
   afterEach(() => {

spec/services/incident_management/pager_duty/create_incident_issue_service_spec.rb

@@ -12,84 +12,63 @@ RSpec.describe IncidentManagement::PagerDuty::CreateIncidentIssueService do
   subject(:execute) { described_class.new(project, incident_payload).execute }
 
   describe '#execute' do
-    context 'when pagerduty_webhook feature enabled' do
-      before do
-        stub_feature_flags(pagerduty_webhook: project)
-      end
+    context 'when PagerDuty webhook setting is active' do
+      let_it_be(:incident_management_setting) { create(:project_incident_management_setting, project: project, pagerduty_active: true) }
 
-      context 'when PagerDuty webhook setting is active' do
-        let_it_be(:incident_management_setting) { create(:project_incident_management_setting, project: project, pagerduty_active: true) }
-
-        context 'when issue can be created' do
-          it 'creates a new issue' do
-            expect { execute }.to change(Issue, :count).by(1)
-          end
-
-          it 'responds with success' do
-            response = execute
-
-            expect(response).to be_success
-            expect(response.payload[:issue]).to be_kind_of(Issue)
-          end
-
-          it 'the issue author is Alert bot' do
-            expect(execute.payload[:issue].author).to eq(User.alert_bot)
-          end
-
-          it 'issue has a correct title' do
-            expect(execute.payload[:issue].title).to eq(incident_payload['title'])
-          end
-
-          it 'issue has a correct description' do
-            markdown_line_break = '  '
-
-            expect(execute.payload[:issue].description).to eq(
-              <<~MARKDOWN.chomp
-                **Incident:** [My new incident](https://webdemo.pagerduty.com/incidents/PRORDTY)#{markdown_line_break}
-                **Incident number:** 33#{markdown_line_break}
-                **Urgency:** high#{markdown_line_break}
-                **Status:** triggered#{markdown_line_break}
-                **Incident key:** #{markdown_line_break}
-                **Created at:** 26 September 2017, 3:14PM (UTC)#{markdown_line_break}
-                **Assignees:** [Laura Haley](https://webdemo.pagerduty.com/users/P553OPV)#{markdown_line_break}
-                **Impacted services:** [Production XDB Cluster](https://webdemo.pagerduty.com/services/PN49J75)
-              MARKDOWN
-            )
-          end
+      context 'when issue can be created' do
+        it 'creates a new issue' do
+          expect { execute }.to change(Issue, :count).by(1)
         end
 
-        context 'when the payload does not contain a title' do
-          let(:incident_payload) { {} }
+        it 'responds with success' do
+          response = execute
+
+          expect(response).to be_success
+          expect(response.payload[:issue]).to be_kind_of(Issue)
+        end
 
-          it 'does not create a GitLab issue' do
-            expect { execute }.not_to change(Issue, :count)
-          end
+        it 'the issue author is Alert bot' do
+          expect(execute.payload[:issue].author).to eq(User.alert_bot)
+        end
 
-          it 'responds with error' do
-            expect(execute).to be_error
-            expect(execute.message).to eq("Title can't be blank")
-          end
+        it 'issue has a correct title' do
+          expect(execute.payload[:issue].title).to eq(incident_payload['title'])
+        end
+
+        it 'issue has a correct description' do
+          markdown_line_break = '  '
+
+          expect(execute.payload[:issue].description).to eq(
+            <<~MARKDOWN.chomp
+              **Incident:** [My new incident](https://webdemo.pagerduty.com/incidents/PRORDTY)#{markdown_line_break}
+              **Incident number:** 33#{markdown_line_break}
+              **Urgency:** high#{markdown_line_break}
+              **Status:** triggered#{markdown_line_break}
+              **Incident key:** #{markdown_line_break}
+              **Created at:** 26 September 2017, 3:14PM (UTC)#{markdown_line_break}
+              **Assignees:** [Laura Haley](https://webdemo.pagerduty.com/users/P553OPV)#{markdown_line_break}
+              **Impacted services:** [Production XDB Cluster](https://webdemo.pagerduty.com/services/PN49J75)
+            MARKDOWN
+          )
         end
       end
 
-      context 'when PagerDuty webhook setting is not active' do
-        let_it_be(:incident_management_setting) { create(:project_incident_management_setting, project: project, pagerduty_active: false) }
+      context 'when the payload does not contain a title' do
+        let(:incident_payload) { {} }
 
         it 'does not create a GitLab issue' do
           expect { execute }.not_to change(Issue, :count)
         end
 
-        it 'responds with forbidden' do
+        it 'responds with error' do
           expect(execute).to be_error
-          expect(execute.http_status).to eq(:forbidden)
+          expect(execute.message).to eq("Title can't be blank")
         end
       end
     end
 
-    context 'when pagerduty_webhook feature disabled' do
-      before do
-        stub_feature_flags(pagerduty_webhook: false)
-      end
+    context 'when PagerDuty webhook setting is not active' do
+      let_it_be(:incident_management_setting) { create(:project_incident_management_setting, project: project, pagerduty_active: false) }
 
       it 'does not create a GitLab issue' do
         expect { execute }.not_to change(Issue, :count)

spec/services/incident_management/pager_duty/process_webhook_service_spec.rb

@@ -19,92 +19,68 @@ RSpec.describe IncidentManagement::PagerDuty::ProcessWebhookService do
 
     subject(:execute) { described_class.new(project, nil, webhook_payload).execute(token) }
 
-    context 'when pagerduty_webhook feature is enabled' do
-      before do
-        stub_feature_flags(pagerduty_webhook: project)
-      end
-
-      context 'when PagerDuty webhook setting is active' do
-        let_it_be(:incident_management_setting) { create(:project_incident_management_setting, project: project, pagerduty_active: true) }
-
-        context 'when token is valid' do
-          let(:token) { incident_management_setting.pagerduty_token }
+    context 'when PagerDuty webhook setting is active' do
+      let_it_be(:incident_management_setting) { create(:project_incident_management_setting, project: project, pagerduty_active: true) }
 
-          context 'when webhook payload has acceptable size' do
-            it 'responds with Accepted' do
-              result = execute
+      context 'when token is valid' do
+        let(:token) { incident_management_setting.pagerduty_token }
 
-              expect(result).to be_success
-              expect(result.http_status).to eq(:accepted)
-            end
-
-            it 'processes issues' do
-              incident_payload = ::PagerDuty::WebhookPayloadParser.call(webhook_payload).first['incident']
-
-              expect(::IncidentManagement::PagerDuty::ProcessIncidentWorker)
-                .to receive(:perform_async)
-                .with(project.id, incident_payload)
-                .once
+        context 'when webhook payload has acceptable size' do
+          it 'responds with Accepted' do
+            result = execute
 
-              execute
-            end
+            expect(result).to be_success
+            expect(result.http_status).to eq(:accepted)
           end
 
-          context 'when webhook payload is too big' do
-            let(:deep_size) { instance_double(Gitlab::Utils::DeepSize, valid?: false) }
-
-            before do
-              allow(Gitlab::Utils::DeepSize)
-                .to receive(:new)
-                .with(webhook_payload, max_size: described_class::PAGER_DUTY_PAYLOAD_SIZE_LIMIT)
-                .and_return(deep_size)
-            end
+          it 'processes issues' do
+            incident_payload = ::PagerDuty::WebhookPayloadParser.call(webhook_payload).first['incident']
 
-            it 'responds with Bad Request' do
-              result = execute
+            expect(::IncidentManagement::PagerDuty::ProcessIncidentWorker)
+              .to receive(:perform_async)
+              .with(project.id, incident_payload)
+              .once
 
-              expect(result).to be_error
-              expect(result.http_status).to eq(:bad_request)
-            end
-
-            it_behaves_like 'does not process incidents'
+            execute
           end
+        end
 
-          context 'when webhook payload is blank' do
-            let(:webhook_payload) { nil }
+        context 'when webhook payload is too big' do
+          let(:deep_size) { instance_double(Gitlab::Utils::DeepSize, valid?: false) }
 
-            it 'responds with Accepted' do
-              result = execute
+          before do
+            allow(Gitlab::Utils::DeepSize)
+              .to receive(:new)
+              .with(webhook_payload, max_size: described_class::PAGER_DUTY_PAYLOAD_SIZE_LIMIT)
+              .and_return(deep_size)
+          end
 
-              expect(result).to be_success
-              expect(result.http_status).to eq(:accepted)
-            end
+          it 'responds with Bad Request' do
+            result = execute
 
-            it_behaves_like 'does not process incidents'
+            expect(result).to be_error
+            expect(result.http_status).to eq(:bad_request)
           end
+
+          it_behaves_like 'does not process incidents'
         end
 
-        context 'when token is invalid' do
-          let(:token) { 'invalid-token' }
+        context 'when webhook payload is blank' do
+          let(:webhook_payload) { nil }
 
-          it 'responds with Unauthorized' do
+          it 'responds with Accepted' do
             result = execute
 
-            expect(result).to be_error
-            expect(result.http_status).to eq(:unauthorized)
+            expect(result).to be_success
+            expect(result.http_status).to eq(:accepted)
           end
 
           it_behaves_like 'does not process incidents'
         end
       end
 
-      context 'when both tokens are nil' do
-        let_it_be(:incident_management_setting) { create(:project_incident_management_setting, project: project, pagerduty_active: false) }
-        let(:token) { nil }
-
-        before do
-          incident_management_setting.update_column(:pagerduty_active, true)
-        end
+      context 'when token is invalid' do
+        let(:token) { 'invalid-token' }
 
         it 'responds with Unauthorized' do
           result = execute
@@ -115,25 +91,28 @@ RSpec.describe IncidentManagement::PagerDuty::ProcessWebhookService do
 
         it_behaves_like 'does not process incidents'
       end
+    end
 
-      context 'when PagerDuty webhook setting is not active' do
-        let_it_be(:incident_management_setting) { create(:project_incident_management_setting, project: project, pagerduty_active: false) }
+    context 'when both tokens are nil' do
+      let_it_be(:incident_management_setting) { create(:project_incident_management_setting, project: project, pagerduty_active: false) }
+      let(:token) { nil }
 
-        it 'responds with Forbidden' do
-          result = execute
+      before do
+        incident_management_setting.update_column(:pagerduty_active, true)
+      end
 
-          expect(result).to be_error
-          expect(result.http_status).to eq(:forbidden)
-        end
+      it 'responds with Unauthorized' do
+        result = execute
 
-        it_behaves_like 'does not process incidents'
+        expect(result).to be_error
+        expect(result.http_status).to eq(:unauthorized)
       end
+
+      it_behaves_like 'does not process incidents'
     end
 
-    context 'when pagerduty_webhook feature is disabled' do
-      before do
-        stub_feature_flags(pagerduty_webhook: false)
-      end
+    context 'when PagerDuty webhook setting is not active' do
+      let_it_be(:incident_management_setting) { create(:project_incident_management_setting, project: project, pagerduty_active: false) }
 
       it 'responds with Forbidden' do
         result = execute