Commit f679450b authored by Paul Gascou-Vaillancourt's avatar Paul Gascou-Vaillancourt Committed by Imre Farkas

Add ability to edit DAST site profiles

- The DAST site profile form now supports editing existing profiles
- Updated specs
parent 07de2c2d
<script> <script>
import * as Sentry from '@sentry/browser'; import * as Sentry from '@sentry/browser';
import { isEqual } from 'lodash';
import { __, s__ } from '~/locale'; import { __, s__ } from '~/locale';
import { isAbsolute, redirectTo } from '~/lib/utils/url_utility'; import { isAbsolute, redirectTo } from '~/lib/utils/url_utility';
import { GlAlert, GlButton, GlForm, GlFormGroup, GlFormInput, GlModal } from '@gitlab/ui'; import { GlAlert, GlButton, GlForm, GlFormGroup, GlFormInput, GlModal } from '@gitlab/ui';
import dastSiteProfileCreateMutation from '../graphql/dast_site_profile_create.mutation.graphql'; import dastSiteProfileCreateMutation from '../graphql/dast_site_profile_create.mutation.graphql';
import dastSiteProfileUpdateMutation from '../graphql/dast_site_profile_update.mutation.graphql';
const initField = value => ({ const initField = value => ({
value, value,
...@@ -11,6 +13,9 @@ const initField = value => ({ ...@@ -11,6 +13,9 @@ const initField = value => ({
feedback: null, feedback: null,
}); });
const extractFormValues = form =>
Object.fromEntries(Object.entries(form).map(([key, { value }]) => [key, value]));
export default { export default {
name: 'DastSiteProfileForm', name: 'DastSiteProfileForm',
components: { components: {
...@@ -30,33 +35,56 @@ export default { ...@@ -30,33 +35,56 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
siteProfile: {
type: Object,
required: false,
default: null,
},
}, },
data() { data() {
const { name = '', targetUrl = '' } = this.siteProfile || {};
const form = {
profileName: initField(name),
targetUrl: initField(targetUrl),
};
return { return {
form: { form,
profileName: initField(''), initialFormValues: extractFormValues(form),
targetUrl: initField(''),
},
loading: false, loading: false,
showAlert: false, showAlert: false,
}; };
}, },
computed: { computed: {
formData() { isEdit() {
return Boolean(this.siteProfile?.id);
},
i18n() {
const { isEdit } = this;
return { return {
fullPath: this.fullPath, title: isEdit
...Object.fromEntries(Object.entries(this.form).map(([key, { value }]) => [key, value])), ? s__('DastProfiles|Edit site profile')
: s__('DastProfiles|New site profile'),
errorMessage: isEdit
? s__('DastProfiles|Could not update the site profile. Please try again.')
: s__('DastProfiles|Could not create the site profile. Please try again.'),
modal: {
title: isEdit
? s__('DastProfiles|Do you want to discard your changes?')
: s__('DastProfiles|Do you want to discard this site profile?'),
okTitle: __('Discard'),
cancelTitle: __('Cancel'),
},
}; };
}, },
formTouched() {
return !isEqual(extractFormValues(this.form), this.initialFormValues);
},
formHasErrors() { formHasErrors() {
return Object.values(this.form).some(({ state }) => state === false); return Object.values(this.form).some(({ state }) => state === false);
}, },
someFieldEmpty() { someFieldEmpty() {
return Object.values(this.form).some(({ value }) => !value); return Object.values(this.form).some(({ value }) => !value);
}, },
everyFieldEmpty() {
return Object.values(this.form).every(({ value }) => !value);
},
isSubmitDisabled() { isSubmitDisabled() {
return this.formHasErrors || this.someFieldEmpty; return this.formHasErrors || this.someFieldEmpty;
}, },
...@@ -76,19 +104,32 @@ export default { ...@@ -76,19 +104,32 @@ export default {
onSubmit() { onSubmit() {
this.loading = true; this.loading = true;
this.hideErrors(); this.hideErrors();
const variables = {
fullPath: this.fullPath,
...(this.isEdit ? { id: this.siteProfile.id } : {}),
...extractFormValues(this.form),
};
this.$apollo this.$apollo
.mutate({ .mutate({
mutation: dastSiteProfileCreateMutation, mutation: this.isEdit ? dastSiteProfileUpdateMutation : dastSiteProfileCreateMutation,
variables: this.formData, variables,
})
.then(({ data: { dastSiteProfileCreate: { errors } } }) => {
if (errors?.length > 0) {
this.showErrors(errors);
this.loading = false;
} else {
redirectTo(this.profilesLibraryPath);
}
}) })
.then(
({
data: {
[this.isEdit ? 'dastSiteProfileUpdate' : 'dastSiteProfileCreate']: { errors = [] },
},
}) => {
if (errors.length > 0) {
this.showErrors(errors);
this.loading = false;
} else {
redirectTo(this.profilesLibraryPath);
}
},
)
.catch(e => { .catch(e => {
Sentry.captureException(e); Sentry.captureException(e);
this.showErrors(); this.showErrors();
...@@ -96,7 +137,7 @@ export default { ...@@ -96,7 +137,7 @@ export default {
}); });
}, },
onCancelClicked() { onCancelClicked() {
if (this.everyFieldEmpty) { if (!this.formTouched) {
this.discard(); this.discard();
} else { } else {
this.$refs[this.$options.modalId].show(); this.$refs[this.$options.modalId].show();
...@@ -115,22 +156,17 @@ export default { ...@@ -115,22 +156,17 @@ export default {
}, },
}, },
modalId: 'deleteDastProfileModal', modalId: 'deleteDastProfileModal',
i18n: {
modalTitle: s__('DastProfiles|Do you want to discard this site profile?'),
modalOkTitle: __('Discard'),
modalCancelTitle: __('Cancel'),
},
}; };
</script> </script>
<template> <template>
<gl-form @submit.prevent="onSubmit"> <gl-form @submit.prevent="onSubmit">
<h2 class="gl-mb-6"> <h2 class="gl-mb-6">
{{ s__('DastProfiles|New site profile') }} {{ i18n.title }}
</h2> </h2>
<gl-alert v-if="showAlert" variant="danger" class="gl-mb-5" @dismiss="hideErrors"> <gl-alert v-if="showAlert" variant="danger" class="gl-mb-5" @dismiss="hideErrors">
{{ s__('DastProfiles|Could not create the site profile. Please try again.') }} {{ i18n.errorMessage }}
<ul v-if="errors.length" class="gl-mt-3 gl-mb-0"> <ul v-if="errors.length" class="gl-mt-3 gl-mb-0">
<li v-for="error in errors" :key="error" v-text="error"></li> <li v-for="error in errors" :key="error" v-text="error"></li>
</ul> </ul>
...@@ -182,9 +218,9 @@ export default { ...@@ -182,9 +218,9 @@ export default {
<gl-modal <gl-modal
:ref="$options.modalId" :ref="$options.modalId"
:modal-id="$options.modalId" :modal-id="$options.modalId"
:title="$options.i18n.modalTitle" :title="i18n.modal.title"
:ok-title="$options.i18n.modalOkTitle" :ok-title="i18n.modal.okTitle"
:cancel-title="$options.i18n.modalCancelTitle" :cancel-title="i18n.modal.cancelTitle"
ok-variant="danger" ok-variant="danger"
body-class="gl-display-none" body-class="gl-display-none"
data-testid="dast-site-profile-form-cancel-modal" data-testid="dast-site-profile-form-cancel-modal"
......
mutation dastSiteProfileUpdate(
$id: ID!
$fullPath: ID!
$profileName: String!
$targetUrl: String
) {
project(fullPath: $fullPath) {
dastSiteProfileUpdate(input: { id: $id, profileName: $profileName, targetUrl: $targetUrl }) {
id
errors
}
}
}
import Vue from 'vue'; import Vue from 'vue';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import apolloProvider from './graphql/provider'; import apolloProvider from './graphql/provider';
import DastSiteProfileForm from './components/dast_site_profile_form.vue'; import DastSiteProfileForm from './components/dast_site_profile_form.vue';
...@@ -10,16 +11,22 @@ export default () => { ...@@ -10,16 +11,22 @@ export default () => {
const { fullPath, profilesLibraryPath } = el.dataset; const { fullPath, profilesLibraryPath } = el.dataset;
const props = {
fullPath,
profilesLibraryPath,
};
if (el.dataset.siteProfile) {
props.siteProfile = convertObjectPropsToCamelCase(JSON.parse(el.dataset.siteProfile));
}
// eslint-disable-next-line no-new // eslint-disable-next-line no-new
new Vue({ new Vue({
el, el,
apolloProvider, apolloProvider,
render(h) { render(h) {
return h(DastSiteProfileForm, { return h(DastSiteProfileForm, {
props: { props,
fullPath,
profilesLibraryPath,
},
}); });
}, },
}); });
......
import initDastSiteProfileForm from 'ee/dast_site_profiles_form';
document.addEventListener('DOMContentLoaded', initDastSiteProfileForm);
...@@ -7,6 +7,13 @@ module Projects ...@@ -7,6 +7,13 @@ module Projects
def new def new
end end
def edit
@site_profile = @project
.dast_site_profiles
.with_dast_site
.find(params[:id])
end
private private
def authorize_read_on_demand_scans! def authorize_read_on_demand_scans!
......
- add_to_breadcrumbs s_('OnDemandScans|On-demand Scans'), project_on_demand_scans_path(@project)
- add_to_breadcrumbs s_('DastProfiles|Manage profiles'), project_profiles_path(@project)
- breadcrumb_title s_('DastProfiles|Edit site profile')
- page_title s_('DastProfiles|Edit site profile')
.js-dast-site-profile-form{ data: { full_path: @project.path_with_namespace,
profiles_library_path: project_profiles_path(@project),
site_profile: { id: @site_profile.id, name: @site_profile.name, target_url: @site_profile.dast_site.url }.to_json } }
...@@ -97,7 +97,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do ...@@ -97,7 +97,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
root 'on_demand_scans#index', as: 'on_demand_scans' root 'on_demand_scans#index', as: 'on_demand_scans'
scope :profiles do scope :profiles do
root 'dast_profiles#index', as: 'profiles' root 'dast_profiles#index', as: 'profiles'
resources :dast_site_profiles, only: [:new] resources :dast_site_profiles, only: [:new, :edit]
end end
end end
......
import merge from 'lodash/merge'; import merge from 'lodash/merge';
import { within } from '@testing-library/dom';
import { mount, shallowMount } from '@vue/test-utils'; import { mount, shallowMount } from '@vue/test-utils';
import { GlAlert, GlForm, GlModal } from '@gitlab/ui'; import { GlAlert, GlForm, GlModal } from '@gitlab/ui';
import { TEST_HOST } from 'helpers/test_constants'; import { TEST_HOST } from 'helpers/test_constants';
import DastSiteProfileForm from 'ee/dast_site_profiles_form/components/dast_site_profile_form.vue'; import DastSiteProfileForm from 'ee/dast_site_profiles_form/components/dast_site_profile_form.vue';
import dastSiteProfileCreateMutation from 'ee/dast_site_profiles_form/graphql/dast_site_profile_create.mutation.graphql'; import dastSiteProfileCreateMutation from 'ee/dast_site_profiles_form/graphql/dast_site_profile_create.mutation.graphql';
import dastSiteProfileUpdateMutation from 'ee/dast_site_profiles_form/graphql/dast_site_profile_update.mutation.graphql';
import { redirectTo } from '~/lib/utils/url_utility'; import { redirectTo } from '~/lib/utils/url_utility';
jest.mock('~/lib/utils/url_utility', () => ({ jest.mock('~/lib/utils/url_utility', () => ({
...@@ -24,6 +26,8 @@ const defaultProps = { ...@@ -24,6 +26,8 @@ const defaultProps = {
describe('OnDemandScansApp', () => { describe('OnDemandScansApp', () => {
let wrapper; let wrapper;
const withinComponent = () => within(wrapper.element);
const findForm = () => wrapper.find(GlForm); const findForm = () => wrapper.find(GlForm);
const findProfileNameInput = () => wrapper.find('[data-testid="profile-name-input"]'); const findProfileNameInput = () => wrapper.find('[data-testid="profile-name-input"]');
const findTargetUrlInput = () => wrapper.find('[data-testid="target-url-input"]'); const findTargetUrlInput = () => wrapper.find('[data-testid="target-url-input"]');
...@@ -115,117 +119,133 @@ describe('OnDemandScansApp', () => { ...@@ -115,117 +119,133 @@ describe('OnDemandScansApp', () => {
}); });
}); });
describe('submission', () => { describe.each`
const createdProfileId = 30203; title | siteProfile | mutation | mutationVars | mutationKind
${'New site profile'} | ${null} | ${dastSiteProfileCreateMutation} | ${{}} | ${'dastSiteProfileCreate'}
describe('on success', () => { ${'Edit site profile'} | ${{ id: 1, name: 'foo', targetUrl: 'bar' }} | ${dastSiteProfileUpdateMutation} | ${{ id: 1 }} | ${'dastSiteProfileUpdate'}
beforeEach(() => { `('$title', ({ siteProfile, title, mutation, mutationVars, mutationKind }) => {
createComponent(); beforeEach(() => {
jest createFullComponent({
.spyOn(wrapper.vm.$apollo, 'mutate') propsData: {
.mockResolvedValue({ data: { dastSiteProfileCreate: { id: createdProfileId } } }); siteProfile,
findProfileNameInput().vm.$emit('input', profileName); },
findTargetUrlInput().vm.$emit('input', targetUrl);
submitForm();
}); });
});
it('sets loading state', () => { it('sets the correct title', () => {
expect(findSubmitButton().props('loading')).toBe(true); expect(withinComponent().getByRole('heading', { name: title })).not.toBeNull();
}); });
it('triggers GraphQL mutation', () => { it('populates the fields with the data passed in via the siteProfile prop', () => {
expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({ expect(findProfileNameInput().element.value).toBe(siteProfile?.name ?? '');
mutation: dastSiteProfileCreateMutation, });
variables: {
profileName, describe('submission', () => {
targetUrl, const createdProfileId = 30203;
fullPath,
}, describe('on success', () => {
beforeEach(() => {
jest
.spyOn(wrapper.vm.$apollo, 'mutate')
.mockResolvedValue({ data: { [mutationKind]: { id: createdProfileId } } });
findProfileNameInput().vm.$emit('input', profileName);
findTargetUrlInput().vm.$emit('input', targetUrl);
submitForm();
}); });
});
it('redirects to the profiles library', () => { it('sets loading state', () => {
expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath); expect(findSubmitButton().props('loading')).toBe(true);
}); });
it('does not show an alert', () => { it('triggers GraphQL mutation', () => {
expect(findAlert().exists()).toBe(false); expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({
}); mutation,
}); variables: {
profileName,
targetUrl,
fullPath,
...mutationVars,
},
});
});
describe('on top-level error', () => { it('redirects to the profiles library', () => {
beforeEach(() => { expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath);
createComponent(); });
jest.spyOn(wrapper.vm.$apollo, 'mutate').mockRejectedValue();
const input = findTargetUrlInput();
input.vm.$emit('input', targetUrl);
submitForm();
});
it('resets loading state', () => { it('does not show an alert', () => {
expect(findSubmitButton().props('loading')).toBe(false); expect(findAlert().exists()).toBe(false);
});
}); });
it('shows an error alert', () => { describe('on top-level error', () => {
expect(findAlert().exists()).toBe(true); beforeEach(() => {
}); jest.spyOn(wrapper.vm.$apollo, 'mutate').mockRejectedValue();
}); const input = findTargetUrlInput();
input.vm.$emit('input', targetUrl);
submitForm();
});
describe('on errors as data', () => { it('resets loading state', () => {
const errors = ['error#1', 'error#2', 'error#3']; expect(findSubmitButton().props('loading')).toBe(false);
});
beforeEach(() => {
createComponent();
jest
.spyOn(wrapper.vm.$apollo, 'mutate')
.mockResolvedValue({ data: { dastSiteProfileCreate: { pipelineUrl: null, errors } } });
const input = findTargetUrlInput();
input.vm.$emit('input', targetUrl);
submitForm();
});
it('resets loading state', () => { it('shows an error alert', () => {
expect(findSubmitButton().props('loading')).toBe(false); expect(findAlert().exists()).toBe(true);
});
}); });
it('shows an alert with the returned errors', () => { describe('on errors as data', () => {
const alert = findAlert(); const errors = ['error#1', 'error#2', 'error#3'];
expect(alert.exists()).toBe(true); beforeEach(() => {
errors.forEach(error => { jest
expect(alert.text()).toContain(error); .spyOn(wrapper.vm.$apollo, 'mutate')
.mockResolvedValue({ data: { [mutationKind]: { errors } } });
const input = findTargetUrlInput();
input.vm.$emit('input', targetUrl);
submitForm();
}); });
});
});
});
describe('cancellation', () => { it('resets loading state', () => {
beforeEach(() => { expect(findSubmitButton().props('loading')).toBe(false);
createFullComponent(); });
});
it('shows an alert with the returned errors', () => {
const alert = findAlert();
describe('form empty', () => { expect(alert.exists()).toBe(true);
it('redirects to the profiles library', () => { errors.forEach(error => {
findCancelButton().vm.$emit('click'); expect(alert.text()).toContain(error);
expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath); });
});
}); });
}); });
describe('form not empty', () => { describe('cancellation', () => {
beforeEach(() => { describe('form unchanged', () => {
findTargetUrlInput().setValue(targetUrl); it('redirects to the profiles library', () => {
findProfileNameInput().setValue(profileName); findCancelButton().vm.$emit('click');
expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath);
});
}); });
it('asks the user to confirm the action', () => { describe('form changed', () => {
jest.spyOn(findCancelModal().vm, 'show').mockReturnValue(); beforeEach(() => {
findCancelButton().trigger('click'); findTargetUrlInput().setValue(targetUrl);
expect(findCancelModal().vm.show).toHaveBeenCalled(); findProfileNameInput().setValue(profileName);
}); });
it('redirects to the profiles library if confirmed', () => { it('asks the user to confirm the action', () => {
findCancelModal().vm.$emit('ok'); jest.spyOn(findCancelModal().vm, 'show').mockReturnValue();
expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath); findCancelButton().trigger('click');
expect(findCancelModal().vm.show).toHaveBeenCalled();
});
it('redirects to the profiles library if confirmed', () => {
findCancelModal().vm.$emit('ok');
expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath);
});
}); });
}); });
}); });
......
...@@ -5,8 +5,9 @@ require 'spec_helper' ...@@ -5,8 +5,9 @@ require 'spec_helper'
RSpec.describe Projects::DastSiteProfilesController, type: :request do RSpec.describe Projects::DastSiteProfilesController, type: :request do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:dast_site_profile) { create(:dast_site_profile, project: project) }
describe 'GET #new' do shared_examples 'a GET request' do
context 'feature available' do context 'feature available' do
before do before do
stub_feature_flags(security_on_demand_scans_feature_flag: true) stub_feature_flags(security_on_demand_scans_feature_flag: true)
...@@ -21,7 +22,7 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do ...@@ -21,7 +22,7 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do
end end
it 'can access page' do it 'can access page' do
get project_profiles_path(project) get path
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
end end
...@@ -35,7 +36,7 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do ...@@ -35,7 +36,7 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do
end end
it 'sees a 404 error' do it 'sees a 404 error' do
get project_profiles_path(project) get path
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
...@@ -53,7 +54,7 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do ...@@ -53,7 +54,7 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do
it 'sees a 404 error' do it 'sees a 404 error' do
stub_feature_flags(security_on_demand_scans_feature_flag: false) stub_feature_flags(security_on_demand_scans_feature_flag: false)
stub_licensed_features(security_on_demand_scans: true) stub_licensed_features(security_on_demand_scans: true)
get project_profiles_path(project) get path
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
...@@ -63,11 +64,23 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do ...@@ -63,11 +64,23 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do
it 'sees a 404 error' do it 'sees a 404 error' do
stub_feature_flags(security_on_demand_scans_feature_flag: true) stub_feature_flags(security_on_demand_scans_feature_flag: true)
stub_licensed_features(security_on_demand_scans: false) stub_licensed_features(security_on_demand_scans: false)
get project_profiles_path(project) get path
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
end end
end end
end end
describe 'GET #new' do
it_behaves_like 'a GET request' do
let(:path) { new_project_dast_site_profile_path(project) }
end
end
describe 'GET #edit' do
it_behaves_like 'a GET request' do
let(:path) { edit_project_dast_site_profile_path(project, dast_site_profile) }
end
end
end end
...@@ -7548,12 +7548,21 @@ msgstr "" ...@@ -7548,12 +7548,21 @@ msgstr ""
msgid "DastProfiles|Could not create the site profile. Please try again." msgid "DastProfiles|Could not create the site profile. Please try again."
msgstr "" msgstr ""
msgid "DastProfiles|Could not update the site profile. Please try again."
msgstr ""
msgid "DastProfiles|Do you want to discard this site profile?" msgid "DastProfiles|Do you want to discard this site profile?"
msgstr "" msgstr ""
msgid "DastProfiles|Do you want to discard your changes?"
msgstr ""
msgid "DastProfiles|Edit feature will come soon. Please create a new profile if changes needed" msgid "DastProfiles|Edit feature will come soon. Please create a new profile if changes needed"
msgstr "" msgstr ""
msgid "DastProfiles|Edit site profile"
msgstr ""
msgid "DastProfiles|Error fetching the profiles list. Please check your network connection and try again." msgid "DastProfiles|Error fetching the profiles list. Please check your network connection and try again."
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