Commit eb78ff48 authored by Mark Florian's avatar Mark Florian Committed by Sean McGivern

Implement SAST Configuration form submission

Part of the [SAST Configuration UI][epic] feature, this implements form
submission behaviour, via a regular POST request. A future iteration
will replace this with a [GraphQL mutation][mutation].

Addresses https://gitlab.com/gitlab-org/gitlab/-/issues/223879.

[epic]: https://gitlab.com/groups/gitlab-org/-/epics/3659
[mutation]: https://gitlab.com/gitlab-org/gitlab/-/issues/227575
parent 20c4464c
...@@ -2,12 +2,12 @@ ...@@ -2,12 +2,12 @@
import { GlAlert, GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui'; import { GlAlert, GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui';
import { s__ } from '~/locale'; import { s__ } from '~/locale';
import sastCiConfigurationQuery from '../graphql/sast_ci_configuration.query.graphql'; import sastCiConfigurationQuery from '../graphql/sast_ci_configuration.query.graphql';
import DynamicFields from './dynamic_fields.vue'; import ConfigurationForm from './configuration_form.vue';
import { extractSastConfigurationEntities } from './utils'; import { extractSastConfigurationEntities } from './utils';
export default { export default {
components: { components: {
DynamicFields, ConfigurationForm,
GlAlert, GlAlert,
GlLink, GlLink,
GlLoadingIcon, GlLoadingIcon,
...@@ -89,6 +89,6 @@ export default { ...@@ -89,6 +89,6 @@ export default {
$options.i18n.loadingErrorText $options.i18n.loadingErrorText
}}</gl-alert> }}</gl-alert>
<dynamic-fields v-else v-model="sastConfigurationEntities" /> <configuration-form v-else :entities="sastConfigurationEntities" />
</article> </article>
</template> </template>
<script>
import { GlAlert, GlButton } from '@gitlab/ui';
import * as Sentry from '@sentry/browser';
import { cloneDeep } from 'lodash';
import { __, s__ } from '~/locale';
import axios from '~/lib/utils/axios_utils';
import { redirectTo } from '~/lib/utils/url_utility';
import DynamicFields from './dynamic_fields.vue';
import { isValidConfigurationEntity } from './utils';
export default {
components: {
DynamicFields,
GlAlert,
GlButton,
},
inject: {
createSastMergeRequestPath: {
from: 'createSastMergeRequestPath',
default: '',
},
securityConfigurationPath: {
from: 'securityConfigurationPath',
default: '',
},
},
props: {
entities: {
type: Array,
required: true,
validator: value => value.every(isValidConfigurationEntity),
},
},
data() {
return {
formEntities: cloneDeep(this.entities),
hasSubmissionError: false,
isSubmitting: false,
};
},
methods: {
onSubmit() {
this.isSubmitting = true;
this.hasSubmissionError = false;
return axios
.post(this.createSastMergeRequestPath, this.getFormData())
.then(({ data }) => {
const { filePath } = data;
if (!filePath) {
// eslint-disable-next-line @gitlab/require-i18n-strings
throw new Error('SAST merge request creation failed');
}
redirectTo(filePath);
})
.catch(error => {
this.isSubmitting = false;
this.hasSubmissionError = true;
Sentry.captureException(error);
});
},
getFormData() {
return this.formEntities.reduce((acc, { field, value }) => {
acc[field] = value;
return acc;
}, {});
},
},
i18n: {
submissionError: s__(
'SecurityConfiguration|An error occurred while creating the merge request.',
),
submitButton: s__('SecurityConfiguration|Create Merge Request'),
cancelButton: __('Cancel'),
},
};
</script>
<template>
<form @submit.prevent="onSubmit">
<dynamic-fields v-model="formEntities" />
<hr />
<gl-alert v-if="hasSubmissionError" class="gl-mb-5" variant="danger" :dismissible="false">{{
$options.i18n.submissionError
}}</gl-alert>
<div class="gl-display-flex">
<gl-button
ref="submitButton"
class="gl-mr-3 js-no-auto-disable"
:loading="isSubmitting"
type="submit"
variant="success"
category="primary"
>{{ $options.i18n.submitButton }}</gl-button
>
<gl-button ref="cancelButton" :href="securityConfigurationPath">{{
$options.i18n.cancelButton
}}</gl-button>
</div>
</form>
</template>
...@@ -16,12 +16,19 @@ export default function init() { ...@@ -16,12 +16,19 @@ export default function init() {
defaultClient: createDefaultClient(), defaultClient: createDefaultClient(),
}); });
const { projectPath, sastDocumentationPath } = el.dataset; const {
securityConfigurationPath,
createSastMergeRequestPath,
projectPath,
sastDocumentationPath,
} = el.dataset;
return new Vue({ return new Vue({
el, el,
apolloProvider, apolloProvider,
provide: { provide: {
securityConfigurationPath,
createSastMergeRequestPath,
projectPath, projectPath,
sastDocumentationPath, sastDocumentationPath,
}, },
......
...@@ -3,8 +3,10 @@ ...@@ -3,8 +3,10 @@
module Projects::Security::SastConfigurationHelper module Projects::Security::SastConfigurationHelper
def sast_configuration_data(project) def sast_configuration_data(project)
{ {
create_sast_merge_request_path: project_security_configuration_sast_path(project),
project_path: project.full_path,
sast_documentation_path: help_page_path('user/application_security/sast/index', anchor: 'configuration'), sast_documentation_path: help_page_path('user/application_security/sast/index', anchor: 'configuration'),
project_path: project.full_path security_configuration_path: project_security_configuration_path(project)
} }
end end
end end
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import { GlAlert, GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui'; import { GlAlert, GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui';
import SASTConfigurationApp from 'ee/security_configuration/sast/components/app.vue'; import SASTConfigurationApp from 'ee/security_configuration/sast/components/app.vue';
import DynamicFields from 'ee/security_configuration/sast/components/dynamic_fields.vue'; import ConfigurationForm from 'ee/security_configuration/sast/components/configuration_form.vue';
import { makeEntities } from './helpers'; import { makeEntities } from './helpers';
const sastDocumentationPath = '/help/sast'; const sastDocumentationPath = '/help/sast';
...@@ -39,7 +39,7 @@ describe('SAST Configuration App', () => { ...@@ -39,7 +39,7 @@ describe('SAST Configuration App', () => {
const findHeader = () => wrapper.find('header'); const findHeader = () => wrapper.find('header');
const findSubHeading = () => findHeader().find('p'); const findSubHeading = () => findHeader().find('p');
const findLink = (container = wrapper) => container.find(GlLink); const findLink = (container = wrapper) => container.find(GlLink);
const findDynamicFields = () => wrapper.find(DynamicFields); const findConfigurationForm = () => wrapper.find(ConfigurationForm);
const findLoadingIcon = () => wrapper.find(GlLoadingIcon); const findLoadingIcon = () => wrapper.find(GlLoadingIcon);
const findErrorAlert = () => wrapper.find(GlAlert); const findErrorAlert = () => wrapper.find(GlAlert);
...@@ -75,8 +75,8 @@ describe('SAST Configuration App', () => { ...@@ -75,8 +75,8 @@ describe('SAST Configuration App', () => {
expect(findLoadingIcon().exists()).toBe(true); expect(findLoadingIcon().exists()).toBe(true);
}); });
it('does not display the dynamic fields component', () => { it('does not display the configuration form', () => {
expect(findDynamicFields().exists()).toBe(false); expect(findConfigurationForm().exists()).toBe(false);
}); });
it('does not display an alert message', () => { it('does not display an alert message', () => {
...@@ -95,8 +95,8 @@ describe('SAST Configuration App', () => { ...@@ -95,8 +95,8 @@ describe('SAST Configuration App', () => {
expect(findLoadingIcon().exists()).toBe(false); expect(findLoadingIcon().exists()).toBe(false);
}); });
it('does not display the dynamic fields component', () => { it('does not display the configuration form', () => {
expect(findDynamicFields().exists()).toBe(false); expect(findConfigurationForm().exists()).toBe(false);
}); });
it('displays an alert message', () => { it('displays an alert message', () => {
...@@ -105,9 +105,10 @@ describe('SAST Configuration App', () => { ...@@ -105,9 +105,10 @@ describe('SAST Configuration App', () => {
}); });
describe('when loaded', () => { describe('when loaded', () => {
const entities = makeEntities(3); let entities;
beforeEach(() => { beforeEach(() => {
entities = makeEntities(3);
createComponent({ createComponent({
sastConfigurationEntities: entities, sastConfigurationEntities: entities,
}); });
...@@ -117,29 +118,16 @@ describe('SAST Configuration App', () => { ...@@ -117,29 +118,16 @@ describe('SAST Configuration App', () => {
expect(findLoadingIcon().exists()).toBe(false); expect(findLoadingIcon().exists()).toBe(false);
}); });
it('displays the dynamic fields component', () => { it('displays the configuration form', () => {
const dynamicFields = findDynamicFields(); expect(findConfigurationForm().exists()).toBe(true);
expect(dynamicFields.exists()).toBe(true);
expect(dynamicFields.props('entities')).toBe(entities);
}); });
it('does not display an alert message', () => { it('passes the sastConfigurationEntities to the entities prop', () => {
expect(findErrorAlert().exists()).toBe(false); expect(findConfigurationForm().props('entities')).toBe(entities);
}); });
describe('when the dynamic fields component emits an input event', () => { it('does not display an alert message', () => {
let dynamicFields; expect(findErrorAlert().exists()).toBe(false);
let newEntities;
beforeEach(() => {
dynamicFields = findDynamicFields();
newEntities = makeEntities(3, { value: 'foo' });
dynamicFields.vm.$emit(DynamicFields.model.event, newEntities);
});
it('updates the entities binding', () => {
expect(dynamicFields.props('entities')).toBe(newEntities);
});
}); });
}); });
}); });
import * as Sentry from '@sentry/browser';
import AxiosMockAdapter from 'axios-mock-adapter';
import { GlAlert } from '@gitlab/ui';
import axios from '~/lib/utils/axios_utils';
import { redirectTo } from '~/lib/utils/url_utility';
import waitForPromises from 'helpers/wait_for_promises';
import { shallowMount } from '@vue/test-utils';
import ConfigurationForm from 'ee/security_configuration/sast/components/configuration_form.vue';
import DynamicFields from 'ee/security_configuration/sast/components/dynamic_fields.vue';
import { makeEntities } from './helpers';
jest.mock('~/lib/utils/url_utility', () => ({
redirectTo: jest.fn(),
}));
const createSastMergeRequestPath = '/merge_request/create';
const securityConfigurationPath = '/security/configuration';
const newMergeRequestPath = '/merge_request/new';
describe('ConfigurationForm component', () => {
let wrapper;
let entities;
let axiosMock;
const createComponent = ({ props = {} } = {}) => {
entities = makeEntities(3, { value: 'foo' });
wrapper = shallowMount(ConfigurationForm, {
provide: {
createSastMergeRequestPath,
securityConfigurationPath,
},
propsData: {
entities,
...props,
},
});
};
const findForm = () => wrapper.find('form');
const findSubmitButton = () => wrapper.find({ ref: 'submitButton' });
const findErrorAlert = () => wrapper.find(GlAlert);
const findCancelButton = () => wrapper.find({ ref: 'cancelButton' });
const findDynamicFieldsComponent = () => wrapper.find(DynamicFields);
const expectPayloadForEntities = () => {
const { post } = axiosMock.history;
expect(post).toHaveLength(1);
const postedData = JSON.parse(post[0].data);
entities.forEach(entity => {
expect(postedData[entity.field]).toBe(entity.value);
});
};
beforeEach(() => {
axiosMock = new AxiosMockAdapter(axios);
});
afterEach(() => {
wrapper.destroy();
wrapper = null;
axiosMock.restore();
});
describe('the DynamicFields component', () => {
beforeEach(() => {
createComponent();
});
it('renders', () => {
expect(findDynamicFieldsComponent().exists()).toBe(true);
});
it('recieves a copy of the entities prop', () => {
const entitiesProp = findDynamicFieldsComponent().props('entities');
expect(entitiesProp).not.toBe(entities);
expect(entitiesProp).toEqual(entities);
});
describe('when the dynamic fields component emits an input event', () => {
let dynamicFields;
let newEntities;
beforeEach(() => {
dynamicFields = findDynamicFieldsComponent();
newEntities = makeEntities(3, { value: 'foo' });
dynamicFields.vm.$emit(DynamicFields.model.event, newEntities);
});
it('updates the entities binding', () => {
expect(dynamicFields.props('entities')).toBe(newEntities);
});
});
});
describe('when submitting the form', () => {
beforeEach(() => {
jest.spyOn(Sentry, 'captureException').mockImplementation();
});
describe.each`
context | filePath | statusCode | partialErrorMessage
${'a response error code'} | ${newMergeRequestPath} | ${500} | ${'500'}
${'no filePath'} | ${''} | ${200} | ${/merge request.*fail/}
`(
'given an unsuccessful endpoint response due to $context',
({ filePath, statusCode, partialErrorMessage }) => {
beforeEach(() => {
axiosMock.onPost(createSastMergeRequestPath).replyOnce(statusCode, { filePath });
createComponent();
findForm().trigger('submit');
});
it('includes the value of each entity in the payload', expectPayloadForEntities);
it(`sets the submit button's loading prop to true`, () => {
expect(findSubmitButton().props('loading')).toBe(true);
});
describe('after async tasks', () => {
beforeEach(() => waitForPromises());
it('does not call redirectTo', () => {
expect(redirectTo).not.toHaveBeenCalled();
});
it('displays an alert message', () => {
expect(findErrorAlert().exists()).toBe(true);
});
it('sends the error to Sentry', () => {
expect(Sentry.captureException.mock.calls).toMatchObject([
[{ message: expect.stringMatching(partialErrorMessage) }],
]);
});
it(`sets the submit button's loading prop to false`, () => {
expect(findSubmitButton().props('loading')).toBe(false);
});
describe('submitting again after a previous error', () => {
beforeEach(() => {
findForm().trigger('submit');
});
it('hides the alert message', () => {
expect(findErrorAlert().exists()).toBe(false);
});
});
});
},
);
describe('given a successful endpoint response', () => {
beforeEach(() => {
axiosMock
.onPost(createSastMergeRequestPath)
.replyOnce(200, { filePath: newMergeRequestPath });
createComponent();
findForm().trigger('submit');
});
it('includes the value of each entity in the payload', expectPayloadForEntities);
it(`sets the submit button's loading prop to true`, () => {
expect(findSubmitButton().props().loading).toBe(true);
});
describe('after async tasks', () => {
beforeEach(() => waitForPromises());
it('calls redirectTo', () => {
expect(redirectTo).toHaveBeenCalledWith(newMergeRequestPath);
});
it('does not display an alert message', () => {
expect(findErrorAlert().exists()).toBe(false);
});
it('does not call Sentry.captureException', () => {
expect(Sentry.captureException).not.toHaveBeenCalled();
});
it('keeps the loading prop set to true', () => {
// This is done for UX reasons. If the loading prop is set to false
// on success, then there's a period where the button is clickable
// again. Instead, we want the button to display a loading indicator
// for the remainder of the lifetime of the page (i.e., until the
// browser can start painting the new page it's been redirected to).
expect(findSubmitButton().props().loading).toBe(true);
});
});
});
});
describe('the cancel button', () => {
beforeEach(() => {
createComponent();
});
it('exists', () => {
expect(findCancelButton().exists()).toBe(true);
});
it('links to the Security Configuration page', () => {
expect(findCancelButton().attributes('href')).toBe(securityConfigurationPath);
});
});
});
...@@ -12,8 +12,10 @@ RSpec.describe Projects::Security::SastConfigurationHelper do ...@@ -12,8 +12,10 @@ RSpec.describe Projects::Security::SastConfigurationHelper do
it { it {
is_expected.to eq({ is_expected.to eq({
create_sast_merge_request_path: project_security_configuration_sast_path(project),
project_path: project_path,
sast_documentation_path: docs_path, sast_documentation_path: docs_path,
project_path: project_path security_configuration_path: project_security_configuration_path(project)
}) })
} }
end end
......
...@@ -21303,6 +21303,9 @@ msgstr "" ...@@ -21303,6 +21303,9 @@ msgstr ""
msgid "SecurityConfiguration|Could not retrieve configuration data. Please refresh the page, or try again later." msgid "SecurityConfiguration|Could not retrieve configuration data. Please refresh the page, or try again later."
msgstr "" msgstr ""
msgid "SecurityConfiguration|Create Merge Request"
msgstr ""
msgid "SecurityConfiguration|Customize common SAST settings to suit your requirements. Configuration changes made here override those provided by GitLab and are excluded from updates. For details of more advanced configuration options, see the %{linkStart}GitLab SAST documentation%{linkEnd}." msgid "SecurityConfiguration|Customize common SAST settings to suit your requirements. Configuration changes made here override those provided by GitLab and are excluded from updates. For details of more advanced configuration options, see the %{linkStart}GitLab SAST documentation%{linkEnd}."
msgstr "" msgstr ""
......
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