Commit 66a92513 authored by Tom Quirk's avatar Tom Quirk

Address reviewer feedback

- Move i18n to consts
- add better alert-related specs
parent d4a20000
<script> <script>
import { GlFormGroup, GlButton, GlFormInput, GlForm, GlAlert } from '@gitlab/ui'; import { GlFormGroup, GlButton, GlFormInput, GlForm, GlAlert } from '@gitlab/ui';
import { __ } from '~/locale'; import {
CREATE_BRANCH_ERROR_GENERIC,
CREATE_BRANCH_ERROR_WITH_CONTEXT,
CREATE_BRANCH_SUCCESS_ALERT,
I18N_NEW_BRANCH_FORM,
} from '../constants';
import createBranchMutation from '../graphql/mutations/create_branch.mutation.graphql'; import createBranchMutation from '../graphql/mutations/create_branch.mutation.graphql';
import ProjectDropdown from './project_dropdown.vue'; import ProjectDropdown from './project_dropdown.vue';
import SourceBranchDropdown from './source_branch_dropdown.vue'; import SourceBranchDropdown from './source_branch_dropdown.vue';
...@@ -75,7 +80,7 @@ export default { ...@@ -75,7 +80,7 @@ export default {
...DEFAULT_ALERT_PARAMS, ...DEFAULT_ALERT_PARAMS,
}; };
}, },
async onProjectSelect(project) { onProjectSelect(project) {
this.selectedProject = project; this.selectedProject = project;
this.selectedSourceBranchName = null; // reset branch selection this.selectedSourceBranchName = null; // reset branch selection
}, },
...@@ -106,23 +111,20 @@ export default { ...@@ -106,23 +111,20 @@ export default {
const { errors } = data.createBranch; const { errors } = data.createBranch;
if (errors.length > 0) { if (errors.length > 0) {
this.onError({ this.onError({
title: __('Failed to create branch.'), title: CREATE_BRANCH_ERROR_WITH_CONTEXT,
message: errors[0], message: errors[0],
}); });
return; return;
} }
this.displayAlert({ this.displayAlert({
title: __('New branch was successfully created.'), ...CREATE_BRANCH_SUCCESS_ALERT,
message: __('You can now close this window and return to Jira.'),
variant: 'success', variant: 'success',
primaryButtonLink: 'jira',
primaryButtonText: __('Return to Jira'),
}); });
}) })
.catch(() => { .catch(() => {
this.onError({ this.onError({
message: __('Failed to create branch. Please try again.'), message: CREATE_BRANCH_ERROR_GENERIC,
}); });
}) })
.finally(() => { .finally(() => {
...@@ -131,11 +133,7 @@ export default { ...@@ -131,11 +133,7 @@ export default {
}, },
}, },
i18n: { i18n: {
pageTitle: __('New branch'), I18N_NEW_BRANCH_FORM,
projectDropdownLabel: __('Project'),
branchNameInputLabel: __('Branch name'),
sourceBranchDropdownLabel: __('Source branch'),
formSubmitButtonText: __('Create branch'),
}, },
}; };
</script> </script>
...@@ -144,7 +142,7 @@ export default { ...@@ -144,7 +142,7 @@ export default {
<div> <div>
<div class="gl-border-1 gl-border-b-solid gl-border-gray-100 gl-mb-5 gl-mt-7"> <div class="gl-border-1 gl-border-b-solid gl-border-gray-100 gl-mb-5 gl-mt-7">
<h1 class="page-title"> <h1 class="page-title">
{{ $options.i18n.pageTitle }} {{ $options.i18n.I18N_NEW_BRANCH_FORM.pageTitle }}
</h1> </h1>
</div> </div>
...@@ -161,7 +159,10 @@ export default { ...@@ -161,7 +159,10 @@ export default {
</gl-alert> </gl-alert>
<gl-form @submit.prevent="onSubmit"> <gl-form @submit.prevent="onSubmit">
<gl-form-group :label="$options.i18n.projectDropdownLabel" label-for="project-select"> <gl-form-group
:label="$options.i18n.I18N_NEW_BRANCH_FORM.labels.projectDropdown"
label-for="project-select"
>
<project-dropdown <project-dropdown
id="project-select" id="project-select"
:selected-project="selectedProject" :selected-project="selectedProject"
...@@ -170,12 +171,15 @@ export default { ...@@ -170,12 +171,15 @@ export default {
/> />
</gl-form-group> </gl-form-group>
<gl-form-group :label="$options.i18n.branchNameInputLabel" label-for="branch-name-input"> <gl-form-group
:label="$options.i18n.I18N_NEW_BRANCH_FORM.labels.branchNameInput"
label-for="branch-name-input"
>
<gl-form-input id="branch-name-input" v-model="branchName" type="text" required /> <gl-form-input id="branch-name-input" v-model="branchName" type="text" required />
</gl-form-group> </gl-form-group>
<gl-form-group <gl-form-group
:label="$options.i18n.sourceBranchDropdownLabel" :label="$options.i18n.I18N_NEW_BRANCH_FORM.labels.sourceBranchDropdown"
label-for="source-branch-select" label-for="source-branch-select"
> >
<source-branch-dropdown <source-branch-dropdown
...@@ -194,7 +198,7 @@ export default { ...@@ -194,7 +198,7 @@ export default {
variant="confirm" variant="confirm"
:disabled="disableSubmitButton" :disabled="disableSubmitButton"
> >
{{ $options.i18n.formSubmitButtonText }} {{ $options.i18n.I18N_NEW_BRANCH_FORM.formSubmitButtonText }}
</gl-button> </gl-button>
</div> </div>
</gl-form> </gl-form>
......
...@@ -60,7 +60,7 @@ export default { ...@@ -60,7 +60,7 @@ export default {
}, },
}, },
methods: { methods: {
async onProjectSelect(project) { onProjectSelect(project) {
this.$emit('change', project); this.$emit('change', project);
}, },
onError({ message } = {}) { onError({ message } = {}) {
......
import { __ } from '~/locale';
export const BRANCHES_PER_PAGE = 20; export const BRANCHES_PER_PAGE = 20;
export const PROJECTS_PER_PAGE = 20; export const PROJECTS_PER_PAGE = 20;
export const I18N_NEW_BRANCH_FORM = {
pageTitle: __('New branch'),
labels: {
projectDropdown: __('Project'),
branchNameInput: __('Branch name'),
sourceBranchDropdown: __('Source branch'),
},
formSubmitButtonText: __('Create branch'),
};
export const CREATE_BRANCH_ERROR_GENERIC = __('Failed to create branch. Please try again.');
export const CREATE_BRANCH_ERROR_WITH_CONTEXT = __('Failed to create branch.');
export const CREATE_BRANCH_SUCCESS_ALERT = {
title: __('New branch was successfully created.'),
message: __('You can now close this window and return to Jira.'),
};
import initJiraConnectBranches from '~/jira_connect/branches';
initJiraConnectBranches();
...@@ -28032,9 +28032,6 @@ msgstr "" ...@@ -28032,9 +28032,6 @@ msgstr ""
msgid "Retry verification" msgid "Retry verification"
msgstr "" msgstr ""
msgid "Return to Jira"
msgstr ""
msgid "Reveal value" msgid "Reveal value"
msgid_plural "Reveal values" msgid_plural "Reveal values"
msgstr[0] "" msgstr[0] ""
......
...@@ -6,6 +6,11 @@ import waitForPromises from 'helpers/wait_for_promises'; ...@@ -6,6 +6,11 @@ import waitForPromises from 'helpers/wait_for_promises';
import NewBranchForm from '~/jira_connect/branches/components/new_branch_form.vue'; import NewBranchForm from '~/jira_connect/branches/components/new_branch_form.vue';
import ProjectDropdown from '~/jira_connect/branches/components/project_dropdown.vue'; import ProjectDropdown from '~/jira_connect/branches/components/project_dropdown.vue';
import SourceBranchDropdown from '~/jira_connect/branches/components/source_branch_dropdown.vue'; import SourceBranchDropdown from '~/jira_connect/branches/components/source_branch_dropdown.vue';
import {
CREATE_BRANCH_ERROR_GENERIC,
CREATE_BRANCH_ERROR_WITH_CONTEXT,
CREATE_BRANCH_SUCCESS_ALERT,
} from '~/jira_connect/branches/constants';
import createBranchMutation from '~/jira_connect/branches/graphql/mutations/create_branch.mutation.graphql'; import createBranchMutation from '~/jira_connect/branches/graphql/mutations/create_branch.mutation.graphql';
const mockProject = { const mockProject = {
...@@ -16,7 +21,6 @@ const mockProject = { ...@@ -16,7 +21,6 @@ const mockProject = {
rootRef: 'main', rootRef: 'main',
}, },
}; };
const localVue = createLocalVue();
const mockCreateBranchMutationResponse = { const mockCreateBranchMutationResponse = {
data: { data: {
createBranch: { createBranch: {
...@@ -25,12 +29,25 @@ const mockCreateBranchMutationResponse = { ...@@ -25,12 +29,25 @@ const mockCreateBranchMutationResponse = {
}, },
}, },
}; };
const mockCreateBranchMutationResponseWithErrors = {
data: {
createBranch: {
clientMutationId: 1,
errors: ['everything is broken, sorry.'],
},
},
};
const mockCreateBranchMutationSuccess = jest const mockCreateBranchMutationSuccess = jest
.fn() .fn()
.mockResolvedValue(mockCreateBranchMutationResponse); .mockResolvedValue(mockCreateBranchMutationResponse);
const mockCreateBranchMutationWithErrors = jest
.fn()
.mockResolvedValue(mockCreateBranchMutationResponseWithErrors);
const mockCreateBranchMutationFailed = jest.fn().mockRejectedValue(new Error('GraphQL error')); const mockCreateBranchMutationFailed = jest.fn().mockRejectedValue(new Error('GraphQL error'));
const mockMutationLoading = jest.fn().mockReturnValue(new Promise(() => {})); const mockMutationLoading = jest.fn().mockReturnValue(new Promise(() => {}));
const localVue = createLocalVue();
describe('NewBranchForm', () => { describe('NewBranchForm', () => {
let wrapper; let wrapper;
...@@ -57,8 +74,8 @@ describe('NewBranchForm', () => { ...@@ -57,8 +74,8 @@ describe('NewBranchForm', () => {
return mockApollo; return mockApollo;
} }
function createComponent({ mockApollo, mountFn = shallowMount } = {}) { function createComponent({ mockApollo } = {}) {
wrapper = mountFn(NewBranchForm, { wrapper = shallowMount(NewBranchForm, {
localVue, localVue,
apolloProvider: mockApollo || createMockApolloProvider(), apolloProvider: mockApollo || createMockApolloProvider(),
}); });
...@@ -125,10 +142,9 @@ describe('NewBranchForm', () => { ...@@ -125,10 +142,9 @@ describe('NewBranchForm', () => {
it('displays a success message', () => { it('displays a success message', () => {
const alert = findAlert(); const alert = findAlert();
expect(alert.exists()).toBe(true); expect(alert.exists()).toBe(true);
expect(alert.text()).toBe(CREATE_BRANCH_SUCCESS_ALERT.message);
expect(alert.props()).toMatchObject({ expect(alert.props()).toMatchObject({
primaryButtonLink: 'jira', title: CREATE_BRANCH_SUCCESS_ALERT.title,
primaryButtonText: 'Return to Jira',
title: 'New branch was successfully created.',
variant: 'success', variant: 'success',
}); });
}); });
...@@ -147,26 +163,33 @@ describe('NewBranchForm', () => { ...@@ -147,26 +163,33 @@ describe('NewBranchForm', () => {
}); });
describe('when form submission fails', () => { describe('when form submission fails', () => {
beforeEach(async () => { describe.each`
createComponent({ scenario | mutation | alertTitle | alertText
mockApollo: createMockApolloProvider({ ${'with errors-as-data'} | ${mockCreateBranchMutationWithErrors} | ${CREATE_BRANCH_ERROR_WITH_CONTEXT} | ${mockCreateBranchMutationResponseWithErrors.data.createBranch.errors[0]}
mockCreateBranchMutation: mockCreateBranchMutationFailed, ${'top-level error'} | ${mockCreateBranchMutationFailed} | ${''} | ${CREATE_BRANCH_ERROR_GENERIC}
}), `('', ({ mutation, alertTitle, alertText }) => {
beforeEach(async () => {
createComponent({
mockApollo: createMockApolloProvider({
mockCreateBranchMutation: mutation,
}),
});
await completeForm();
await findForm().vm.$emit('submit', new Event('submit'));
await waitForPromises();
});
it('displays an alert', () => {
const alert = findAlert();
expect(alert.exists()).toBe(true);
expect(alert.text()).toBe(alertText);
expect(alert.props()).toMatchObject({ title: alertTitle, variant: 'danger' });
}); });
await completeForm(); it('sets submit button `loading` prop to `false`', () => {
expect(findButton().props('loading')).toBe(false);
await findForm().vm.$emit('submit', new Event('submit')); });
await waitForPromises();
});
it('displays an alert', () => {
const alert = findAlert();
expect(alert.exists()).toBe(true);
});
it('sets submit button `loading` prop to `false`', () => {
expect(findButton().props('loading')).toBe(false);
}); });
}); });
}); });
...@@ -188,6 +211,7 @@ describe('NewBranchForm', () => { ...@@ -188,6 +211,7 @@ describe('NewBranchForm', () => {
const alert = findAlert(); const alert = findAlert();
expect(alert.exists()).toBe(true); expect(alert.exists()).toBe(true);
expect(alert.text()).toBe(mockErrorMessage); expect(alert.text()).toBe(mockErrorMessage);
expect(alert.props('variant')).toBe('danger');
}); });
describe('when alert is dismissed', () => { describe('when alert is dismissed', () => {
......
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