Refactor subscription table

- refactor app
- update / add tests

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

@@ -7,7 +7,11 @@ export default {
   components: {
     SubscriptionTable,
   },
-  inject: ['planUpgradeHref', 'planRenewHref', 'namespaceId', 'customerPortalUrl', 'namespaceName'],
+  inject: {
+    namespaceId: {
+      default: '',
+    },
+  },
   created() {
     this.setNamespaceId(this.namespaceId);
   },
@@ -18,10 +22,5 @@ export default {
 </script>
 
 <template>
-  <subscription-table
-    :namespace-name="namespaceName"
-    :plan-upgrade-href="planUpgradeHref"
-    :plan-renew-href="planRenewHref"
-    :customer-portal-url="customerPortalUrl"
-  />
+  <subscription-table />
 </template>

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

 <script>
 import { escape } from 'lodash';
 import { mapActions, mapState, mapGetters } from 'vuex';
-import { GlLoadingIcon } from '@gitlab/ui';
+import { GlButton, GlLoadingIcon } from '@gitlab/ui';
 import { TABLE_TYPE_DEFAULT, TABLE_TYPE_FREE, TABLE_TYPE_TRIAL } from 'ee/billings/constants';
 import { s__ } from '~/locale';
 import SubscriptionTableRow from './subscription_table_row.vue';
-import glFeaturesFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 const createButtonProps = (text, href, testId) => ({ text, href, testId });
 
 export default {
   name: 'SubscriptionTable',
   components: {
-    SubscriptionTableRow,
+    GlButton,
     GlLoadingIcon,
+    SubscriptionTableRow,
   },
-  mixins: [glFeaturesFlagMixin()],
-  props: {
-    namespaceName: {
-      type: String,
-      required: true,
+  mixins: [glFeatureFlagsMixin()],
+  inject: {
+    planUpgradeHref: {
+      default: '',
     },
-    customerPortalUrl: {
-      type: String,
-      required: false,
+    planRenewHref: {
       default: '',
     },
-    planUpgradeHref: {
-      type: String,
-      required: false,
+    namespaceId: {
       default: '',
     },
-    planRenewHref: {
-      type: String,
-      required: false,
+    customerPortalUrl: {
+      default: '',
+    },
+    namespaceName: {
       default: '',
     },
-  },
-  inject: {
     addSeatsHref: {
       default: '',
     },
@@ -65,12 +60,20 @@ export default {
     },
     addSeatsButton() {
       return this.canAddSeats
-        ? createButtonProps(s__('SubscriptionTable|Add seats'), this.addSeatsHref, 'add-seats')
+        ? createButtonProps(
+            s__('SubscriptionTable|Add seats'),
+            this.addSeatsHref,
+            'add-seats-button',
+          )
         : null;
     },
     upgradeButton() {
       return this.canUpgrade
-        ? createButtonProps(s__('SubscriptionTable|Upgrade'), this.upgradeButtonHref)
+        ? createButtonProps(
+            s__('SubscriptionTable|Upgrade'),
+            this.upgradeButtonHref,
+            'upgrade-button',
+          )
         : null;
     },
     upgradeButtonHref() {
@@ -78,12 +81,16 @@ export default {
     },
     renewButton() {
       return this.canRenew
-        ? createButtonProps(s__('SubscriptionTable|Renew'), this.planRenewHref)
+        ? createButtonProps(s__('SubscriptionTable|Renew'), this.planRenewHref, 'renew-button')
         : null;
     },
     manageButton() {
       return !this.isFreePlan
-        ? createButtonProps(s__('SubscriptionTable|Manage'), this.customerPortalUrl)
+        ? createButtonProps(
+            s__('SubscriptionTable|Manage'),
+            this.customerPortalUrl,
+            'manage-button',
+          )
         : null;
     },
     buttons() {
@@ -108,6 +115,9 @@ export default {
   },
   methods: {
     ...mapActions(['fetchSubscription']),
+    isLast(index) {
+      return index === this.visibleRows.length - 1;
+    },
   },
 };
 </script>
@@ -118,29 +128,31 @@ export default {
       v-if="!isLoadingSubscription && !hasErrorSubscription"
       class="card gl-mt-3 subscription-table js-subscription-table"
     >
-      <div class="js-subscription-header card-header">
+      <div class="card-header" data-testid="subscription-header">
         <strong>{{ subscriptionHeader }}</strong>
         <div class="controls">
-          <a
+          <gl-button
             v-for="(button, index) in buttons"
             :key="button.text"
             :href="button.href"
-            target="_blank"
-            rel="noopener noreferrer"
-            class="btn btn-inverted-secondary"
             :class="{ 'gl-ml-3': index !== 0 }"
             :data-testid="button.testId"
-            >{{ button.text }}</a
+            category="secondary"
+            target="_blank"
+            variant="info"
+            >{{ button.text }}</gl-button
           >
         </div>
       </div>
-      <div class="card-body flex-grid d-flex flex-column flex-sm-row flex-md-row flex-lg-column">
+      <div
+        class="card-body gl-display-flex gl-flex-column gl-sm-flex-direction-row flex-lg-column flex-grid"
+      >
         <subscription-table-row
           v-for="(row, i) in visibleRows"
           :key="`subscription-rows-${i}`"
+          :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, mapState } from 'vuex';
+import { mapActions, mapGetters, mapState } from 'vuex';
 import { dateInWords } from '~/lib/utils/datetime_utility';
 import Popover from '~/vue_shared/components/help_popover.vue';
 
@@ -20,7 +20,7 @@ export default {
       type: Array,
       required: true,
     },
-    isFreePlan: {
+    last: {
       type: Boolean,
       required: false,
       default: false,
@@ -29,6 +29,10 @@ export default {
   inject: ['billableSeatsHref'],
   computed: {
     ...mapState(['hasBillableGroupMembers']),
+    ...mapGetters(['isFreePlan']),
+    rowClasses() {
+      return !this.last ? 'gl-border-b-gray-100 gl-border-b-1 gl-border-b-solid' : null;
+    },
   },
   created() {
     this.fetchHasBillableGroupMembers();
@@ -64,7 +68,10 @@ export default {
 </script>
 
 <template>
-  <div class="grid-row d-flex flex-grow-1 flex-column flex-sm-column flex-md-column flex-lg-row">
+  <div
+    :class="rowClasses"
+    class="gl-display-flex gl-flex-grow-1 gl-flex-direction-column gl-lg-flex-direction-row"
+  >
     <div class="grid-cell header-cell" data-testid="header-cell">
       <span class="icon-wrapper">
         <gl-icon v-if="header.icon" class="gl-mr-3" :name="header.icon" />

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

-// Jest Snapshot v1, https://goo.gl/fbAQLP
-
-exports[`SubscriptionTable component given a bronze plan with state: isFreePlan=false, upgradable=true, isTrialPlan=false has Upgrade and Renew and Manage buttons 1`] = `
-Array [
-  Object {
-    "href": "http://test.host/plan/upgrade/bronze",
-    "text": "Upgrade",
-  },
-  Object {
-    "href": "https://customers.gitlab.com/subscriptions",
-    "text": "Manage",
-  },
-]
-`;
-
-exports[`SubscriptionTable component given a free plan with state: isFreePlan=true, upgradable=true, isTrialPlan=false has Upgrade and Renew and Manage buttons 1`] = `
-Array [
-  Object {
-    "href": "http://test.host/plan/upgrade/free",
-    "text": "Upgrade",
-  },
-  Object {
-    "href": "https://customers.gitlab.com/subscriptions",
-    "text": "Manage",
-  },
-]
-`;
-
-exports[`SubscriptionTable component given a gold plan with state: isFreePlan=false, upgradable=false, isTrialPlan=false has Renew and Manage buttons 1`] = `
-Array [
-  Object {
-    "href": "https://customers.gitlab.com/subscriptions",
-    "text": "Manage",
-  },
-]
-`;
-
-exports[`SubscriptionTable component given a trial-gold plan with state: isFreePlan=false, upgradable=false, isTrialPlan=true has Manage button 1`] = `
-Array [
-  Object {
-    "href": "https://customers.gitlab.com/subscriptions",
-    "text": "Manage",
-  },
-]
-`;
-
-exports[`SubscriptionTable component when created matches the snapshot 1`] = `
-<div>
-  <gl-loading-icon-stub
-    class="gl-mt-3 gl-mb-3"
-    color="dark"
-    label="Loading subscriptions"
-    size="lg"
-  />
-</div>
-`;

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

@@ -2,7 +2,6 @@ import { shallowMount, createLocalVue } from '@vue/test-utils';
 import Vuex from 'vuex';
 import initialStore from 'ee/billings/subscriptions/store';
 import SubscriptionApp from 'ee/billings/subscriptions/components/app.vue';
-import SubscriptionTable from 'ee/billings/subscriptions/components/subscription_table.vue';
 import * as types from 'ee/billings/subscriptions/store/mutation_types';
 import { mockDataSeats } from 'ee_jest/billings/mock_data';
 
@@ -34,15 +33,9 @@ describe('SubscriptionApp component', () => {
     });
   };
 
-  const expectComponentWithProps = (Component, props = {}) => {
-    const componentWrapper = wrapper.find(Component);
-
-    expect(componentWrapper.isVisible()).toBeTruthy();
-    expect(componentWrapper.props()).toEqual(expect.objectContaining(props));
-  };
-
   afterEach(() => {
     wrapper.destroy();
+    wrapper = null;
   });
 
   describe('on creation', () => {
@@ -54,14 +47,5 @@ describe('SubscriptionApp component', () => {
     it('dispatches expected actions on created', () => {
       expect(store.dispatch.mock.calls).toEqual([['setNamespaceId', '42']]);
     });
-
-    it('passes the correct props to the subscriptions table', () => {
-      expectComponentWithProps(SubscriptionTable, {
-        namespaceName: providedFields.namespaceName,
-        planUpgradeHref: providedFields.planUpgradeHref,
-        planRenewHref: providedFields.planRenewHref,
-        customerPortalUrl: providedFields.customerPortalUrl,
-      });
-    });
   });
 });

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

@@ -137,6 +137,21 @@ describe('subscription table row', () => {
     });
   });
 
+  describe('with free plan', () => {
+    const dateColumn = {
+      id: 'a',
+      label: 'Column A',
+      value: 0,
+      colClass: 'number',
+    };
+
+    it('renders a dash when the value is zero', () => {
+      createComponent({ props: { columns: [dateColumn] } });
+
+      expect(wrapper.find('[data-testid="property-value"]').text()).toBe('-');
+    });
+  });
+
   describe('date column', () => {
     const dateColumn = {
       id: 'c',
@@ -156,7 +171,6 @@ describe('subscription table row', () => {
       const outputDate = dateInWords(new Date(d[0], d[1] - 1, d[2]));
 
       expect(currentCol.find('[data-testid="property-label"]').text()).toMatch(dateColumn.label);
-
       expect(currentCol.find('[data-testid="property-value"]').text()).toMatch(outputDate);
     });
   });