Merge branch...

Merge branch '222455-remove-non-free-plan-subscriptions-from-namespace-list-in-the-trial-flow' into 'master'

Omit non-free namespaces from trial select

See merge request gitlab-org/gitlab!36394

ee/app/helpers/ee/trial_helper.rb

@@ -16,22 +16,20 @@ module EE
     def namespace_options_for_select(selected = nil)
       grouped_options = {
         'New' => [[_('Create group'), 0]],
-        'Groups' => trial_groups,
-        'Users' => trial_users
+        'Groups' => trial_group_namespaces.map { |n| [n.name, n.id] },
+        'Users' => trial_user_namespaces.map { |n| [n.name, n.id] }
       }
 
       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]]
+    def trial_group_namespaces
+      current_user.manageable_groups_eligible_for_trial
     end
 
-    def trial_groups
-      current_user.manageable_groups_eligible_for_trial.map { |g| [g.name, g.id] }
+    def trial_user_namespaces
+      user_namespace = current_user.namespace
+      user_namespace.eligible_for_trial? ? [user_namespace] : []
     end
 
     def show_trial_errors?(namespace, service_result)

ee/app/models/ee/namespace.rb

@@ -37,6 +37,14 @@ module EE
       scope :include_gitlab_subscription, -> { includes(:gitlab_subscription) }
       scope :join_gitlab_subscription, -> { joins("LEFT OUTER JOIN gitlab_subscriptions ON gitlab_subscriptions.namespace_id=namespaces.id") }
 
+      scope :eligible_for_trial, -> do
+        left_joins(gitlab_subscription: :hosted_plan)
+          .where(
+            gitlab_subscriptions: { trial: [nil, false], trial_ends_on: [nil] },
+            plans: { name: [nil, *::Plan::PLANS_ELIGIBLE_FOR_TRIAL] }
+          )
+      end
+
       scope :with_feature_available_in_plan, -> (feature) do
         plans = plans_with_feature(feature)
         matcher = ::Plan.where(name: plans)
@@ -276,9 +284,9 @@ module EE
 
     def eligible_for_trial?
       ::Gitlab.com? &&
-        parent_id.nil? &&
-        trial_ends_on.blank? &&
-        [::Plan::EARLY_ADOPTER, ::Plan::FREE].include?(actual_plan_name)
+        !has_parent? &&
+        never_had_trial? &&
+        plan_eligible_for_trial?
     end
 
     def trial_active?
@@ -290,9 +298,7 @@ module EE
     end
 
     def trial_expired?
-      trial_ends_on.present? &&
-        trial_ends_on < Date.today &&
-        actual_plan_name == ::Plan::FREE
+      trial_ends_on.present? && trial_ends_on < Date.today
     end
 
     # A namespace may not have a file template project
@@ -333,6 +339,10 @@ module EE
       actual_plan_name == ::Plan::GOLD
     end
 
+    def plan_eligible_for_trial?
+      ::Plan::PLANS_ELIGIBLE_FOR_TRIAL.include?(actual_plan_name)
+    end
+
     def use_elasticsearch?
       ::Gitlab::CurrentSettings.elasticsearch_indexes_namespace?(self)
     end

ee/app/models/ee/plan.rb

@@ -16,6 +16,7 @@ module EE
       PAID_HOSTED_PLANS = [BRONZE, SILVER, GOLD].freeze
       FREE_HOSTED_PLANS = [EARLY_ADOPTER].freeze
       EE_ALL_PLANS = (EE_DEFAULT_PLANS + PAID_HOSTED_PLANS + FREE_HOSTED_PLANS).freeze
+      PLANS_ELIGIBLE_FOR_TRIAL = [FREE, *FREE_HOSTED_PLANS].freeze
 
       # This constant must keep ordered by tier.
       ALL_HOSTED_PLANS = (PAID_HOSTED_PLANS + FREE_HOSTED_PLANS).freeze

ee/app/models/ee/user.rb

@@ -258,10 +258,7 @@ module EE
     end
 
     def manageable_groups_eligible_for_trial
-      manageable_groups
-        .left_joins(:gitlab_subscription)
-        .where(gitlab_subscriptions: { trial: [nil, false] })
-        .order(:name)
+      manageable_groups.eligible_for_trial.order(:name)
     end
 
     override :has_current_license?

ee/app/views/shared/billings/_trial_status.html.haml

@@ -6,7 +6,7 @@
   = html_escape(s_("BillingPlans|Learn more about each plan by reading our %{faq_link}, or start a free 30-day trial of GitLab.com Gold.")) % { faq_link: faq_link.html_safe }
 - elsif namespace.trial_active?
   = html_escape(s_("BillingPlans|Your GitLab.com %{plan} trial will %{strong_open}expire after %{expiration_date}%{strong_close}. You can retain access to the %{plan} features by upgrading below.")) % { plan: namespace.gitlab_subscription&.plan_title, expiration_date: namespace.trial_ends_on, strong_open: '<strong>'.html_safe, strong_close: '</strong>'.html_safe }
