feat: take into account enrollments in dashboard -> search route redi…

…rects for new users (#1208)
openedx · Oct 17, 2024 · b3c1e56 · b3c1e56
1 parent 5b1583b
commit b3c1e56
Show file tree

Hide file tree

Showing 5 changed files with 171 additions and 107 deletions.
diff --git a/src/components/app/routes/data/utils.js b/src/components/app/routes/data/utils.js
@@ -130,26 +130,7 @@ export async function ensureEnterpriseAppData({
       queryClient.ensureQueryData(queryNotices()),
     );
   }
-  const enterpriseAppData = await Promise.all(enterpriseAppDataQueries);
-  return enterpriseAppData;
-}
-
-/**
- * Determines whether the user is visiting the dashboard for the first time.
- * @param {URL} requestUrl - The request URL.
- * @returns {boolean} - Whether the user is visiting the dashboard for the first time.
- */
-function isFirstDashboardPageVisit(requestUrl) {
-  // Check whether the request URL matches the dashboard page route. If not, return early.
-  const isDashboardRoute = matchPath('/:enterpriseSlug', requestUrl.pathname);
-  if (!isDashboardRoute) {
-    return false;
-  }
-
-  // User is visiting the dashboard for the first time if the 'has-user-visited-learner-dashboard'
-  // localStorage item is not set.
-  const hasUserVisitedDashboard = localStorage.getItem('has-user-visited-learner-dashboard');
-  return !hasUserVisitedDashboard;
+  await Promise.all(enterpriseAppDataQueries);
 }
 
 /**
@@ -161,22 +142,37 @@ function isFirstDashboardPageVisit(requestUrl) {
  *
  * @param {Object} params - The parameters object.
  * @param {string} params.enterpriseSlug - The enterprise slug.
- * @param {Object} params.enterpriseAppData - The enterprise app data.
- * @param {URL} params.requestUrl - The request URL.
+ * @param {Array} params.enterpriseCourseEnrollments - The enterprise course enrollments.
+ * @param {Object} params.redeemablePolicies - The redeemable policies.
  */
-export function redirectToSearchPageForNewUser({ enterpriseSlug, enterpriseAppData, requestUrl }) {
-  const isFirstDashboardVisit = isFirstDashboardPageVisit(requestUrl);
-  if (!isFirstDashboardVisit) {
+export function redirectToSearchPageForNewUser({
+  enterpriseSlug,
+  enterpriseCourseEnrollments,
+  redeemablePolicies,
+}) {
+  // If the user has already visited the dashboard, return early.
+  if (localStorage.getItem('has-user-visited-learner-dashboard')) {
     return;
   }
 
-  // Set the localStorage item to indicate that the user has visited the learner dashboard.
+  // Otherwise, set the localStorage item to indicate that the user has visited the dashboard.
   localStorage.setItem('has-user-visited-learner-dashboard', true);
 
-  // Check whether the user has any assignments for display. If not, redirect to the search page.
-  const redeemablePolicies = enterpriseAppData[1];
+  // If the current URL does not match the dashboard, return early. This covers the use
+  // case where user may be on a non-dashboard route (e.g., search) and then explicitly
+  // navigates to the dashboard route. If the user is not already on the dashboard route,
+  // we do not want to trigger a redirect to the search page as the user explicitly requested
+  // to view the dashboard.
+  const isCurrentUrlMatchingDashboard = matchPath('/:enterpriseSlug', global.location.pathname);
+  if (!isCurrentUrlMatchingDashboard) {
+    return;
+  }
+
+  // Check whether user has any assignments for display or active enterprise course
+  // enrollments. If not, redirect to the search page.
   const { hasAssignmentsForDisplay } = redeemablePolicies.learnerContentAssignments;
-  if (!hasAssignmentsForDisplay) {
+  const hasEnterpriseCourseEnrollments = enterpriseCourseEnrollments.length > 0;
+  if (!(hasAssignmentsForDisplay || hasEnterpriseCourseEnrollments)) {
     throw redirect(generatePath('/:enterpriseSlug/search', { enterpriseSlug }));
   }
 }

diff --git a/src/components/app/routes/loaders/rootLoader.ts b/src/components/app/routes/loaders/rootLoader.ts
@@ -3,7 +3,6 @@ import {
   ensureAuthenticatedUser,
   ensureEnterpriseAppData,
   redirectToRemoveTrailingSlash,
-  redirectToSearchPageForNewUser,
   ensureActiveEnterpriseCustomerUser,
 } from '../data';
 
@@ -62,7 +61,7 @@ const makeRootLoader: Types.MakeRouteLoaderFunctionWithQueryClient = function ma
     }
 
     // Fetch all enterprise app data.
-    const enterpriseAppData = await ensureEnterpriseAppData({
+    await ensureEnterpriseAppData({
       enterpriseCustomer,
       allLinkedEnterpriseCustomerUsers,
       userId,
@@ -71,13 +70,6 @@ const makeRootLoader: Types.MakeRouteLoaderFunctionWithQueryClient = function ma
       requestUrl,
     });
 
-    // Redirect user to search page, for first-time users with no assignments.
-    redirectToSearchPageForNewUser({
-      enterpriseSlug: enterpriseSlug as string,
-      enterpriseAppData,
-      requestUrl,
-    });
-
     // Redirect to the same URL without a trailing slash, if applicable.
     redirectToRemoveTrailingSlash(requestUrl);
 

diff --git a/src/components/app/routes/loaders/tests/rootLoader.test.jsx b/src/components/app/routes/loaders/tests/rootLoader.test.jsx
@@ -1,7 +1,5 @@
-import { useEffect } from 'react';
 import { screen, waitFor } from '@testing-library/react';
 import { when } from 'jest-when';
-import { Outlet, useLocation } from 'react-router-dom';
 import '@testing-library/jest-dom/extend-expect';
 
 import { renderWithRouterProvider } from '../../../../../utils/tests';
@@ -40,19 +38,9 @@ const mockQueryClient = {
   ensureQueryData: jest.fn().mockResolvedValue(),
 };
 
-let locationPathname;
-const ComponentWithLocation = ({ children }) => {
-  const { pathname } = useLocation();
-  useEffect(() => {
-    locationPathname = pathname;
-  }, [pathname]);
-  return children || null;
-};
-
 describe('rootLoader', () => {
   beforeEach(() => {
     jest.clearAllMocks();
-    localStorage.clear();
     ensureAuthenticatedUser.mockResolvedValue(mockAuthenticatedUser);
     extractEnterpriseCustomer.mockResolvedValue(mockEnterpriseCustomer);
   });
@@ -108,7 +96,6 @@ describe('rootLoader', () => {
         { enterpriseCustomer: mockEnterpriseCustomerTwo },
       ],
       isStaffUser: false,
-      shouldRedirectToSearch: false,
     },
     {
       enterpriseSlug: mockEnterpriseCustomerTwo.slug,
@@ -119,7 +106,6 @@ describe('rootLoader', () => {
         { enterpriseCustomer: mockEnterpriseCustomerTwo },
       ],
       isStaffUser: false,
-      shouldRedirectToSearch: true,
     },
     {
       enterpriseSlug: mockEnterpriseCustomerTwo.slug,
@@ -129,7 +115,6 @@ describe('rootLoader', () => {
         { enterpriseCustomer: mockEnterpriseCustomer },
       ],
       isStaffUser: false,
-      shouldRedirectToSearch: false,
     },
     {
       enterpriseSlug: mockEnterpriseCustomerTwo.slug,
@@ -139,7 +124,6 @@ describe('rootLoader', () => {
         { enterpriseCustomer: mockEnterpriseCustomer },
       ],
       isStaffUser: false,
-      shouldRedirectToSearch: true,
     },
     {
       enterpriseSlug: mockEnterpriseCustomerTwo.slug,
@@ -149,7 +133,6 @@ describe('rootLoader', () => {
         { enterpriseCustomer: mockEnterpriseCustomer },
       ],
       isStaffUser: true,
-      shouldRedirectToSearch: false,
     },
     {
       enterpriseSlug: mockEnterpriseCustomerTwo.slug,
@@ -159,15 +142,13 @@ describe('rootLoader', () => {
         { enterpriseCustomer: mockEnterpriseCustomer },
       ],
       isStaffUser: true,
-      shouldRedirectToSearch: true,
     },
   ])('ensures all requisite root loader queries are resolved with an active enterprise customer user (%s)', async ({
     isStaffUser,
     enterpriseSlug,
     enterpriseCustomer,
     activeEnterpriseCustomer,
     allLinkedEnterpriseCustomerUsers,
-    shouldRedirectToSearch,
   }) => {
     const enterpriseLearnerQuery = queryEnterpriseLearner(mockAuthenticatedUser.username, enterpriseSlug);
     const enterpriseLearnerQueryTwo = queryEnterpriseLearner(mockAuthenticatedUser.username, enterpriseCustomer.slug);
@@ -197,13 +178,10 @@ describe('rootLoader', () => {
     const mockRedeemablePolicies = {
       redeemablePolicies: [],
       learnerContentAssignments: {
-        hasAssignmentsForDisplay: !shouldRedirectToSearch,
+        hasAssignmentsForDisplay: false,
       },
     };
-    const redeemablePoliciesQuery = queryRedeemablePolicies({
-      enterpriseUuid: enterpriseCustomer.uuid,
-      lmsUserId: 3,
-    });
+    const redeemablePoliciesQuery = queryRedeemablePolicies({ enterpriseUuid: enterpriseCustomer.uuid, lmsUserId: 3 });
     when(mockQueryClient.ensureQueryData).calledWith(
       expect.objectContaining({
         queryKey: redeemablePoliciesQuery.queryKey,
@@ -223,15 +201,9 @@ describe('rootLoader', () => {
     ).mockResolvedValue(mockSubscriptionsData);
 
     renderWithRouterProvider({
-      path: '/:enterpriseSlug/*',
-      element: <ComponentWithLocation><Outlet /></ComponentWithLocation>,
+      path: '/:enterpriseSlug',
+      element: <div data-testid="dashboard" />,
       loader: makeRootLoader(mockQueryClient),
-      children: [
-        {
-          path: 'search',
-          element: <ComponentWithLocation />,
-        },
-      ],
     }, {
       initialEntries: [`/${enterpriseSlug}`],
     });
@@ -246,31 +218,11 @@ describe('rootLoader', () => {
         } else {
           expect(mockQueryClient.ensureQueryData).toHaveBeenCalledTimes(2);
         }
-      } else if (shouldRedirectToSearch) {
-        // queries are executed again when redirecting to search
-        expect(mockQueryClient.ensureQueryData).toHaveBeenCalledTimes(18);
       } else {
         expect(mockQueryClient.ensureQueryData).toHaveBeenCalledTimes(9);
       }
     });
 
-    function getExpectedSlugPath() {
-      if (enterpriseSlug === activeEnterpriseCustomer?.slug) {
-        return enterpriseSlug;
-      }
-      if (isLinked || isStaffUser) {
-        return enterpriseCustomer.slug;
-      }
-      return activeEnterpriseCustomer.slug;
-    }
-    const expectedCustomerPath = getExpectedSlugPath();
-    // Assert that the expected number of queries were made.
-    if (shouldRedirectToSearch) {
-      expect(locationPathname).toEqual(`/${expectedCustomerPath}/search`);
-    } else {
-      expect(locationPathname).toEqual(`/${expectedCustomerPath}`);
-    }
-
     // Enterprise learner query
     expect(mockQueryClient.ensureQueryData).toHaveBeenCalledWith(
       expect.objectContaining({