Skip to content

Commit

Permalink
refactor: separate fetchimages into libraries and courses
Browse files Browse the repository at this point in the history
  • Loading branch information
dcoa committed Nov 11, 2024
1 parent 8588bc9 commit ba437c9
Show file tree
Hide file tree
Showing 11 changed files with 91 additions and 118 deletions.
2 changes: 1 addition & 1 deletion src/editors/EditorPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/editors/containers/EditorContainer/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
29 changes: 21 additions & 8 deletions src/editors/data/redux/thunkActions/requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down Expand Up @@ -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,
}));
};
Expand Down
25 changes: 13 additions & 12 deletions src/editors/data/redux/thunkActions/requests.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 7 additions & 6 deletions src/editors/data/services/cms/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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 }),
Expand Down
11 changes: 4 additions & 7 deletions src/editors/data/services/cms/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -137,6 +131,9 @@ export const apiMethods = {
{ params },
);
},
fetchLibraryImages: ({ blockId }) => get(
`${urls.libraryAssets({ blockId })}`,
),
fetchVideos: ({ studioEndpointUrl, learningContextId }) => get(
urls.courseVideos({ studioEndpointUrl, learningContextId }),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,6 @@ export const acceptedImgKeys = StrictDict({
tif: '.tif',
tiff: '.tiff',
ico: '.ico',
svg: '.svg',
webp: '.webp',
});
66 changes: 21 additions & 45 deletions src/editors/utils/formatLibraryImgRequest.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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.
Expand All @@ -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',
Expand All @@ -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,
Expand All @@ -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 = [
Expand All @@ -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',
Expand All @@ -125,10 +100,11 @@ export const parseLibraryImageData = (data: LibraryAssetResponse): TinyMCEImageD

export const getLibraryImageAssets = (
librariesAssets: Array<LibraryAssetResponse>,
): Record<string, TinyMCEImageData> => librariesAssets.reduce((obj, file) => {
if (getFileMimeType(file) !== 'unknown') {
acceptedImgExt:string[],
): Record<string, GalleryImageData> => librariesAssets.reduce((obj, file) => {
if (isImage(file, acceptedImgExt)) {
const imageData = parseLibraryImageData(file);
return { ...obj, [imageData.displayName]: imageData };
}
return obj;
}, {} as Record<string, TinyMCEImageData>);
}, {} as Record<string, GalleryImageData>);
Loading

0 comments on commit ba437c9

Please sign in to comment.