Skip to content

Commit

Permalink
refactor: improve test cases
Browse files Browse the repository at this point in the history
  • Loading branch information
Syed-Ali-Abbas-Zaidi committed Oct 3, 2023
1 parent 3ad4324 commit a6cc833
Show file tree
Hide file tree
Showing 23 changed files with 106 additions and 241 deletions.
6 changes: 3 additions & 3 deletions src/components/course/data/hooks.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ export const useCourseEnrollmentUrl = ({
* @returns An object containing the Algolia objectId and queryId that led to a page view of the Course page.
*/
export const useExtractAndRemoveSearchParamsFromURL = () => {
const { search } = useLocation();
const { pathname, search } = useLocation();
const navigate = useNavigate();
const [algoliaSearchParams, setAlgoliaSearchParams] = useState({});

Expand All @@ -385,13 +385,13 @@ export const useExtractAndRemoveSearchParamsFromURL = () => {
});
queryParams.delete('queryId');
queryParams.delete('objectId');
navigate({
navigate(pathname, {
search: queryParams.toString(),
replace: true,
});
}
},
[navigate, queryParams],
[navigate, queryParams, pathname],
);

return algoliaSearchParams;
Expand Down
3 changes: 2 additions & 1 deletion src/components/course/data/tests/hooks.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,7 @@ describe('useExtractAndRemoveSearchParamsFromURL', () => {

beforeEach(() => {
useLocation.mockReturnValue({
pathname: '/',
search: '?queryId=123&objectId=abc',
});
});
Expand All @@ -869,7 +870,7 @@ describe('useExtractAndRemoveSearchParamsFromURL', () => {
expect(screen.getByText('Query ID: 123')).toBeTruthy();
expect(screen.getByText('Object ID: abc')).toBeTruthy();
expect(mockNavigate).toHaveBeenCalledTimes(1);
expect(mockNavigate).toHaveBeenCalledWith({ replace: true, search: '' });
expect(mockNavigate).toHaveBeenCalledWith('/', { replace: true, search: '' });
});
});

Expand Down
6 changes: 3 additions & 3 deletions src/components/course/tests/CoursePage.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { LEARNER_CREDIT_SUBSIDY_TYPE as mockLearnerCreditSubsidyType } from '../
import { mockCourseService } from './constants';

const mockGetActiveCourseRun = jest.fn();
const mockNavigation = jest.fn();
const mockNavigate = jest.fn();

jest.mock('../data/utils', () => ({
...jest.requireActual('../data/utils'),
Expand All @@ -27,7 +27,7 @@ jest.mock('react-router-dom', () => ({
useLocation: () => ({
pathname: '/test-enterprise-uuid/course/test-course-key',
}),
useNavigate: () => mockNavigation,
useNavigate: () => mockNavigate,
useParams: () => ({ enterpriseSlug: 'test-enterprise-uuid', courseKey: 'test-course-key' }),
}));

Expand Down Expand Up @@ -223,7 +223,7 @@ describe('CoursePage', () => {
it('Redirects to using course type slug if path does not include it', async () => {
mockGetActiveCourseRun.mockImplementation(() => ({ staff: [] }));
render(<CoursePageWrapper />);
expect(mockNavigation).toHaveBeenCalledWith('/test-enterprise-uuid/executive-education-2u/course/test-course-key', { replace: true });
expect(mockNavigate).toHaveBeenCalledWith('/test-enterprise-uuid/executive-education-2u/course/test-course-key', { replace: true });
expect(screen.getByTestId('course-enrollments-context-provider')).toBeInTheDocument();
expect(screen.getByTestId('course-context-provider')).toBeInTheDocument();
expect(screen.getByTestId('course-page-routes')).toBeInTheDocument();
Expand Down
10 changes: 7 additions & 3 deletions src/components/dashboard/DashboardPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import { IntegrationWarningModal } from '../integration-warning-modal';
import SubscriptionExpirationModal from './SubscriptionExpirationModal';

const DashboardPage = () => {
const { pathname, state } = useLocation();
const {
pathname, state, search, hash,
} = useLocation();
const navigate = useNavigate();
const { enterpriseConfig, authenticatedUser } = useContext(AppContext);
const { username } = authenticatedUser;
Expand Down Expand Up @@ -52,9 +54,11 @@ const DashboardPage = () => {
if (state?.activationSuccess) {
const updatedLocationState = { ...state };
delete updatedLocationState.activationSuccess;
navigate(pathname, { state: updatedLocationState });
navigate(pathname, {
search, hash, state: updatedLocationState, replace: true,
});
}
}, [pathname, navigate, state]);
}, [pathname, navigate, state, search, hash]);

const userFirstName = useMemo(() => authenticatedUser?.name.split(' ').shift(), [authenticatedUser]);
const PAGE_TITLE = `Dashboard - ${enterpriseConfig.name}`;
Expand Down
15 changes: 1 addition & 14 deletions src/components/dashboard/tests/CourseRecommendations.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,13 @@ import React from 'react';
import '@testing-library/jest-dom/extend-expect';
import { screen } from '@testing-library/react';
import { AppContext } from '@edx/frontend-platform/react';
import { Link } from 'react-router-dom';

import userEvent from '@testing-library/user-event';
import {
renderWithRouter,
} from '../../../utils/tests';
import { CourseRecommendations } from '../main-content';

jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
Link: jest.fn().mockImplementation(({ to, children }) => (
<a href={to}>{children}</a>
)),
}));

const mockAuthenticatedUser = { username: 'myspace-tom', name: 'John Doe' };

const defaultAppState = {
Expand Down Expand Up @@ -51,11 +43,6 @@ describe('<CourseRecommendations />', () => {
renderWithRouter(<CourseRecommendationsContext />);
const courseRecommendationsButton = screen.getByText('Recommend courses for me');
userEvent.click(courseRecommendationsButton);
expect(Link).toHaveBeenCalledWith(
expect.objectContaining({
to: '/BearsRUs/skills-quiz',
}),
expect.any(Object),
);
expect(window.location.pathname).toEqual('/BearsRUs/skills-quiz');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const EnterpriseCustomerRedirect = () => {
return <NotFoundPage />;
}

return <Navigate to={`/${enterpriseCustomer.slug}`} />;
return <Navigate to={`/${enterpriseCustomer.slug}`} replace />;
};

export default EnterpriseCustomerRedirect;
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { useContext } from 'react';
import { Navigate, useLocation } from 'react-router-dom';
import { Navigate, useParams } from 'react-router-dom';
import { AppContext } from '@edx/frontend-platform/react';

import NotFoundPage from '../NotFoundPage';
Expand All @@ -12,8 +12,7 @@ import {

const EnterprisePageRedirect = () => {
const { authenticatedUser } = useContext(AppContext);
const { pathname } = useLocation();
const redirectPath = pathname.substring(3);
const { '*': redirectPath } = useParams();
const { roles } = authenticatedUser;
const selectedEnterpriseUUID = useSelectedEnterpriseUUIDByUserRoles(roles);
const [enterpriseCustomer, isLoading] = useEnterpriseCustomerByUUID(selectedEnterpriseUUID);
Expand All @@ -31,10 +30,10 @@ const EnterprisePageRedirect = () => {
}

if (!redirectPath) {
return <Navigate to={`/${enterpriseCustomer.slug}`} />;
return <Navigate to={`/${enterpriseCustomer.slug}`} replace />;
}

return <Navigate to={`/${enterpriseCustomer.slug}/${redirectPath}`} />;
return <Navigate to={`/${enterpriseCustomer.slug}/${redirectPath}`} replace />;
};

export default EnterprisePageRedirect;
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import { AppContext } from '@edx/frontend-platform/react';
import { screen, waitFor } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';
import { mockNavigate } from 'react-router-dom';

import EnterpriseCustomerRedirect from '../EnterpriseCustomerRedirect';

Expand All @@ -12,22 +11,6 @@ import { renderWithRouter } from '../../../utils/tests';

jest.mock('../data/service');

jest.mock('react-router-dom', () => {
const mockNavigation = jest.fn();

// eslint-disable-next-line react/prop-types
const Navigate = ({ to }) => {
mockNavigation(to);
return <div />;
};

return {
...jest.requireActual('react-router-dom'),
Navigate,
mockNavigate: mockNavigation,
};
});

const TEST_ENTERPRISES = [
{
uuid: 'some-fake-uuid',
Expand Down Expand Up @@ -98,7 +81,7 @@ describe('<EnterpriseCustomerRedirect />', () => {

await waitFor(() => expect(fetchEnterpriseCustomerByUUID).toHaveBeenCalledTimes(1));

expect(mockNavigate).toHaveBeenCalledWith(`/${TEST_ENTERPRISES[0].slug}`);
expect(window.location.pathname).toEqual(`/${TEST_ENTERPRISES[0].slug}`);
});

test('redirects to selected Enterprise Customer when user is linked to more than 1 Enterprise Customer', async () => {
Expand Down Expand Up @@ -126,6 +109,6 @@ describe('<EnterpriseCustomerRedirect />', () => {

await waitFor(() => expect(fetchEnterpriseCustomerByUUID).toHaveBeenCalledTimes(1));

expect(mockNavigate).toHaveBeenCalledWith(`/${TEST_ENTERPRISES[1].slug}`);
expect(window.location.pathname).toEqual(`/${TEST_ENTERPRISES[1].slug}`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import '@testing-library/jest-dom/extend-expect';
import Cookies from 'universal-cookie';

import { mockNavigate } from 'react-router-dom';
import { renderWithRouter } from '../../../utils/tests';
import EnterpriseLearnerFirstVisitRedirect from '../EnterpriseLearnerFirstVisitRedirect';

Expand All @@ -13,22 +12,6 @@ const TEST_ENTERPRISE = {
slug: 'test-enterprise',
};

jest.mock('react-router-dom', () => {
const mockNavigation = jest.fn();

// eslint-disable-next-line react/prop-types
const Navigate = ({ to }) => {
mockNavigation(to);
return <div />;
};

return {
...jest.requireActual('react-router-dom'),
Navigate,
mockNavigate: mockNavigation,
};
});

describe('<EnterpriseLearnerFirstVisitRedirect />', () => {
beforeEach(() => {
const cookies = new Cookies();
Expand All @@ -37,8 +20,8 @@ describe('<EnterpriseLearnerFirstVisitRedirect />', () => {
});

test('redirects to search if user is visiting for the first time.', async () => {
renderWithRouter(<EnterpriseLearnerFirstVisitRedirect />, { route: `/${TEST_ENTERPRISE.slug}` });
expect(mockNavigate).toHaveBeenCalledWith('/r/search');
renderWithRouter(<EnterpriseLearnerFirstVisitRedirect />);
expect(window.location.pathname).toEqual('/r/search');
});

test('Does not redirect the returning user to search.', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import React from 'react';
import {
MemoryRouter, Route, Routes, mockNavigate,
} from 'react-router-dom';
import { useParams } from 'react-router-dom';
import { AppContext } from '@edx/frontend-platform/react';
import { render, screen, waitFor } from '@testing-library/react';
import { screen, waitFor } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';

import EnterprisePageRedirect from '../EnterprisePageRedirect';
Expand All @@ -14,21 +12,10 @@ import { renderWithRouter } from '../../../utils/tests';

jest.mock('../data/service');

jest.mock('react-router-dom', () => {
const mockNavigation = jest.fn();

// eslint-disable-next-line react/prop-types
const Navigate = ({ to }) => {
mockNavigation(to);
return <div />;
};

return {
...jest.requireActual('react-router-dom'),
Navigate,
mockNavigate: mockNavigation,
};
});
jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useParams: jest.fn(),
}));

const TEST_ENTERPRISES = [
{
Expand Down Expand Up @@ -64,10 +51,13 @@ describe('<EnterprisePageRedirect />', () => {

beforeEach(() => {
fetchEnterpriseCustomerByUUID.mockClear();
mockNavigate.mockClear();
});

test('renders NotFoundPage if user is not linked to any Enterprise Customers', async () => {
useParams.mockReturnValue({
'*': '',
});

renderWithRouter(
<EnterprisePageRedirectWithContext initialAppState={initialAppState} />,
);
Expand Down Expand Up @@ -95,18 +85,16 @@ describe('<EnterprisePageRedirect />', () => {
},
};

const Component = (
<MemoryRouter initialEntries={['/r/search']}>
<Routes>
<Route path="/r/:redirectPath" element={<EnterprisePageRedirectWithContext initialAppState={initialState} />} />
</Routes>
</MemoryRouter>
);
render(Component);
useParams.mockReturnValue({
'*': 'search',
});

const Component = (<EnterprisePageRedirectWithContext initialAppState={initialState} />);
renderWithRouter(Component);

await waitFor(() => expect(fetchEnterpriseCustomerByUUID).toHaveBeenCalledTimes(1));

expect(mockNavigate).toHaveBeenCalledWith(`/${TEST_ENTERPRISES[0].slug}/search`);
expect(window.location.pathname).toEqual(`/${TEST_ENTERPRISES[0].slug}/search`);
});

test('redirects to correct ``redirectPath`` from route params when user is linked to more than 1 Enterprise Customer', async () => {
Expand All @@ -129,17 +117,15 @@ describe('<EnterprisePageRedirect />', () => {
},
};

const Component = (
<MemoryRouter initialEntries={['/r/course/edX+DemoX']}>
<Routes>
<Route path="/r/*" element={<EnterprisePageRedirectWithContext initialAppState={initialState} />} />
</Routes>
</MemoryRouter>
);
render(Component);
useParams.mockReturnValue({
'*': 'course/edX+DemoX',
});

const Component = (<EnterprisePageRedirectWithContext initialAppState={initialState} />);
renderWithRouter(Component);

await waitFor(() => expect(fetchEnterpriseCustomerByUUID).toHaveBeenCalledTimes(1));

expect(mockNavigate).toHaveBeenCalledWith(`/${TEST_ENTERPRISES[1].slug}/course/edX+DemoX`);
expect(window.location.pathname).toEqual(`/${TEST_ENTERPRISES[1].slug}/course/edX+DemoX`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const AutoActivateLicense = () => {
state={{
from: location.pathname,
}}
replace
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const EnrollmentCompleted = () => {
const location = useLocation();
const { enterpriseConfig } = useContext(AppContext);
if (!location.state?.data) {
return <Navigate to={`/${enterpriseConfig.slug}`} />;
return <Navigate to={`/${enterpriseConfig.slug}`} replace />;
}
return (
<div className="fill-vertical-space page-light-bg">
Expand Down
1 change: 1 addition & 0 deletions src/components/license-activation/LicenseActivation.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const LicenseActivation = () => {
<Navigate
to={redirectToPath}
state={{ activationSuccess }}
replace
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const LicenseActivationPage = () => {
return (
<Navigate
to={`/${enterpriseConfig.slug}`}
replace
/>
);
}
Expand Down
Loading

0 comments on commit a6cc833

Please sign in to comment.