Merge branch '352231-purchase-workflow-min-users-count-fix' into 'master'

Fixed incorrect minimum number of users on subscription purchase flow

See merge request gitlab-org/gitlab!82480

ee/app/assets/javascripts/subscriptions/graphql/queries/billable_members_count.query.graphql

+query getBillableMembersCount($fullPath: ID!, $requestedHostedPlan: String) {
+  group(fullPath: $fullPath) {
+    id
+    billableMembersCount(requestedHostedPlan: $requestedHostedPlan)
+  }
+}

ee/app/assets/javascripts/subscriptions/new/components/checkout/subscription_details.vue

 <script>
-import { GlAlert, GlFormGroup, GlFormSelect, GlFormInput, GlSprintf, GlLink } from '@gitlab/ui';
+import {
+  GlAlert,
+  GlFormGroup,
+  GlFormSelect,
+  GlFormInput,
+  GlSprintf,
+  GlLink,
+  GlLoadingIcon,
+} from '@gitlab/ui';
 import { isEmpty } from 'lodash';
 import { mapState, mapGetters, mapActions } from 'vuex';
 import { QSR_RECONCILIATION_PATH, STEP_SUBSCRIPTION_DETAILS } from 'ee/subscriptions/constants';
@@ -9,6 +17,7 @@ import { sprintf, s__, __ } from '~/locale';
 import autofocusonshow from '~/vue_shared/directives/autofocusonshow';
 import Tracking from '~/tracking';
 import { helpPagePath } from '~/helpers/help_page_helper';
