Merge branch '243911-improve-new-trial-flow-for-edge-case-scenario' into 'master'

Improve New Trial flow for edge case scenario

See merge request gitlab-org/gitlab!41020

ee/app/helpers/ee/trial_helper.rb

@@ -23,6 +23,22 @@ module EE
       end
     end
 
+    def trial_selection_intro_text
+      if any_trial_user_namespaces? && any_trial_group_namespaces?
+        s_('Trials|You can apply your trial to a new group, an existing group, or your personal account.')
+      elsif any_trial_user_namespaces?
+        s_('Trials|You can apply your trial to a new group or your personal account.')
+      elsif any_trial_group_namespaces?
+        s_('Trials|You can apply your trial to a new group or an existing group.')
+      else
+        s_('Trials|Create a new group to start your GitLab Gold trial.')
+      end
+    end
+
+    def show_trial_namespace_select?
+      any_trial_group_namespaces? || any_trial_user_namespaces?
+    end
+
     def namespace_options_for_select(selected = nil)
       grouped_options = {
         'New' => [[_('Create group'), 0]],
@@ -33,21 +49,35 @@ module EE
       grouped_options_for_select(grouped_options, selected, prompt: _('Please select'))
     end
 
+    def show_trial_errors?(namespace, service_result)
+      namespace&.invalid? || (service_result && !service_result[:success])
+    end
+
+    def trial_errors(namespace, service_result)
+      namespace&.errors&.full_messages&.to_sentence&.presence || service_result&.dig(:errors)&.presence
+    end
+
+    private
+
     def trial_group_namespaces
-      current_user.manageable_groups_eligible_for_trial
+      strong_memoize(:trial_group_namespaces) do
+        current_user.manageable_groups_eligible_for_trial
+      end
     end
 
     def trial_user_namespaces
-      user_namespace = current_user.namespace
-      user_namespace.eligible_for_trial? ? [user_namespace] : []
+      strong_memoize(:trial_user_namespaces) do
+        user_namespace = current_user.namespace
+        user_namespace.eligible_for_trial? ? [user_namespace] : []
+      end
     end
 
-    def show_trial_errors?(namespace, service_result)
-      namespace&.invalid? || (service_result && !service_result[:success])
+    def any_trial_group_namespaces?
+      trial_group_namespaces.any?
     end
 
-    def trial_errors(namespace, service_result)
-      namespace&.errors&.full_messages&.to_sentence&.presence || service_result&.dig(:errors)&.presence
+    def any_trial_user_namespaces?
+      trial_user_namespaces.any?
     end
   end
 end

ee/app/views/trials/select.html.haml

@@ -4,17 +4,18 @@
   = _('Almost there')
 
 %p.center
-  = _('You can apply your Trial to your Personal account or create a New Group.')
+  = trial_selection_intro_text
 
 = render 'errors'
 
 = form_tag apply_trials_path(glm_params), method: :post do
-  .form-group.gl-select2-html5-required-fix
-    = label_tag :namespace_id, _('This subscription is for'), for: :namespace_id, class: 'col-form-label'
-    = select_tag :namespace_id, namespace_options_for_select(params[:namespace_id]), class: 'select2', required: true
-  #group_name.form-group.hidden
+  - if show_trial_namespace_select?
+    .form-group.gl-select2-html5-required-fix
+      = label_tag :namespace_id, _('This subscription is for'), for: :namespace_id, class: 'col-form-label'
+      = select_tag :namespace_id, namespace_options_for_select(params[:namespace_id]), class: 'select2', required: true
+  #group_name.form-group{ class: ("hidden" if show_trial_namespace_select?) }
     = 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'
+    = text_field_tag :new_group_name, nil, class: 'form-control', required: !show_trial_namespace_select?
   - if should_ask_company_question?
     .form-group
       = label_tag :trial_entity, _('Is this GitLab trial for your company?')

ee/changelogs/unreleased/243911-improve-new-trial-flow-for-edge-case-scenario.yml

+---
+title: Improve the UX of the Start a Trial group selection page
+merge_request: 41020
+author:
+type: changed

ee/spec/helpers/ee/trial_helper_spec.rb

@@ -69,13 +69,9 @@ RSpec.describe EE::TrialHelper do
     let(:trial_user_namespaces) { [] }
     let(:trial_group_namespaces) { [] }
 
-    let(:generated_html) do
-      grouped_options_for_select({
-        'New' => [['Create group', 0]],
-        'Groups' => trial_group_namespaces.map { |g| [g.name, g.id] },
-        'Users' => trial_user_namespaces.map { |n| [n.name, n.id] }
-      }, nil, prompt: 'Please select')
-    end
+    let(:new_optgroup_regex) { /<optgroup label="New"><option/ }
+    let(:groups_optgroup_regex) { /<optgroup label="Groups"><option/ }
+    let(:users_optgroup_regex) { /<optgroup label="Users"><option/ }
 
     before do
       allow(helper).to receive(:trial_group_namespaces).and_return(trial_group_namespaces)
@@ -84,120 +80,83 @@ RSpec.describe EE::TrialHelper do
 
     subject { helper.namespace_options_for_select }
 
-    where(can_trial_user: [true, false], can_trial_groups: [true, false])
-
-    with_them do
-      context "when the user’s namespace #{params[:can_trial_user] ? 'can be' : 'has already been'} trialed" do
-        let(:trial_user_namespaces) { can_trial_user ? [user.namespace] : [] }
-
-        context "and the user has #{params[:can_trial_groups] ? 'some groups which' : 'no groups or none of their groups'} can be trialed" do
-          let(:trial_group_namespaces) { can_trial_groups ? [group1, group2] : [] }
-
-          it { is_expected.to eq(generated_html) }
-        end
+    context 'when there are no eligible group or user namespaces' do
+      it 'returns just the "New" option group', :aggregate_failures do
+        is_expected.to match(new_optgroup_regex)
+        is_expected.not_to match(groups_optgroup_regex)
+        is_expected.not_to match(users_optgroup_regex)
       end
     end
-  end
 
-  describe '#trial_group_namespaces' do
-    let_it_be(:user) { create :user }
-    let(:no_groups) { [] }
+    context 'when only group namespaces are eligible' do
+      let(:trial_group_namespaces) { [group1, group2] }
 
-    before do
-      allow(helper).to receive(:current_user).and_return(user)
+      it 'returns the "New" and "Groups" option groups', :aggregate_failures do
+        is_expected.to match(new_optgroup_regex)
+        is_expected.to match(groups_optgroup_regex)
+        is_expected.not_to match(users_optgroup_regex)
+      end
     end
 
-    subject { helper.trial_group_namespaces.map(&:id) }
+    context 'when only the user namespace is eligible' do
+      let(:trial_user_namespaces) { [user.namespace] }
 
-    context 'when the user is not an owner/maintainer of any groups' do
-      it { is_expected.to eq(no_groups) }
+      it 'returns the "New" and "Users" option groups', :aggregate_failures do
+        is_expected.to match(new_optgroup_regex)
+        is_expected.to match(users_optgroup_regex)
+        is_expected.not_to match(groups_optgroup_regex)
+      end
     end
 
-    context 'when the user is an owner/maintainer of some groups' do
-      let_it_be(:group1) { create :group, name: 'Group 1' }
-      let_it_be(:subgroup1) { create :group, parent: group1, name: 'Sub-Group 1' }
-      let_it_be(:group2) { create :group, name: 'Group 2' }
-      let_it_be(:subgroup2) { create :group, parent: group2, name: 'Sub-Group 2' }
-      let_it_be(:subsubgroup1) { create :group, parent: subgroup2, name: 'Sub-Sub-Group 1' }
-
-      let(:top_level_groups) { [group1, group2].map(&:id) }
-
-      before do
-        group1.add_owner(user)
-        group2.add_maintainer(user)
-      end
+    context 'when some group namespaces & the user namespace are eligible' do
+      let(:trial_group_namespaces) { [group1, group2] }
+      let(:trial_user_namespaces) { [user.namespace] }
 
-      context 'and none of the groups have subscriptions' do
-        it { is_expected.to eq(top_level_groups) }
+      it 'returns the "New", "Groups", and "Users" option groups', :aggregate_failures do
+        is_expected.to match(new_optgroup_regex)
+        is_expected.to match(groups_optgroup_regex)
+        is_expected.to match(users_optgroup_regex)
       end
+    end
+  end
 
-      context 'and the groups have subscriptions' do
-        let(:group1_traits) { nil }
-        let(:subgroup1_traits) { nil }
-        let(:group2_traits) { nil }
-        let(:subgroup2_traits) { nil }
-        let(:subsubgroup1_traits) { nil }
-
-        let!(:subscription_group1) { create :gitlab_subscription, :free, *group1_traits, namespace: group1 }
-        let!(:subscription_subgroup1) { create :gitlab_subscription, :free, *subgroup1_traits, namespace: subgroup1 }
-        let!(:subscription_group2) { create :gitlab_subscription, :free, *group2_traits, namespace: group2 }
-        let!(:subscription_subgroup2) { create :gitlab_subscription, :free, *subgroup2_traits, namespace: subgroup2 }
-        let!(:subscription_subsubgroup1) { create :gitlab_subscription, :free, *subsubgroup1_traits, namespace: subsubgroup1 }
-
-        context 'and none of the groups have been trialed yet' do
-          it { is_expected.to eq(top_level_groups) }
-        end
-
-        context 'and some of the groups are being or have been trialed' do
-          let(:group1_traits) { :active_trial }
-          let(:subgroup1_traits) { :expired_trial }
-          let(:subgroup2_traits) { :active_trial }
+  describe '#trial_selection_intro_text' do
+    before do
+      allow(helper).to receive(:any_trial_user_namespaces?).and_return(have_user_namespace)
+      allow(helper).to receive(:any_trial_group_namespaces?).and_return(have_group_namespace)
+    end
 
-          it { is_expected.to eq([group2.id]) }
-        end
+    subject { helper.trial_selection_intro_text }
 
-        context 'and all of the groups are being or have been trialed' do
-          let(:group1_traits) { :expired_trial }
-          let(:subgroup1_traits) { :active_trial }
-          let(:group2_traits) { :expired_trial }
-          let(:subgroup2_traits) { :active_trial }
-          let(:subsubgroup1_traits) { :expired_trial }
+    where(:have_user_namespace, :have_group_namespace, :text) do
+      true  | true  | 'You can apply your trial to a new group, an existing group, or your personal account.'
+      true  | false | 'You can apply your trial to a new group or your personal account.'
+      false | true  | 'You can apply your trial to a new group or an existing group.'
+      false | false | 'Create a new group to start your GitLab Gold trial.'
+    end
 
-          it { is_expected.to eq(no_groups) }
-        end
-      end
+    with_them do
+      it { is_expected.to eq(text) }
     end
   end
 
-  describe '#trial_user_namespaces' do
-    let_it_be(:user) { create :user }
-    let(:user_eligible_for_trial_result) { [user.namespace] }
-    let(:user_ineligible_for_trial_result) { [] }
-
+  describe '#show_trial_namespace_select?' do
     before do
-      allow(helper).to receive(:current_user).and_return(user)
-      allow(::Gitlab).to receive(:com?).and_return(true)
+      allow(helper).to receive(:any_trial_group_namespaces?).and_return(have_group_namespace)
+      allow(helper).to receive(:any_trial_user_namespaces?).and_return(have_user_namespace)
     end
 
-    subject { helper.trial_user_namespaces }
+    subject { helper.show_trial_namespace_select? }
 
-    context 'when the user has no subscription on their namespace' do
-      it { is_expected.to eq(user_eligible_for_trial_result) }
+    where(:have_user_namespace, :have_group_namespace, :result) do
+      true  | true  | true
+      true  | false | true
+      false | true  | true
+      false | false | false
     end
 
-    context 'when the user has a subscription on their namespace' do
-      let(:traits) { nil }
-      let!(:subscription) { create :gitlab_subscription, :free, *traits, namespace: user.namespace }
-
-      context 'and the user has not yet trialed their namespace' do
-        it { is_expected.to eq(user_eligible_for_trial_result) }
-      end
-
-      context 'and the user has already trialed their namespace' do
-        let(:traits) { :expired_trial }
-
-        it { is_expected.to eq(user_ineligible_for_trial_result) }
-      end
+    with_them do
+      it { is_expected.to eq(result) }
     end
   end
 

locale/gitlab.pot

@@ -26696,6 +26696,9 @@ msgstr ""
 msgid "Trending"
 msgstr ""
 
+msgid "Trials|Create a new group to start your GitLab Gold trial."
+msgstr ""
+
 msgid "Trials|Go back to GitLab"
 msgstr ""
 
@@ -26705,6 +26708,15 @@ msgstr ""
 msgid "Trials|You can always resume this process by selecting your avatar and choosing 'Start a Gold trial'"
 msgstr ""
 
+msgid "Trials|You can apply your trial to a new group or an existing group."
+msgstr ""
+
+msgid "Trials|You can apply your trial to a new group or your personal account."
+msgstr ""
+
+msgid "Trials|You can apply your trial to a new group, an existing group, or your personal account."
+msgstr ""
+
 msgid "Trials|You won't get a free trial right now but you can always resume this process by clicking on your avatar and choosing 'Start a free trial'"
 msgstr ""
 
@@ -28775,9 +28787,6 @@ msgstr ""
 msgid "You can always edit this later"
 msgstr ""
 
-msgid "You can apply your Trial to your Personal account or create a New Group."
-msgstr ""
-
 msgid "You can create a new one or check them in your %{pat_link_start}personal access tokens%{pat_link_end} settings"
 msgstr ""