Skip to content

Commit

Permalink
feat: create component to decode params
Browse files Browse the repository at this point in the history
  • Loading branch information
Lunyachek authored and leangseu-edx committed Mar 27, 2023
1 parent aa380e8 commit 52235eb
Show file tree
Hide file tree
Showing 5 changed files with 186 additions and 15 deletions.
6 changes: 4 additions & 2 deletions src/courseware/CoursewareRedirectLandingPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { PageRoute } from '@edx/frontend-platform/react';
import queryString from 'query-string';
import PageLoading from '../generic/PageLoading';

import DecodePageRoute from '../decode-page-route';

const CoursewareRedirectLandingPage = () => {
const { path } = useRouteMatch();
return (
Expand All @@ -21,7 +23,7 @@ const CoursewareRedirectLandingPage = () => {
/>

<Switch>
<PageRoute
<DecodePageRoute
path={`${path}/survey/:courseId`}
render={({ match }) => {
global.location.assign(`${getConfig().LMS_BASE_URL}/courses/${match.params.courseId}/survey`);
Expand All @@ -40,7 +42,7 @@ const CoursewareRedirectLandingPage = () => {
global.location.assign(`${getConfig().LMS_BASE_URL}${consentPath}`);
}}
/>
<PageRoute
<DecodePageRoute
path={`${path}/home/:courseId`}
render={({ match }) => {
global.location.assign(`/course/${match.params.courseId}/home`);
Expand Down
16 changes: 16 additions & 0 deletions src/decode-page-route/__snapshots__/index.test.jsx.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`DecodePageRoute should not modify the url if it does not need to be decoded 1`] = `
<div>
PageRoute: {
"computedMatch": {
"path": "/course/:courseId/home",
"url": "/course/course-v1:edX+DemoX+Demo_Course/home",
"isExact": true,
"params": {
"courseId": "course-v1:edX+DemoX+Demo_Course"
}
}
}
</div>
`;
49 changes: 49 additions & 0 deletions src/decode-page-route/index.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import PropTypes from 'prop-types';
import { PageRoute } from '@edx/frontend-platform/react';
import React from 'react';
import { useHistory, generatePath } from 'react-router';

export const decodeUrl = (encodedUrl) => {
const decodedUrl = decodeURIComponent(encodedUrl);
if (encodedUrl === decodedUrl) {
return encodedUrl;
}
return decodeUrl(decodedUrl);
};

const DecodePageRoute = (props) => {
const history = useHistory();
if (props.computedMatch) {
const { url, path, params } = props.computedMatch;

Object.keys(params).forEach((param) => {
// only decode params not the entire url.
// it is just to be safe and less prone to errors
params[param] = decodeUrl(params[param]);
});

const newUrl = generatePath(path, params);

// if the url get decoded, reroute to the decoded url
if (newUrl !== url) {
history.replace(newUrl);
}
}

return <PageRoute {...props} />;
};

DecodePageRoute.propTypes = {
computedMatch: PropTypes.shape({
url: PropTypes.string.isRequired,
path: PropTypes.string.isRequired,
// eslint-disable-next-line react/forbid-prop-types
params: PropTypes.any,
}),
};

DecodePageRoute.defaultProps = {
computedMatch: null,
};

export default DecodePageRoute;
103 changes: 103 additions & 0 deletions src/decode-page-route/index.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import React from 'react';
import { render } from '@testing-library/react';
import { createMemoryHistory } from 'history';
import { Router, matchPath } from 'react-router';
import DecodePageRoute, { decodeUrl } from '.';

const decodedCourseId = 'course-v1:edX+DemoX+Demo_Course';
const encodedCourseId = encodeURIComponent(decodedCourseId);
const deepEncodedCourseId = (() => {
let path = encodedCourseId;
for (let i = 0; i < 5; i++) {
path = encodeURIComponent(path);
}
return path;
})();

jest.mock('@edx/frontend-platform/react', () => ({
PageRoute: (props) => `PageRoute: ${JSON.stringify(props, null, 2)}`,
}));

const renderPage = (props) => {
const memHistory = createMemoryHistory({
initialEntries: [props?.path],
});

const history = {
...memHistory,
replace: jest.fn(),
};

const { container } = render(
<Router history={history}>
<DecodePageRoute computedMatch={props} />
</Router>,
);

return {
container,
history,
props,
};
};

describe('DecodePageRoute', () => {
it('should not modify the url if it does not need to be decoded', () => {
const props = matchPath(`/course/${decodedCourseId}/home`, {
path: '/course/:courseId/home',
});
const { container, history } = renderPage(props);

expect(props.url).toContain(decodedCourseId);
expect(history.replace).not.toHaveBeenCalled();
expect(container).toMatchSnapshot();
});

it('should decode the url and replace the history if necessary', () => {
const props = matchPath(`/course/${encodedCourseId}/home`, {
path: '/course/:courseId/home',
});
const { history } = renderPage(props);

expect(props.url).not.toContain(decodedCourseId);
expect(props.url).toContain(encodedCourseId);
expect(history.replace.mock.calls[0][0]).toContain(decodedCourseId);
});

it('should decode the url multiple times if necessary', () => {
const props = matchPath(`/course/${deepEncodedCourseId}/home`, {
path: '/course/:courseId/home',
});
const { history } = renderPage(props);

expect(props.url).not.toContain(decodedCourseId);
expect(props.url).toContain(deepEncodedCourseId);
expect(history.replace.mock.calls[0][0]).toContain(decodedCourseId);
});

it('should only decode the url params and not the entire url', () => {
const decodedUnitId = 'some+thing';
const encodedUnitId = encodeURIComponent(decodedUnitId);
const props = matchPath(`/course/${deepEncodedCourseId}/${encodedUnitId}/${encodedUnitId}`, {
path: `/course/:courseId/${encodedUnitId}/:unitId`,
});
const { history } = renderPage(props);

const decodedUrls = history.replace.mock.calls[0][0].split('/');

// unitId get decoded
expect(decodedUrls.pop()).toContain(decodedUnitId);

// path remain encoded
expect(decodedUrls.pop()).toContain(encodedUnitId);

// courseId get decoded
expect(decodedUrls.pop()).toContain(decodedCourseId);
});
});

describe('decodeUrl', () => {
expect(decodeUrl(decodedCourseId)).toEqual(decodedCourseId);
expect(decodeUrl(encodedCourseId)).toEqual(decodedCourseId);
expect(decodeUrl(deepEncodedCourseId)).toEqual(decodedCourseId);
});
27 changes: 14 additions & 13 deletions src/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import NoticesProvider from './generic/notices';
import PathFixesProvider from './generic/path-fixes';
import LiveTab from './course-home/live-tab/LiveTab';
import CourseAccessErrorPage from './generic/CourseAccessErrorPage';
import DecodePageRoute from './decode-page-route';

subscribe(APP_READY, () => {
ReactDOM.render(
Expand All @@ -50,28 +51,28 @@ subscribe(APP_READY, () => {
<Switch>
<PageRoute exact path="/goal-unsubscribe/:token" component={GoalUnsubscribe} />
<PageRoute path="/redirect" component={CoursewareRedirectLandingPage} />
<PageRoute path="/course/:courseId/access-denied" component={CourseAccessErrorPage} />
<PageRoute path="/course/:courseId/home">
<DecodePageRoute path="/course/:courseId/access-denied" component={CourseAccessErrorPage} />
<DecodePageRoute path="/course/:courseId/home">
<TabContainer tab="outline" fetch={fetchOutlineTab} slice="courseHome">
<OutlineTab />
</TabContainer>
</PageRoute>
<PageRoute path="/course/:courseId/live">
</DecodePageRoute>
<DecodePageRoute path="/course/:courseId/live">
<TabContainer tab="lti_live" fetch={fetchLiveTab} slice="courseHome">
<LiveTab />
</TabContainer>
</PageRoute>
<PageRoute path="/course/:courseId/dates">
</DecodePageRoute>
<DecodePageRoute path="/course/:courseId/dates">
<TabContainer tab="dates" fetch={fetchDatesTab} slice="courseHome">
<DatesTab />
</TabContainer>
</PageRoute>
<PageRoute path="/course/:courseId/discussion/:path*">
</DecodePageRoute>
<DecodePageRoute path="/course/:courseId/discussion/:path*">
<TabContainer tab="discussion" fetch={fetchDiscussionTab} slice="courseHome">
<DiscussionTab />
</TabContainer>
</PageRoute>
<PageRoute
</DecodePageRoute>
<DecodePageRoute
path={[
'/course/:courseId/progress/:targetUserId/',
'/course/:courseId/progress',
Expand All @@ -86,12 +87,12 @@ subscribe(APP_READY, () => {
</TabContainer>
)}
/>
<PageRoute path="/course/:courseId/course-end">
<DecodePageRoute path="/course/:courseId/course-end">
<TabContainer tab="courseware" fetch={fetchCourse} slice="courseware">
<CourseExit />
</TabContainer>
</PageRoute>
<PageRoute
</DecodePageRoute>
<DecodePageRoute
path={[
'/course/:courseId/:sequenceId/:unitId',
'/course/:courseId/:sequenceId',
Expand Down

0 comments on commit 52235eb

Please sign in to comment.