Merge branch 'select-correct-plan-name' into 'master'

Select Plan for Billing Page When Multiple Plans Have Same Code

See merge request gitlab-org/gitlab!52397

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

@@ -36,12 +36,15 @@ export default {
     addSeatsHref: {
       default: '',
     },
+    planName: {
+      default: '',
+    },
   },
   computed: {
     ...mapState(['isLoadingSubscription', 'hasErrorSubscription', 'plan', 'tables', 'endpoint']),
     ...mapGetters(['isFreePlan']),
     subscriptionHeader() {
-      const planName = this.isFreePlan ? s__('SubscriptionTable|Free') : escape(this.plan.name);
+      const planName = this.isFreePlan ? s__('SubscriptionTable|Free') : escape(this.planName);
       const suffix = !this.isFreePlan && this.plan.trial ? s__('SubscriptionTable|Trial') : '';
 
       return `${this.namespaceName}: ${planName} ${suffix}`;

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

@@ -22,6 +22,7 @@ export default (containerId = 'js-billing-plans') => {
     planRenewHref,
     customerPortalUrl,
     billableSeatsHref,
+    planName,
   } = containerEl.dataset;
 
   return new Vue({
@@ -36,6 +37,7 @@ export default (containerId = 'js-billing-plans') => {
       planRenewHref,
       customerPortalUrl,
       billableSeatsHref,
+      planName,
     },
     render(createElement) {
       return createElement(SubscriptionApp);

ee/app/helpers/billing_plans_helper.rb

@@ -2,7 +2,8 @@
 
 module BillingPlansHelper
   def subscription_plan_info(plans_data, current_plan_code)
-    plans_data.find { |plan| plan.code == current_plan_code }
+    current_plan = plans_data.find { |plan| plan.code == current_plan_code && plan.current_subscription_plan? }
+    current_plan || plans_data.find { |plan| plan.code == current_plan_code }
   end
 
   def number_to_plan_currency(value)
@@ -20,7 +21,8 @@ module BillingPlansHelper
       plan_upgrade_href: plan_upgrade_url(namespace, plan),
       plan_renew_href: plan_renew_url(namespace),
       customer_portal_url: "#{EE::SUBSCRIPTIONS_URL}/subscriptions",
-      billable_seats_href: billable_seats_href(namespace)
+      billable_seats_href: billable_seats_href(namespace),
+      plan_name: plan&.name
     }
   end
 

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

@@ -18,7 +18,7 @@
     - if namespace_for_user
       = s_("BillingPlans|@%{user_name} you are currently using the %{plan_name} plan.").html_safe % { user_name: current_user.username, plan_name: plan.code.titleize }
     - else
-      = s_("BillingPlans|%{group_name} is currently using the %{plan_name} plan.").html_safe % { group_name: namespace.full_name, plan_name: plan.code.titleize }
+      = s_("BillingPlans|%{group_name} is currently using the %{plan_name}.").html_safe % { group_name: namespace.full_name, plan_name: plan.name }
 
   - if parent_group
     %p= s_("BillingPlans|This group uses the plan associated with its parent group.")

ee/spec/features/billings/billing_plans_spec.rb

@@ -312,7 +312,7 @@ RSpec.describe 'Billing plan pages', :feature do
 
         it 'displays plan header' do
           page.within('.billing-plan-header') do
-            expect(page).to have_content("#{namespace.name} is currently using the Gold plan")
+            expect(page).to have_content("#{namespace.name} is currently using the Gold Plan")
 
             expect(page).to have_css('.billing-plan-logo .identicon')
           end
@@ -340,7 +340,7 @@ RSpec.describe 'Billing plan pages', :feature do
 
         it 'displays plan header' do
           page.within('.billing-plan-header') do
-            expect(page).to have_content("#{namespace.name} is currently using the Bronze plan")
+            expect(page).to have_content("#{namespace.name} is currently using the Bronze Plan")
 
             expect(page).to have_css('.billing-plan-logo .identicon')
           end
@@ -392,7 +392,7 @@ RSpec.describe 'Billing plan pages', :feature do
 
         it 'displays plan header' do
           page.within('.billing-plan-header') do
-            expect(page).to have_content("#{namespace.name} is currently using the Gold plan")
+            expect(page).to have_content("#{namespace.name} is currently using the Gold Plan")
 
             expect(page).to have_css('.billing-plan-logo .identicon')
           end
@@ -433,7 +433,7 @@ RSpec.describe 'Billing plan pages', :feature do
 
     it 'displays plan header' do
       page.within('.billing-plan-header') do
-        expect(page).to have_content("#{subgroup2.full_name} is currently using the Bronze plan")
+        expect(page).to have_content("#{subgroup2.full_name} is currently using the Bronze Plan")
         expect(page).to have_css('.billing-plan-logo .identicon')
         expect(page.find('.btn-success')).to have_content('Manage plan')
       end

ee/spec/features/groups/billing_spec.rb

@@ -39,7 +39,7 @@ RSpec.describe 'Groups > Billing', :js do
     it 'shows the proper title and subscription data' do
       visit group_billings_path(group)
 
-      expect(page).to have_content("#{group.name} is currently using the Free plan")
+      expect(page).to have_content("#{group.name} is currently using the Free Plan")
       within subscription_table do
         expect(page).to have_content("start date #{formatted_date(subscription.start_date)}")
         expect(page).to have_link("Upgrade", href: "#{EE::SUBSCRIPTIONS_URL}/subscriptions")
@@ -63,7 +63,7 @@ RSpec.describe 'Groups > Billing', :js do
 
       visit group_billings_path(group)
 
-      expect(page).to have_content("#{group.name} is currently using the Bronze plan")
+      expect(page).to have_content("#{group.name} is currently using the Bronze Plan")
       within subscription_table do
         expect(page).to have_content("start date #{formatted_date(subscription.start_date)}")
         expect(page).to have_link("Upgrade", href: upgrade_url)
@@ -100,7 +100,7 @@ RSpec.describe 'Groups > Billing', :js do
     it 'shows the proper title and subscription data' do
       visit group_billings_path(group)
 
-      expect(page).to have_content("#{group.name} is currently using the Bronze plan")
+      expect(page).to have_content("#{group.name} is currently using the Bronze Plan")
       within subscription_table do
         expect(page).not_to have_link("Upgrade")
         expect(page).to have_link("Manage", href: "#{EE::SUBSCRIPTIONS_URL}/subscriptions")

ee/spec/fixtures/gitlab_com_plans.json

@@ -37,7 +37,7 @@
             }
         ],
         "free": true,
-        "name": "Free",
+        "name": "Free Plan",
         "price_per_month": 0.0,
         "price_per_year": 0.0,
         "purchase_link": {
@@ -84,7 +84,7 @@
             }
         ],
         "free": false,
-        "name": "Bronze",
+        "name": "Bronze Plan",
         "price_per_month": 4.0,
         "price_per_year": 48.0,
         "purchase_link": {
@@ -120,7 +120,7 @@
             }
         ],
         "free": false,
