Merge branch '15013-fork-form-validation' into 'master'

Add validation to fork form with validation directive

See merge request gitlab-org/gitlab!55838

app/assets/javascripts/pages/projects/forks/new/components/fork_form.vue

@@ -20,6 +20,7 @@ import axios from '~/lib/utils/axios_utils';
 import csrf from '~/lib/utils/csrf';
 import { redirectTo } from '~/lib/utils/url_utility';
 import { s__ } from '~/locale';
+import validation from '~/vue_shared/directives/validation';
 
 const PRIVATE_VISIBILITY = 'private';
 const INTERNAL_VISIBILITY = 'internal';
@@ -31,6 +32,13 @@ const ALLOWED_VISIBILITY = {
   public: [INTERNAL_VISIBILITY, PRIVATE_VISIBILITY, PUBLIC_VISIBILITY],
 };
 
+const initFormField = ({ value, required = true, skipValidation = false }) => ({
+  value,
+  required,
+  state: skipValidation ? true : null,
+  feedback: null,
+});
+
 export default {
   components: {
     GlForm,
@@ -46,6 +54,9 @@ export default {
     GlFormRadioGroup,
     GlFormSelect,
   },
+  directives: {
+    validation: validation(),
+  },
   inject: {
     newGroupPath: {
       default: '',
@@ -77,7 +88,8 @@ export default {
     },
     projectDescription: {
       type: String,
-      required: true,
+      required: false,
+      default: '',
     },
     projectVisibility: {
       type: String,
@@ -85,16 +97,30 @@ export default {
     },
   },
   data() {
+    const form = {
+      state: false,
+      showValidation: false,
+      fields: {
+        namespace: initFormField({
+          value: null,
+        }),
+        name: initFormField({ value: this.projectName }),
+        slug: initFormField({ value: this.projectPath }),
+        description: initFormField({
+          value: this.projectDescription,
+          required: false,
+          skipValidation: true,
+        }),
+        visibility: initFormField({
+          value: this.projectVisibility,
+          skipValidation: true,
+        }),
+      },
+    };
     return {
       isSaving: false,
       namespaces: [],
-      selectedNamespace: {},
-      fork: {
-        name: this.projectName,
-        slug: this.projectPath,
-        description: this.projectDescription,
-        visibility: this.projectVisibility,
-      },
+      form,
     };
   },
   computed: {
@@ -106,7 +132,7 @@ export default {
     },
     namespaceAllowedVisibility() {
       return (
-        ALLOWED_VISIBILITY[this.selectedNamespace.visibility] ||
+        ALLOWED_VISIBILITY[this.form.fields.namespace.value?.visibility] ||
         ALLOWED_VISIBILITY[PUBLIC_VISIBILITY]
       );
     },
@@ -139,16 +165,17 @@ export default {
     },
   },
   watch: {
-    selectedNamespace(newVal) {
+    // eslint-disable-next-line func-names
+    'form.fields.namespace.value': function (newVal) {
       const { visibility } = newVal;
 
       if (this.projectAllowedVisibility.includes(visibility)) {
-        this.fork.visibility = visibility;
+        this.form.fields.visibility.value = visibility;
       }
     },
     // eslint-disable-next-line func-names
-    'fork.name': function (newVal) {
-      this.fork.slug = kebabCase(newVal);
+    'form.fields.name.value': function (newVal) {
+      this.form.fields.slug.value = kebabCase(newVal);
     },
   },
   mounted() {
@@ -166,19 +193,25 @@ export default {
       );
     },
     async onSubmit() {
+      this.form.showValidation = true;
+
+      if (!this.form.state) {
+        return;
+      }
+
       this.isSaving = true;
+      this.form.showValidation = false;
 
       const { projectId } = this;
-      const { name, slug, description, visibility } = this.fork;
-      const { id: namespaceId } = this.selectedNamespace;
+      const { name, slug, description, visibility, namespace } = this.form.fields;
 
       const postParams = {
         id: projectId,
-        name,
-        namespace_id: namespaceId,
-        path: slug,
-        description,
-        visibility,
+        name: name.value,
+        namespace_id: namespace.value.id,
+        path: slug.value,
+        description: description.value,
+        visibility: visibility.value,
       };
 
       const forkProjectPath = `/api/:version/projects/:id/fork`;
@@ -198,16 +231,34 @@ export default {
 </script>
 
 <template>
-  <gl-form method="POST" @submit.prevent="onSubmit">
+  <gl-form novalidate method="POST" @submit.prevent="onSubmit">
     <input type="hidden" name="authenticity_token" :value="$options.csrf.token" />
 
-    <gl-form-group label="Project name" label-for="fork-name">
-      <gl-form-input id="fork-name" v-model="fork.name" data-testid="fork-name-input" required />
+    <gl-form-group
+      :label="__('Project name')"
+      label-for="fork-name"
+      :invalid-feedback="form.fields.name.feedback"
+    >
+      <gl-form-input
+        id="fork-name"
+        v-model="form.fields.name.value"
+        v-validation:[form.showValidation]
+        name="name"
+        data-testid="fork-name-input"
+        :state="form.fields.name.state"
+        required
+      />
     </gl-form-group>
 
     <div class="gl-md-display-flex">
       <div class="gl-flex-basis-half">
-        <gl-form-group label="Project URL" label-for="fork-url" class="gl-md-mr-3">
+        <gl-form-group
+          :label="__('Project URL')"
+          label-for="fork-url"
+          class="gl-md-mr-3"
+          :state="form.fields.namespace.state"
+          :invalid-feedback="s__('ForkProject|Please select a namespace')"
+        >
           <gl-form-input-group>
             <template #prepend>
               <gl-input-group-text>
@@ -216,9 +267,12 @@ export default {
             </template>
             <gl-form-select
               id="fork-url"
-              v-model="selectedNamespace"
+              v-model="form.fields.namespace.value"
+              v-validation:[form.showValidation]
+              name="namespace"
               data-testid="fork-url-input"
               data-qa-selector="fork_namespace_dropdown"
+              :state="form.fields.namespace.state"
               required
             >
               <template slot="first">
@@ -232,11 +286,19 @@ export default {
         </gl-form-group>
       </div>
       <div class="gl-flex-basis-half">
-        <gl-form-group label="Project slug" label-for="fork-slug" class="gl-md-ml-3">
+        <gl-form-group
+          :label="__('Project slug')"
+          label-for="fork-slug"
+          class="gl-md-ml-3"
+          :invalid-feedback="form.fields.slug.feedback"
+        >
           <gl-form-input
             id="fork-slug"
-            v-model="fork.slug"
+            v-model="form.fields.slug.value"
+            v-validation:[form.showValidation]
             data-testid="fork-slug-input"
+            name="slug"
+            :state="form.fields.slug.state"
             required
           />
         </gl-form-group>
@@ -250,11 +312,13 @@ export default {
       </gl-link>
     </p>
 
-    <gl-form-group label="Project description (optional)" label-for="fork-description">
+    <gl-form-group :label="__('Project description (optional)')" label-for="fork-description">
       <gl-form-textarea
         id="fork-description"
-        v-model="fork.description"
+        v-model="form.fields.description.value"
         data-testid="fork-description-textarea"
+        name="description"
+        :state="form.fields.description.state"
       />
     </gl-form-group>
 
@@ -266,8 +330,9 @@ export default {
         </gl-link>
       </label>
       <gl-form-radio-group
-        v-model="fork.visibility"
+        v-model="form.fields.visibility.value"
         data-testid="fork-visibility-radio-group"
+        name="visibility"
         required
       >
         <gl-form-radio
@@ -291,6 +356,7 @@ export default {
         type="submit"
         category="primary"
         variant="confirm"
+        class="js-no-auto-disable"
         data-testid="submit-button"
         data-qa-selector="fork_project_button"
         :loading="isSaving"

locale/gitlab.pot

@@ -14071,6 +14071,9 @@ msgstr ""
 msgid "ForkProject|Internal"
 msgstr ""
 
+msgid "ForkProject|Please select a namespace"
+msgstr ""
+
 msgid "ForkProject|Private"
 msgstr ""
 

spec/features/projects/fork_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe 'Project fork' do
   include ProjectForksHelper
 
   let(:user) { create(:user) }
-  let(:project) { create(:project, :public, :repository) }
+  let(:project) { create(:project, :public, :repository, description: 'some description') }
 
   before do
     sign_in(user)

spec/frontend/pages/projects/forks/new/components/fork_form_spec.js

-import { GlForm, GlFormInputGroup, GlFormInput } from '@gitlab/ui';
-import { shallowMount } from '@vue/test-utils';
+import { GlFormInputGroup, GlFormInput, GlForm } from '@gitlab/ui';
+import { mount, shallowMount } from '@vue/test-utils';
 import axios from 'axios';
 import AxiosMockAdapter from 'axios-mock-adapter';
 import { kebabCase } from 'lodash';
@@ -43,8 +43,8 @@ describe('ForkForm component', () => {
     axiosMock.onGet(DEFAULT_PROPS.endpoint).replyOnce(statusCode, data);
   };
 
-  const createComponent = (props = {}, data = {}) => {
-    wrapper = shallowMount(ForkForm, {
+  const createComponentFactory = (mountFn) => (props = {}, data = {}) => {
+    wrapper = mountFn(ForkForm, {
       provide: {
         newGroupPath: 'some/groups/path',
         visibilityHelpPath: 'some/visibility/help/path',
@@ -65,6 +65,9 @@ describe('ForkForm component', () => {
     });
   };
 
+  const createComponent = createComponentFactory(shallowMount);
+  const createFullComponent = createComponentFactory(mount);
+
   beforeEach(() => {
     axiosMock = new AxiosMockAdapter(axios);
     window.gon = {
@@ -99,44 +102,6 @@ describe('ForkForm component', () => {
     expect(cancelButton.attributes('href')).toBe(projectFullPath);
   });
 
-  it('make POST request with project param', async () => {
-    jest.spyOn(axios, 'post');
-
-    const namespaceId = 20;
-
-    mockGetRequest();
-    createComponent(
-      {},
-      {
-        selectedNamespace: {
-          id: namespaceId,
-        },
-      },
-    );
-
-    wrapper.find(GlForm).vm.$emit('submit', { preventDefault: () => {} });
-
-    const {
-      projectId,
-      projectDescription,
-      projectName,
-      projectPath,
-      projectVisibility,
-    } = DEFAULT_PROPS;
-
-    const url = `/api/${GON_API_VERSION}/projects/${projectId}/fork`;
-    const project = {
-      description: projectDescription,
-      id: projectId,
-      name: projectName,
-      namespace_id: namespaceId,
-      path: projectPath,
-      visibility: projectVisibility,
-    };
-
-    expect(axios.post).toHaveBeenCalledWith(url, project);
-  });
-
   it('has input with csrf token', () => {
     mockGetRequest();
     createComponent();
@@ -258,9 +223,7 @@ describe('ForkForm component', () => {
             projectVisibility: project,
           },
           {
-            selectedNamespace: {
-              visibility: namespace,
-            },
+            form: { fields: { namespace: { value: { visibility: namespace } } } },
           },
         );
 
@@ -274,34 +237,101 @@ describe('ForkForm component', () => {
   describe('onSubmit', () => {
     beforeEach(() => {
       jest.spyOn(urlUtility, 'redirectTo').mockImplementation();
+
+      mockGetRequest();
+      createFullComponent(
+        {},
+        {
+          namespaces: MOCK_NAMESPACES_RESPONSE,
+          form: {
+            state: true,
+          },
+        },
+      );
     });
 
-    it('redirect to POST web_url response', async () => {
-      const webUrl = `new/fork-project`;
+    const selectedMockNamespaceIndex = 1;
+    const namespaceId = MOCK_NAMESPACES_RESPONSE[selectedMockNamespaceIndex].id;
 
-      jest.spyOn(axios, 'post').mockResolvedValue({ data: { web_url: webUrl } });
+    const fillForm = async () => {
+      const namespaceOptions = findForkUrlInput().findAll('option');
 
-      mockGetRequest();
-      createComponent();
+      await namespaceOptions.at(selectedMockNamespaceIndex + 1).setSelected();
+    };
 
-      await wrapper.vm.onSubmit();
+    const submitForm = async () => {
+      await fillForm();
+      const form = wrapper.find(GlForm);
 
-      expect(urlUtility.redirectTo).toHaveBeenCalledWith(webUrl);
+      await form.trigger('submit');
+      await wrapper.vm.$nextTick();
+    };
+
+    describe('with invalid form', () => {
+      it('does not make POST request', async () => {
+        jest.spyOn(axios, 'post');
+
+        expect(axios.post).not.toHaveBeenCalled();
+      });
+
+      it('does not redirect the current page', async () => {
+        await submitForm();
+
+        expect(urlUtility.redirectTo).not.toHaveBeenCalled();
+      });
     });
 
-    it('display flash when POST is unsuccessful', async () => {
-      const dummyError = 'Fork project failed';
+    describe('with valid form', () => {
+      beforeEach(() => {
+        fillForm();
+      });
 
-      jest.spyOn(axios, 'post').mockRejectedValue(dummyError);
+      it('make POST request with project param', async () => {
+        jest.spyOn(axios, 'post');
+
+        await submitForm();
+
+        const {
+          projectId,
+          projectDescription,
+          projectName,
+          projectPath,
+          projectVisibility,
+        } = DEFAULT_PROPS;
+
+        const url = `/api/${GON_API_VERSION}/projects/${projectId}/fork`;
+        const project = {
+          description: projectDescription,
+          id: projectId,
+          name: projectName,
+          namespace_id: namespaceId,
+          path: projectPath,
+          visibility: projectVisibility,
+        };
 
-      mockGetRequest();
-      createComponent();
+        expect(axios.post).toHaveBeenCalledWith(url, project);
+      });
+
+      it('redirect to POST web_url response', async () => {
+        const webUrl = `new/fork-project`;
+        jest.spyOn(axios, 'post').mockResolvedValue({ data: { web_url: webUrl } });
+
+        await submitForm();
+
+        expect(urlUtility.redirectTo).toHaveBeenCalledWith(webUrl);
+      });
+
+      it('display flash when POST is unsuccessful', async () => {
+        const dummyError = 'Fork project failed';
+
+        jest.spyOn(axios, 'post').mockRejectedValue(dummyError);
 
-      await wrapper.vm.onSubmit();
+        await submitForm();
 
-      expect(urlUtility.redirectTo).not.toHaveBeenCalled();
-      expect(createFlash).toHaveBeenCalledWith({
-        message: dummyError,
+        expect(urlUtility.redirectTo).not.toHaveBeenCalled();
+        expect(createFlash).toHaveBeenCalledWith({
+          message: dummyError,
+        });
       });
     });
   });