Refactor to better adhere to SRP

ee/app/helpers/ee/trial_helper.rb

@@ -14,23 +14,26 @@ module EE
     end
 
     def namespace_options_for_select(selected = nil)
-      groups = current_user.manageable_groups_eligible_for_trial.map { |g| [g.name, g.id] }
-      user_namespace = current_user.namespace
-      users = if user_namespace.gitlab_subscription&.trial?
-                []
-              else
-                [[user_namespace.name, user_namespace.id]]
-              end
-
       grouped_options = {
         'New' => [[_('Create group'), 0]],
-        'Groups' => groups,
-        'Users' => users
+        'Groups' => trial_groups,
+        'Users' => trial_users
       }
 
       grouped_options_for_select(grouped_options, selected, prompt: _('Please select'))
     end
 
+    def trial_users
+      user_namespace = current_user.namespace
+      return [] if user_namespace.gitlab_subscription&.trial?
+
+      [[user_namespace.name, user_namespace.id]]
+    end
+
+    def trial_groups
+      current_user.manageable_groups_eligible_for_trial.map { |g| [g.name, g.id] }
+    end
+
     def show_trial_errors?(namespace, service_result)
       namespace&.invalid? || (service_result && !service_result[:success])
     end

ee/spec/helpers/ee/trial_helper_spec.rb

@@ -7,142 +7,152 @@ RSpec.describe EE::TrialHelper do
 
   describe '#namespace_options_for_select' do
     let_it_be(:user) { create :user }
+    let_it_be(:group1) { create :group }
+    let_it_be(:group2) { create :group }
 
-    let(:expected_prompt_option) { 'Please select' }
-    let(:expected_new_options) { [['Create group', 0]] }
     let(:expected_group_options) { [] }
     let(:expected_user_options) { [[user.namespace.name, user.namespace.id]] }
     let(:generated_html) do
       grouped_options_for_select({
-        'New' => expected_new_options,
+        'New' => [['Create group', 0]],
         'Groups' => expected_group_options,
         'Users' => expected_user_options
-      }, nil, prompt: expected_prompt_option)
+      }, nil, prompt: 'Please select')
     end
 
     before do
-      user.reload # necessary to cache-bust the user.namespace.gitlab_subscription object
-      allow(helper).to receive(:current_user).and_return(user)
+      allow(helper).to receive(:trial_groups).and_return(expected_group_options)
+      allow(helper).to receive(:trial_users).and_return(expected_user_options)
     end
 
     subject { helper.namespace_options_for_select }
 
-    context 'when a user owns no groups beyond their personal namespace' do
-      it 'shows “Create group” & user namespace options' do
-        expect(subject).to eq(generated_html)
+    context 'when the user’s namespace can be trialed' do
+      context 'and the user has no groups or none of their groups can be trialed' do
+        it { is_expected.to eq(generated_html) }
       end
 
-      context 'when they have already trialed their personal namespace' do
-        before do
-          create :gitlab_subscription, namespace: user.namespace, trial: true
-        end
-
-        let(:expected_user_options) { [] }
+      context 'and the user has some groups which can be trialed' do
+        let(:expected_group_options) { [group1, group2].map {|g| [g.name, g.id]} }
 
-        it 'only shows “Create group” option' do
-          expect(subject).to eq(generated_html)
-        end
+        it { is_expected.to eq(generated_html) }
       end
     end
 
-    context 'when a user is an owner of a group beyond their personal namespace' do
-      let_it_be(:group) { create :group }
+    context 'when the user’s namespace has already been trialed' do
+      let(:expected_user_options) { [] }
 
-      before do
-        group.add_owner(user)
+      context 'and the user has no groups or none of their groups can be trialed' do
+        it { is_expected.to eq(generated_html) }
       end
 
-      context 'when the group is eligible for a trial' do
-        let(:expected_group_options) { [[group.name, group.id]] }
+      context 'and the user has some groups which can be trialed' do
+        let(:expected_group_options) { [group1, group2].map {|g| [g.name, g.id]} }
 
-        it 'shows options for “Create group,” for the group namespace, & for their user namespace' do
-          expect(subject).to eq(generated_html)
-        end
+        it { is_expected.to eq(generated_html) }
       end
+    end
+  end
 
-      context 'when they have already trialed the group' do
-        before do
-          create :gitlab_subscription, namespace: group, trial: true
-        end
+  describe '#trial_users' do
+    let_it_be(:user) { create :user }
+    let(:user_eligible_for_trial_result) { [[user.namespace.name, user.namespace.id]] }
+    let(:user_ineligible_for_trial_result) { [] }
 
-        it 'shows options for “Create group” & their user namespace, but not for the group' do
-          expect(subject).to eq(generated_html)
-        end
-      end
+    before do
+      user.reload # necessary to cache-bust the user.namespace.gitlab_subscription object
+      allow(helper).to receive(:current_user).and_return(user)
+    end
 
-      context 'when they have already trialed their personal namespace' do
-        before do
-          create :gitlab_subscription, namespace: user.namespace, trial: true
-        end
+    subject { helper.trial_users }
 
