Merge branch '223879-fe-implementation-mr-creation-flow-for-sast-ui-config-changes' into 'master'

Implement SAST Configuration form submission

See merge request gitlab-org/gitlab!38480

ee/app/assets/javascripts/security_configuration/sast/components/app.vue

@@ -2,12 +2,12 @@
 import { GlAlert, GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui';
 import { s__ } from '~/locale';
 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';
 
 export default {
   components: {
-    DynamicFields,
+    ConfigurationForm,
     GlAlert,
     GlLink,
     GlLoadingIcon,
@@ -89,6 +89,6 @@ export default {
       $options.i18n.loadingErrorText
     }}</gl-alert>
 
-    <dynamic-fields v-else v-model="sastConfigurationEntities" />
+    <configuration-form v-else :entities="sastConfigurationEntities" />
   </article>
 </template>

ee/app/assets/javascripts/security_configuration/sast/components/configuration_form.vue

+<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>

ee/app/assets/javascripts/security_configuration/sast/index.js

@@ -16,12 +16,19 @@ export default function init() {
     defaultClient: createDefaultClient(),
   });
 
-  const { projectPath, sastDocumentationPath } = el.dataset;
+  const {
+    securityConfigurationPath,
+    createSastMergeRequestPath,
+    projectPath,
+    sastDocumentationPath,
+  } = el.dataset;
 
   return new Vue({
     el,
     apolloProvider,
     provide: {
+      securityConfigurationPath,
+      createSastMergeRequestPath,
       projectPath,
       sastDocumentationPath,
     },

ee/app/helpers/projects/security/sast_configuration_helper.rb

@@ -3,8 +3,10 @@
 module Projects::Security::SastConfigurationHelper
   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'),
-      project_path: project.full_path
+      security_configuration_path: project_security_configuration_path(project)
     }
   end
 end

ee/spec/frontend/security_configuration/sast/components/app_spec.js

 import { shallowMount } from '@vue/test-utils';
 import { GlAlert, GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui';
 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';
 
 const sastDocumentationPath = '/help/sast';
@@ -39,7 +39,7 @@ describe('SAST Configuration App', () => {
   const findHeader = () => wrapper.find('header');
   const findSubHeading = () => findHeader().find('p');
   const findLink = (container = wrapper) => container.find(GlLink);
-  const findDynamicFields = () => wrapper.find(DynamicFields);
+  const findConfigurationForm = () => wrapper.find(ConfigurationForm);
   const findLoadingIcon = () => wrapper.find(GlLoadingIcon);
   const findErrorAlert = () => wrapper.find(GlAlert);
 
@@ -75,8 +75,8 @@ describe('SAST Configuration App', () => {
       expect(findLoadingIcon().exists()).toBe(true);
     });
 
-    it('does not display the dynamic fields component', () => {
-      expect(findDynamicFields().exists()).toBe(false);
+    it('does not display the configuration form', () => {
+      expect(findConfigurationForm().exists()).toBe(false);
     });
 
     it('does not display an alert message', () => {
@@ -95,8 +95,8 @@ describe('SAST Configuration App', () => {
       expect(findLoadingIcon().exists()).toBe(false);
     });
 
-    it('does not display the dynamic fields component', () => {
-      expect(findDynamicFields().exists()).toBe(false);
+    it('does not display the configuration form', () => {
+      expect(findConfigurationForm().exists()).toBe(false);
     });
 
     it('displays an alert message', () => {
@@ -105,9 +105,10 @@ describe('SAST Configuration App', () => {
   });
 
   describe('when loaded', () => {
-    const entities = makeEntities(3);
+    let entities;
 
     beforeEach(() => {
+      entities = makeEntities(3);
       createComponent({
         sastConfigurationEntities: entities,
       });
@@ -117,29 +118,16 @@ describe('SAST Configuration App', () => {
       expect(findLoadingIcon().exists()).toBe(false);
     });
 
-    it('displays the dynamic fields component', () => {
-      const dynamicFields = findDynamicFields();
-      expect(dynamicFields.exists()).toBe(true);
-      expect(dynamicFields.props('entities')).toBe(entities);
+    it('displays the configuration form', () => {
+      expect(findConfigurationForm().exists()).toBe(true);
     });
 
-    it('does not display an alert message', () => {
-      expect(findErrorAlert().exists()).toBe(false);
+    it('passes the sastConfigurationEntities to the entities prop', () => {
+      expect(findConfigurationForm().props('entities')).toBe(entities);
     });
 
-    describe('when the dynamic fields component emits an input event', () => {
-      let dynamicFields;
-      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);
-      });
+    it('does not display an alert message', () => {
+      expect(findErrorAlert().exists()).toBe(false);
     });
   });
 });

ee/spec/frontend/security_configuration/sast/components/configuration_form_spec.js

+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);
+    });
+  });
+});

ee/spec/helpers/ee/projects/security/sast_configuration_helper_spec.rb

@@ -12,8 +12,10 @@ RSpec.describe Projects::Security::SastConfigurationHelper do
 
     it {
       is_expected.to eq({
+        create_sast_merge_request_path: project_security_configuration_sast_path(project),
+        project_path: project_path,
         sast_documentation_path: docs_path,
-        project_path: project_path
+        security_configuration_path: project_security_configuration_path(project)
       })
     }
   end

locale/gitlab.pot

@@ -21303,6 +21303,9 @@ msgstr ""
 msgid "SecurityConfiguration|Could not retrieve configuration data. Please refresh the page, or try again later."
 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}."
 msgstr ""