Merge branch...

Merge branch '429410-new-organization-form-swap-mock-graphql-mutation-for-real-mutation' into 'master' 

Use real GraphQL mutation when creating a new organization

See merge request gitlab-org/gitlab!136147



Merged-by: Michael Lunøe <michael.lunoe@gmail.com>
Approved-by: Camellia X Yang <xyang@gitlab.com>
Approved-by: Ammar Alakkad <aalakkad@gitlab.com>
Approved-by: Michael Lunøe <michael.lunoe@gmail.com>
Reviewed-by: Ammar Alakkad <aalakkad@gitlab.com>
Reviewed-by: Michael Lunøe <michael.lunoe@gmail.com>
Co-authored-by: Peter Hegman <phegman@gitlab.com>

app/assets/javascripts/organizations/mock_data.js

@@ -281,12 +281,25 @@ export const organizationGroups = {
   ],
 };
 
-export const createOrganizationResponse = {
-  organization: {
-    name: 'Default',
-    path: '/-/organizations/default',
+export const organizationCreateResponse = {
+  data: {
+    organizationCreate: {
+      organization: {
+        id: 'gid://gitlab/Organizations::Organization/1',
+        webUrl: 'http://127.0.0.1:3000/-/organizations/default',
+      },
+      errors: [],
+    },
+  },
+};
+
+export const organizationCreateResponseWithErrors = {
+  data: {
+    organizationCreate: {
+      organization: null,
+      errors: ['Path is too short (minimum is 2 characters)'],
+    },
   },
-  errors: [],
 };
 
 export const updateOrganizationResponse = {

app/assets/javascripts/organizations/new/components/app.vue

@@ -4,12 +4,13 @@ import { s__ } from '~/locale';
 import { visitUrlWithAlerts } from '~/lib/utils/url_utility';
 import { createAlert } from '~/alert';
 import { helpPagePath } from '~/helpers/help_page_helper';
-import createOrganizationMutation from '../graphql/mutations/create_organization.mutation.graphql';
+import FormErrorsAlert from '~/vue_shared/components/form/errors_alert.vue';
+import organizationCreateMutation from '../graphql/mutations/organization_create.mutation.graphql';
 import NewEditForm from '../../shared/components/new_edit_form.vue';
 
 export default {
   name: 'OrganizationNewApp',
-  components: { NewEditForm, GlSprintf, GlLink },
+  components: { NewEditForm, GlSprintf, GlLink, FormErrorsAlert },
   i18n: {
     pageTitle: s__('Organization|New organization'),
     pageDescription: s__(
@@ -22,6 +23,7 @@ export default {
   data() {
     return {
       loading: false,
+      errors: [],
     };
   },
   computed: {
@@ -35,21 +37,22 @@ export default {
       try {
         const {
           data: {
-            createOrganization: { organization, errors },
+            organizationCreate: { organization, errors },
           },
         } = await this.$apollo.mutate({
-          mutation: createOrganizationMutation,
+          mutation: organizationCreateMutation,
           variables: {
-            ...formValues,
+            input: { name: formValues.name, path: formValues.path },
           },
         });
 
         if (errors.length) {
-          // TODO: handle errors when using real API after https://gitlab.com/gitlab-org/gitlab/-/issues/417891 is complete.
+          this.errors = errors;
+
           return;
         }
 
-        visitUrlWithAlerts(organization.path, [
+        visitUrlWithAlerts(organization.webUrl, [
           {
             id: 'organization-successfully-created',
             title: this.$options.i18n.successAlertTitle,
@@ -69,6 +72,7 @@ export default {
 
 <template>
   <div class="gl-py-6">
+    <form-errors-alert v-model="errors" />
     <h1 class="gl-mt-0 gl-font-size-h-display">{{ $options.i18n.pageTitle }}</h1>
     <p>
       <gl-sprintf :message="$options.i18n.pageDescription">

app/assets/javascripts/organizations/new/graphql/mutations/create_organization.mutation.graphql

-mutation createOrganization($input: LocalCreateOrganizationInput!) {
-  createOrganization(input: $input) @client {
-    organization {
-      name
-      path
-    }
-    errors
-  }
-}

app/assets/javascripts/organizations/new/graphql/mutations/organization_create.mutation.graphql

+mutation organizationCreate($input: OrganizationCreateInput!) {
+  organizationCreate(input: $input) {
+    organization {
+      id
+      webUrl
+    }
+    errors
+  }
+}

app/assets/javascripts/organizations/new/graphql/typedefs.graphql

-# TODO: Use real input type when https://gitlab.com/gitlab-org/gitlab/-/issues/417891 is complete.
-input LocalCreateOrganizationInput {
-  name: String
-  path: String
-}

app/assets/javascripts/organizations/new/index.js

@@ -3,7 +3,6 @@ import VueApollo from 'vue-apollo';
 
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import createDefaultClient from '~/lib/graphql';
-import resolvers from '../shared/graphql/resolvers';
 import App from './components/app.vue';
 
 export const initOrganizationsNew = () => {
@@ -17,7 +16,7 @@ export const initOrganizationsNew = () => {
   const { organizationsPath, rootUrl } = convertObjectPropsToCamelCase(JSON.parse(appData));
 
   const apolloProvider = new VueApollo({
-    defaultClient: createDefaultClient(resolvers),
+    defaultClient: createDefaultClient(),
   });
 
   return new Vue({

app/assets/javascripts/organizations/shared/components/new_edit_form.vue

@@ -103,7 +103,13 @@ export default {
         },
         [FORM_FIELD_PATH]: {
           label: s__('Organization|Organization URL'),
-          validators: [formValidators.required(s__('Organization|Organization URL is required.'))],
+          validators: [
+            formValidators.required(s__('Organization|Organization URL is required.')),
+            formValidators.factory(
+              s__('Organization|Organization URL must be a minimum of two characters.'),
+              (val) => val.length >= 2,
+            ),
+          ],
           groupAttrs: {
             class: 'gl-w-full',
           },

app/assets/javascripts/organizations/shared/graphql/resolvers.js

@@ -2,7 +2,6 @@ import {
   organizations,
   organizationProjects,
   organizationGroups,
-  createOrganizationResponse,
   updateOrganizationResponse,
 } from '../../mock_data';
 
@@ -35,12 +34,6 @@ export default {
     },
   },
   Mutation: {
-    createOrganization: async () => {
-      // Simulate API loading
-      await simulateLoading();
-
-      return createOrganizationResponse;
-    },
     updateOrganization: async () => {
       // Simulate API loading
       await simulateLoading();

app/assets/javascripts/vue_shared/components/form/errors_alert.stories.js

+import ErrorsAlert from './errors_alert.vue';
+
+export default {
+  component: ErrorsAlert,
+  title: 'vue_shared/form/errors_alert',
+};
+
+const defaultProps = {
+  errors: ['Name must be at least 5 characters.', 'Name cannot contain special characters.'],
+};
+
+const Template = (args) => ({
+  components: { ErrorsAlert },
+  data() {
+    return { errors: args.errors };
+  },
+  template: `<errors-alert v-model="errors" />`,
+});
+
+export const Default = Template.bind({});
+Default.args = defaultProps;

app/assets/javascripts/vue_shared/components/form/errors_alert.vue

+<script>
+import { GlAlert } from '@gitlab/ui';
+import { n__ } from '~/locale';
+
+export default {
+  components: { GlAlert },
+  model: {
+    prop: 'errors',
+  },
+  props: {
+    errors: {
+      type: Array,
+      required: true,
+    },
+  },
+  computed: {
+    title() {
+      return n__(
+        'The form contains the following error:',
+        'The form contains the following errors:',
+        this.errors.length,
+      );
+    },
+  },
+};
+</script>
+
+<template>
+  <gl-alert
+    v-if="errors.length"
+    class="gl-mb-5"
+    :title="title"
+    variant="danger"
+    @dismiss="$emit('input', [])"
+  >
+    <ul class="gl-pl-5 gl-mb-0">
+      <li v-for="error in errors" :key="error">
+        {{ error }}
+      </li>
+    </ul>
+  </gl-alert>
+</template>

app/graphql/types/organizations/organization_type.rb

@@ -33,6 +33,10 @@ class OrganizationType < BaseObject
         null: false,
         description: 'Path of the organization.',
         alpha: { milestone: '16.4' }
+      field :web_url, GraphQL::Types::String,
+        null: false,
+        description: 'Web URL of the organization.',
+        alpha: { milestone: '16.6' }
     end
   end
 end

app/models/organizations/organization.rb

@@ -42,6 +42,10 @@ def user?(user)
       organization_users.exists?(user: user)
     end
 
+    def web_url(only_path: nil)
+      Gitlab::UrlBuilder.build(self, only_path: only_path)
+    end
+
     private
 
     def check_if_default_organization

doc/api/graphql/reference/index.md

@@ -22200,6 +22200,7 @@ Active period time range for on-call rotation.
 | <a id="organizationname"></a>`name` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Name of the organization. |
 | <a id="organizationorganizationusers"></a>`organizationUsers` **{warning-solid}** | [`OrganizationUserConnection!`](#organizationuserconnection) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Users with access to the organization. |
 | <a id="organizationpath"></a>`path` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Path of the organization. |
+| <a id="organizationweburl"></a>`webUrl` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.6. This feature is an Experiment. It can be changed or removed at any time. Web URL of the organization. |
 #### Fields with arguments

lib/gitlab/url_builder.rb

@@ -40,6 +40,8 @@ def build(object, **options)
           note_url(object, **options)
         when Release
           instance.release_url(object, **options)
+        when Organizations::Organization
+          instance.organization_url(object, **options)
         when Project
           instance.project_url(object, **options)
         when Snippet

locale/gitlab.pot

@@ -33515,6 +33515,9 @@ msgstr ""
 msgid "Organization|Organization URL is required."
 msgstr ""
+msgid "Organization|Organization URL must be a minimum of two characters."
+msgstr ""
+
 msgid "Organization|Organization name"
 msgstr ""

spec/frontend/organizations/new/components/app_spec.js

@@ -3,16 +3,19 @@ import Vue, { nextTick } from 'vue';
 
 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 import App from '~/organizations/new/components/app.vue';
-import resolvers from '~/organizations/shared/graphql/resolvers';
+import organizationCreateMutation from '~/organizations/new/graphql/mutations/organization_create.mutation.graphql';
 import NewEditForm from '~/organizations/shared/components/new_edit_form.vue';
 import { visitUrlWithAlerts } from '~/lib/utils/url_utility';
-import { createOrganizationResponse } from '~/organizations/mock_data';
+import FormErrorsAlert from '~/vue_shared/components/form/errors_alert.vue';
+import {
+  organizationCreateResponse,
+  organizationCreateResponseWithErrors,
+} from '~/organizations/mock_data';
 import { createAlert } from '~/alert';
 import createMockApollo from 'helpers/mock_apollo_helper';
 import waitForPromises from 'helpers/wait_for_promises';
 
 Vue.use(VueApollo);
-jest.useFakeTimers();
 
 jest.mock('~/lib/utils/url_utility');
 jest.mock('~/alert');
@@ -21,8 +24,12 @@ describe('OrganizationNewApp', () => {
   let wrapper;
   let mockApollo;
 
-  const createComponent = ({ mockResolvers = resolvers } = {}) => {
-    mockApollo = createMockApollo([], mockResolvers);
+  const createComponent = ({
+    handlers = [
+      [organizationCreateMutation, jest.fn().mockResolvedValue(organizationCreateResponse)],
+    ],
+  } = {}) => {
+    mockApollo = createMockApollo(handlers);
 
     wrapper = shallowMountExtended(App, { apolloProvider: mockApollo });
   };
@@ -46,13 +53,11 @@ describe('OrganizationNewApp', () => {
   describe('when form is submitted', () => {
     describe('when API is loading', () => {
       beforeEach(async () => {
-        const mockResolvers = {
-          Mutation: {
-            createOrganization: jest.fn().mockReturnValueOnce(new Promise(() => {})),
-          },
-        };
-
-        createComponent({ mockResolvers });
+        createComponent({
+          handlers: [
+            [organizationCreateMutation, jest.fn().mockReturnValueOnce(new Promise(() => {}))],
+          ],
+        });
 
         await submitForm();
       });
@@ -66,13 +71,12 @@ describe('OrganizationNewApp', () => {
       beforeEach(async () => {
         createComponent();
         await submitForm();
-        jest.runAllTimers();
         await waitForPromises();
       });
 
-      it('redirects user to organization path', () => {
+      it('redirects user to organization web url', () => {
         expect(visitUrlWithAlerts).toHaveBeenCalledWith(
-          createOrganizationResponse.organization.path,
+          organizationCreateResponse.data.organizationCreate.organization.webUrl,
           [
             {
               id: 'organization-successfully-created',
@@ -86,26 +90,44 @@ describe('OrganizationNewApp', () => {
     });
 
     describe('when API request is not successful', () => {
-      const error = new Error();
-
-      beforeEach(async () => {
-        const mockResolvers = {
-          Mutation: {
-            createOrganization: jest.fn().mockRejectedValueOnce(error),
-          },
-        };
+      describe('when there is a network error', () => {
+        const error = new Error();
+
+        beforeEach(async () => {
+          createComponent({
+            handlers: [[organizationCreateMutation, jest.fn().mockRejectedValue(error)]],
+          });
+          await submitForm();
+          await waitForPromises();
+        });
 
-        createComponent({ mockResolvers });
-        await submitForm();
-        jest.runAllTimers();
-        await waitForPromises();
+        it('displays error alert', () => {
+          expect(createAlert).toHaveBeenCalledWith({
+            message: 'An error occurred creating an organization. Please try again.',
+            error,
+            captureError: true,
+          });
+        });
       });
 
-      it('displays error alert', () => {
-        expect(createAlert).toHaveBeenCalledWith({
-          message: 'An error occurred creating an organization. Please try again.',
-          error,
-          captureError: true,
+      describe('when there are GraphQL errors', () => {
+        beforeEach(async () => {
+          createComponent({
+            handlers: [
+              [
+                organizationCreateMutation,
+                jest.fn().mockResolvedValue(organizationCreateResponseWithErrors),
+              ],
+            ],
+          });
+          await submitForm();
+          await waitForPromises();
+        });
+
+        it('displays form errors alert', () => {
+          expect(wrapper.findComponent(FormErrorsAlert).props('errors')).toEqual(
+            organizationCreateResponseWithErrors.data.organizationCreate.errors,
+          );
         });
       });
     });

spec/frontend/organizations/shared/components/new_edit_form_spec.js

@@ -49,6 +49,17 @@ describe('NewEditForm', () => {
     expect(findUrlField().exists()).toBe(true);
   });
 
+  it('requires `Organization URL` field to be a minimum of two characters', async () => {
+    createComponent();
+
+    await findUrlField().setValue('f');
+    await submitForm();
+
+    expect(
+      wrapper.findByText('Organization URL must be a minimum of two characters.').exists(),
+    ).toBe(true);
+  });
+
   describe('when `fieldsToRender` prop is set', () => {
     beforeEach(() => {
       createComponent({ propsData: { fieldsToRender: [FORM_FIELD_ID] } });

spec/frontend/vue_shared/components/form/errors_alert_spec.js

+import { GlAlert } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
+import FormErrorsAlert from '~/vue_shared/components/form/errors_alert.vue';
+
+describe('FormErrorsAlert', () => {
+  let wrapper;
+
+  const defaultPropsData = {
+    errors: ['Foo', 'Bar', 'Baz'],
+  };
+
+  function createComponent({ propsData = {} } = {}) {
+    wrapper = shallowMount(FormErrorsAlert, {
+      propsData: {
+        ...defaultPropsData,
+        ...propsData,
+      },
+    });
+  }
+
+  const findAlert = () => wrapper.findComponent(GlAlert);
+
+  describe('when there are no errors', () => {
+    it('renders nothing', () => {
+      createComponent({ propsData: { errors: [] } });
+
+      expect(wrapper.html()).toBe('');
+    });
+  });
+
+  describe('when there is one error', () => {
+    it('renders correct title and message', () => {
+      createComponent({ propsData: { errors: ['Foo'] } });
+
+      expect(findAlert().props('title')).toBe('The form contains the following error:');
+      expect(findAlert().text()).toContain('Foo');
+    });
+  });
+
+  describe('when there are multiple errors', () => {
+    it('renders correct title and message', () => {
+      createComponent();
+
+      expect(findAlert().props('title')).toBe('The form contains the following errors:');
+      expect(findAlert().text()).toContain('Foo');
+      expect(findAlert().text()).toContain('Bar');
+      expect(findAlert().text()).toContain('Baz');
+    });
+  });
+
+  describe('when alert is dismissed', () => {
+    it('emits input event with empty array as payload', () => {
+      createComponent();
+
+      findAlert().vm.$emit('dismiss');
+
+      expect(wrapper.emitted('input')).toEqual([[[]]]);
+    });
+  });
+});

spec/graphql/types/organizations/organization_type_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe GitlabSchema.types['Organization'], feature_category: :cell do
-  let(:expected_fields) { %w[groups id name organization_users path] }
+  let(:expected_fields) { %w[groups id name organization_users path web_url] }
 
   specify { expect(described_class.graphql_name).to eq('Organization') }
   specify { expect(described_class).to require_graphql_authorizations(:read_organization) }

spec/lib/gitlab/url_builder_spec.rb

@@ -30,6 +30,7 @@
       :project_snippet   | ->(snippet)       { "/#{snippet.project.full_path}/-/snippets/#{snippet.id}" }
       :project_wiki      | ->(wiki)          { "/#{wiki.container.full_path}/-/wikis/home" }
       :release           | ->(release)       { "/#{release.project.full_path}/-/releases/#{release.tag}" }
+      :organization      | ->(organization)  { "/-/organizations/#{organization.path}" }
       :ci_build          | ->(build)         { "/#{build.project.full_path}/-/jobs/#{build.id}" }
       :design            | ->(design)        { "/#{design.project.full_path}/-/design_management/designs/#{design.id}/raw_image" }