Merge branch...

Merge branch '258638-feature-flag-rollout-of-enable_environment_path_in_alert_details' into 'master'

Remove FF expose_environment_path_in_alert_details

See merge request gitlab-org/gitlab!44130

app/assets/javascripts/alert_management/components/alert_details.vue

@@ -30,7 +30,6 @@ import AlertSidebar from './alert_sidebar.vue';
 import AlertMetrics from './alert_metrics.vue';
 import AlertDetailsTable from '~/vue_shared/components/alert_details_table.vue';
 import AlertSummaryRow from './alert_summary_row.vue';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 const containerEl = document.querySelector('.page-with-contextual-sidebar');
 
@@ -77,7 +76,6 @@ export default {
     SystemNote,
     AlertMetrics,
   },
-  mixins: [glFeatureFlagsMixin()],
   inject: {
     projectPath: {
       default: '',
@@ -150,13 +148,10 @@ export default {
       },
     },
     environmentName() {
-      return this.shouldDisplayEnvironment && this.alert?.environment?.name;
+      return this.alert?.environment?.name;
     },
     environmentPath() {
-      return this.shouldDisplayEnvironment && this.alert?.environment?.path;
-    },
-    shouldDisplayEnvironment() {
-      return this.glFeatures.exposeEnvironmentPathInAlertDetails;
+      return this.alert?.environment?.path;
     },
   },
   mounted() {

app/assets/javascripts/vue_shared/components/alert_details_table.vue

@@ -7,7 +7,6 @@ import {
   convertToSentenceCase,
   splitCamelCase,
 } from '~/lib/utils/text_utility';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 const thClass = 'gl-bg-transparent! gl-border-1! gl-border-b-solid! gl-border-gray-200!';
 const tdClass = 'gl-border-gray-100! gl-p-5!';
@@ -25,6 +24,7 @@ const allowedFields = [
   'endedAt',
   'details',
   'hosts',
+  'environment',
 ];
 
 export default {
@@ -32,7 +32,6 @@ export default {
     GlLoadingIcon,
     GlTable,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
     alert: {
       type: Object,
@@ -60,9 +59,6 @@ export default {
     },
   ],
   computed: {
-    flaggedAllowedFields() {
-      return this.shouldDisplayEnvironment ? [...allowedFields, 'environment'] : allowedFields;
-    },
     items() {
       if (!this.alert) {
         return [];
@@ -84,13 +80,10 @@ export default {
         [],
       );
     },
-    shouldDisplayEnvironment() {
-      return this.glFeatures.exposeEnvironmentPathInAlertDetails;
-    },
   },
   methods: {
     isAllowed(fieldName) {
-      return this.flaggedAllowedFields.includes(fieldName);
+      return allowedFields.includes(fieldName);
     },
   },
 };

app/controllers/projects/alert_management_controller.rb

@@ -10,6 +10,5 @@ class Projects::AlertManagementController < Projects::ApplicationController
 
   def details
     @alert_id = params[:id]
-    push_frontend_feature_flag(:expose_environment_path_in_alert_details, @project)
   end
 end

app/graphql/types/environment_type.rb

@@ -18,9 +18,8 @@ module Types
     field :state, GraphQL::STRING_TYPE, null: false,
           description: 'State of the environment, for example: available/stopped'
 
-    field :path, GraphQL::STRING_TYPE, null: true,
-          description: 'The path to the environment. Will always return null ' \
-                       'if `expose_environment_path_in_alert_details` feature flag is disabled'
+    field :path, GraphQL::STRING_TYPE, null: false,
+          description: 'The path to the environment.'
 
     field :metrics_dashboard, Types::Metrics::DashboardType, null: true,
           description: 'Metrics dashboard schema for the environment',

app/presenters/environment_presenter.rb

@@ -6,8 +6,6 @@ class EnvironmentPresenter < Gitlab::View::Presenter::Delegated
   presents :environment
 
   def path
-    if Feature.enabled?(:expose_environment_path_in_alert_details, project)
-      project_environment_path(project, self)
-    end
+    project_environment_path(project, self)
   end
 end

changelogs/unreleased/258638-feature-flag-rollout-of-enable_environment_path_in_alert_details.yml

+---
+title: Show the environment link on alert details page
+merge_request: 44130
+author:
+type: added

config/feature_flags/development/expose_environment_path_in_alert_details.yml

----
-name: expose_environment_path_in_alert_details
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/43414
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/258638
-type: development
-group: group::progressive delivery
-default_enabled: false

doc/api/graphql/reference/gitlab_schema.graphql

@@ -6231,10 +6231,9 @@ type Environment {
   name: String!
 
   """
-  The path to the environment. Will always return null if
-  `expose_environment_path_in_alert_details` feature flag is disabled
+  The path to the environment.
   """
-  path: String
+  path: String!
 
   """
   State of the environment, for example: available/stopped

doc/api/graphql/reference/gitlab_schema.json

@@ -17214,14 +17214,18 @@
             },
             {
               "name": "path",
-              "description": "The path to the environment. Will always return null if `expose_environment_path_in_alert_details` feature flag is disabled",
+              "description": "The path to the environment.",
               "args": [
 
               ],
               "type": {
-                "kind": "SCALAR",
-                "name": "String",
-                "ofType": null
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "String",
+                  "ofType": null
+                }
               },
               "isDeprecated": false,
               "deprecationReason": null

doc/api/graphql/reference/index.md

@@ -1020,7 +1020,7 @@ Describes where code is deployed for a project.
 | `latestOpenedMostSevereAlert` | AlertManagementAlert | The most severe open alert for the environment. If multiple alerts have equal severity, the most recent is returned |
 | `metricsDashboard` | MetricsDashboard | Metrics dashboard schema for the environment |
 | `name` | String! | Human-readable name of the environment |
-| `path` | String | The path to the environment. Will always return null if `expose_environment_path_in_alert_details` feature flag is disabled |
+| `path` | String! | The path to the environment. |
 | `state` | String! | State of the environment, for example: available/stopped |
 
 ### Epic

doc/operations/incident_management/alerts.md

@@ -219,46 +219,11 @@ the correct runbook:
 
 ## View the environment that generated the alert
 
-> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/232492) in GitLab 13.5.
-> - It's [deployed behind a feature flag](../../user/feature_flags.md), disabled by default.
-> - It's disabled on GitLab.com.
-> - It's not recommended for production use.
-> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-environment-link-in-alert-details). **(CORE ONLY)**
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/232492) in GitLab 13.5
+    behind a feature flag, disabled by default.
+> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/232492) in GitLab 13.6.
 
 CAUTION: **Warning:**
 This feature might not be available to you. Check the **version history** note above for details.
 
 The environment information and the link are displayed in the [Alert Details tab](#alert-details-tab).
-
-### Enable or disable Environment Link in Alert Details **(CORE ONLY)**
-
-Viewing the environment is under development and not ready for production use. It is
-deployed behind a feature flag that is **disabled by default**.
-[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md)
-can enable it.
-
-To enable it:
-
-```ruby
-Feature.enable(:expose_environment_path_in_alert_details)
-```
-
-To enable for just a particular project:
-
-```ruby
-project = Project.find_by_full_path('your-group/your-project')
-Feature.enable(:expose_environment_path_in_alert_details, project)
-```
-
-To disable it:
-
-```ruby
-Feature.disable(:expose_environment_path_in_alert_details)
-```
-
-To disable for just a particular project:
-
-```ruby
-project = Project.find_by_full_path('your-group/your-project')
-Feature.disable(:expose_environment_path_in_alert_details, project)
-```

spec/features/alert_management_spec.rb

@@ -41,21 +41,6 @@ RSpec.describe 'Alert management', :js do
             expect(page).to have_content(environment.name)
           end
         end
-
-        context 'when expose_environment_path_in_alert_details feature flag is disabled' do
-          before do
-            stub_feature_flags(expose_environment_path_in_alert_details: false)
-          end
-
-          it 'does not show the environment name' do
-            visit(details_project_alert_management_path(project, alert))
-
-            within('.alert-management-details-table') do
-              expect(page).to have_content(alert.title)
-              expect(page).not_to have_content(environment.name)
-            end
-          end
-        end
       end
     end
   end

spec/frontend/alert_management/components/alert_details_spec.js

@@ -20,11 +20,7 @@ const environmentName = 'Production';
 const environmentPath = '/fake/path';
 
 describe('AlertDetails', () => {
-  let environmentData = {
-    name: environmentName,
-    path: environmentPath,
-  };
-  let glFeatures = { exposeEnvironmentPathInAlertDetails: false };
+  let environmentData = { name: environmentName, path: environmentPath };
   let mock;
   let wrapper;
   const projectPath = 'root/alerts';
@@ -40,7 +36,6 @@ describe('AlertDetails', () => {
           projectPath,
           projectIssuesPath,
           projectId,
-          glFeatures,
         },
         data() {
           return {
@@ -159,33 +154,21 @@ describe('AlertDetails', () => {
     });
 
     describe('environment fields', () => {
-      describe('when exposeEnvironmentPathInAlertDetails is disabled', () => {
-        beforeEach(mountComponent);
+      it('should show the environment name with a link to the path', () => {
+        mountComponent();
+        const path = findEnvironmentPath();
 
-        it('should not show the environment', () => {
-          expect(findEnvironmentName().exists()).toBe(false);
-          expect(findEnvironmentPath().exists()).toBe(false);
-        });
+        expect(findEnvironmentName().exists()).toBe(false);
+        expect(path.text()).toBe(environmentName);
+        expect(path.attributes('href')).toBe(environmentPath);
       });
 
-      describe('when exposeEnvironmentPathInAlertDetails is enabled', () => {
-        beforeEach(() => {
-          glFeatures = { exposeEnvironmentPathInAlertDetails: true };
-          mountComponent();
-        });
-
-        it('should show the environment name with link to path', () => {
-          expect(findEnvironmentName().exists()).toBe(false);
-          expect(findEnvironmentPath().text()).toBe(environmentName);
-          expect(findEnvironmentPath().attributes('href')).toBe(environmentPath);
-        });
+      it('should only show the environment name if the path is not provided', () => {
+        environmentData = { name: environmentName, path: null };
+        mountComponent();
 
-        it('should only show the environment name if the path is not provided', () => {
-          environmentData = { name: environmentName, path: null };
-          mountComponent();
-          expect(findEnvironmentPath().exists()).toBe(false);
-          expect(findEnvironmentName().text()).toBe(environmentName);
-        });
+        expect(findEnvironmentPath().exists()).toBe(false);
+        expect(findEnvironmentName().text()).toBe(environmentName);
       });
     });
 
@@ -195,6 +178,7 @@ describe('AlertDetails', () => {
         mountComponent({
           data: { alert: { ...mockAlert, issueIid }, sidebarStatus: false },
         });
+
         expect(findViewIncidentBtn().exists()).toBe(true);
         expect(findViewIncidentBtn().attributes('href')).toBe(
           joinPaths(projectIssuesPath, issueIid),
@@ -220,8 +204,8 @@ describe('AlertDetails', () => {
         jest
           .spyOn(wrapper.vm.$apollo, 'mutate')
           .mockResolvedValue({ data: { createAlertIssue: { issue: { iid: issueIid } } } });
-
         findCreateIncidentBtn().trigger('click');
+
         expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({
           mutation: createIssueMutation,
           variables: {
@@ -251,6 +235,7 @@ describe('AlertDetails', () => {
       beforeEach(() => {
         mountComponent({ data: { alert: mockAlert } });
       });
+
       it('should display a table of raw alert details data', () => {
         expect(findDetailsTable().exists()).toBe(true);
       });

spec/frontend/vue_shared/components/alert_details_table_spec.js

@@ -23,14 +23,10 @@ const environmentPath = '/fake/path';
 
 describe('AlertDetails', () => {
   let environmentData = { name: environmentName, path: environmentPath };
-  let glFeatures = { exposeEnvironmentPathInAlertDetails: false };
   let wrapper;
 
   function mountComponent(propsData = {}) {
     wrapper = mount(AlertDetailsTable, {
-      provide: {
-        glFeatures,
-      },
       propsData: {
         alert: {
           ...mockAlert,
@@ -97,34 +93,19 @@ describe('AlertDetails', () => {
         expect(findTableField(fields, 'Severity').exists()).toBe(true);
         expect(findTableField(fields, 'Status').exists()).toBe(true);
         expect(findTableField(fields, 'Hosts').exists()).toBe(true);
-        expect(findTableField(fields, 'Environment').exists()).toBe(false);
+        expect(findTableField(fields, 'Environment').exists()).toBe(true);
       });
 
-      it('should not show disallowed and flaggedAllowed alert fields', () => {
+      it('should not show disallowed alert fields', () => {
         const fields = findTableKeys();
 
         expect(findTableField(fields, 'Typename').exists()).toBe(false);
         expect(findTableField(fields, 'Todos').exists()).toBe(false);
         expect(findTableField(fields, 'Notes').exists()).toBe(false);
         expect(findTableField(fields, 'Assignees').exists()).toBe(false);
-        expect(findTableField(fields, 'Environment').exists()).toBe(false);
-      });
-    });
-
-    describe('when exposeEnvironmentPathInAlertDetails is enabled', () => {
-      beforeEach(() => {
-        glFeatures = { exposeEnvironmentPathInAlertDetails: true };
-        mountComponent();
-      });
-
-      it('should show flaggedAllowed alert fields', () => {
-        const fields = findTableKeys();
-
-        expect(findTableField(fields, 'Environment').exists()).toBe(true);
       });
 
       it('should display only the name for the environment', () => {
-        expect(findTableFieldValueByKey('Iid').text()).toBe('1527542');
         expect(findTableFieldValueByKey('Environment').text()).toBe(environmentName);
       });
 

spec/graphql/types/environment_type_spec.rb

@@ -44,18 +44,12 @@ RSpec.describe GitlabSchema.types['Environment'] do
       expect(subject['data']['project']['environment']['name']).to eq(environment.name)
     end
 
-    it 'returns the path when the feature is enabled' do
+    it 'returns the path to the environment' do
       expect(subject['data']['project']['environment']['path']).to eq(
         Gitlab::Routing.url_helpers.project_environment_path(project, environment)
       )
     end
 
-    it 'does not return the path when the feature is disabled' do
-      stub_feature_flags(expose_environment_path_in_alert_details: false)
-
-      expect(subject['data']['project']['environment']['path']).to be_nil
-    end
-
     context 'when query alert data for the environment' do
       let_it_be(:query) do
         %(