Merge branch '273573-remove-multiple_http_integrations_custom_mapping-ff' into 'master'

Remove `multiple_http_integrations_custom_mapping` feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!56021

app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue

@@ -16,7 +16,6 @@ import {
 import * as Sentry from '@sentry/browser';
 import { isEmpty, omit } from 'lodash';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import {
   integrationTypes,
   integrationSteps,
@@ -60,7 +59,6 @@ export default {
   directives: {
     GlModal: GlModalDirective,
   },
-  mixins: [glFeatureFlagsMixin()],
   inject: {
     generic: {
       default: {},
@@ -163,12 +161,7 @@ export default {
       };
     },
     showMappingBuilder() {
-      return (
-        this.multiIntegrations &&
-        this.glFeatures.multipleHttpIntegrationsCustomMapping &&
-        this.isHttp &&
-        this.alertFields?.length
-      );
+      return this.multiIntegrations && this.isHttp && this.alertFields?.length;
     },
     hasSamplePayload() {
       return this.isValidNonEmptyJSON(this.currentIntegration?.payloadExample);
@@ -234,12 +227,10 @@ export default {
     },
     submit() {
       const { name, apiUrl } = this.integrationForm;
-      const customMappingVariables = this.glFeatures.multipleHttpIntegrationsCustomMapping
-        ? {
-            payloadAttributeMappings: this.mapping,
-            payloadExample: this.samplePayload.json || '{}',
-          }
-        : {};
+      const customMappingVariables = {
+        payloadAttributeMappings: this.mapping,
+        payloadExample: this.samplePayload.json || '{}',
+      };
 
       const variables =
         this.selectedIntegration === typeSet.http

app/assets/javascripts/alerts_settings/index.js

@@ -63,10 +63,7 @@ export default (el) => {
     render(createElement) {
       return createElement('alert-settings-wrapper', {
         props: {
-          alertFields:
-            gon.features?.multipleHttpIntegrationsCustomMapping && parseBoolean(multiIntegrations)
-              ? JSON.parse(alertFields)
-              : null,
+          alertFields: parseBoolean(multiIntegrations) ? JSON.parse(alertFields) : null,
         },
       });
     },

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(:multiple_http_integrations_custom_mapping, @project)
-      end
-
       respond_to :json, only: [:reset_alerting_token, :reset_pagerduty_token]
 
       helper_method :error_tracking_setting

config/feature_flags/development/multiple_http_integrations_custom_mapping.yml

----
-name: multiple_http_integrations_custom_mapping
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46437
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/273573
-milestone: '13.6'
-type: development
-group: group::monitor
-default_enabled: false

ee/changelogs/unreleased/273573-remove-multiple_http_integrations_custom_mapping-ff.yml

+---
+title: Custom mapping for HTTP endpoints
+merge_request: 56021
+author:
+type: added

ee/lib/gitlab/alert_management.rb

@@ -3,8 +3,7 @@
 module Gitlab
   module AlertManagement
     def self.custom_mapping_available?(project)
-      ::Feature.enabled?(:multiple_http_integrations_custom_mapping, project) &&
-        project.feature_available?(:multiple_alert_http_integrations)
+      project.feature_available?(:multiple_alert_http_integrations)
     end
 
     # Returns the complete list of alert fields for the custom mapping to be consumed by the frontend and the GraphQL API.

ee/spec/helpers/ee/operations_helper_spec.rb

@@ -85,28 +85,14 @@ RSpec.describe OperationsHelper, :routing do
 
         it { is_expected.to include('multi_integrations' => 'true') }
 
-        context 'with multiple_http_integrations_custom_mapping feature flag enabled' do
-          before do
-            stub_feature_flags(multiple_http_integrations_custom_mapping: true)
-          end
-
-          it 'has the correct list of fields', :aggregate_failures do
-            fields = Gitlab::Json.parse(alerts_settings_data['alert_fields'])
-
-            expect(fields.count).to eq(10)
-            expect(fields.first.keys).to eq(%w[name label types])
-            expect(fields.map { |f| f['name'] }).to match_array(
-              %w[title description start_time end_time service monitoring_tool hosts severity fingerprint gitlab_environment_name]
-            )
-          end
-        end
-
-        context 'with multiple_http_integrations_custom_mapping feature flag disabled' do
-          before do
-            stub_feature_flags(multiple_http_integrations_custom_mapping: false)
-          end
+        it 'has the correct list of fields', :aggregate_failures do
+          fields = Gitlab::Json.parse(alerts_settings_data['alert_fields'])
 
-          it { is_expected.not_to have_key('alert_fields') }
+          expect(fields.count).to eq(10)
+          expect(fields.first.keys).to eq(%w[name label types])
+          expect(fields.map { |f| f['name'] }).to match_array(
+            %w[title description start_time end_time service monitoring_tool hosts severity fingerprint gitlab_environment_name]
+          )
         end
       end
 

ee/spec/requests/api/graphql/mutations/alert_management/http_integration/create_spec.rb

@@ -71,7 +71,6 @@ RSpec.describe 'Creating a new HTTP Integration' do
     project.add_maintainer(current_user)
 
     stub_licensed_features(multiple_alert_http_integrations: true)
-    stub_feature_flags(multiple_http_integrations_custom_mapping: project)
   end
 
   it_behaves_like 'creating a new HTTP integration'
@@ -108,14 +107,6 @@ RSpec.describe 'Creating a new HTTP Integration' do
     it_behaves_like 'ignoring the custom mapping'
   end
 
-  context 'with multiple_http_integrations_custom_mapping feature flag disabled' do
-    before do
-      stub_feature_flags(multiple_http_integrations_custom_mapping: false)
-    end
-
-    it_behaves_like 'ignoring the custom mapping'
-  end
-
   it_behaves_like 'validating the payload_example'
   it_behaves_like 'validating the payload_attribute_mappings'
 end

ee/spec/requests/api/graphql/mutations/alert_management/http_integration/update_spec.rb

@@ -61,7 +61,6 @@ RSpec.describe 'Updating an existing HTTP Integration' do
     project.add_maintainer(current_user)
 
     stub_licensed_features(multiple_alert_http_integrations: true)
-    stub_feature_flags(multiple_http_integrations_custom_mapping: project)
   end
 
   it_behaves_like 'updating an existing HTTP integration'
@@ -75,12 +74,4 @@ RSpec.describe 'Updating an existing HTTP Integration' do
 
     it_behaves_like 'ignoring the custom mapping'
   end
-
-  context 'with multiple_http_integrations_custom_mapping feature flag disabled' do
-    before do
-      stub_feature_flags(multiple_http_integrations_custom_mapping: false)
-    end
-
-    it_behaves_like 'ignoring the custom mapping'
-  end
 end

ee/spec/requests/api/graphql/project/alert_management/http_integrations_spec.rb

@@ -57,7 +57,6 @@ RSpec.describe 'getting Alert Management HTTP Integrations' do
 
   before do
     stub_licensed_features(multiple_alert_http_integrations: true)
-    stub_feature_flags(multiple_http_integrations_custom_mapping: project)
   end
 
   before_all do

ee/spec/requests/api/graphql/project/alert_management/payload_fields_spec.rb

@@ -47,7 +47,6 @@ RSpec.describe 'parse alert payload fields' do
 
   before do
     stub_licensed_features(multiple_alert_http_integrations: license)
-    stub_feature_flags(multiple_http_integrations_custom_mapping: feature_flag)
 
     post_graphql(query, current_user: current_user)
   end
@@ -94,10 +93,4 @@ RSpec.describe 'parse alert payload fields' do
 
     it_behaves_like 'query with error', 'Failed to parse payload'
   end
-
-  context 'without feature flag' do
-    let(:feature_flag) { false }
-
-    it_behaves_like 'query with error', 'Feature not available'
-  end
 end

ee/spec/services/alert_management/extract_alert_payload_fields_service_spec.rb

@@ -22,76 +22,59 @@ RSpec.describe AlertManagement::ExtractAlertPayloadFieldsService do
       stub_licensed_features(multiple_alert_http_integrations: true)
     end
 
-    context 'with feature flag enabled' do
+    context 'with permissions' do
       before do
-        stub_feature_flags(multiple_http_integrations_custom_mapping: project)
+        project.add_maintainer(user_with_permissions)
       end
 
-      context 'with permissions' do
-        before do
-          project.add_maintainer(user_with_permissions)
-        end
-
-        context 'when payload is valid JSON' do
-          context 'when payload has an acceptable size' do
-            it 'responds with success' do
-              is_expected.to be_success
-            end
-
-            it 'returns parsed fields' do
-              fields = response.payload[:payload_alert_fields]
-              field = fields.first
-
-              expect(fields.count).to eq(1)
-              expect(field.label).to eq('Foo')
-              expect(field.type).to eq('string')
-              expect(field.path).to eq(%w[foo])
-            end
+      context 'when payload is valid JSON' do
+        context 'when payload has an acceptable size' do
+          it 'responds with success' do
+            is_expected.to be_success
           end
 
-          context 'when limits are exceeded' do
-            before do
-              allow(Gitlab::Utils::DeepSize)
-                .to receive(:new)
-                .with(Gitlab::Json.parse(payload_json))
-                .and_return(double(valid?: false))
-            end
+          it 'returns parsed fields' do
+            fields = response.payload[:payload_alert_fields]
+            field = fields.first
 
-            it 'returns payload size exceeded error' do
-              is_expected.to be_error
-              expect(response.message).to eq('Payload size exceeded')
-            end
+            expect(fields.count).to eq(1)
+            expect(field.label).to eq('Foo')
+            expect(field.type).to eq('string')
+            expect(field.path).to eq(%w[foo])
           end
         end
 
-        context 'when payload is not a valid JSON' do
-          let(:payload) { 'not a JSON' }
+        context 'when limits are exceeded' do
+          before do
+            allow(Gitlab::Utils::DeepSize)
+                .to receive(:new)
+                        .with(Gitlab::Json.parse(payload_json))
+                        .and_return(double(valid?: false))
+          end
 
-          it 'returns payload parse failure error' do
+          it 'returns payload size exceeded error' do
             is_expected.to be_error
-            expect(response.message).to eq('Failed to parse payload')
+            expect(response.message).to eq('Payload size exceeded')
           end
         end
       end
 
-      context 'without permissions' do
-        let_it_be(:user) { user_without_permissions }
+      context 'when payload is not a valid JSON' do
+        let(:payload) { 'not a JSON' }
 
-        it 'returns insufficient permissions error' do
+        it 'returns payload parse failure error' do
           is_expected.to be_error
-          expect(response.message).to eq('Insufficient permissions')
+          expect(response.message).to eq('Failed to parse payload')
         end
       end
     end
 
-    context 'with feature flag disabled' do
-      before do
-        stub_feature_flags(multiple_http_integrations_custom_mapping: false)
-      end
+    context 'without permissions' do
+      let_it_be(:user) { user_without_permissions }
 
-      it 'returns feature not available error' do
+      it 'returns insufficient permissions error' do
         is_expected.to be_error
-        expect(response.message).to eq('Feature not available')
+        expect(response.message).to eq('Insufficient permissions')
       end
     end
   end

ee/spec/services/ee/alert_management/http_integrations/create_service_spec.rb

@@ -8,23 +8,23 @@ RSpec.describe AlertManagement::HttpIntegrations::CreateService do
 
   let(:payload_example) do
     {
-      'alert' => { 'name' => 'Test alert' },
-      'started_at' => Time.current.strftime('%d %B %Y, %-l:%M%p (%Z)')
+        'alert' => { 'name' => 'Test alert' },
+        'started_at' => Time.current.strftime('%d %B %Y, %-l:%M%p (%Z)')
     }
   end
 
   let(:payload_attribute_mapping) do
     {
-      'title' => { 'path' => %w[alert name], 'type' => 'string' },
-      'start_time' => { 'path' => %w[started_at], 'type' => 'datetime' }
+        'title' => { 'path' => %w[alert name], 'type' => 'string' },
+        'start_time' => { 'path' => %w[started_at], 'type' => 'datetime' }
     }
   end
 
   let(:params) do
     {
-      name: 'New HTTP Integration',
-      payload_example: payload_example,
-      payload_attribute_mapping: payload_attribute_mapping
+        name: 'New HTTP Integration',
+        payload_example: payload_example,
+        payload_attribute_mapping: payload_attribute_mapping
     }
   end
 
@@ -68,28 +68,14 @@ RSpec.describe AlertManagement::HttpIntegrations::CreateService do
         end
       end
 
-      context 'with multiple_http_integrations_custom_mapping feature flag enabled' do
-        before do
-          stub_feature_flags(multiple_http_integrations_custom_mapping: project)
-        end
-
-        it 'successfully creates a new integration with the custom mappings' do
-          expect(response).to be_success
-
-          integration = response.payload[:integration]
-          expect(integration).to be_a(::AlertManagement::HttpIntegration)
-          expect(integration.name).to eq('New HTTP Integration')
-          expect(integration.payload_example).to eq(payload_example)
-          expect(integration.payload_attribute_mapping).to eq(payload_attribute_mapping)
-        end
-      end
-
-      context 'with multiple_http_integrations_custom_mapping feature flag disabled' do
-        before do
-          stub_feature_flags(multiple_http_integrations_custom_mapping: false)
-        end
+      it 'successfully creates a new integration with the custom mappings' do
+        expect(response).to be_success
 
-        it_behaves_like 'ignoring the custom mapping'
+        integration = response.payload[:integration]
+        expect(integration).to be_a(::AlertManagement::HttpIntegration)
+        expect(integration.name).to eq('New HTTP Integration')
+        expect(integration.payload_example).to eq(payload_example)
+        expect(integration.payload_attribute_mapping).to eq(payload_attribute_mapping)
       end
     end
   end

ee/spec/services/ee/alert_management/http_integrations/update_service_spec.rb

@@ -9,23 +9,23 @@ RSpec.describe AlertManagement::HttpIntegrations::UpdateService do
 
   let(:payload_example) do
     {
-      'alert' => { 'name' => 'Test alert' },
-      'started_at' => Time.current.strftime('%d %B %Y, %-l:%M%p (%Z)')
+        'alert' => { 'name' => 'Test alert' },
+        'started_at' => Time.current.strftime('%d %B %Y, %-l:%M%p (%Z)')
     }
   end
 
   let(:payload_attribute_mapping) do
     {
-      'title' => { 'path' => %w[alert name], 'type' => 'string' },
-      'start_time' => { 'path' => %w[started_at], 'type' => 'datetime' }
+        'title' => { 'path' => %w[alert name], 'type' => 'string' },
+        'start_time' => { 'path' => %w[started_at], 'type' => 'datetime' }
     }
   end
 
   let(:params) do
     {
-      name: 'New name',
-      payload_example: payload_example,
-      payload_attribute_mapping: payload_attribute_mapping
+        name: 'New name',
+        payload_example: payload_example,
+        payload_attribute_mapping: payload_attribute_mapping
     }
   end
 
@@ -36,17 +36,6 @@ RSpec.describe AlertManagement::HttpIntegrations::UpdateService do
   end
 
   describe '#execute' do
-    shared_examples 'ignoring the custom mapping' do
-      it 'creates integration without the custom mapping params' do
-        expect(response).to be_success
-
-        integration = response.payload[:integration]
-        expect(integration).to be_a(::AlertManagement::HttpIntegration)
-        expect(integration.payload_example).to eq({})
-        expect(integration.payload_attribute_mapping).to eq({})
-      end
-    end
-
     subject(:response) { service.execute }
 
     context 'with multiple HTTP integrations feature available' do
@@ -54,31 +43,15 @@ RSpec.describe AlertManagement::HttpIntegrations::UpdateService do
         stub_licensed_features(multiple_alert_http_integrations: true)
       end
 
-      context 'with multiple_http_integrations_custom_mapping feature flag enabled' do
-        before do
-          stub_feature_flags(multiple_http_integrations_custom_mapping: project)
-        end
-
-        it 'successfully creates a new integration with the custom mappings' do
-          expect(response).to be_success
-
-          integration = response.payload[:integration]
-          expect(integration).to be_a(::AlertManagement::HttpIntegration)
-          expect(integration.name).to eq('New name')
-          expect(integration.payload_example).to eq(payload_example)
-          expect(integration.payload_attribute_mapping).to eq(payload_attribute_mapping)
-        end
-      end
-
-      context 'with multiple_http_integrations_custom_mapping feature flag disabled' do
-        before do
-          stub_feature_flags(multiple_http_integrations_custom_mapping: false)
-        end
+      it 'successfully creates a new integration with the custom mappings' do
+        expect(response).to be_success
 
-        it_behaves_like 'ignoring the custom mapping'
+        integration = response.payload[:integration]
+        expect(integration).to be_a(::AlertManagement::HttpIntegration)
+        expect(integration.name).to eq('New name')
+        expect(integration.payload_example).to eq(payload_example)
+        expect(integration.payload_attribute_mapping).to eq(payload_attribute_mapping)
       end
     end
-
-    it_behaves_like 'ignoring the custom mapping'
   end
 end

spec/features/alerts_settings/user_views_alerts_settings_spec.rb

@@ -19,7 +19,6 @@ RSpec.describe 'Alert integrations settings form', :js do
   describe 'when viewing alert integrations as a maintainer' do
     context 'with the default page permissions' do
       before do
-        stub_feature_flags(multiple_http_integrations_custom_mapping: false)
         visit project_settings_operations_path(project, anchor: 'js-alert-management-settings')
         wait_for_requests
       end

spec/frontend/alerts_settings/components/alerts_settings_form_spec.js

@@ -12,12 +12,7 @@ describe('AlertsSettingsForm', () => {
   let wrapper;
   const mockToastShow = jest.fn();
 
-  const createComponent = ({
-    data = {},
-    props = {},
-    multipleHttpIntegrationsCustomMapping = false,
-    multiIntegrations = true,
-  } = {}) => {
+  const createComponent = ({ data = {}, props = {}, multiIntegrations = true } = {}) => {
     wrapper = mount(AlertsSettingsForm, {
       data() {
         return { ...data };
@@ -29,7 +24,6 @@ describe('AlertsSettingsForm', () => {
       },
       provide: {
         ...defaultAlertSettingsConfig,
-        glFeatures: { multipleHttpIntegrationsCustomMapping },
         multiIntegrations,
       },
       mocks: {
@@ -142,27 +136,8 @@ describe('AlertsSettingsForm', () => {
 
   describe('submitting integration form', () => {
     describe('HTTP', () => {
-      it('create', async () => {
-        createComponent();
-
-        const integrationName = 'Test integration';
-        await selectOptionAtIndex(1);
-        enableIntegration(0, integrationName);
-
-        const submitBtn = findSubmitButton();
-        expect(submitBtn.exists()).toBe(true);
-        expect(submitBtn.text()).toBe('Save integration');
-
-        findForm().trigger('submit');
-
-        expect(wrapper.emitted('create-new-integration')[0]).toEqual([
-          { type: typeSet.http, variables: { name: integrationName, active: true } },
-        ]);
-      });
-
       it('create with custom mapping', async () => {
         createComponent({
-          multipleHttpIntegrationsCustomMapping: true,
           multiIntegrations: true,
           props: { alertFields },
         });
@@ -208,9 +183,19 @@ describe('AlertsSettingsForm', () => {
 
         findForm().trigger('submit');
 
-        expect(wrapper.emitted('update-integration')[0]).toEqual([
-          { type: typeSet.http, variables: { name: updatedIntegrationName, active: true } },
-        ]);
+        expect(wrapper.emitted('update-integration')[0]).toEqual(
+          expect.arrayContaining([
+            {
+              type: typeSet.http,
+              variables: {
+                name: updatedIntegrationName,
+                active: true,
+                payloadAttributeMappings: [],
+                payloadExample: '{}',
+              },
+            },
+          ]),
+        );
       });
     });
 
@@ -301,7 +286,6 @@ describe('AlertsSettingsForm', () => {
 
     beforeEach(() => {
       createComponent({
-        multipleHttpIntegrationsCustomMapping: true,
         data: {
           currentIntegration: {
             type: typeSet.http,
@@ -408,22 +392,18 @@ describe('AlertsSettingsForm', () => {
 
   describe('Mapping builder section', () => {
     describe.each`
-      alertFieldsProvided | multiIntegrations | featureFlag | integrationOption | visible
-      ${true}             | ${true}           | ${true}     | ${1}              | ${true}
-      ${true}             | ${true}           | ${true}     | ${2}              | ${false}
-      ${true}             | ${true}           | ${false}    | ${1}              | ${false}
-      ${true}             | ${true}           | ${false}    | ${2}              | ${false}
-      ${true}             | ${false}          | ${true}     | ${1}              | ${false}
-      ${false}            | ${true}           | ${true}     | ${1}              | ${false}
-    `('', ({ alertFieldsProvided, multiIntegrations, featureFlag, integrationOption, visible }) => {
+      alertFieldsProvided | multiIntegrations | integrationOption | visible
+      ${true}             | ${true}           | ${1}              | ${true}
+      ${true}             | ${true}           | ${2}              | ${false}
+      ${true}             | ${false}          | ${1}              | ${false}
+      ${false}            | ${true}           | ${1}              | ${false}
+    `('', ({ alertFieldsProvided, multiIntegrations, integrationOption, visible }) => {
       const visibleMsg = visible ? 'is rendered' : 'is not rendered';
-      const featureFlagMsg = featureFlag ? 'is enabled' : 'is disabled';
       const alertFieldsMsg = alertFieldsProvided ? 'are provided' : 'are not provided';
       const integrationType = integrationOption === 1 ? typeSet.http : typeSet.prometheus;
 
-      it(`${visibleMsg} when multipleHttpIntegrationsCustomMapping feature flag ${featureFlagMsg} and integration type is ${integrationType} and alert fields ${alertFieldsMsg}`, async () => {
+      it(`${visibleMsg} when integration type is ${integrationType} and alert fields ${alertFieldsMsg}`, async () => {
         createComponent({
-          multipleHttpIntegrationsCustomMapping: featureFlag,
           multiIntegrations,
           props: {
             alertFields: alertFieldsProvided ? alertFields : [],