Merge branch 'fix-trial-callout-plan' into 'master'

Fix trial callout to use the correct plan

See merge request gitlab-org/gitlab!29014

ee/app/helpers/ee/user_callouts_helper.rb

@@ -52,7 +52,8 @@ module EE
     def render_dashboard_gold_trial(user)
       return unless show_gold_trial?(user, GOLD_TRIAL) &&
           user_default_dashboard?(user) &&
-          has_no_trial_or_gold_plan?(user) &&
+          ::Feature.enabled?(:render_dashboard_gold_trial, default_enabled: true) &&
+          has_no_trial_or_paid_plan?(user) &&
           has_some_namespaces_with_no_trials?(user)
 
       render 'shared/gold_trial_callout_content'
@@ -118,8 +119,8 @@ module EE
       ::Gitlab.com? && !::Gitlab::Database.read_only?
     end
 
-    def has_no_trial_or_gold_plan?(user)
-      return false if user.any_namespace_with_gold?
+    def has_no_trial_or_paid_plan?(user)
+      return false if user.owns_paid_namespace?
 
       !user.any_namespace_with_trial?
     end

ee/app/models/ee/user.rb

@@ -217,30 +217,31 @@ module EE
 
     def any_namespace_with_trial?
       ::Namespace
-        .from("(#{namespace_union(:trial_ends_on)}) #{::Namespace.table_name}")
+        .from("(#{namespace_union_for_owned(:trial_ends_on)}) #{::Namespace.table_name}")
         .where('trial_ends_on > ?', Time.now.utc)
         .any?
     end
 
     def any_namespace_without_trial?
       ::Namespace
-        .from("(#{namespace_union(:trial_ends_on)}) #{::Namespace.table_name}")
+        .from("(#{namespace_union_for_owned(:trial_ends_on)}) #{::Namespace.table_name}")
         .where(trial_ends_on: nil)
         .any?
     end
 
     def has_paid_namespace?
       ::Namespace
-        .from("(#{namespace_union_for_reporter_developer_maintainer_owned(:plan_id)}) #{::Namespace.table_name}")
-        .where(plan_id: Plan.where(name: Plan::PAID_HOSTED_PLANS).select(:id))
+        .from("(#{namespace_union_for_reporter_developer_maintainer_owned}) #{::Namespace.table_name}")
+        .include_gitlab_subscription
+        .where(gitlab_subscriptions: { hosted_plan: Plan.where(name: Plan::PAID_HOSTED_PLANS) })
         .any?
     end
 
-    def any_namespace_with_gold?
+    def owns_paid_namespace?
       ::Namespace
-        .includes(:plan)
-        .where("namespaces.id IN (#{namespace_union})") # rubocop:disable GitlabSecurity/SqlInjection
-        .where.not(plans: { id: nil })
+        .from("(#{namespace_union_for_owned}) #{::Namespace.table_name}")
+        .include_gitlab_subscription
+        .where(gitlab_subscriptions: { hosted_plan: Plan.where(name: Plan::PAID_HOSTED_PLANS) })
         .any?
     end
 
@@ -342,7 +343,7 @@ module EE
 
     private
 