-        let(:expected_group_options) { [[group.name, group.id]] }
-        let(:expected_user_options) { [] }
+    context 'when the user has no subscription on their namespace' do
+      it { is_expected.to eq(user_eligible_for_trial_result) }
+    end
 
-        it 'shows options for “Create group” & the group namespace, but not their user namespace' do
-          expect(subject).to eq(generated_html)
-        end
-      end
+    context 'when the user has a subscription on their namespace' do
+      let(:trialed) { false }
+      let!(:subscription) { create :gitlab_subscription, namespace: user.namespace, trial: trialed }
 
-      context 'when they have already trialed the group & their personal namespace' do
-        before do
-          create :gitlab_subscription, namespace: user.namespace, trial: true
-          create :gitlab_subscription, namespace: group, trial: true
-        end
+      context 'and the user has not yet trialed their namespace' do
+        it { is_expected.to eq(user_eligible_for_trial_result) }
+      end
 
-        let(:expected_user_options) { [] }
+      context 'and the user has already trialed their namespace' do
+        let(:trialed) { true }
 
-        it 'only shows the “Create group” option' do
-          expect(subject).to eq(generated_html)
-        end
+        it { is_expected.to eq(user_ineligible_for_trial_result) }
       end
     end
+  end
 
-    context 'when a user is an owner or maintainer of several namespaces' do
+  describe '#trial_groups' do
+    let_it_be(:user) { create :user }
+    let(:no_groups) { [] }
+
+    before do
+      allow(helper).to receive(:current_user).and_return(user)
+    end
+
+    subject { helper.trial_groups }
+
+    context 'when the user is not an owner/maintainer of any groups' do
+      it { is_expected.to eq(no_groups) }
+    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(:subgroup) { create :group, parent: group1, name: 'Sub-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(:subsubgroup) { create :group, parent: subgroup2, name: 'Sub-Sub-Group 1' }
+      let_it_be(:subsubgroup1) { create :group, parent: subgroup2, name: 'Sub-Sub-Group 1' }
+
+      let(:all_groups) { [group1, group2, subgroup1, subgroup2, subsubgroup1].map {|g| [g.name, g.id] } }
 
       before do
         group1.add_owner(user)
         group2.add_maintainer(user)
       end
 
-      context 'when each namespace is eligible for a trial' do
-        let(:expected_group_options) do
-          [
-            [group1.name, group1.id],
-            [group2.name, group2.id],
-            [subgroup.name, subgroup.id],
-            [subgroup2.name, subgroup2.id],
-            [subsubgroup.name, subsubgroup.id]
-          ]
-        end
-
-        it 'shows options for “Create group,” for each group namespace, & for their user namespace' do
-          expect(subject).to eq(generated_html)
-        end
+      context 'and none of the groups have subscriptions' do
+        it { is_expected.to eq(all_groups) }
       end
 
-      context 'when they have already trialed for their user & some of the groups' do
-        before do
-          create :gitlab_subscription, namespace: user.namespace, trial: true
-          create :gitlab_subscription, namespace: group1, trial: true
-          create :gitlab_subscription, namespace: group2, trial: true
-          create :gitlab_subscription, namespace: subgroup, trial: true
+      context 'and the groups have subscriptions' do
+        let(:trialed_group1) { false }
+        let(:trialed_subgroup1) { false }
+        let(:trialed_group2) { false }
+        let(:trialed_subgroup2) { false }
+        let(:trialed_subsubgroup1) { false }
+
+        let!(:subscription_group1) { create :gitlab_subscription, namespace: group1, trial: trialed_group1 }
+        let!(:subscription_subgroup1) { create :gitlab_subscription, namespace: subgroup1, trial: trialed_subgroup1 }
+        let!(:subscription_group2) { create :gitlab_subscription, namespace: group2, trial: trialed_group2 }
+        let!(:subscription_subgroup2) { create :gitlab_subscription, namespace: subgroup2, trial: trialed_subgroup2 }
+        let!(:subscription_subsubgroup1) { create :gitlab_subscription, namespace: subsubgroup1, trial: trialed_subsubgroup1 }
+
+        context 'and none of the groups have been trialed yet' do
+          it { is_expected.to eq(all_groups) }
         end
 
-        let(:expected_group_options) do
-          [
-            [subgroup2.name, subgroup2.id],
-            [subsubgroup.name, subsubgroup.id]
-          ]
+        context 'and some of the groups have been trialed' do
+          let(:trialed_group1) { true }
+          let(:trialed_subgroup1) { true }
+          let(:trialed_subgroup2) { true }
+
+          let(:some_groups) { [group2, subsubgroup1].map {|g| [g.name, g.id]} }
+
+          it { is_expected.to eq(some_groups) }
         end
-        let(:expected_user_options) { [] }
 
-        it 'only shows options for “Create group” & for the non-trialed groups' do
-          expect(subject).to eq(generated_html)
+        context 'and all of the groups have already been trialed' do
+          let(:trialed_group1) { true }
+          let(:trialed_subgroup1) { true }
+          let(:trialed_group2) { true }
+          let(:trialed_subgroup2) { true }
+          let(:trialed_subsubgroup1) { true }
+
+          it { is_expected.to eq(no_groups) }
         end
       end
     end