Skip to content

Commit

Permalink
refactor: after review
Browse files Browse the repository at this point in the history
  • Loading branch information
PKulkoRaccoonGang committed Nov 3, 2024
1 parent b40903b commit 5f19746
Show file tree
Hide file tree
Showing 13 changed files with 69 additions and 115 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"extends @edx/browserslist-config"
],
"scripts": {
"build": "sh run-build-for-gh-deps.sh",
"build": "fedx-scripts webpack",
"i18n_extract": "fedx-scripts formatjs extract",
"stylelint": "stylelint \"plugins/**/*.scss\" \"src/**/*.scss\" \"scss/**/*.scss\" --config .stylelintrc.json",
"lint": "npm run stylelint && fedx-scripts eslint --ext .js --ext .jsx --ext .ts --ext .tsx .",
Expand Down
39 changes: 0 additions & 39 deletions run-build-for-gh-deps.sh

This file was deleted.

9 changes: 7 additions & 2 deletions src/CourseAuthoringPage.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import CourseAuthoringPage from './CourseAuthoringPage';
import PagesAndResources from './pages-and-resources/PagesAndResources';
import { executeThunk } from './utils';
import { fetchCourseApps } from './pages-and-resources/data/thunks';
import { fetchCourseDetail } from './data/thunks';
import { fetchCourseDetail, fetchWaffleFlags } from './data/thunks';
import { getApiWaffleFlagsUrl } from './data/api';

const courseId = 'course-v1:edX+TestX+Test_Course';
let mockPathname = '/evilguy/';
Expand All @@ -25,7 +26,7 @@ jest.mock('react-router-dom', () => ({
let axiosMock;
let store;

beforeEach(() => {
beforeEach(async () => {
initializeMockApp({
authenticatedUser: {
userId: 3,
Expand All @@ -36,6 +37,10 @@ beforeEach(() => {
});
store = initializeStore();
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
axiosMock
.onGet(getApiWaffleFlagsUrl(courseId))
.reply(200, {});
await executeThunk(fetchWaffleFlags(courseId), store.dispatch);
});

describe('Editor Pages Load no header', () => {
Expand Down
13 changes: 12 additions & 1 deletion src/CourseAuthoringRoutes.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,21 @@ import { AppProvider } from '@edx/frontend-platform/react';
import { initializeMockApp } from '@edx/frontend-platform';
import { render, screen } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';
import MockAdapter from 'axios-mock-adapter';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
import CourseAuthoringRoutes from './CourseAuthoringRoutes';
import initializeStore from './store';
import { executeThunk } from './utils';
import { getApiWaffleFlagsUrl } from './data/api';
import { fetchWaffleFlags } from './data/thunks';

const courseId = 'course-v1:edX+TestX+Test_Course';
const pagesAndResourcesMockText = 'Pages And Resources';
const editorContainerMockText = 'Editor Container';
const videoSelectorContainerMockText = 'Video Selector Container';
const customPagesMockText = 'Custom Pages';
let store;
let axiosMock;
const mockComponentFn = jest.fn();

jest.mock('react-router-dom', () => ({
Expand Down Expand Up @@ -50,7 +56,7 @@ jest.mock('./custom-pages/CustomPages', () => (props) => {
});

describe('<CourseAuthoringRoutes>', () => {
beforeEach(() => {
beforeEach(async () => {
initializeMockApp({
authenticatedUser: {
userId: 3,
Expand All @@ -60,6 +66,11 @@ describe('<CourseAuthoringRoutes>', () => {
},
});
store = initializeStore();
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
axiosMock
.onGet(getApiWaffleFlagsUrl(courseId))
.reply(200, {});
await executeThunk(fetchWaffleFlags(courseId), store.dispatch);
});

fit('renders the PagesAndResources component when the pages and resources route is active', () => {
Expand Down
4 changes: 2 additions & 2 deletions src/course-outline/hooks.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,15 @@ const useCourseOutline = ({ courseId }) => {
};

const getUnitUrl = (locator) => {
if (getConfig().ENABLE_UNIT_PAGE === 'true' || waffleFlags.useNewUnitPage) {
if (getConfig().ENABLE_UNIT_PAGE === 'true' && waffleFlags.useNewUnitPage) {
return `/course/${courseId}/container/${locator}`;
}
return `${getConfig().STUDIO_BASE_URL}/container/${locator}`;
};

const openUnitPage = (locator) => {
const url = getUnitUrl(locator);
if (getConfig().ENABLE_UNIT_PAGE === 'true' || waffleFlags.useNewUnitPage) {
if (getConfig().ENABLE_UNIT_PAGE === 'true' && waffleFlags.useNewUnitPage) {
navigate(url);
} else {
window.location.assign(url);
Expand Down
1 change: 0 additions & 1 deletion src/data/api.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable import/prefer-default-export */
import { camelCaseObject, getConfig } from '@edx/frontend-platform';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';

Expand Down
2 changes: 1 addition & 1 deletion src/data/slice.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const slice = createSlice({
useNewExportPage: true,
useNewFilesUploadsPage: true,
useNewVideoUploadsPage: true,
useNewCourseOutlinePage: false,
useNewCourseOutlinePage: true,
useNewUnitPage: false,
useNewCourseTeamPage: true,
useNewCertificatesPage: true,
Expand Down
13 changes: 3 additions & 10 deletions src/data/thunks.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,8 @@ export function fetchWaffleFlags(courseId) {
return async (dispatch) => {
dispatch(updateStatus({ courseId, status: RequestStatus.IN_PROGRESS }));

try {
const waffleFlags = await getWaffleFlags(courseId);
dispatch(updateStatus({ courseId, status: RequestStatus.SUCCESSFUL }));
dispatch(fetchWaffleFlagsSuccess({ waffleFlags }));
} catch (error) {
// If fetching the waffle flags is unsuccessful,
// the pages will still be accessible and display without any issues.
// eslint-disable-next-line no-console
console.error({ courseId, status: RequestStatus.NOT_FOUND });
}
const waffleFlags = await getWaffleFlags(courseId);
dispatch(updateStatus({ courseId, status: RequestStatus.SUCCESSFUL }));
dispatch(fetchWaffleFlagsSuccess({ waffleFlags }));
};
}
4 changes: 1 addition & 3 deletions src/header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { getConfig } from '@edx/frontend-platform';
import { useIntl } from '@edx/frontend-platform/i18n';
import { StudioHeader } from '@edx/frontend-component-header';
import { type Container, useToggle } from '@openedx/paragon';
import { generatePath, useHref, useNavigate } from 'react-router-dom';
import { generatePath, useHref } from 'react-router-dom';

import { getWaffleFlags } from '../data/selectors';
import { SearchModal } from '../search-modal';
Expand Down Expand Up @@ -33,7 +33,6 @@ const Header = ({
}: HeaderProps) => {
const intl = useIntl();
const libraryHref = useHref('/library/:libraryId');
const navigate = useNavigate();
const waffleFlags = useSelector(getWaffleFlags);

const [isShowSearchModalOpen, openSearchModal, closeSearchModal] = useToggle(false);
Expand Down Expand Up @@ -80,7 +79,6 @@ const Header = ({
outlineLink={getOutlineLink()}
searchButtonAction={meiliSearchEnabled ? openSearchModal : undefined}
containerProps={containerProps}
onNavigate={(url) => navigate(url)}
isNewHomePage={waffleFlags.useNewHomePage}
/>
{meiliSearchEnabled && (
Expand Down
10 changes: 10 additions & 0 deletions src/studio-home/StudioHome.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { executeThunk } from '../utils';
import { studioHomeMock } from './__mocks__';
import { getStudioHomeApiUrl } from './data/api';
import { fetchStudioHomeData } from './data/thunks';
import { getApiWaffleFlagsUrl } from '../data/api';
import { fetchWaffleFlags } from '../data/thunks';
import messages from './messages';
import createNewCourseMessages from './create-new-course-form/messages';
import createOrRerunCourseMessages from '../generic/create-or-rerun-course/messages';
Expand Down Expand Up @@ -84,6 +86,10 @@ describe('<StudioHome />', () => {
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
axiosMock.onGet(getStudioHomeApiUrl()).reply(404);
await executeThunk(fetchStudioHomeData(), store.dispatch);
axiosMock
.onGet(getApiWaffleFlagsUrl())
.reply(200, {});
await executeThunk(fetchWaffleFlags(), store.dispatch);
useSelector.mockReturnValue({ studioHomeLoadingStatus: RequestStatus.FAILED });
});

Expand Down Expand Up @@ -113,6 +119,10 @@ describe('<StudioHome />', () => {
axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock);
await executeThunk(fetchStudioHomeData(), store.dispatch);
useSelector.mockReturnValue(studioHomeMock);
axiosMock
.onGet(getApiWaffleFlagsUrl())
.reply(200, {});
await executeThunk(fetchWaffleFlags(), store.dispatch);
});

it('should render page and page title correctly', () => {
Expand Down
8 changes: 5 additions & 3 deletions src/studio-home/card-item/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ const CardItem: React.FC<Props> = ({
} = useSelector(getStudioHomeData);
const waffleFlags = useSelector(getWaffleFlags);

const destinationUrl: string = waffleFlags.useNewCourseOutlinePage
? path ?? url
: path ?? new URL(url, getConfig().STUDIO_BASE_URL).toString();
const destinationUrl: string = path ?? (
waffleFlags.useNewCourseOutlinePage
? url
: new URL(url, getConfig().STUDIO_BASE_URL).toString()
);
const subtitle = isLibraries ? `${org} / ${number}` : `${org} / ${number} / ${run}`;
const readOnlyItem = !(lmsLink || rerunLink || url || path);
const showActions = !(readOnlyItem || isLibraries);
Expand Down
8 changes: 6 additions & 2 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,19 @@ export const createCorrectInternalRoute = (checkPath) => {
basePath = basePath.slice(0, -1);
}

if (!checkPath.startsWith(basePath)) {
return `${basePath}${checkPath}`;
}

return checkPath;
};

export function getPagePath(courseId, isMfePageEnabled, urlParameter) {
if (isMfePageEnabled === 'true') {
if (urlParameter === 'tabs') {
return createCorrectInternalRoute(`/course/${courseId}/pages-and-resources`);
return `/course/${courseId}/pages-and-resources`;
}
return createCorrectInternalRoute(`/course/${courseId}/${urlParameter}`);
return `/course/${courseId}/${urlParameter}`;
}
return `${getConfig().STUDIO_BASE_URL}/${urlParameter}/${courseId}`;
}
Expand Down
71 changes: 21 additions & 50 deletions src/utils.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { getConfig, getPath } from '@edx/frontend-platform';

import {
getFileSizeToClosestByte,
createCorrectInternalRoute,
convertObjectToSnakeCase,
deepConvertingKeysToCamelCase,
deepConvertingKeysToSnakeCase,
Expand All @@ -11,6 +8,7 @@ import {
convertToDateFromString,
convertToStringFromDate,
isValidDate,
getPagePath,
} from './utils';

jest.mock('@edx/frontend-platform', () => ({
Expand Down Expand Up @@ -53,53 +51,6 @@ describe('FilesAndUploads utils', () => {
});
});

describe('createCorrectInternalRoute', () => {
beforeEach(() => {
getConfig.mockReset();
getPath.mockReset();
});

it('returns the correct internal route when checkPath is not prefixed with basePath', () => {
getConfig.mockReturnValue({ PUBLIC_PATH: 'example.com' });
getPath.mockReturnValue('/');

const checkPath = '/some/path';
const result = createCorrectInternalRoute(checkPath);

expect(result).toBe('/some/path');
});

it('returns the input checkPath when it is already prefixed with basePath', () => {
getConfig.mockReturnValue({ PUBLIC_PATH: 'example.com' });
getPath.mockReturnValue('/course-authoring');

const checkPath = '/course-authoring/some/path';
const result = createCorrectInternalRoute(checkPath);

expect(result).toBe('/course-authoring/some/path');
});

it('handles basePath ending with a slash correctly', () => {
getConfig.mockReturnValue({ PUBLIC_PATH: 'example.com/' });
getPath.mockReturnValue('/course-authoring/');

const checkPath = '/course-authoring/some/path';
const result = createCorrectInternalRoute(checkPath);

expect(result).toBe('/course-authoring/some/path');
});

it('returns checkPath as is when basePath is part of checkPath', () => {
getConfig.mockReturnValue({ PUBLIC_PATH: 'example.com' });
getPath.mockReturnValue('/example-base/');

const checkPath = '/example-base/some/path';
const result = createCorrectInternalRoute(checkPath);

expect(result).toBe(checkPath);
});
});

describe('convertObjectToSnakeCase', () => {
it('converts object keys to snake_case', () => {
const input = { firstName: 'John', lastName: 'Doe' };
Expand Down Expand Up @@ -205,3 +156,23 @@ describe('FilesAndUploads utils', () => {
});
});
});

describe('getPagePath', () => {
it('returns MFE path when isMfePageEnabled is true and urlParameter is "tabs"', () => {
const courseId = '12345';
const isMfePageEnabled = 'true';
const urlParameter = 'tabs';

const result = getPagePath(courseId, isMfePageEnabled, urlParameter);
expect(result).toBe(`/course/${courseId}/pages-and-resources`);
});

it('returns MFE path when isMfePageEnabled is true and urlParameter is not "tabs"', () => {
const courseId = '12345';
const isMfePageEnabled = 'true';
const urlParameter = 'other-page';

const result = getPagePath(courseId, isMfePageEnabled, urlParameter);
expect(result).toBe(`/course/${courseId}/${urlParameter}`);
});
});

0 comments on commit 5f19746

Please sign in to comment.