Merge branch 'vs-refactor-subscription-table-row' into 'master'

Remove redundant request for fetching billable members

See merge request gitlab-org/gitlab!56874

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

@@ -108,7 +108,7 @@ export default {
       return this.tables[tableKey].rows;
     },
   },
-  mounted() {
+  created() {
     this.fetchSubscription();
   },
   methods: {
@@ -151,6 +151,7 @@ export default {
           :last="isLast(i)"
           :header="row.header"
           :columns="row.columns"
+          :is-free-plan="isFreePlan"
         />
       </div>
     </div>

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

 <script>
 import { GlIcon, GlButton } from '@gitlab/ui';
-import { mapActions, mapGetters, mapState } from 'vuex';
 import { dateInWords } from '~/lib/utils/datetime_utility';
 import Popover from '~/vue_shared/components/help_popover.vue';
 
@@ -11,7 +10,11 @@ export default {
     GlIcon,
     Popover,
   },
-  inject: ['billableSeatsHref', 'isGroup'],
+  inject: {
+    billableSeatsHref: {
+      default: '',
+    },
+  },
   props: {
     header: {
       type: Object,
@@ -26,21 +29,18 @@ export default {
       required: false,
       default: false,
     },
+    isFreePlan: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   computed: {
-    ...mapState(['hasBillableGroupMembers']),
-    ...mapGetters(['isFreePlan']),
     rowClasses() {
       return !this.last ? 'gl-border-b-gray-100 gl-border-b-1 gl-border-b-solid' : null;
     },
   },
-  created() {
-    if (this.isGroup) {
-      this.fetchHasBillableGroupMembers();
-    }
-  },
   methods: {
-    ...mapActions(['fetchHasBillableGroupMembers']),
     getPopoverOptions(col) {
       const defaults = {
         placement: 'bottom',
@@ -63,7 +63,7 @@ export default {
       return typeof col.value !== 'undefined' && col.value !== null ? col.value : ' - ';
     },
     isSeatsUsageButtonShown(col) {
-      return this.hasBillableGroupMembers && this.billableSeatsHref && col.id === 'seatsInUse';
+      return this.billableSeatsHref && col.id === 'seatsInUse';
     },
   },
 };

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';
 
@@ -17,7 +16,6 @@ export default (containerId = 'js-billing-plans') => {
     namespaceId,
     namespaceName,
     addSeatsHref,
-    isGroup,
     planUpgradeHref,
     planRenewHref,
     customerPortalUrl,
@@ -32,7 +30,6 @@ export default (containerId = 'js-billing-plans') => {
       namespaceId,
       namespaceName,
       addSeatsHref,
-      isGroup: parseBoolean(isGroup),
       planUpgradeHref,
       planRenewHref,
       customerPortalUrl,

ee/app/assets/javascripts/billings/subscriptions/store/actions.js

@@ -29,27 +29,3 @@ export const receiveSubscriptionError = ({ commit }) => {
   });
   commit(types.RECEIVE_SUBSCRIPTION_ERROR);
 };
-
-/**
- * Billable Members
- */
-export const fetchHasBillableGroupMembers = ({ dispatch, state }) => {
-  dispatch('requestHasBillableGroupMembers');
-
-  return Api.fetchBillableGroupMembersList(state.namespaceId, { per_page: 1, page: 1 })
-    .then((data) => dispatch('receiveHasBillableGroupMembersSuccess', data))
-    .catch(() => dispatch('receiveHasBillableGroupMembersError'));
-};
-
-export const requestHasBillableGroupMembers = ({ commit }) =>
-  commit(types.REQUEST_HAS_BILLABLE_MEMBERS);
-
-export const receiveHasBillableGroupMembersSuccess = ({ commit }, response) =>
-  commit(types.RECEIVE_HAS_BILLABLE_MEMBERS_SUCCESS, response);
-
-export const receiveHasBillableGroupMembersError = ({ commit }) => {
-  createFlash({
-    message: s__('SubscriptionTable|An error occurred while loading billable members list'),
-  });
-  commit(types.RECEIVE_HAS_BILLABLE_MEMBERS_ERROR);
-};

ee/app/assets/javascripts/billings/subscriptions/store/mutation_types.js

@@ -3,7 +3,3 @@ export const SET_NAMESPACE_ID = 'SET_NAMESPACE_ID';
 export const REQUEST_SUBSCRIPTION = 'REQUEST_SUBSCRIPTION';
 export const RECEIVE_SUBSCRIPTION_SUCCESS = 'RECEIVE_SUBSCRIPTION_SUCCESS';
 export const RECEIVE_SUBSCRIPTION_ERROR = 'RECEIVE_SUBSCRIPTION_ERROR';
-
-export const REQUEST_HAS_BILLABLE_MEMBERS = 'REQUEST_HAS_BILLABLE_MEMBERS';
-export const RECEIVE_HAS_BILLABLE_MEMBERS_SUCCESS = 'RECEIVE_HAS_BILLABLE_MEMBERS_SUCCESS';
-export const RECEIVE_HAS_BILLABLE_MEMBERS_ERROR = 'RECEIVE_HAS_BILLABLE_MEMBERS_ERROR';

ee/app/assets/javascripts/billings/subscriptions/store/mutations.js

-import { parseInt } from 'lodash';
 import Vue from 'vue';
-import {
-  TABLE_TYPE_DEFAULT,
-  TABLE_TYPE_FREE,
-  TABLE_TYPE_TRIAL,
-  HEADER_TOTAL_ENTRIES,
-} from 'ee/billings/constants';
+import { TABLE_TYPE_DEFAULT, TABLE_TYPE_FREE, TABLE_TYPE_TRIAL } from 'ee/billings/constants';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import * as types from './mutation_types';
 
@@ -49,22 +43,4 @@ export default {
     state.isLoadingSubscription = false;
     state.hasErrorSubscription = true;
   },
-
-  [types.REQUEST_HAS_BILLABLE_MEMBERS](state) {
-    state.isLoadingHasBillableMembers = true;
-    state.hasErrorHasBillableMembers = false;
-  },
-
-  [types.RECEIVE_HAS_BILLABLE_MEMBERS_SUCCESS](state, payload) {
-    const { headers } = payload;
-    const hasBillableGroupMembers = parseInt(headers[HEADER_TOTAL_ENTRIES], 10) > 0;
-
-    state.hasBillableGroupMembers = hasBillableGroupMembers;
-    state.isLoadingHasBillableMembers = false;
-  },
-
-  [types.RECEIVE_HAS_BILLABLE_MEMBERS_ERROR](state) {
-    state.isLoadinggHasBillableMembers = false;
-    state.hasErrorHasBillableMembers = true;
-  },
 };

ee/app/assets/javascripts/billings/subscriptions/store/state.js

@@ -3,8 +3,6 @@ import { s__ } from '~/locale';
 export default () => ({
   isLoadingSubscription: false,
   hasErrorSubscription: false,
-  isLoadingBillableMembers: false,
-  hasErrorBillableMembers: false,
   namespaceId: null,
   plan: {
     code: null,
@@ -171,5 +169,4 @@ export default () => ({
       ],
     },
   },
-  hasBillableGroupMembers: false,
 });

ee/app/helpers/billing_plans_helper.rb

@@ -40,7 +40,6 @@ module BillingPlansHelper
     {
       namespace_id: namespace.id,
       namespace_name: namespace.name,
-      is_group: namespace.group?.to_s,
       add_seats_href: add_seats_url(namespace),
       plan_upgrade_href: plan_upgrade_url(namespace, plan),
       plan_renew_href: plan_renew_url(namespace),
@@ -155,8 +154,10 @@ module BillingPlansHelper
     "#{EE::SUBSCRIPTIONS_URL}/gitlab/namespaces/#{group.id}/renew"
   end
 
-  def billable_seats_href(group)
-    group_seat_usage_path(group)
+  def billable_seats_href(namespace)
+    return unless namespace.group?
+
+    group_seat_usage_path(namespace)
   end
 
   def offer_from_previous_tier?(namespace_id, plan_id)

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

@@ -2,8 +2,6 @@ import { shallowMount, createLocalVue } from '@vue/test-utils';
 import Vuex from 'vuex';
 import SubscriptionApp from 'ee/billings/subscriptions/components/app.vue';
 import initialStore from 'ee/billings/subscriptions/store';
-import * as types from 'ee/billings/subscriptions/store/mutation_types';
-import { mockDataSeats } from 'ee_jest/billings/mock_data';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -35,13 +33,11 @@ describe('SubscriptionApp component', () => {
 
   afterEach(() => {
     wrapper.destroy();
-    wrapper = null;
   });
 
   describe('on creation', () => {
     beforeEach(() => {
       factory();
-      store.commit(`${types.RECEIVE_HAS_BILLABLE_MEMBERS_SUCCESS}`, mockDataSeats);
     });
 
     it('dispatches expected actions on created', () => {

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

@@ -39,15 +39,7 @@ describe('subscription table row', () => {
 
   const defaultProps = { header: HEADER, columns: COLUMNS };
 
-  const createComponent = ({
-    props = {},
-    billableSeatsHref = BILLABLE_SEATS_URL,
-    isGroup = true,
-  } = {}) => {
-    if (wrapper) {
-      throw new Error('wrapper already exists!');
-    }
-
+  const createComponent = ({ props = {}, billableSeatsHref = BILLABLE_SEATS_URL } = {}) => {
     wrapper = shallowMount(SubscriptionTableRow, {
       propsData: {
         ...defaultProps,
@@ -55,7 +47,6 @@ describe('subscription table row', () => {
       },
       provide: {
         billableSeatsHref,
-        isGroup,
       },
       store,
       localVue,
@@ -69,7 +60,6 @@ describe('subscription table row', () => {
 
   afterEach(() => {
     wrapper.destroy();
-    wrapper = null;
   });
 
   const findHeaderCell = () => wrapper.find('[data-testid="header-cell"]');
@@ -88,18 +78,6 @@ describe('subscription table row', () => {
 
   const findUsageButton = () => findContentCells().at(0).find('[data-testid="seats-usage-button"]');
 
-  describe('dispatched actions', () => {
-    it('dispatches action when created if namespace is group', () => {
-      createComponent();
-      expect(store.dispatch).toHaveBeenCalledWith('fetchHasBillableGroupMembers');
-    });
-
-    it('does not dispatch action when created if namespace is not group', () => {
-      createComponent({ isGroup: false });
-      expect(store.dispatch).not.toHaveBeenCalledWith('fetchHasBillableGroupMembers');
-    });
-  });
-
   describe('default', () => {
     beforeEach(() => {
       createComponent();
@@ -156,7 +134,7 @@ describe('subscription table row', () => {
     };
 
     it('renders a dash when the value is zero', () => {
-      createComponent({ props: { columns: [dateColumn] } });
+      createComponent({ props: { isFreePlan: true, columns: [dateColumn] } });
 
       expect(wrapper.find('[data-testid="property-value"]').text()).toBe('-');
     });
@@ -185,22 +163,23 @@ describe('subscription table row', () => {
     });
   });
 
-  it.each`
-    state                                 | columnId        | provide                      | exists   | attrs
-    ${{ hasBillableGroupMembers: true }}  | ${'seatsInUse'} | ${{}}                        | ${true}  | ${{ href: BILLABLE_SEATS_URL }}
-    ${{ hasBillableGroupMembers: true }}  | ${'seatsInUse'} | ${{ billableSeatsHref: '' }} | ${false} | ${{}}
-    ${{ hasBillableGroupMembers: true }}  | ${'some_value'} | ${{}}                        | ${false} | ${{}}
-    ${{ hasBillableGroupMembers: false }} | ${'seatsInUse'} | ${{}}                        | ${false} | ${{}}
-  `(
-    'should exists=$exists with (state=$state, columnId=$columnId, provide=$provide)',
-    ({ state, columnId, provide, exists, attrs }) => {
-      Object.assign(store.state, state);
-      createComponent({ props: { columns: [{ id: columnId }] }, ...provide });
-
-      expect(findUsageButton().exists()).toBe(exists);
-      if (exists) {
-        expect(findUsageButton().attributes()).toMatchObject(attrs);
-      }
-    },
-  );
+  describe('seats in use button', () => {
+    it.each`
+      columnId        | provide                        | exists   | attrs
+      ${'seatsInUse'} | ${{}}                          | ${true}  | ${{ href: BILLABLE_SEATS_URL }}
+      ${'seatsInUse'} | ${{ billableSeatsHref: '' }}   | ${false} | ${{}}
+      ${'some_value'} | ${{}}                          | ${false} | ${{}}
+      ${'seatsInUse'} | ${{ billableSeatsHref: null }} | ${false} | ${{}}
+    `(
+      'should exists=$exists with (columnId=$columnId, provide=$provide)',
+      ({ columnId, provide, exists, attrs }) => {
+        createComponent({ props: { columns: [{ id: columnId }] }, ...provide });
+
+        expect(findUsageButton().exists()).toBe(exists);
+        if (exists) {
+          expect(findUsageButton().attributes()).toMatchObject(attrs);
+        }
+      },
+    );
+  });
 });

ee/spec/helpers/billing_plans_helper_spec.rb

@@ -21,7 +21,6 @@ RSpec.describe BillingPlansHelper do
         expect(helper.subscription_plan_data_attributes(group, plan))
           .to eq(namespace_id: group.id,
                  namespace_name: group.name,
-                 is_group: "true",
                  add_seats_href: add_seats_href,
                  plan_upgrade_href: upgrade_href,
                  plan_renew_href: renew_href,
@@ -51,7 +50,6 @@ RSpec.describe BillingPlansHelper do
           .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,
@@ -71,7 +69,6 @@ RSpec.describe BillingPlansHelper do
         expect(helper.subscription_plan_data_attributes(group, plan))
           .to eq(namespace_id: group.id,
                  namespace_name: group.name,
-                 is_group: "true",
                  customer_portal_url: customer_portal_url,
                  billable_seats_href: billable_seats_href,
                  add_seats_href: add_seats_href,
@@ -81,13 +78,23 @@ RSpec.describe BillingPlansHelper do
       end
     end
 
-    context 'when namespace is passed in' do
-      it 'returns false for is_group' do
-        namespace = build(:namespace)
+    context 'with different namespaces' do
+      subject { helper.subscription_plan_data_attributes(namespace, plan) }
 
-        result = helper.subscription_plan_data_attributes(namespace, plan)
+      context 'with namespace' do
+        let(:namespace) { build(:namespace) }
 
-        expect(result).to include(is_group: "false")
+        it 'does not return billable_seats_href' do
+          expect(subject).not_to include(billable_seats_href: helper.group_seat_usage_path(namespace))
+        end
+      end
+
+      context 'with group' do
+        let(:namespace) { build(:group) }
+
+        it 'returns billable_seats_href for group' do
+          expect(subject).to include(billable_seats_href: helper.group_seat_usage_path(namespace))
+        end
       end
     end
   end

locale/gitlab.pot

@@ -29191,9 +29191,6 @@ msgstr ""
 msgid "SubscriptionTable|Add seats"
 msgstr ""
 
-msgid "SubscriptionTable|An error occurred while loading billable members list"
-msgstr ""
-
 msgid "SubscriptionTable|An error occurred while loading the subscription details."
 msgstr ""