Commit cfdf5c3b authored by Fernando's avatar Fernando

Move computed props to parent component

* Move rendering logic up a level
* Additional typo and fixes
parent 8e11f9a0
......@@ -35,10 +35,14 @@ class ProjectsController < Projects::ApplicationController
before_action :export_rate_limit, only: [:export, :download_export, :generate_new_export]
# Experiments
before_action only: [:new, :create, :edit] do
before_action only: [:new, :create] do
frontend_experimentation_tracking_data(:new_create_project_ui, 'click_tab')
push_frontend_feature_flag(:new_create_project_ui) if experiment_enabled?(:new_create_project_ui)
push_frontend_feature_flag(:service_desk_custom_address, @project)
end
before_action only: [:edit] do
push_frontend_feature_flag(:approval_suggestions, @project)
end
......
......@@ -30,7 +30,7 @@ export default {
: __('Update approval rule');
},
defaultRuleName() {
return this.rule && this.rule.defaultRuleName;
return this.rule?.defaultRuleName;
},
},
methods: {
......
<script>
import { camelCase } from 'lodash';
import { GlButton, GlLink, GlSprintf, GlSkeletonLoading } from '@gitlab/ui';
import { JOB_TYPES } from 'ee/approvals/constants';
import { GlButton, GlLink, GlSprintf } from '@gitlab/ui';
export default {
components: {
GlButton,
GlLink,
GlSprintf,
GlSkeletonLoading,
},
featureTypes: {
vulnerabilityCheck: [
JOB_TYPES.SAST,
JOB_TYPES.DAST,
JOB_TYPES.DEPENDENCY_SCANNING,
JOB_TYPES.SECRET_DETECTION,
JOB_TYPES.COVERAGE_FUZZING,
],
licenseCheck: [JOB_TYPES.LICENSE_SCANNING],
},
props: {
configuration: {
type: Object,
required: true,
},
rules: {
type: Array,
required: true,
},
matchRule: {
rule: {
type: Object,
required: true,
},
isLoading: {
type: Boolean,
required: true,
},
},
computed: {
hasApprovalRuleDefined() {
return this.rules.some(rule => {
return this.matchRule.name === rule.name;
});
},
hasConfiguredJob() {
const { features = [] } = this.configuration;
return this.$options.featureTypes[camelCase(this.matchRule.name)].some(featureType => {
return features.some(feature => {
return feature.type === featureType && feature.configured;
});
});
},
},
};
</script>
<template>
<!--
Excessive conditional logic is due to:
- Can't create wrapper <div> for conditional rendering
because parent component is a table and expects a root <tr> element
- Can't have multiple root <tr> nodes
- Can't create wrapper <div> since <tr> expects a <td> child element
- Root element can't be another <template>
-->
<tr v-if="!hasApprovalRuleDefined || !hasConfiguredJob">
<td v-if="isLoading" colspan="3">
<gl-skeleton-loading :lines="3" />
</td>
<template v-else>
<!-- Suggested approval rule creation row -->
<template v-if="hasConfiguredJob">
<td class="js-name" colspan="4">
<div>{{ matchRule.name }}</div>
<div class="gl-text-gray-500">
<gl-sprintf :message="matchRule.enableDescription">
<template #link="{ content }">
<gl-link :href="matchRule.docsPath" target="_blank">{{ content }}</gl-link>
</template>
</gl-sprintf>
</div>
</td>
<td class="gl-px-2! gl-text-right">
<gl-button @click="$emit('enable')">
{{ s__('Enable') }}
</gl-button>
</td>
</template>
<!-- Approval rule suggestion when lacking appropriate CI job for the rule -->
<td v-else class="js-name" colspan="5">
<div>{{ matchRule.name }}</div>
<tr>
<!-- Suggested approval rule creation row -->
<template v-if="rule.hasConfiguredJob">
<td class="js-name" colspan="4">
<div>{{ rule.name }}</div>
<div class="gl-text-gray-500">
<gl-sprintf :message="matchRule.description">
<gl-sprintf :message="rule.enableDescription">
<template #link="{ content }">
<gl-link :href="matchRule.docsPath" target="_blank">{{ content }}</gl-link>
<gl-link :href="rule.docsPath" target="_blank">{{ content }}</gl-link>
</template>
</gl-sprintf>
</div>
</td>
<td class="gl-px-2! gl-text-right">
<gl-button @click="$emit('enable')">
{{ s__('Enable') }}
</gl-button>
</td>
</template>
<!-- Approval rule suggestion when lacking appropriate CI job for the rule -->
<td v-else class="js-name" colspan="5">
<div>{{ rule.name }}</div>
<div class="gl-text-gray-500">
<gl-sprintf :message="rule.description">
<template #link="{ content }">
<gl-link :href="rule.docsPath" target="_blank">{{ content }}</gl-link>
</template>
</gl-sprintf>
</div>
</td>
</tr>
</template>
<script>
import { camelCase } from 'lodash';
import { mapState, mapActions } from 'vuex';
import { LICENSE_CHECK_NAME, VULNERABILITY_CHECK_NAME } from 'ee/approvals/constants';
import { GlSkeletonLoading } from '@gitlab/ui';
import { LICENSE_CHECK_NAME, VULNERABILITY_CHECK_NAME, JOB_TYPES } from 'ee/approvals/constants';
import { s__ } from '~/locale';
import UnconfiguredSecurityRule from './unconfigured_security_rule.vue';
export default {
components: {
UnconfiguredSecurityRule,
GlSkeletonLoading,
},
props: {},
inject: {
vulnerabilityCheckHelpPagePath: {
from: 'vulnerabilityCheckHelpPagePath',
......@@ -19,6 +21,16 @@ export default {
default: '',
},
},
featureTypes: {
vulnerabilityCheck: [
JOB_TYPES.SAST,
JOB_TYPES.DAST,
JOB_TYPES.DEPENDENCY_SCANNING,
JOB_TYPES.SECRET_DETECTION,
JOB_TYPES.COVERAGE_FUZZING,
],
licenseCheck: [JOB_TYPES.LICENSE_SCANNING],
},
computed: {
...mapState('securityConfiguration', ['configuration']),
...mapState({
......@@ -53,6 +65,17 @@ export default {
},
];
},
unconfiguredRules() {
return this.securityRules.reduce((filtered, securityRule) => {
const hasApprovalRuleDefined = this.hasApprovalRuleDefined(securityRule);
const hasConfiguredJob = this.hasConfiguredJob(securityRule);
if (!hasApprovalRuleDefined || !hasConfiguredJob) {
filtered.push({ ...securityRule, hasConfiguredJob });
}
return filtered;
}, []);
},
},
created() {
this.fetchSecurityConfiguration();
......@@ -60,6 +83,19 @@ export default {
methods: {
...mapActions('securityConfiguration', ['fetchSecurityConfiguration']),
...mapActions({ openCreateModal: 'createModal/open' }),
hasApprovalRuleDefined(matchRule) {
return this.rules.some(rule => {
return matchRule.name === rule.name;
});
},
hasConfiguredJob(matchRule) {
const { features = [] } = this.configuration;
return this.$options.featureTypes[camelCase(matchRule.name)].some(featureType => {
return features.some(feature => {
return feature.type === featureType && feature.configured;
});
});
},
},
};
</script>
......@@ -67,14 +103,18 @@ export default {
<template>
<table class="table m-0">
<tbody>
<tr v-if="isRulesLoading">
<td colspan="3">
<gl-skeleton-loading :lines="3" />
</td>
</tr>
<unconfigured-security-rule
v-for="securityRule in securityRules"
:key="securityRule.name"
:configuration="configuration"
:rules="rules"
:is-loading="isRulesLoading"
:match-rule="securityRule"
@enable="openCreateModal({ defaultRuleName: securityRule.name })"
v-for="rule in unconfiguredRules"
v-else
:key="rule.name"
:rule="rule"
@enable="openCreateModal({ defaultRuleName: rule.name })"
/>
</tbody>
</table>
......
......@@ -124,7 +124,7 @@ describe('Approvals ProjectRules', () => {
expect(nameCell.find('.js-help').exists()).toBeFalsy();
});
it('should not render the unconfigured-security-rule component', () => {
it('should not render the unconfigured-security-rules component', () => {
expect(wrapper.contains(UnconfiguredSecurityRules)).toBe(false);
});
});
......@@ -148,7 +148,7 @@ describe('Approvals ProjectRules', () => {
);
});
it('should render the unconfigured-security-rule component', () => {
it('should render the unconfigured-security-rules component', () => {
expect(wrapper.contains(UnconfiguredSecurityRules)).toBe(true);
});
});
......@@ -172,7 +172,7 @@ describe('Approvals ProjectRules', () => {
);
});
it('should notrender the unconfigured-security-rule component', () => {
it('should not render the unconfigured-security-rule component', () => {
expect(wrapper.contains(UnconfiguredSecurityRules)).toBe(false);
});
});
......
......@@ -2,7 +2,7 @@ import Vuex from 'vuex';
import { LICENSE_CHECK_NAME, VULNERABILITY_CHECK_NAME } from 'ee/approvals/constants';
import UnconfiguredSecurityRule from 'ee/approvals/components/security_configuration/unconfigured_security_rule.vue';
import { mount, createLocalVue } from '@vue/test-utils';
import { GlSkeletonLoading, GlSprintf, GlButton } from '@gitlab/ui';
import { GlSprintf, GlButton } from '@gitlab/ui';
const localVue = createLocalVue();
localVue.use(Vuex);
......@@ -14,80 +14,20 @@ describe('UnconfiguredSecurityRule component', () => {
const findDescription = () => wrapper.find(GlSprintf);
const findButton = () => wrapper.find(GlButton);
const vulnCheckMatchRule = {
const vulnCheckRule = {
name: VULNERABILITY_CHECK_NAME,
description: 'vuln-check description without enable button',
enableDescription: 'vuln-check description with enable button',
docsPath: 'docs/vuln-check',
};
const licenseCheckMatchRule = {
const licenseCheckRule = {
name: LICENSE_CHECK_NAME,
description: 'license-check description without enable button',
enableDescription: 'license-check description with enable button',
docsPath: 'docs/license-check',
};
const licenseCheckRule = {
name: LICENSE_CHECK_NAME,
};
const vulnCheckRule = {
name: VULNERABILITY_CHECK_NAME,
};
const features = [
{
type: 'sast',
configured: true,
description: 'Analyze your source code for known vulnerabilities.',
link: '/help/user/application_security/sast/index',
name: 'Static Application Security Testing (SAST)',
},
{
type: 'dast',
configured: false,
description: 'Analyze a review version of your web application.',
link: '/help/user/application_security/dast/index',
name: 'Dynamic Application Security Testing (DAST)',
},
{
type: 'dependency_scanning',
configured: true,
description: 'Analyze your dependencies for known vulnerabilities.',
link: '/help/user/application_security/dependency_scanning/index',
name: 'Dependency Scanning',
},
{
type: 'container_scanning',
configured: true,
description: 'Check your Docker images for known vulnerabilities.',
link: '/help/user/application_security/container_scanning/index',
name: 'Container Scanning',
},
{
type: 'secret_detection',
configured: false,
description: 'Analyze your source code and git history for secrets.',
link: '/help/user/application_security/secret_detection/index',
name: 'Secret Detection',
},
{
type: 'coverage_fuzzing',
configured: false,
description: 'Find bugs in your code with coverage-guided fuzzing',
link: '/help/user/application_security/coverage_fuzzing/index',
name: 'Coverage Fuzzing',
},
{
type: 'license_scanning',
configured: false,
description: 'Search your project dependencies for their licenses and apply policies.',
link: '/help/user/compliance/license_compliance/index',
name: 'License Compliance',
},
];
const createWrapper = (props = {}, options = {}) => {
wrapper = mount(UnconfiguredSecurityRule, {
localVue,
......@@ -103,123 +43,46 @@ describe('UnconfiguredSecurityRule component', () => {
wrapper = null;
});
describe('while loading', () => {
describe.each`
rule | ruleName | descriptionText
${licenseCheckRule} | ${licenseCheckRule.name} | ${licenseCheckRule.enableDescription}
${vulnCheckRule} | ${vulnCheckRule.name} | ${vulnCheckRule.enableDescription}
`('with a configured job that is eligible for $ruleName', ({ rule, descriptionText }) => {
beforeEach(() => {
createWrapper({
rules: [],
configuration: {},
matchRule: vulnCheckMatchRule,
isLoading: true,
rule: { ...rule, hasConfiguredJob: true },
});
description = findDescription();
});
it('should render the loading skeleton', () => {
expect(wrapper.contains(GlSkeletonLoading)).toBe(true);
it('should render the row with the enable decription and enable button', () => {
expect(description.exists()).toBe(true);
expect(description.text()).toBe(descriptionText);
expect(findButton().exists()).toBe(true);
});
});
describe('with a configured job that is eligable for Vulnerability-Check', () => {
describe('with a Vulnerability-Check rule defined', () => {
beforeEach(() => {
createWrapper({
rules: [vulnCheckRule],
configuration: { features },
matchRule: vulnCheckMatchRule,
isLoading: false,
});
});
it('should not render the loading skeleton', () => {
expect(wrapper.contains(GlSkeletonLoading)).toBe(false);
});
it('should not render the row', () => {
expect(wrapper.find('tr').exists()).toBe(false);
});
});
describe('without a Vulnerability-Check rule defined', () => {
let enableButtonHandler;
beforeEach(() => {
enableButtonHandler = jest.fn();
createWrapper(
{
rules: [],
configuration: { features },
matchRule: vulnCheckMatchRule,
isLoading: false,
},
{
listeners: {
enable: enableButtonHandler,
},
},
);
description = findDescription();
});
it('should not render the loading skeleton', () => {
expect(wrapper.contains(GlSkeletonLoading)).toBe(false);
});
it('should render the row with the enable decription and enable button', () => {
expect(description.exists()).toBe(true);
expect(description.text()).toBe(vulnCheckMatchRule.enableDescription);
expect(findButton().exists()).toBe(true);
});
it('should emit the "enable" event when the button is clicked', () => {
findButton().trigger('click');
expect(enableButtonHandler).toHaveBeenCalled();
});
it('should emit the "enable" event when the button is clicked', () => {
findButton().trigger('click');
expect(wrapper.emitted('enable')).toEqual([[]]);
});
});
describe('with a unconfigured job that is eligable for License-Check', () => {
describe('with a License-Check rule defined', () => {
beforeEach(() => {
createWrapper({
rules: [licenseCheckRule],
configuration: { features },
matchRule: licenseCheckMatchRule,
isLoading: false,
});
description = findDescription();
});
it('should not render the loading skeleton', () => {
expect(wrapper.contains(GlSkeletonLoading)).toBe(false);
});
it('should render the row with the decription and no button', () => {
expect(description.exists()).toBe(true);
expect(description.text()).toBe(licenseCheckMatchRule.description);
expect(findButton().exists()).toBe(false);
describe.each`
rule | ruleName | descriptionText
${licenseCheckRule} | ${licenseCheckRule.name} | ${licenseCheckRule.description}
${vulnCheckRule} | ${vulnCheckRule.name} | ${vulnCheckRule.description}
`('with a unconfigured job that is eligible for $ruleName', ({ rule, descriptionText }) => {
beforeEach(() => {
createWrapper({
rule: { ...rule, hasConfiguredJob: false },
});
description = findDescription();
});
describe('without a License-Check rule defined', () => {
beforeEach(() => {
createWrapper({
rules: [],
configuration: { features },
matchRule: licenseCheckMatchRule,
isLoading: false,
});
description = findDescription();
});
it('should not render the loading skeleton', () => {
expect(wrapper.contains(GlSkeletonLoading)).toBe(false);
});
it('should render the row with the decription and no button', () => {
expect(description.exists()).toBe(true);
expect(description.text()).toBe(licenseCheckMatchRule.description);
expect(findButton().exists()).toBe(false);
});
it('should render the row with the decription and no button', () => {
expect(description.exists()).toBe(true);
expect(description.text()).toBe(descriptionText);
expect(findButton().exists()).toBe(false);
});
});
});
import { shallowMount, createLocalVue } from '@vue/test-utils';
import Vuex from 'vuex';
import { GlSkeletonLoading } from '@gitlab/ui';
import UnconfiguredSecurityRules from 'ee/approvals/components/security_configuration/unconfigured_security_rules.vue';
import UnconfiguredSecurityRule from 'ee/approvals/components/security_configuration/unconfigured_security_rule.vue';
import { createStoreOptions } from 'ee/approvals/stores';
import projectSettingsModule from 'ee/approvals/stores/modules/project_settings';
import { shallowMount, createLocalVue } from '@vue/test-utils';
const localVue = createLocalVue();
localVue.use(Vuex);
......@@ -11,22 +12,16 @@ localVue.use(Vuex);
describe('UnconfiguredSecurityRules component', () => {
let wrapper;
let store;
let mockFetch;
const TEST_PROJECT_ID = '7';
const createWrapper = (props = {}) => {
mockFetch = jest.fn();
wrapper = shallowMount(UnconfiguredSecurityRules, {
localVue,
store,
propsData: {
...props,
},
methods: {
fetchSecurityConfiguration: mockFetch,
},
provide: {
vulnerabilityCheckHelpPagePath: '',
licenseCheckHelpPagePath: '',
......@@ -38,6 +33,7 @@ describe('UnconfiguredSecurityRules component', () => {
store = new Vuex.Store(
createStoreOptions(projectSettingsModule(), { projectId: TEST_PROJECT_ID }),
);
jest.spyOn(store, 'dispatch');
});
afterEach(() => {
......@@ -51,11 +47,30 @@ describe('UnconfiguredSecurityRules component', () => {
});
it('should fetch the security configuration', () => {
expect(mockFetch).toHaveBeenCalled();
expect(store.dispatch).toHaveBeenCalledWith('securityConfiguration/fetchSecurityConfiguration', undefined);
});
it('should render a unconfigured-security-rule component for every security rule ', () => {
expect(wrapper.findAll(UnconfiguredSecurityRule).length).toBe(2);
});
});
describe.each`
approvalsLoading | securityConfigurationLoading | shouldRender
${false} | ${false} | ${false}
${true} | ${false} | ${true}
${false} | ${true} | ${true}
${true} | ${true} | ${true}
`
('while approvalsLoading is $approvalsLoading and securityConfigurationLoading is $securityConfigurationLoading', ({approvalsLoading, securityConfigurationLoading, shouldRender}) => {
beforeEach(() => {
createWrapper();
store.state.approvals.isLoading = approvalsLoading;
store.state.securityConfiguration.isLoading = securityConfigurationLoading;
});
it(`should render the loading skeleton is ${shouldRender}`, () => {
expect(wrapper.contains(GlSkeletonLoading)).toBe(shouldRender);
});
});
});
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