Merge branch 'astoicescu_remove_plan_cards_and_upgrade_button' into 'master'

Remove upgrade CTAs from billing page for free personal namespaces

See merge request gitlab-org/gitlab!61142

ee/app/assets/javascripts/billings/subscriptions/components/subscription_table.vue

@@ -37,6 +37,9 @@ export default {
     planName: {
       default: '',
     },
+    freePersonalNamespace: {
+      default: false,
+    },
   },
   computed: {
     ...mapState(['isLoadingSubscription', 'hasErrorSubscription', 'plan', 'tables', 'endpoint']),
@@ -54,7 +57,7 @@ export default {
       return this.isSubscription && !this.plan.trial;
     },
     canUpgrade() {
-      return this.isFreePlan || this.plan.upgradable;
+      return !this.freePersonalNamespace && (this.isFreePlan || this.plan.upgradable);
     },
     canUpgradeEEPlan() {
       return this.isSubscription && this.planUpgradeHref;

ee/app/assets/javascripts/billings/subscriptions/index.js

 import Vue from 'vue';
 import Vuex from 'vuex';
+import { parseBoolean } from '~/lib/utils/common_utils';
 import SubscriptionApp from './components/app.vue';
 import initialStore from './store';
 
@@ -21,6 +22,7 @@ export default (containerId = 'js-billing-plans') => {
     customerPortalUrl,
     billableSeatsHref,
     planName,
+    freePersonalNamespace,
   } = containerEl.dataset;
 
   return new Vue({
@@ -35,6 +37,7 @@ export default (containerId = 'js-billing-plans') => {
       customerPortalUrl,
       billableSeatsHref,
       planName,
+      freePersonalNamespace: parseBoolean(freePersonalNamespace),
     },
     render(createElement) {
       return createElement(SubscriptionApp);

ee/app/helpers/billing_plans_helper.rb

@@ -45,7 +45,8 @@ module BillingPlansHelper
       plan_renew_href: plan_renew_url(namespace),
       customer_portal_url: "#{EE::SUBSCRIPTIONS_URL}/subscriptions",
       billable_seats_href: billable_seats_href(namespace),
-      plan_name: plan&.name
+      plan_name: plan&.name,
+      free_personal_namespace: namespace.free_personal?.to_s
     }
   end
 
@@ -84,6 +85,8 @@ module BillingPlansHelper
   end
 
   def show_plans?(namespace)
+    return false if namespace.free_personal?
+
     namespace.trial_active? || !(namespace.gold_plan? || namespace.ultimate_plan?)
   end
 
@@ -126,6 +129,10 @@ module BillingPlansHelper
     end
   end
 
+  def show_start_free_trial_messages?(namespace)
+    !namespace.free_personal? && namespace.eligible_for_trial?
+  end
+
   private
 
   def add_seats_url(group)

ee/app/models/ee/namespace.rb

@@ -370,6 +370,10 @@ module EE
       ::Plan::PLANS_ELIGIBLE_FOR_TRIAL.include?(actual_plan_name)
     end
 
+    def free_personal?
+      user? && !paid?
+    end
+
     def use_elasticsearch?
       ::Gitlab::CurrentSettings.elasticsearch_indexes_namespace?(self)
     end

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

@@ -28,6 +28,6 @@
   - else
     = render 'shared/billings/trial_status', namespace: namespace
 
-  - if namespace.eligible_for_trial?
+  - if show_start_free_trial_messages?(namespace)
     - glm_content = namespace_for_user ? 'user-billing' : 'group-billing'
     %p= link_to 'Start your free trial', new_trial_registration_path(glm_source: 'gitlab.com', glm_content: glm_content), class: 'btn btn-confirm gl-button', data: { qa_selector: 'start_your_free_trial' }

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

 - faq_link          = link_to s_("BillingPlans|frequently asked questions"), "https://about.gitlab.com/gitlab-com/#faq"
 - pricing_page_link = link_to s_("BillingPlans|Pricing page"), "https://about.gitlab.com/pricing"
 
-- if namespace.eligible_for_trial?
+- if show_start_free_trial_messages?(namespace)
   = html_escape(s_("BillingPlans|Learn more about each plan by reading our %{faq_link}, or start a free 30-day trial of GitLab.com Ultimate.")) % { 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 }

ee/spec/features/billings/billing_plans_spec.rb

@@ -46,6 +46,12 @@ RSpec.describe 'Billing plan pages', :feature, :js do
     end
   end
 
+  shared_examples 'does not display the billing plans' do
+    it 'does not display the plans' do
+      expect(page).not_to have_css('.billing-plans')
+    end
+  end
+
   shared_examples 'upgradable plan' do
     before do
       visit page_path
@@ -204,41 +210,8 @@ RSpec.describe 'Billing plan pages', :feature, :js do
         visit page_path
       end
 
-      it 'displays all plans' do
-        page.within('.billing-plans') do
-          panels = page.all('.card')
-
-          expect(panels.length).to eq(plans_data.length)
-
-          plans_data.each.with_index do |data, index|
-            expect(panels[index].find('.card-header')).to have_content(data[:name])
-          end
-        end
-      end
-
-      it 'displays correct plan actions' do
-        expected_actions = plans_data.map { |data| data.fetch(:purchase_link).fetch(:action) }
-        plan_actions = page.all('.billing-plans .card .card-footer')
-        expect(plan_actions.length).to eq(expected_actions.length)
-
-        expected_actions.each_with_index do |expected_action, index|
-          action = plan_actions[index]
-
-          case expected_action
-          when 'downgrade'
-            expect(action).not_to have_link('Upgrade')
-            expect(action).not_to have_css('.disabled')
-          when 'current_plan'
-            expect(action).not_to have_link('Upgrade')
-          when 'upgrade'
-            expect(action).to have_link('Upgrade')
-            expect(action).not_to have_css('.disabled')
-          end
-        end
-      end
-
+      it_behaves_like 'does not display the billing plans'
       it_behaves_like 'plan with subscription table'
-      it_behaves_like 'can contact sales'
     end
 
     context 'on bronze plan' do
@@ -348,40 +321,7 @@ RSpec.describe 'Billing plan pages', :feature, :js do
         visit page_path
       end
 
-      it 'displays all plans' do
-        page.within('.billing-plans') do
-          panels = page.all('.card')
-
-          expect(panels.length).to eq(plans_data.length)
-
-          plans_data.each.with_index do |data, index|
-            expect(panels[index].find('.card-header')).to have_content(data[:name])
-          end
-        end
-      end
-
-      it 'displays correct plan actions' do
-        expected_actions = plans_data.map { |data| data.fetch(:purchase_link).fetch(:action) }
-        plan_actions = page.all('.billing-plans .card .card-footer')
-        expect(plan_actions.length).to eq(expected_actions.length)
-
-        expected_actions.each_with_index do |expected_action, index|
-          action = plan_actions[index]
-
-          case expected_action
-          when 'downgrade'
-            expect(action).not_to have_link('Upgrade')
-            expect(action).not_to have_css('.disabled')
-          when 'current_plan'
-            expect(action).not_to have_css('.disabled')
-          when 'upgrade'
-            expect(action).to have_link('Upgrade')
-            expect(action).not_to have_css('.disabled')
-          end
-        end
-      end
-
-      it_behaves_like 'can contact sales'
+      it_behaves_like 'does not display the billing plans'
     end
 
     context 'on bronze plan' do
@@ -495,7 +435,9 @@ RSpec.describe 'Billing plan pages', :feature, :js do
   end
 
   context 'with unexpected JSON' do
-    let(:plan) { free_plan }
+    let(:plan) { premium_plan }
+
+    let!(:subscription) { create(:gitlab_subscription, namespace: namespace, hosted_plan: plan, seats: 15) }
 
     let(:plans_data) do
       [

ee/spec/frontend/billings/subscriptions/components/subscription_table_spec.js

@@ -8,9 +8,12 @@ import * as types from 'ee/billings/subscriptions/store/mutation_types';
 import { mockDataSubscription } from 'ee_jest/billings/mock_data';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 
-const namespaceName = 'GitLab.com';
-const customerPortalUrl = 'https://customers.gitlab.com/subscriptions';
-const planName = 'Gold';
+const defaultInjectedProps = {
+  namespaceName: 'GitLab.com',
+  customerPortalUrl: 'https://customers.gitlab.com/subscriptions',
+  planName: 'Gold',
+  freePersonalNamespace: false,
+};
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -33,9 +36,7 @@ describe('SubscriptionTable component', () => {
         store,
         localVue,
         provide: {
-          customerPortalUrl,
-          namespaceName,
-          planName,
+          ...defaultInjectedProps,
           ...provide,
         },
         propsData: props,
@@ -189,20 +190,27 @@ describe('SubscriptionTable component', () => {
 
   describe('Upgrade button', () => {
     describe.each`
-      planCode    | upgradable | expected | testDescription
-      ${'bronze'} | ${true}    | ${true}  | ${'renders the button'}
-      ${'bronze'} | ${false}   | ${false} | ${'does not render the button'}
-      ${null}     | ${true}    | ${true}  | ${'renders the button'}
-      ${null}     | ${false}   | ${true}  | ${'renders the button'}
-      ${'free'}   | ${true}    | ${true}  | ${'renders the button'}
-      ${'free'}   | ${false}   | ${true}  | ${'renders the button'}
+      planCode    | upgradable | freePersonalNamespace | expected
+      ${null}     | ${false}   | ${false}              | ${true}
+      ${null}     | ${true}    | ${false}              | ${true}
+      ${null}     | ${false}   | ${true}               | ${false}
+      ${null}     | ${true}    | ${true}               | ${false}
+      ${'free'}   | ${false}   | ${false}              | ${true}
+      ${'free'}   | ${true}    | ${false}              | ${true}
+      ${'free'}   | ${false}   | ${true}               | ${false}
+      ${'free'}   | ${true}    | ${true}               | ${false}
+      ${'bronze'} | ${false}   | ${false}              | ${false}
+      ${'bronze'} | ${true}    | ${false}              | ${true}
+      ${'bronze'} | ${false}   | ${true}               | ${false}
+      ${'bronze'} | ${true}    | ${true}               | ${false}
     `(
-      'given a plan with state: planCode = $planCode, upgradable = $upgradable',
-      ({ planCode, upgradable, expected, testDescription }) => {
+      'given a plan with state: planCode = $planCode, upgradable = $upgradable, freePersonalNamespace = $freePersonalNamespace',
+      ({ planCode, upgradable, freePersonalNamespace, expected }) => {
         beforeEach(() => {
           createComponentWithStore({
             provide: {
               planUpgradeHref: '',
+              freePersonalNamespace,
             },
             state: {
               isLoadingSubscription: false,
@@ -214,6 +222,9 @@ describe('SubscriptionTable component', () => {
           });
         });
 
+        const testDescription =
+          expected === true ? 'renders the button' : 'does not render the button';
+
         it(testDescription, () => {
           expect(findUpgradeButton().exists()).toBe(expected);
         });

ee/spec/helpers/billing_plans_helper_spec.rb

@@ -26,7 +26,8 @@ RSpec.describe BillingPlansHelper do
                  plan_renew_href: renew_href,
                  customer_portal_url: customer_portal_url,
                  billable_seats_href: billable_seats_href,
-                 plan_name: plan.name)
+                 plan_name: plan.name,
+                 free_personal_namespace: 'false')
       end
     end
 
@@ -54,7 +55,8 @@ RSpec.describe BillingPlansHelper do
                  namespace_name: group.name,
                  plan_renew_href: renew_href,
                  plan_upgrade_href: nil,
-                 plan_name: nil)
+                 plan_name: nil,
+                 free_personal_namespace: 'false')
       end
     end
 
@@ -74,7 +76,8 @@ RSpec.describe BillingPlansHelper do
                  add_seats_href: add_seats_href,
                  plan_renew_href: renew_href,
                  plan_upgrade_href: nil,
-                 plan_name: plan.name)
+                 plan_name: plan.name,
+                 free_personal_namespace: 'false')
       end
     end
 
@@ -97,6 +100,27 @@ RSpec.describe BillingPlansHelper do
         end
       end
     end
+
+    context 'when the namespace belongs to a user' do
+      let(:group) { build(:group, type: 'user') }
+
+      context 'when the namespace is free plan' do
+        it 'returns attributes with free_personal_namespace true' do
+          expect(helper.subscription_plan_data_attributes(group, plan))
+            .to include(free_personal_namespace: 'true')
+        end
+      end
+
+      context 'when the namespace is paid plan' do
+        let(:group) { build(:group, type: 'user') }
+        let!(:gitlab_subscription) { build(:gitlab_subscription, :ultimate, namespace: group) }
+
+        it 'returns attributes with free_personal_namespace false' do
+          expect(helper.subscription_plan_data_attributes(group, plan))
+            .to include(free_personal_namespace: 'false')
+        end
+      end
+    end
   end
 
   describe '#use_new_purchase_flow?' do
@@ -337,7 +361,7 @@ RSpec.describe BillingPlansHelper do
     end
   end
 
-  describe '#upgrade_button_css_classe' do
+  describe '#upgrade_button_css_classes' do
     using RSpec::Parameterized::TableSyntax
 
     let(:plan) { double('Plan', deprecated?: false) }
@@ -453,4 +477,60 @@ RSpec.describe BillingPlansHelper do
       expect(helper.subscription_plan_info([other_plan, current_plan], 'premium')).to eq(current_plan)
     end
   end
+
+  describe '#show_plans?' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:group) { build(:group) }
+
+    where(:free_personal, :trial_active, :gold_plan, :ultimate_plan, :expectations) do
+      false |  false | false | false | true
+      false |  true  | false | false | true
+      false |  false | true  | false | false
+      false |  true  | true  | false | true
+      false |  false | false | true  | false
+      false |  true  | false | true  | true
+      false |  false | true  | true  | false
+      false |  true  | true  | true  | true
+      false |  true  | true  | true  | true
+      false |  true  | true  | true  | true
+      true  |  true  | true  | true  | false
+    end
+
+    with_them do
+      before do
+        allow(group).to receive(:trial_active?).and_return(trial_active)
+        allow(group).to receive(:gold_plan?).and_return(gold_plan)
+        allow(group).to receive(:ultimate_plan?).and_return(ultimate_plan)
+        allow(group).to receive(:free_personal?).and_return(free_personal)
+      end
+
+      it 'returns boolean' do
+        expect(helper.show_plans?(group)).to eql(expectations)
+      end
+    end
+  end
+
+  describe '#show_start_free_trial_messages?' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:namespace) { build(:namespace) }
+
+    where(:free_personal, :eligible_for_trial, :expected) do
+      false  | true   | true
+      true   | true   | false
+      false  | false  | false
+    end
+
+    with_them do
+      before do
+        allow(namespace).to receive(:free_personal?).and_return(free_personal)
+        allow(namespace).to receive(:eligible_for_trial?).and_return(eligible_for_trial)
+      end
+
+      it 'returns correct boolean value' do
+        expect(helper.show_start_free_trial_messages?(namespace)).to eql(expected)
+      end
+    end
+  end
 end

ee/spec/models/ee/namespace_spec.rb

@@ -63,6 +63,25 @@ RSpec.describe Namespace do
     end
   end
 
+  describe '#free_personal?' do
+    where(:user, :paid, :expected) do
+      true  | false | true
+      false | false | false
+      false | true  | false
+    end
+
+    with_them do
+      before do
+        allow(namespace).to receive(:user?).and_return(user)
+        allow(namespace).to receive(:paid?).and_return(paid)
+      end
+
+      it 'returns expected boolean value' do
+        expect(namespace.free_personal?).to eq(expected)
+      end
+    end
+  end
+
   describe '#use_elasticsearch?' do
     let(:namespace) { create :namespace }