+import getBillableMembersCountQuery from 'ee/subscriptions/graphql/queries/billable_members_count.query.graphql';
 
 export default {
   components: {
@@ -18,12 +27,44 @@ export default {
     GlFormInput,
     GlSprintf,
     GlLink,
+    GlLoadingIcon,
     Step,
   },
   directives: {
     autofocusonshow,
   },
   mixins: [Tracking.mixin()],
+  apollo: {
+    billableMembersMin: {
+      query: getBillableMembersCountQuery,
+      loadingKey: 'isLoading',
+      variables() {
+        return {
+          fullPath: this.selectedGroupFullPath,
+          requestedHostedPlan: this.selectedPlanDetails.code,
+        };
+      },
+      update(data) {
+        this.resetError();
+
+        return data.group.billableMembersCount || 1;
+      },
+      skip() {
+        return this.shouldSkipQuery;
+      },
+      error(error) {
+        this.handleError(error);
+      },
+    },
+  },
+  data() {
+    return {
+      billableMembersMin: 1,
+      errorMessage: '',
+      isLoading: 0,
+      showError: false,
+    };
+  },
   computed: {
     ...mapState([
       'availablePlans',
@@ -39,10 +80,14 @@ export default {
       'selectedPlanText',
       'selectedPlanDetails',
       'selectedGroupId',
+      'selectedGroupData',
       'isGroupSelected',
-      'selectedGroupUsers',
       'selectedGroupName',
+      'isSelectedGroupPresent',
     ]),
+    hasError() {
+      return Boolean(this.errorMessage);
+    },
     selectedPlanModel: {
       get() {
         return this.selectedPlan;
@@ -78,6 +123,23 @@ export default {
     selectedPlanTextLine() {
       return sprintf(this.$options.i18n.selectedPlan, { selectedPlanText: this.selectedPlanText });
     },
+    selectedGroupFullPath() {
+      return this.selectedGroupData?.fullPath;
+    },
+    shouldSkipQuery() {
+      return (
+        !this.selectedGroupFullPath || !this.hasSelectedPlan || this.shouldDisableNumberOfUsers
+      );
+    },
+    numberOfUsersLabelDescription() {
+      if (this.shouldSkipQuery || this.hasError) {
+        return null;
+      }
+
+      return sprintf(this.$options.i18n.numberOfUsersLabelDescription, {
+        minimumNumberOfUsers: this.billableMembersMin,
+      });
+    },
     hasAtLeastOneUser() {
       return this.numberOfUsers > 0;
     },
@@ -94,7 +156,7 @@ export default {
       return true;
     },
     isSelectedUsersEqualOrGreaterThanGroupUsers() {
-      return this.numberOfUsers >= this.selectedGroupUsers;
+      return this.numberOfUsers >= this.billableMembersMin;
     },
     isValid() {
       return (
@@ -132,6 +194,16 @@ export default {
       return this.isNewUser && !this.isSetupForCompany;
     },
   },
+  watch: {
+    billableMembersMin(val) {
+      this.updateNumberOfUsers(val);
+    },
+    isSelectedGroupPresent(isSelectedGroupPresent) {
+      if (!isSelectedGroupPresent) {
+        this.billableMembersMin = 1;
+      }
+    },
+  },
   methods: {
     ...mapActions([
       'updateSelectedPlan',
@@ -140,6 +212,17 @@ export default {
       'updateNumberOfUsers',
       'updateOrganizationName',
     ]),
+    hideError() {
+      this.showError = false;
+    },
+    handleError(error) {
+      this.errorMessage = error || __('An unexpected error occurred');
+      this.showError = true;
+    },
+    resetError() {
+      this.errorMessage = '';
+      this.showError = false;
+    },
     trackStepTransition() {
       this.track('click_button', {
         label: 'update_plan_type',
@@ -167,6 +250,10 @@ export default {
     createNewGroupDescription: s__("Checkout|You'll create your new group after checkout"),
     organizationNameLabel: s__('Checkout|Name of company or organization using GitLab'),
     numberOfUsersLabel: s__('Checkout|Number of users'),
+    numberOfUsersLabelDescription: s__(
+      'Checkout|This number must be %{minimumNumberOfUsers} (your seats in use) or more.',
+    ),
+    loadingText: s__('Checkout|Calculating your subscription...'),
     needMoreUsersLink: s__('Checkout|Need more users? Purchase GitLab for your %{company}.'),
     companyOrTeam: s__('Checkout|company or team'),
     selectedPlan: s__('Checkout|%{selectedPlanText} plan'),
@@ -182,6 +269,23 @@ export default {
 </script>
 <template>
   <step
+    v-if="isLoading"
+    :step-id="$options.stepId"
+    :title="$options.i18n.stepTitle"
+    :is-valid="false"
+  >
+    <template #body>
+      <div
+        data-testid="subscription-loading-container"
+        class="gl-display-flex gl-h-200! gl-justify-content-center gl-align-items-center gl-flex-direction-column"
+      >
+        <gl-loading-icon v-if="true" size="md" />
+        <span>{{ $options.i18n.loadingText }}</span>
+      </div>
+    </template>
+  </step>
+  <step
+    v-else
     :step-id="$options.stepId"
     :title="$options.i18n.stepTitle"
     :is-valid="isValid"
@@ -190,6 +294,14 @@ export default {
     @stepEdit="trackStepEdit"
   >
     <template #body>
+      <gl-alert
+        v-if="hasError && showError"
+        data-testid="error-message"
+        class="gl-mb-5"
+        variant="danger"
+        @dismiss="hideError"
+        >{{ errorMessage }}</gl-alert
+      >
       <gl-form-group :label="$options.i18n.selectedPlanLabel" label-size="sm" class="mb-3">
         <gl-form-select
           v-model="selectedPlanModel"
@@ -222,15 +334,18 @@ export default {
       </gl-form-group>
       <div class="combined d-flex">
         <gl-form-group
+          data-testid="number-of-users-field"
           :label="$options.i18n.numberOfUsersLabel"
           label-size="sm"
-          class="number gl-mb-0"
+          class="gl-mb-0"
+          :label-description="numberOfUsersLabelDescription"
         >
           <gl-form-input
             ref="number-of-users"
             v-model.number="numberOfUsersModel"
+            class="number"
             type="number"
-            :min="selectedGroupUsers"
+            :min="billableMembersMin"
             :disabled="shouldDisableNumberOfUsers"
             data-qa-selector="number_of_users"
           />

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

@@ -11,14 +11,12 @@ import * as types from './mutation_types';
 
 export const updateSelectedPlan = ({ commit, getters }, selectedPlan) => {
   commit(types.UPDATE_SELECTED_PLAN, selectedPlan);
-  commit(types.UPDATE_NUMBER_OF_USERS, getters.selectedGroupUsers);
   trackCheckout(selectedPlan, getters.confirmOrderParams?.subscription?.quantity);
 };
 
-export const updateSelectedGroup = ({ commit, getters }, selectedGroup) => {
+export const updateSelectedGroup = ({ commit }, selectedGroup) => {
   commit(types.UPDATE_SELECTED_GROUP, selectedGroup);
   commit(types.UPDATE_ORGANIZATION_NAME, null);
-  commit(types.UPDATE_NUMBER_OF_USERS, getters.selectedGroupUsers);
 };
 
 export const toggleIsSetupForCompany = ({ state, commit }) => {

ee/app/assets/javascripts/subscriptions/new/store/getters.js

@@ -60,28 +60,18 @@ export const usersPresent = (state) => state.numberOfUsers > 0;
 export const isGroupSelected = (state) =>
   state.selectedGroup !== null && state.selectedGroup !== NEW_GROUP;
 
-export const isSelectedGroupPresent = (state, getters) => {
-  return (
-    getters.isGroupSelected && state.groupData.some((group) => group.value === state.selectedGroup)
-  );
-};
+export const isSelectedGroupPresent = (state, getters) => Boolean(getters.selectedGroupData);
 
-export const selectedGroupUsers = (state, getters) => {
+export const selectedGroupData = (state, getters) => {
   if (!getters.isGroupSelected) {
-    return 1;
-  } else if (getters.isSelectedGroupPresent && getters.isUltimatePlan) {
-    const selectedGroup = state.groupData.find((group) => group.value === state.selectedGroup);
-    return selectedGroup.numberOfUsers - selectedGroup.numberOfGuests;
-  } else if (getters.isSelectedGroupPresent) {
-    return state.groupData.find((group) => group.value === state.selectedGroup).numberOfUsers;
+    return null;
   }
 
-  return null;
+  return state.groupData.find((group) => group.value === state.selectedGroup);
 };
 
 export const selectedGroupName = (state, getters) => {
-  if (!getters.isGroupSelected) return null;
-  return state.groupData.find((group) => group.value === state.selectedGroup).text;
+  return getters.selectedGroupData?.text;
 };
 
 export const selectedGroupId = (state, getters) =>

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

@@ -14,8 +14,7 @@ const parseGroupData = (groupData) =>
   JSON.parse(groupData).map((group) => ({
     value: group.id,
     text: group.name,
-    numberOfUsers: group.users,
-    numberOfGuests: group.guests,
+    fullPath: group.full_path,
   }));
 
 const determineSelectedPlan = (planId, plans) => {
@@ -25,20 +24,6 @@ const determineSelectedPlan = (planId, plans) => {
   return plans[0] && plans[0].value;
 };
 
-const determineNumberOfUsers = (groupId, groups) => {
-  if (!groupId || !groups) {
-    return 1;
-  }
-
-  const chosenGroup = groups.find((group) => group.value === groupId);
-
-  if (chosenGroup?.numberOfUsers > 1) {
-    return chosenGroup.numberOfUsers;
-  }
-
-  return 1;
-};
-
 export default ({
   availablePlans: plansData = '[]',
   planId,
@@ -66,7 +51,7 @@ export default ({
     groupData: groups,
     selectedGroup: groupId,
     organizationName: null,
-    numberOfUsers: determineNumberOfUsers(groupId, groups),
+    numberOfUsers: 1,
     country: null,
     streetAddressLine1: null,
     streetAddressLine2: null,

ee/app/helpers/subscriptions_helper.rb

@@ -66,7 +66,8 @@ module SubscriptionsHelper
       account_id: account_id,
       name: namespace.name,
       users: namespace.member_count,
-      guests: namespace.guest_count
+      guests: namespace.guest_count,
+      full_path: namespace.full_path
     }
   end
 end

ee/spec/frontend/subscriptions/new/store/actions_spec.js

@@ -39,11 +39,8 @@ describe('Subscriptions Actions', () => {
       await testAction(
         actions.updateSelectedPlan,
         'planId',
-        { selectedGroupUsers: 4 },
-        [
-          { type: 'UPDATE_SELECTED_PLAN', payload: 'planId' },
-          { type: 'UPDATE_NUMBER_OF_USERS', payload: 4 },
-        ],
+        {},
+        [{ type: 'UPDATE_SELECTED_PLAN', payload: 'planId' }],
         [],
       );
     });
@@ -54,11 +51,10 @@ describe('Subscriptions Actions', () => {
       await testAction(
         actions.updateSelectedGroup,
         'groupId',
-        { selectedGroupUsers: 3 },
+        {},
         [
           { type: 'UPDATE_SELECTED_GROUP', payload: 'groupId' },
           { type: 'UPDATE_ORGANIZATION_NAME', payload: null },
-          { type: 'UPDATE_NUMBER_OF_USERS', payload: 3 },
         ],
         [],
       );

ee/spec/frontend/subscriptions/new/store/getters_spec.js

@@ -151,90 +151,56 @@ describe('Subscriptions Getters', () => {
     });
   });
 
-  describe('selectedGroupUsers', () => {
-    it('returns 1 when no group is selected', () => {
-      expect(
-        getters.selectedGroupUsers(
-          { groupData: [{ numberOfUsers: 3, value: 123 }], selectedGroup: 123 },
-          { isGroupSelected: false },
-        ),
-      ).toBe(1);
-    });
-
-    it('returns `null` when a group is selected, but not present', () => {
-      expect(
-        getters.selectedGroupUsers(
-          { groupData: [{ numberOfUsers: 3, value: 123 }], selectedGroup: 123 },
-          { isGroupSelected: true, isSelectedGroupPresent: false },
-        ),
-      ).toBe(null);
-    });
-
-    it('returns the number of users of the selected group when a group is selected and plan is not ultimate', () => {
-      expect(
-        getters.selectedGroupUsers(
-          { groupData: [{ numberOfUsers: 3, numberOfGuests: 1, value: 123 }], selectedGroup: 123 },
-          { isGroupSelected: true, isSelectedGroupPresent: true, isUltimatePlan: false },
-        ),
-      ).toBe(3);
+  describe('isSelectedGroupPresent', () => {
+    it('returns true when known group is selected', () => {
+      expect(getters.isSelectedGroupPresent({}, { selectedGroupData: {} })).toBe(true);
     });
 
-    it('returns difference between the number of users and guests of the selected group if the selected plan is ultimate', () => {
-      expect(
-        getters.selectedGroupUsers(
-          { groupData: [{ numberOfUsers: 3, numberOfGuests: 1, value: 123 }], selectedGroup: 123 },
-          { isGroupSelected: true, isSelectedGroupPresent: true, isUltimatePlan: true },
-        ),
-      ).toBe(2);
+    it('returns false when no group is selected', () => {
+      expect(getters.isSelectedGroupPresent({}, { selectedGroupData: undefined })).toBe(false);
     });
   });
 
-  describe('isSelectedGroupPresent', () => {
-    it('returns false when group is not selected', () => {
+  describe('selectedGroupData', () => {
+    it('returns null when no group is selected', () => {
       expect(
-        getters.isSelectedGroupPresent(
-          { groupData: [{ numberOfUsers: 3, value: 123 }], selectedGroup: null },
+        getters.selectedGroupData(
+          {
+            groupData: [
+              { text: 'Not selected group', value: 'not-selected-group' },
+              { text: 'Selected group', value: 'selected-group' },
+            ],
+          },
           { isGroupSelected: false },
         ),
-      ).toBe(false);
-    });
-
-    it('returns false when group is selected, but not present', () => {
-      expect(
-        getters.isSelectedGroupPresent(
-          { groupData: [{ numberOfUsers: 3, value: 123 }], selectedGroup: 321 },
-          { isGroupSelected: true },
-        ),
-      ).toBe(false);
+      ).toBe(null);
     });
 
-    it('returns true when group is selected and is present', () => {
+    it('returns the selected group when a group is selected', () => {
       expect(
-        getters.isSelectedGroupPresent(
-          { groupData: [{ numberOfUsers: 3, value: 123 }], selectedGroup: 123 },
+        getters.selectedGroupData(
+          {
+            selectedGroup: 'selected-group',
+            groupData: [
+              { text: 'Not selected group', value: 'not-selected-group' },
+              { text: 'Selected group', value: 'selected-group' },
+            ],
+          },
           { isGroupSelected: true },
         ),
-      ).toBe(true);
+      ).toEqual({ text: 'Selected group', value: 'selected-group' });
     });
   });
 
   describe('selectedGroupName', () => {
     it('returns null when no group is selected', () => {
-      expect(
-        getters.selectedGroupName(
-          { groupData: [{ text: 'Selected group', value: 123 }], selectedGroup: 123 },
-          { isGroupSelected: false },
-        ),
-      ).toBe(null);
+      expect(getters.selectedGroupName({}, { selectedGroupData: undefined })).toBe(undefined);
     });
 
     it('returns the text attribute of the selected group when a group is selected', () => {
-      expect(
-        getters.selectedGroupName(
-          { groupData: [{ text: 'Selected group', value: 123 }], selectedGroup: 123 },
-          { isGroupSelected: true },
-        ),
-      ).toBe('Selected group');
+      expect(getters.selectedGroupName({}, { selectedGroupData: { text: 'Selected group' } })).toBe(
+        'Selected group',
+      );
     });
   });
 

ee/spec/frontend/subscriptions/new/store/state_spec.js

@@ -10,8 +10,8 @@ describe('projectsSelector default state', () => {
   ];
 
   const groupData = [
-    { id: 132, name: 'My first group', users: 3, guests: 1 },
-    { id: 483, name: 'My second group', users: 12, guests: 0 },
+    { id: 132, name: 'My first group', full_path: 'my-first-group' },
+    { id: 483, name: 'My second group', full_path: 'my-second-group' },
   ];
 
   const initialData = {
@@ -98,8 +98,8 @@ describe('projectsSelector default state', () => {
   describe('groupData', () => {
     it('sets the groupData to the provided parsed groupData', () => {
       expect(state.groupData).toEqual([
-        { value: 132, text: 'My first group', numberOfUsers: 3, numberOfGuests: 1 },
-        { value: 483, text: 'My second group', numberOfUsers: 12, numberOfGuests: 0 },
+        { value: 132, text: 'My first group', fullPath: 'my-first-group' },
+        { value: 483, text: 'My second group', fullPath: 'my-second-group' },
       ]);
     });
 

ee/spec/frontend/vue_shared/purchase_flow/spec_helper.js

@@ -4,8 +4,13 @@ import stepListQuery from 'ee/vue_shared/purchase_flow/graphql/queries/step_list
 import resolvers from 'ee/vue_shared/purchase_flow/graphql/resolvers';
 import createMockApollo from 'helpers/mock_apollo_helper';
 
-export function createMockApolloProvider(stepList, initialStepIndex = 0, additionalResolvers = {}) {
-  const mockApollo = createMockApollo([], merge({}, resolvers, additionalResolvers));
+export function createMockApolloProvider(
+  stepList,
+  initialStepIndex = 0,
+  additionalResolvers = {},
+  handlers = [],
+) {
+  const mockApollo = createMockApollo(handlers, merge({}, resolvers, additionalResolvers));
   mockApollo.clients.defaultClient.cache.writeQuery({
     query: stepListQuery,
     data: { stepList },

ee/spec/helpers/subscriptions_helper_spec.rb

@@ -51,7 +51,7 @@ RSpec.describe SubscriptionsHelper do
     it { is_expected.to include(plan_id: 'bronze_id') }
     it { is_expected.to include(namespace_id: group.id.to_s) }
     it { is_expected.to include(source: 'some_source') }
-    it { is_expected.to include(group_data: %Q{[{"id":#{group.id},"account_id":null,"name":"My Namespace","users":2,"guests":1}]}) }
+    it { is_expected.to include(group_data: %Q{[{"id":#{group.id},"account_id":null,"name":"My Namespace","users":2,"guests":1,"full_path":"my_namespace"}]}) }
     it { is_expected.to include(trial: 'false') }
     it { is_expected.to include(new_trial_registration_path: '/-/trial_registrations/new') }
 
@@ -163,7 +163,7 @@ RSpec.describe SubscriptionsHelper do
     it { is_expected.to include(namespace_id: group.id.to_s) }
     it { is_expected.to include(active_subscription: active_subscription) }
     it { is_expected.to include(source: 'some_source') }
-    it { is_expected.to include(group_data: %Q{[{"id":#{group.id},"account_id":"#{account_id}","name":"My Namespace","users":1,"guests":0}]}) }
+    it { is_expected.to include(group_data: %Q{[{"id":#{group.id},"account_id":"#{account_id}","name":"My Namespace","users":1,"guests":0,"full_path":"my_namespace"}]}) }
     it { is_expected.to include(redirect_after_success: group_usage_quotas_path(group, anchor: anchor, purchased_product: purchased_product)) }
   end
 end

locale/gitlab.pot

@@ -4122,6 +4122,9 @@ msgstr ""
 msgid "An unauthenticated user"
 msgstr ""
 
+msgid "An unexpected error occurred"
+msgstr ""
+
 msgid "An unexpected error occurred while checking the project environment."
 msgstr ""
 
@@ -7106,6 +7109,9 @@ msgstr ""
 msgid "Checkout|CI minutes"
 msgstr ""
 
+msgid "Checkout|Calculating your subscription..."
+msgstr ""
+
 msgid "Checkout|Checkout"
 msgstr ""
 
@@ -7220,6 +7226,9 @@ msgstr ""
 msgid "Checkout|Tax"
 msgstr ""
 
+msgid "Checkout|This number must be %{minimumNumberOfUsers} (your seats in use) or more."
+msgstr ""
+
 msgid "Checkout|Total"
 msgstr ""