Skip to content

Commit

Permalink
refactor: remove history pacakge (openedx#853)
Browse files Browse the repository at this point in the history
* refactor: remove history pacakge

* chore: improve test coverage
  • Loading branch information
Syed-Ali-Abbas-Zaidi authored and snglth committed Jan 9, 2024
1 parent d129eca commit c7b5bd5
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 36 deletions.
24 changes: 13 additions & 11 deletions src/account-settings/AccountSettingsPage.jsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
};
Expand All @@ -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,
Expand Down Expand Up @@ -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 (
<div>
<Alert variant="danger">
Expand Down Expand Up @@ -934,6 +934,8 @@ AccountSettingsPage.propTypes = {
proctored_exam_attempt_id: PropTypes.number,
}),
),
navigate: PropTypes.func.isRequired,
location: PropTypes.string.isRequired,
};

AccountSettingsPage.defaultProps = {
Expand Down Expand Up @@ -961,12 +963,12 @@ AccountSettingsPage.defaultProps = {
verifiedNameHistory: [],
};

export default connect(accountSettingsPageSelector, {
export default withLocation(withNavigate(connect(accountSettingsPageSelector, {
fetchCourseList,
fetchSettings,
saveSettings,
saveMultipleSettings,
updateDraft,
fetchSiteLanguages,
beginNameChange,
})(injectIntl(AccountSettingsPage));
})(injectIntl(AccountSettingsPage))));
5 changes: 2 additions & 3 deletions src/account-settings/data/utils/sagaUtils.js
Original file line number Diff line number Diff line change
@@ -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 }));
}
Expand All @@ -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);
}
}
19 changes: 19 additions & 0 deletions src/account-settings/hoc.jsx
Original file line number Diff line number Diff line change
@@ -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 <Component {...props} navigate={navigate} />;
};
return WrappedComponent;
};

export const withLocation = Component => {
const WrappedComponent = props => {
const location = useLocation();
return <Component {...props} location={location.pathname} />;
};
return WrappedComponent;
};
38 changes: 38 additions & 0 deletions src/account-settings/hoc.test.jsx
Original file line number Diff line number Diff line change
@@ -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
<button id="btn" onClick={() => navigate('/some-route')}>{location}</button>
);
const WrappedComponent = withNavigate(withLocation(MockComponent));

test('Provide Navigation to Component', () => {
const wrapper = mount(
<WrappedComponent />,
);
const btn = wrapper.find('#btn');
btn.simulate('click');

expect(mockedNavigator).toHaveBeenCalledWith('/some-route');
});

test('Provide Location Pathname to Component', () => {
const wrapper = mount(
<WrappedComponent />,
);

expect(wrapper.find('#btn').text()).toContain('/current-location');
});
3 changes: 2 additions & 1 deletion src/account-settings/site-language/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => ({
Expand Down
4 changes: 2 additions & 2 deletions src/account-settings/site-language/sagas.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
5 changes: 1 addition & 4 deletions src/id-verification/tests/panels/SummaryPanel.test.jsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -21,8 +20,6 @@ dataService.submitIdVerification = jest.fn().mockReturnValue({ success: true });

const IntlSummaryPanel = injectIntl(SummaryPanel);

const history = createMemoryHistory();

describe('SummaryPanel', () => {
const defaultProps = {
intl: {},
Expand All @@ -41,7 +38,7 @@ describe('SummaryPanel', () => {

const getPanel = async () => {
await act(async () => render((
<Router history={history}>
<Router>
<IntlProvider locale="en">
<VerifiedNameContext.Provider value={verifiedNameContextValue}>
<IdVerificationContext.Provider value={appContextValue}>
Expand Down
24 changes: 9 additions & 15 deletions src/notification-preferences/NotificationPreferences.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -79,20 +78,15 @@ const setupStore = (override = {}) => {
return store;
};

const renderComponent = (store = {}) => {
const history = createMemoryHistory();
history.push(`/notifications/${courseId}`);
return render(
<Router history={history}>
<IntlProvider locale="en">
<Provider store={store}>
<NotificationPreferences />
</Provider>
</IntlProvider>
</Router>,
);
};

const renderComponent = (store = {}) => render(
<Router>
<IntlProvider locale="en">
<Provider store={store}>
<NotificationPreferences />
</Provider>
</IntlProvider>
</Router>,
);
describe('Notification Preferences', () => {
let store;
beforeEach(() => {
Expand Down

0 comments on commit c7b5bd5

Please sign in to comment.