Merge branch '348711-error-handling-for-configuration-security-training-mutation' into 'master'

Add error handling to security training configuration (GraphQL query and mutation)

See merge request gitlab-org/gitlab!77166

app/assets/javascripts/security_configuration/components/training_provider_list.vue

 <script>
-import { GlCard, GlToggle, GlLink, GlSkeletonLoader } from '@gitlab/ui';
+import { GlAlert, GlCard, GlToggle, GlLink, GlSkeletonLoader } from '@gitlab/ui';
+import { __ } from '~/locale';
 import securityTrainingProvidersQuery from '../graphql/security_training_providers.query.graphql';
 import configureSecurityTrainingProvidersMutation from '../graphql/configure_security_training_providers.mutation.graphql';
 
+const i18n = {
+  providerQueryErrorMessage: __(
+    'Could not fetch training providers. Please refresh the page, or try again later.',
+  ),
+  configMutationErrorMessage: __(
+    'Could not save configuration. Please refresh the page, or try again later.',
+  ),
+};
+
 export default {
   components: {
+    GlAlert,
     GlCard,
     GlToggle,
     GlLink,
@@ -14,10 +25,14 @@ export default {
   apollo: {
     securityTrainingProviders: {
       query: securityTrainingProvidersQuery,
+      error() {
+        this.errorMessage = this.$options.i18n.providerQueryErrorMessage;
+      },
     },
   },
   data() {
     return {
+      errorMessage: '',
       toggleLoading: false,
       securityTrainingProviders: [],
     };
@@ -34,17 +49,21 @@ export default {
         ...(provider.id === selectedProviderId && { isEnabled: !provider.isEnabled }),
       }));
 
-      this.storeEnabledProviders(toggledProviders);
-    },
-    storeEnabledProviders(toggledProviders) {
       const enabledProviderIds = toggledProviders
         .filter(({ isEnabled }) => isEnabled)
         .map(({ id }) => id);
 
+      this.storeEnabledProviders(toggledProviders, enabledProviderIds);
+    },
+    async storeEnabledProviders(toggledProviders, enabledProviderIds) {
       this.toggleLoading = true;
 
-      return this.$apollo
-        .mutate({
+      try {
+        const {
+          data: {
+            configureSecurityTrainingProviders: { errors = [] },
+          },
+        } = await this.$apollo.mutate({
           mutation: configureSecurityTrainingProvidersMutation,
           variables: {
             input: {
@@ -52,50 +71,63 @@ export default {
               fullPath: this.projectPath,
             },
           },
-        })
-        .then(() => {
-          this.toggleLoading = false;
         });
+
+        if (errors.length > 0) {
+          // throwing an error here means we can handle scenarios within the `catch` block below
+          throw new Error();
+        }
+      } catch {
+        this.errorMessage = this.$options.i18n.configMutationErrorMessage;
+      } finally {
+        this.toggleLoading = false;
+      }
     },
   },
+  i18n,
 };
 </script>
 
 <template>
-  <div
-    v-if="isLoading"
-    class="gl-bg-white gl-py-6 gl-rounded-base gl-border-1 gl-border-solid gl-border-gray-100"
-  >
-    <gl-skeleton-loader :width="350" :height="44">
-      <rect width="200" height="8" x="10" y="0" rx="4" />
-      <rect width="300" height="8" x="10" y="15" rx="4" />
-      <rect width="100" height="8" x="10" y="35" rx="4" />
-    </gl-skeleton-loader>
-  </div>
-  <ul v-else class="gl-list-style-none gl-m-0 gl-p-0">
-    <li
-      v-for="{ id, isEnabled, name, description, url } in securityTrainingProviders"
-      :key="id"
-      class="gl-mb-6"
+  <div>
+    <gl-alert v-if="errorMessage" variant="danger" :dismissible="false" class="gl-mb-6">
+      {{ errorMessage }}
+    </gl-alert>
+    <div
+      v-if="isLoading"
+      class="gl-bg-white gl-py-6 gl-rounded-base gl-border-1 gl-border-solid gl-border-gray-100"
     >
-      <gl-card>
-        <div class="gl-display-flex">
-          <gl-toggle
-            :value="isEnabled"
-            :label="__('Training mode')"
-            label-position="hidden"
-            :is-loading="toggleLoading"
-            @change="toggleProvider(id)"
-          />
-          <div class="gl-ml-5">
-            <h3 class="gl-font-lg gl-m-0 gl-mb-2">{{ name }}</h3>
-            <p>
-              {{ description }}
-              <gl-link :href="url" target="_blank">{{ __('Learn more.') }}</gl-link>
-            </p>
+      <gl-skeleton-loader :width="350" :height="44">
+        <rect width="200" height="8" x="10" y="0" rx="4" />
+        <rect width="300" height="8" x="10" y="15" rx="4" />
+        <rect width="100" height="8" x="10" y="35" rx="4" />
+      </gl-skeleton-loader>
+    </div>
+    <ul v-else class="gl-list-style-none gl-m-0 gl-p-0">
+      <li
+        v-for="{ id, isEnabled, name, description, url } in securityTrainingProviders"
+        :key="id"
+        class="gl-mb-6"
+      >
+        <gl-card>
+          <div class="gl-display-flex">
+            <gl-toggle
+              :value="isEnabled"
+              :label="__('Training mode')"
+              label-position="hidden"
+              :is-loading="toggleLoading"
+              @change="toggleProvider(id)"
+            />
+            <div class="gl-ml-5">
+              <h3 class="gl-font-lg gl-m-0 gl-mb-2">{{ name }}</h3>
+              <p>
+                {{ description }}
+                <gl-link :href="url" target="_blank">{{ __('Learn more.') }}</gl-link>
+              </p>
+            </div>
           </div>
-        </div>
-      </gl-card>
-    </li>
-  </ul>
+        </gl-card>
+      </li>
+    </ul>
+  </div>
 </template>

app/assets/javascripts/security_configuration/graphql/configure_security_training_providers.mutation.graphql

 mutation configureSecurityTrainingProviders($input: configureSecurityTrainingProvidersInput!) {
   configureSecurityTrainingProviders(input: $input) @client {
+    errors
     securityTrainingProviders {
       id
       isEnabled

locale/gitlab.pot

@@ -9853,6 +9853,9 @@ msgstr ""
 msgid "Could not fetch policy because existing policy YAML is invalid"
 msgstr ""
 
+msgid "Could not fetch training providers. Please refresh the page, or try again later."
+msgstr ""
+
 msgid "Could not find design."
 msgstr ""
 
@@ -9889,6 +9892,9 @@ msgstr ""
 msgid "Could not revoke project access token %{project_access_token_name}."
 msgstr ""
 
+msgid "Could not save configuration. Please refresh the page, or try again later."
+msgstr ""
+
 msgid "Could not save group ID"
 msgstr ""
 

spec/frontend/security_configuration/components/training_provider_list_spec.js

-import { GlLink, GlToggle, GlCard, GlSkeletonLoader } from '@gitlab/ui';
+import { GlAlert, GlLink, GlToggle, GlCard, GlSkeletonLoader } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import Vue from 'vue';
 import VueApollo from 'vue-apollo';
@@ -8,7 +8,7 @@ import configureSecurityTrainingProvidersMutation from '~/security_configuration
 import waitForPromises from 'helpers/wait_for_promises';
 import {
   securityTrainingProviders,
-  mockResolvers,
+  createMockResolvers,
   testProjectPath,
   textProviderIds,
 } from '../mock_data';
@@ -17,113 +17,177 @@ Vue.use(VueApollo);
 
 describe('TrainingProviderList component', () => {
   let wrapper;
-  let mockApollo;
-  let mockSecurityTrainingProvidersData;
+  let apolloProvider;
 
-  const createComponent = () => {
-    mockApollo = createMockApollo([], mockResolvers);
+  const createApolloProvider = ({ resolvers } = {}) => {
+    apolloProvider = createMockApollo([], createMockResolvers({ resolvers }));
+  };
 
+  const createComponent = () => {
     wrapper = shallowMount(TrainingProviderList, {
       provide: {
         projectPath: testProjectPath,
       },
-      apolloProvider: mockApollo,
+      apolloProvider,
     });
   };
 
   const waitForQueryToBeLoaded = () => waitForPromises();
+  const waitForMutationToBeLoaded = waitForQueryToBeLoaded;
 
   const findCards = () => wrapper.findAllComponents(GlCard);
   const findLinks = () => wrapper.findAllComponents(GlLink);
   const findToggles = () => wrapper.findAllComponents(GlToggle);
+  const findFirstToggle = () => findToggles().at(0);
   const findLoader = () => wrapper.findComponent(GlSkeletonLoader);
+  const findErrorAlert = () => wrapper.findComponent(GlAlert);
 
-  beforeEach(() => {
-    mockSecurityTrainingProvidersData = jest.fn();
-    mockSecurityTrainingProvidersData.mockResolvedValue(securityTrainingProviders);
-
-    createComponent();
-  });
+  const toggleFirstProvider = () => findFirstToggle().vm.$emit('change');
 
   afterEach(() => {
     wrapper.destroy();
-    mockApollo = null;
+    apolloProvider = null;
   });
 
-  describe('when loading', () => {
-    it('shows the loader', () => {
-      expect(findLoader().exists()).toBe(true);
+  describe('with a successful response', () => {
+    beforeEach(() => {
+      createApolloProvider();
+      createComponent();
     });
 
-    it('does not show the cards', () => {
-      expect(findCards().exists()).toBe(false);
-    });
-  });
-
-  describe('basic structure', () => {
-    beforeEach(async () => {
-      await waitForQueryToBeLoaded();
-    });
+    describe('when loading', () => {
+      it('shows the loader', () => {
+        expect(findLoader().exists()).toBe(true);
+      });
 
-    it('renders correct amount of cards', () => {
-      expect(findCards()).toHaveLength(securityTrainingProviders.length);
+      it('does not show the cards', () => {
+        expect(findCards().exists()).toBe(false);
+      });
     });
 
-    securityTrainingProviders.forEach(({ name, description, url, isEnabled }, index) => {
-      it(`shows the name for card ${index}`, () => {
-        expect(findCards().at(index).text()).toContain(name);
+    describe('basic structure', () => {
+      beforeEach(async () => {
+        await waitForQueryToBeLoaded();
       });
 
-      it(`shows the description for card ${index}`, () => {
-        expect(findCards().at(index).text()).toContain(description);
+      it('renders correct amount of cards', () => {
+        expect(findCards()).toHaveLength(securityTrainingProviders.length);
       });
 
-      it(`shows the learn more link for card ${index}`, () => {
-        expect(findLinks().at(index).attributes()).toEqual({
-          target: '_blank',
-          href: url,
+      securityTrainingProviders.forEach(({ name, description, url, isEnabled }, index) => {
+        it(`shows the name for card ${index}`, () => {
+          expect(findCards().at(index).text()).toContain(name);
+        });
+
+        it(`shows the description for card ${index}`, () => {
+          expect(findCards().at(index).text()).toContain(description);
+        });
+
+        it(`shows the learn more link for card ${index}`, () => {
+          expect(findLinks().at(index).attributes()).toEqual({
+            target: '_blank',
+            href: url,
+          });
+        });
+
+        it(`shows the toggle with the correct value for card ${index}`, () => {
+          expect(findToggles().at(index).props('value')).toEqual(isEnabled);
+        });
+
+        it('does not show loader when query is populated', () => {
+          expect(findLoader().exists()).toBe(false);
         });
       });
+    });
+
+    describe('storing training provider settings', () => {
+      beforeEach(async () => {
+        jest.spyOn(apolloProvider.defaultClient, 'mutate');
 
-      it(`shows the toggle with the correct value for card ${index}`, () => {
-        expect(findToggles().at(index).props('value')).toEqual(isEnabled);
+        await waitForMutationToBeLoaded();
+
+        toggleFirstProvider();
       });
 
-      it('does not show loader when query is populated', () => {
-        expect(findLoader().exists()).toBe(false);
+      it.each`
+        loading  | wait     | desc
+        ${true}  | ${false} | ${'enables loading of GlToggle when mutation is called'}
+        ${false} | ${true}  | ${'disables loading of GlToggle when mutation is complete'}
+      `('$desc', async ({ loading, wait }) => {
+        if (wait) {
+          await waitForMutationToBeLoaded();
+        }
+        expect(findFirstToggle().props('isLoading')).toBe(loading);
+      });
+
+      it('calls mutation when toggle is changed', () => {
+        expect(apolloProvider.defaultClient.mutate).toHaveBeenCalledWith(
+          expect.objectContaining({
+            mutation: configureSecurityTrainingProvidersMutation,
+            variables: { input: { enabledProviders: textProviderIds, fullPath: testProjectPath } },
+          }),
+        );
       });
     });
   });
 
-  describe('success mutation', () => {
-    const firstToggle = () => findToggles().at(0);
+  describe('with errors', () => {
+    const expectErrorAlertToExist = () => {
+      expect(findErrorAlert().props()).toMatchObject({
+        dismissible: false,
+        variant: 'danger',
+      });
+    };
+
+    describe('when fetching training providers', () => {
+      beforeEach(async () => {
+        createApolloProvider({
+          resolvers: {
+            Query: {
+              securityTrainingProviders: jest.fn().mockReturnValue(new Error()),
+            },
+          },
+        });
+        createComponent();
 
-    beforeEach(async () => {
-      jest.spyOn(mockApollo.defaultClient, 'mutate');
+        await waitForQueryToBeLoaded();
+      });
 
-      await waitForQueryToBeLoaded();
+      it('shows an non-dismissible error alert', () => {
+        expectErrorAlertToExist();
+      });
 
-      firstToggle().vm.$emit('change');
+      it('shows an error description', () => {
+        expect(findErrorAlert().text()).toBe(TrainingProviderList.i18n.providerQueryErrorMessage);
+      });
     });
 
-    it('calls mutation when toggle is changed', () => {
-      expect(mockApollo.defaultClient.mutate).toHaveBeenCalledWith(
-        expect.objectContaining({
-          mutation: configureSecurityTrainingProvidersMutation,
-          variables: { input: { enabledProviders: textProviderIds, fullPath: testProjectPath } },
-        }),
-      );
-    });
+    describe('when storing training provider configurations', () => {
+      beforeEach(async () => {
+        createApolloProvider({
+          resolvers: {
+            Mutation: {
+              configureSecurityTrainingProviders: () => ({
+                errors: ['something went wrong!'],
+                securityTrainingProviders: [],
+              }),
+            },
+          },
+        });
+        createComponent();
 
-    it.each`
-      loading  | wait     | desc
-      ${true}  | ${false} | ${'enables loading of GlToggle when mutation is called'}
-      ${false} | ${true}  | ${'disables loading of GlToggle when mutation is complete'}
-    `('$desc', async ({ loading, wait }) => {
-      if (wait) {
-        await waitForPromises();
-      }
-      expect(firstToggle().props('isLoading')).toBe(loading);
+        await waitForQueryToBeLoaded();
+        toggleFirstProvider();
+        await waitForMutationToBeLoaded();
+      });
+
+      it('shows an non-dismissible error alert', () => {
+        expectErrorAlertToExist();
+      });
+
+      it('shows an error description', () => {
+        expect(findErrorAlert().text()).toBe(TrainingProviderList.i18n.configMutationErrorMessage);
+      });
     });
   });
 });

spec/frontend/security_configuration/mock_data.js

@@ -25,10 +25,15 @@ export const securityTrainingProvidersResponse = {
   },
 };
 
-export const mockResolvers = {
+const defaultMockResolvers = {
   Query: {
     securityTrainingProviders() {
       return securityTrainingProviders;
     },
   },
 };
+
+export const createMockResolvers = ({ resolvers: customMockResolvers = {} } = {}) => ({
+  ...defaultMockResolvers,
+  ...customMockResolvers,
+});