Commit efce11f0 authored by Max Woolf's avatar Max Woolf

Removes compliance framework administration abilities from subgroups

Currently subgroup owners can attempt to edit or create
compliance frameworks but this raises two issues.

* Compliance frameworks are always added to the root ancestor anyway.
* Subgroup owners aren't always root ancestor owners.

This commit resolves these issues by hiding the add/edit/delete
actions from all users on the subgroup settings page.
parent 4daa93d9
...@@ -26,11 +26,13 @@ export default { ...@@ -26,11 +26,13 @@ export default {
props: { props: {
addFrameworkPath: { addFrameworkPath: {
type: String, type: String,
required: true, required: false,
default: null,
}, },
editFrameworkPath: { editFrameworkPath: {
type: String, type: String,
required: true, required: false,
default: null,
}, },
emptyStateSvgPath: { emptyStateSvgPath: {
type: String, type: String,
...@@ -177,6 +179,7 @@ export default { ...@@ -177,6 +179,7 @@ export default {
<gl-tab disabled :title="$options.i18n.regulatedTab" /> <gl-tab disabled :title="$options.i18n.regulatedTab" />
<template #tabs-end> <template #tabs-end>
<gl-button <gl-button
v-if="addFrameworkPath"
class="gl-align-self-center gl-ml-auto" class="gl-align-self-center gl-ml-auto"
category="primary" category="primary"
variant="confirm" variant="confirm"
......
...@@ -9,17 +9,19 @@ export default { ...@@ -9,17 +9,19 @@ export default {
props: { props: {
imagePath: { imagePath: {
type: String, type: String,
required: true, required: false,
default: null,
}, },
addFrameworkPath: { addFrameworkPath: {
type: String, type: String,
required: true, required: false,
default: null,
}, },
}, },
i18n: { i18n: {
heading: s__('ComplianceFrameworks|There are no compliance frameworks set up yet'), heading: s__('ComplianceFrameworks|There are no compliance frameworks set up yet'),
description: s__( description: s__(
'ComplianceFrameworks|Once you have created a compliance framework it will appear here.', 'ComplianceFrameworks|Once a compliance framework is added it will appear here.',
), ),
addButton: s__('ComplianceFrameworks|Add framework'), addButton: s__('ComplianceFrameworks|Add framework'),
}, },
......
...@@ -47,7 +47,7 @@ export default { ...@@ -47,7 +47,7 @@ export default {
<p class="gl-w-full gl-m-0!" data-testid="compliance-framework-description"> <p class="gl-w-full gl-m-0!" data-testid="compliance-framework-description">
{{ framework.description }} {{ framework.description }}
</p> </p>
<div class="gl-display-flex"> <div v-if="framework.editPath" class="gl-display-flex">
<gl-button <gl-button
v-gl-tooltip="$options.i18n.editFramework" v-gl-tooltip="$options.i18n.editFramework"
:disabled="loading" :disabled="loading"
......
...@@ -4,7 +4,7 @@ import { isNumeric } from '~/lib/utils/number_utils'; ...@@ -4,7 +4,7 @@ import { isNumeric } from '~/lib/utils/number_utils';
import { EDIT_PATH_ID_FORMAT, PIPELINE_CONFIGURATION_PATH_FORMAT } from './constants'; import { EDIT_PATH_ID_FORMAT, PIPELINE_CONFIGURATION_PATH_FORMAT } from './constants';
export const injectIdIntoEditPath = (path, id) => { export const injectIdIntoEditPath = (path, id) => {
if (!path.match(EDIT_PATH_ID_FORMAT) || !isNumeric(id)) { if (!path || !path.match(EDIT_PATH_ID_FORMAT) || !isNumeric(id)) {
return ''; return '';
} }
......
...@@ -8,18 +8,18 @@ module ComplianceManagement ...@@ -8,18 +8,18 @@ module ComplianceManagement
end end
def compliance_frameworks_list_data(group) def compliance_frameworks_list_data(group)
{ {}.tap do |data|
empty_state_svg_path: image_path('illustrations/welcome/ee_trial.svg'), data[:empty_state_svg_path] = image_path('illustrations/welcome/ee_trial.svg')
group_path: group.full_path, data[:group_path] = group.root_ancestor.full_path
add_framework_path: new_group_compliance_framework_path(group), data[:add_framework_path] = new_group_compliance_framework_path(group) unless group.subgroup?
edit_framework_path: edit_group_compliance_framework_path(group, :id) data[:edit_framework_path] = edit_group_compliance_framework_path(group, :id) unless group.subgroup?
} end
end end
def compliance_frameworks_form_data(group, framework_id = nil) def compliance_frameworks_form_data(group, framework_id = nil)
{ {
framework_id: framework_id, framework_id: framework_id,
group_path: group.full_path, group_path: group.root_ancestor.full_path,
group_edit_path: edit_group_path(group, anchor: 'js-compliance-frameworks-settings'), group_edit_path: edit_group_path(group, anchor: 'js-compliance-frameworks-settings'),
graphql_field_name: ComplianceManagement::Framework.name, graphql_field_name: ComplianceManagement::Framework.name,
pipeline_configuration_full_path_enabled: pipeline_configuration_full_path_enabled?(group).to_s pipeline_configuration_full_path_enabled: pipeline_configuration_full_path_enabled?(group).to_s
......
---
title: Remove compliance framework administration abilities from subgroups
merge_request: 58873
author:
type: changed
...@@ -26,7 +26,7 @@ describe('ListEmptyState', () => { ...@@ -26,7 +26,7 @@ describe('ListEmptyState', () => {
expect(findEmptyState().props()).toMatchObject({ expect(findEmptyState().props()).toMatchObject({
title: 'There are no compliance frameworks set up yet', title: 'There are no compliance frameworks set up yet',
description: 'Once you have created a compliance framework it will appear here.', description: 'Once a compliance framework is added it will appear here.',
svgPath: 'dir/image.svg', svgPath: 'dir/image.svg',
primaryButtonLink: 'group/framework/new', primaryButtonLink: 'group/framework/new',
primaryButtonText: 'Add framework', primaryButtonText: 'Add framework',
......
...@@ -19,6 +19,7 @@ describe('ListItem', () => { ...@@ -19,6 +19,7 @@ describe('ListItem', () => {
color: '#112233', color: '#112233',
editPath: 'group/framework/1/edit', editPath: 'group/framework/1/edit',
}; };
const findLabel = () => wrapper.findComponent(GlLabel); const findLabel = () => wrapper.findComponent(GlLabel);
const findDescription = () => wrapper.findByTestId('compliance-framework-description'); const findDescription = () => wrapper.findByTestId('compliance-framework-description');
const findEditButton = () => wrapper.findByTestId('compliance-framework-edit-button'); const findEditButton = () => wrapper.findByTestId('compliance-framework-edit-button');
...@@ -50,6 +51,15 @@ describe('ListItem', () => { ...@@ -50,6 +51,15 @@ describe('ListItem', () => {
expect(button.attributes('aria-label')).toBe(ariaLabel); expect(button.attributes('aria-label')).toBe(ariaLabel);
}; };
it('does not show modification buttons when framework is missing paths', () => {
createComponent({
framework: { ...framework, editPath: null },
});
expect(findEditButton().exists()).toBe(false);
expect(findDeleteButton().exists()).toBe(false);
});
it('displays the description defined by the framework', () => { it('displays the description defined by the framework', () => {
createComponent(); createComponent();
...@@ -65,7 +75,9 @@ describe('ListItem', () => { ...@@ -65,7 +75,9 @@ describe('ListItem', () => {
}); });
it('displays the label as scoped', () => { it('displays the label as scoped', () => {
createComponent({ framework: { ...framework, name: 'scoped::framework' } }); createComponent({
framework: { ...framework, name: 'scoped::framework' },
});
expect(findLabel().props('title')).toBe('scoped::framework'); expect(findLabel().props('title')).toBe('scoped::framework');
expect(findLabel().props('target')).toBe(framework.editPath); expect(findLabel().props('target')).toBe(framework.editPath);
......
...@@ -12,7 +12,6 @@ import { PIPELINE_CONFIGURATION_PATH_FORMAT } from 'ee/groups/settings/complianc ...@@ -12,7 +12,6 @@ import { PIPELINE_CONFIGURATION_PATH_FORMAT } from 'ee/groups/settings/complianc
import getComplianceFrameworkQuery from 'ee/groups/settings/compliance_frameworks/graphql/queries/get_compliance_framework.query.graphql'; import getComplianceFrameworkQuery from 'ee/groups/settings/compliance_frameworks/graphql/queries/get_compliance_framework.query.graphql';
import createMockApollo from 'helpers/mock_apollo_helper'; import createMockApollo from 'helpers/mock_apollo_helper';
import waitForPromises from 'helpers/wait_for_promises'; import waitForPromises from 'helpers/wait_for_promises';
import { validFetchResponse, emptyFetchResponse } from '../mock_data'; import { validFetchResponse, emptyFetchResponse } from '../mock_data';
const localVue = createLocalVue(); const localVue = createLocalVue();
...@@ -44,7 +43,7 @@ describe('List', () => { ...@@ -44,7 +43,7 @@ describe('List', () => {
return createMockApollo(requestHandlers); return createMockApollo(requestHandlers);
} }
function createComponentWithApollo(resolverMock) { function createComponentWithApollo(resolverMock, props = {}) {
return shallowMount(List, { return shallowMount(List, {
localVue, localVue,
apolloProvider: createMockApolloProvider(resolverMock), apolloProvider: createMockApolloProvider(resolverMock),
...@@ -53,6 +52,7 @@ describe('List', () => { ...@@ -53,6 +52,7 @@ describe('List', () => {
editFrameworkPath: 'group/framework/id/edit', editFrameworkPath: 'group/framework/id/edit',
emptyStateSvgPath: 'dir/image.svg', emptyStateSvgPath: 'dir/image.svg',
groupPath: 'group-1', groupPath: 'group-1',
...props,
}, },
stubs: { stubs: {
GlLoadingIcon, GlLoadingIcon,
...@@ -193,6 +193,19 @@ describe('List', () => { ...@@ -193,6 +193,19 @@ describe('List', () => {
it('renders the delete modal', () => { it('renders the delete modal', () => {
expect(findDeleteModal().exists()).toBe(true); expect(findDeleteModal().exists()).toBe(true);
}); });
describe('when no paths are provided', () => {
beforeEach(() => {
wrapper = createComponentWithApollo(fetch, {
addFrameworkPath: null,
editFrameworkPath: null,
});
});
it('does not show the add framework button', () => {
expect(findAddBtn().exists()).toBe(false);
});
});
}); });
describe('delete framework', () => { describe('delete framework', () => {
......
...@@ -22,15 +22,23 @@ RSpec.describe ComplianceManagement::ComplianceFramework::GroupSettingsHelper do ...@@ -22,15 +22,23 @@ RSpec.describe ComplianceManagement::ComplianceFramework::GroupSettingsHelper do
end end
context 'the user does not have permission' do context 'the user does not have permission' do
before do context 'group is not a subgroup' do
allow(helper).to receive(:can?).with(current_user, :admin_compliance_framework, group).and_return(false) before do
end allow(helper).to receive(:can?).with(current_user, :admin_compliance_framework, group).and_return(false)
end
it { is_expected.to be false } it { is_expected.to be false }
end
end end
end end
describe '#compliance_frameworks_list_data' do describe '#compliance_frameworks_list_data' do
subject { helper.compliance_frameworks_list_data(group) }
before do
allow(helper).to receive(:can?).with(current_user, :admin_compliance_framework, group).and_return(true)
end
it 'returns the correct data' do it 'returns the correct data' do
expect(helper.compliance_frameworks_list_data(group)).to contain_exactly( expect(helper.compliance_frameworks_list_data(group)).to contain_exactly(
[:empty_state_svg_path, ActionController::Base.helpers.image_path('illustrations/welcome/ee_trial.svg')], [:empty_state_svg_path, ActionController::Base.helpers.image_path('illustrations/welcome/ee_trial.svg')],
...@@ -39,6 +47,19 @@ RSpec.describe ComplianceManagement::ComplianceFramework::GroupSettingsHelper do ...@@ -39,6 +47,19 @@ RSpec.describe ComplianceManagement::ComplianceFramework::GroupSettingsHelper do
[:edit_framework_path, edit_group_compliance_framework_path(group, :id)] [:edit_framework_path, edit_group_compliance_framework_path(group, :id)]
) )
end end
context 'group is a subgroup' do
let_it_be(:group) { create(:group, :nested) }
it 'contains the root ancestor as group_path' do
expect(subject[:group_path]).to eq(group.root_ancestor.full_path)
end
it 'does not contain the add_framework_path or edit_framework_path keys' do
expect(subject.keys).not_to include('add_framework_path')
expect(subject.keys).not_to include('edit_framework_path')
end
end
end end
describe '#compliance_frameworks_form_data' do describe '#compliance_frameworks_form_data' do
...@@ -83,5 +104,15 @@ RSpec.describe ComplianceManagement::ComplianceFramework::GroupSettingsHelper do ...@@ -83,5 +104,15 @@ RSpec.describe ComplianceManagement::ComplianceFramework::GroupSettingsHelper do
context 'the user does not have pipeline configuration permission' do context 'the user does not have pipeline configuration permission' do
it_behaves_like 'returns the correct data', [false] it_behaves_like 'returns the correct data', [false]
end end
context 'group is a subgroup' do
let_it_be(:group) { create(:group, :nested) }
it 'returns the root ancestor full path as group_path' do
allow(helper).to receive(:can?).with(current_user, :admin_compliance_pipeline_configuration, group).and_return(true)
expect(subject[:group_path]).to eq(group.root_ancestor.full_path)
end
end
end end
end end
...@@ -8021,7 +8021,7 @@ msgstr "" ...@@ -8021,7 +8021,7 @@ msgstr ""
msgid "ComplianceFrameworks|Invalid format: it should follow the format [PATH].y(a)ml@[GROUP]/[PROJECT]" msgid "ComplianceFrameworks|Invalid format: it should follow the format [PATH].y(a)ml@[GROUP]/[PROJECT]"
msgstr "" msgstr ""
msgid "ComplianceFrameworks|Once you have created a compliance framework it will appear here." msgid "ComplianceFrameworks|Once a compliance framework is added it will appear here."
msgstr "" msgstr ""
msgid "ComplianceFrameworks|Regulated" msgid "ComplianceFrameworks|Regulated"
......
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