-- elsif namespace.trial_expired?
+- elsif namespace.trial_expired? && namespace.free_plan?
   = s_("BillingPlans|Your GitLab.com trial expired on %{expiration_date}. You can restore access to the features at any time by upgrading below.") % { expiration_date: namespace.trial_ends_on }
 - else
   = html_escape(s_("BillingPlans|Learn more about each plan by visiting our %{pricing_page_link}.")) % { pricing_page_link: pricing_page_link.html_safe }

ee/changelogs/unreleased/222455-remove-non-free-plan-subscriptions-from-namespace-list-in-the-t.yml

+---
+title: Omit namespaces w/ non-free plans from trial select
+merge_request: 36394
+author:
+type: changed

ee/spec/factories/gitlab_subscriptions.rb

@@ -9,6 +9,21 @@ FactoryBot.define do
     end_date { Date.today.advance(years: 1) }
     trial { false }
 
+    trait :expired do
+      start_date { Date.today.advance(years: -1, months: -1) }
+      end_date { Date.today.advance(months: -1) }
+    end
+
+    trait :active_trial do
+      trial { true }
+      trial_ends_on { Date.today.advance(months: 1) }
+    end
+
+    trait :expired_trial do
+      trial { true }
+      trial_ends_on { Date.today.advance(days: -1) }
+    end
+
     trait :free do
       hosted_plan_id { nil }
     end

ee/spec/helpers/ee/trial_helper_spec.rb

@@ -10,83 +10,40 @@ RSpec.describe EE::TrialHelper do
     let_it_be(:group1) { create :group }
     let_it_be(:group2) { create :group }
 
-    let(:expected_group_options) { [] }
-    let(:expected_user_options) { [[user.namespace.name, user.namespace.id]] }
+    let(:trial_user_namespaces) { [] }
+    let(:trial_group_namespaces) { [] }
+
     let(:generated_html) do
       grouped_options_for_select({
         'New' => [['Create group', 0]],
-        'Groups' => expected_group_options,
-        'Users' => expected_user_options
+        '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
 
     before do
-      allow(helper).to receive(:trial_groups).and_return(expected_group_options)
-      allow(helper).to receive(:trial_users).and_return(expected_user_options)
+      allow(helper).to receive(:trial_group_namespaces).and_return(trial_group_namespaces)
+      allow(helper).to receive(:trial_user_namespaces).and_return(trial_user_namespaces)
     end
 
     subject { helper.namespace_options_for_select }
 
-    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 'and the user has some groups which can be trialed' do
-        let(:expected_group_options) { [group1, group2].map {|g| [g.name, g.id]} }
-
-        it { is_expected.to eq(generated_html) }
-      end
-    end
-
-    context 'when the user’s namespace has already been trialed' do
-      let(:expected_user_options) { [] }
-
-      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 'and the user has some groups which can be trialed' do
-        let(:expected_group_options) { [group1, group2].map {|g| [g.name, g.id]} }
-
-        it { is_expected.to eq(generated_html) }
-      end
-    end
-  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) { [] }
-
-    before do
-      user.reload # necessary to cache-bust the user.namespace.gitlab_subscription object
-      allow(helper).to receive(:current_user).and_return(user)
-    end
+    where(can_trial_user: [true, false], can_trial_groups: [true, false])
 
-    subject { helper.trial_users }
+    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 'when the user has no subscription on their namespace' do
-      it { is_expected.to eq(user_eligible_for_trial_result) }
-    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 '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 #{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] : [] }
 
-      context 'and the user has already trialed their namespace' do
-        let(:trialed) { true }
-
-        it { is_expected.to eq(user_ineligible_for_trial_result) }
+          it { is_expected.to eq(generated_html) }
+        end
       end
     end
   end
 
-  describe '#trial_groups' do
+  describe '#trial_group_namespaces' do
     let_it_be(:user) { create :user }
     let(:no_groups) { [] }
 
@@ -94,7 +51,7 @@ RSpec.describe EE::TrialHelper do
       allow(helper).to receive(:current_user).and_return(user)
     end
 
-    subject { helper.trial_groups }
+    subject { helper.trial_group_namespaces.map(&:id) }
 
     context 'when the user is not an owner/maintainer of any groups' do
       it { is_expected.to eq(no_groups) }
@@ -107,7 +64,7 @@ RSpec.describe EE::TrialHelper do
       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(:all_groups) { [group1, group2, subgroup1, subgroup2, subsubgroup1].map {|g| [g.name, g.id] } }
+      let(:all_groups) { [group1, group2, subgroup1, subgroup2, subsubgroup1].map(&:id) }
 
       before do
         group1.add_owner(user)
@@ -119,38 +76,38 @@ RSpec.describe EE::TrialHelper do
       end
 
       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 }
+        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(all_groups) }
         end
 
-        context 'and some of the groups have been trialed' do
-          let(:trialed_group1) { true }
-          let(:trialed_subgroup1) { true }
-          let(:trialed_subgroup2) { true }
+        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 }
 
