Commit acecfb84 authored by Robert May's avatar Robert May

Merge branch '334682-feature-flag-rollout-of-compliance_dashboard_drawer' into 'master'

Remove development feature flag `compliance_dashboard_drawer`"

See merge request gitlab-org/gitlab!65943
parents 89ce649a 0351c9db
...@@ -25,10 +25,6 @@ The Compliance Dashboard shows only the latest MR on each project. ...@@ -25,10 +25,6 @@ The Compliance Dashboard shows only the latest MR on each project.
## Merge request drawer ## Merge request drawer
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/299357) in GitLab 14.1. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/299357) in GitLab 14.1.
> - It's [deployed behind a feature flag](../../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-merge-request-drawer).
When you click on a row, a drawer is shown that provides further details about the merge When you click on a row, a drawer is shown that provides further details about the merge
request: request:
...@@ -104,28 +100,3 @@ the dropdown next to the **List of all merge commits** button at the top of the ...@@ -104,28 +100,3 @@ the dropdown next to the **List of all merge commits** button at the top of the
NOTE: NOTE:
The Chain of Custody report download is a CSV file, with a maximum size of 15 MB. The Chain of Custody report download is a CSV file, with a maximum size of 15 MB.
The remaining records are truncated when this limit is reached. The remaining records are truncated when this limit is reached.
## Enable or disable merge request drawer **(ULTIMATE SELF)**
The merge request drawer 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
# For the instance
Feature.enable(:compliance_dashboard_drawer)
# For a single group
Feature.enable(:compliance_dashboard_drawer, Group.find(<group id>))
```
To disable it:
```ruby
# For the instance
Feature.disable(:compliance_dashboard_drawer)
# For a single group
Feature.disable(:compliance_dashboard_drawer, Group.find(<group id>)
```
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
import { GlTabs, GlTab } from '@gitlab/ui'; import { GlTabs, GlTab } from '@gitlab/ui';
import Cookies from 'js-cookie'; import Cookies from 'js-cookie';
import { __ } from '~/locale'; import { __ } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { COMPLIANCE_TAB_COOKIE_KEY } from '../constants'; import { COMPLIANCE_TAB_COOKIE_KEY } from '../constants';
import MergeRequestDrawer from './drawer.vue'; import MergeRequestDrawer from './drawer.vue';
import EmptyState from './empty_state.vue'; import EmptyState from './empty_state.vue';
...@@ -19,7 +18,6 @@ export default { ...@@ -19,7 +18,6 @@ export default {
GlTabs, GlTabs,
MergeCommitsExportButton, MergeCommitsExportButton,
}, },
mixins: [glFeatureFlagsMixin()],
props: { props: {
emptyStateSvgPath: { emptyStateSvgPath: {
type: String, type: String,
...@@ -53,9 +51,6 @@ export default { ...@@ -53,9 +51,6 @@ export default {
hasMergeCommitsCsvExportPath() { hasMergeCommitsCsvExportPath() {
return this.mergeCommitsCsvExportPath !== ''; return this.mergeCommitsCsvExportPath !== '';
}, },
drawerEnabled() {
return this.glFeatures.complianceDashboardDrawer;
},
}, },
methods: { methods: {
showTabs() { showTabs() {
...@@ -114,11 +109,9 @@ export default { ...@@ -114,11 +109,9 @@ export default {
v-else v-else
:merge-requests="mergeRequests" :merge-requests="mergeRequests"
:is-last-page="isLastPage" :is-last-page="isLastPage"
:drawer-enabled="drawerEnabled"
@toggleDrawer="toggleDrawer" @toggleDrawer="toggleDrawer"
/> />
<merge-request-drawer <merge-request-drawer
v-if="drawerEnabled"
:show-drawer="showDrawer" :show-drawer="showDrawer"
:merge-request="drawerData" :merge-request="drawerData"
z-index="252" z-index="252"
......
...@@ -32,11 +32,6 @@ export default { ...@@ -32,11 +32,6 @@ export default {
required: false, required: false,
default: false, default: false,
}, },
drawerEnabled: {
type: Boolean,
required: false,
default: false,
},
}, },
methods: { methods: {
key(id, value) { key(id, value) {
...@@ -78,8 +73,6 @@ export default { ...@@ -78,8 +73,6 @@ export default {
<grid-column-heading :heading="$options.strings.pipelineStatusLabel" class="gl-text-center" /> <grid-column-heading :heading="$options.strings.pipelineStatusLabel" class="gl-text-center" />
<grid-column-heading :heading="$options.strings.updatesLabel" class="gl-text-right" /> <grid-column-heading :heading="$options.strings.updatesLabel" class="gl-text-right" />
<!-- TODO: Remove the if/else and duplicate components with https://gitlab.com/gitlab-org/gitlab/-/issues/334682 -->
<template v-if="drawerEnabled">
<div <div
v-for="mergeRequest in mergeRequests" v-for="mergeRequest in mergeRequests"
:key="mergeRequest.id" :key="mergeRequest.id"
...@@ -134,55 +127,6 @@ export default { ...@@ -134,55 +127,6 @@ export default {
</time-ago-tooltip> </time-ago-tooltip>
</div> </div>
</div> </div>
</template>
<template v-else>
<template v-for="mergeRequest in mergeRequests">
<merge-request
:key="key(mergeRequest.id, $options.keyTypes.mergeRequest)"
:merge-request="mergeRequest"
/>
<status
:key="key(mergeRequest.id, 'approval')"
:status="{ type: 'approval', data: mergeRequest.approval_status }"
/>
<status
:key="key(mergeRequest.id, 'pipeline')"
:status="{ type: 'pipeline', data: mergeRequest.pipeline_status }"
/>
<div
:key="key(mergeRequest.id, $options.keyTypes.updates)"
class="gl-text-right gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5 gl-relative"
>
<approvers :approvers="mergeRequest.approved_by_users" />
<branch-details
v-if="hasBranchDetails(mergeRequest)"
class="gl-justify-content-end gl-text-gray-900"
:source-branch="{
name: mergeRequest.source_branch,
uri: mergeRequest.source_branch_uri,
}"
:target-branch="{
name: mergeRequest.target_branch,
uri: mergeRequest.target_branch_uri,
}"
/>
<time-ago-tooltip
:time="mergeRequest.merged_at"
tooltip-placement="bottom"
class="gl-text-gray-500"
>
<template #default="{ timeAgo }">
<gl-sprintf :message="$options.strings.mergedAtText">
<template #timeAgo>{{ timeAgo }}</template>
</gl-sprintf>
</template>
</time-ago-tooltip>
</div>
</template>
</template>
</div> </div>
<pagination class="gl-mt-5" :is-last-page="isLastPage" /> <pagination class="gl-mt-5" :is-last-page="isLastPage" />
......
...@@ -6,9 +6,6 @@ class Groups::Security::ComplianceDashboardsController < Groups::ApplicationCont ...@@ -6,9 +6,6 @@ class Groups::Security::ComplianceDashboardsController < Groups::ApplicationCont
layout 'group' layout 'group'
before_action :authorize_compliance_dashboard! before_action :authorize_compliance_dashboard!
before_action do
push_frontend_feature_flag(:compliance_dashboard_drawer, @group, default_enabled: :yaml)
end
track_unique_visits :show, target_id: 'g_compliance_dashboard' track_unique_visits :show, target_id: 'g_compliance_dashboard'
......
---
name: compliance_dashboard_drawer
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64771
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/334682
milestone: '14.1'
type: development
group: group::compliance
default_enabled: false
...@@ -44,7 +44,10 @@ exports[`ComplianceDashboard component when there are merge requests and the sho ...@@ -44,7 +44,10 @@ exports[`ComplianceDashboard component when there are merge requests and the sho
</b-tab-stub> </b-tab-stub>
</gl-tabs-stub> </gl-tabs-stub>
<!----> <merge-request-drawer-stub
mergerequest="[object Object]"
z-index="252"
/>
</div> </div>
`; `;
...@@ -76,7 +79,10 @@ exports[`ComplianceDashboard component when there are merge requests matches the ...@@ -76,7 +79,10 @@ exports[`ComplianceDashboard component when there are merge requests matches the
mergerequests="[object Object],[object Object]" mergerequests="[object Object],[object Object]"
/> />
<!----> <merge-request-drawer-stub
mergerequest="[object Object]"
z-index="252"
/>
</div> </div>
`; `;
......
...@@ -21,7 +21,7 @@ describe('ComplianceDashboard component', () => { ...@@ -21,7 +21,7 @@ describe('ComplianceDashboard component', () => {
const findMergeCommitsExportButton = () => wrapper.findComponent(MergeCommitsExportButton); const findMergeCommitsExportButton = () => wrapper.findComponent(MergeCommitsExportButton);
const findDashboardTabs = () => wrapper.findComponent(GlTabs); const findDashboardTabs = () => wrapper.findComponent(GlTabs);
const createComponent = (props = {}, glFeatures = {}) => { const createComponent = (props = {}) => {
return shallowMount(ComplianceDashboard, { return shallowMount(ComplianceDashboard, {
propsData: { propsData: {
mergeRequests, mergeRequests,
...@@ -30,9 +30,6 @@ describe('ComplianceDashboard component', () => { ...@@ -30,9 +30,6 @@ describe('ComplianceDashboard component', () => {
emptyStateSvgPath: 'empty.svg', emptyStateSvgPath: 'empty.svg',
...props, ...props,
}, },
provide: {
glFeatures,
},
stubs: { stubs: {
GlTab, GlTab,
}, },
...@@ -113,15 +110,13 @@ describe('ComplianceDashboard component', () => { ...@@ -113,15 +110,13 @@ describe('ComplianceDashboard component', () => {
}); });
describe('with the merge requests drawer', () => { describe('with the merge requests drawer', () => {
describe('when the feature is enabled', () => {
beforeEach(() => { beforeEach(() => {
wrapper = createComponent({}, { complianceDashboardDrawer: true }); wrapper = createComponent();
}); });
it('opens the drawer', async () => { it('opens the drawer', async () => {
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]); await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]);
expect(findMergeRequestsGrid().props('drawerEnabled')).toBe(true);
expect(findMergeRequestsDrawer().props('showDrawer')).toBe(true); expect(findMergeRequestsDrawer().props('showDrawer')).toBe(true);
expect(findMergeRequestsDrawer().props('mergeRequest')).toStrictEqual(mergeRequests[0]); expect(findMergeRequestsDrawer().props('mergeRequest')).toStrictEqual(mergeRequests[0]);
}); });
...@@ -129,7 +124,6 @@ describe('ComplianceDashboard component', () => { ...@@ -129,7 +124,6 @@ describe('ComplianceDashboard component', () => {
it('closes the drawer via the drawer close event', async () => { it('closes the drawer via the drawer close event', async () => {
await findMergeRequestsDrawer().vm.$emit('close'); await findMergeRequestsDrawer().vm.$emit('close');
expect(findMergeRequestsGrid().props('drawerEnabled')).toBe(true);
expect(findMergeRequestsDrawer().props('showDrawer')).toBe(false); expect(findMergeRequestsDrawer().props('showDrawer')).toBe(false);
expect(findMergeRequestsDrawer().props('mergeRequest')).toEqual({}); expect(findMergeRequestsDrawer().props('mergeRequest')).toEqual({});
}); });
...@@ -138,7 +132,6 @@ describe('ComplianceDashboard component', () => { ...@@ -138,7 +132,6 @@ describe('ComplianceDashboard component', () => {
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]); await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]);
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]); await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]);
expect(findMergeRequestsGrid().props('drawerEnabled')).toBe(true);
expect(findMergeRequestsDrawer().props('showDrawer')).toBe(false); expect(findMergeRequestsDrawer().props('showDrawer')).toBe(false);
expect(findMergeRequestsDrawer().props('mergeRequest')).toEqual({}); expect(findMergeRequestsDrawer().props('mergeRequest')).toEqual({});
}); });
...@@ -147,21 +140,8 @@ describe('ComplianceDashboard component', () => { ...@@ -147,21 +140,8 @@ describe('ComplianceDashboard component', () => {
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]); await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]);
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[1]); await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[1]);
expect(findMergeRequestsGrid().props('drawerEnabled')).toBe(true);
expect(findMergeRequestsDrawer().props('showDrawer')).toBe(true); expect(findMergeRequestsDrawer().props('showDrawer')).toBe(true);
expect(findMergeRequestsDrawer().props('mergeRequest')).toStrictEqual(mergeRequests[1]); expect(findMergeRequestsDrawer().props('mergeRequest')).toStrictEqual(mergeRequests[1]);
}); });
}); });
describe('when the feature is disabled', () => {
beforeEach(() => {
wrapper = createComponent({}, { complianceDashboardDrawer: false });
});
it('does not show the drawer', () => {
expect(findMergeRequestsGrid().props('drawerEnabled')).toBe(false);
expect(findMergeRequestsDrawer().exists()).toBe(false);
});
});
});
}); });
// Jest Snapshot v1, https://goo.gl/fbAQLP // Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`MergeRequestsGrid component when drawer enabled is false when initialized matches the snapshot 1`] = ` exports[`MergeRequestsGrid component when initialized matches the snapshot 1`] = `
<div>
<div
class="dashboard-grid gl-display-grid gl-grid-tpl-rows-auto"
>
<grid-column-heading-stub
heading="Merge Request"
/>
<grid-column-heading-stub
class="gl-text-center"
heading="Approval Status"
/>
<grid-column-heading-stub
class="gl-text-center"
heading="Pipeline"
/>
<grid-column-heading-stub
class="gl-text-right"
heading="Updates"
/>
<div
data-testid="merge-request"
>
<a
data-testid="merge-request-link"
href=""
>
Merge request 0
</a>
</div>
<status-stub
status="[object Object]"
/>
<status-stub
status="[object Object]"
/>
<div
class="gl-text-right gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5 gl-relative"
>
<approvers-stub
approvers=""
/>
<!---->
<time-ago-tooltip-stub
class="gl-text-gray-500"
cssclass=""
time="2020-01-01T00:00:00.000Z"
tooltipplacement="bottom"
/>
</div>
<div
data-testid="merge-request"
>
<a
data-testid="merge-request-link"
href=""
>
Merge request 1
</a>
</div>
<status-stub
status="[object Object]"
/>
<status-stub
status="[object Object]"
/>
<div
class="gl-text-right gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5 gl-relative"
>
<approvers-stub
approvers=""
/>
<!---->
<time-ago-tooltip-stub
class="gl-text-gray-500"
cssclass=""
time="2020-01-01T00:00:00.000Z"
tooltipplacement="bottom"
/>
</div>
</div>
<pagination-stub
class="gl-mt-5"
/>
</div>
`;
exports[`MergeRequestsGrid component when drawer enabled is true when initialized matches the snapshot 1`] = `
<div> <div>
<div <div
class="dashboard-grid gl-display-grid gl-grid-tpl-rows-auto" class="dashboard-grid gl-display-grid gl-grid-tpl-rows-auto"
......
...@@ -18,12 +18,11 @@ describe('MergeRequestsGrid component', () => { ...@@ -18,12 +18,11 @@ describe('MergeRequestsGrid component', () => {
const findApprovers = () => wrapper.findComponent(Approvers); const findApprovers = () => wrapper.findComponent(Approvers);
const findBranchDetails = () => wrapper.findComponent(BranchDetails); const findBranchDetails = () => wrapper.findComponent(BranchDetails);
const createComponent = (mergeRequests = {}, drawerEnabled = false) => { const createComponent = (mergeRequests = {}) => {
return shallowMountExtended(MergeRequestsGrid, { return shallowMountExtended(MergeRequestsGrid, {
propsData: { propsData: {
mergeRequests, mergeRequests,
isLastPage: false, isLastPage: false,
drawerEnabled,
}, },
stubs: { stubs: {
MergeRequest: { MergeRequest: {
...@@ -38,11 +37,9 @@ describe('MergeRequestsGrid component', () => { ...@@ -38,11 +37,9 @@ describe('MergeRequestsGrid component', () => {
wrapper.destroy(); wrapper.destroy();
}); });
// TODO: Remove the each with https://gitlab.com/gitlab-org/gitlab/-/issues/334682
describe.each([true, false])('when drawer enabled is %s', (drawerEnabled) => {
describe('when initialized', () => { describe('when initialized', () => {
beforeEach(() => { beforeEach(() => {
wrapper = createComponent(createMergeRequests({ count: 2, props: {} }), drawerEnabled); wrapper = createComponent(createMergeRequests({ count: 2, props: {} }));
}); });
it('matches the snapshot', () => { it('matches the snapshot', () => {
...@@ -66,7 +63,7 @@ describe('MergeRequestsGrid component', () => { ...@@ -66,7 +63,7 @@ describe('MergeRequestsGrid component', () => {
const mergeRequest = createMergeRequests({ count: 1 }); const mergeRequest = createMergeRequests({ count: 1 });
beforeEach(() => { beforeEach(() => {
wrapper = createComponent(mergeRequest, drawerEnabled); wrapper = createComponent(mergeRequest);
}); });
it('passes the correct props to the statuses', () => { it('passes the correct props to the statuses', () => {
...@@ -91,7 +88,7 @@ describe('MergeRequestsGrid component', () => { ...@@ -91,7 +88,7 @@ describe('MergeRequestsGrid component', () => {
describe('branch details', () => { describe('branch details', () => {
it('does not render if there are no branch details', () => { it('does not render if there are no branch details', () => {
wrapper = createComponent(createMergeRequests({ count: 2, props: {} }), drawerEnabled); wrapper = createComponent(createMergeRequests({ count: 2, props: {} }));
expect(findBranchDetails().exists()).toBe(false); expect(findBranchDetails().exists()).toBe(false);
}); });
...@@ -102,27 +99,23 @@ describe('MergeRequestsGrid component', () => { ...@@ -102,27 +99,23 @@ describe('MergeRequestsGrid component', () => {
count: 2, count: 2,
props: { target_branch: 'main', source_branch: 'feature' }, props: { target_branch: 'main', source_branch: 'feature' },
}), }),
drawerEnabled,
); );
expect(findBranchDetails().exists()).toBe(true); expect(findBranchDetails().exists()).toBe(true);
}); });
}); });
});
describe('when the drawer is enabled', () => { describe.each(['click', 'keypress.enter'])('when the %s event is triggered', (event) => {
const mergeRequests = createMergeRequests({ count: 2, props: {} }); const mergeRequest = createMergeRequests({ count: 1 });
beforeEach(() => { beforeEach(() => {
const mergeRequest = createMergeRequests({ count: 1 });
wrapper = createComponent(mergeRequest, true); wrapper = createComponent(mergeRequest, true);
}); });
describe.each(['click', 'keypress.enter'])('when the %s event is triggered', (event) => {
it('toggles the drawer when a merge request drawer toggle is the target', () => { it('toggles the drawer when a merge request drawer toggle is the target', () => {
findMergeRequestDrawerToggles().at(0).trigger(event); findMergeRequestDrawerToggles().at(0).trigger(event);
expect(wrapper.emitted('toggleDrawer')[0][0]).toStrictEqual(mergeRequests[0]); expect(wrapper.emitted('toggleDrawer')[0][0]).toStrictEqual(mergeRequest[0]);
}); });
it('does not toggle the drawer if an inner link is the target', () => { it('does not toggle the drawer if an inner link is the target', () => {
...@@ -131,5 +124,4 @@ describe('MergeRequestsGrid component', () => { ...@@ -131,5 +124,4 @@ describe('MergeRequestsGrid component', () => {
expect(wrapper.emitted('toggleDrawer')).toBe(undefined); expect(wrapper.emitted('toggleDrawer')).toBe(undefined);
}); });
}); });
});
}); });
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