From c7b5bd579c370049e9d5e5d9377c4113d9805b62 Mon Sep 17 00:00:00 2001 From: Syed Ali Abbas Zaidi <88369802+Syed-Ali-Abbas-Zaidi@users.noreply.github.com> Date: Wed, 23 Aug 2023 18:21:29 +0500 Subject: [PATCH] refactor: remove history pacakge (#853) * refactor: remove history pacakge * chore: improve test coverage --- src/account-settings/AccountSettingsPage.jsx | 24 ++++++------ src/account-settings/data/utils/sagaUtils.js | 5 +-- src/account-settings/hoc.jsx | 19 ++++++++++ src/account-settings/hoc.test.jsx | 38 +++++++++++++++++++ src/account-settings/site-language/actions.js | 3 +- src/account-settings/site-language/sagas.js | 4 +- .../tests/panels/SummaryPanel.test.jsx | 5 +-- .../NotificationPreferences.test.jsx | 24 +++++------- 8 files changed, 86 insertions(+), 36 deletions(-) create mode 100644 src/account-settings/hoc.jsx create mode 100644 src/account-settings/hoc.test.jsx diff --git a/src/account-settings/AccountSettingsPage.jsx b/src/account-settings/AccountSettingsPage.jsx index abdb807ae..e9ac1087d 100644 --- a/src/account-settings/AccountSettingsPage.jsx +++ b/src/account-settings/AccountSettingsPage.jsx @@ -1,5 +1,5 @@ import { AppContext } from '@edx/frontend-platform/react'; -import { getConfig, history, getQueryParameters } from '@edx/frontend-platform'; +import { getConfig, getQueryParameters } from '@edx/frontend-platform'; import React from 'react'; import PropTypes from 'prop-types'; import { connect } from 'react-redux'; @@ -51,19 +51,13 @@ import { fetchSiteLanguages } from './site-language'; import CoachingToggle from './coaching/CoachingToggle'; import DemographicsSection from './demographics/DemographicsSection'; import { fetchCourseList } from '../notification-preferences/data/thunks'; +import { withLocation, withNavigate } from './hoc'; class AccountSettingsPage extends React.Component { constructor(props, context) { super(props, context); - // If there is a "duplicate_provider" query parameter, that's the backend's - // way of telling us that the provider account the user tried to link is already linked - // to another user account on the platform. We use this to display a message to that effect, - // and remove the parameter from the URL. const duplicateTpaProvider = getQueryParameters().duplicate_provider; - if (duplicateTpaProvider !== undefined) { - history.replace(history.location.pathname); - } this.state = { duplicateTpaProvider, }; @@ -82,7 +76,7 @@ class AccountSettingsPage extends React.Component { componentDidMount() { this.props.fetchCourseList(); this.props.fetchSettings(); - this.props.fetchSiteLanguages(); + this.props.fetchSiteLanguages(this.props.navigate); sendTrackingLogEvent('edx.user.settings.viewed', { page: 'account', visibility: null, @@ -202,6 +196,12 @@ class AccountSettingsPage extends React.Component { return null; } + // If there is a "duplicate_provider" query parameter, that's the backend's + // way of telling us that the provider account the user tried to link is already linked + // to another user account on the platform. We use this to display a message to that effect, + // and remove the parameter from the URL. + this.props.navigate(this.props.location, { replace: true }); + return (
@@ -934,6 +934,8 @@ AccountSettingsPage.propTypes = { proctored_exam_attempt_id: PropTypes.number, }), ), + navigate: PropTypes.func.isRequired, + location: PropTypes.string.isRequired, }; AccountSettingsPage.defaultProps = { @@ -961,7 +963,7 @@ AccountSettingsPage.defaultProps = { verifiedNameHistory: [], }; -export default connect(accountSettingsPageSelector, { +export default withLocation(withNavigate(connect(accountSettingsPageSelector, { fetchCourseList, fetchSettings, saveSettings, @@ -969,4 +971,4 @@ export default connect(accountSettingsPageSelector, { updateDraft, fetchSiteLanguages, beginNameChange, -})(injectIntl(AccountSettingsPage)); +})(injectIntl(AccountSettingsPage)))); diff --git a/src/account-settings/data/utils/sagaUtils.js b/src/account-settings/data/utils/sagaUtils.js index 0ce3ecd08..b15dea168 100644 --- a/src/account-settings/data/utils/sagaUtils.js +++ b/src/account-settings/data/utils/sagaUtils.js @@ -1,8 +1,7 @@ import { put } from 'redux-saga/effects'; import { logError } from '@edx/frontend-platform/logging'; -import { history } from '@edx/frontend-platform'; -export default function* handleFailure(error, failureAction = null, failureRedirectPath = null) { +export default function* handleFailure(error, navigate, failureAction = null, failureRedirectPath = null) { if (error.fieldErrors && failureAction !== null) { yield put(failureAction({ fieldErrors: error.fieldErrors })); } @@ -11,6 +10,6 @@ export default function* handleFailure(error, failureAction = null, failureRedir yield put(failureAction(error.message)); } if (failureRedirectPath !== null) { - history.push(failureRedirectPath); + navigate(failureRedirectPath); } } diff --git a/src/account-settings/hoc.jsx b/src/account-settings/hoc.jsx new file mode 100644 index 000000000..2b49fe9ac --- /dev/null +++ b/src/account-settings/hoc.jsx @@ -0,0 +1,19 @@ +import React from 'react'; + +import { useLocation, useNavigate } from 'react-router-dom'; + +export const withNavigate = Component => { + const WrappedComponent = props => { + const navigate = useNavigate(); + return ; + }; + return WrappedComponent; +}; + +export const withLocation = Component => { + const WrappedComponent = props => { + const location = useLocation(); + return ; + }; + return WrappedComponent; +}; diff --git a/src/account-settings/hoc.test.jsx b/src/account-settings/hoc.test.jsx new file mode 100644 index 000000000..86b05a925 --- /dev/null +++ b/src/account-settings/hoc.test.jsx @@ -0,0 +1,38 @@ +import React from 'react'; +import { mount } from 'enzyme'; + +import { withLocation, withNavigate } from './hoc'; + +const mockedNavigator = jest.fn(); + +jest.mock('react-router-dom', () => ({ + useNavigate: () => mockedNavigator, + useLocation: () => ({ + pathname: '/current-location', + }), +})); + +// eslint-disable-next-line react/prop-types +const MockComponent = ({ navigate, location }) => ( + // eslint-disable-next-line react/button-has-type, react/prop-types + +); +const WrappedComponent = withNavigate(withLocation(MockComponent)); + +test('Provide Navigation to Component', () => { + const wrapper = mount( + , + ); + const btn = wrapper.find('#btn'); + btn.simulate('click'); + + expect(mockedNavigator).toHaveBeenCalledWith('/some-route'); +}); + +test('Provide Location Pathname to Component', () => { + const wrapper = mount( + , + ); + + expect(wrapper.find('#btn').text()).toContain('/current-location'); +}); diff --git a/src/account-settings/site-language/actions.js b/src/account-settings/site-language/actions.js index d10e64b27..20a9030ef 100644 --- a/src/account-settings/site-language/actions.js +++ b/src/account-settings/site-language/actions.js @@ -2,8 +2,9 @@ import { AsyncActionType } from '../data/utils'; export const FETCH_SITE_LANGUAGES = new AsyncActionType('SITE_LANGUAGE', 'FETCH_SITE_LANGUAGES'); -export const fetchSiteLanguages = () => ({ +export const fetchSiteLanguages = handleNavigation => ({ type: FETCH_SITE_LANGUAGES.BASE, + payload: { handleNavigation }, }); export const fetchSiteLanguagesBegin = () => ({ diff --git a/src/account-settings/site-language/sagas.js b/src/account-settings/site-language/sagas.js index 461e384cc..5eccc0134 100644 --- a/src/account-settings/site-language/sagas.js +++ b/src/account-settings/site-language/sagas.js @@ -10,13 +10,13 @@ import { import { getSiteLanguageList } from './service'; import { handleFailure } from '../data/utils'; -function* handleFetchSiteLanguages() { +function* handleFetchSiteLanguages(action) { try { yield put(fetchSiteLanguagesBegin()); const siteLanguageList = yield call(getSiteLanguageList); yield put(fetchSiteLanguagesSuccess(siteLanguageList)); } catch (e) { - yield call(handleFailure, e, fetchSiteLanguagesFailure); + yield call(handleFailure, e, action.payload.handleNavigation, fetchSiteLanguagesFailure); } } diff --git a/src/id-verification/tests/panels/SummaryPanel.test.jsx b/src/id-verification/tests/panels/SummaryPanel.test.jsx index 8ca15bfb4..1402f8b7d 100644 --- a/src/id-verification/tests/panels/SummaryPanel.test.jsx +++ b/src/id-verification/tests/panels/SummaryPanel.test.jsx @@ -1,7 +1,6 @@ /* eslint-disable no-import-assign */ import React from 'react'; import { BrowserRouter as Router } from 'react-router-dom'; -import { createMemoryHistory } from 'history'; import { render, cleanup, act, screen, fireEvent, waitFor, } from '@testing-library/react'; @@ -21,8 +20,6 @@ dataService.submitIdVerification = jest.fn().mockReturnValue({ success: true }); const IntlSummaryPanel = injectIntl(SummaryPanel); -const history = createMemoryHistory(); - describe('SummaryPanel', () => { const defaultProps = { intl: {}, @@ -41,7 +38,7 @@ describe('SummaryPanel', () => { const getPanel = async () => { await act(async () => render(( - + diff --git a/src/notification-preferences/NotificationPreferences.test.jsx b/src/notification-preferences/NotificationPreferences.test.jsx index e709c069a..f57e50beb 100644 --- a/src/notification-preferences/NotificationPreferences.test.jsx +++ b/src/notification-preferences/NotificationPreferences.test.jsx @@ -2,7 +2,6 @@ import { Provider } from 'react-redux'; import { BrowserRouter as Router } from 'react-router-dom'; import configureStore from 'redux-mock-store'; -import { createMemoryHistory } from 'history'; import { fireEvent, render, screen } from '@testing-library/react'; import * as auth from '@edx/frontend-platform/auth'; import { IntlProvider } from '@edx/frontend-platform/i18n'; @@ -79,20 +78,15 @@ const setupStore = (override = {}) => { return store; }; -const renderComponent = (store = {}) => { - const history = createMemoryHistory(); - history.push(`/notifications/${courseId}`); - return render( - - - - - - - , - ); -}; - +const renderComponent = (store = {}) => render( + + + + + + + , +); describe('Notification Preferences', () => { let store; beforeEach(() => {