Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: upload images from the TinyMCE in library editor #1458

Merged
merged 8 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
1 change: 1 addition & 0 deletions src/editors/data/redux/app/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => ({
Expand Down
8 changes: 2 additions & 6 deletions src/editors/data/redux/thunkActions/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to resetImages() here, any why only for libraries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I was testing the functionality I notice that closing the modal don't clean the state, so I can see the images of one component in the other one.

My assumption about why we can see that behavior in courses is not happening in courses because we have a change in the URL (from the legacy unit view to the MFE) that clean the state of the component.

dispatch(module.fetchImages({ pageNumber: 0 }));
break;
default:
break;
Expand Down
40 changes: 36 additions & 4 deletions src/editors/data/redux/thunkActions/requests.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -10,6 +10,8 @@ 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';
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 @@ -121,25 +123,55 @@ 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());
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: selectors.app.learningContextId(getState()),
learningContextId,
})
.then(({ data }) => ({ images: loadImages(data.assets), imageCount: data.totalCount })),
...rest,
Expand Down
154 changes: 115 additions & 39 deletions src/editors/data/redux/thunkActions/requests.test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down 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 All @@ -40,6 +41,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;
Expand Down Expand Up @@ -241,31 +248,59 @@ 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 } } });
}));
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;
});
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', () => {
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),
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: { files: { fetchImages: args } } });
}));
jest.spyOn(api, apiKeys.fetchLibraryImages).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', () => {
Expand Down Expand Up @@ -316,21 +351,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';
Expand Down
Loading