From 15c6433a21d24c49f794aa30fb107de26425655e Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Wed, 6 Nov 2024 01:03:12 +1100 Subject: [PATCH 1/8] feat: enable to upload images from libreries in TinyMCE --- src/editors/data/redux/app/reducer.js | 1 + src/editors/data/redux/thunkActions/app.js | 8 +- .../data/redux/thunkActions/requests.js | 27 +++- .../data/redux/thunkActions/requests.test.js | 133 +++++++++++++---- src/editors/data/services/cms/api.test.ts | 45 +++++- src/editors/data/services/cms/api.ts | 22 ++- src/editors/data/services/cms/urls.ts | 4 + src/editors/data/services/cms/utils.ts | 9 ++ .../TinyMceWidget/pluginConfig.js | 11 +- src/editors/utils/formatLibraryImgRequest.ts | 134 ++++++++++++++++++ .../utils/formatLibreryImgRequest.test.ts | 121 ++++++++++++++++ src/editors/utils/index.ts | 1 + 12 files changed, 459 insertions(+), 57 deletions(-) create mode 100644 src/editors/utils/formatLibraryImgRequest.ts create mode 100644 src/editors/utils/formatLibreryImgRequest.test.ts diff --git a/src/editors/data/redux/app/reducer.js b/src/editors/data/redux/app/reducer.js index 043de8bf69..3e019c8a54 100644 --- a/src/editors/data/redux/app/reducer.js +++ b/src/editors/data/redux/app/reducer.js @@ -53,6 +53,7 @@ const app = createSlice({ images: { ...state.images, ...payload.images }, imageCount: payload.imageCount, }), + resetImages: (state) => ({ ...state, images: {}, imageCount: 0 }), setVideos: (state, { payload }) => ({ ...state, videos: payload }), setCourseDetails: (state, { payload }) => ({ ...state, courseDetails: payload }), setShowRawEditor: (state, { payload }) => ({ diff --git a/src/editors/data/redux/thunkActions/app.js b/src/editors/data/redux/thunkActions/app.js index 478ef4240c..09ab9eb61b 100644 --- a/src/editors/data/redux/thunkActions/app.js +++ b/src/editors/data/redux/thunkActions/app.js @@ -103,12 +103,8 @@ export const initialize = (data) => (dispatch) => { dispatch(module.fetchCourseDetails()); break; case 'html': - if (isLibraryKey(data.learningContextId)) { - // eslint-disable-next-line no-console - console.log('Not fetching image assets - not implemented yet for content libraries.'); - } else { - dispatch(module.fetchImages({ pageNumber: 0 })); - } + if (isLibraryKey(data.learningContextId)) { dispatch(actions.app.resetImages()); } + dispatch(module.fetchImages({ pageNumber: 0 })); break; default: break; diff --git a/src/editors/data/redux/thunkActions/requests.js b/src/editors/data/redux/thunkActions/requests.js index 46f9d1a03a..e3b4379a1e 100644 --- a/src/editors/data/redux/thunkActions/requests.js +++ b/src/editors/data/redux/thunkActions/requests.js @@ -1,4 +1,4 @@ -import { StrictDict } from '../../../utils'; +import { StrictDict, parseLibraryImageData, getLibraryImageAssets } from '../../../utils'; import { RequestKeys } from '../../constants/requests'; import api, { loadImages } from '../../services/cms/api'; @@ -10,6 +10,7 @@ import { selectors as appSelectors } from '../app'; // should be re-thought and cleaned up to avoid this pattern. // eslint-disable-next-line import/no-self-import import * as module from './requests'; +import { isLibraryKey } from '../../../../generic/key-utils'; // Similar to `import { actions, selectors } from '..';` but avoid circular imports: const actions = { requests: requestsActions }; @@ -121,27 +122,45 @@ export const saveBlock = ({ content, ...rest }) => (dispatch, getState) => { })); }; export const uploadAsset = ({ asset, ...rest }) => (dispatch, getState) => { + const learningContextId = selectors.app.learningContextId(getState()); dispatch(module.networkRequest({ requestKey: RequestKeys.uploadAsset, promise: api.uploadAsset({ - learningContextId: selectors.app.learningContextId(getState()), + learningContextId, asset, studioEndpointUrl: selectors.app.studioEndpointUrl(getState()), + blockId: selectors.app.blockId(getState()), + }).then((resp) => { + if (isLibraryKey(learningContextId)) { + return ({ + ...resp, + data: { asset: parseLibraryImageData(resp.data) }, + }); + } + return resp; }), ...rest, })); }; export const fetchImages = ({ pageNumber, ...rest }) => (dispatch, getState) => { + const learningContextId = selectors.app.learningContextId(getState()); dispatch(module.networkRequest({ requestKey: RequestKeys.fetchImages, promise: api .fetchImages({ pageNumber, + blockId: selectors.app.blockId(getState()), studioEndpointUrl: selectors.app.studioEndpointUrl(getState()), - learningContextId: selectors.app.learningContextId(getState()), + learningContextId, }) - .then(({ data }) => ({ images: loadImages(data.assets), imageCount: data.totalCount })), + .then(({ data }) => { + if (isLibraryKey(learningContextId)) { + const images = getLibraryImageAssets(data.files); + return { images, imageCount: Object.keys(images).length }; + } + return { images: loadImages(data.assets), imageCount: data.totalCount }; + }), ...rest, })); }; diff --git a/src/editors/data/redux/thunkActions/requests.test.js b/src/editors/data/redux/thunkActions/requests.test.js index 4b5961b9eb..d389465de9 100644 --- a/src/editors/data/redux/thunkActions/requests.test.js +++ b/src/editors/data/redux/thunkActions/requests.test.js @@ -1,4 +1,4 @@ -import { keyStore } from '../../../utils'; +import { keyStore, parseLibraryImageData, getLibraryImageAssets } from '../../../utils'; import { RequestKeys } from '../../constants/requests'; import api from '../../services/cms/api'; import * as requests from './requests'; @@ -40,6 +40,12 @@ jest.mock('../../services/cms/api', () => ({ uploadVideo: (args) => args, })); +jest.mock('../../../utils', () => ({ + ...jest.requireActual('../../../utils'), + parseLibraryImageData: jest.fn(), + getLibraryImageAssets: jest.fn(() => ({})), +})); + const apiKeys = keyStore(api); let dispatch; @@ -241,10 +247,6 @@ describe('requests thunkActions module', () => { let fetchImages; let loadImages; let dispatchedAction; - const expectedArgs = { - studioEndpointUrl: selectors.app.studioEndpointUrl(testState), - learningContextId: selectors.app.learningContextId(testState), - }; beforeEach(() => { fetchImages = jest.fn((args) => new Promise((resolve) => { resolve({ data: { assets: { fetchImages: args } } }); @@ -254,18 +256,50 @@ describe('requests thunkActions module', () => { requests.fetchImages({ ...fetchParams, onSuccess, onFailure })(dispatch, () => testState); [[dispatchedAction]] = dispatch.mock.calls; }); - it('dispatches networkRequest', () => { - expect(dispatchedAction.networkRequest).not.toEqual(undefined); - }); - test('forwards onSuccess and onFailure', () => { - expect(dispatchedAction.networkRequest.onSuccess).toEqual(onSuccess); - expect(dispatchedAction.networkRequest.onFailure).toEqual(onFailure); - }); - test('api.fetchImages promise called with studioEndpointUrl and learningContextId', () => { - expect(fetchImages).toHaveBeenCalledWith(expectedArgs); + describe('courses', () => { + const expectedArgs = { + blockId: selectors.app.blockId(testState), + studioEndpointUrl: selectors.app.studioEndpointUrl(testState), + learningContextId: selectors.app.learningContextId(testState), + }; + it('dispatches networkRequest', () => { + expect(dispatchedAction.networkRequest).not.toEqual(undefined); + }); + test('forwards onSuccess and onFailure', () => { + expect(dispatchedAction.networkRequest.onSuccess).toEqual(onSuccess); + expect(dispatchedAction.networkRequest.onFailure).toEqual(onFailure); + }); + test('api.fetchImages promise called with studioEndpointUrl and learningContextId', () => { + expect(fetchImages).toHaveBeenCalledWith(expectedArgs); + }); + test('promise is chained with api.loadImages', () => { + expect(loadImages).toHaveBeenCalledWith({ fetchImages: expectedArgs }); + }); + test('promise is chained with api.loadImages', () => { + expect(loadImages).toHaveBeenCalledWith({ fetchImages: expectedArgs }); + }); }); - test('promise is chained with api.loadImages', () => { - expect(loadImages).toHaveBeenCalledWith({ fetchImages: expectedArgs }); + describe('libraries', () => { + const expectedArgs = { + learningContextId: 'lib:demo', + studioEndpointUrl: selectors.app.studioEndpointUrl(testState), + blockId: selectors.app.blockId(testState), + }; + beforeEach(() => { + jest.spyOn(selectors.app, 'learningContextId').mockImplementationOnce(() => ('lib:demo')); + fetchImages = jest.fn((args) => new Promise((resolve) => { + resolve({ data: { assets: { fetchImages: args } } }); + })); + jest.spyOn(api, apiKeys.fetchImages).mockImplementationOnce(fetchImages); + requests.fetchImages({ + ...fetchParams, onSuccess, onFailure, + })(dispatch, () => testState); + [[dispatchedAction]] = dispatch.mock.calls; + }); + test('api.fetchImages promise called with studioEndpointUrl and blockId', () => { + expect(fetchImages).toHaveBeenCalledWith(expectedArgs); + expect(getLibraryImageAssets).toHaveBeenCalled(); + }); }); }); describe('fetchVideos', () => { @@ -316,21 +350,62 @@ describe('requests thunkActions module', () => { }); describe('uploadAsset', () => { const asset = 'SoME iMage CoNtent As String'; - testNetworkRequestAction({ - action: requests.uploadAsset, - args: { asset, ...fetchParams }, - expectedString: 'with uploadAsset promise', - expectedData: { - ...fetchParams, - requestKey: RequestKeys.uploadAsset, - promise: api.uploadAsset({ - learningContextId: selectors.app.learningContextId(testState), - asset, - studioEndpointUrl: selectors.app.studioEndpointUrl(testState), - }), - }, + let uploadAsset; + let dispatchedAction; + + describe('courses', () => { + const expectedArgs = { + learningContextId: selectors.app.learningContextId(testState), + studioEndpointUrl: selectors.app.studioEndpointUrl(testState), + blockId: selectors.app.blockId(testState), + asset, + }; + beforeEach(() => { + uploadAsset = jest.fn((args) => new Promise((resolve) => { + resolve({ data: { asset: args } }); + })); + jest.spyOn(api, apiKeys.uploadAsset).mockImplementationOnce(uploadAsset); + requests.uploadAsset({ + asset, ...fetchParams, onSuccess, onFailure, + })(dispatch, () => testState); + [[dispatchedAction]] = dispatch.mock.calls; + }); + it('dispatches networkRequest', () => { + expect(dispatchedAction.networkRequest).not.toEqual(undefined); + }); + test('forwards onSuccess and onFailure', () => { + expect(dispatchedAction.networkRequest.onSuccess).toEqual(onSuccess); + expect(dispatchedAction.networkRequest.onFailure).toEqual(onFailure); + }); + test('api.uploadAsset promise called with studioEndpointUrl, blockId and learningContextId', () => { + expect(uploadAsset).toHaveBeenCalledWith(expectedArgs); + }); + }); + describe('libraries', () => { + const expectedArgs = { + learningContextId: 'lib:demo', + studioEndpointUrl: selectors.app.studioEndpointUrl(testState), + blockId: selectors.app.blockId(testState), + asset, + }; + beforeEach(() => { + jest.spyOn(selectors.app, 'learningContextId').mockImplementationOnce(() => ('lib:demo')); + uploadAsset = jest.fn((args) => new Promise((resolve) => { + resolve({ data: { asset: args } }); + })); + jest.spyOn(api, apiKeys.uploadAsset).mockImplementationOnce(uploadAsset); + requests.uploadAsset({ + asset, ...fetchParams, onSuccess, onFailure, + })(dispatch, () => testState); + [[dispatchedAction]] = dispatch.mock.calls; + }); + test('api.uploadAsset promise called with studioEndpointUrl and blockId', () => { + expect(uploadAsset).toHaveBeenCalledWith(expectedArgs); + expect(parseLibraryImageData).toHaveBeenCalled(); + }); }); }); + describe('uploadThumbnail', () => { const thumbnail = 'SoME tHumbNAil CoNtent As String'; const videoId = 'SoME VidEOid CoNtent As String'; diff --git a/src/editors/data/services/cms/api.test.ts b/src/editors/data/services/cms/api.test.ts index d7b553fb96..80563a812a 100644 --- a/src/editors/data/services/cms/api.test.ts +++ b/src/editors/data/services/cms/api.test.ts @@ -1,12 +1,15 @@ import * as api from './api'; import * as urls from './urls'; -import { get, post, deleteObject } from './utils'; +import { + get, post, put, deleteObject, +} from './utils'; jest.mock('./urls', () => ({ block: jest.fn().mockReturnValue('urls.block'), blockAncestor: jest.fn().mockReturnValue('urls.blockAncestor'), blockStudioView: jest.fn().mockReturnValue('urls.StudioView'), courseAssets: jest.fn().mockReturnValue('urls.courseAssets'), + libraryAssets: jest.fn().mockReturnValue('urls.libraryAssets'), videoTranscripts: jest.fn().mockReturnValue('urls.videoTranscripts'), allowThumbnailUpload: jest.fn().mockReturnValue('urls.allowThumbnailUpload'), thumbnailUpload: jest.fn().mockReturnValue('urls.thumbnailUpload'), @@ -25,19 +28,21 @@ jest.mock('./urls', () => ({ jest.mock('./utils', () => ({ get: jest.fn().mockName('get'), post: jest.fn().mockName('post'), + put: jest.fn().mockName('put'), deleteObject: jest.fn().mockName('deleteObject'), })); const { apiMethods } = api; const blockId = 'block-v1-coursev1:2uX@4345432'; -const learningContextId = 'demo2uX'; +let learningContextId; const studioEndpointUrl = 'hortus.coa'; const title = 'remember this needs to go into metadata to save'; describe('cms api', () => { beforeEach(() => { jest.clearAllMocks(); + learningContextId = 'demo2uX'; }); describe('apiMethods', () => { describe('fetchBlockId', () => { @@ -102,7 +107,9 @@ describe('cms api', () => { describe('fetchImages', () => { it('should call get with url.courseAssets', () => { - apiMethods.fetchImages({ learningContextId, studioEndpointUrl, pageNumber: 0 }); + apiMethods.fetchImages({ + blockId, learningContextId, studioEndpointUrl, pageNumber: 0, + }); const params = { asset_type: 'Images', page: 0, @@ -112,6 +119,15 @@ describe('cms api', () => { { params }, ); }); + it('should call get with urls.libraryAssets for library V2', () => { + learningContextId = 'lib:demo2uX'; + apiMethods.fetchImages({ + blockId, learningContextId, studioEndpointUrl, pageNumber: 0, + }); + expect(get).toHaveBeenCalledWith( + urls.libraryAssets({ studioEndpointUrl, blockId }), + ); + }); }); describe('fetchCourseDetails', () => { @@ -246,11 +262,14 @@ describe('cms api', () => { }); describe('uploadAsset', () => { - const asset = new Blob(['data'], { type: 'image/jpeg' }); + const img = new Blob(['data'], { type: 'image/jpeg' }); + const filename = 'image.jpg'; + const asset = new File([img], filename, { type: 'image/jpeg' }); + const mockFormdata = new FormData(); + mockFormdata.append('file', asset); it('should call post with urls.courseAssets and imgdata', () => { - const mockFormdata = new FormData(); - mockFormdata.append('file', asset); apiMethods.uploadAsset({ + blockId, learningContextId, studioEndpointUrl, asset, @@ -260,6 +279,20 @@ describe('cms api', () => { mockFormdata, ); }); + it('should call post with urls.libraryAssets and imgdata', () => { + learningContextId = 'lib:demo2uX'; + mockFormdata.append('content', asset); + apiMethods.uploadAsset({ + blockId, + learningContextId, + studioEndpointUrl, + asset, + }); + expect(put).toHaveBeenCalledWith( + `${urls.libraryAssets({ blockId, studioEndpointUrl })}static/${encodeURI(filename)}`, + mockFormdata, + ); + }); }); describe('uploadVideo', () => { diff --git a/src/editors/data/services/cms/api.ts b/src/editors/data/services/cms/api.ts index d40c9d5f36..20aa84daa4 100644 --- a/src/editors/data/services/cms/api.ts +++ b/src/editors/data/services/cms/api.ts @@ -2,7 +2,9 @@ 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'; +import { + get, post, put, deleteObject, +} from './utils'; import { durationStringFromValue } from '../../../containers/VideoEditor/components/VideoSettingsModal/components/DurationWidget/hooks'; const fetchByUnitIdOptions: AxiosRequestConfig = {}; @@ -116,17 +118,15 @@ export const apiMethods = { urls.blockStudioView({ studioEndpointUrl, blockId }), ), fetchImages: ({ + blockId, 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, - }, - }); + return get( + `${urls.libraryAssets({ studioEndpointUrl, blockId })}`, + ); } const params = { asset_type: 'Images', @@ -147,12 +147,20 @@ export const apiMethods = { urls.courseAdvanceSettings({ studioEndpointUrl, learningContextId }), ), uploadAsset: ({ + blockId, learningContextId, studioEndpointUrl, asset, }) => { const data = new FormData(); data.append('file', asset); + if (isLibraryKey(learningContextId)) { + data.set('content', asset); + return put( + `${urls.libraryAssets({ blockId, studioEndpointUrl })}static/${encodeURI(asset.name)}`, + data, + ); + } return post( urls.courseAssets({ studioEndpointUrl, learningContextId }), data, diff --git a/src/editors/data/services/cms/urls.ts b/src/editors/data/services/cms/urls.ts index 3618499915..d6dd3e362b 100644 --- a/src/editors/data/services/cms/urls.ts +++ b/src/editors/data/services/cms/urls.ts @@ -61,6 +61,10 @@ export const courseAssets = (({ studioEndpointUrl, learningContextId }) => ( `${studioEndpointUrl}/assets/${learningContextId}/` )) satisfies UrlFunction; +export const libraryAssets = (({ blockId, studioEndpointUrl }) => ( + `${studioEndpointUrl}/api/libraries/v2/blocks/${blockId}/assets/` +)) satisfies UrlFunction; + export const thumbnailUpload = (({ studioEndpointUrl, learningContextId, videoId }) => ( `${studioEndpointUrl}/video_images/${learningContextId}/${videoId}` )) satisfies UrlFunction; diff --git a/src/editors/data/services/cms/utils.ts b/src/editors/data/services/cms/utils.ts index b7d6276fe7..86ba95d902 100644 --- a/src/editors/data/services/cms/utils.ts +++ b/src/editors/data/services/cms/utils.ts @@ -16,6 +16,15 @@ export const get: Axios['get'] = (...args) => client().get(...args); * @param {object|string} data - post payload */ export const post: Axios['post'] = (...args) => client().post(...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 put: Axios['put'] = (...args) => client().put(...args); + /** * delete(url, data) * simple wrapper providing an authenticated Http client delete action diff --git a/src/editors/sharedComponents/TinyMceWidget/pluginConfig.js b/src/editors/sharedComponents/TinyMceWidget/pluginConfig.js index 7d7120dc25..c33ab2977f 100644 --- a/src/editors/sharedComponents/TinyMceWidget/pluginConfig.js +++ b/src/editors/sharedComponents/TinyMceWidget/pluginConfig.js @@ -1,13 +1,14 @@ +import { isLibraryV1Key } from '../../../generic/key-utils'; import { StrictDict } from '../../utils'; import { buttons, plugins } from '../../data/constants/tinyMCE'; const mapToolbars = toolbars => toolbars.map(toolbar => toolbar.join(' ')).join(' | '); -const pluginConfig = ({ isLibrary, placeholder, editorType }) => { - const image = isLibrary ? '' : plugins.image; - const imageTools = isLibrary ? '' : plugins.imagetools; - const imageUploadButton = isLibrary ? '' : buttons.imageUploadButton; - const editImageSettings = isLibrary ? '' : buttons.editImageSettings; +const pluginConfig = ({ learningContextId, placeholder, editorType }) => { + const image = isLibraryV1Key(learningContextId) ? '' : plugins.image; + const imageTools = isLibraryV1Key(learningContextId) ? '' : plugins.imagetools; + const imageUploadButton = isLibraryV1Key(learningContextId) ? '' : buttons.imageUploadButton; + const editImageSettings = isLibraryV1Key(learningContextId) ? '' : buttons.editImageSettings; const codePlugin = editorType === 'text' ? plugins.code : ''; const codeButton = editorType === 'text' ? buttons.code : ''; const labelButton = editorType === 'question' ? buttons.customLabelButton : ''; diff --git a/src/editors/utils/formatLibraryImgRequest.ts b/src/editors/utils/formatLibraryImgRequest.ts new file mode 100644 index 0000000000..6a0580cee9 --- /dev/null +++ b/src/editors/utils/formatLibraryImgRequest.ts @@ -0,0 +1,134 @@ +import StrictDict from './StrictDict'; + +/** + * A dictionary that maps file extensions to their corresponding MIME types for images. + * + * @example + * acceptedImgMimeTypes.gif // "image/gif" + */ + +const acceptedImgMimeTypes = StrictDict({ + gif: 'image/gif', + jpg: 'image/jpg', + jpeg: 'image/jpeg', + png: 'image/png', + tif: 'image/tiff', + tiff: 'image/tiff', + ico: 'image/x-icon', +}); + +type TinyMCEImageData = { + displayName: string, + contentType: string, + url: string, + externalUrl: string, + portableUrl: string, + thumbnail: string, + id: string, + locked: boolean, +}; + +export type LibraryAssetResponse = { + path: string, + size: number, + url: string, +}; + +/** + * Extracts the file name from a file path. + * This function strips the directory structure and returns the base file name. + * + * @param data - The asset data containing the file path. + * @returns The file name extracted from the path. + * + * @example + * const data = { path: '/static/example.jpg', size: 12345, url: 'http://example.com/static/example.jpg' }; + * const fileName = getFileName(data); // "example.jpg" + */ + +export const getFileName = (data: LibraryAssetResponse): string => data.path.replace(/^.*[\\/]/, ''); + +/** + * Determines the MIME type of a file based on its extension. + * + * @param data - The asset data containing the file path. + * @returns The MIME type of the file, or 'unknown' if the MIME type is not recognized. + * + * @example + * const data = { path: '/static/example.jpg', size: 12345, url: 'http://example.com/static/example.jpg' }; + * const mimeType = getFileMimeType(data); // "image/jpg" + */ + +export const getFileMimeType = (data: LibraryAssetResponse): string => { + const ext = data.path.split('.').pop()?.toLowerCase(); // Extract and lowercase the file extension + return ext && acceptedImgMimeTypes[ext] ? acceptedImgMimeTypes[ext] : 'unknown'; +}; +/** + * Parses a `LibraryAssetResponse` into a `TinyMCEImageData` object. + * This includes extracting the file name, MIME type, and constructing other image-related metadata. + * + * @param data - The asset data to parse. + * @returns The parsed image data with properties like `displayName`, `contentType`, etc. + * + * @example + * const data = { path: '/static/example.jpg', size: 12345, url: 'http://example.com/static/example.jpg' }; + * const imageData = parseLibraryImageData(data); + * // { + * // displayName: 'example.jpg', + * // contentType: 'image/jpg', + * // url: 'http://example.com/static/example.jpg', + * // externalUrl: 'http://example.com/static/example.jpg', + * // portableUrl: '/static/example.jpg', + * // thumbnail: 'http://example.com/static/example.jpg', + * // id: '/static/example.jpg', + * // locked: false + * // } + */ + +export const parseLibraryImageData = (data: LibraryAssetResponse): TinyMCEImageData => ({ + displayName: getFileName(data), + contentType: getFileMimeType(data), + url: data.url, + externalUrl: data.url, + portableUrl: data.path, + thumbnail: data.url, + id: data.path, + locked: false, +}); + +/** + * Filters and transforms an array of `LibrariesAssetResponse` objects into a dictionary of `TinyMCEImageData`. + * Only assets with recognized MIME types (i.e., valid image files) are included in the result. + * + * @param librariesAssets - The array of asset data to process. + * @returns A dictionary where each key is the file name and the value is the corresponding `TinyMCEImageData`. + * + * @example + * const assets = [ + * { path: '/static/example.jpg', size: 12345, url: 'http://example.com/static/example.jpg' }, + * { path: '/assets/files/unsupported.xyz', size: 67890, url: 'http://example.com/assets/files/unsupported.xyz' } + * ]; + * const imageAssets = getLibraryImageAssets(assets); + * // { + * // 'example.jpg': { + * // displayName: 'example.jpg', + * // contentType: 'image/jpg', + * // url: 'http://example.com/static/example.jpg', + * // externalUrl: 'http://example.com/static/example.jpg', + * // portableUrl: '/static/example.jpg', + * // thumbnail: 'http://example.com/static/example.jpg', + * // id: '/static/example.jpg', + * // locked: false + * // } + * // } + */ + +export const getLibraryImageAssets = ( + librariesAssets: Array, +): Record => librariesAssets.reduce((obj, file) => { + if (getFileMimeType(file) !== 'unknown') { + const imageData = parseLibraryImageData(file); + return { ...obj, [imageData.displayName]: imageData }; + } + return obj; +}, {} as Record); diff --git a/src/editors/utils/formatLibreryImgRequest.test.ts b/src/editors/utils/formatLibreryImgRequest.test.ts new file mode 100644 index 0000000000..b4d23a75ef --- /dev/null +++ b/src/editors/utils/formatLibreryImgRequest.test.ts @@ -0,0 +1,121 @@ +import { + parseLibraryImageData, getLibraryImageAssets, getFileMimeType, getFileName, LibraryAssetResponse, +} from './formatLibraryImgRequest'; + +// Mock the StrictDict function to avoid unnecessary complexity in the test +jest.mock('./StrictDict', () => ({ + __esModule: true, + default: jest.fn().mockReturnValue({ + gif: 'image/gif', + jpg: 'image/jpg', + jpeg: 'image/jpeg', + png: 'image/png', + tif: 'image/tiff', + tiff: 'image/tiff', + ico: 'image/x-icon', + }), +})); + +describe('parseLibraryImageData', () => { + describe('getFileName', () => { + it('should return the file name from the path', () => { + const data: LibraryAssetResponse = { + path: 'static/example.jpg', + size: 12345, + url: 'http://example.com/static/example.jpg', + }; + + const result = getFileName(data); + expect(result).toBe('example.jpg'); + }); + }); + + describe('getFileMimeType', () => { + it('should return the correct MIME type for supported file extensions', () => { + const data: LibraryAssetResponse = { + path: 'static/example.jpg', + size: 12345, + url: 'http://example.com/static/example.jpg', + }; + + const result = getFileMimeType(data); + expect(result).toBe('image/jpg'); + }); + + it('should return "unknown" for unsupported file extensions', () => { + const data: LibraryAssetResponse = { + path: '/assets/files/unknown.xyz', + size: 12345, + url: 'http://example.com/assets/files/unknown.xyz', + }; + + const result = getFileMimeType(data); + expect(result).toBe('unknown'); + }); + }); + + describe('parseLibraryImageData', () => { + it('should correctly parse a valid LibraryAssetResponse into TinyMCEImageData', () => { + const data: LibraryAssetResponse = { + path: 'static/example.jpg', + size: 12345, + url: 'http://example.com/static/example.jpg', + }; + + const result = parseLibraryImageData(data); + expect(result).toEqual({ + displayName: 'example.jpg', + contentType: 'image/jpg', + url: 'http://example.com/static/example.jpg', + externalUrl: 'http://example.com/static/example.jpg', + portableUrl: 'static/example.jpg', + thumbnail: 'http://example.com/static/example.jpg', + id: 'static/example.jpg', + locked: false, + }); + }); + + it('should handle unknown MIME types by setting a fallback MIME type', () => { + const data: LibraryAssetResponse = { + path: '/assets/files/unknown.xyz', + size: 12345, + url: 'http://example.com/assets/files/unknown.xyz', + }; + + const result = parseLibraryImageData(data); + expect(result.contentType).toBe('unknown'); + }); + }); + + describe('getLibraryImageAssets', () => { + it('should filter out assets with unsupported MIME types and return a dictionary of valid images', () => { + const assets: LibraryAssetResponse[] = [ + { path: 'static/example.jpg', size: 12345, url: 'http://example.com/static/example.jpg' }, + { path: '/assets/files/unsupported.xyz', size: 67890, url: 'http://example.com/assets/files/unsupported.xyz' }, + ]; + + const result = getLibraryImageAssets(assets); + expect(result).toEqual({ + 'example.jpg': { + displayName: 'example.jpg', + contentType: 'image/jpg', + url: 'http://example.com/static/example.jpg', + externalUrl: 'http://example.com/static/example.jpg', + portableUrl: 'static/example.jpg', + thumbnail: 'http://example.com/static/example.jpg', + id: 'static/example.jpg', + locked: false, + }, + }); + }); + + it('should return an empty object if no valid images are found', () => { + const assets: LibraryAssetResponse[] = [ + { path: '/assets/files/unsupported.xyz', size: 67890, url: 'http://example.com/assets/files/unsupported.xyz' }, + ]; + + const result = getLibraryImageAssets(assets); + expect(result).toEqual({}); + }); + }); +}); diff --git a/src/editors/utils/index.ts b/src/editors/utils/index.ts index e314669159..34e2a3c355 100644 --- a/src/editors/utils/index.ts +++ b/src/editors/utils/index.ts @@ -5,3 +5,4 @@ export { default as camelizeKeys } from './camelizeKeys'; export { default as removeItemOnce } from './removeOnce'; export { default as formatDuration } from './formatDuration'; export { default as snakeCaseKeys } from './snakeCaseKeys'; +export * from './formatLibraryImgRequest'; From 6e5aecd3f7f12a07c232b0dc57665487f3ba61b0 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Wed, 6 Nov 2024 01:04:30 +1100 Subject: [PATCH 2/8] refactor: hide serach, filter and load more button in the modal --- .../SelectImageModal/__snapshots__/index.test.jsx.snap | 2 +- .../ImageUploadModal/SelectImageModal/index.jsx | 4 ++++ .../ImageUploadModal/SelectImageModal/messages.js | 2 +- src/editors/sharedComponents/SelectionModal/Gallery.jsx | 4 +++- src/editors/sharedComponents/SelectionModal/GalleryCard.jsx | 2 ++ src/editors/sharedComponents/SelectionModal/index.jsx | 5 ++++- .../TinyMceWidget/__snapshots__/index.test.jsx.snap | 4 ++-- src/editors/sharedComponents/TinyMceWidget/hooks.js | 4 +--- src/editors/sharedComponents/TinyMceWidget/index.jsx | 3 ++- src/editors/sharedComponents/TinyMceWidget/index.test.jsx | 2 +- 10 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/__snapshots__/index.test.jsx.snap b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/__snapshots__/index.test.jsx.snap index fd2b660d1e..587f4ce227 100644 --- a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/__snapshots__/index.test.jsx.snap +++ b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/__snapshots__/index.test.jsx.snap @@ -59,7 +59,7 @@ exports[`SelectImageModal component snapshot 1`] = ` "id": "authoring.texteditor.selectimagemodal.next.label", }, "fetchError": { - "defaultMessage": "Failed to obtain course images. Please try again.", + "defaultMessage": "Failed to obtain images. Please try again.", "description": "Message presented to user when images are not found", "id": "authoring.texteditor.selectimagemodal.error.fetchImagesError", }, diff --git a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.jsx b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.jsx index 81e4802cb7..0e0439a58b 100644 --- a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.jsx +++ b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.jsx @@ -17,6 +17,7 @@ const SelectImageModal = ({ isLoaded, isFetchError, isUploadError, + isLibrary, imageCount, }) => { const { @@ -57,6 +58,7 @@ const SelectImageModal = ({ isLoaded, isFetchError, isUploadError, + isLibrary, }} /> ); @@ -73,12 +75,14 @@ SelectImageModal.propTypes = { isFetchError: PropTypes.bool.isRequired, isUploadError: PropTypes.bool.isRequired, imageCount: PropTypes.number.isRequired, + isLibrary: PropTypes.bool, }; export const mapStateToProps = (state) => ({ isLoaded: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchImages }), isFetchError: selectors.requests.isFailed(state, { requestKey: RequestKeys.fetchImages }), isUploadError: selectors.requests.isFailed(state, { requestKey: RequestKeys.uploadAsset }), + isLibrary: selectors.app.isLibrary(state), imageCount: state.app.imageCount, }); diff --git a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/messages.js b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/messages.js index f39c307798..14794b641f 100644 --- a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/messages.js +++ b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/messages.js @@ -56,7 +56,7 @@ const messages = defineMessages({ }, fetchImagesError: { id: 'authoring.texteditor.selectimagemodal.error.fetchImagesError', - defaultMessage: 'Failed to obtain course images. Please try again.', + defaultMessage: 'Failed to obtain images. Please try again.', description: 'Message presented to user when images are not found', }, fileSizeError: { diff --git a/src/editors/sharedComponents/SelectionModal/Gallery.jsx b/src/editors/sharedComponents/SelectionModal/Gallery.jsx index 1e01a109fb..fc71e81cb3 100644 --- a/src/editors/sharedComponents/SelectionModal/Gallery.jsx +++ b/src/editors/sharedComponents/SelectionModal/Gallery.jsx @@ -23,6 +23,7 @@ const Gallery = ({ showIdsOnCards, height, isLoaded, + isLibrary, thumbnailFallback, allowLazyLoad, fetchNextPage, @@ -79,7 +80,7 @@ const Gallery = ({ /> )) } - {allowLazyLoad && ( + {(allowLazyLoad && !isLibrary) && ( )} + {asset.dateAdded && (

+ )} diff --git a/src/editors/sharedComponents/SelectionModal/index.jsx b/src/editors/sharedComponents/SelectionModal/index.jsx index f96a29c832..c345f8232d 100644 --- a/src/editors/sharedComponents/SelectionModal/index.jsx +++ b/src/editors/sharedComponents/SelectionModal/index.jsx @@ -34,6 +34,7 @@ const SelectionModal = ({ isLoaded, isFetchError, isUploadError, + isLibrary, }) => { const intl = useIntl(); const { @@ -54,6 +55,7 @@ const SelectionModal = ({ const galleryPropsValues = { isLoaded, + isLibrary, ...galleryProps, }; @@ -83,7 +85,7 @@ const SelectionModal = ({ )} title={intl.formatMessage(titleMsg)} bodyStyle={{ background }} - headerComponent={( + headerComponent={!isLibrary && (
@@ -160,6 +162,7 @@ SelectionModal.propTypes = { isLoaded: PropTypes.bool.isRequired, isFetchError: PropTypes.bool.isRequired, isUploadError: PropTypes.bool.isRequired, + isLibrary: PropTypes.bool, }; export default SelectionModal; diff --git a/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap b/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap index 01e328f8cc..feb3d25635 100644 --- a/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap +++ b/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap @@ -34,8 +34,8 @@ exports[`TinyMceWidget snapshots ImageUploadModal is not rendered 1`] = ` ], }, "initializeEditor": undefined, - "isLibrary": true, - "learningContextId": "course+org+run", + "isLibrary": false, + "learningContextId": "library-v1:org+t01", "lmsEndpointUrl": "sOmEvaLue.cOm", "minHeight": undefined, "openImgModal": [MockFunction modal.openModal], diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.js b/src/editors/sharedComponents/TinyMceWidget/hooks.js index 9f959f1638..762f432c9f 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.js +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.js @@ -245,7 +245,6 @@ export const editorConfig = ({ setEditorRef, editorContentHtml, images, - isLibrary, placeholder, initializeEditor, openImgModal, @@ -268,9 +267,8 @@ export const editorConfig = ({ imageToolbar, quickbarsInsertToolbar, quickbarsSelectionToolbar, - } = pluginConfig({ isLibrary, placeholder, editorType }); + } = pluginConfig({ learningContextId, placeholder, editorType }); const isLocaleRtl = isRtl(getLocale()); - return { onInit: (evt, editor) => { setEditorRef(editor); diff --git a/src/editors/sharedComponents/TinyMceWidget/index.jsx b/src/editors/sharedComponents/TinyMceWidget/index.jsx index 9294cf0bda..a887ba5773 100644 --- a/src/editors/sharedComponents/TinyMceWidget/index.jsx +++ b/src/editors/sharedComponents/TinyMceWidget/index.jsx @@ -13,6 +13,7 @@ import ImageUploadModal from '../ImageUploadModal'; import SourceCodeModal from '../SourceCodeModal'; import * as hooks from './hooks'; import './customTinyMcePlugins/embedIframePlugin'; +import { isLibraryV1Key } from '../../../generic/key-utils'; export { prepareEditorRef } from './hooks'; @@ -54,7 +55,7 @@ const TinyMceWidget = ({ return ( <> - {!isLibrary && ( + {!isLibraryV1Key(learningContextId) && ( { expect(wrapper.instance.findByType(SourceCodeModal).length).toBe(0); }); test('ImageUploadModal is not rendered', () => { - const wrapper = shallow(); + const wrapper = shallow(); expect(wrapper.snapshot).toMatchSnapshot(); expect(wrapper.instance.findByType(ImageUploadModal).length).toBe(0); }); From 80f23f67bae16d9bf5809ca978858cd66f8ee004 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Thu, 7 Nov 2024 16:19:52 +1100 Subject: [PATCH 3/8] docs: remove jsDoc from axios wrapper --- src/editors/data/services/cms/utils.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/editors/data/services/cms/utils.ts b/src/editors/data/services/cms/utils.ts index 86ba95d902..2e77435cd5 100644 --- a/src/editors/data/services/cms/utils.ts +++ b/src/editors/data/services/cms/utils.ts @@ -18,10 +18,8 @@ export const get: Axios['get'] = (...args) => client().get(...args); export const post: Axios['post'] = (...args) => client().post(...args); /** - * post(url, data) - * simple wrapper providing an authenticated Http client post action - * @param {string} url - target url - * @param {object|string} data - post payload + * put(url, data) + * simple wrapper providing an authenticated Http client put action */ export const put: Axios['put'] = (...args) => client().put(...args); From 8490b78878eb87f75f3beea0770a809f94664491 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Mon, 11 Nov 2024 13:17:08 +1100 Subject: [PATCH 4/8] refactor: use getXBlockAssetsApiUrl to get the library endpoint --- src/editors/data/services/cms/api.test.ts | 4 ++-- src/editors/data/services/cms/api.ts | 4 ++-- src/editors/data/services/cms/urls.ts | 7 +++++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/editors/data/services/cms/api.test.ts b/src/editors/data/services/cms/api.test.ts index 80563a812a..8a0e3ae57f 100644 --- a/src/editors/data/services/cms/api.test.ts +++ b/src/editors/data/services/cms/api.test.ts @@ -125,7 +125,7 @@ describe('cms api', () => { blockId, learningContextId, studioEndpointUrl, pageNumber: 0, }); expect(get).toHaveBeenCalledWith( - urls.libraryAssets({ studioEndpointUrl, blockId }), + urls.libraryAssets({ blockId }), ); }); }); @@ -289,7 +289,7 @@ describe('cms api', () => { asset, }); expect(put).toHaveBeenCalledWith( - `${urls.libraryAssets({ blockId, studioEndpointUrl })}static/${encodeURI(filename)}`, + `${urls.libraryAssets({ blockId, assetName: asset.name })}`, mockFormdata, ); }); diff --git a/src/editors/data/services/cms/api.ts b/src/editors/data/services/cms/api.ts index 20aa84daa4..e9d891d3a5 100644 --- a/src/editors/data/services/cms/api.ts +++ b/src/editors/data/services/cms/api.ts @@ -125,7 +125,7 @@ export const apiMethods = { }): Promise<{ data: AssetResponse & Pagination }> => { if (isLibraryKey(learningContextId)) { return get( - `${urls.libraryAssets({ studioEndpointUrl, blockId })}`, + `${urls.libraryAssets({ blockId })}`, ); } const params = { @@ -157,7 +157,7 @@ export const apiMethods = { if (isLibraryKey(learningContextId)) { data.set('content', asset); return put( - `${urls.libraryAssets({ blockId, studioEndpointUrl })}static/${encodeURI(asset.name)}`, + `${urls.libraryAssets({ blockId, assetName: asset.name })}`, data, ); } diff --git a/src/editors/data/services/cms/urls.ts b/src/editors/data/services/cms/urls.ts index d6dd3e362b..a137ffb9f8 100644 --- a/src/editors/data/services/cms/urls.ts +++ b/src/editors/data/services/cms/urls.ts @@ -1,4 +1,5 @@ import { isLibraryKey, isLibraryV1Key } from '../../../../generic/key-utils'; +import { getXBlockAssetsApiUrl } from '../../../../library-authoring/data/api'; /** * A little helper so we can write the types of these functions more compactly @@ -61,8 +62,10 @@ export const courseAssets = (({ studioEndpointUrl, learningContextId }) => ( `${studioEndpointUrl}/assets/${learningContextId}/` )) satisfies UrlFunction; -export const libraryAssets = (({ blockId, studioEndpointUrl }) => ( - `${studioEndpointUrl}/api/libraries/v2/blocks/${blockId}/assets/` +export const libraryAssets = (({ blockId, assetName }) => ( + assetName + ? `${getXBlockAssetsApiUrl(blockId)}static/${encodeURI(assetName)}` + : `${getXBlockAssetsApiUrl(blockId)}` )) satisfies UrlFunction; export const thumbnailUpload = (({ studioEndpointUrl, learningContextId, videoId }) => ( From 67ab0ab922f7ecd954922fc57d4627ef54874153 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Mon, 11 Nov 2024 13:44:13 +1100 Subject: [PATCH 5/8] fix: set src image to a relative path --- .../sharedComponents/ImageUploadModal/index.jsx | 11 ++++++++++- .../TinyMceWidget/__snapshots__/index.test.jsx.snap | 2 ++ src/editors/sharedComponents/TinyMceWidget/index.jsx | 1 + 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/editors/sharedComponents/ImageUploadModal/index.jsx b/src/editors/sharedComponents/ImageUploadModal/index.jsx index e5a705ce37..9a324787e3 100644 --- a/src/editors/sharedComponents/ImageUploadModal/index.jsx +++ b/src/editors/sharedComponents/ImageUploadModal/index.jsx @@ -21,12 +21,17 @@ export const imgProps = ({ selection, lmsEndpointUrl, editorType, + isLibrary, }) => { let url = selection?.externalUrl; if (url?.startsWith(lmsEndpointUrl) && editorType !== 'expandable') { const sourceEndIndex = lmsEndpointUrl.length; url = url.substring(sourceEndIndex); } + if (isLibrary) { + const index = url.indexOf('static/'); + url = url.substring(index); + } return { src: url, alt: settings.isDecorative ? '' : settings.altText, @@ -36,13 +41,14 @@ export const imgProps = ({ }; export const saveToEditor = ({ - settings, selection, lmsEndpointUrl, editorType, editorRef, + settings, selection, lmsEndpointUrl, editorType, editorRef, isLibrary, }) => { const newImgTag = module.hooks.imgTag({ settings, selection, lmsEndpointUrl, editorType, + isLibrary, }); editorRef.current.execCommand( @@ -130,6 +136,7 @@ const ImageUploadModal = ({ images, editorType, lmsEndpointUrl, + isLibrary, }) => { if (selection && selection.externalUrl) { return ( @@ -148,6 +155,7 @@ const ImageUploadModal = ({ setSelection, lmsEndpointUrl, clearSelection, + isLibrary, }), returnToSelection: clearSelection, }} @@ -190,6 +198,7 @@ ImageUploadModal.propTypes = { images: PropTypes.shape({}).isRequired, lmsEndpointUrl: PropTypes.string.isRequired, editorType: PropTypes.string, + isLibrary: PropTypes.string, }; export const ImageUploadModalInternal = ImageUploadModal; // For testing only diff --git a/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap b/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap index feb3d25635..46672bee9b 100644 --- a/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap +++ b/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap @@ -77,6 +77,7 @@ exports[`TinyMceWidget snapshots SourcecodeModal is not rendered 1`] = ` ], } } + isLibrary={true} isOpen={false} lmsEndpointUrl="http://localhost:18000" selection="hooks.selectedImage.selection" @@ -146,6 +147,7 @@ exports[`TinyMceWidget snapshots renders as expected with default behavior 1`] = ], } } + isLibrary={true} isOpen={false} lmsEndpointUrl="http://localhost:18000" selection="hooks.selectedImage.selection" diff --git a/src/editors/sharedComponents/TinyMceWidget/index.jsx b/src/editors/sharedComponents/TinyMceWidget/index.jsx index a887ba5773..3306f28d63 100644 --- a/src/editors/sharedComponents/TinyMceWidget/index.jsx +++ b/src/editors/sharedComponents/TinyMceWidget/index.jsx @@ -63,6 +63,7 @@ const TinyMceWidget = ({ images={imagesRef} editorType={editorType} lmsEndpointUrl={getConfig().LMS_BASE_URL} + isLibrary {...imageSelection} /> )} From 956d5fd33a6dbfa620ce3679e75f1845890570ca Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Mon, 11 Nov 2024 20:09:53 +1100 Subject: [PATCH 6/8] refactor: separate fetchimages into libraries and courses --- src/editors/EditorPage.test.tsx | 2 +- .../containers/EditorContainer/index.test.tsx | 2 +- .../data/redux/thunkActions/requests.js | 29 +++++--- .../data/redux/thunkActions/requests.test.js | 25 +++---- src/editors/data/services/cms/api.test.ts | 13 ++-- src/editors/data/services/cms/api.ts | 11 ++-- .../__snapshots__/index.test.jsx.snap | 2 + .../SelectImageModal/utils.js | 2 + src/editors/utils/formatLibraryImgRequest.ts | 66 ++++++------------- .../utils/formatLibreryImgRequest.test.ts | 51 ++++---------- src/library-authoring/data/api.ts | 6 ++ 11 files changed, 91 insertions(+), 118 deletions(-) diff --git a/src/editors/EditorPage.test.tsx b/src/editors/EditorPage.test.tsx index e4d0de4091..4ae9817d3b 100644 --- a/src/editors/EditorPage.test.tsx +++ b/src/editors/EditorPage.test.tsx @@ -11,7 +11,7 @@ import EditorPage from './EditorPage'; // Mock this plugins component: jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCss: '' })); // Always mock out the "fetch course images" endpoint: -jest.spyOn(editorCmsApi, 'fetchImages').mockImplementation(async () => ( // eslint-disable-next-line +jest.spyOn(editorCmsApi, 'fetchCourseImages').mockImplementation(async () => ( // eslint-disable-next-line { data: { assets: [], start: 0, end: 0, page: 0, pageSize: 50, totalCount: 0 } } )); // Mock out the 'get ancestors' API: diff --git a/src/editors/containers/EditorContainer/index.test.tsx b/src/editors/containers/EditorContainer/index.test.tsx index 3f427bbebc..a35e4d74b8 100644 --- a/src/editors/containers/EditorContainer/index.test.tsx +++ b/src/editors/containers/EditorContainer/index.test.tsx @@ -13,7 +13,7 @@ import EditorPage from '../../EditorPage'; // Mock this plugins component: jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCss: '' })); // Always mock out the "fetch course images" endpoint: -jest.spyOn(editorCmsApi, 'fetchImages').mockImplementation(async () => ( // eslint-disable-next-line +jest.spyOn(editorCmsApi, 'fetchCourseImages').mockImplementation(async () => ( // eslint-disable-next-line { data: { assets: [], start: 0, end: 0, page: 0, pageSize: 50, totalCount: 0 } } )); // Mock out the 'get ancestors' API: diff --git a/src/editors/data/redux/thunkActions/requests.js b/src/editors/data/redux/thunkActions/requests.js index e3b4379a1e..edff3bf875 100644 --- a/src/editors/data/redux/thunkActions/requests.js +++ b/src/editors/data/redux/thunkActions/requests.js @@ -11,6 +11,7 @@ import { selectors as appSelectors } from '../app'; // eslint-disable-next-line import/no-self-import import * as module from './requests'; import { isLibraryKey } from '../../../../generic/key-utils'; +import { acceptedImgKeys } from '../../../sharedComponents/ImageUploadModal/SelectImageModal/utils'; // Similar to `import { actions, selectors } from '..';` but avoid circular imports: const actions = { requests: requestsActions }; @@ -145,22 +146,34 @@ export const uploadAsset = ({ asset, ...rest }) => (dispatch, getState) => { export const fetchImages = ({ pageNumber, ...rest }) => (dispatch, getState) => { const learningContextId = selectors.app.learningContextId(getState()); + if (isLibraryKey(learningContextId)) { + dispatch(module.networkRequest({ + requestKey: RequestKeys.fetchImages, + promise: api + .fetchLibraryImages({ + pageNumber, + blockId: selectors.app.blockId(getState()), + studioEndpointUrl: selectors.app.studioEndpointUrl(getState()), + learningContextId, + }) + .then(({ data }) => { + const images = getLibraryImageAssets(data.files, Object.keys(acceptedImgKeys)); + return { images, imageCount: Object.keys(images).length }; + }), + ...rest, + })); + return; + } dispatch(module.networkRequest({ requestKey: RequestKeys.fetchImages, promise: api - .fetchImages({ + .fetchCourseImages({ pageNumber, blockId: selectors.app.blockId(getState()), studioEndpointUrl: selectors.app.studioEndpointUrl(getState()), learningContextId, }) - .then(({ data }) => { - if (isLibraryKey(learningContextId)) { - const images = getLibraryImageAssets(data.files); - return { images, imageCount: Object.keys(images).length }; - } - return { images: loadImages(data.assets), imageCount: data.totalCount }; - }), + .then(({ data }) => ({ images: loadImages(data.assets), imageCount: data.totalCount })), ...rest, })); }; diff --git a/src/editors/data/redux/thunkActions/requests.test.js b/src/editors/data/redux/thunkActions/requests.test.js index d389465de9..ece5411780 100644 --- a/src/editors/data/redux/thunkActions/requests.test.js +++ b/src/editors/data/redux/thunkActions/requests.test.js @@ -26,7 +26,8 @@ jest.mock('../../services/cms/api', () => ({ fetchByUnitId: ({ id, url }) => ({ id, url }), fetchCourseDetails: (args) => args, saveBlock: (args) => args, - fetchImages: ({ id, url }) => ({ id, url }), + fetchCourseImages: ({ id, url }) => ({ id, url }), + fetchLibraryImages: ({ id, url }) => ({ id, url }), fetchVideos: ({ id, url }) => ({ id, url }), uploadAsset: (args) => args, loadImages: jest.fn(), @@ -247,16 +248,16 @@ describe('requests thunkActions module', () => { let fetchImages; let loadImages; let dispatchedAction; - beforeEach(() => { - fetchImages = jest.fn((args) => new Promise((resolve) => { - resolve({ data: { assets: { fetchImages: args } } }); - })); - jest.spyOn(api, apiKeys.fetchImages).mockImplementationOnce(fetchImages); - loadImages = jest.spyOn(api, apiKeys.loadImages).mockImplementationOnce(() => ({})); - requests.fetchImages({ ...fetchParams, onSuccess, onFailure })(dispatch, () => testState); - [[dispatchedAction]] = dispatch.mock.calls; - }); describe('courses', () => { + beforeEach(() => { + fetchImages = jest.fn((args) => new Promise((resolve) => { + resolve({ data: { assets: { fetchImages: args } } }); + })); + jest.spyOn(api, apiKeys.fetchCourseImages).mockImplementationOnce(fetchImages); + loadImages = jest.spyOn(api, apiKeys.loadImages).mockImplementationOnce(() => ({})); + requests.fetchImages({ ...fetchParams, onSuccess, onFailure })(dispatch, () => testState); + [[dispatchedAction]] = dispatch.mock.calls; + }); const expectedArgs = { blockId: selectors.app.blockId(testState), studioEndpointUrl: selectors.app.studioEndpointUrl(testState), @@ -288,9 +289,9 @@ describe('requests thunkActions module', () => { beforeEach(() => { jest.spyOn(selectors.app, 'learningContextId').mockImplementationOnce(() => ('lib:demo')); fetchImages = jest.fn((args) => new Promise((resolve) => { - resolve({ data: { assets: { fetchImages: args } } }); + resolve({ data: { files: { fetchImages: args } } }); })); - jest.spyOn(api, apiKeys.fetchImages).mockImplementationOnce(fetchImages); + jest.spyOn(api, apiKeys.fetchLibraryImages).mockImplementationOnce(fetchImages); requests.fetchImages({ ...fetchParams, onSuccess, onFailure, })(dispatch, () => testState); diff --git a/src/editors/data/services/cms/api.test.ts b/src/editors/data/services/cms/api.test.ts index 8a0e3ae57f..c3df32ccbf 100644 --- a/src/editors/data/services/cms/api.test.ts +++ b/src/editors/data/services/cms/api.test.ts @@ -105,10 +105,10 @@ describe('cms api', () => { }); }); - describe('fetchImages', () => { + describe('fetchCourseImages', () => { it('should call get with url.courseAssets', () => { - apiMethods.fetchImages({ - blockId, learningContextId, studioEndpointUrl, pageNumber: 0, + apiMethods.fetchCourseImages({ + learningContextId, studioEndpointUrl, pageNumber: 0, }); const params = { asset_type: 'Images', @@ -119,10 +119,11 @@ describe('cms api', () => { { params }, ); }); + }); + describe('fetchLibraryImages', () => { it('should call get with urls.libraryAssets for library V2', () => { - learningContextId = 'lib:demo2uX'; - apiMethods.fetchImages({ - blockId, learningContextId, studioEndpointUrl, pageNumber: 0, + apiMethods.fetchLibraryImages({ + blockId, }); expect(get).toHaveBeenCalledWith( urls.libraryAssets({ blockId }), diff --git a/src/editors/data/services/cms/api.ts b/src/editors/data/services/cms/api.ts index e9d891d3a5..e979eb4455 100644 --- a/src/editors/data/services/cms/api.ts +++ b/src/editors/data/services/cms/api.ts @@ -117,17 +117,11 @@ export const apiMethods = { fetchStudioView: ({ blockId, studioEndpointUrl }) => get( urls.blockStudioView({ studioEndpointUrl, blockId }), ), - fetchImages: ({ - blockId, + fetchCourseImages: ({ learningContextId, studioEndpointUrl, pageNumber, }): Promise<{ data: AssetResponse & Pagination }> => { - if (isLibraryKey(learningContextId)) { - return get( - `${urls.libraryAssets({ blockId })}`, - ); - } const params = { asset_type: 'Images', page: pageNumber, @@ -137,6 +131,9 @@ export const apiMethods = { { params }, ); }, + fetchLibraryImages: ({ blockId }) => get( + `${urls.libraryAssets({ blockId })}`, + ), fetchVideos: ({ studioEndpointUrl, learningContextId }) => get( urls.courseVideos({ studioEndpointUrl, learningContextId }), ), diff --git a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/__snapshots__/index.test.jsx.snap b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/__snapshots__/index.test.jsx.snap index 587f4ce227..f94b18b185 100644 --- a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/__snapshots__/index.test.jsx.snap +++ b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/__snapshots__/index.test.jsx.snap @@ -9,8 +9,10 @@ exports[`SelectImageModal component snapshot 1`] = ` "jpeg": ".jpeg", "jpg": ".jpg", "png": ".png", + "svg": ".svg", "tif": ".tif", "tiff": ".tiff", + "webp": ".webp", } } close={[MockFunction props.close]} diff --git a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/utils.js b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/utils.js index a94a6ebf24..a1af9fa61b 100644 --- a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/utils.js +++ b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/utils.js @@ -44,4 +44,6 @@ export const acceptedImgKeys = StrictDict({ tif: '.tif', tiff: '.tiff', ico: '.ico', + svg: '.svg', + webp: '.webp', }); diff --git a/src/editors/utils/formatLibraryImgRequest.ts b/src/editors/utils/formatLibraryImgRequest.ts index 6a0580cee9..e5f1780da0 100644 --- a/src/editors/utils/formatLibraryImgRequest.ts +++ b/src/editors/utils/formatLibraryImgRequest.ts @@ -1,25 +1,7 @@ -import StrictDict from './StrictDict'; +import { LibraryAssetResponse } from '../../library-authoring/data/api'; -/** - * A dictionary that maps file extensions to their corresponding MIME types for images. - * - * @example - * acceptedImgMimeTypes.gif // "image/gif" - */ - -const acceptedImgMimeTypes = StrictDict({ - gif: 'image/gif', - jpg: 'image/jpg', - jpeg: 'image/jpeg', - png: 'image/png', - tif: 'image/tiff', - tiff: 'image/tiff', - ico: 'image/x-icon', -}); - -type TinyMCEImageData = { +type GalleryImageData = { displayName: string, - contentType: string, url: string, externalUrl: string, portableUrl: string, @@ -28,12 +10,6 @@ type TinyMCEImageData = { locked: boolean, }; -export type LibraryAssetResponse = { - path: string, - size: number, - url: string, -}; - /** * Extracts the file name from a file path. * This function strips the directory structure and returns the base file name. @@ -49,33 +25,33 @@ export type LibraryAssetResponse = { export const getFileName = (data: LibraryAssetResponse): string => data.path.replace(/^.*[\\/]/, ''); /** - * Determines the MIME type of a file based on its extension. + * Checks if the provided asset data corresponds to an accepted image file type based on its extension. * * @param data - The asset data containing the file path. - * @returns The MIME type of the file, or 'unknown' if the MIME type is not recognized. + * @param acceptedImgExt - The array of accepted image extensions. + * @returns `true` if the file has an accepted image extension, otherwise `false`. * * @example * const data = { path: '/static/example.jpg', size: 12345, url: 'http://example.com/static/example.jpg' }; - * const mimeType = getFileMimeType(data); // "image/jpg" + * const isImg = isImage(data); // Returns true */ -export const getFileMimeType = (data: LibraryAssetResponse): string => { - const ext = data.path.split('.').pop()?.toLowerCase(); // Extract and lowercase the file extension - return ext && acceptedImgMimeTypes[ext] ? acceptedImgMimeTypes[ext] : 'unknown'; +export const isImage = (data: LibraryAssetResponse, acceptedImgExt:string[]): boolean => { + const ext = data.path.split('.').pop()?.toLowerCase() ?? ''; // Extract and lowercase the file extension + return ext !== '' && acceptedImgExt.includes(ext); }; /** - * Parses a `LibraryAssetResponse` into a `TinyMCEImageData` object. - * This includes extracting the file name, MIME type, and constructing other image-related metadata. + * Parses a `LibraryAssetResponse` into a `GalleryImageData` object. + * This includes extracting the file name and constructing other image-related metadata. * * @param data - The asset data to parse. - * @returns The parsed image data with properties like `displayName`, `contentType`, etc. + * @returns The parsed image data with properties like `displayName`, `externalUrl`, etc. * * @example * const data = { path: '/static/example.jpg', size: 12345, url: 'http://example.com/static/example.jpg' }; * const imageData = parseLibraryImageData(data); * // { * // displayName: 'example.jpg', - * // contentType: 'image/jpg', * // url: 'http://example.com/static/example.jpg', * // externalUrl: 'http://example.com/static/example.jpg', * // portableUrl: '/static/example.jpg', @@ -85,9 +61,8 @@ export const getFileMimeType = (data: LibraryAssetResponse): string => { * // } */ -export const parseLibraryImageData = (data: LibraryAssetResponse): TinyMCEImageData => ({ +export const parseLibraryImageData = (data: LibraryAssetResponse): GalleryImageData => ({ displayName: getFileName(data), - contentType: getFileMimeType(data), url: data.url, externalUrl: data.url, portableUrl: data.path, @@ -97,11 +72,12 @@ export const parseLibraryImageData = (data: LibraryAssetResponse): TinyMCEImageD }); /** - * Filters and transforms an array of `LibrariesAssetResponse` objects into a dictionary of `TinyMCEImageData`. - * Only assets with recognized MIME types (i.e., valid image files) are included in the result. + * Filters and transforms an array of `LibrariesAssetResponse` objects into a dictionary of `GalleryImageData`. + * Only assets with recognized extension (i.e., valid image files) are included in the result. * * @param librariesAssets - The array of asset data to process. - * @returns A dictionary where each key is the file name and the value is the corresponding `TinyMCEImageData`. + * @param acceptedImgExt - The array of accepted image extensions. + * @returns A dictionary where each key is the file name and the value is the corresponding `GalleryImageData`. * * @example * const assets = [ @@ -112,7 +88,6 @@ export const parseLibraryImageData = (data: LibraryAssetResponse): TinyMCEImageD * // { * // 'example.jpg': { * // displayName: 'example.jpg', - * // contentType: 'image/jpg', * // url: 'http://example.com/static/example.jpg', * // externalUrl: 'http://example.com/static/example.jpg', * // portableUrl: '/static/example.jpg', @@ -125,10 +100,11 @@ export const parseLibraryImageData = (data: LibraryAssetResponse): TinyMCEImageD export const getLibraryImageAssets = ( librariesAssets: Array, -): Record => librariesAssets.reduce((obj, file) => { - if (getFileMimeType(file) !== 'unknown') { + acceptedImgExt:string[], +): Record => librariesAssets.reduce((obj, file) => { + if (isImage(file, acceptedImgExt)) { const imageData = parseLibraryImageData(file); return { ...obj, [imageData.displayName]: imageData }; } return obj; -}, {} as Record); +}, {} as Record); diff --git a/src/editors/utils/formatLibreryImgRequest.test.ts b/src/editors/utils/formatLibreryImgRequest.test.ts index b4d23a75ef..3e4a05d55c 100644 --- a/src/editors/utils/formatLibreryImgRequest.test.ts +++ b/src/editors/utils/formatLibreryImgRequest.test.ts @@ -1,20 +1,9 @@ import { - parseLibraryImageData, getLibraryImageAssets, getFileMimeType, getFileName, LibraryAssetResponse, + parseLibraryImageData, getLibraryImageAssets, isImage, getFileName, } from './formatLibraryImgRequest'; +import { LibraryAssetResponse } from '../../library-authoring/data/api'; -// Mock the StrictDict function to avoid unnecessary complexity in the test -jest.mock('./StrictDict', () => ({ - __esModule: true, - default: jest.fn().mockReturnValue({ - gif: 'image/gif', - jpg: 'image/jpg', - jpeg: 'image/jpeg', - png: 'image/png', - tif: 'image/tiff', - tiff: 'image/tiff', - ico: 'image/x-icon', - }), -})); +const acceptedImgExt = ['jpg']; describe('parseLibraryImageData', () => { describe('getFileName', () => { @@ -30,27 +19,26 @@ describe('parseLibraryImageData', () => { }); }); - describe('getFileMimeType', () => { - it('should return the correct MIME type for supported file extensions', () => { + describe('isImage', () => { + it('should return true for supported file extensions', () => { const data: LibraryAssetResponse = { path: 'static/example.jpg', size: 12345, url: 'http://example.com/static/example.jpg', }; - - const result = getFileMimeType(data); - expect(result).toBe('image/jpg'); + const result = isImage(data, acceptedImgExt); + expect(result).toBe(true); }); - it('should return "unknown" for unsupported file extensions', () => { + it('should return false for unsupported file extensions', () => { const data: LibraryAssetResponse = { path: '/assets/files/unknown.xyz', size: 12345, url: 'http://example.com/assets/files/unknown.xyz', }; - const result = getFileMimeType(data); - expect(result).toBe('unknown'); + const result = isImage(data, acceptedImgExt); + expect(result).toBe(false); }); }); @@ -65,7 +53,6 @@ describe('parseLibraryImageData', () => { const result = parseLibraryImageData(data); expect(result).toEqual({ displayName: 'example.jpg', - contentType: 'image/jpg', url: 'http://example.com/static/example.jpg', externalUrl: 'http://example.com/static/example.jpg', portableUrl: 'static/example.jpg', @@ -74,31 +61,19 @@ describe('parseLibraryImageData', () => { locked: false, }); }); - - it('should handle unknown MIME types by setting a fallback MIME type', () => { - const data: LibraryAssetResponse = { - path: '/assets/files/unknown.xyz', - size: 12345, - url: 'http://example.com/assets/files/unknown.xyz', - }; - - const result = parseLibraryImageData(data); - expect(result.contentType).toBe('unknown'); - }); }); describe('getLibraryImageAssets', () => { - it('should filter out assets with unsupported MIME types and return a dictionary of valid images', () => { + it('should filter out assets and return a dictionary of valid images', () => { const assets: LibraryAssetResponse[] = [ { path: 'static/example.jpg', size: 12345, url: 'http://example.com/static/example.jpg' }, { path: '/assets/files/unsupported.xyz', size: 67890, url: 'http://example.com/assets/files/unsupported.xyz' }, ]; - const result = getLibraryImageAssets(assets); + const result = getLibraryImageAssets(assets, acceptedImgExt); expect(result).toEqual({ 'example.jpg': { displayName: 'example.jpg', - contentType: 'image/jpg', url: 'http://example.com/static/example.jpg', externalUrl: 'http://example.com/static/example.jpg', portableUrl: 'static/example.jpg', @@ -114,7 +89,7 @@ describe('parseLibraryImageData', () => { { path: '/assets/files/unsupported.xyz', size: 67890, url: 'http://example.com/assets/files/unsupported.xyz' }, ]; - const result = getLibraryImageAssets(assets); + const result = getLibraryImageAssets(assets, acceptedImgExt); expect(result).toEqual({}); }); }); diff --git a/src/library-authoring/data/api.ts b/src/library-authoring/data/api.ts index 06f8c50f06..0c58039a5c 100644 --- a/src/library-authoring/data/api.ts +++ b/src/library-authoring/data/api.ts @@ -183,6 +183,12 @@ export interface GetLibrariesV2CustomParams { search?: string, } +export type LibraryAssetResponse = { + path: string, + size: number, + url: string, +}; + export interface CreateBlockDataRequest { libraryId: string; blockType: string; From 66c94184bec68539da61947d95005c1e448709f3 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Wed, 13 Nov 2024 14:25:33 +1100 Subject: [PATCH 7/8] fix: pass isLibrary to imgProps --- src/editors/sharedComponents/ImageUploadModal/index.jsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/editors/sharedComponents/ImageUploadModal/index.jsx b/src/editors/sharedComponents/ImageUploadModal/index.jsx index 9a324787e3..fea13225c8 100644 --- a/src/editors/sharedComponents/ImageUploadModal/index.jsx +++ b/src/editors/sharedComponents/ImageUploadModal/index.jsx @@ -109,12 +109,14 @@ export const hooks = { selection, lmsEndpointUrl, editorType, + isLibrary, }) => { const props = module.imgProps({ settings, selection, lmsEndpointUrl, editorType, + isLibrary, }); return ``; }, From 93c20c9219dbd50a54bb67dfdecda02da5285912 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Wed, 13 Nov 2024 14:28:46 +1100 Subject: [PATCH 8/8] refactor: use LibraryAssetResponse type in getXBlockAssets --- src/library-authoring/data/api.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library-authoring/data/api.ts b/src/library-authoring/data/api.ts index 0c58039a5c..2efdf9175b 100644 --- a/src/library-authoring/data/api.ts +++ b/src/library-authoring/data/api.ts @@ -445,7 +445,7 @@ export async function publishXBlock(usageKey: string) { * Fetch the asset (static file) list for the given XBlock. */ // istanbul ignore next -export async function getXBlockAssets(usageKey: string): Promise<{ path: string; url: string; size: number }[]> { +export async function getXBlockAssets(usageKey: string): Promise { const { data } = await getAuthenticatedHttpClient().get(getXBlockAssetsApiUrl(usageKey)); return data.files; }