chore: PR feedback

openedx · Feb 25, 2025 · e51d9f2 · e51d9f2
1 parent 9f7bae7
commit e51d9f2
Show file tree

Hide file tree

Showing 14 changed files with 131 additions and 74 deletions.
diff --git a/src/components/app/data/hooks/useBFF.test.jsx b/src/components/app/data/hooks/useBFF.test.jsx
@@ -136,32 +136,32 @@ describe('useBFF', () => {
   it.each([
     // BFF disabled route (without query options)
     {
-      isMatchedRoute: false,
+      isMatchedBFFRoute: false,
       hasQueryOptions: false,
     },
     // BFF enabled route (without query options)
     {
-      isMatchedRoute: true,
+      isMatchedBFFRoute: true,
       hasQueryOptions: false,
     },
     // BFF enabled route (with query options)
     {
-      isMatchedRoute: true,
+      isMatchedBFFRoute: true,
       hasQueryOptions: true,
     },
 
     // BFF disabled route(with query options)
     {
-      isMatchedRoute: false,
+      isMatchedBFFRoute: false,
       hasQueryOptions: true,
     },
   ])('should handle resolved value correctly for based on route (%s)', async ({
-    isMatchedRoute,
+    isMatchedBFFRoute,
     hasQueryOptions,
   }) => {
     const mockFallbackData = { fallback: 'data' };
     const mockSelect = jest.fn(() => {
-      if (isMatchedRoute) {
+      if (isMatchedBFFRoute) {
         return mockBFFDashboardData;
       }
       return mockFallbackData;
@@ -176,7 +176,7 @@ describe('useBFF', () => {
       mockBFFQueryOptions.select = mockSelect;
       mockFallbackQueryConfig.select = mockSelect;
     }
-    const initialEntries = isMatchedRoute ? ['/test-enterprise'] : ['/test-enterprise/search'];
+    const initialEntries = isMatchedBFFRoute ? ['/test-enterprise'] : ['/test-enterprise/search'];
     const { result, waitForNextUpdate } = renderHook(
       () => useBFF({
         bffQueryOptions: {
@@ -194,7 +194,7 @@ describe('useBFF', () => {
     );
     await waitForNextUpdate();
 
-    const expectedData = isMatchedRoute ? mockBFFDashboardData : mockFallbackData;
+    const expectedData = isMatchedBFFRoute ? mockBFFDashboardData : mockFallbackData;
     expect(result.current).toEqual(
       expect.objectContaining({
         data: expectedData,
@@ -205,7 +205,7 @@ describe('useBFF', () => {
 
     if (hasQueryOptions) {
       expect(mockSelect).toHaveBeenCalledTimes(1);
-      if (isMatchedRoute) {
+      if (isMatchedBFFRoute) {
         // Expects the select function to be called with the resolved BFF data
         expect(mockSelect).toHaveBeenCalledWith(mockBFFDashboardData);
       } else {
@@ -214,7 +214,7 @@ describe('useBFF', () => {
       }
     }
 
-    if (isMatchedRoute) {
+    if (isMatchedBFFRoute) {
       expect(fetchEnterpriseLearnerDashboard).toHaveBeenCalledTimes(1);
       expect(fetchEnterpriseLearnerDashboard).toHaveBeenCalledWith(
         expect.objectContaining({

diff --git a/src/components/app/data/hooks/useEnterpriseCustomer.js b/src/components/app/data/hooks/useEnterpriseCustomer.js
@@ -13,7 +13,7 @@ export default function useEnterpriseCustomer(queryOptions = {}) {
       const transformedData = data.transformed.enterpriseCustomer;
       if (select) {
         return select({
-          original: data,
+          original: data.original,
           transformed: transformedData,
         });
       }

diff --git a/src/components/app/data/hooks/useEnterpriseCustomer.test.jsx b/src/components/app/data/hooks/useEnterpriseCustomer.test.jsx
@@ -53,7 +53,7 @@ const mockBFFDashboardData = {
   warnings: [],
 };
 
-const mockExpectedEnterpriseCustomers = (isMatchedRoute) => (isMatchedRoute
+const mockExpectedEnterpriseCustomers = (isMatchedBFFRoute) => (isMatchedBFFRoute
   ? mockBFFDashboardData.enterpriseCustomer
   : mockEnterpriseLearnerData.enterpriseCustomer);
 
@@ -77,14 +77,14 @@ describe('useEnterpriseCustomer', () => {
     fetchEnterpriseLearnerDashboard.mockResolvedValue(mockBFFDashboardData);
   });
   it.each([
-    { isMatchedRoute: false },
-    { isMatchedRoute: true },
-  ])('should return enterprise customers correctly (%s)', async ({ isMatchedRoute }) => {
+    { isMatchedBFFRoute: false },
+    { isMatchedBFFRoute: true },
+  ])('should return enterprise customers correctly (%s)', async ({ isMatchedBFFRoute }) => {
     const mockSelect = jest.fn(data => data.transformed);
-    const initialEntries = isMatchedRoute ? ['/test-enterprise'] : ['/test-enterprise/search'];
+    const initialEntries = isMatchedBFFRoute ? ['/test-enterprise'] : ['/test-enterprise/search'];
     const { result, waitForNextUpdate } = renderHook(
       () => {
-        if (isMatchedRoute) {
+        if (isMatchedBFFRoute) {
           return useEnterpriseCustomer({ select: mockSelect });
         }
         return useEnterpriseCustomer();
@@ -98,13 +98,13 @@ describe('useEnterpriseCustomer', () => {
       },
     );
     await waitForNextUpdate();
-    if (isMatchedRoute) {
+    if (isMatchedBFFRoute) {
       expect(mockSelect).toHaveBeenCalledTimes(4);
     } else {
       expect(mockSelect).toHaveBeenCalledTimes(0);
     }
 
     const actualEnterpriseFeatures = result.current.data;
-    expect(actualEnterpriseFeatures).toEqual(mockExpectedEnterpriseCustomers(isMatchedRoute));
+    expect(actualEnterpriseFeatures).toEqual(mockExpectedEnterpriseCustomers(isMatchedBFFRoute));
   });
 });
diff --git a/src/components/app/data/hooks/useEnterpriseFeatures.js b/src/components/app/data/hooks/useEnterpriseFeatures.js
@@ -8,7 +8,7 @@ export default function useEnterpriseFeatures(queryOptions = {}) {
       const transformedData = data.transformed.enterpriseFeatures;
       if (select) {
         return select({
-          original: data,
+          original: data.original,
           transformed: transformedData,
         });
       }

diff --git a/src/components/app/data/hooks/useEnterpriseFeatures.test.jsx b/src/components/app/data/hooks/useEnterpriseFeatures.test.jsx
@@ -51,7 +51,7 @@ const mockBFFDashboardData = {
   warnings: [],
 };
 
-const mockExpectedEnterpriseFeatures = (isMatchedRoute) => (isMatchedRoute
+const mockExpectedEnterpriseFeatures = (isMatchedBFFRoute) => (isMatchedBFFRoute
   ? mockBFFDashboardData.enterpriseFeatures
   : mockEnterpriseLearnerData.enterpriseFeatures);
 describe('useEnterpriseFeatures', () => {
@@ -76,14 +76,14 @@ describe('useEnterpriseFeatures', () => {
   });
 
   it.each([
-    { isMatchedRoute: false },
-    { isMatchedRoute: true },
-  ])('should return enterprise features correctly (%s)', async ({ isMatchedRoute }) => {
+    { isMatchedBFFRoute: false },
+    { isMatchedBFFRoute: true },
+  ])('should return enterprise features correctly (%s)', async ({ isMatchedBFFRoute }) => {
     const mockSelect = jest.fn(data => data.transformed);
-    const initialEntries = isMatchedRoute ? ['/test-enterprise'] : ['/test-enterprise/search'];
+    const initialEntries = isMatchedBFFRoute ? ['/test-enterprise'] : ['/test-enterprise/search'];
     const { result, waitForNextUpdate } = renderHook(
       () => {
-        if (isMatchedRoute) {
+        if (isMatchedBFFRoute) {
           return useEnterpriseFeatures({ select: mockSelect });
         }
         return useEnterpriseFeatures();
@@ -97,13 +97,13 @@ describe('useEnterpriseFeatures', () => {
       },
     );
     await waitForNextUpdate();
-    if (isMatchedRoute) {
+    if (isMatchedBFFRoute) {
       expect(mockSelect).toHaveBeenCalledTimes(4);
     } else {
       expect(mockSelect).toHaveBeenCalledTimes(0);
     }
 
     const actualEnterpriseFeatures = result.current.data;
-    expect(actualEnterpriseFeatures).toEqual(mockExpectedEnterpriseFeatures(isMatchedRoute));
+    expect(actualEnterpriseFeatures).toEqual(mockExpectedEnterpriseFeatures(isMatchedBFFRoute));
   });
 });
diff --git a/src/components/app/data/hooks/useEnterpriseLearner.js b/src/components/app/data/hooks/useEnterpriseLearner.js
@@ -39,6 +39,9 @@ export default function useEnterpriseLearner(queryOptions = {}) {
       ...queryEnterpriseLearner(authenticatedUser.username, enterpriseSlug),
       ...queryOptions,
       select: (data) => {
+        // To maintain parity with BFF-enabled routes in the function signature passed to the custom `select`
+        // function, the legacy `queryEnterpriseLearner` also passes its `data` to the custom `select` function
+        // as both the `original` and `transformed` properties, since no data transformations occur here.
         if (select) {
           return select({
             original: data,

diff --git a/src/components/app/data/hooks/useEnterpriseLearner.test.jsx b/src/components/app/data/hooks/useEnterpriseLearner.test.jsx
@@ -55,7 +55,7 @@ const mockBFFDashboardData = {
   warnings: [],
 };
 
-const mockExpectedEnterpriseLearner = (isMatchedRoute) => (isMatchedRoute
+const mockExpectedEnterpriseLearner = (isMatchedBFFRoute) => (isMatchedBFFRoute
   ? {
     enterpriseCustomer: mockBFFDashboardData.enterpriseCustomer,
     allLinkedEnterpriseCustomerUsers: mockBFFDashboardData.allLinkedEnterpriseCustomerUsers,
@@ -80,32 +80,32 @@ describe('useEnterpriseLearner', () => {
     useParams.mockReturnValue({ enterpriseSlug: mockEnterpriseCustomer.slug });
   });
   it.each([
-    { isMatchedRoute: false },
-    { isMatchedRoute: true },
-  ])('should return enterprise learners correctly (%s)', async ({ isMatchedRoute }) => {
+    { isMatchedBFFRoute: false },
+    { isMatchedBFFRoute: true },
+  ])('should return enterprise learners correctly (%s)', async ({ isMatchedBFFRoute }) => {
     const mockSelect = jest.fn(data => data.transformed);
     const { result, waitForNextUpdate } = renderHook(
       () => {
-        if (isMatchedRoute) {
+        if (isMatchedBFFRoute) {
           return useEnterpriseLearner({ select: mockSelect });
         }
         return useEnterpriseLearner();
       },
       {
         wrapper: ({ children }) => Wrapper({
-          routes: isMatchedRoute ? '/test-enterprise' : 'test-enterprise/search',
+          routes: isMatchedBFFRoute ? '/test-enterprise' : 'test-enterprise/search',
           children,
         }),
       },
     );
     await waitForNextUpdate();
-    if (isMatchedRoute) {
+    if (isMatchedBFFRoute) {
       expect(mockSelect).toHaveBeenCalledTimes(2);
     } else {
       expect(mockSelect).toHaveBeenCalledTimes(0);
     }
 
     const actualEnterpriseFeatures = result.current.data;
-    expect(actualEnterpriseFeatures).toEqual(mockExpectedEnterpriseLearner(isMatchedRoute));
+    expect(actualEnterpriseFeatures).toEqual(mockExpectedEnterpriseLearner(isMatchedBFFRoute));
   });
 });
diff --git a/src/components/app/data/hooks/useIsBFFEnabled.test.jsx b/src/components/app/data/hooks/useIsBFFEnabled.test.jsx
@@ -1,6 +1,6 @@
 import { renderHook } from '@testing-library/react-hooks';
 import { QueryClientProvider } from '@tanstack/react-query';
-import { MemoryRouter, useLocation } from 'react-router-dom';
+import { MemoryRouter } from 'react-router-dom';
 import { AppContext } from '@edx/frontend-platform/react';
 import useIsBFFEnabled from './useIsBFFEnabled';
 import { queryEnterpriseLearnerDashboardBFF, resolveBFFQuery } from '../queries';
@@ -11,10 +11,7 @@ jest.mock('../queries', () => ({
   ...jest.requireActual('../queries'),
   resolveBFFQuery: jest.fn(),
 }));
-jest.mock('react-router-dom', () => ({
-  ...jest.requireActual('react-router-dom'),
-  useLocation: jest.fn(),
-}));
+
 const mockAuthenticatedUser = authenticatedUserFactory();
 
 describe('useIsBFFEnabled', () => {
@@ -29,7 +26,6 @@ describe('useIsBFFEnabled', () => {
   );
   beforeEach(() => {
     jest.clearAllMocks();
-    useLocation.mockReturnValue({ pathname: '/test-enterprise' });
   });
 
   it.each([
@@ -39,7 +35,6 @@ describe('useIsBFFEnabled', () => {
     const route = hasBFFEnabled ? '/test-enterprise' : 'test-enterprise/search';
 
     resolveBFFQuery.mockReturnValue(hasBFFEnabled ? queryEnterpriseLearnerDashboardBFF : null);
-    useLocation.mockReturnValue({ pathname: route });
 
     const { result } = renderHook(() => useIsBFFEnabled(), {
       wrapper: ({ children }) => Wrapper({