Commit f12a3767 authored by Dallas Reedy's avatar Dallas Reedy Committed by Illya Klymov

Extract complex view logic into helper methods

Simplifies the logic of the view, and its related view specs, by pulling
out lengthy & complex bits of logic from the view into two new helper
methods: show_signup_flow_progress_bar? & welcome_submit_button_text
parent 56acd67e
...@@ -23,4 +23,3 @@ ...@@ -23,4 +23,3 @@
= render "registrations/welcome/button" = render "registrations/welcome/button"
- else - else
= f.submit _('Get started!'), class: 'btn-register btn btn-block gl-mb-0 gl-p-3' = f.submit _('Get started!'), class: 'btn-register btn btn-block gl-mb-0 gl-p-3'
...@@ -40,6 +40,27 @@ module EE ...@@ -40,6 +40,27 @@ module EE
end end
end end
def show_signup_flow_progress_bar?
return true if in_subscription_flow?
return false if in_invitation_flow? || in_oauth_flow? || in_trial_flow?
onboarding_issues_experiment_enabled?
end
def welcome_submit_button_text
continue = _('Continue')
get_started = _('Get started!')
return continue if in_subscription_flow? || in_trial_flow?
return get_started if in_invitation_flow? || in_oauth_flow?
onboarding_issues_experiment_enabled? ? continue : get_started
end
def onboarding_issues_experiment_enabled?
experiment_enabled?(:onboarding_issues)
end
private private
def redirect_path def redirect_path
......
- onboarding_issues_experiment_enabled = experiment_enabled?(:onboarding_issues) = button_tag welcome_submit_button_text, class: %w[btn btn-success w-100], data: { qa_selector: 'get_started_button' }
= button_tag class: %w[btn btn-success w-100], data: { qa_selector: 'get_started_button' } do
= in_subscription_flow? || in_trial_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow? && !in_oauth_flow?) ? _('Continue') : _('Get started!')
- onboarding_issues_experiment_enabled = experiment_enabled?(:onboarding_issues) - if show_signup_flow_progress_bar?
#progress-bar{ data: { is_in_subscription_flow: in_subscription_flow?.to_s, is_onboarding_issues_experiment_enabled: onboarding_issues_experiment_enabled?.to_s } }
- if in_subscription_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow? && !in_oauth_flow? && !in_trial_flow?)
#progress-bar{ data: { is_in_subscription_flow: in_subscription_flow?.to_s, is_onboarding_issues_experiment_enabled: onboarding_issues_experiment_enabled.to_s } }
...@@ -106,4 +106,142 @@ RSpec.describe EE::RegistrationsHelper do ...@@ -106,4 +106,142 @@ RSpec.describe EE::RegistrationsHelper do
] ]
end end
end end
shared_context 'with the various user flows' do
let(:in_subscription_flow) { false }
let(:in_invitation_flow) { false }
let(:in_oauth_flow) { false }
let(:in_trial_flow) { false }
before do
allow(helper).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
allow(helper).to receive(:in_invitation_flow?).and_return(in_invitation_flow)
allow(helper).to receive(:in_oauth_flow?).and_return(in_oauth_flow)
allow(helper).to receive(:in_trial_flow?).and_return(in_trial_flow)
end
end
shared_context 'with the onboarding issues experiment' do
let(:onboarding_issues_experiment_enabled) { false }
before do
allow(helper).to receive(:onboarding_issues_experiment_enabled?).and_return(onboarding_issues_experiment_enabled)
end
end
describe '#show_signup_flow_progress_bar?' do
include_context 'with the various user flows'
include_context 'with the onboarding issues experiment'
subject { helper.show_signup_flow_progress_bar? }
context 'when in the subscription flow, regardless of all other flows' do
let(:in_subscription_flow) { true }
where(:in_invitation_flow, :in_oauth_flow, :in_trial_flow) do
true | false | false
false | true | false
false | false | true
end
with_them do
context 'regardless of if the onboarding issues experiment is enabled' do
where(onboarding_issues_experiment_enabled: [true, false])
with_them do
it { is_expected.to be_truthy }
end
end
end
end
context 'when not in the subscription flow' do
context 'but in the invitation, oauth, or trial flow' do
where(:in_invitation_flow, :in_oauth_flow, :in_trial_flow) do
true | false | false
false | true | false
false | false | true
end
with_them do
context 'regardless of if the onboarding issues experiment is enabled' do
where(onboarding_issues_experiment_enabled: [true, false])
with_them do
it { is_expected.to be_falsey }
end
end
end
end
context 'and not in the invitation, oauth, or trial flow' do
where(:onboarding_issues_experiment_enabled, :result) do
true | true
false | false
end
with_them do
it 'depends on whether or not the onboarding issues experiment is enabled' do
is_expected.to eq(result)
end
end
end
end
end
describe '#welcome_submit_button_text' do
include_context 'with the various user flows'
include_context 'with the onboarding issues experiment'
subject { helper.welcome_submit_button_text }
context 'when in the subscription or trial flow' do
where(:in_subscription_flow, :in_trial_flow) do
true | false
false | true
end
with_them do
context 'regardless of if the onboarding issues experiment is enabled' do
where(onboarding_issues_experiment_enabled: [true, false])
with_them do
it { is_expected.to eq('Continue') }
end
end
end
end
context 'when not in the subscription or trial flow' do
context 'but in the invitation or oauth flow' do
where(:in_invitation_flow, :in_oauth_flow) do
true | false
false | true
end
with_them do
context 'regardless of if the onboarding issues experiment is enabled' do
where(onboarding_issues_experiment_enabled: [true, false])
with_them do
it { is_expected.to eq('Get started!') }
end
end
end
end
context 'and not in the invitation or oauth flow' do
where(:onboarding_issues_experiment_enabled, :result) do
true | 'Continue'
false | 'Get started!'
end
with_them do
it 'depends on whether or not the onboarding issues experiment is enabled' do
is_expected.to eq(result)
end
end
end
end
end
end end
...@@ -9,10 +9,7 @@ RSpec.describe 'registrations/welcome' do ...@@ -9,10 +9,7 @@ RSpec.describe 'registrations/welcome' do
before do before do
allow(view).to receive(:current_user).and_return(user) allow(view).to receive(:current_user).and_return(user)
allow(view).to receive(:in_subscription_flow?).and_return(in_subscription_flow) allow(view).to receive(:redirect_path).and_return(redirect_path)
allow(view).to receive(:in_trial_flow?).and_return(in_trial_flow)
allow(view).to receive(:in_invitation_flow?).and_return(in_invitation_flow)
allow(view).to receive(:in_oauth_flow?).and_return(in_oauth_flow)
allow(view).to receive(:experiment_enabled?).with(:onboarding_issues).and_return(onboarding_issues_experiment_enabled) allow(view).to receive(:experiment_enabled?).with(:onboarding_issues).and_return(onboarding_issues_experiment_enabled)
allow(Gitlab).to receive(:com?).and_return(true) allow(Gitlab).to receive(:com?).and_return(true)
...@@ -21,42 +18,36 @@ RSpec.describe 'registrations/welcome' do ...@@ -21,42 +18,36 @@ RSpec.describe 'registrations/welcome' do
subject { rendered } subject { rendered }
where(:in_subscription_flow, :in_trial_flow, :in_invitation_flow, :in_oauth_flow, :onboarding_issues_experiment_enabled, :shows_progress_bar, :label_key, :is_continue_btn) do where(:redirect_path, :onboarding_issues_experiment_enabled, :show_progress_bar, :flow, :is_continue) do
false | false | false | false | false | false | nil | false # regular '/-/subscriptions/new' | false | true | :subscription | true
true | false | false | false | false | true | :subscription | true # subscription '/-/subscriptions/new' | true | true | :subscription | true
false | true | false | false | false | false | :trial | true # trial '/-/trials/new' | false | false | :trial | true
false | false | true | false | false | false | nil | false # invitation '/-/trials/new' | true | false | :trial | true
false | false | false | true | false | false | nil | false # oauth '/-/invites/abc123' | false | false | nil | false
false | false | false | false | true | true | nil | true # onboarding '/-/invites/abc123' | true | false | nil | false
true | false | false | false | true | true | :subscription | true # onboarding + subscription '/oauth/authorize/abc123' | false | false | nil | false
false | true | false | false | true | false | :trial | true # onboarding + trial '/oauth/authorize/abc123' | true | false | nil | false
false | false | true | false | true | false | nil | false # onboarding + invitation nil | false | false | nil | false
false | false | false | true | true | false | nil | false # onboarding + oauth nil | true | true | nil | true
end end
def button_text with_them do
is_continue_btn ? 'Continue' : 'Get started!' it 'shows the correct text for the :setup_for_company label' do
end expected_text = "Who will be using #{flow.nil? ? 'GitLab' : "this GitLab #{flow}"}?"
def label_text is_expected.to have_selector('label[for="user_setup_for_company"]', text: expected_text)
if label_key == :subscription
'Who will be using this GitLab subscription?'
elsif label_key == :trial
'Who will be using this GitLab trial?'
else
'Who will be using GitLab?'
end end
it 'shows the correct text for the submit button' do
expected_text = is_continue ? 'Continue' : 'Get started!'
is_expected.to have_button(expected_text)
end end
with_them do if params[:show_progress_bar]
it { is_expected.to have_button(button_text) } it { is_expected.to have_selector('#progress-bar') }
it { is_expected.to have_selector('label[for="user_setup_for_company"]', text: label_text) }
it do
if shows_progress_bar
is_expected.to have_selector('#progress-bar')
else else
is_expected.not_to have_selector('#progress-bar') it { is_expected.not_to have_selector('#progress-bar') }
end
end end
end end
end end
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