-          let(:some_groups) { [group2, subsubgroup1].map {|g| [g.name, g.id]} }
+          let(:some_groups) { [group2, subsubgroup1].map(&:id) }
 
           it { is_expected.to eq(some_groups) }
         end
 
-        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 }
+        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 }
 
           it { is_expected.to eq(no_groups) }
         end
@@ -158,6 +115,38 @@ RSpec.describe EE::TrialHelper do
     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) { [] }
+
+    before do
+      allow(helper).to receive(:current_user).and_return(user)
+      allow(::Gitlab).to receive(:com?).and_return(true)
+    end
+
+    subject { helper.trial_user_namespaces }
+
+    context 'when the user has no subscription on their namespace' do
+      it { is_expected.to eq(user_eligible_for_trial_result) }
+    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
+    end
+  end
+
   describe '#show_trial_errors?' do
     shared_examples 'shows errors based on trial generation result' do
       where(:trial_result, :expected_result) do

ee/spec/models/ee/namespace_spec.rb

@@ -170,6 +170,60 @@ RSpec.describe Namespace do
         end
       end
     end
+
+    describe '.eligible_for_trial' do
+      let_it_be(:namespace) { create :namespace }
+
+      subject { described_class.eligible_for_trial.first }
+
+      context 'when there is no subscription' do
+        it { is_expected.to eq(namespace) }
+      end
+
+      context 'when there is a subscription' do
+        context 'with a plan that is eligible for a trial' do
+          where(plan: ::Plan::PLANS_ELIGIBLE_FOR_TRIAL)
+
+          with_them do
+            context 'and has not yet been trialed' do
+              before do
+                create :gitlab_subscription, plan, namespace: namespace
+              end
+
+              it { is_expected.to eq(namespace) }
+            end
+
+            context 'but has already had a trial' do
+              before do
+                create :gitlab_subscription, plan, :expired_trial, namespace: namespace
+              end
+
+              it { is_expected.to be_nil }
+            end
+
+            context 'but is currently being trialed' do
+              before do
+                create :gitlab_subscription, plan, :active_trial, namespace: namespace
+              end
+
+              it { is_expected.to be_nil }
+            end
+          end
+        end
+
+        context 'with a plan that is ineligible for a trial' do
+          where(plan: ::Plan::PAID_HOSTED_PLANS)
+
+          with_them do
+            before do
+              create :gitlab_subscription, plan, namespace: namespace
+            end
+
+            it { is_expected.to be_nil }
+          end
+        end
+      end
+    end
   end
 
   context 'validation' do
@@ -1262,6 +1316,36 @@ RSpec.describe Namespace do
     end
   end
 
+  describe '#eligible_for_trial?' do
+    subject { namespace.eligible_for_trial? }
+
+    where(
+      on_dot_com: [true, false],
+      has_parent: [true, false],
+      never_had_trial: [true, false],
+      plan_eligible_for_trial: [true, false]
+    )
+
+    with_them do
+      before do
+        allow(Gitlab).to receive(:com?).and_return(on_dot_com)
+        allow(namespace).to receive(:has_parent?).and_return(has_parent)
+        allow(namespace).to receive(:never_had_trial?).and_return(never_had_trial)
+        allow(namespace).to receive(:plan_eligible_for_trial?).and_return(plan_eligible_for_trial)
+      end
+
+      context "when#{' not' unless params[:on_dot_com]} on .com" do
+        context "and the namespace #{params[:has_parent] ? 'has' : 'is'} a parent namespace" do
+          context "and the namespace has#{' not yet' if params[:never_had_trial]} been trialed" do
+            context "and the namespace is#{' not' unless params[:plan_eligible_for_trial]} eligible for a trial" do
+              it { is_expected.to eq(on_dot_com && !has_parent && never_had_trial && plan_eligible_for_trial) }
+            end
+          end
+        end
+      end
+    end
+  end
+
   describe '#file_template_project_id' do
     it 'is cleared before validation' do
       project = create(:project, namespace: namespace)

ee/spec/models/user_spec.rb

@@ -1052,9 +1052,9 @@ RSpec.describe User do
 
   describe '#manageable_groups_eligible_for_trial' do
     let_it_be(:user) { create :user }
-    let_it_be(:non_trialed_group_z) { create :group, name: 'Zeta', gitlab_subscription: create(:gitlab_subscription) }
-    let_it_be(:non_trialed_group_a) { create :group, name: 'Alpha', gitlab_subscription: create(:gitlab_subscription) }
-    let_it_be(:trialed_group) { create :group, name: 'Omitted', gitlab_subscription: create(:gitlab_subscription, trial: true) }
+    let_it_be(:non_trialed_group_z) { create :group, name: 'Zeta', gitlab_subscription: create(:gitlab_subscription, :free) }
+    let_it_be(:non_trialed_group_a) { create :group, name: 'Alpha', gitlab_subscription: create(:gitlab_subscription, :free) }
+    let_it_be(:trialed_group) { create :group, name: 'Omitted', gitlab_subscription: create(:gitlab_subscription, :free, trial: true) }
 
     subject { user.manageable_groups_eligible_for_trial }