Commit ac1c9fa4 authored by Angelo Gulina's avatar Angelo Gulina Committed by Shinya Maeda

Show the environment link on alert details page

Remove the feature flag
Remove all feature flag related code
parent 540de669
...@@ -30,7 +30,6 @@ import AlertSidebar from './alert_sidebar.vue'; ...@@ -30,7 +30,6 @@ import AlertSidebar from './alert_sidebar.vue';
import AlertMetrics from './alert_metrics.vue'; import AlertMetrics from './alert_metrics.vue';
import AlertDetailsTable from '~/vue_shared/components/alert_details_table.vue'; import AlertDetailsTable from '~/vue_shared/components/alert_details_table.vue';
import AlertSummaryRow from './alert_summary_row.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'); const containerEl = document.querySelector('.page-with-contextual-sidebar');
...@@ -77,7 +76,6 @@ export default { ...@@ -77,7 +76,6 @@ export default {
SystemNote, SystemNote,
AlertMetrics, AlertMetrics,
}, },
mixins: [glFeatureFlagsMixin()],
inject: { inject: {
projectPath: { projectPath: {
default: '', default: '',
...@@ -150,13 +148,10 @@ export default { ...@@ -150,13 +148,10 @@ export default {
}, },
}, },
environmentName() { environmentName() {
return this.shouldDisplayEnvironment && this.alert?.environment?.name; return this.alert?.environment?.name;
}, },
environmentPath() { environmentPath() {
return this.shouldDisplayEnvironment && this.alert?.environment?.path; return this.alert?.environment?.path;
},
shouldDisplayEnvironment() {
return this.glFeatures.exposeEnvironmentPathInAlertDetails;
}, },
}, },
mounted() { mounted() {
......
...@@ -7,7 +7,6 @@ import { ...@@ -7,7 +7,6 @@ import {
convertToSentenceCase, convertToSentenceCase,
splitCamelCase, splitCamelCase,
} from '~/lib/utils/text_utility'; } 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 thClass = 'gl-bg-transparent! gl-border-1! gl-border-b-solid! gl-border-gray-200!';
const tdClass = 'gl-border-gray-100! gl-p-5!'; const tdClass = 'gl-border-gray-100! gl-p-5!';
...@@ -25,6 +24,7 @@ const allowedFields = [ ...@@ -25,6 +24,7 @@ const allowedFields = [
'endedAt', 'endedAt',
'details', 'details',
'hosts', 'hosts',
'environment',
]; ];
export default { export default {
...@@ -32,7 +32,6 @@ export default { ...@@ -32,7 +32,6 @@ export default {
GlLoadingIcon, GlLoadingIcon,
GlTable, GlTable,
}, },
mixins: [glFeatureFlagsMixin()],
props: { props: {
alert: { alert: {
type: Object, type: Object,
...@@ -60,9 +59,6 @@ export default { ...@@ -60,9 +59,6 @@ export default {
}, },
], ],
computed: { computed: {
flaggedAllowedFields() {
return this.shouldDisplayEnvironment ? [...allowedFields, 'environment'] : allowedFields;
},
items() { items() {
if (!this.alert) { if (!this.alert) {
return []; return [];
...@@ -84,13 +80,10 @@ export default { ...@@ -84,13 +80,10 @@ export default {
[], [],
); );
}, },
shouldDisplayEnvironment() {
return this.glFeatures.exposeEnvironmentPathInAlertDetails;
},
}, },
methods: { methods: {
isAllowed(fieldName) { isAllowed(fieldName) {
return this.flaggedAllowedFields.includes(fieldName); return allowedFields.includes(fieldName);
}, },
}, },
}; };
......
...@@ -10,6 +10,5 @@ class Projects::AlertManagementController < Projects::ApplicationController ...@@ -10,6 +10,5 @@ class Projects::AlertManagementController < Projects::ApplicationController
def details def details
@alert_id = params[:id] @alert_id = params[:id]
push_frontend_feature_flag(:expose_environment_path_in_alert_details, @project)
end end
end end
...@@ -18,9 +18,8 @@ module Types ...@@ -18,9 +18,8 @@ module Types
field :state, GraphQL::STRING_TYPE, null: false, field :state, GraphQL::STRING_TYPE, null: false,
description: 'State of the environment, for example: available/stopped' description: 'State of the environment, for example: available/stopped'
field :path, GraphQL::STRING_TYPE, null: true, field :path, GraphQL::STRING_TYPE, null: false,
description: 'The path to the environment. Will always return null ' \ description: 'The path to the environment.'
'if `expose_environment_path_in_alert_details` feature flag is disabled'
field :metrics_dashboard, Types::Metrics::DashboardType, null: true, field :metrics_dashboard, Types::Metrics::DashboardType, null: true,
description: 'Metrics dashboard schema for the environment', description: 'Metrics dashboard schema for the environment',
......
...@@ -6,8 +6,6 @@ class EnvironmentPresenter < Gitlab::View::Presenter::Delegated ...@@ -6,8 +6,6 @@ class EnvironmentPresenter < Gitlab::View::Presenter::Delegated
presents :environment presents :environment
def path def path
if Feature.enabled?(:expose_environment_path_in_alert_details, project) project_environment_path(project, self)
project_environment_path(project, self)
end
end end
end end
---
title: Show the environment link on alert details page
merge_request: 44130
author:
type: added
---
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
...@@ -6009,10 +6009,9 @@ type Environment { ...@@ -6009,10 +6009,9 @@ type Environment {
name: String! name: String!
""" """
The path to the environment. Will always return null if The path to the environment.
`expose_environment_path_in_alert_details` feature flag is disabled
""" """
path: String path: String!
""" """
State of the environment, for example: available/stopped State of the environment, for example: available/stopped
......
...@@ -16567,14 +16567,18 @@ ...@@ -16567,14 +16567,18 @@
}, },
{ {
"name": "path", "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": [ "args": [
], ],
"type": { "type": {
"kind": "SCALAR", "kind": "NON_NULL",
"name": "String", "name": null,
"ofType": null "ofType": {
"kind": "SCALAR",
"name": "String",
"ofType": null
}
}, },
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "deprecationReason": null
...@@ -973,7 +973,7 @@ Describes where code is deployed for a project. ...@@ -973,7 +973,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 | | `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 | | `metricsDashboard` | MetricsDashboard | Metrics dashboard schema for the environment |
| `name` | String! | Human-readable name of 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 | | `state` | String! | State of the environment, for example: available/stopped |
### Epic ### Epic
......
...@@ -219,46 +219,11 @@ the correct runbook: ...@@ -219,46 +219,11 @@ the correct runbook:
## View the environment that generated the alert ## View the environment that generated the alert
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/232492) in GitLab 13.5. > - [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. behind a feature flag, disabled by default.
> - It's disabled on GitLab.com. > - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/232492) in GitLab 13.6.
> - 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)**
CAUTION: **Warning:** CAUTION: **Warning:**
This feature might not be available to you. Check the **version history** note above for details. 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). 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)
```
...@@ -41,21 +41,6 @@ RSpec.describe 'Alert management', :js do ...@@ -41,21 +41,6 @@ RSpec.describe 'Alert management', :js do
expect(page).to have_content(environment.name) expect(page).to have_content(environment.name)
end end
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 end
end end
......
...@@ -20,11 +20,7 @@ const environmentName = 'Production'; ...@@ -20,11 +20,7 @@ const environmentName = 'Production';
const environmentPath = '/fake/path'; const environmentPath = '/fake/path';
describe('AlertDetails', () => { describe('AlertDetails', () => {
let environmentData = { let environmentData = { name: environmentName, path: environmentPath };
name: environmentName,
path: environmentPath,
};
let glFeatures = { exposeEnvironmentPathInAlertDetails: false };
let mock; let mock;
let wrapper; let wrapper;
const projectPath = 'root/alerts'; const projectPath = 'root/alerts';
...@@ -40,7 +36,6 @@ describe('AlertDetails', () => { ...@@ -40,7 +36,6 @@ describe('AlertDetails', () => {
projectPath, projectPath,
projectIssuesPath, projectIssuesPath,
projectId, projectId,
glFeatures,
}, },
data() { data() {
return { return {
...@@ -159,33 +154,21 @@ describe('AlertDetails', () => { ...@@ -159,33 +154,21 @@ describe('AlertDetails', () => {
}); });
describe('environment fields', () => { describe('environment fields', () => {
describe('when exposeEnvironmentPathInAlertDetails is disabled', () => { it('should show the environment name with a link to the path', () => {
beforeEach(mountComponent); mountComponent();
const path = findEnvironmentPath();
it('should not show the environment', () => { expect(findEnvironmentName().exists()).toBe(false);
expect(findEnvironmentName().exists()).toBe(false); expect(path.text()).toBe(environmentName);
expect(findEnvironmentPath().exists()).toBe(false); expect(path.attributes('href')).toBe(environmentPath);
});
}); });
describe('when exposeEnvironmentPathInAlertDetails is enabled', () => { it('should only show the environment name if the path is not provided', () => {
beforeEach(() => { environmentData = { name: environmentName, path: null };
glFeatures = { exposeEnvironmentPathInAlertDetails: true }; mountComponent();
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', () => { expect(findEnvironmentPath().exists()).toBe(false);
environmentData = { name: environmentName, path: null }; expect(findEnvironmentName().text()).toBe(environmentName);
mountComponent();
expect(findEnvironmentPath().exists()).toBe(false);
expect(findEnvironmentName().text()).toBe(environmentName);
});
}); });
}); });
...@@ -195,6 +178,7 @@ describe('AlertDetails', () => { ...@@ -195,6 +178,7 @@ describe('AlertDetails', () => {
mountComponent({ mountComponent({
data: { alert: { ...mockAlert, issueIid }, sidebarStatus: false }, data: { alert: { ...mockAlert, issueIid }, sidebarStatus: false },
}); });
expect(findViewIncidentBtn().exists()).toBe(true); expect(findViewIncidentBtn().exists()).toBe(true);
expect(findViewIncidentBtn().attributes('href')).toBe( expect(findViewIncidentBtn().attributes('href')).toBe(
joinPaths(projectIssuesPath, issueIid), joinPaths(projectIssuesPath, issueIid),
...@@ -220,8 +204,8 @@ describe('AlertDetails', () => { ...@@ -220,8 +204,8 @@ describe('AlertDetails', () => {
jest jest
.spyOn(wrapper.vm.$apollo, 'mutate') .spyOn(wrapper.vm.$apollo, 'mutate')
.mockResolvedValue({ data: { createAlertIssue: { issue: { iid: issueIid } } } }); .mockResolvedValue({ data: { createAlertIssue: { issue: { iid: issueIid } } } });
findCreateIncidentBtn().trigger('click'); findCreateIncidentBtn().trigger('click');
expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({ expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({
mutation: createIssueMutation, mutation: createIssueMutation,
variables: { variables: {
...@@ -251,6 +235,7 @@ describe('AlertDetails', () => { ...@@ -251,6 +235,7 @@ describe('AlertDetails', () => {
beforeEach(() => { beforeEach(() => {
mountComponent({ data: { alert: mockAlert } }); mountComponent({ data: { alert: mockAlert } });
}); });
it('should display a table of raw alert details data', () => { it('should display a table of raw alert details data', () => {
expect(findDetailsTable().exists()).toBe(true); expect(findDetailsTable().exists()).toBe(true);
}); });
......
...@@ -23,14 +23,10 @@ const environmentPath = '/fake/path'; ...@@ -23,14 +23,10 @@ const environmentPath = '/fake/path';
describe('AlertDetails', () => { describe('AlertDetails', () => {
let environmentData = { name: environmentName, path: environmentPath }; let environmentData = { name: environmentName, path: environmentPath };
let glFeatures = { exposeEnvironmentPathInAlertDetails: false };
let wrapper; let wrapper;
function mountComponent(propsData = {}) { function mountComponent(propsData = {}) {
wrapper = mount(AlertDetailsTable, { wrapper = mount(AlertDetailsTable, {
provide: {
glFeatures,
},
propsData: { propsData: {
alert: { alert: {
...mockAlert, ...mockAlert,
...@@ -97,34 +93,19 @@ describe('AlertDetails', () => { ...@@ -97,34 +93,19 @@ describe('AlertDetails', () => {
expect(findTableField(fields, 'Severity').exists()).toBe(true); expect(findTableField(fields, 'Severity').exists()).toBe(true);
expect(findTableField(fields, 'Status').exists()).toBe(true); expect(findTableField(fields, 'Status').exists()).toBe(true);
expect(findTableField(fields, 'Hosts').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(); const fields = findTableKeys();
expect(findTableField(fields, 'Typename').exists()).toBe(false); expect(findTableField(fields, 'Typename').exists()).toBe(false);
expect(findTableField(fields, 'Todos').exists()).toBe(false); expect(findTableField(fields, 'Todos').exists()).toBe(false);
expect(findTableField(fields, 'Notes').exists()).toBe(false); expect(findTableField(fields, 'Notes').exists()).toBe(false);
expect(findTableField(fields, 'Assignees').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', () => { it('should display only the name for the environment', () => {
expect(findTableFieldValueByKey('Iid').text()).toBe('1527542');
expect(findTableFieldValueByKey('Environment').text()).toBe(environmentName); expect(findTableFieldValueByKey('Environment').text()).toBe(environmentName);
}); });
......
...@@ -44,18 +44,12 @@ RSpec.describe GitlabSchema.types['Environment'] do ...@@ -44,18 +44,12 @@ RSpec.describe GitlabSchema.types['Environment'] do
expect(subject['data']['project']['environment']['name']).to eq(environment.name) expect(subject['data']['project']['environment']['name']).to eq(environment.name)
end 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( expect(subject['data']['project']['environment']['path']).to eq(
Gitlab::Routing.url_helpers.project_environment_path(project, environment) Gitlab::Routing.url_helpers.project_environment_path(project, environment)
) )
end 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 context 'when query alert data for the environment' do
let_it_be(:query) do let_it_be(:query) do
%( %(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment