refactor: remove BFF transform for subscriptions data (#1253)

openedx · Jan 15, 2025 · e7d4a70 · e7d4a70
1 parent f586265
commit e7d4a70
Show file tree

Hide file tree

Showing 9 changed files with 265 additions and 197 deletions.
diff --git a/src/components/app/data/constants.js b/src/components/app/data/constants.js
@@ -85,7 +85,7 @@ export const getBaseSubscriptionsData = () => {
     customerAgreement: null,
     subscriptionLicense: null,
     subscriptionPlan: null,
-    licensesByStatus: _cloneDeep(baseLicensesByStatus),
+    subscriptionLicensesByStatus: _cloneDeep(baseLicensesByStatus),
     showExpirationNotifications: false,
   };
   return {

diff --git a/src/components/app/data/hooks/useCatalogsForSubsidyRequests.test.jsx b/src/components/app/data/hooks/useCatalogsForSubsidyRequests.test.jsx
@@ -2,7 +2,7 @@ import { renderHook } from '@testing-library/react-hooks';
 import { QueryClientProvider } from '@tanstack/react-query';
 import { queryClient } from '../../../../utils/tests';
 import useCatalogsForSubsidyRequest from './useCatalogsForSubsidyRequests';
-import { LICENSE_STATUS } from '../../../enterprise-user-subsidy/data/constants';
+import { getBaseSubscriptionsData } from '../constants';
 import useSubscriptions from './useSubscriptions';
 import { useBrowseAndRequestConfiguration } from './useBrowseAndRequest';
 import useCouponCodes from './useCouponCodes';
@@ -17,19 +17,7 @@ jest.mock('../services', () => ({
   fetchSubscriptions: jest.fn().mockResolvedValue(null),
   fetchBrowseAndRequestConfiguration: jest.fn().mockResolvedValue(null),
 }));
-const licensesByStatus = {
-  [LICENSE_STATUS.ACTIVATED]: [],
-  [LICENSE_STATUS.ASSIGNED]: [],
-  [LICENSE_STATUS.REVOKED]: [],
-};
-const mockSubscriptionsData = {
-  subscriptionLicenses: [],
-  customerAgreement: null,
-  subscriptionLicense: null,
-  subscriptionPlan: null,
-  licensesByStatus,
-  showExpirationNotifications: false,
-};
+const { baseSubscriptionsData } = getBaseSubscriptionsData();
 const mockCouponsOverviewResponse = [{ id: 123 }];
 const mockBrowseAndRequestConfiguration = {
   id: 123,
@@ -44,7 +32,7 @@ describe('useCatalogsForSubsidyRequests', () => {
   beforeEach(() => {
     jest.clearAllMocks();
     useBrowseAndRequestConfiguration.mockReturnValue({ data: mockBrowseAndRequestConfiguration });
-    useSubscriptions.mockReturnValue({ data: mockSubscriptionsData });
+    useSubscriptions.mockReturnValue({ data: baseSubscriptionsData });
     useCouponCodes.mockReturnValue({ data: { couponsOverview: mockCouponsOverviewResponse } });
   });
   it('should handle return when subsidy request not enabled for browseAndRequestConfiguration', () => {
@@ -56,7 +44,7 @@ describe('useCatalogsForSubsidyRequests', () => {
       availableSubscriptionCatalogs: ['test-catalog1', 'test-catalog2'],
     };
     const mockUpdatedSubscriptionsData = {
-      ...mockSubscriptionsData,
+      ...baseSubscriptionsData,
       customerAgreement,
     };
     const mockUpdatedBrowseAndRequestConfiguration = {
@@ -118,7 +106,7 @@ describe('useCatalogsForSubsidyRequests', () => {
       enterpriseCatalogUuid: 'test-catalog6',
     }];
     useBrowseAndRequestConfiguration.mockReturnValue({ data: mockUpdatedBrowseAndRequestConfiguration });
-    useSubscriptions.mockReturnValue({ data: mockSubscriptionsData });
+    useSubscriptions.mockReturnValue({ data: baseSubscriptionsData });
     useCouponCodes.mockReturnValue({ data: { couponsOverview: mockUpdatedCouponsOverviewResponse } });
     const { result } = renderHook(() => useCatalogsForSubsidyRequest(), { wrapper: Wrapper });
 

diff --git a/src/components/app/data/hooks/useSubscriptions.js b/src/components/app/data/hooks/useSubscriptions.js
@@ -1,7 +1,6 @@
 import { querySubscriptions } from '../queries';
 import useEnterpriseCustomer from './useEnterpriseCustomer';
 import useBFF from './useBFF';
-import { transformSubscriptionsData } from '../services';
 
 /**
  * Custom hook to get subscriptions data for the enterprise.
@@ -16,10 +15,7 @@ export default function useSubscriptions(queryOptions = {}) {
     bffQueryOptions: {
       ...queryOptionsRest,
       select: (data) => {
-        const transformedData = transformSubscriptionsData(
-          data?.enterpriseCustomerUserSubsidies?.subscriptions,
-          { isBFFData: true },
-        );
+        const transformedData = data?.enterpriseCustomerUserSubsidies?.subscriptions;
 
         // When custom `select` function is provided in `queryOptions`, call it with original and transformed data.
         if (select) {

diff --git a/src/components/app/data/hooks/useSubscriptions.test.jsx b/src/components/app/data/hooks/useSubscriptions.test.jsx
@@ -9,6 +9,7 @@ import useSubscriptions from './useSubscriptions';
 import { LICENSE_STATUS } from '../../../enterprise-user-subsidy/data/constants';
 import { queryEnterpriseLearnerDashboardBFF, resolveBFFQuery } from '../queries';
 import useEnterpriseFeatures from './useEnterpriseFeatures';
+import { getBaseSubscriptionsData } from '../constants';
 
 jest.mock('./useEnterpriseCustomer');
 jest.mock('./useEnterpriseFeatures');
@@ -28,19 +29,7 @@ jest.mock('react-router-dom', () => ({
 }));
 
 const mockEnterpriseCustomer = enterpriseCustomerFactory();
-const licensesByStatus = {
-  [LICENSE_STATUS.ACTIVATED]: [],
-  [LICENSE_STATUS.ASSIGNED]: [],
-  [LICENSE_STATUS.REVOKED]: [],
-};
-const mockSubscriptionsData = {
-  subscriptionLicenses: [],
-  customerAgreement: null,
-  subscriptionLicense: null,
-  subscriptionPlan: null,
-  licensesByStatus,
-  showExpirationNotifications: false,
-};
+const { baseSubscriptionsData, baseLicensesByStatus } = getBaseSubscriptionsData();
 
 describe('useSubscriptions', () => {
   const Wrapper = ({ children }) => (
@@ -53,7 +42,7 @@ describe('useSubscriptions', () => {
     jest.clearAllMocks();
     useEnterpriseCustomer.mockReturnValue({ data: mockEnterpriseCustomer });
     useEnterpriseFeatures.mockReturnValue({ data: undefined });
-    fetchSubscriptions.mockResolvedValue(mockSubscriptionsData);
+    fetchSubscriptions.mockResolvedValue(baseSubscriptionsData);
     useLocation.mockReturnValue({ pathname: '/test-enterprise' });
     useParams.mockReturnValue({ enterpriseSlug: 'test-enterprise' });
     resolveBFFQuery.mockReturnValue(null);
@@ -90,24 +79,23 @@ describe('useSubscriptions', () => {
     }
     const queryOptions = hasQueryOptions ? { select: mockSelect } : undefined;
     const mockSubscriptionLicensesByStatus = {
-      ...mockSubscriptionsData.licensesByStatus,
+      ...baseLicensesByStatus,
       [mockSubscriptionLicense.status]: [mockSubscriptionLicense],
     };
     const mockSubscriptionsDataWithLicense = {
-      ...mockSubscriptionsData,
+      ...baseSubscriptionsData,
       subscriptionLicenses: [mockSubscriptionLicense],
       customerAgreement: {
         uuid: 'mock-customer-agreement-uuid',
         disableExpirationNotifications: true,
       },
       subscriptionLicense: mockSubscriptionLicense,
       subscriptionPlan: mockSubscriptionLicense.subscriptionPlan,
-      licensesByStatus: mockSubscriptionLicensesByStatus,
+      subscriptionLicensesByStatus: mockSubscriptionLicensesByStatus,
       showExpirationNotifications: false,
     };
     if (isBFFQueryEnabled) {
       mockSubscriptionsDataWithLicense.subscriptionLicensesByStatus = mockSubscriptionLicensesByStatus;
-      delete mockSubscriptionsDataWithLicense.licensesByStatus;
       resolveBFFQuery.mockReturnValue(queryEnterpriseLearnerDashboardBFF);
       fetchEnterpriseLearnerDashboard.mockResolvedValue({
         enterpriseCustomerUserSubsidies: {
@@ -131,9 +119,8 @@ describe('useSubscriptions', () => {
 
     const expectedSubscriptionsdata = {
       ...mockSubscriptionsDataWithLicense,
-      licensesByStatus: mockSubscriptionLicensesByStatus,
+      subscriptionLicensesByStatus: mockSubscriptionLicensesByStatus,
     };
-    delete expectedSubscriptionsdata.subscriptionLicensesByStatus;
 
     if (hasQueryOptions && isBFFQueryEnabled) {
       expect(mockSelect).toHaveBeenCalledWith({

diff --git a/src/components/app/data/services/subsidies/subscriptions.js b/src/components/app/data/services/subsidies/subscriptions.js
@@ -8,6 +8,7 @@ import { features } from '../../../../../config';
 import { LICENSE_STATUS } from '../../../../enterprise-user-subsidy/data/constants';
 import { fetchPaginatedData } from '../utils';
 import { getBaseSubscriptionsData, SESSION_STORAGE_KEY_LICENSE_ACTIVATION_MESSAGE } from '../../constants';
+import { addLicenseToSubscriptionLicensesByStatus } from '../../utils';
 
 // Subscriptions
 
@@ -134,7 +135,7 @@ export async function activateOrAutoApplySubscriptionLicense({
 
   const {
     customerAgreement,
-    licensesByStatus,
+    subscriptionLicensesByStatus,
   } = subscriptionsData;
   // If there is no available customer agreement for the current customer,
   // or if there is no *current* plan available within such a customer agreement,
@@ -151,9 +152,9 @@ export async function activateOrAutoApplySubscriptionLicense({
     isCurrentSubscriptionLicenseFilter,
   ).length > 0;
 
-  const hasActivatedSubscriptionLicense = filterLicenseStatus(licensesByStatus[LICENSE_STATUS.ACTIVATED]);
-  const hasRevokedSubscriptionLicense = filterLicenseStatus(licensesByStatus[LICENSE_STATUS.REVOKED]);
-  const subscriptionLicenseToActivate = licensesByStatus[LICENSE_STATUS.ASSIGNED].filter(
+  const hasActivatedSubscriptionLicense = filterLicenseStatus(subscriptionLicensesByStatus[LICENSE_STATUS.ACTIVATED]);
+  const hasRevokedSubscriptionLicense = filterLicenseStatus(subscriptionLicensesByStatus[LICENSE_STATUS.REVOKED]);
+  const subscriptionLicenseToActivate = subscriptionLicensesByStatus[LICENSE_STATUS.ASSIGNED].filter(
     isCurrentSubscriptionLicenseFilter,
   )[0];
 
@@ -185,35 +186,21 @@ export async function activateOrAutoApplySubscriptionLicense({
   return activatedOrAutoAppliedLicense;
 }
 
-export function transformSubscriptionsData(subscriptions, options = {}) {
-  const { isBFFData } = options;
-  const { baseSubscriptionsData, baseLicensesByStatus } = getBaseSubscriptionsData();
-
-  const {
-    customerAgreement,
-    subscriptionLicenses,
-    subscriptionLicensesByStatus,
-  } = subscriptions;
-
-  const licensesByStatus = isBFFData && subscriptionLicensesByStatus
-    ? subscriptionLicensesByStatus
-    : baseLicensesByStatus;
-
-  const subscriptionsData = {
-    ...baseSubscriptionsData,
-    licensesByStatus,
-  };
+export function transformSubscriptionsData({ customerAgreement, subscriptionLicenses }) {
+  const { baseSubscriptionsData } = getBaseSubscriptionsData();
+  const subscriptionsData = { ...baseSubscriptionsData };
 
   if (subscriptionLicenses) {
     subscriptionsData.subscriptionLicenses = subscriptionLicenses;
   }
-
   if (customerAgreement) {
     subscriptionsData.customerAgreement = customerAgreement;
   }
+
   subscriptionsData.showExpirationNotifications = !(
     customerAgreement?.disableExpirationNotifications || customerAgreement?.hasCustomLicenseExpirationMessagingV2
   );
+
   // Sort licenses within each license status by whether the associated subscription plans
   // are current; current plans should be prioritized over non-current plans.
   subscriptionsData.subscriptionLicenses = [...subscriptionLicenses].sort((a, b) => {
@@ -226,30 +213,30 @@ export function transformSubscriptionsData(subscriptions, options = {}) {
   });
 
   // Group licenses by status.
-  if (!isBFFData) {
-    subscriptionsData.subscriptionLicenses.forEach((license) => {
-      const { subscriptionPlan, status } = license;
-      const isUnassignedLicense = status === LICENSE_STATUS.UNASSIGNED;
-      if (isUnassignedLicense || !subscriptionPlan.isActive) {
-        return;
-      }
-      subscriptionsData.licensesByStatus[license.status].push(license);
+  subscriptionsData.subscriptionLicenses.forEach((license) => {
+    if (license.status === LICENSE_STATUS.UNASSIGNED) {
+      return;
+    }
+    const updatedLicensesByStatus = addLicenseToSubscriptionLicensesByStatus({
+      subscriptionLicensesByStatus: subscriptionsData.subscriptionLicensesByStatus,
+      subscriptionLicense: license,
     });
-  }
+    subscriptionsData.subscriptionLicensesByStatus = updatedLicensesByStatus;
+  });
 
   // Extracts a single subscription license for the user, from the ordered licenses by status.
-  const applicableSubscriptionLicense = Object.values(
-    subscriptionsData.licensesByStatus,
-  ).flat()[0];
+  const applicableSubscriptionLicense = Object.values(subscriptionsData.subscriptionLicensesByStatus).flat()[0];
   if (applicableSubscriptionLicense) {
     subscriptionsData.subscriptionLicense = applicableSubscriptionLicense;
     subscriptionsData.subscriptionPlan = applicableSubscriptionLicense.subscriptionPlan;
   }
+
+  // Return the transformed subscriptions data.
   return subscriptionsData;
 }
 
 /**
- * TODO
+ * Fetches subscriptions data for the enterprise customer
  * @returns
  * @param enterpriseUUID
  */
@@ -266,24 +253,21 @@ export async function fetchSubscriptions(enterpriseUUID) {
    * applicable license for use by the rest of the application.
    *
    * Example: an activated license will be chosen as the applicable license because activated licenses
-   * come first in ``licensesByStatus`` even if the user also has a revoked license.
+   * come first in ``subscriptionLicensesByStatus`` even if the user also has a revoked license.
    */
-  const { baseSubscriptionsData } = getBaseSubscriptionsData();
   try {
     const {
       results: subscriptionLicenses,
       response,
     } = await fetchPaginatedData(url);
     const { customerAgreement } = response;
-    const subscriptionsData = {
+    return transformSubscriptionsData({
       customerAgreement,
       subscriptionLicenses,
-    };
-    return transformSubscriptionsData(
-      subscriptionsData,
-    );
+    });
   } catch (error) {
     logError(error);
+    const { baseSubscriptionsData } = getBaseSubscriptionsData();
     return baseSubscriptionsData;
   }
 }