Skip to content

G
gitlab-ce
Commit fb193190 authored by Kushal Pandya's avatar Kushal Pandya
Browse files
Options

Merge branch... 

Merge branch '229516-dast-site-profile-library-implementation-iteration-1-delete-profile-frontend' into 'master'

Add capability to delete site profile from DAST on-demand library

See merge request gitlab-org/gitlab!38555
parents 21d6deca 78ac3eaf
Show whitespace changes
Inline Side-by-side
Showing with 432 additions and 98 deletions
ee/app/assets/javascripts/dast_profiles/components/dast_profiles.vue
View file @ fb193190
<script>
import * as Sentry from '@sentry/browser';
import { GlButton, GlTab, GlTabs } from '@gitlab/ui';
import { s__ } from '~/locale';
import ProfilesList from './dast_profiles_list.vue';
import dastSiteProfilesQuery from '../graphql/dast_site_profiles.query.graphql';
import dastSiteProfilesDelete from '../graphql/dast_site_profiles_delete.mutation.graphql';
import * as cacheUtils from '../graphql/cache_utils';
export default {
components: {
......@@ -25,17 +28,17 @@ export default {
return {
siteProfiles: [],
siteProfilesPageInfo: {},
hasSiteProfilesLoadingError: false,
errorMessage: '',
errorDetails: [],
};
},
apollo: {
siteProfiles: {
query: dastSiteProfilesQuery,
variables() {
siteProfiles() {
return {
query: dastSiteProfilesQuery,
variables: {
fullPath: this.projectFullPath,
first: this.$options.profilesPerPage,
};
},
result({ data, error }) {
if (!error) {
......@@ -47,9 +50,13 @@ export default {
return siteProfileEdges.map(({ node }) => node);
},
error(e) {
this.handleLoadingError(e);
error(error) {
this.handleError({
exception: error,
message: this.$options.i18n.errorMessages.fetchNetworkError,
});
},
};
},
},
computed: {
......@@ -61,34 +68,100 @@ export default {
},
},
methods: {
handleLoadingError(e) {
Sentry.captureException(e);
this.hasSiteProfilesLoadingError = true;
handleError({ exception, message = '', details = [] }) {
Sentry.captureException(exception);
this.errorMessage = message;
this.errorDetails = details;
},
resetErrors() {
this.errorMessage = '';
this.errorDetails = [];
},
fetchMoreProfiles() {
const { $apollo, siteProfilesPageInfo } = this;
const {
$apollo,
siteProfilesPageInfo,
$options: { i18n },
} = this;
this.hasSiteProfilesLoadingError = false;
this.resetErrors();
$apollo.queries.siteProfiles
.fetchMore({
variables: { after: siteProfilesPageInfo.endCursor },
updateQuery: (previousResult, { fetchMoreResult }) => {
const newResult = { ...fetchMoreResult };
const previousEdges = previousResult.project.siteProfiles.edges;
const newEdges = newResult.project.siteProfiles.edges;
updateQuery: cacheUtils.appendToPreviousResult,
})
.catch(error => {
this.handleError({ exception: error, message: i18n.errorMessages.fetchNetworkError });
});
},
deleteSiteProfile(profileToBeDeletedId) {
const {
projectFullPath,
handleError,
$options: { i18n },
$apollo: {
queries: {
siteProfiles: { options: siteProfilesQueryOptions },
},
},
} = this;
newResult.project.siteProfiles.edges = [...previousEdges, ...newEdges];
this.resetErrors();
return newResult;
this.$apollo
.mutate({
mutation: dastSiteProfilesDelete,
variables: {
projectFullPath,
profileId: profileToBeDeletedId,
},
update(
store,
{
data: {
dastSiteProfileDelete: { errors = [] },
},
},
) {
if (errors.length === 0) {
cacheUtils.removeProfile({
store,
queryBody: {
query: siteProfilesQueryOptions.query,
variables: siteProfilesQueryOptions.variables,
},
profileToBeDeletedId,
});
} else {
handleError({
message: i18n.errorMessages.deletionBackendError,
details: errors,
});
}
},
optimisticResponse: cacheUtils.dastSiteProfilesDeleteResponse(),
})
.catch(e => {
this.handleLoadingError(e);
.catch(error => {
this.handleError({
exception: error,
message: i18n.errorMessages.deletionNetworkError,
});
});
},
},
profilesPerPage: 10,
i18n: {
errorMessages: {
fetchNetworkError: s__(
'DastProfiles|Could not fetch site profiles. Please refresh the page, or try again later.',
),
deletionNetworkError: s__(
'DastProfiles|Could not delete site profile. Please refresh the page, or try again later.',
),
deletionBackendError: s__('DastProfiles|Could not delete site profiles:'),
},
},
};
</script>
......@@ -124,12 +197,14 @@ export default {
</template>
<profiles-list
:has-error="hasSiteProfilesLoadingError"
:error-message="errorMessage"
:error-details="errorDetails"
:has-more-profiles-to-load="hasMoreSiteProfiles"
:is-loading="isLoadingSiteProfiles"
:profiles-per-page="$options.profilesPerPage"
:profiles="siteProfiles"
@loadMoreProfiles="fetchMoreProfiles"
@deleteProfile="deleteSiteProfile"
/>
</gl-tab>
</gl-tabs>
......
ee/app/assets/javascripts/dast_profiles/components/dast_profiles_list.vue
View file @ fb193190
<script>
import { uniqueId } from 'lodash';
import {
GlAlert,
GlButton,
GlIcon,
GlModal,
GlSkeletonLoader,
GlTable,
GlTooltipDirective,
......@@ -13,6 +15,7 @@ export default {
GlAlert,
GlButton,
GlIcon,
GlModal,
GlSkeletonLoader,
GlTable,
},
......@@ -24,10 +27,15 @@ export default {
type: Array,
required: true,
},
hasError: {
type: Boolean,
errorMessage: {
type: String,
required: false,
default: false,
default: '',
},
errorDetails: {
type: Array,
required: false,
default: () => [],
},
isLoading: {
type: Boolean,
......@@ -46,10 +54,16 @@ export default {
},
data() {
return {
isErrorDismissed: false,
toBeDeletedProfileId: null,
};
},
computed: {
hasError() {
return this.errorMessage !== '';
},
hasErrorDetails() {
return this.errorDetails.length > 0;
},
hasProfiles() {
return this.profiles.length > 0;
},
......@@ -59,6 +73,21 @@ export default {
shouldShowTable() {
return this.isLoadingInitialProfiles || this.hasProfiles || this.hasError;
},
modalId() {
return `dast-profiles-list-${uniqueId()}`;
},
},
methods: {
handleDelete() {
this.$emit('deleteProfile', this.toBeDeletedProfileId);
},
prepareProfileDeletion(profileId) {
this.toBeDeletedProfileId = profileId;
this.$refs[this.modalId].show();
},
handleCancel() {
this.toBeDeletedProfileId = null;
},
},
tableFields: [
{
......@@ -73,7 +102,7 @@ export default {
key: 'validationStatus',
// NOTE: hidden for now, since the site validation is still WIP and will be finished in an upcoming iteration
// roadmap: https://gitlab.com/groups/gitlab-org/-/epics/2912#ui-configuration
class: 'gl-display-none',
class: 'gl-display-none!',
},
{
key: 'actions',
......@@ -92,6 +121,21 @@ export default {
stacked="md"
thead-class="gl-display-none"
>
<template v-if="hasError" #top-row>
<td :colspan="$options.tableFields.length">
<gl-alert class="gl-my-4" variant="danger" :dismissible="false">
{{ errorMessage }}
<ul
v-if="hasErrorDetails"
:aria-label="__('DastProfiles|Error Details')"
class="gl-p-0 gl-m-0"
>
<li v-for="errorDetail in errorDetails" :key="errorDetail">{{ errorDetail }}</li>
</ul>
</gl-alert>
</td>
</template>
<template #cell(profileName)="{ value }">
<strong>{{ value }}</strong>
</template>
......@@ -107,7 +151,16 @@ export default {
</span>
</template>
<template #cell(actions)>
<template #cell(actions)="{ item }">
<div class="gl-text-right">
<gl-button
icon="remove"
variant="danger"
category="secondary"
class="gl-mr-3"
:aria-label="__('Delete')"
@click="prepareProfileDeletion(item.id)"
/>
<!--
NOTE: The tooltip and `disable` on the button is temporary until the edit feature has been implemented
further details: https://gitlab.com/gitlab-org/gitlab/-/issues/222479#proposal
......@@ -122,6 +175,7 @@ export default {
>
<gl-button disabled>{{ __('Edit') }}</gl-button>
</span>
</div>
</template>
<template #table-busy>
......@@ -134,18 +188,6 @@ export default {
</gl-skeleton-loader>
</div>
</template>
<template v-if="hasError && !isErrorDismissed" #bottom-row>
<td :colspan="$options.tableFields.length">
<gl-alert class="gl-my-4" variant="danger" :dismissible="false">
{{
s__(
'DastProfiles|Error fetching the profiles list. Please check your network connection and try again.',
)
}}
</gl-alert>
</td>
</template>
</gl-table>
<p v-if="hasMoreProfilesToLoad" class="gl-display-flex gl-justify-content-center">
......@@ -153,13 +195,27 @@ export default {
data-testid="loadMore"
:loading="isLoading && !hasError"
@click="$emit('loadMoreProfiles')"
>{{ __('Load more') }}</gl-button
>
{{ __('Load more') }}
</gl-button>
</p>
</div>
<p v-else class="gl-my-4">
{{ s__('DastProfiles|No profiles created yet') }}
</p>
<gl-modal
:ref="modalId"
:modal-id="modalId"
:title="s__('DastProfiles|Are you sure you want to delete this profile?')"
:ok-title="__('Delete')"
:static="true"
:lazy="true"
ok-variant="danger"
body-class="gl-display-none"
@ok="handleDelete"
@cancel="handleCancel"
/>
</section>
</template>
ee/app/assets/javascripts/dast_profiles/graphql/cache_utils.js 0 → 100644
View file @ fb193190
/**
* Appends paginated results to existing ones
* - to be used with $apollo.queries.x.fetchMore
*
* @param previousResult
* @param fetchMoreResult
* @returns {*}
*/
export const appendToPreviousResult = (previousResult, { fetchMoreResult }) => {
const newResult = { ...fetchMoreResult };
const previousEdges = previousResult.project.siteProfiles.edges;
const newEdges = newResult.project.siteProfiles.edges;
newResult.project.siteProfiles.edges = [...previousEdges, ...newEdges];
return newResult;
};
/**
* Removes profile with given id from the cache and writes the result to it
*
* @param store
* @param queryBody
* @param profileToBeDeletedId
*/
export const removeProfile = ({ store, queryBody, profileToBeDeletedId }) => {
const data = store.readQuery(queryBody);
data.project.siteProfiles.edges = data.project.siteProfiles.edges.filter(({ node }) => {
return node.id !== profileToBeDeletedId;
});
store.writeQuery({ ...queryBody, data });
};
/**
* Returns an object representing a optimistic response for site-profile deletion
*
* @returns {{__typename: string, dastSiteProfileDelete: {__typename: string, errors: []}}}
*/
export const dastSiteProfilesDeleteResponse = () => ({
// eslint-disable-next-line @gitlab/require-i18n-strings
__typename: 'Mutation',
dastSiteProfileDelete: {
__typename: 'DastSiteProfileDeletePayload',
errors: [],
},
});
ee/app/assets/javascripts/dast_profiles/graphql/dast_site_profiles_delete.mutation.graphql 0 → 100644
View file @ fb193190
mutation dastSiteProfileDelete($projectFullPath: ID!, $profileId: DastSiteProfileID!) {
dastSiteProfileDelete(input: { fullPath: $projectFullPath, id: $profileId }) {
errors
}
}
ee/spec/frontend/dast_profiles/components/dast_profiles_list_spec.js
View file @ fb193190
import { merge } from 'lodash';
import { mount } from '@vue/test-utils';
import { mount, shallowMount, createWrapper } from '@vue/test-utils';
import { within } from '@testing-library/dom';
import { GlModal } from '@gitlab/ui';
import DastProfilesList from 'ee/dast_profiles/components/dast_profiles_list.vue';
const TEST_ERROR_MESSAGE = 'something went wrong';
describe('EE - DastProfilesList', () => {
let wrapper;
const createComponent = (options = {}) => {
const createComponentFactory = (mountFn = shallowMount) => (options = {}) => {
const defaultProps = {
profiles: [],
hasMorePages: false,
profilesPerPage: 10,
errorMessage: '',
errorDetails: [],
};
wrapper = mount(
wrapper = mountFn(
DastProfilesList,
merge(
{},
......@@ -25,6 +30,9 @@ describe('EE - DastProfilesList', () => {
);
};
const createComponent = createComponentFactory();
const createFullComponent = createComponentFactory(mount);
const withinComponent = () => within(wrapper.element);
const getTable = () => withinComponent().getByRole('table', { name: /site profiles/i });
const getAllRowGroups = () => within(getTable()).getAllByRole('rowgroup');
......@@ -36,7 +44,11 @@ describe('EE - DastProfilesList', () => {
const getAllTableRows = () => within(getTableBody()).getAllByRole('row');
const getLoadMoreButton = () => wrapper.find('[data-testid="loadMore"]');
const getAllLoadingIndicators = () => withinComponent().queryAllByTestId('loadingIndicator');
const getErrorMessage = () => withinComponent().queryByText(/error fetching the profiles list/i);
const getErrorMessage = () => withinComponent().queryByText(TEST_ERROR_MESSAGE);
const getErrorDetails = () => withinComponent().queryByRole('list', { name: /error details/i });
const getDeleteButtonWithin = element =>
createWrapper(within(element).queryByRole('button', { name: /delete/i }));
const getModal = () => wrapper.find(GlModal);
afterEach(() => {
wrapper.destroy();
......@@ -48,7 +60,7 @@ describe('EE - DastProfilesList', () => {
describe('initial load', () => {
beforeEach(() => {
createComponent({ propsData: { isLoading: true, profilesPerPage } });
createFullComponent({ propsData: { isLoading: true, profilesPerPage } });
});
it('shows a loading indicator for each profile item', () => {
......@@ -106,14 +118,13 @@ describe('EE - DastProfilesList', () => {
const getTableRowForProfile = profile => getAllTableRows()[profiles.indexOf(profile)];
it('does not show loading indicators', () => {
createComponent({});
expect(getAllLoadingIndicators()).toHaveLength(0);
});
describe('profiles list', () => {
beforeEach(() => {
createComponent({ propsData: { profiles } });
createFullComponent({ propsData: { profiles } });
});
it('does not show loading indicators', () => {
expect(getAllLoadingIndicators()).toHaveLength(0);
});
it('renders a list of profiles', () => {
......@@ -133,6 +144,7 @@ describe('EE - DastProfilesList', () => {
expect(targetUrlCell.innerText).toContain(profile.targetUrl);
expect(validationStatusCell.innerText).toContain(profile.validationStatus);
expect(within(actionsCell).getByRole('button', { name: /edit/i })).not.toBe(null);
expect(within(actionsCell).getByRole('button', { name: /delete/i })).not.toBe(null);
});
});
......@@ -145,14 +157,14 @@ describe('EE - DastProfilesList', () => {
describe('with more profiles', () => {
beforeEach(() => {
createComponent({ propsData: { profiles, hasMoreProfilesToLoad: true } });
createFullComponent({ propsData: { profiles, hasMoreProfilesToLoad: true } });
});
it('shows that there are more projects to be loaded', () => {
expect(getLoadMoreButton().exists()).toBe(true);
});
it('emits "loadMore" when the load-more button is clicked', async () => {
it('emits "loadMoreProfiles" when the load-more button is clicked', async () => {
expect(wrapper.emitted('loadMoreProfiles')).toBe(undefined);
await getLoadMoreButton().trigger('click');
......@@ -161,19 +173,59 @@ describe('EE - DastProfilesList', () => {
});
});
});
describe.each(profiles)('delete profile', profile => {
beforeEach(() => {
createFullComponent({ propsData: { profiles } });
});
const getCurrentProfileDeleteButton = () =>
getDeleteButtonWithin(getTableRowForProfile(profile));
it('opens a modal with the correct title when a delete button is clicked', async () => {
expect(getModal().isEmpty()).toBe(true);
getCurrentProfileDeleteButton().trigger('click');
await wrapper.vm.$nextTick();
expect(
within(getModal().element).getByText(/are you sure you want to delete this profile/i),
).not.toBe(null);
});
it(`emits "@deleteProfile" with the right payload when the modal's primary action is triggered`, async () => {
expect(wrapper.emitted('deleteProfile')).toBe(undefined);
getCurrentProfileDeleteButton().trigger('click');
await wrapper.vm.$nextTick();
getModal().vm.$emit('ok');
expect(wrapper.emitted('deleteProfile')[0]).toEqual([profile.id]);
});
});
});
describe('errors', () => {
it('does not show an error message by default', () => {
createComponent();
createFullComponent();
expect(getErrorMessage()).toBe(null);
expect(getErrorDetails()).toBe(null);
});
it('shows an error message', () => {
createComponent({ propsData: { hasError: true } });
it('shows an error message and details', () => {
const errorDetails = ['foo', 'bar'];
createFullComponent({
propsData: { errorMessage: TEST_ERROR_MESSAGE, errorDetails },
});
expect(getErrorMessage()).not.toBe(null);
expect(getErrorDetails()).not.toBe(null);
expect(within(getErrorDetails()).getByText(errorDetails[0])).not.toBe(null);
expect(within(getErrorDetails()).getByText(errorDetails[1])).not.toBe(null);
});
});
});
ee/spec/frontend/dast_profiles/components/dast_profiles_spec.js
View file @ fb193190
......@@ -19,9 +19,12 @@ describe('EE - DastProfiles', () => {
const defaultMocks = {
$apollo: {
queries: {
siteProfiles: {},
siteProfiles: {
fetchMore: jest.fn().mockResolvedValue(),
},
},
mutate: jest.fn().mockResolvedValue(),
},
};
wrapper = mountFn(
......@@ -99,7 +102,8 @@ describe('EE - DastProfiles', () => {
it('passes down the correct default props', () => {
expect(getSiteProfilesComponent().props()).toEqual({
hasError: false,
errorMessage: '',
errorDetails: [],
hasMoreProfilesToLoad: false,
isLoading: false,
profilesPerPage: expect.any(Number),
......@@ -107,35 +111,51 @@ describe('EE - DastProfiles', () => {
});
});
it.each([true, false])('passes down the error state', async hasError => {
wrapper.setData({ hasSiteProfilesLoadingError: hasError });
await wrapper.vm.$nextTick();
it.each([true, false])('passes down the loading state', loading => {
createComponent({ mocks: { $apollo: { queries: { siteProfiles: { loading } } } } });
expect(getSiteProfilesComponent().props('hasError')).toBe(hasError);
expect(getSiteProfilesComponent().props('isLoading')).toBe(loading);
});
it.each([true, false])('passes down the pagination information', async hasNextPage => {
wrapper.setData({ siteProfilesPageInfo: { hasNextPage } });
it.each`
givenData | propName | expectedPropValue
${{ errorMessage: 'foo' }} | ${'errorMessage'} | ${'foo'}
${{ siteProfilesPageInfo: { hasNextPage: true } }} | ${'hasMoreProfilesToLoad'} | ${true}
${{ siteProfiles: [{ foo: 'bar' }] }} | ${'profiles'} | ${[{ foo: 'bar' }]}
`('passes down $propName correctly', async ({ givenData, propName, expectedPropValue }) => {
wrapper.setData(givenData);
await wrapper.vm.$nextTick();
expect(getSiteProfilesComponent().props('hasMoreProfilesToLoad')).toBe(hasNextPage);
expect(getSiteProfilesComponent().props(propName)).toEqual(expectedPropValue);
});
it.each([true, false])('passes down the loading state', loading => {
createComponent({ mocks: { $apollo: { queries: { siteProfiles: { loading } } } } });
it('fetches more results when "@loadMoreProfiles" is emitted', () => {
const {
$apollo: {
queries: {
siteProfiles: { fetchMore },
},
},
} = wrapper.vm;
expect(getSiteProfilesComponent().props('isLoading')).toBe(loading);
expect(fetchMore).not.toHaveBeenCalled();
getSiteProfilesComponent().vm.$emit('loadMoreProfiles');
expect(fetchMore).toHaveBeenCalledTimes(1);
});
it('passes down the profiles data', async () => {
const siteProfiles = [{}];
wrapper.setData({ siteProfiles });
it('deletes profile when "@deleteProfile" is emitted', () => {
const {
$apollo: { mutate },
} = wrapper.vm;
await wrapper.vm.$nextTick();
expect(mutate).not.toHaveBeenCalled();
getSiteProfilesComponent().vm.$emit('deleteProfile');
expect(getSiteProfilesComponent().props('profiles')).toBe(siteProfiles);
expect(mutate).toHaveBeenCalledTimes(1);
});
});
});
ee/spec/frontend/dast_profiles/graphql/cache_utils_spec.js 0 → 100644
View file @ fb193190
import {
appendToPreviousResult,
removeProfile,
dastSiteProfilesDeleteResponse,
} from 'ee/dast_profiles/graphql/cache_utils';
describe('EE - DastProfiles GraphQL CacheUtils', () => {
describe('appendToPreviousResult', () => {
it('appends new results to previous', () => {
const previousResult = { project: { siteProfiles: { edges: ['foo'] } } };
const fetchMoreResult = { project: { siteProfiles: { edges: ['bar'] } } };
const expected = { project: { siteProfiles: { edges: ['foo', 'bar'] } } };
const result = appendToPreviousResult(previousResult, { fetchMoreResult });
expect(result).toEqual(expected);
});
});
describe('removeProfile', () => {
it('removes the profile with the given id from the cache', () => {
const mockQueryBody = { query: 'foo', variables: { foo: 'bar' } };
const mockProfiles = [{ id: 0 }, { id: 1 }];
const mockData = {
project: {
siteProfiles: {
edges: [{ node: mockProfiles[0] }, { node: mockProfiles[1] }],
},
},
};
const mockStore = {
readQuery: () => mockData,
writeQuery: jest.fn(),
};
removeProfile({
store: mockStore,
queryBody: mockQueryBody,
profileToBeDeletedId: mockProfiles[0].id,
});
expect(mockStore.writeQuery).toHaveBeenCalledWith({
...mockQueryBody,
data: {
project: {
siteProfiles: {
edges: [{ node: mockProfiles[1] }],
},
},
},
});
});
});
describe('dastSiteProfilesDeleteResponse', () => {
it('returns a mutation response with the correct shape', () => {
expect(dastSiteProfilesDeleteResponse()).toEqual({
__typename: 'Mutation',
dastSiteProfileDelete: {
__typename: 'DastSiteProfileDeletePayload',
errors: [],
},
});
});
});
});
locale/gitlab.pot
View file @ fb193190
......@@ -7554,9 +7554,21 @@ msgstr ""
msgid "Dashboard|Unable to add %{invalidProjects}. This dashboard is available for public projects, and private projects in groups with a Silver plan."
msgstr ""
msgid "DastProfiles|Are you sure you want to delete this profile?"
msgstr ""
msgid "DastProfiles|Could not create the site profile. Please try again."
msgstr ""
msgid "DastProfiles|Could not delete site profile. Please refresh the page, or try again later."
msgstr ""
msgid "DastProfiles|Could not delete site profiles:"
msgstr ""
msgid "DastProfiles|Could not fetch site profiles. Please refresh the page, or try again later."
msgstr ""
msgid "DastProfiles|Could not update the site profile. Please try again."
msgstr ""
......@@ -7572,7 +7584,7 @@ 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 Details"
msgstr ""
msgid "DastProfiles|Manage Profiles"
......
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