diff --git a/ee/app/helpers/ee/user_callouts_helper.rb b/ee/app/helpers/ee/user_callouts_helper.rb index b67adc6403ca4e71c24f504b03e8e28f6a1f21fb..ff8c704d9edf25b64c8c2e0feca0cc59e84575e6 100644 --- a/ee/app/helpers/ee/user_callouts_helper.rb +++ b/ee/app/helpers/ee/user_callouts_helper.rb @@ -52,7 +52,8 @@ def show_migrate_hashed_storage_warning? 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 @@ def show_gold_trial_suitable_env? ::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 diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index 588fc5afe3f71a49a14af89cfd963861152e2394..9b0cfb7c325f280d3f08afa4ae22443e111e29c9 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -217,30 +217,31 @@ def group_view 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 @@ def password_required?(*) 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) diff --git a/ee/app/views/shared/billings/_trial_status.html.haml b/ee/app/views/shared/billings/_trial_status.html.haml index a9b9b67b12b7760d8b8f8cd43ed38b8b173a4896..8384806d62c4a6395f656333b9b0bbdb17be85a0 100644 --- a/ee/app/views/shared/billings/_trial_status.html.haml +++ b/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 expire after %{expiration_date}. 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 expire after %{expiration_date}. 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 diff --git a/ee/spec/helpers/ee/user_callouts_helper_spec.rb b/ee/spec/helpers/ee/user_callouts_helper_spec.rb index dea61ce3e6b58201f966d07e4f63a10b39d921af..cbd8564dbf305384d7845ea46cc3ceffa22d48d1 100644 --- a/ee/spec/helpers/ee/user_callouts_helper_spec.rb +++ b/ee/spec/helpers/ee/user_callouts_helper_spec.rb @@ -177,7 +177,7 @@ 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 @@ 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 @@ 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 diff --git a/ee/spec/models/user_spec.rb b/ee/spec/models/user_spec.rb index 2740d0bc5412cb0e40a13a479ba873528325c71e..56f7aff1e8836108df1fdce32f1e8429eb24815e 100644 --- a/ee/spec/models/user_spec.rb +++ b/ee/spec/models/user_spec.rb @@ -1029,4 +1029,70 @@ 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 diff --git a/ee/spec/views/shared/billings/_trial_status.html.haml_spec.rb b/ee/spec/views/shared/billings/_trial_status.html.haml_spec.rb index d042600cf1acc735736d6d7eea66fa2b1b17950e..ac8d4227ffb5d532432b49b61c4f3e9465f72ad2 100644 --- a/ee/spec/views/shared/billings/_trial_status.html.haml_spec.rb +++ b/ee/spec/views/shared/billings/_trial_status.html.haml_spec.rb @@ -6,18 +6,12 @@ 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