-    def namespace_union(select = :id)
+    def namespace_union_for_owned(select = :id)
       ::Gitlab::SQL::Union.new([
         ::Namespace.select(select).where(type: nil, owner: self),
         owned_groups.select(select).where(parent_id: nil)

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

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

ee/spec/helpers/ee/user_callouts_helper_spec.rb

@@ -177,7 +177,7 @@ describe EE::UserCalloutsHelper do
     let_it_be(:gold_plan) { create(:gold_plan) }
     let(:user) { namespace.owner }
 
-    where(:has_some_namespaces_with_no_trials?, :show_gold_trial?, :user_default_dashboard?, :has_no_trial_or_gold_plan?, :should_render?) do
+    where(:has_some_namespaces_with_no_trials?, :show_gold_trial?, :user_default_dashboard?, :has_no_trial_or_paid_plan?, :should_render?) do
       true  | true  | true  | true  | true
       true  | true  | true  | false | false
       true  | true  | false | true  | false
@@ -201,7 +201,10 @@ describe EE::UserCalloutsHelper do
         allow(helper).to receive(:show_gold_trial?) { show_gold_trial? }
         allow(helper).to receive(:user_default_dashboard?) { user_default_dashboard? }
         allow(helper).to receive(:has_some_namespaces_with_no_trials?) { has_some_namespaces_with_no_trials? }
-        namespace.update(plan: gold_plan) unless has_no_trial_or_gold_plan?
+
+        unless has_no_trial_or_paid_plan?
+          create(:gitlab_subscription, hosted_plan: gold_plan, namespace: namespace)
+        end
       end
 
       it do
@@ -214,6 +217,22 @@ describe EE::UserCalloutsHelper do
         helper.render_dashboard_gold_trial(user)
       end
     end
+
+    context 'when render_dashboard_gold_trial feature is disabled' do
+      before do
+        stub_feature_flags(render_dashboard_gold_trial: false)
+
+        allow(helper).to receive(:show_gold_trial?).and_return(true)
+        allow(helper).to receive(:user_default_dashboard?).and_return(true)
+        allow(helper).to receive(:has_some_namespaces_with_no_trials?).and_return(true)
+      end
+
+      it 'does not render' do
+        expect(helper).not_to receive(:render)
+
+        helper.render_dashboard_gold_trial(user)
+      end
+    end
   end
 
   describe '#render_billings_gold_trial' do

ee/spec/models/user_spec.rb

@@ -1029,4 +1029,70 @@ describe User do
       end
     end
   end
+
+  context 'paid namespaces' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:gold_group) { create(:group_with_plan, plan: :gold_plan) }
+    let_it_be(:bronze_group) { create(:group_with_plan, plan: :bronze_plan) }
+    let_it_be(:free_group) { create(:group_with_plan, plan: :free_plan) }
+    let_it_be(:group_without_plan) { create(:group) }
+
+    describe '#has_paid_namespace?' do
+      context 'when the user has Reporter or higher on at least one paid group' do
+        it 'returns true' do
+          gold_group.add_reporter(user)
+          bronze_group.add_guest(user)
+
+          expect(user.has_paid_namespace?).to eq(true)
+        end
+      end
+
+      context 'when the user is only a Guest on paid groups' do
+        it 'returns false' do
+          gold_group.add_guest(user)
+          bronze_group.add_guest(user)
+          free_group.add_owner(user)
+
+          expect(user.has_paid_namespace?).to eq(false)
+        end
+      end
+
+      context 'when the user is not a member of any groups with plans' do
+        it 'returns false' do
+          group_without_plan.add_owner(user)
+
+          expect(user.has_paid_namespace?).to eq(false)
+        end
+      end
+    end
+
+    describe '#owns_paid_namespace?' do
+      context 'when the user is an owner of at least one paid group' do
+        it 'returns true' do
+          gold_group.add_owner(user)
+          bronze_group.add_owner(user)
+
+          expect(user.owns_paid_namespace?).to eq(true)
+        end
+      end
+
+      context 'when the user is only a Maintainer on paid groups' do
+        it 'returns false' do
+          gold_group.add_maintainer(user)
+          bronze_group.add_maintainer(user)
+          free_group.add_owner(user)
+
+          expect(user.owns_paid_namespace?).to eq(false)
+        end
+      end
+
+      context 'when the user is not a member of any groups with plans' do
+        it 'returns false' do
+          group_without_plan.add_owner(user)
+
+          expect(user.owns_paid_namespace?).to eq(false)
+        end
+      end
+    end
+  end
 end

ee/spec/views/shared/billings/_trial_status.html.haml_spec.rb

@@ -6,18 +6,12 @@ describe 'shared/billings/_trial_status.html.haml' do
   include ApplicationHelper
 
   let_it_be(:group) { create(:group) }
-  let_it_be(:gitlab_subscription) { create(:gitlab_subscription, namespace: group) }
   let(:plan) { nil }
   let(:trial_ends_on) { nil }
   let(:trial) { false }
 
   before do
-    gitlab_subscription.update(
-      hosted_plan: plan,
-      trial_ends_on: trial_ends_on,
-      trial: trial
-    )
-    group.update(plan: plan)
+    create(:gitlab_subscription, namespace: group, hosted_plan: plan, trial_ends_on: trial_ends_on, trial: trial)
   end
 
   context 'when not eligible for trial' do