Skip to content
Snippets Groups Projects
Commit 1cc4fee7 authored by Doug Stull's avatar Doug Stull :two: Committed by Frédéric Caplette
Browse files

Polish the trials form and reduce code duplication

- reduce duplication and confusion
- polish UI

Changelog: other
EE: true
parent ee56441e
No related branches found
No related tags found
1 merge request!120536Move new group name input field fully into javascript and polish form
...@@ -23,7 +23,7 @@ export default { ...@@ -23,7 +23,7 @@ export default {
<gl-alert <gl-alert
variant="danger" variant="danger"
:dismissible="false" :dismissible="false"
class="gl-mt-3" class="gl-mt-3 gl-mb-5"
:title="$options.i18n.errorsFound" :title="$options.i18n.errorsFound"
data-testid="alert-danger" data-testid="alert-danger"
> >
......
...@@ -11,8 +11,13 @@ export default { ...@@ -11,8 +11,13 @@ export default {
defaultToggleText: __('Please select a group'), defaultToggleText: __('Please select a group'),
newGroupNameLabel: __('New Group Name'), newGroupNameLabel: __('New Group Name'),
}, },
inputSize: { md: 'lg' },
components: { GlFormGroup, GlFormInput, ListboxInput }, components: { GlFormGroup, GlFormInput, ListboxInput },
props: { props: {
anyTrialEligibleNamespaces: {
type: Boolean,
required: true,
},
items: { items: {
type: Array, type: Array,
required: true, required: true,
...@@ -22,15 +27,21 @@ export default { ...@@ -22,15 +27,21 @@ export default {
required: false, required: false,
default: null, default: null,
}, },
newGroupName: {
type: String,
required: false,
default: null,
},
}, },
data() { data() {
return { return {
selectedGroup: this.initialValue, selectedGroup: this.initialValue,
groupName: this.newGroupName,
}; };
}, },
computed: { computed: {
showNewGroupNameField() { showNewGroupNameField() {
return this.selectedGroup === CREATE_GROUP_OPTION_VALUE; return !this.anyTrialEligibleNamespaces || this.selectedGroup === CREATE_GROUP_OPTION_VALUE;
}, },
}, },
}; };
...@@ -39,15 +50,23 @@ export default { ...@@ -39,15 +50,23 @@ export default {
<template> <template>
<div> <div>
<listbox-input <listbox-input
v-if="anyTrialEligibleNamespaces"
v-model="selectedGroup" v-model="selectedGroup"
name="namespace_id" name="namespace_id"
label-for="namespace_id"
data-qa-selector="subscription_for" data-qa-selector="subscription_for"
:label="$options.i18n.groupSelectLabel" :label="$options.i18n.groupSelectLabel"
:items="items" :items="items"
:default-toggle-text="$options.i18n.defaultToggleText" :default-toggle-text="$options.i18n.defaultToggleText"
/> />
<gl-form-group v-if="showNewGroupNameField" :label="$options.i18n.newGroupNameLabel"> <gl-form-group
v-if="showNewGroupNameField"
:label="$options.i18n.newGroupNameLabel"
label-for="new_group_name"
>
<gl-form-input <gl-form-input
v-model="groupName"
:size="$options.inputSize"
name="new_group_name" name="new_group_name"
data-qa-selector="new_group_name" data-qa-selector="new_group_name"
data-testid="new-group-name-input" data-testid="new-group-name-input"
......
import Vue from 'vue'; import Vue from 'vue';
import { parseBoolean } from '~/lib/utils/common_utils';
import NamespaceSelector from './components/namespace_selector.vue'; import NamespaceSelector from './components/namespace_selector.vue';
const SELECTOR = '.js-namespace-selector'; const SELECTOR = '.js-namespace-selector';
...@@ -11,13 +12,15 @@ export const initNamespaceSelector = () => { ...@@ -11,13 +12,15 @@ export const initNamespaceSelector = () => {
} }
const items = JSON.parse(el.dataset.items); const items = JSON.parse(el.dataset.items);
const { initialValue } = el.dataset; const { anyTrialEligibleNamespaces, newGroupName, initialValue } = el.dataset;
return new Vue({ return new Vue({
el, el,
render: (createElement) => render: (createElement) =>
createElement(NamespaceSelector, { createElement(NamespaceSelector, {
props: { props: {
anyTrialEligibleNamespaces: parseBoolean(anyTrialEligibleNamespaces),
newGroupName,
initialValue, initialValue,
items, items,
}, },
......
...@@ -14,15 +14,9 @@ ...@@ -14,15 +14,9 @@
.js-error-alert{ data: { errors: @create_errors } } .js-error-alert{ data: { errors: @create_errors } }
= form_tag apply_trials_path(glm_params), method: :post, class: 'js-saas-trial-group' do = form_tag apply_trials_path(glm_params), method: :post, class: 'js-saas-trial-group', data: { testid: 'trial-form' } do
- if show_trial_namespace_select? .js-namespace-selector{ data: { any_trial_eligible_namespaces: any_trial_eligible_namespaces?.to_s, new_group_name: params[:new_group_name],
.js-namespace-selector{ data: { items: namespace_options_for_listbox.to_json, items: namespace_options_for_listbox.to_json, initial_value: params[:namespace_id] } }
initial_value: params[:namespace_id] } }
- else
#group_name.form-group
= label_tag :new_group_name, _('New Group Name'), for: :new_group_name, class: 'col-form-label'
= text_field_tag :new_group_name, nil, class: 'form-control', required: !show_trial_namespace_select?,
data: { qa_selector: 'new_group_name' }
- if should_ask_company_question? - if should_ask_company_question?
.form-group .form-group
= label_tag :trial_entity, _('Who will be using GitLab?') = label_tag :trial_entity, _('Who will be using GitLab?')
...@@ -35,7 +29,7 @@ ...@@ -35,7 +29,7 @@
= radio_button_tag :trial_entity, :individual, params[:trial_entity]=='individual', required: true, = radio_button_tag :trial_entity, :individual, params[:trial_entity]=='individual', required: true,
class: 'form-check-input', data: { qa_selector: 'trial_individual' } class: 'form-check-input', data: { qa_selector: 'trial_individual' }
= label_tag :trial_entity_individual, _('Just me'), class: 'form-check-label' = label_tag :trial_entity_individual, _('Just me'), class: 'form-check-label'
= submit_tag _('Start your free trial'), class: 'gl-button btn btn-confirm gl-w-20', = submit_tag _('Start your free trial'), class: 'gl-button btn btn-confirm gl-w-20 gl-xs-w-full',
data: { qa_selector: 'start_your_free_trial' } data: { qa_selector: 'start_your_free_trial' }
.col-md-4.gl-display-inline-flex.gl-vertical-align-middle .col-md-4.gl-display-inline-flex.gl-vertical-align-middle
......
...@@ -94,11 +94,9 @@ ...@@ -94,11 +94,9 @@
click_button 'Start your free trial' click_button 'Start your free trial'
# We really shouldn't be showing the selector at this point. # We really shouldn't be showing the selector at this point.
# The input should also have the currently invalid group name.
# issue: https://gitlab.com/gitlab-org/gitlab/-/issues/405125 # issue: https://gitlab.com/gitlab-org/gitlab/-/issues/405125
expect_to_be_on_namespace_selection expect_to_be_on_namespace_selection
expect(page).to have_content('We have found the following errors') expect_to_have_namespace_creation_errors
expect(page).to have_content('Group URL must not start or end with a special character')
# success # success
fill_in_trial_selection_form_for_new_group fill_in_trial_selection_form_for_new_group
...@@ -130,11 +128,9 @@ ...@@ -130,11 +128,9 @@
click_button 'Start your free trial' click_button 'Start your free trial'
# We really shouldn't be showing the selector at this point. # We really shouldn't be showing the selector at this point.
# The input should also have the currently invalid group name.
# issue: https://gitlab.com/gitlab-org/gitlab/-/issues/405125 # issue: https://gitlab.com/gitlab-org/gitlab/-/issues/405125
expect_to_be_on_namespace_selection expect_to_be_on_namespace_selection
expect(page).to have_content('We have found the following errors') expect_to_have_namespace_creation_errors
expect(page).to have_content('Group URL must not start or end with a special character')
# success when choosing an existing namespace instead # success when choosing an existing namespace instead
fill_in_trial_selection_form(from: 'Create group') fill_in_trial_selection_form(from: 'Create group')
...@@ -202,7 +198,9 @@ ...@@ -202,7 +198,9 @@
end end
def fill_in_trial_selection_form_for_new_group(name: 'gitlab') def fill_in_trial_selection_form_for_new_group(name: 'gitlab')
expect(page).to have_selector('fieldset', text: 'New Group Name') within('[data-testid="trial-form"]') do
expect(page).to have_text('New Group Name')
end
fill_in_trial_form_for_new_group(name: name, glm_source: nil) fill_in_trial_form_for_new_group(name: name, glm_source: nil)
end end
......
...@@ -19,18 +19,19 @@ ...@@ -19,18 +19,19 @@
expect_to_be_on_namespace_creation expect_to_be_on_namespace_creation
click_button 'Start your free trial'
# required field check
message = page.find_field('new_group_name').native.attribute('validationMessage')
expect(message).to eq('Please fill out this field.')
# namespace invalid check # namespace invalid check
fill_in_trial_form_for_new_group(name: '_invalid group name_') fill_in_trial_form_for_new_group(name: '_invalid group name_')
click_button 'Start your free trial' click_button 'Start your free trial'
expect_to_be_on_namespace_creation expect_to_be_on_namespace_creation
expect(page).to have_content('We have found the following errors') expect_to_have_namespace_creation_errors
expect(page).to have_content('Group URL must not start or end with a special character')
# required field check
message = page.find_field('new_group_name').native.attribute('validationMessage')
expect(message).to eq('Please fill out this field.')
# success # success
fill_in_trial_form_for_new_group fill_in_trial_form_for_new_group
......
...@@ -18,15 +18,24 @@ describe('NamespaceSelector', () => { ...@@ -18,15 +18,24 @@ describe('NamespaceSelector', () => {
wrapper = shallowMountExtended(NamespaceSelector, { wrapper = shallowMountExtended(NamespaceSelector, {
propsData: { propsData: {
items, items,
anyTrialEligibleNamespaces: true,
...props, ...props,
}, },
}); });
}; };
it('passes the item to the listbox input', () => { describe('listbox input', () => {
createComponent(); it('passes the item to the listbox input', () => {
createComponent();
expect(findListboxInput().props('items')).toBe(items);
});
expect(findListboxInput().props('items')).toBe(items); it('is hidden if anyTrialEligibleNamespaces is false', () => {
createComponent({ anyTrialEligibleNamespaces: false });
expect(findListboxInput().exists()).toBe(false);
});
}); });
describe('"New group name" input', () => { describe('"New group name" input', () => {
...@@ -42,6 +51,18 @@ describe('NamespaceSelector', () => { ...@@ -42,6 +51,18 @@ describe('NamespaceSelector', () => {
expect(findNewGroupNameInput().exists()).toBe(true); expect(findNewGroupNameInput().exists()).toBe(true);
}); });
it('is visible and has value if the initially selected option is "Create group"', () => {
createComponent({ newGroupName: '_name_', initialValue: CREATE_GROUP_OPTION_VALUE });
expect(findNewGroupNameInput().attributes('value')).toEqual('_name_');
});
it('is visible and has value if anyTrialEligibleNamespaces is false', () => {
createComponent({ newGroupName: '_name_', anyTrialEligibleNamespaces: false });
expect(findNewGroupNameInput().attributes('value')).toEqual('_name_');
});
it('is revealed when selecting the "Create group" option', async () => { it('is revealed when selecting the "Create group" option', async () => {
createComponent(); createComponent();
await findListboxInput().vm.$emit('select', CREATE_GROUP_OPTION_VALUE); await findListboxInput().vm.$emit('select', CREATE_GROUP_OPTION_VALUE);
......
...@@ -22,6 +22,17 @@ def expect_to_be_on_namespace_selection ...@@ -22,6 +22,17 @@ def expect_to_be_on_namespace_selection
expect(page).to have_content('Who will be using GitLab?') expect(page).to have_content('Who will be using GitLab?')
end end
def expect_to_have_namespace_creation_errors(group_name: '_invalid group name_')
within('[data-testid="alert-danger"]') do
expect(page).to have_content('We have found the following errors')
expect(page).to have_content('Group URL must not start or end with a special character')
end
within('[data-testid="trial-form"]') do
expect(page.find_field('new_group_name').value).to eq(group_name)
end
end
def expect_to_be_on_lead_form_with_errors def expect_to_be_on_lead_form_with_errors
expect(page).to have_content('We have found the following errors') expect(page).to have_content('We have found the following errors')
expect(page).to have_content('_lead_fail_') expect(page).to have_content('_lead_fail_')
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment