Commit 9fda3c4d authored by David Pisek's avatar David Pisek Committed by Savas Vedova

Add primary provider support to sec training

Updates the UI to include a means to change the primary provider
for displaying security training providers.
parent d812a054
......@@ -2,7 +2,7 @@
import { GlAlert, GlCard, GlToggle, GlLink, GlSkeletonLoader } from '@gitlab/ui';
import * as Sentry from '@sentry/browser';
import Tracking from '~/tracking';
import { __ } from '~/locale';
import { __, s__ } from '~/locale';
import {
TRACK_TOGGLE_TRAINING_PROVIDER_ACTION,
TRACK_TOGGLE_TRAINING_PROVIDER_LABEL,
......@@ -10,9 +10,12 @@ import {
TRACK_PROVIDER_LEARN_MORE_CLICK_LABEL,
} from '~/security_configuration/constants';
import dismissUserCalloutMutation from '~/graphql_shared/mutations/dismiss_user_callout.mutation.graphql';
import { updateSecurityTrainingOptimisticResponse } from '~/security_configuration/graphql/utils/optimistic_response';
import securityTrainingProvidersQuery from '../graphql/security_training_providers.query.graphql';
import configureSecurityTrainingProvidersMutation from '../graphql/configure_security_training_providers.mutation.graphql';
import securityTrainingProvidersQuery from '~/security_configuration/graphql/security_training_providers.query.graphql';
import configureSecurityTrainingProvidersMutation from '~/security_configuration/graphql/configure_security_training_providers.mutation.graphql';
import {
updateSecurityTrainingCache,
updateSecurityTrainingOptimisticResponse,
} from '~/security_configuration/graphql/cache_utils';
const i18n = {
providerQueryErrorMessage: __(
......@@ -21,6 +24,7 @@ const i18n = {
configMutationErrorMessage: __(
'Could not save configuration. Please refresh the page, or try again later.',
),
primaryTraining: s__('SecurityTraining|Primary Training'),
};
export default {
......@@ -57,6 +61,9 @@ export default {
};
},
computed: {
enabledProviders() {
return this.securityTrainingProviders.filter(({ isEnabled }) => isEnabled);
},
isLoading() {
return this.$apollo.queries.securityTrainingProviders.loading;
},
......@@ -91,14 +98,42 @@ export default {
Sentry.captureException(e);
}
},
toggleProvider(provider) {
const { isEnabled } = provider;
async toggleProvider(provider) {
const { isEnabled, isPrimary } = provider;
const toggledIsEnabled = !isEnabled;
this.trackProviderToggle(provider.id, toggledIsEnabled);
this.storeProvider({ ...provider, isEnabled: toggledIsEnabled });
// when the current primary provider gets disabled then set the first enabled to be the new primary
if (!toggledIsEnabled && isPrimary && this.enabledProviders.length > 1) {
const firstOtherEnabledProvider = this.enabledProviders.find(
({ id }) => id !== provider.id,
);
this.setPrimaryProvider(firstOtherEnabledProvider);
}
this.storeProvider({
...provider,
isEnabled: toggledIsEnabled,
});
},
setPrimaryProvider(provider) {
this.storeProvider({ ...provider, isPrimary: true });
},
async storeProvider({ id, isEnabled, isPrimary }) {
async storeProvider(provider) {
const { id, isEnabled, isPrimary } = provider;
let nextIsPrimary = isPrimary;
// if the current provider has been disabled it can't be primary
if (!isEnabled) {
nextIsPrimary = false;
}
// if the current provider is the only enabled provider it should be primary
if (isEnabled && !this.enabledProviders.length) {
nextIsPrimary = true;
}
try {
const {
data: {
......@@ -111,13 +146,17 @@ export default {
projectPath: this.projectFullPath,
providerId: id,
isEnabled,
isPrimary,
isPrimary: nextIsPrimary,
},
},
optimisticResponse: updateSecurityTrainingOptimisticResponse({
id,
isEnabled,
isPrimary,
isPrimary: nextIsPrimary,
}),
update: updateSecurityTrainingCache({
query: securityTrainingProvidersQuery,
variables: { fullPath: this.projectFullPath },
}),
});
......@@ -188,6 +227,27 @@ export default {
{{ __('Learn more.') }}
</gl-link>
</p>
<!-- Note: The following `div` and it's content will be replaced by 'GlFormRadio' once https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1720#note_857342988 is resolved -->
<div
class="gl-form-radio custom-control custom-radio"
data-testid="primary-provider-radio"
>
<input
:id="`security-training-provider-${provider.id}`"
type="radio"
:checked="provider.isPrimary"
name="radio-group-name"
class="custom-control-input"
:disabled="!provider.isEnabled"
@change="setPrimaryProvider(provider)"
/>
<label
class="custom-control-label"
:for="`security-training-provider-${provider.id}`"
>
{{ $options.i18n.primaryTraining }}
</label>
</div>
</div>
</div>
</gl-card>
......
import produce from 'immer';
export const updateSecurityTrainingOptimisticResponse = (changes) => ({
// False positive i18n lint: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26
// eslint-disable-next-line @gitlab/require-i18n-strings
......@@ -11,3 +13,28 @@ export const updateSecurityTrainingOptimisticResponse = (changes) => ({
errors: [],
},
});
export const updateSecurityTrainingCache = ({ query, variables }) => (cache, { data }) => {
const {
securityTrainingUpdate: { training: updatedProvider },
} = data;
const { project } = cache.readQuery({ query, variables });
if (!updatedProvider.isPrimary) {
return;
}
// when we set a new primary provider, we need to unset the previous one(s)
const updatedProject = produce(project, (draft) => {
draft.securityTrainingProviders.forEach((provider) => {
// eslint-disable-next-line no-param-reassign
provider.isPrimary = provider.id === updatedProvider.id;
});
});
// write to the cache
cache.writeQuery({
query,
variables,
data: { project: updatedProject },
});
};
......@@ -45,7 +45,7 @@ module Mutations
return unless training.provider
training.provider.tap do |provider|
provider.assign_attributes(is_enabled: !training.destroyed?, is_primary: training.is_primary)
provider.assign_attributes(is_enabled: !training.destroyed?, is_primary: !training.destroyed? && training.is_primary)
end
end
end
......
......@@ -20,14 +20,14 @@ module Security
# if there are other trainings enabled for the project.
# Users have to select another primary before deleting trainings.
def prevent_deleting_primary
return unless is_primary? && only_training_available?
return unless is_primary? && other_trainings_available?
errors.add(:base, _("Can not delete primary training"))
throw :abort # rubocop:disable Cop/BanCatchThrow
end
def only_training_available?
def other_trainings_available?
project.security_trainings.not_including(self).exists?
end
end
......
......@@ -20,10 +20,7 @@ import {
} from '~/security_configuration/constants';
import createMockApollo from 'helpers/mock_apollo_helper';
import waitForPromises from 'helpers/wait_for_promises';
import {
securityTrainingProvidersResponse,
disabledSecurityTrainingProvidersResponse,
} from 'jest/security_configuration/mock_data';
import { getSecurityTrainingProvidersData } from 'jest/security_configuration/mock_data';
const defaultProps = {
identifiers: [
......@@ -36,6 +33,12 @@ const mockSuccessTrainingUrl = 'training/path';
Vue.use(VueApollo);
const TEST_TRAINING_PROVIDERS_ALL_DISABLED = getSecurityTrainingProvidersData();
const TEST_TRAINING_PROVIDERS_FIRST_ENABLED = getSecurityTrainingProvidersData({
providerOverrides: { first: { isEnabled: true } },
});
const TEST_TRAINING_PROVIDERS_DEFAULT = TEST_TRAINING_PROVIDERS_FIRST_ENABLED;
describe('VulnerabilityTraining component', () => {
let wrapper;
let apolloProvider;
......@@ -45,7 +48,7 @@ describe('VulnerabilityTraining component', () => {
apolloProvider = createMockApollo([
[
securityTrainingProvidersQuery,
queryHandler || jest.fn().mockResolvedValue(securityTrainingProvidersResponse),
queryHandler || jest.fn().mockResolvedValue(TEST_TRAINING_PROVIDERS_DEFAULT.response),
],
]);
};
......@@ -106,7 +109,7 @@ describe('VulnerabilityTraining component', () => {
it('does not render component when there are no enabled securityTrainingProviders', async () => {
createApolloProvider({
queryHandler: jest.fn().mockResolvedValue(disabledSecurityTrainingProvidersResponse),
queryHandler: jest.fn().mockResolvedValue(TEST_TRAINING_PROVIDERS_ALL_DISABLED.response),
});
createComponent();
await waitForQueryToBeLoaded();
......
......@@ -46,6 +46,7 @@ RSpec.describe Mutations::Security::TrainingProviderUpdate do
subject { mutation_result[:training] }
context 'when the training is deleted' do
context 'when training is not primary' do
before do
training.destroy!
end
......@@ -53,6 +54,16 @@ RSpec.describe Mutations::Security::TrainingProviderUpdate do
it { is_expected.to have_attributes(is_enabled: false, is_primary: false) }
end
context 'when training is primary' do
before do
training.update!(is_primary: true)
training.destroy!
end
it { is_expected.to have_attributes(is_enabled: false, is_primary: false) }
end
end
context 'when the training is not deleted' do
it { is_expected.to have_attributes(is_enabled: true, is_primary: false) }
end
......
......@@ -33097,6 +33097,9 @@ msgstr ""
msgid "SecurityReports|scanned resources"
msgstr ""
msgid "SecurityTraining|Primary Training"
msgstr ""
msgid "See example DevOps Score page in our documentation."
msgstr ""
......
import * as Sentry from '@sentry/browser';
import { GlAlert, GlLink, GlToggle, GlCard, GlSkeletonLoader } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils';
import Vue from 'vue';
import VueApollo from 'vue-apollo';
import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
import createMockApollo from 'helpers/mock_apollo_helper';
import { mockTracking, unmockTracking } from 'helpers/tracking_helper';
import {
......@@ -12,7 +12,7 @@ import {
TRACK_PROVIDER_LEARN_MORE_CLICK_LABEL,
} from '~/security_configuration/constants';
import TrainingProviderList from '~/security_configuration/components/training_provider_list.vue';
import { updateSecurityTrainingOptimisticResponse } from '~/security_configuration/graphql/utils/optimistic_response';
import { updateSecurityTrainingOptimisticResponse } from '~/security_configuration/graphql/cache_utils';
import securityTrainingProvidersQuery from '~/security_configuration/graphql/security_training_providers.query.graphql';
import configureSecurityTrainingProvidersMutation from '~/security_configuration/graphql/configure_security_training_providers.mutation.graphql';
import dismissUserCalloutMutation from '~/graphql_shared/mutations/dismiss_user_callout.mutation.graphql';
......@@ -20,8 +20,7 @@ import waitForPromises from 'helpers/wait_for_promises';
import {
dismissUserCalloutResponse,
dismissUserCalloutErrorResponse,
securityTrainingProviders,
securityTrainingProvidersResponse,
getSecurityTrainingProvidersData,
updateSecurityTrainingProvidersResponse,
updateSecurityTrainingProvidersErrorResponse,
testProjectPath,
......@@ -30,6 +29,19 @@ import {
Vue.use(VueApollo);
const TEST_TRAINING_PROVIDERS_ALL_DISABLED = getSecurityTrainingProvidersData();
const TEST_TRAINING_PROVIDERS_FIRST_ENABLED = getSecurityTrainingProvidersData({
providerOverrides: { first: { isEnabled: true, isPrimary: true } },
});
const TEST_TRAINING_PROVIDERS_ALL_ENABLED = getSecurityTrainingProvidersData({
providerOverrides: {
first: { isEnabled: true, isPrimary: true },
second: { isEnabled: true, isPrimary: false },
third: { isEnabled: true, isPrimary: false },
},
});
const TEST_TRAINING_PROVIDERS_DEFAULT = TEST_TRAINING_PROVIDERS_ALL_DISABLED;
describe('TrainingProviderList component', () => {
let wrapper;
let apolloProvider;
......@@ -38,7 +50,7 @@ describe('TrainingProviderList component', () => {
const defaultHandlers = [
[
securityTrainingProvidersQuery,
jest.fn().mockResolvedValue(securityTrainingProvidersResponse),
jest.fn().mockResolvedValue(TEST_TRAINING_PROVIDERS_DEFAULT.response),
],
[
configureSecurityTrainingProvidersMutation,
......@@ -53,7 +65,7 @@ describe('TrainingProviderList component', () => {
};
const createComponent = () => {
wrapper = shallowMount(TrainingProviderList, {
wrapper = shallowMountExtended(TrainingProviderList, {
provide: {
projectFullPath: testProjectPath,
},
......@@ -68,6 +80,7 @@ describe('TrainingProviderList component', () => {
const findLinks = () => wrapper.findAllComponents(GlLink);
const findToggles = () => wrapper.findAllComponents(GlToggle);
const findFirstToggle = () => findToggles().at(0);
const findPrimaryProviderRadios = () => wrapper.findAllByTestId('primary-provider-radio');
const findLoader = () => wrapper.findComponent(GlSkeletonLoader);
const findErrorAlert = () => wrapper.findComponent(GlAlert);
......@@ -107,7 +120,7 @@ describe('TrainingProviderList component', () => {
Mutation: {
configureSecurityTrainingProviders: () => ({
errors: [],
securityTrainingProviders: [],
TEST_TRAINING_PROVIDERS_DEFAULT: [],
}),
},
},
......@@ -122,10 +135,11 @@ describe('TrainingProviderList component', () => {
});
it('renders correct amount of cards', () => {
expect(findCards()).toHaveLength(securityTrainingProviders.length);
expect(findCards()).toHaveLength(TEST_TRAINING_PROVIDERS_DEFAULT.data.length);
});
securityTrainingProviders.forEach(({ name, description, url, isEnabled }, index) => {
TEST_TRAINING_PROVIDERS_DEFAULT.data.forEach(
({ name, description, url, isEnabled }, index) => {
it(`shows the name for card ${index}`, () => {
expect(findCards().at(index).text()).toContain(name);
});
......@@ -145,10 +159,24 @@ describe('TrainingProviderList component', () => {
expect(findToggles().at(index).props('value')).toEqual(isEnabled);
});
it(`shows a radio button to select the provider as primary within card ${index}`, () => {
const primaryProviderRadioForCurrentCard = findPrimaryProviderRadios().at(index);
// if the given provider is not enabled it should not be possible select it as primary
expect(primaryProviderRadioForCurrentCard.find('input').attributes('disabled')).toBe(
isEnabled ? undefined : 'disabled',
);
expect(primaryProviderRadioForCurrentCard.text()).toBe(
TrainingProviderList.i18n.primaryTraining,
);
});
it('does not show loader when query is populated', () => {
expect(findLoader().exists()).toBe(false);
});
});
},
);
});
describe('storing training provider settings', () => {
......@@ -168,7 +196,7 @@ describe('TrainingProviderList component', () => {
input: {
providerId: testProviderIds[0],
isEnabled: true,
isPrimary: false,
isPrimary: true,
projectPath: testProjectPath,
},
},
......@@ -178,9 +206,9 @@ describe('TrainingProviderList component', () => {
it('returns an optimistic response when calling the mutation', () => {
const optimisticResponse = updateSecurityTrainingOptimisticResponse({
id: securityTrainingProviders[0].id,
id: TEST_TRAINING_PROVIDERS_DEFAULT.data[0].id,
isEnabled: true,
isPrimary: false,
isPrimary: true,
});
expect(apolloProvider.defaultClient.mutate).toHaveBeenCalledWith(
......@@ -243,7 +271,7 @@ describe('TrainingProviderList component', () => {
// Once https://gitlab.com/gitlab-org/gitlab/-/issues/348985 and https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79492
// are merged this will be much easer to do and should be tackled then.
expect(trackingSpy).toHaveBeenCalledWith(undefined, TRACK_TOGGLE_TRAINING_PROVIDER_ACTION, {
property: securityTrainingProviders[0].id,
property: TEST_TRAINING_PROVIDERS_DEFAULT.data[0].id,
label: TRACK_TOGGLE_TRAINING_PROVIDER_LABEL,
extra: {
providerIsEnabled: true,
......@@ -253,7 +281,7 @@ describe('TrainingProviderList component', () => {
it(`tracks when a provider's "Learn more" link is clicked`, () => {
const firstProviderLink = findLinks().at(0);
const [{ id: firstProviderId }] = securityTrainingProviders;
const [{ id: firstProviderId }] = TEST_TRAINING_PROVIDERS_DEFAULT.data;
expect(trackingSpy).not.toHaveBeenCalled();
......@@ -271,6 +299,37 @@ describe('TrainingProviderList component', () => {
});
});
describe('primary provider settings', () => {
it.each`
description | initialProviderData | expectedMutationInput
${'sets the provider to be non-primary when it gets disabled'} | ${TEST_TRAINING_PROVIDERS_FIRST_ENABLED.response} | ${{ providerId: TEST_TRAINING_PROVIDERS_FIRST_ENABLED.data[0].id, isEnabled: false, isPrimary: false }}
${'sets a provider to be primary when it is the only one enabled'} | ${TEST_TRAINING_PROVIDERS_ALL_DISABLED.response} | ${{ providerId: TEST_TRAINING_PROVIDERS_ALL_DISABLED.data[0].id, isEnabled: true, isPrimary: true }}
${'sets the first other enabled provider to be primary when the primary one gets disabled'} | ${TEST_TRAINING_PROVIDERS_ALL_ENABLED.response} | ${{ providerId: TEST_TRAINING_PROVIDERS_ALL_ENABLED.data[1].id, isEnabled: true, isPrimary: true }}
`('$description', async ({ initialProviderData, expectedMutationInput }) => {
createApolloProvider({
handlers: [
[securityTrainingProvidersQuery, jest.fn().mockResolvedValue(initialProviderData)],
],
});
jest.spyOn(apolloProvider.defaultClient, 'mutate');
createComponent();
await waitForQueryToBeLoaded();
await toggleFirstProvider();
expect(apolloProvider.defaultClient.mutate).toHaveBeenNthCalledWith(
1,
expect.objectContaining({
variables: {
input: expect.objectContaining({
...expectedMutationInput,
}),
},
}),
);
});
});
describe('with errors', () => {
const expectErrorAlertToExist = () => {
expect(findErrorAlert().props()).toMatchObject({
......
import {
updateSecurityTrainingCache,
updateSecurityTrainingOptimisticResponse,
} from '~/security_configuration/graphql/cache_utils';
describe('EE - Security configuration graphQL cache utils', () => {
describe('updateSecurityTrainingOptimisticResponse', () => {
it('returns an optimistic response in the correct shape', () => {
const changes = { isEnabled: true, isPrimary: true };
const mutationResponse = updateSecurityTrainingOptimisticResponse(changes);
expect(mutationResponse).toEqual({
__typename: 'Mutation',
securityTrainingUpdate: {
__typename: 'SecurityTrainingUpdatePayload',
training: {
__typename: 'ProjectSecurityTraining',
...changes,
},
errors: [],
},
});
});
});
describe('updateSecurityTrainingCache', () => {
let mockCache;
beforeEach(() => {
// freezing the data makes sure that we don't mutate the original project
const mockCacheData = Object.freeze({
project: {
securityTrainingProviders: [
{ id: 1, isEnabled: true, isPrimary: true },
{ id: 2, isEnabled: true, isPrimary: false },
{ id: 3, isEnabled: false, isPrimary: false },
],
},
});
mockCache = {
readQuery: () => mockCacheData,
writeQuery: jest.fn(),
};
});
it('does not update the cache when the primary provider is not getting disabled', () => {
const providerAfterUpdate = {
id: 2,
isEnabled: true,
isPrimary: false,
};
updateSecurityTrainingCache({
query: 'GraphQL query',
variables: { fullPath: 'gitlab/project' },
})(mockCache, {
data: {
securityTrainingUpdate: {
training: {
...providerAfterUpdate,
},
},
},
});
expect(mockCache.writeQuery).not.toHaveBeenCalled();
});
it('sets the previous primary provider to be non-primary when another provider gets set as primary', () => {
const providerAfterUpdate = {
id: 2,
isEnabled: true,
isPrimary: true,
};
const expectedTrainingProvidersWrittenToCache = [
// this was the previous primary primary provider and it should not be primary any longer
{ id: 1, isEnabled: true, isPrimary: false },
{ id: 2, isEnabled: true, isPrimary: true },
{ id: 3, isEnabled: false, isPrimary: false },
];
updateSecurityTrainingCache({
query: 'GraphQL query',
variables: { fullPath: 'gitlab/project' },
})(mockCache, {
data: {
securityTrainingUpdate: {
training: {
...providerAfterUpdate,
},
},
},
});
expect(mockCache.writeQuery).toHaveBeenCalledWith(
expect.objectContaining({
data: {
project: {
securityTrainingProviders: expectedTrainingProvidersWrittenToCache,
},
},
}),
);
});
});
});
export const testProjectPath = 'foo/bar';
export const testProviderIds = [101, 102];
export const testProviderIds = [101, 102, 103];
export const securityTrainingProviders = [
const createSecurityTrainingProviders = ({ providerOverrides = {} }) => [
{
id: testProviderIds[0],
name: 'Vendor Name 1',
......@@ -10,33 +10,43 @@ export const securityTrainingProviders = [
url: 'https://www.example.org/security/training',
isEnabled: false,
isPrimary: false,
...providerOverrides.first,
},
{
id: testProviderIds[1],
name: 'Vendor Name 2',
description: 'Security training with guide and learning pathways.',
url: 'https://www.vendornametwo.com/',
isEnabled: true,
isEnabled: false,
isPrimary: false,
...providerOverrides.second,
},
{
id: testProviderIds[2],
name: 'Vendor Name 3',
description: 'Security training for the everyday developer.',
url: 'https://www.vendornamethree.com/',
isEnabled: false,
isPrimary: false,
...providerOverrides.third,
},
];
export const securityTrainingProvidersResponse = {
export const getSecurityTrainingProvidersData = (providerOverrides = {}) => {
const securityTrainingProviders = createSecurityTrainingProviders(providerOverrides);
const response = {
data: {
project: {
id: 1,
securityTrainingProviders,
},
},
};
};
export const disabledSecurityTrainingProvidersResponse = {
data: {
project: {
id: 1,
securityTrainingProviders: [securityTrainingProviders[0]],
},
},
return {
response,
data: securityTrainingProviders,
};
};
export const dismissUserCalloutResponse = {
......
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