Skip to content

G
gitlab-ce
Commit 0351c9db authored by Robert Hunt's avatar Robert Hunt
Browse files
Options

Removed the compliance_dashboard_drawer feature flag 

- Removed the YAML definition
- Removed the frontend flag
- Removed the duplicate frontend code
- Updated specs

Changelog: added
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65943
EE: true
parent 5a0d8403
Hide whitespace changes
Inline Side-by-side
Showing with 140 additions and 367 deletions
doc/user/compliance/compliance_dashboard/index.md
View file @ 0351c9db
......@@ -25,10 +25,6 @@ The Compliance Dashboard shows only the latest MR on each project.
## Merge request drawer
> - [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
request:
......@@ -104,28 +100,3 @@ the dropdown next to the **List of all merge commits** button at the top of the
NOTE:
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.
## 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>)
```
ee/app/assets/javascripts/compliance_dashboard/components/dashboard.vue
View file @ 0351c9db
......@@ -2,7 +2,6 @@
import { GlTabs, GlTab } from '@gitlab/ui';
import Cookies from 'js-cookie';
import { __ } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { COMPLIANCE_TAB_COOKIE_KEY } from '../constants';
import MergeRequestDrawer from './drawer.vue';
import EmptyState from './empty_state.vue';
......@@ -19,7 +18,6 @@ export default {
GlTabs,
MergeCommitsExportButton,
},
mixins: [glFeatureFlagsMixin()],
props: {
emptyStateSvgPath: {
type: String,
......@@ -53,9 +51,6 @@ export default {
hasMergeCommitsCsvExportPath() {
return this.mergeCommitsCsvExportPath !== '';
},
drawerEnabled() {
return this.glFeatures.complianceDashboardDrawer;
},
},
methods: {
showTabs() {
......@@ -114,11 +109,9 @@ export default {
v-else
:merge-requests="mergeRequests"
:is-last-page="isLastPage"
:drawer-enabled="drawerEnabled"
@toggleDrawer="toggleDrawer"
/>
<merge-request-drawer
v-if="drawerEnabled"
:show-drawer="showDrawer"
:merge-request="drawerData"
z-index="252"
......
ee/app/assets/javascripts/compliance_dashboard/components/merge_requests/grid.vue
View file @ 0351c9db
......@@ -32,11 +32,6 @@ export default {
required: false,
default: false,
},
drawerEnabled: {
type: Boolean,
required: false,
default: false,
},
},
methods: {
key(id, value) {
......@@ -78,111 +73,60 @@ export default {
<grid-column-heading :heading="$options.strings.pipelineStatusLabel" class="gl-text-center" />
<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
v-for="mergeRequest in mergeRequests"
:key="mergeRequest.id"
class="dashboard-merge-request dashboard-grid gl-display-grid gl-grid-tpl-rows-auto gl-hover-bg-blue-50 gl-hover-text-decoration-none gl-hover-cursor-pointer"
data-testid="merge-request-drawer-toggle"
tabindex="0"
@click="onRowClick($event, mergeRequest)"
@keypress.enter="onRowClick($event, mergeRequest)"
>
<merge-request
:key="key(mergeRequest.id, $options.keyTypes.mergeRequest)"
:merge-request="mergeRequest"
/>
<div
v-for="mergeRequest in mergeRequests"
:key="mergeRequest.id"
class="dashboard-merge-request dashboard-grid gl-display-grid gl-grid-tpl-rows-auto gl-hover-bg-blue-50 gl-hover-text-decoration-none gl-hover-cursor-pointer"
data-testid="merge-request-drawer-toggle"
tabindex="0"
@click="onRowClick($event, mergeRequest)"
@keypress.enter="onRowClick($event, mergeRequest)"
>
<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, 'approval')"
:status="{ type: 'approval', data: mergeRequest.approval_status }"
/>
<status
:key="key(mergeRequest.id, 'pipeline')"
:status="{ type: 'pipeline', data: mergeRequest.pipeline_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>
</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,
}"
/>
<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"
<time-ago-tooltip
:time="mergeRequest.merged_at"
tooltip-placement="bottom"
class="gl-text-gray-500"
>
<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>
<template #default="{ timeAgo }">
<gl-sprintf :message="$options.strings.mergedAtText">
<template #timeAgo>{{ timeAgo }}</template>
</gl-sprintf>
</template>
</time-ago-tooltip>
</div>
</div>
</div>
<pagination class="gl-mt-5" :is-last-page="isLastPage" />
......
ee/app/controllers/groups/security/compliance_dashboards_controller.rb
View file @ 0351c9db
......@@ -6,9 +6,6 @@ class Groups::Security::ComplianceDashboardsController < Groups::ApplicationCont
layout 'group'
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'
......
ee/config/feature_flags/development/compliance_dashboard_drawer.yml deleted 100644 → 0
View file @ 5a0d8403
---
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
ee/spec/frontend/compliance_dashboard/components/__snapshots__/dashboard_spec.js.snap
View file @ 0351c9db
......@@ -44,7 +44,10 @@ exports[`ComplianceDashboard component when there are merge requests and the sho
</b-tab-stub>
</gl-tabs-stub>
<!---->
<merge-request-drawer-stub
mergerequest="[object Object]"
z-index="252"
/>
</div>
`;
......@@ -76,7 +79,10 @@ exports[`ComplianceDashboard component when there are merge requests matches the
mergerequests="[object Object],[object Object]"
/>
<!---->
<merge-request-drawer-stub
mergerequest="[object Object]"
z-index="252"
/>
</div>
`;
......
ee/spec/frontend/compliance_dashboard/components/dashboard_spec.js
View file @ 0351c9db
......@@ -21,7 +21,7 @@ describe('ComplianceDashboard component', () => {
const findMergeCommitsExportButton = () => wrapper.findComponent(MergeCommitsExportButton);
const findDashboardTabs = () => wrapper.findComponent(GlTabs);
const createComponent = (props = {}, glFeatures = {}) => {
const createComponent = (props = {}) => {
return shallowMount(ComplianceDashboard, {
propsData: {
mergeRequests,
......@@ -30,9 +30,6 @@ describe('ComplianceDashboard component', () => {
emptyStateSvgPath: 'empty.svg',
...props,
},
provide: {
glFeatures,
},
stubs: {
GlTab,
},
......@@ -113,55 +110,38 @@ describe('ComplianceDashboard component', () => {
});
describe('with the merge requests drawer', () => {
describe('when the feature is enabled', () => {
beforeEach(() => {
wrapper = createComponent({}, { complianceDashboardDrawer: true });
});
it('opens the drawer', async () => {
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]);
expect(findMergeRequestsGrid().props('drawerEnabled')).toBe(true);
expect(findMergeRequestsDrawer().props('showDrawer')).toBe(true);
expect(findMergeRequestsDrawer().props('mergeRequest')).toStrictEqual(mergeRequests[0]);
});
beforeEach(() => {
wrapper = createComponent();
});
it('closes the drawer via the drawer close event', async () => {
await findMergeRequestsDrawer().vm.$emit('close');
it('opens the drawer', async () => {
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]);
expect(findMergeRequestsGrid().props('drawerEnabled')).toBe(true);
expect(findMergeRequestsDrawer().props('showDrawer')).toBe(false);
expect(findMergeRequestsDrawer().props('mergeRequest')).toEqual({});
});
expect(findMergeRequestsDrawer().props('showDrawer')).toBe(true);
expect(findMergeRequestsDrawer().props('mergeRequest')).toStrictEqual(mergeRequests[0]);
});
it('closes the drawer via the grid toggle event', async () => {
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]);
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]);
it('closes the drawer via the drawer close event', async () => {
await findMergeRequestsDrawer().vm.$emit('close');
expect(findMergeRequestsGrid().props('drawerEnabled')).toBe(true);
expect(findMergeRequestsDrawer().props('showDrawer')).toBe(false);
expect(findMergeRequestsDrawer().props('mergeRequest')).toEqual({});
});
expect(findMergeRequestsDrawer().props('showDrawer')).toBe(false);
expect(findMergeRequestsDrawer().props('mergeRequest')).toEqual({});
});
it('swaps the drawer when a new merge request is selected', async () => {
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]);
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[1]);
it('closes the drawer via the grid toggle event', 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('mergeRequest')).toStrictEqual(mergeRequests[1]);
});
expect(findMergeRequestsDrawer().props('showDrawer')).toBe(false);
expect(findMergeRequestsDrawer().props('mergeRequest')).toEqual({});
});
describe('when the feature is disabled', () => {
beforeEach(() => {
wrapper = createComponent({}, { complianceDashboardDrawer: false });
});
it('swaps the drawer when a new merge request is selected', async () => {
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[0]);
await findMergeRequestsGrid().vm.$emit('toggleDrawer', mergeRequests[1]);
it('does not show the drawer', () => {
expect(findMergeRequestsGrid().props('drawerEnabled')).toBe(false);
expect(findMergeRequestsDrawer().exists()).toBe(false);
});
expect(findMergeRequestsDrawer().props('showDrawer')).toBe(true);
expect(findMergeRequestsDrawer().props('mergeRequest')).toStrictEqual(mergeRequests[1]);
});
});
});
ee/spec/frontend/compliance_dashboard/components/merge_requests/__snapshots__/grid_spec.js.snap
View file @ 0351c9db
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`MergeRequestsGrid component when drawer enabled is false 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`] = `
exports[`MergeRequestsGrid component when initialized matches the snapshot 1`] = `
<div>
<div
class="dashboard-grid gl-display-grid gl-grid-tpl-rows-auto"
......
ee/spec/frontend/compliance_dashboard/components/merge_requests/grid_spec.js
View file @ 0351c9db
......@@ -18,12 +18,11 @@ describe('MergeRequestsGrid component', () => {
const findApprovers = () => wrapper.findComponent(Approvers);
const findBranchDetails = () => wrapper.findComponent(BranchDetails);
const createComponent = (mergeRequests = {}, drawerEnabled = false) => {
const createComponent = (mergeRequests = {}) => {
return shallowMountExtended(MergeRequestsGrid, {
propsData: {
mergeRequests,
isLastPage: false,
drawerEnabled,
},
stubs: {
MergeRequest: {
......@@ -38,98 +37,91 @@ describe('MergeRequestsGrid component', () => {
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', () => {
beforeEach(() => {
wrapper = createComponent(createMergeRequests({ count: 2, props: {} }), drawerEnabled);
});
describe('when initialized', () => {
beforeEach(() => {
wrapper = createComponent(createMergeRequests({ count: 2, props: {} }));
});
it('matches the snapshot', () => {
expect(wrapper.element).toMatchSnapshot();
});
it('matches the snapshot', () => {
expect(wrapper.element).toMatchSnapshot();
});
it('renders a list of merge requests', () => {
expect(findMergeRequests()).toHaveLength(2);
});
it('renders a list of merge requests', () => {
expect(findMergeRequests()).toHaveLength(2);
});
it('renders the approvers list', () => {
expect(findApprovers().exists()).toBe(true);
});
it('renders the approvers list', () => {
expect(findApprovers().exists()).toBe(true);
});
it('renders the "merged at" time', () => {
expect(findTime().props('time')).toEqual(mergedAt());
});
it('renders the "merged at" time', () => {
expect(findTime().props('time')).toEqual(mergedAt());
});
});
describe('statuses', () => {
const mergeRequest = createMergeRequests({ count: 1 });
describe('statuses', () => {
const mergeRequest = createMergeRequests({ count: 1 });
beforeEach(() => {
wrapper = createComponent(mergeRequest, drawerEnabled);
});
beforeEach(() => {
wrapper = createComponent(mergeRequest);
});
it('passes the correct props to the statuses', () => {
findStatuses().wrappers.forEach((status) => {
const { type, data } = status.props('status');
it('passes the correct props to the statuses', () => {
findStatuses().wrappers.forEach((status) => {
const { type, data } = status.props('status');
switch (type) {
case 'pipeline':
expect(data).toEqual(mergeRequest[0].pipeline_status);
break;
switch (type) {
case 'pipeline':
expect(data).toEqual(mergeRequest[0].pipeline_status);
break;
case 'approval':
expect(data).toEqual(mergeRequest[0].approval_status);
break;
case 'approval':
expect(data).toEqual(mergeRequest[0].approval_status);
break;
default:
throw new Error('Unknown status type');
}
});
default:
throw new Error('Unknown status type');
}
});
});
});
describe('branch details', () => {
it('does not render if there are no branch details', () => {
wrapper = createComponent(createMergeRequests({ count: 2, props: {} }), drawerEnabled);
describe('branch details', () => {
it('does not render if there are no branch details', () => {
wrapper = createComponent(createMergeRequests({ count: 2, props: {} }));
expect(findBranchDetails().exists()).toBe(false);
});
expect(findBranchDetails().exists()).toBe(false);
});
it('renders if there are branch details', () => {
wrapper = createComponent(
createMergeRequests({
count: 2,
props: { target_branch: 'main', source_branch: 'feature' },
}),
drawerEnabled,
);
it('renders if there are branch details', () => {
wrapper = createComponent(
createMergeRequests({
count: 2,
props: { target_branch: 'main', source_branch: 'feature' },
}),
);
expect(findBranchDetails().exists()).toBe(true);
});
expect(findBranchDetails().exists()).toBe(true);
});
});
describe('when the drawer is enabled', () => {
const mergeRequests = createMergeRequests({ count: 2, props: {} });
describe.each(['click', 'keypress.enter'])('when the %s event is triggered', (event) => {
const mergeRequest = createMergeRequests({ count: 1 });
beforeEach(() => {
const mergeRequest = createMergeRequests({ count: 1 });
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', () => {
findMergeRequestDrawerToggles().at(0).trigger(event);
it('toggles the drawer when a merge request drawer toggle is the target', () => {
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', () => {
findMergeRequestLinks().at(0).trigger(event);
it('does not toggle the drawer if an inner link is the target', () => {
findMergeRequestLinks().at(0).trigger(event);
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7