-        "name": "Silver",
+        "name": "Silver Plan",
         "price_per_month": 19.0,
         "price_per_year": 228.0,
         "purchase_link": {
@@ -159,7 +159,7 @@
             }
         ],
         "free": false,
-        "name": "Gold",
+        "name": "Gold Plan",
         "price_per_month": 99.0,
         "price_per_year": 1188.0,
         "purchase_link": {

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

@@ -10,6 +10,7 @@ import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 
 const namespaceName = 'GitLab.com';
 const customerPortalUrl = 'https://customers.gitlab.com/subscriptions';
+const planName = 'Gold';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -41,6 +42,7 @@ describe('SubscriptionTable component', () => {
         provide: {
           customerPortalUrl,
           namespaceName,
+          planName,
           ...provide,
           glFeatures: {
             defaultFlags,

ee/spec/helpers/billing_plans_helper_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe BillingPlansHelper do
 
     let(:group) { build(:group) }
     let(:plan) do
-      Hashie::Mash.new(id: 'external-paid-plan-hash-code')
+      Hashie::Mash.new(id: 'external-paid-plan-hash-code', name: 'Bronze Plan')
     end
 
     context 'when group and plan with ID present' do
@@ -26,7 +26,8 @@ RSpec.describe BillingPlansHelper do
                  plan_upgrade_href: upgrade_href,
                  plan_renew_href: renew_href,
                  customer_portal_url: customer_portal_url,
-                 billable_seats_href: billable_seats_href)
+                 billable_seats_href: billable_seats_href,
+                 plan_name: plan.name)
       end
     end
 
@@ -38,8 +39,29 @@ RSpec.describe BillingPlansHelper do
       end
     end
 
+    context 'when plan not present' do
+      let(:plan) { nil }
+
+      it 'returns attributes' do
+        add_seats_href = "#{EE::SUBSCRIPTIONS_URL}/gitlab/namespaces/#{group.id}/extra_seats"
+        billable_seats_href = helper.group_seat_usage_path(group)
+        renew_href = "#{EE::SUBSCRIPTIONS_URL}/gitlab/namespaces/#{group.id}/renew"
+
+        expect(helper.subscription_plan_data_attributes(group, plan))
+          .to eq(add_seats_href:  add_seats_href,
+                 billable_seats_href: billable_seats_href,
+                 customer_portal_url: customer_portal_url,
+                 is_group: "true",
+                 namespace_id: nil,
+                 namespace_name: group.name,
+                 plan_renew_href: renew_href,
+                 plan_upgrade_href: nil,
+                 plan_name: nil)
+      end
+    end
+
     context 'when plan with ID not present' do
-      let(:plan) { Hashie::Mash.new(id: nil) }
+      let(:plan) { Hashie::Mash.new(id: nil, name: 'Bronze Plan') }
 
       it 'returns data attributes without upgrade href' do
         add_seats_href = "#{EE::SUBSCRIPTIONS_URL}/gitlab/namespaces/#{group.id}/extra_seats"
@@ -54,7 +76,8 @@ RSpec.describe BillingPlansHelper do
                  billable_seats_href: billable_seats_href,
                  add_seats_href: add_seats_href,
                  plan_renew_href: renew_href,
-                 plan_upgrade_href: nil)
+                 plan_upgrade_href: nil,
+                 plan_name: plan.name)
       end
     end
 
@@ -315,4 +338,41 @@ RSpec.describe BillingPlansHelper do
       end
     end
   end
+
+  describe '#subscription_plan_info' do
+    it 'returns the current plan' do
+      other_plan = Hashie::Mash.new(code: 'bronze')
+      current_plan = Hashie::Mash.new(code: 'gold')
+
+      expect(helper.subscription_plan_info([other_plan, current_plan], 'gold')).to eq(current_plan)
+    end
+
+    it 'returns nil if no plan matches the code' do
+      plan_a = Hashie::Mash.new(code: 'bronze')
+      plan_b = Hashie::Mash.new(code: 'gold')
+
+      expect(helper.subscription_plan_info([plan_a, plan_b], 'default')).to be_nil
+    end
+
+    it 'breaks a tie with the current_subscription_plan attribute if multiple plans have the same code' do
+      other_plan = Hashie::Mash.new(current_subscription_plan: false, code: 'silver')
+      current_plan = Hashie::Mash.new(current_subscription_plan: true, code: 'silver')
+
+      expect(helper.subscription_plan_info([other_plan, current_plan], 'silver')).to eq(current_plan)
+    end
+
+    it 'returns nil if no plan matches the code even if current_subscription_plan is true' do
+      other_plan = Hashie::Mash.new(current_subscription_plan: false, code: 'free')
+      current_plan = Hashie::Mash.new(current_subscription_plan: true, code: 'bronze')
+
+      expect(helper.subscription_plan_info([other_plan, current_plan], 'default')).to be_nil
+    end
+
+    it 'returns the plan matching the plan code even if current_subscription_plan is false' do
+      other_plan = Hashie::Mash.new(current_subscription_plan: false, code: 'bronze')
+      current_plan = Hashie::Mash.new(current_subscription_plan: false, code: 'silver')
+
+      expect(helper.subscription_plan_info([other_plan, current_plan], 'silver')).to eq(current_plan)
+    end
+  end
 end

locale/gitlab.pot

@@ -4448,7 +4448,7 @@ msgstr ""
 msgid "Billing"
 msgstr ""
 
-msgid "BillingPlans|%{group_name} is currently using the %{plan_name} plan."
+msgid "BillingPlans|%{group_name} is currently using the %{plan_name}."
 msgstr ""
 
 msgid "BillingPlans|@%{user_name} you are currently using the %{plan_name} plan."