diff --git a/src/editors/EditorContainer.tsx b/src/editors/EditorContainer.tsx index fc6fa417c1..c5b2d5ec89 100644 --- a/src/editors/EditorContainer.tsx +++ b/src/editors/EditorContainer.tsx @@ -7,32 +7,29 @@ import EditorPage from './EditorPage'; interface Props { /** Course ID or Library ID */ learningContextId: string; - /** Event handler for when user cancels out of the editor page */ + /** Event handler sometimes called when user cancels out of the editor page */ onClose?: () => void; - /** Event handler called after when user saves their changes using an editor */ - afterSave?: () => (newData: Record) => void; + /** + * Event handler called after when user saves their changes using an editor + * and sometimes called when user cancels the editor, instead of onClose. + * If changes are saved, newData will be present, and if it was cancellation, + * newData will be undefined. + * TODO: clean this up so there are separate onCancel and onSave callbacks, + * and they are used consistently instead of this mess. + */ + returnFunction?: () => (newData: Record | undefined) => void; } const EditorContainer: React.FC = ({ learningContextId, onClose, - afterSave, + returnFunction, }) => { const { blockType, blockId } = useParams(); if (blockType === undefined || blockId === undefined) { // istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker. return
Error: missing URL parameters
; } - if (!!onClose !== !!afterSave) { - /* istanbul ignore next */ - throw new Error('You must specify both onClose and afterSave or neither.'); - // These parameters are a bit messy so I'm trying to help make it more - // consistent here. For example, if you specify onClose, then returnFunction - // is only called if the save is successful. But if you leave onClose - // undefined, then returnFunction is called in either case, and with - // different arguments. The underlying EditorPage should be refactored to - // have more clear events like onCancel and onSaveSuccess - } return (
= ({ studioEndpointUrl={getConfig().STUDIO_BASE_URL} lmsEndpointUrl={getConfig().LMS_BASE_URL} onClose={onClose} - returnFunction={afterSave} + returnFunction={returnFunction} />
); diff --git a/src/editors/data/constants/requests.js b/src/editors/data/constants/requests.ts similarity index 97% rename from src/editors/data/constants/requests.js rename to src/editors/data/constants/requests.ts index ec736f27ea..e12905d588 100644 --- a/src/editors/data/constants/requests.js +++ b/src/editors/data/constants/requests.ts @@ -5,7 +5,7 @@ export const RequestStates = StrictDict({ pending: 'pending', completed: 'completed', failed: 'failed', -}); +} as const); export const RequestKeys = StrictDict({ fetchVideos: 'fetchVideos', @@ -27,4 +27,4 @@ export const RequestKeys = StrictDict({ uploadAsset: 'uploadAsset', fetchAdvancedSettings: 'fetchAdvancedSettings', fetchVideoFeatures: 'fetchVideoFeatures', -}); +} as const); diff --git a/src/editors/data/redux/app/selectors.js b/src/editors/data/redux/app/selectors.js index 43e7d6861a..91d880a2d6 100644 --- a/src/editors/data/redux/app/selectors.js +++ b/src/editors/data/redux/app/selectors.js @@ -1,5 +1,6 @@ import { createSelector } from 'reselect'; import { blockTypes } from '../../constants/app'; +import { isLibraryV1Key } from '../../../../generic/key-utils'; import * as urls from '../../services/cms/urls'; // This 'module' self-import hack enables mocking during tests. // See src/editors/decisions/0005-internal-editor-testability-decisions.md. The whole approach to how hooks are tested @@ -46,7 +47,7 @@ export const isLibrary = createSelector( module.simpleSelectors.blockId, ], (learningContextId, blockId) => { - if (learningContextId && learningContextId.startsWith('library-v1')) { + if (isLibraryV1Key(learningContextId)) { return true; } if (blockId && blockId.startsWith('lb:')) { diff --git a/src/editors/data/redux/thunkActions/app.js b/src/editors/data/redux/thunkActions/app.js index 2e210bb80d..478ef4240c 100644 --- a/src/editors/data/redux/thunkActions/app.js +++ b/src/editors/data/redux/thunkActions/app.js @@ -1,4 +1,5 @@ import { StrictDict, camelizeKeys } from '../../../utils'; +import { isLibraryKey } from '../../../../generic/key-utils'; import * as requests from './requests'; // This 'module' self-import hack enables mocking during tests. // See src/editors/decisions/0005-internal-editor-testability-decisions.md. The whole approach to how hooks are tested @@ -102,7 +103,7 @@ export const initialize = (data) => (dispatch) => { dispatch(module.fetchCourseDetails()); break; case 'html': - if (data.learningContextId?.startsWith('lib:')) { + if (isLibraryKey(data.learningContextId)) { // eslint-disable-next-line no-console console.log('Not fetching image assets - not implemented yet for content libraries.'); } else { diff --git a/src/editors/data/redux/thunkActions/problem.test.js b/src/editors/data/redux/thunkActions/problem.test.ts similarity index 96% rename from src/editors/data/redux/thunkActions/problem.test.js rename to src/editors/data/redux/thunkActions/problem.test.ts index 828547b5b4..873ad0e8a9 100644 --- a/src/editors/data/redux/thunkActions/problem.test.js +++ b/src/editors/data/redux/thunkActions/problem.test.ts @@ -45,6 +45,9 @@ describe('problem thunkActions', () => { getState = jest.fn(() => ({ problem: { }, + app: { + learningContextId: 'course-v1:org+course+run', + }, })); }); @@ -52,7 +55,7 @@ describe('problem thunkActions', () => { jest.restoreAllMocks(); }); test('initializeProblem visual Problem :', () => { - initializeProblem(blockValue)(dispatch); + initializeProblem(blockValue)(dispatch, getState); expect(dispatch).toHaveBeenCalled(); }); test('switchToAdvancedEditor visual Problem', () => { diff --git a/src/editors/data/redux/thunkActions/problem.js b/src/editors/data/redux/thunkActions/problem.ts similarity index 72% rename from src/editors/data/redux/thunkActions/problem.js rename to src/editors/data/redux/thunkActions/problem.ts index 62de4f44a9..79916195e4 100644 --- a/src/editors/data/redux/thunkActions/problem.js +++ b/src/editors/data/redux/thunkActions/problem.ts @@ -1,16 +1,20 @@ import _ from 'lodash'; import { actions as problemActions } from '../problem'; +import { actions as requestActions } from '../requests'; +import { selectors as appSelectors } from '../app'; import * as requests from './requests'; +import { isLibraryKey } from '../../../../generic/key-utils'; import { OLXParser } from '../../../containers/ProblemEditor/data/OLXParser'; import { parseSettings } from '../../../containers/ProblemEditor/data/SettingsParser'; import { ProblemTypeKeys } from '../../constants/problem'; import ReactStateOLXParser from '../../../containers/ProblemEditor/data/ReactStateOLXParser'; -import { blankProblemOLX } from '../../../containers/ProblemEditor/data/mockData/olxTestData'; import { camelizeKeys } from '../../../utils'; import { fetchEditorContent } from '../../../containers/ProblemEditor/components/EditProblemView/hooks'; +import { RequestKeys } from '../../constants/requests'; // Similar to `import { actions, selectors } from '..';` but avoid circular imports: -const actions = { problem: problemActions }; +const actions = { problem: problemActions, requests: requestActions }; +const selectors = { app: appSelectors }; export const switchToAdvancedEditor = () => (dispatch, getState) => { const state = getState(); @@ -21,7 +25,7 @@ export const switchToAdvancedEditor = () => (dispatch, getState) => { }; export const isBlankProblem = ({ rawOLX }) => { - if (rawOLX.replace(/\s/g, '') === blankProblemOLX.rawOLX) { + if (['', ''].includes(rawOLX.replace(/\s/g, ''))) { return true; } return false; @@ -67,7 +71,7 @@ export const fetchAdvancedSettings = ({ rawOLX, rawSettings }) => (dispatch) => dispatch(requests.fetchAdvancedSettings({ onSuccess: (response) => { const defaultSettings = {}; - Object.entries(response.data).forEach(([key, value]) => { + Object.entries(response.data as Record).forEach(([key, value]) => { if (advancedProblemSettingKeys.includes(key)) { defaultSettings[key] = value.value; } @@ -79,10 +83,20 @@ export const fetchAdvancedSettings = ({ rawOLX, rawSettings }) => (dispatch) => })); }; -export const initializeProblem = (blockValue) => (dispatch) => { +export const initializeProblem = (blockValue) => (dispatch, getState) => { const rawOLX = _.get(blockValue, 'data.data', {}); const rawSettings = _.get(blockValue, 'data.metadata', {}); - dispatch(fetchAdvancedSettings({ rawOLX, rawSettings })); + const learningContextId = selectors.app.learningContextId(getState()); + if (isLibraryKey(learningContextId)) { + // Content libraries don't yet support defaults for fields like max_attempts, showanswer, etc. + // So proceed with loading the problem. + // Though first we need to fake the request or else the problem type selection UI won't display: + dispatch(actions.requests.completeRequest({ requestKey: RequestKeys.fetchAdvancedSettings, response: {} })); + dispatch(loadProblem({ rawOLX, rawSettings, defaultSettings: {} })); + } else { + // Load the defaults (for max_attempts, etc.) from the course's advanced settings, then proceed: + dispatch(fetchAdvancedSettings({ rawOLX, rawSettings })); + } }; export default { initializeProblem, switchToAdvancedEditor, fetchAdvancedSettings }; diff --git a/src/editors/data/services/cms/api.test.js b/src/editors/data/services/cms/api.test.ts similarity index 92% rename from src/editors/data/services/cms/api.test.js rename to src/editors/data/services/cms/api.test.ts index 7abec953cc..656fb4db49 100644 --- a/src/editors/data/services/cms/api.test.js +++ b/src/editors/data/services/cms/api.test.ts @@ -1,19 +1,7 @@ -/* eslint-disable no-import-assign */ -import * as utils from '../../../utils'; import * as api from './api'; -import * as mockApi from './mockApi'; import * as urls from './urls'; import { get, post, deleteObject } from './utils'; -jest.mock('../../../utils', () => { - const camelizeMap = (obj) => ({ ...obj, camelized: true }); - return { - ...jest.requireActual('../../../utils'), - camelize: camelizeMap, - camelizeKeys: (list) => list.map(camelizeMap), - }; -}); - jest.mock('./urls', () => ({ block: jest.fn().mockReturnValue('urls.block'), blockAncestor: jest.fn().mockReturnValue('urls.blockAncestor'), @@ -40,8 +28,6 @@ jest.mock('./utils', () => ({ deleteObject: jest.fn().mockName('deleteObject'), })); -const { camelize } = utils; - const { apiMethods } = api; const blockId = 'block-v1-coursev1:2uX@4345432'; @@ -200,7 +186,7 @@ describe('cms api', () => { licenseType: 'LiCeNsETYpe', licenseDetails: 'liCENSeDetAIls', }; - const html5Sources = 'hTML5souRCES'; + const html5Sources = ['hTML5souRCES']; const edxVideoId = 'eDXviDEOid'; const youtubeId = 'yOUtUBeid'; const license = 'LiCEnsE'; @@ -241,6 +227,7 @@ describe('cms api', () => { jest.restoreAllMocks(); }); test('throw error for invalid blockType', () => { + // @ts-expect-error because we're not passing 'blockId' or other parameters expect(() => { apiMethods.normalizeContent({ blockType: 'somethingINVALID' }); }) .toThrow(TypeError); }); @@ -271,7 +258,7 @@ describe('cms api', () => { }); describe('uploadAsset', () => { - const asset = { photo: 'dAta' }; + const asset = new Blob(['data'], { type: 'image/jpeg' }); it('should call post with urls.courseAssets and imgdata', () => { const mockFormdata = new FormData(); mockFormdata.append('file', asset); @@ -318,18 +305,17 @@ describe('cms api', () => { { id: ids[0], some: 'data' }, { id: ids[1], other: 'data' }, { id: ids[2], some: 'DATA' }, - { id: ids[3], other: 'DATA' }, + { id: ids[3], other_data: 'DATA' }, ]; - const oldLoadImage = api.loadImage; - api.loadImage = (imageData) => ({ loadImage: imageData }); + const spy = jest.spyOn(api, 'loadImage').mockImplementation((imageData) => ({ loadImage: imageData })); const out = api.loadImages(testData); expect(out).toEqual({ - [ids[0]]: api.loadImage(camelize(testData[0])), - [ids[1]]: api.loadImage(camelize(testData[1])), - [ids[2]]: api.loadImage(camelize(testData[2])), - [ids[3]]: api.loadImage(camelize(testData[3])), + [ids[0]]: api.loadImage(testData[0]), + [ids[1]]: api.loadImage(testData[1]), + [ids[2]]: api.loadImage(testData[2]), + [ids[3]]: api.loadImage({ id: ids[3], otherData: 'DATA' }), // Verify its 'other_data' key was camelized }); - api.loadImage = oldLoadImage; + spy.mockClear(); }); }); describe('uploadThumbnail', () => { @@ -382,7 +368,7 @@ describe('cms api', () => { }); }); describe('uploadTranscript', () => { - const transcript = { transcript: 'dAta' }; + const transcript = new Blob(['dAta']); it('should call post with urls.videoTranscripts and transcript data', () => { const mockFormdata = new FormData(); mockFormdata.append('file', transcript); @@ -606,31 +592,6 @@ describe('cms api', () => { expect(api.processLicense(licenseType, licenseDetails)).toEqual('all-rights-reserved'); }); }); - describe('checkMockApi', () => { - const envTemp = process.env; - beforeEach(() => { - jest.resetModules(); - process.env = { ...envTemp }; - }); - afterEach(() => { - process.env = envTemp; - }); - describe('if REACT_APP_DEVGALLERY is true', () => { - it('should return the mockApi version of a call when it exists', () => { - process.env.REACT_APP_DEVGALLERY = true; - expect(api.checkMockApi('fetchBlockById')).toEqual(mockApi.fetchBlockById); - }); - it('should return an empty mock when the call does not exist', () => { - process.env.REACT_APP_DEVGALLERY = true; - expect(api.checkMockApi('someRAnDomThINg')).toEqual(mockApi.emptyMock); - }); - }); - describe('if REACT_APP_DEVGALLERY is not true', () => { - it('should return the appropriate call', () => { - expect(api.checkMockApi('fetchBlockById')).toEqual(apiMethods.fetchBlockById); - }); - }); - }); describe('fetchVideoFeatures', () => { it('should call get with url.videoFeatures', () => { const args = { studioEndpointUrl }; diff --git a/src/editors/data/services/cms/api.js b/src/editors/data/services/cms/api.ts similarity index 83% rename from src/editors/data/services/cms/api.js rename to src/editors/data/services/cms/api.ts index 7d732c15d4..bce827b5ae 100644 --- a/src/editors/data/services/cms/api.js +++ b/src/editors/data/services/cms/api.ts @@ -1,15 +1,11 @@ +import type { AxiosRequestConfig } from 'axios'; import { camelizeKeys } from '../../../utils'; +import { isLibraryKey } from '../../../../generic/key-utils'; import * as urls from './urls'; import { get, post, deleteObject } from './utils'; -// This 'module' self-import hack enables mocking during tests. -// See src/editors/decisions/0005-internal-editor-testability-decisions.md. The whole approach to how hooks are tested -// should be re-thought and cleaned up to avoid this pattern. -// eslint-disable-next-line import/no-self-import -import * as module from './api'; -import * as mockApi from './mockApi'; import { durationStringFromValue } from '../../../containers/VideoEditor/components/VideoSettingsModal/components/DurationWidget/hooks'; -const fetchByUnitIdOptions = {}; +const fetchByUnitIdOptions: AxiosRequestConfig = {}; // For some reason, the local webpack-dev-server of library-authoring does not accept the normal Accept header. // This is a workaround only for that specific case; the idea is to only do this locally and only for library-authoring. @@ -19,6 +15,87 @@ if (process.env.NODE_ENV === 'development' && process.env.MFE_NAME === 'frontend }; } +interface Pagination { + start: number; + end: number; + page: number; + pageSize: number; + totalCount: number; +} + +interface AssetResponse { + assets: Record[]; // In the raw response here, these are NOT camel-cased yet. +} + +export const loadImage = (imageData) => ({ + ...imageData, + dateAdded: new Date(imageData.dateAdded.replace(' at', '')).getTime(), +}); + +export const loadImages = (rawImages) => camelizeKeys(rawImages).reduce( + (obj, image) => ({ ...obj, [image.id]: loadImage(image) }), + {}, +); + +export const parseYoutubeId = (src: string): string | null => { + const youtubeRegex = /^((?:https?:)?\/\/)?((?:www|m)\.)?((?:youtube\.com|youtu.be))(\/(?:[\w-]+\?v=|embed\/|v\/)?)([\w-]+)(\S+)?$/; + const match = src.match(youtubeRegex); + if (!match) { + return null; + } + return match[5]; +}; + +export const processVideoIds = ({ + videoId, + videoUrl, + fallbackVideos, +}: { videoId: string, videoUrl: string, fallbackVideos: string[] }) => { + let youtubeId: string | null = ''; + const html5Sources: string[] = []; + + if (videoUrl) { + if (parseYoutubeId(videoUrl)) { + youtubeId = parseYoutubeId(videoUrl); + } else { + html5Sources.push(videoUrl); + } + } + + if (fallbackVideos) { + fallbackVideos.forEach((src) => (src ? html5Sources.push(src) : null)); + } + + return { + edxVideoId: videoId, + html5Sources, + youtubeId, + }; +}; + +export const isEdxVideo = (src: string): boolean => { + const uuid4Regex = /^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$/; + if (src && src.match(uuid4Regex)) { + return true; + } + return false; +}; + +export const processLicense = (licenseType, licenseDetails) => { + if (licenseType === 'creative-commons') { + return 'creative-commons: ver=4.0'.concat( + (licenseDetails.attribution ? ' BY' : ''), + (licenseDetails.noncommercial ? ' NC' : ''), + (licenseDetails.noDerivatives ? ' ND' : ''), + (licenseDetails.shareAlike ? ' SA' : ''), + ); + } + if (licenseType === 'all-rights-reserved') { + return 'all-rights-reserved'; + } + return ''; +}; + export const apiMethods = { fetchBlockById: ({ blockId, studioEndpointUrl }) => get( urls.block({ blockId, studioEndpointUrl }), @@ -30,7 +107,19 @@ export const apiMethods = { fetchStudioView: ({ blockId, studioEndpointUrl }) => get( urls.blockStudioView({ studioEndpointUrl, blockId }), ), - fetchImages: ({ learningContextId, studioEndpointUrl, pageNumber }) => { + fetchImages: ({ + learningContextId, + studioEndpointUrl, + pageNumber, + }): Promise<{ data: AssetResponse & Pagination }> => { + if (isLibraryKey(learningContextId)) { + // V2 content libraries don't support static assets yet: + return Promise.resolve({ + data: { + assets: [], start: 0, end: 0, page: 0, pageSize: 50, totalCount: 0, + }, + }); + } const params = { asset_type: 'Images', page: pageNumber, @@ -150,6 +239,12 @@ export const apiMethods = { content, learningContextId, title, + }: { + blockId: string, + blockType: string, + content: any, // string for 'html' blocks, otherwise Record + learningContextId: string, + title: string, }) => { let response = {}; if (blockType === 'html') { @@ -175,7 +270,7 @@ export const apiMethods = { html5Sources, edxVideoId, youtubeId, - } = module.processVideoIds({ + } = processVideoIds({ videoId: content.videoId, videoUrl: content.videoSource, fallbackVideos: content.fallbackVideos, @@ -199,7 +294,7 @@ export const apiMethods = { handout: content.handout, start_time: durationStringFromValue(content.duration.startTime), end_time: durationStringFromValue(content.duration.stopTime), - license: module.processLicense(content.licenseType, content.licenseDetails), + license: processLicense(content.licenseType, content.licenseDetails), }, }; } else { @@ -216,7 +311,7 @@ export const apiMethods = { title, }) => post( urls.block({ studioEndpointUrl, blockId }), - module.apiMethods.normalizeContent({ + apiMethods.normalizeContent({ blockType, content, blockId, @@ -239,82 +334,4 @@ export const apiMethods = { ), }; -export const loadImage = (imageData) => ({ - ...imageData, - dateAdded: new Date(imageData.dateAdded.replace(' at', '')).getTime(), -}); - -export const loadImages = (rawImages) => camelizeKeys(rawImages).reduce( - (obj, image) => ({ ...obj, [image.id]: module.loadImage(image) }), - {}, -); - -export const processVideoIds = ({ - videoId, - videoUrl, - fallbackVideos, -}) => { - let youtubeId = ''; - const html5Sources = []; - - if (videoUrl) { - if (module.parseYoutubeId(videoUrl)) { - youtubeId = module.parseYoutubeId(videoUrl); - } else { - html5Sources.push(videoUrl); - } - } - - if (fallbackVideos) { - fallbackVideos.forEach((src) => (src ? html5Sources.push(src) : null)); - } - - return { - edxVideoId: videoId, - html5Sources, - youtubeId, - }; -}; - -export const isEdxVideo = (src) => { - const uuid4Regex = /^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$/; - if (src && src.match(uuid4Regex)) { - return true; - } - return false; -}; - -export const parseYoutubeId = (src) => { - const youtubeRegex = /^((?:https?:)?\/\/)?((?:www|m)\.)?((?:youtube\.com|youtu.be))(\/(?:[\w-]+\?v=|embed\/|v\/)?)([\w-]+)(\S+)?$/; - if (!src.match(youtubeRegex)) { - return null; - } - return src.match(youtubeRegex)[5]; -}; - -export const processLicense = (licenseType, licenseDetails) => { - if (licenseType === 'creative-commons') { - return 'creative-commons: ver=4.0'.concat( - (licenseDetails.attribution ? ' BY' : ''), - (licenseDetails.noncommercial ? ' NC' : ''), - (licenseDetails.noDerivatives ? ' ND' : ''), - (licenseDetails.shareAlike ? ' SA' : ''), - ); - } - if (licenseType === 'all-rights-reserved') { - return 'all-rights-reserved'; - } - return ''; -}; - -export const checkMockApi = (key) => { - if (process.env.REACT_APP_DEVGALLERY) { - return mockApi[key] ? mockApi[key] : mockApi.emptyMock; - } - return module.apiMethods[key]; -}; - -export default Object.keys(apiMethods).reduce( - (obj, key) => ({ ...obj, [key]: checkMockApi(key) }), - {}, -); +export default apiMethods; diff --git a/src/editors/data/services/cms/mockApi.js b/src/editors/data/services/cms/mockApi.js deleted file mode 100644 index ca50e0c0a6..0000000000 --- a/src/editors/data/services/cms/mockApi.js +++ /dev/null @@ -1,297 +0,0 @@ -/* istanbul ignore file */ -import * as urls from './urls'; - -const mockPromise = (returnValue) => new Promise(resolve => { resolve(returnValue); }); - -// TODO: update to return block data appropriate per block ID, which will equal block type -// eslint-disable-next-line -export const fetchBlockById = ({ blockId, studioEndpointUrl }) => { - let data = {}; - if (blockId === 'html-block-id') { - data = { - data: ` - `, - display_name: 'My Text Prompt', - metadata: { - display_name: 'Welcome!', - download_track: true, - download_video: true, - edx_video_id: 'f36f06b5-92e5-47c7-bb26-bcf986799cb7', - html5_sources: [ - 'https://www.youtube.com/watch?v=dQw4w9WgXcQ', - 'https://www.youtube.com/watch?v=dQw4w9WgXcQ', - ], - show_captions: true, - sub: '', - track: '', - transcripts: { - en: { filename: 'my-transcript-url' }, - }, - xml_attributes: { - source: '', - }, - youtube_id_1_0: 'dQw4w9WgXcQ', - }, - }; - } else if (blockId === 'problem-block-id') { - data = { - data: ` - `, - display_name: 'Dropdown', - metadata: { - markdown: `You can use this template as a guide to the simple editor markdown and OLX markup to use for dropdown problems. Edit this component to replace this template with your own assessment. - >>Add the question text, or prompt, here. This text is required.||You can add an optional tip or note related to the prompt like this. << - [[ - an incorrect answer - (the correct answer) - an incorrect answer - ]]`, - attempts_before_showanswer_button: 7, - max_attempts: 5, - show_reset_button: true, - showanswer: 'after_attempts', - submission_wait_seconds: 15, - weight: 29, - }, - }; - } else if (blockId === 'game-block-id') { - data = { - display_name: 'Game Block', - // TODO: insert mock data from backend here - }; - } - return mockPromise({ data: { ...data } }); -}; - -// TODO: update to return block data appropriate per block ID, which will equal block type -// eslint-disable-next-line -export const fetchByUnitId = ({ blockId, studioEndpointUrl }) => mockPromise({ - data: { ancestors: [{ id: 'unitUrl' }] }, -}); -// eslint-disable-next-line -export const fetchImages = ({ learningContextId, studioEndpointUrl }) => mockPromise({ - data: { - assets: [ - { - displayName: 'shahrukh.jpg', - contentType: 'image/jpeg', - dateAdded: 'Jan 05, 2022 at 17:38 UTC', - url: '/asset-v1:edX+test101+2021_T1+type@asset+block@shahrukh.jpg', - externalUrl: 'https://courses.edx.org/asset-v1:edX+test101+2021_T1+type@asset+block@shahrukh.jpg', - portableUrl: '/static/shahrukh.jpg', - thumbnail: '/asset-v1:edX+test101+2021_T1+type@thumbnail+block@shahrukh.jpg', - locked: false, - id: 'asset-v1:edX+test101+2021_T1+type@asset+block@shahrukh.jpg', - }, - { - displayName: 'IMG_5899.jpg', - contentType: 'image/jpeg', - dateAdded: 'Nov 16, 2021 at 18:55 UTC', - url: '/asset-v1:edX+test101+2021_T1+type@asset+block@IMG_5899.jpg', - externalUrl: 'https://courses.edx.org/asset-v1:edX+test101+2021_T1+type@asset+block@IMG_5899.jpg', - portableUrl: '/static/IMG_5899.jpg', - thumbnail: '/asset-v1:edX+test101+2021_T1+type@thumbnail+block@IMG_5899.jpg', - locked: false, - id: 'asset-v1:edX+test101+2021_T1+type@asset+block@IMG_5899.jpg', - }, - { - displayName: 'ccexample.srt', - contentType: 'application/octet-stream', - dateAdded: 'Nov 01, 2021 at 15:42 UTC', - url: '/asset-v1:edX+test101+2021_T1+type@asset+block@ccexample.srt', - externalUrl: 'https://courses.edx.org/asset-v1:edX+test101+2021_T1+type@asset+block@ccexample.srt', - portableUrl: '/static/ccexample.srt', - thumbnail: null, - locked: false, - id: 'asset-v1:edX+test101+2021_T1+type@asset+block@ccexample.srt', - }, - { - displayName: 'Tennis Ball.jpeg', - contentType: 'image/jpeg', - dateAdded: 'Aug 04, 2021 at 16:52 UTC', - url: '/asset-v1:edX+test101+2021_T1+type@asset+block@Tennis_Ball.jpeg', - externalUrl: 'https://courses.edx.org/asset-v1:edX+test101+2021_T1+type@asset+block@Tennis_Ball.jpeg', - portableUrl: '/static/Tennis_Ball.jpeg', - thumbnail: '/asset-v1:edX+test101+2021_T1+type@thumbnail+block@Tennis_Ball-jpeg.jpg', - locked: false, - id: 'asset-v1:edX+test101+2021_T1+type@asset+block@Tennis_Ball.jpeg', - }, - ], - }, -}); -// eslint-disable-next-line -export const fetchCourseDetails = ({ studioEndpointUrl, learningContextId }) => mockPromise({ - data: { - // license: "creative-commons: ver=4.0 BY NC", - license: 'all-rights-reserved', - }, -}); -// eslint-disable-next-line -export const checkTranscripts = ({youTubeId, studioEndpointUrl, blockId, videoId}) => mockPromise({ - data: { - command: 'import', - }, -}); -// eslint-disable-next-line -export const importTranscript = ({youTubeId, studioEndpointUrl, blockId}) => mockPromise({ - data: { - edx_video_id: 'f36f06b5-92e5-47c7-bb26-bcf986799cb7', - }, -}); -// eslint-disable-next-line -export const fetchAdvanceSettings = ({ studioEndpointUrl, learningContextId }) => mockPromise({ - data: { allow_unsupported_xblocks: { value: true } }, -}); -// eslint-disable-next-line -export const fetchVideoFeatures = ({ studioEndpointUrl }) => mockPromise({ - data: { - allowThumbnailUpload: true, - videoSharingEnabledForCourse: true, - }, -}); - -export const normalizeContent = ({ - blockId, - blockType, - content, - learningContextId, - title, -}) => { - let response = {}; - if (blockType === 'html') { - response = { - category: blockType, - couseKey: learningContextId, - data: content, - has_changes: true, - id: blockId, - metadata: { display_name: title }, - }; - } else if (blockType === 'problem') { - response = { - data: content.olx, - category: blockType, - couseKey: learningContextId, - has_changes: true, - id: blockId, - metadata: { display_name: title, ...content.settings }, - }; - } else { - throw new TypeError(`No Block in V2 Editors named /"${blockType}/", Cannot Save Content.`); - } - return { ...response }; -}; - -export const saveBlock = ({ - blockId, - blockType, - content, - learningContextId, - studioEndpointUrl, - title, -}) => mockPromise({ - url: urls.block({ studioEndpointUrl, blockId }), - content: normalizeContent({ - blockType, - content, - blockId, - learningContextId, - title, - }), -}); - -export const uploadAsset = ({ - learningContextId, - studioEndpointUrl, - // image, -}) => mockPromise({ - url: urls.courseAssets({ studioEndpointUrl, learningContextId }), - asset: { - asset: { - display_name: 'journey_escape.jpg', - content_type: 'image/jpeg', - date_added: 'Jan 05, 2022 at 21:26 UTC', - url: '/asset-v1:edX+test101+2021_T1+type@asset+block@journey_escape.jpg', - external_url: 'https://courses.edx.org/asset-v1:edX+test101+2021_T1+type@asset+block@journey_escape.jpg', - portable_url: '/static/journey_escape.jpg', - thumbnail: '/asset-v1:edX+test101+2021_T1+type@thumbnail+block@journey_escape.jpg', - locked: false, - id: 'asset-v1:edX+test101+2021_T1+type@asset+block@journey_escape.jpg', - }, - msg: 'Upload completed', - }, -}); - -// TODO: update to return block data appropriate per block ID, which will equal block type -// eslint-disable-next-line -export const fetchStudioView = ({ blockId, studioEndpointUrl }) => { - let data = {}; - if (blockId === 'html-block-id') { - data = { - data: '

Test prompt content

', - display_name: 'My Text Prompt', - metadata: { - display_name: 'Welcome!', - download_track: true, - download_video: true, - edx_video_id: 'f36f06b5-92e5-47c7-bb26-bcf986799cb7', - html5_sources: [ - 'https://www.youtube.com/watch?v=dQw4w9WgXcQ', - 'https://www.youtube.com/watch?v=dQw4w9WgXcQ', - ], - show_captions: true, - sub: '', - track: '', - transcripts: { - en: { filename: 'my-transcript-url' }, - }, - xml_attributes: { - source: '', - }, - youtube_id_1_0: 'dQw4w9WgXcQ', - }, - }; - } else if (blockId === 'problem-block-id') { - data = { - data: ` - -

You can use this template as a guide to the simple editor markdown and OLX markup to use for dropdown problems. Edit this component to replace this template with your own assessment.

- - You can add an optional tip or note related to the prompt like this. - - - - - -
-
`, - display_name: 'Dropdown', - metadata: { - markdown: `You can use this template as a guide to the simple editor markdown and OLX markup to use for dropdown problems. Edit this component to replace this template with your own assessment. - >>Add the question text, or prompt, here. This text is required.||You can add an optional tip or note related to the prompt like this. << - [[ - an incorrect answer - (the correct answer) - an incorrect answer - ]]`, - attempts_before_showanswer_button: 7, - max_attempts: 5, - rerandomize: 'per_student', - show_reset_button: true, - showanswer: 'after_attempts', - submission_wait_seconds: 15, - weight: 29, - }, - }; - } - - return mockPromise({ - data: { - // The following is sent for 'raw' editors. - html: blockId.includes('mockRaw') ? 'data-editor="raw"' : '', - ...data, - }, - }); -}; - -export const emptyMock = () => mockPromise({}); diff --git a/src/editors/data/services/cms/types.js b/src/editors/data/services/cms/types.ts similarity index 94% rename from src/editors/data/services/cms/types.js rename to src/editors/data/services/cms/types.ts index f35ec2441d..f093b12359 100644 --- a/src/editors/data/services/cms/types.js +++ b/src/editors/data/services/cms/types.ts @@ -1,6 +1,6 @@ import PropTypes from 'prop-types'; -import { ProblemTypes, ShowAnswerTypes } from '../../constants/problem'; +/* export const videoDataProps = { videoSource: PropTypes.string, videoId: PropTypes.string, @@ -25,6 +25,7 @@ export const videoDataProps = { shareAlike: PropTypes.bool, }), }; +*/ export const answerOptionProps = PropTypes.shape({ id: PropTypes.string, @@ -35,6 +36,7 @@ export const answerOptionProps = PropTypes.shape({ unselectedFeedback: PropTypes.string, }); +/* export const problemDataProps = { rawOLX: PropTypes.string, problemType: PropTypes.instanceOf(ProblemTypes), @@ -68,9 +70,8 @@ export const problemDataProps = { }), }), }; +*/ export default { - videoDataProps, - problemDataProps, answerOptionProps, }; diff --git a/src/editors/data/services/cms/urls.test.js b/src/editors/data/services/cms/urls.test.ts similarity index 95% rename from src/editors/data/services/cms/urls.test.js rename to src/editors/data/services/cms/urls.test.ts index 94b39828c7..95a3968959 100644 --- a/src/editors/data/services/cms/urls.test.js +++ b/src/editors/data/services/cms/urls.test.ts @@ -44,7 +44,9 @@ describe('cms url methods', () => { }, }; it('returns the library page when given the v1 library', () => { - expect(returnUrl({ studioEndpointUrl, unitUrl, learningContextId: libraryLearningContextId })) + expect(returnUrl({ + studioEndpointUrl, unitUrl, learningContextId: libraryLearningContextId, blockId: libraryV1Id, + })) .toEqual(`${studioEndpointUrl}/library/${libraryLearningContextId}`); }); // it('throws error when given the v2 library', () => { @@ -52,8 +54,9 @@ describe('cms url methods', () => { // .toThrow('Return url not available (or needed) for V2 libraries'); // }); it('returns empty url when given the v2 library', () => { - expect(returnUrl({ studioEndpointUrl, unitUrl, learningContextId: libraryV2Id })) - .toEqual(''); + expect(returnUrl({ + studioEndpointUrl, unitUrl, learningContextId: libraryV2Id, blockId: libraryV2Id, + })).toEqual(''); }); it('returnUrl function should return url with studioEndpointUrl, unitUrl, and blockId', () => { expect(returnUrl({ @@ -68,7 +71,9 @@ describe('cms url methods', () => { .toEqual(''); }); it('throws error if no unit url', () => { - expect(returnUrl({ studioEndpointUrl, unitUrl: null, learningContextId: courseId })) + expect(returnUrl({ + studioEndpointUrl, unitUrl: null, learningContextId: courseId, blockId, + })) .toEqual(''); }); it('returns the library page when given the library', () => { diff --git a/src/editors/data/services/cms/urls.js b/src/editors/data/services/cms/urls.ts similarity index 54% rename from src/editors/data/services/cms/urls.js rename to src/editors/data/services/cms/urls.ts index d101ff9241..3618499915 100644 --- a/src/editors/data/services/cms/urls.js +++ b/src/editors/data/services/cms/urls.ts @@ -1,3 +1,11 @@ +import { isLibraryKey, isLibraryV1Key } from '../../../../generic/key-utils'; + +/** + * A little helper so we can write the types of these functions more compactly + * The main purpose of this is to indicate the params are all strings. + */ +type UrlFunction = (args: Record) => string; + export const libraryV1 = ({ studioEndpointUrl, learningContextId }) => ( `${studioEndpointUrl}/library/${learningContextId}` ); @@ -8,12 +16,14 @@ export const unit = ({ studioEndpointUrl, unitUrl, blockId }) => ( export const returnUrl = ({ studioEndpointUrl, unitUrl, learningContextId, blockId, -}) => { - if (learningContextId && learningContextId.startsWith('library-v1')) { +}): string => { + // Is this a v1 library? + if (isLibraryV1Key(learningContextId)) { // when the learning context is a v1 library, return to the library page return libraryV1({ studioEndpointUrl, learningContextId }); } - if (learningContextId && learningContextId.startsWith('lib')) { + // Is this a v2 library? + if (isLibraryKey(learningContextId)) { // when it's a v2 library, there will be no return url (instead a closed popup) // (temporary) don't throw error, just return empty url. it will fail it's network connection but otherwise // the app will run @@ -28,69 +38,69 @@ export const returnUrl = ({ return ''; }; -export const block = ({ studioEndpointUrl, blockId }) => ( +export const block = (({ studioEndpointUrl, blockId }) => ( blockId.startsWith('lb:') ? `${studioEndpointUrl}/api/xblock/v2/xblocks/${blockId}/fields/` : `${studioEndpointUrl}/xblock/${blockId}` -); +)) satisfies UrlFunction; -export const blockAncestor = ({ studioEndpointUrl, blockId }) => { +export const blockAncestor = (({ studioEndpointUrl, blockId }) => { if (blockId.includes('block-v1')) { return `${block({ studioEndpointUrl, blockId })}?fields=ancestorInfo`; } throw new Error('Block ancestor not available (and not needed) for V2 blocks'); -}; +}) satisfies UrlFunction; -export const blockStudioView = ({ studioEndpointUrl, blockId }) => ( +export const blockStudioView = (({ studioEndpointUrl, blockId }) => ( blockId.includes('block-v1') ? `${block({ studioEndpointUrl, blockId })}/studio_view` : `${studioEndpointUrl}/api/xblock/v2/xblocks/${blockId}/view/studio_view/` -); +)) satisfies UrlFunction; -export const courseAssets = ({ studioEndpointUrl, learningContextId }) => ( +export const courseAssets = (({ studioEndpointUrl, learningContextId }) => ( `${studioEndpointUrl}/assets/${learningContextId}/` -); +)) satisfies UrlFunction; -export const thumbnailUpload = ({ studioEndpointUrl, learningContextId, videoId }) => ( +export const thumbnailUpload = (({ studioEndpointUrl, learningContextId, videoId }) => ( `${studioEndpointUrl}/video_images/${learningContextId}/${videoId}` -); +)) satisfies UrlFunction; -export const videoTranscripts = ({ studioEndpointUrl, blockId }) => ( +export const videoTranscripts = (({ studioEndpointUrl, blockId }) => ( `${block({ studioEndpointUrl, blockId })}/handler/studio_transcript/translation` -); +)) satisfies UrlFunction; -export const downloadVideoTranscriptURL = ({ studioEndpointUrl, blockId, language }) => ( +export const downloadVideoTranscriptURL = (({ studioEndpointUrl, blockId, language }) => ( `${videoTranscripts({ studioEndpointUrl, blockId })}?language_code=${language}` -); +)) satisfies UrlFunction; -export const mediaTranscriptURL = ({ studioEndpointUrl, transcriptUrl }) => ( +export const mediaTranscriptURL = (({ studioEndpointUrl, transcriptUrl }) => ( `${studioEndpointUrl}${transcriptUrl}` -); +)) satisfies UrlFunction; -export const downloadVideoHandoutUrl = ({ studioEndpointUrl, handout }) => ( +export const downloadVideoHandoutUrl = (({ studioEndpointUrl, handout }) => ( `${studioEndpointUrl}${handout}` -); +)) satisfies UrlFunction; -export const courseDetailsUrl = ({ studioEndpointUrl, learningContextId }) => ( +export const courseDetailsUrl = (({ studioEndpointUrl, learningContextId }) => ( `${studioEndpointUrl}/settings/details/${learningContextId}` -); +)) satisfies UrlFunction; -export const checkTranscriptsForImport = ({ studioEndpointUrl, parameters }) => ( +export const checkTranscriptsForImport = (({ studioEndpointUrl, parameters }) => ( `${studioEndpointUrl}/transcripts/check?data=${parameters}` -); +)) satisfies UrlFunction; -export const replaceTranscript = ({ studioEndpointUrl, parameters }) => ( +export const replaceTranscript = (({ studioEndpointUrl, parameters }) => ( `${studioEndpointUrl}/transcripts/replace?data=${parameters}` -); +)) satisfies UrlFunction; -export const courseAdvanceSettings = ({ studioEndpointUrl, learningContextId }) => ( +export const courseAdvanceSettings = (({ studioEndpointUrl, learningContextId }) => ( `${studioEndpointUrl}/api/contentstore/v0/advanced_settings/${learningContextId}` -); +)) satisfies UrlFunction; -export const videoFeatures = ({ studioEndpointUrl }) => ( +export const videoFeatures = (({ studioEndpointUrl }) => ( `${studioEndpointUrl}/video_features/` -); +)) satisfies UrlFunction; -export const courseVideos = ({ studioEndpointUrl, learningContextId }) => ( +export const courseVideos = (({ studioEndpointUrl, learningContextId }) => ( `${studioEndpointUrl}/videos/${learningContextId}` -); +)) satisfies UrlFunction; diff --git a/src/editors/data/services/cms/utils.test.js b/src/editors/data/services/cms/utils.test.ts similarity index 91% rename from src/editors/data/services/cms/utils.test.js rename to src/editors/data/services/cms/utils.test.ts index 19aef7a360..868c8a63b6 100644 --- a/src/editors/data/services/cms/utils.test.js +++ b/src/editors/data/services/cms/utils.test.ts @@ -10,7 +10,7 @@ describe('cms service utils', () => { it('forwards arguments to authenticatedHttpClient().get', () => { const get = jest.fn((...args) => ({ get: args })); getAuthenticatedHttpClient.mockReturnValue({ get }); - const args = ['some', 'args', 'for', 'the', 'test']; + const args = ['url 1', { headers: {} }] as const; expect(utils.get(...args)).toEqual(get(...args)); }); }); @@ -18,7 +18,7 @@ describe('cms service utils', () => { it('forwards arguments to authenticatedHttpClient().post', () => { const post = jest.fn((...args) => ({ post: args })); getAuthenticatedHttpClient.mockReturnValue({ post }); - const args = ['some', 'args', 'for', 'the', 'test']; + const args = ['url 2', { headers: {} }] as const; expect(utils.post(...args)).toEqual(post(...args)); }); }); diff --git a/src/editors/data/services/cms/utils.js b/src/editors/data/services/cms/utils.ts similarity index 63% rename from src/editors/data/services/cms/utils.js rename to src/editors/data/services/cms/utils.ts index ec9c567417..b7d6276fe7 100644 --- a/src/editors/data/services/cms/utils.js +++ b/src/editors/data/services/cms/utils.ts @@ -1,24 +1,25 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import type { Axios } from 'axios'; + +export const client: () => Axios = getAuthenticatedHttpClient; /** * get(url) * simple wrapper providing an authenticated Http client get action * @param {string} url - target url */ -export const get = (...args) => getAuthenticatedHttpClient().get(...args); +export const get: Axios['get'] = (...args) => client().get(...args); /** * post(url, data) * simple wrapper providing an authenticated Http client post action * @param {string} url - target url * @param {object|string} data - post payload */ -export const post = (...args) => getAuthenticatedHttpClient().post(...args); +export const post: Axios['post'] = (...args) => client().post(...args); /** * delete(url, data) * simple wrapper providing an authenticated Http client delete action * @param {string} url - target url * @param {object|string} data - delete payload */ -export const deleteObject = (...args) => getAuthenticatedHttpClient().delete(...args); - -export const client = getAuthenticatedHttpClient; +export const deleteObject: Axios['delete'] = (...args) => client().delete(...args); diff --git a/src/editors/sharedComponents/ErrorBoundary/ErrorPage.jsx b/src/editors/sharedComponents/ErrorBoundary/ErrorPage.jsx index e00552b1aa..f4c32c13d1 100644 --- a/src/editors/sharedComponents/ErrorBoundary/ErrorPage.jsx +++ b/src/editors/sharedComponents/ErrorBoundary/ErrorPage.jsx @@ -1,4 +1,3 @@ -import React from 'react'; import { connect } from 'react-redux'; import PropTypes from 'prop-types'; import { @@ -7,6 +6,7 @@ import { import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import messages from './messages'; +import { isLibraryV1Key } from '../../../generic/key-utils'; import { navigateTo } from '../../hooks'; import { selectors } from '../../data/redux'; @@ -23,7 +23,7 @@ const ErrorPage = ({ // injected intl, }) => { - const outlineType = learningContextId?.startsWith('library-v1') ? 'library' : 'course'; + const outlineType = isLibraryV1Key(learningContextId) ? 'library' : 'course'; const outlineUrl = `${studioEndpointUrl}/${outlineType}/${learningContextId}`; const unitUrl = unitData?.data ? `${studioEndpointUrl}/container/${unitData?.data.ancestors[0].id}` : null; diff --git a/src/editors/utils/index.js b/src/editors/utils/index.ts similarity index 100% rename from src/editors/utils/index.js rename to src/editors/utils/index.ts diff --git a/src/generic/key-utils.test.ts b/src/generic/key-utils.test.ts new file mode 100644 index 0000000000..d3763d27cd --- /dev/null +++ b/src/generic/key-utils.test.ts @@ -0,0 +1,78 @@ +import { + getBlockType, + getLibraryId, + isLibraryKey, + isLibraryV1Key, +} from './key-utils'; + +describe('component utils', () => { + describe('getBlockType', () => { + for (const [input, expected] of [ + ['lb:org:lib:html:id', 'html'], + ['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'html'], + ['lb:Axim:beta:problem:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'problem'], + ]) { + it(`returns '${expected}' for usage key '${input}'`, () => { + expect(getBlockType(input)).toStrictEqual(expected); + }); + } + + for (const input of ['', undefined, null, 'not a key', 'lb:foo']) { + it(`throws an exception for usage key '${input}'`, () => { + expect(() => getBlockType(input as any)).toThrow(`Invalid usageKey: ${input}`); + }); + } + }); + + describe('getLibraryId', () => { + for (const [input, expected] of [ + ['lb:org:lib:html:id', 'lib:org:lib'], + ['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'lib:OpenCraftX:ALPHA'], + ['lb:Axim:beta:problem:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'lib:Axim:beta'], + ]) { + it(`returns '${expected}' for usage key '${input}'`, () => { + expect(getLibraryId(input)).toStrictEqual(expected); + }); + } + + for (const input of ['', undefined, null, 'not a key', 'lb:foo']) { + it(`throws an exception for usage key '${input}'`, () => { + expect(() => getLibraryId(input as any)).toThrow(`Invalid usageKey: ${input}`); + }); + } + }); + + describe('isLibraryKey', () => { + for (const [input, expected] of [ + ['lib:org:lib', true], + ['lib:OpenCraftX:ALPHA', true], + ['lb:org:lib:html:id', false], + ['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', false], + ['library-v1:AximX+L1', false], + ['course-v1:AximX+TS100+23', false], + ['', false], + [undefined, false], + ] as const) { + it(`returns '${expected}' for learning context key '${input}'`, () => { + expect(isLibraryKey(input)).toStrictEqual(expected); + }); + } + }); + + describe('isLibraryV1Key', () => { + for (const [input, expected] of [ + ['library-v1:AximX+L1', true], + ['lib:org:lib', false], + ['lib:OpenCraftX:ALPHA', false], + ['lb:org:lib:html:id', false], + ['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', false], + ['course-v1:AximX+TS100+23', false], + ['', false], + [undefined, false], + ] as const) { + it(`returns '${expected}' for learning context key '${input}'`, () => { + expect(isLibraryV1Key(input)).toStrictEqual(expected); + }); + } + }); +}); diff --git a/src/generic/key-utils.ts b/src/generic/key-utils.ts new file mode 100644 index 0000000000..103396fda6 --- /dev/null +++ b/src/generic/key-utils.ts @@ -0,0 +1,40 @@ +/** + * Given a usage key like `lb:org:lib:html:id`, get the type (e.g. `html`) + * @param usageKey e.g. `lb:org:lib:html:id` + * @returns The block type as a string + */ +export function getBlockType(usageKey: string): string { + if (usageKey && usageKey.startsWith('lb:')) { + const blockType = usageKey.split(':')[3]; + if (blockType) { + return blockType; + } + } + throw new Error(`Invalid usageKey: ${usageKey}`); +} + +/** + * Given a usage key like `lb:org:lib:html:id`, get the library key + * @param usageKey e.g. `lb:org:lib:html:id` + * @returns The library key, e.g. `lib:org:lib` + */ +export function getLibraryId(usageKey: string): string { + if (usageKey && usageKey.startsWith('lb:')) { + const org = usageKey.split(':')[1]; + const lib = usageKey.split(':')[2]; + if (org && lib) { + return `lib:${org}:${lib}`; + } + } + throw new Error(`Invalid usageKey: ${usageKey}`); +} + +/** Check if this is a V2 library key. */ +export function isLibraryKey(learningContextKey: string | undefined): learningContextKey is string { + return typeof learningContextKey === 'string' && learningContextKey.startsWith('lib:'); +} + +/** Check if this is a V1 library key. */ +export function isLibraryV1Key(learningContextKey: string | undefined): learningContextKey is string { + return typeof learningContextKey === 'string' && learningContextKey.startsWith('library-v1:'); +} diff --git a/src/library-authoring/LibraryLayout.tsx b/src/library-authoring/LibraryLayout.tsx index 5d316a4aa6..59ee11797c 100644 --- a/src/library-authoring/LibraryLayout.tsx +++ b/src/library-authoring/LibraryLayout.tsx @@ -27,21 +27,34 @@ const LibraryLayout = () => { const goBack = React.useCallback(() => { // Go back to the library navigate(`/library/${libraryId}`); - // The following function is called only if changes are saved: - return ({ id: usageKey }) => { + }, []); + const returnFunction = React.useCallback(() => { + // When changes are cancelled, either onClose (goBack) or this returnFunction will be called. + // When changes are saved, this returnFunction is called. + goBack(); + return (args) => { + if (args === undefined) { + return; // Do nothing - the user cancelled the changes + } + const { id: usageKey } = args; // invalidate any queries that involve this XBlock: invalidateComponentData(queryClient, libraryId, usageKey); }; - }, []); + }, [goBack]); return ( + {/* + TODO: we should be opening this editor as a modal, not making it a separate page/URL. + That will be a much nicer UX because users can just close the modal and be on the same page they were already + on, instead of always getting sent back to the library home. + */} - + )} /> diff --git a/src/library-authoring/__mocks__/libraryComponentsMock.ts b/src/library-authoring/__mocks__/libraryComponentsMock.ts index 6e1db6a719..48a920392a 100644 --- a/src/library-authoring/__mocks__/libraryComponentsMock.ts +++ b/src/library-authoring/__mocks__/libraryComponentsMock.ts @@ -2,8 +2,9 @@ export default [ { id: '1', usageKey: 'lb:org:lib:html:1', - displayName: 'Text', + displayName: 'Text Component 1', formatted: { + displayName: 'Text Component 1', content: { htmlContent: 'This is a text: ID=1', }, @@ -11,13 +12,14 @@ export default [ tags: { level0: ['1', '2', '3'], }, - blockType: 'text', + blockType: 'html', }, { id: '2', usageKey: 'lb:org:lib:html:2', - displayName: 'Text', + displayName: 'Text Component 2', formatted: { + displayName: 'Text Component 2', content: { htmlContent: 'This is a text: ID=2', }, @@ -25,15 +27,15 @@ export default [ tags: { level0: ['1', '2', '3'], }, - blockType: 'text', + blockType: 'html', }, { id: '3', usageKey: 'lb:org:lib:video:3', - displayName: 'Video', + displayName: 'Video Component 3', formatted: { + displayName: 'Video Component 3', content: { - htmlContent: 'This is a video: ID=3', }, }, tags: { @@ -44,24 +46,24 @@ export default [ { id: '4', usageKey: 'lb:org:lib:video:4', - displayName: 'Video', + displayName: 'Video Component 4', formatted: { - content: { - htmlContent: 'This is a video: ID=4', - }, + displayName: 'Video Component 4', + content: {}, }, tags: { level0: ['1', '2'], }, - blockType: 'text', + blockType: 'video', }, { id: '5', usageKey: 'lb:org:lib:problem:5', displayName: 'Problem', formatted: { + displayName: 'Problem', content: { - htmlContent: 'This is a problem: ID=5', + capaContent: 'This is a problem: ID=5', }, }, blockType: 'problem', @@ -71,8 +73,9 @@ export default [ usageKey: 'lb:org:lib:problem:6', displayName: 'Problem', formatted: { + displayName: 'Problem', content: { - htmlContent: 'This is a problem: ID=6', + capaContent: 'This is a problem: ID=6', }, }, blockType: 'problem', diff --git a/src/library-authoring/add-content/AddContentWorkflow.test.tsx b/src/library-authoring/add-content/AddContentWorkflow.test.tsx index 5a77c92953..7bc17321ad 100644 --- a/src/library-authoring/add-content/AddContentWorkflow.test.tsx +++ b/src/library-authoring/add-content/AddContentWorkflow.test.tsx @@ -85,20 +85,55 @@ describe('AddContentWorkflow test', () => { }); it('can create a Problem component', async () => { - const { mockShowToast } = initializeMocks(); + initializeMocks(); render(, renderOpts); // Click "New [Component]" const newComponentButton = await screen.findByRole('button', { name: /New/ }); fireEvent.click(newComponentButton); - // Pre-condition - this is NOT shown yet: - expect(screen.queryByText('Content created successfully.')).not.toBeInTheDocument(); - // Click "Problem" to create a capa problem component fireEvent.click(await screen.findByRole('button', { name: /Problem/ })); - // We haven't yet implemented the problem editor, so we expect only a toast to appear + // Then the editor should open + expect(await screen.findByRole('heading', { name: /Select problem type/ })).toBeInTheDocument(); + + // Select the type: Numerical Input + fireEvent.click(await screen.findByRole('button', { name: 'Numerical input' })); + fireEvent.click(screen.getByRole('button', { name: 'Select' })); + + expect(await screen.findByRole('heading', { name: /Numerical input/ })).toBeInTheDocument(); + + // Enter an answer value: + const inputA = await screen.findByPlaceholderText('Enter an answer'); + fireEvent.change(inputA, { target: { value: '123456' } }); + + // Mock the save() REST API method: + const saveSpy = jest.spyOn(editorCmsApi as any, 'saveBlock').mockImplementationOnce(async () => ({ + status: 200, data: { id: mockXBlockFields.usageKeyNewProblem }, + })); + + // Click Save + const saveButton = screen.getByLabelText('Save changes and return to learning context'); + fireEvent.click(saveButton); + expect(saveSpy).toHaveBeenCalledTimes(2); // TODO: why is this called twice? + }); + + it('can create a Video component', async () => { + const { mockShowToast } = initializeMocks(); + render(, renderOpts); + + // Click "New [Component]" + const newComponentButton = await screen.findByRole('button', { name: /New/ }); + fireEvent.click(newComponentButton); + + // Pre-condition - the success toast is NOT shown yet: + expect(mockShowToast).not.toHaveBeenCalled(); + + // Click "Video" to create a video component + fireEvent.click(await screen.findByRole('button', { name: /Video/ })); + + // We haven't yet implemented the video editor, so we expect only a toast to appear await waitFor(() => expect(mockShowToast).toHaveBeenCalledWith('Content created successfully.')); }); }); diff --git a/src/library-authoring/components/ComponentCard.test.tsx b/src/library-authoring/components/ComponentCard.test.tsx index cb800e762e..dae278eba7 100644 --- a/src/library-authoring/components/ComponentCard.test.tsx +++ b/src/library-authoring/components/ComponentCard.test.tsx @@ -34,7 +34,7 @@ const contentHit: ContentHit = { tags: { level0: ['1', '2', '3'], }, - blockType: 'text', + blockType: 'html', created: 1722434322294, modified: 1722434322294, lastPublished: null, diff --git a/src/library-authoring/components/ComponentCard.tsx b/src/library-authoring/components/ComponentCard.tsx index 6978fd6d9d..39a23926fc 100644 --- a/src/library-authoring/components/ComponentCard.tsx +++ b/src/library-authoring/components/ComponentCard.tsx @@ -74,7 +74,11 @@ const ComponentCard = ({ contentHit, blockTypeDisplayName } : ComponentCardProps tags, usageKey, } = contentHit; - const description = formatted?.content?.htmlContent ?? ''; + const description: string = (/* eslint-disable */ + blockType === 'html' ? formatted?.content?.htmlContent : + blockType === 'problem' ? formatted?.content?.capaContent : + undefined + ) ?? '';/* eslint-enable */ const displayName = formatted?.displayName ?? ''; return ( diff --git a/src/library-authoring/components/LibraryComponents.test.tsx b/src/library-authoring/components/LibraryComponents.test.tsx index fd45dac12c..ed490b92d6 100644 --- a/src/library-authoring/components/LibraryComponents.test.tsx +++ b/src/library-authoring/components/LibraryComponents.test.tsx @@ -173,8 +173,8 @@ describe('', () => { expect(await screen.findByText('This is a text: ID=1')).toBeInTheDocument(); expect(screen.getByText('This is a text: ID=2')).toBeInTheDocument(); - expect(screen.getByText('This is a video: ID=3')).toBeInTheDocument(); - expect(screen.getByText('This is a video: ID=4')).toBeInTheDocument(); + expect(screen.getByText('Video Component 3')).toBeInTheDocument(); + expect(screen.getByText('Video Component 4')).toBeInTheDocument(); expect(screen.getByText('This is a problem: ID=5')).toBeInTheDocument(); expect(screen.getByText('This is a problem: ID=6')).toBeInTheDocument(); }); @@ -189,8 +189,8 @@ describe('', () => { expect(await screen.findByText('This is a text: ID=1')).toBeInTheDocument(); expect(screen.getByText('This is a text: ID=2')).toBeInTheDocument(); - expect(screen.getByText('This is a video: ID=3')).toBeInTheDocument(); - expect(screen.getByText('This is a video: ID=4')).toBeInTheDocument(); + expect(screen.getByText('Video Component 3')).toBeInTheDocument(); + expect(screen.getByText('Video Component 4')).toBeInTheDocument(); expect(screen.queryByText('This is a problem: ID=5')).not.toBeInTheDocument(); expect(screen.queryByText('This is a problem: ID=6')).not.toBeInTheDocument(); }); diff --git a/src/library-authoring/components/utils.test.ts b/src/library-authoring/components/utils.test.ts index 8d09c43986..b31018d27d 100644 --- a/src/library-authoring/components/utils.test.ts +++ b/src/library-authoring/components/utils.test.ts @@ -1,50 +1,14 @@ -import { getBlockType, getEditUrl, getLibraryId } from './utils'; +import { getEditUrl } from './utils'; describe('component utils', () => { - describe('getBlockType', () => { - for (const [input, expected] of [ - ['lb:org:lib:html:id', 'html'], - ['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'html'], - ['lb:Axim:beta:problem:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'problem'], - ]) { - it(`returns '${expected}' for usage key '${input}'`, () => { - expect(getBlockType(input)).toStrictEqual(expected); - }); - } - - for (const input of ['', undefined, null, 'not a key', 'lb:foo']) { - it(`throws an exception for usage key '${input}'`, () => { - expect(() => getBlockType(input as any)).toThrow(`Invalid usageKey: ${input}`); - }); - } - }); - - describe('getLibraryId', () => { - for (const [input, expected] of [ - ['lb:org:lib:html:id', 'lib:org:lib'], - ['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'lib:OpenCraftX:ALPHA'], - ['lb:Axim:beta:problem:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'lib:Axim:beta'], - ]) { - it(`returns '${expected}' for usage key '${input}'`, () => { - expect(getLibraryId(input)).toStrictEqual(expected); - }); - } - - for (const input of ['', undefined, null, 'not a key', 'lb:foo']) { - it(`throws an exception for usage key '${input}'`, () => { - expect(() => getLibraryId(input as any)).toThrow(`Invalid usageKey: ${input}`); - }); - } - }); - describe('getEditUrl', () => { it('returns the right URL for an HTML (Text) block', () => { const usageKey = 'lb:org:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd'; expect(getEditUrl(usageKey)).toStrictEqual(`/library/lib:org:ALPHA/editor/html/${usageKey}`); }); - it('doesn\'t yet allow editing a problem block', () => { + it('returns the right URL for editing a Problem block', () => { const usageKey = 'lb:org:beta:problem:571fe018-f3ce-45c9-8f53-5dafcb422fdd'; - expect(getEditUrl(usageKey)).toBeUndefined(); + expect(getEditUrl(usageKey)).toStrictEqual(`/library/lib:org:beta/editor/problem/${usageKey}`); }); it('doesn\'t yet allow editing a video block', () => { const usageKey = 'lb:org:beta:video:571fe018-f3ce-45c9-8f53-5dafcb422fdd'; diff --git a/src/library-authoring/components/utils.ts b/src/library-authoring/components/utils.ts index 095d956a00..2b99dcc2b1 100644 --- a/src/library-authoring/components/utils.ts +++ b/src/library-authoring/components/utils.ts @@ -1,34 +1,6 @@ -/** - * Given a usage key like `lb:org:lib:html:id`, get the type (e.g. `html`) - * @param usageKey e.g. `lb:org:lib:html:id` - * @returns The block type as a string - */ -export function getBlockType(usageKey: string): string { - if (usageKey && usageKey.startsWith('lb:')) { - const blockType = usageKey.split(':')[3]; - if (blockType) { - return blockType; - } - } - throw new Error(`Invalid usageKey: ${usageKey}`); -} - -/** - * Given a usage key like `lb:org:lib:html:id`, get the library key - * @param usageKey e.g. `lb:org:lib:html:id` - * @returns The library key, e.g. `lib:org:lib` - */ -export function getLibraryId(usageKey: string): string { - if (usageKey && usageKey.startsWith('lb:')) { - const org = usageKey.split(':')[1]; - const lib = usageKey.split(':')[2]; - if (org && lib) { - return `lib:${org}:${lib}`; - } - } - throw new Error(`Invalid usageKey: ${usageKey}`); -} +import { getBlockType, getLibraryId } from '../../generic/key-utils'; +/* eslint-disable import/prefer-default-export */ export function getEditUrl(usageKey: string): string | undefined { let blockType: string; let libraryId: string; @@ -39,7 +11,8 @@ export function getEditUrl(usageKey: string): string | undefined { return undefined; } - const mfeEditorTypes = ['html']; + // Which XBlock/component types are supported by the 'editors' built in to this repo? + const mfeEditorTypes = ['html', 'problem']; if (mfeEditorTypes.includes(blockType)) { return `/library/${libraryId}/editor/${blockType}/${usageKey}`; } diff --git a/src/library-authoring/data/api.mocks.ts b/src/library-authoring/data/api.mocks.ts index 480e78d661..0002f7516a 100644 --- a/src/library-authoring/data/api.mocks.ts +++ b/src/library-authoring/data/api.mocks.ts @@ -112,11 +112,14 @@ mockContentLibrary.applyMock = () => jest.spyOn(api, 'getContentLibrary').mockIm export async function mockCreateLibraryBlock( args: api.CreateBlockDataRequest, ): ReturnType { - if (args.blockType === 'html' && args.libraryId === mockContentLibrary.libraryId) { - return mockCreateLibraryBlock.newHtmlData; - } - if (args.blockType === 'problem' && args.libraryId === mockContentLibrary.libraryId) { - return mockCreateLibraryBlock.newProblemData; + if (args.libraryId === mockContentLibrary.libraryId) { + switch (args.blockType) { + case 'html': return mockCreateLibraryBlock.newHtmlData; + case 'problem': return mockCreateLibraryBlock.newProblemData; + case 'video': return mockCreateLibraryBlock.newVideoData; + default: + // Continue to error handling below. + } } throw new Error(`mockCreateLibraryBlock doesn't know how to mock ${JSON.stringify(args)}`); } @@ -146,6 +149,19 @@ mockCreateLibraryBlock.newProblemData = { created: '2024-07-22T21:37:49Z', tagsCount: 0, } satisfies api.LibraryBlockMetadata; +mockCreateLibraryBlock.newVideoData = { + id: 'lb:Axim:TEST:video:prob1', + defKey: 'video1', + blockType: 'video', + displayName: 'New Video', + hasUnpublishedChanges: true, + lastPublished: null, // or e.g. '2024-08-30T16:37:42Z', + publishedBy: null, // or e.g. 'test_author', + lastDraftCreated: '2024-07-22T21:37:49Z', + lastDraftCreatedBy: null, + created: '2024-07-22T21:37:49Z', + tagsCount: 0, +} satisfies api.LibraryBlockMetadata; /** Apply this mock. Returns a spy object that can tell you if it's been called. */ mockCreateLibraryBlock.applyMock = () => ( jest.spyOn(api, 'createLibraryBlock').mockImplementation(mockCreateLibraryBlock) @@ -163,6 +179,7 @@ export async function mockXBlockFields(usageKey: string): Promise jest.spyOn(api, 'getXBlockFields').mockImplementation(mockXBlockFields); diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index 91b07c6a7f..efbd59efc0 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -7,7 +7,7 @@ import { type QueryClient, } from '@tanstack/react-query'; -import { getLibraryId } from '../components/utils'; +import { getLibraryId } from '../../generic/key-utils'; import { type GetLibrariesV2CustomParams, type ContentLibrary, diff --git a/src/search-modal/SearchResult.tsx b/src/search-modal/SearchResult.tsx index 6d7e618f94..9ba6ee9516 100644 --- a/src/search-modal/SearchResult.tsx +++ b/src/search-modal/SearchResult.tsx @@ -11,6 +11,7 @@ import { useSelector } from 'react-redux'; import { useNavigate } from 'react-router-dom'; import { getItemIcon } from '../generic/block-type-utils'; +import { isLibraryKey } from '../generic/key-utils'; import { useSearchContext, type ContentHit, Highlight } from '../search-manager'; import { getStudioHomeData } from '../studio-home/data/selectors'; import { constructLibraryAuthoringURL } from '../utils'; @@ -116,7 +117,7 @@ const SearchResult: React.FC<{ hit: ContentHit }> = ({ hit }) => { return `/${urlSuffix}`; } - if (contextKey.startsWith('lib:')) { + if (isLibraryKey(contextKey)) { const urlSuffix = getLibraryComponentUrlSuffix(hit); if (redirectToLibraryAuthoringMfe && libraryAuthoringMfeUrl) { return constructLibraryAuthoringURL(libraryAuthoringMfeUrl, urlSuffix);