Skip to content

Commit

Permalink
fix: when course search url is relative, use LMS as origin
Browse files Browse the repository at this point in the history
  when platformSettings.courseSearchUrl is relative which is
  returned from a call to api/learner_home/init, use LMS_BASE_URL
  as origin.

  This fixes uses the same pattern which already used for
  different url, this probably wasn't an issue for edx.org given
  platformSettings.courseSearchUrl value is abosulte at edx.org
  • Loading branch information
ghassanmas committed Jun 12, 2023
1 parent 48ec29d commit ce88d29
Show file tree
Hide file tree
Showing 22 changed files with 28,046 additions and 28,035 deletions.
55,980 changes: 27,990 additions & 27,990 deletions package-lock.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ exports[`NoCoursesView snapshot 1`] = `
</p>
<Button
as="a"
href="course-search-url"
href="http://localhost:18000/course-search-url"
iconBefore={[MockFunction icons.Search]}
variant="brand"
>
Expand Down
3 changes: 2 additions & 1 deletion src/containers/CourseList/NoCoursesView/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import { useIntl } from '@edx/frontend-platform/i18n';
import { Button, Image } from '@edx/paragon';
import { Search } from '@edx/paragon/icons';
import { baseAppUrl } from 'data/services/lms/urls';

import emptyCourseSVG from 'assets/empty-course.svg';
import { reduxHooks } from 'hooks';
Expand All @@ -27,7 +28,7 @@ export const NoCoursesView = () => {
<Button
variant="brand"
as="a"
href={courseSearchUrl}
href={baseAppUrl(courseSearchUrl)}
iconBefore={Search}
>
{formatMessage(messages.exploreCoursesButton)}
Expand Down
2 changes: 1 addition & 1 deletion src/containers/CourseList/NoCoursesView/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import EmptyCourse from '.';
jest.mock('hooks', () => ({
reduxHooks: {
usePlatformSettingsData: jest.fn(() => ({
courseSearchUrl: 'course-search-url',
courseSearchUrl: '/course-search-url',
})),
},
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const CollapseMenuBody = ({ isOpen }) => {
const dashboard = reduxHooks.useEnterpriseDashboardData();
const { courseSearchUrl } = reduxHooks.usePlatformSettingsData();

const exploreCoursesClick = findCoursesNavDropdownClicked(courseSearchUrl);
const exploreCoursesClick = findCoursesNavDropdownClicked(urls.baseAppUrl(courseSearchUrl));

return (
isOpen && (
Expand All @@ -34,7 +34,7 @@ export const CollapseMenuBody = ({ isOpen }) => {
</Button>
<Button
as="a"
href={courseSearchUrl}
href={urls.baseAppUrl(courseSearchUrl)}
variant="inverse-primary"
onClick={exploreCoursesClick}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jest.mock('hooks', () => ({
url: 'url',
}),
usePlatformSettingsData: () => ({
courseSearchUrl: 'courseSearchUrl',
courseSearchUrl: '/courseSearchUrl',
}),
},
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ exports[`CollapseMenuBody render 1`] = `
</Button>
<Button
as="a"
href="courseSearchUrl"
onClick={[MockFunction findCoursesNavDropdownClicked("courseSearchUrl")]}
href="http://localhost:18000/courseSearchUrl"
onClick={[MockFunction findCoursesNavDropdownClicked("http://localhost:18000/courseSearchUrl")]}
variant="inverse-primary"
>
Discover New
Expand Down Expand Up @@ -86,8 +86,8 @@ exports[`CollapseMenuBody render unauthenticated 1`] = `
</Button>
<Button
as="a"
href="courseSearchUrl"
onClick={[MockFunction findCoursesNavDropdownClicked("courseSearchUrl")]}
href="http://localhost:18000/courseSearchUrl"
onClick={[MockFunction findCoursesNavDropdownClicked("http://localhost:18000/courseSearchUrl")]}
variant="inverse-primary"
>
Discover New
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { useIsCollapsed } from '../hooks';
jest.mock('@edx/frontend-platform', () => ({
getConfig: jest.fn(),
}));

jest.mock('@edx/frontend-platform/react', () => ({
AppContext: {
authenticatedUser: {
Expand All @@ -17,11 +18,13 @@ jest.mock('@edx/frontend-platform/react', () => ({
},
},
}));
const COURSE_SEARCH_URL = 'test-course-search-url';

jest.mock('hooks', () => ({
reduxHooks: {
useEnterpriseDashboardData: jest.fn(),
usePlatformSettingsData: jest.fn(() => ({
courseSearchUrl: 'test-course-search-url',
courseSearchUrl: COURSE_SEARCH_URL,
})),
},
}));
Expand All @@ -31,6 +34,7 @@ jest.mock('../hooks', () => ({
}));

jest.mock('data/services/lms/urls', () => ({
baseAppUrl: (url) => (url),
programsUrl: 'http://localhost:18000/dashboard/programs',
}));

Expand All @@ -41,6 +45,7 @@ const config = {
ORDER_HISTORY_URL: 'http://order-history-url.test',
SUPPORT_URL: 'http://localhost:18000/support',
CAREER_LINK_URL: 'http://localhost:18000/career',
LMS_BASE_URL: 'http:/localhost:18000',
};
getConfig.mockReturnValue(config);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ exports[`ExpandedHeader render 1`] = `
<Button
as="a"
className="p-4"
href="courseSearchUrl"
onClick={[MockFunction findCoursesNavClicked("courseSearchUrl")]}
href="http://localhost:18000/courseSearchUrl"
onClick={[MockFunction findCoursesNavClicked("http://localhost:18000/courseSearchUrl")]}
variant="inverse-primary"
>
Discover New
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const ExpandedHeader = () => {
const { courseSearchUrl } = reduxHooks.usePlatformSettingsData();
const isCollapsed = useIsCollapsed();

const exploreCoursesClick = findCoursesNavClicked(courseSearchUrl);
const exploreCoursesClick = findCoursesNavClicked(urls.baseAppUrl(courseSearchUrl));

return (
!isCollapsed && (
Expand All @@ -44,7 +44,7 @@ export const ExpandedHeader = () => {
</Button>
<Button
as="a"
href={courseSearchUrl}
href={urls.baseAppUrl(courseSearchUrl)}
variant="inverse-primary"
className="p-4"
onClick={exploreCoursesClick}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ import { useIsCollapsed } from '../hooks';

jest.mock('data/services/lms/urls', () => ({
programsUrl: 'programsUrl',
baseAppUrl: url => (`http://localhost:18000${url}`),
}));

jest.mock('hooks', () => ({
reduxHooks: {
usePlatformSettingsData: () => ({
courseSearchUrl: 'courseSearchUrl',
courseSearchUrl: '/courseSearchUrl',
}),
},
}));
Expand Down
2 changes: 1 addition & 1 deletion src/data/services/lms/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import * as module from './api';
* GET Actions
*********************************************************************************/
export const initializeList = ({ user } = {}) => get(
stringifyUrl(urls.getInitApi(), { [apiKeys.user]: user }),
stringifyUrl(urls.getInitApiUrl(), { [apiKeys.user]: user }),
);

export const updateEntitlementEnrollment = ({ uuid, courseId }) => post(
Expand Down
2 changes: 1 addition & 1 deletion src/data/services/lms/api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('lms api methods', () => {
[apiKeys.user]: testUser,
};
expect(api.initializeList(userArg)).toEqual(
utils.get(utils.stringifyUrl(urls.getInitApi(), userArg)),
utils.get(utils.stringifyUrl(urls.getInitApiUrl(), userArg)),
);
});
});
Expand Down
23 changes: 11 additions & 12 deletions src/data/services/lms/urls.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,41 +2,40 @@ import { StrictDict } from 'utils';

import { getConfig } from '@edx/frontend-platform';

export const ecommerceUrl = `${getConfig.ECOMMERCE_BASE_URL}`;
export const getEcommerceUrl = () => getConfig().ECOMMERCE_BASE_URL;

const getBaseUrl = () => (`${getConfig().LMS_BASE_URL}`);
const getBaseUrl = () => getConfig().LMS_BASE_URL;

export const getApi = () => (`${getConfig().LMS_BASE_URL}/api`);
export const getApiUrl = () => (`${getConfig().LMS_BASE_URL}/api`);

// const getInitApi = `${getApi()}learner_home/mock/init`; // mock endpoint for testing
const getInitApi = () => (`${getApi()}/learner_home/init`);
const getInitApiUrl = () => (`${getApiUrl()}/learner_home/init`);

const event = `${getBaseUrl()}/event`;
const courseUnenroll = `${getBaseUrl()}/change_enrollment`;
const updateEmailSettings = `${getApi()}/change_email_settings`;
const entitlementEnrollment = (uuid) => `${getApi()}/entitlements/v1/entitlements/${uuid}/enrollments`;
const updateEmailSettings = `${getApiUrl()}/change_email_settings`;
const entitlementEnrollment = (uuid) => `${getApiUrl()}/entitlements/v1/entitlements/${uuid}/enrollments`;

// if url is null or absolute, return it as is
const updateUrl = (base, url) => ((url == null || url.startsWith('http://') || url.startsWith('https://')) ? url : `${base}${url}`);
export const updateUrl = (base, url) => ((url == null || url.startsWith('http://') || url.startsWith('https://')) ? url : `${base}${url}`);

export const baseAppUrl = (url) => updateUrl(getBaseUrl(), url);
export const learningMfeUrl = (url) => updateUrl(getConfig().LEARNING_BASE_URL, url);

// static view url
const programsUrl = baseAppUrl('/dashboard/programs');

export const creditPurchaseUrl = (courseId) => `${getConfig().ECOMMERCE_PUBLIC_URL_ROOT}/credit/checkout/${courseId}/`;
export const creditRequestUrl = (providerId) => `${getApi()}/credit/v1/providers/${providerId}/request/`;
export const creditPurchaseUrl = (courseId) => `${getEcommerceUrl()}/credit/checkout/${courseId}/`;
export const creditRequestUrl = (providerId) => `${getApiUrl()}/credit/v1/providers/${providerId}/request/`;

export default StrictDict({
getApi,
getApiUrl,
baseAppUrl,
courseUnenroll,
creditPurchaseUrl,
creditRequestUrl,
entitlementEnrollment,
event,
getInitApi,
getInitApiUrl,
learningMfeUrl,
programsUrl,
updateEmailSettings,
Expand Down
2 changes: 1 addition & 1 deletion src/data/services/lms/urls.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('urls', () => {
it('builds from api url and loads providerId', () => {
const providerId = 'test-provider-id';
const url = urls.creditRequestUrl(providerId);
expect(url.startsWith(urls.getApi())).toEqual(true);
expect(url.startsWith(urls.getApiUrl())).toEqual(true);
expect(url).toEqual(expect.stringContaining(providerId));
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ exports[`LookingForChallengeWidget snapshots default 1`] = `
<h5>
<Hyperlink
className="d-flex align-items-center"
destination="course-search-url"
onClick={[MockFunction track.findCoursesWidgetClicked('course-search-url')]}
destination="http://localhost:18000/course-search-url"
onClick={[MockFunction track.findCoursesWidgetClicked('http://localhost:18000/course-search-url')]}
variant="brand"
>
<format-message-function
Expand Down
5 changes: 3 additions & 2 deletions src/widgets/LookingForChallengeWidget/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { ArrowForward } from '@edx/paragon/icons';

import { reduxHooks } from 'hooks';
import moreCoursesSVG from 'assets/more-courses-sidewidget.svg';
import { baseAppUrl } from 'data/services/lms/urls';

import track from '../RecommendationsPanel/track';
import messages from './messages';
Expand All @@ -29,8 +30,8 @@ export const LookingForChallengeWidget = () => {
<h5>
<Hyperlink
variant="brand"
destination={courseSearchUrl}
onClick={track.findCoursesWidgetClicked(courseSearchUrl)}
destination={baseAppUrl(courseSearchUrl)}
onClick={track.findCoursesWidgetClicked(baseAppUrl(courseSearchUrl))}
className="d-flex align-items-center"
>
{formatMessage(messages.findCoursesButton, { arrow: arrowIcon })}
Expand Down
2 changes: 1 addition & 1 deletion src/widgets/LookingForChallengeWidget/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import LookingForChallengeWidget from '.';
jest.mock('hooks', () => ({
reduxHooks: {
usePlatformSettingsData: () => ({
courseSearchUrl: 'course-search-url',
courseSearchUrl: 'http://localhost:18000/course-search-url',
}),
},
}));
Expand Down
5 changes: 3 additions & 2 deletions src/widgets/RecommendationsPanel/LoadedView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import PropTypes from 'prop-types';
import { useIntl } from '@edx/frontend-platform/i18n';
import { Button } from '@edx/paragon';
import { Search } from '@edx/paragon/icons';
import { baseAppUrl } from 'data/services/lms/urls';

import { reduxHooks } from 'hooks';
import track from './track';
Expand Down Expand Up @@ -38,8 +39,8 @@ export const LoadedView = ({
variant="tertiary"
iconBefore={Search}
as="a"
href={courseSearchUrl}
onClick={track.findCoursesWidgetClicked(courseSearchUrl)}
href={baseAppUrl(courseSearchUrl)}
onClick={track.findCoursesWidgetClicked(baseAppUrl(courseSearchUrl))}
>
{formatMessage(messages.exploreCoursesButton)}
</Button>
Expand Down
7 changes: 5 additions & 2 deletions src/widgets/RecommendationsPanel/LoadedView.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,20 @@ import LoadedView from './LoadedView';
import mockData from './mockData';
import messages from './messages';

jest.mock('./components/CourseCard', () => 'CourseCard');
jest.mock('hooks', () => ({
reduxHooks: {
usePlatformSettingsData: () => ({
courseSearchUrl: 'course-search-url',
courseSearchUrl: '/course-search-url',
}),
},
}));
jest.mock('data/services/lms/urls', () => ({
baseAppUrl: (url) => (`http://localhost:18000${url}`),
}));
jest.mock('./track', () => ({
findCoursesWidgetClicked: (href) => jest.fn().mockName(`track.findCoursesWidgetClicked('${href}')`),
}));
jest.mock('./components/CourseCard', () => 'CourseCard');

describe('RecommendationsPanel LoadedView', () => {
const props = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ exports[`RecommendationsPanel LoadedView snapshot with personalize recommendatio
>
<Button
as="a"
href="course-search-url"
href="http://localhost:18000/course-search-url"
iconBefore={[MockFunction icons.Search]}
onClick={[MockFunction track.findCoursesWidgetClicked('course-search-url')]}
onClick={[MockFunction track.findCoursesWidgetClicked('http://localhost:18000/course-search-url')]}
variant="tertiary"
>
Explore courses
Expand Down Expand Up @@ -139,9 +139,9 @@ exports[`RecommendationsPanel LoadedView snapshot without personalize recommenda
>
<Button
as="a"
href="course-search-url"
href="http://localhost:18000/course-search-url"
iconBefore={[MockFunction icons.Search]}
onClick={[MockFunction track.findCoursesWidgetClicked('course-search-url')]}
onClick={[MockFunction track.findCoursesWidgetClicked('http://localhost:18000/course-search-url')]}
variant="tertiary"
>
Explore courses
Expand Down
2 changes: 1 addition & 1 deletion src/widgets/RecommendationsPanel/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { StrictDict } from 'utils';
import { get, stringifyUrl } from 'data/services/lms/utils';
import urls from 'data/services/lms/urls';

export const getFetchUrl = () => (`${urls.getApi()}/learner_recommendations/courses/`);
export const getFetchUrl = () => (`${urls.getApiUrl()}/learner_recommendations/courses/`);
export const apiKeys = StrictDict({ user: 'user' });

const fetchRecommendedCourses = () => get(stringifyUrl(getFetchUrl()));
Expand Down

0 comments on commit ce88d29

Please sign in to comment.