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

refactor: block creation in libraries v2 #1574

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
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
29 changes: 29 additions & 0 deletions src/editors/containers/EditorContainer/hooks.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jest.mock('../../data/redux', () => ({
app: {
isInitialized: (state) => ({ isInitialized: state }),
images: (state) => ({ images: state }),
isCreateBlock: (state) => ({ isCreateBlock: state }),
},
requests: {
isFailed: (...args) => ({ requestFailed: args }),
Expand All @@ -26,6 +27,7 @@ jest.mock('../../hooks', () => ({
...jest.requireActual('../../hooks'),
navigateCallback: jest.fn((args) => ({ navigateCallback: args })),
saveBlock: jest.fn((args) => ({ saveBlock: args })),
createBlock: jest.fn((args) => ({ createBlock: args })),
}));

const dispatch = jest.fn();
Expand Down Expand Up @@ -53,6 +55,8 @@ describe('EditorContainer hooks', () => {
const getContent = () => 'myTestContentValue';
const setAssetToStaticUrl = () => 'myTestContentValue';
const validateEntry = () => 'vaLIdAteENTry';
reactRedux.useSelector.mockReturnValue(false);

const output = hooks.handleSaveClicked({
getContent,
images: {
Expand All @@ -73,6 +77,31 @@ describe('EditorContainer hooks', () => {
validateEntry,
});
});
it('returns callback to createBlock with dispatch and content if isCreateBlock is true', () => {
const getContent = () => 'myTestContentValue';
const setAssetToStaticUrl = () => 'myTestContentValue';
const validateEntry = () => 'vaLIdAteENTry';
reactRedux.useSelector.mockReturnValue(true);

const output = hooks.handleSaveClicked({
getContent,
images: {
portableUrl: '/static/sOmEuiMAge.jpeg',
displayName: 'sOmEuiMAge',
},
destination: 'testDEsTURL',
analytics: 'soMEanALytics',
dispatch,
validateEntry,
});
output();
expect(appHooks.createBlock).toHaveBeenCalledWith({
content: setAssetToStaticUrl(reactRedux.useSelector(selectors.app.images), getContent),
destination: reactRedux.useSelector(selectors.app.returnUrl),
analytics: reactRedux.useSelector(selectors.app.analytics),
dispatch,
});
});
});

describe('cancelConfirmModalToggle', () => {
Expand Down
13 changes: 12 additions & 1 deletion src/editors/containers/EditorContainer/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export const {
navigateCallback,
nullMethod,
saveBlock,
createBlock,
} = appHooks;

export const state = StrictDict({
Expand All @@ -27,10 +28,20 @@ export const handleSaveClicked = ({
}) => {
// eslint-disable-next-line react-hooks/rules-of-hooks
const returnUrl = useSelector(selectors.app.returnUrl);
// eslint-disable-next-line react-hooks/rules-of-hooks
const isCreateBlock = useSelector(selectors.app.isCreateBlock);
const destination = returnFunction ? '' : returnUrl;
// eslint-disable-next-line react-hooks/rules-of-hooks
const analytics = useSelector(selectors.app.analytics);

if (isCreateBlock) {
return () => createBlock({
analytics,
content: getContent({ dispatch }),
destination,
dispatch,
returnFunction,
});
}
return () => saveBlock({
analytics,
content: getContent({ dispatch }),
Expand Down
22 changes: 11 additions & 11 deletions src/editors/containers/EditorContainer/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,6 @@ jest.spyOn(editorCmsApi, 'fetchCourseImages').mockImplementation(async () => ( /
{ data: { assets: [], start: 0, end: 0, page: 0, pageSize: 50, totalCount: 0 } }
));
// Mock out the 'get ancestors' API:
jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({
status: 200,
data: {
ancestors: [{
id: 'block-v1:Org+TS100+24+type@vertical+block@parent',
display_name: 'You-Knit? The Test Unit',
category: 'vertical',
has_children: true,
}],
},
}));

const isDirtyMock = jest.fn();
jest.mock('../TextEditor/hooks', () => ({
Expand Down Expand Up @@ -60,6 +49,17 @@ describe('EditorContainer', () => {
jest.spyOn(window, 'removeEventListener');
jest.spyOn(mockEvent, 'preventDefault');
Object.defineProperty(mockEvent, 'returnValue', { writable: true });
jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({
status: 200,
data: {
ancestors: [{
id: 'block-v1:Org+TS100+24+type@vertical+block@parent',
display_name: 'You-Knit? The Test Unit',
category: 'vertical',
has_children: true,
}],
},
}));
});

afterEach(() => {
Expand Down
11 changes: 9 additions & 2 deletions src/editors/containers/ProblemEditor/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ jest.mock('../../data/redux', () => ({
selectors: {
app: {
blockValue: jest.fn(state => ({ blockValue: state })),
isCreateBlock: jest.fn(state => ({ isCreateBlock: state })),
},
problem: {
problemType: jest.fn(state => ({ problemType: state })),
Expand Down Expand Up @@ -104,12 +105,18 @@ describe('ProblemEditor', () => {
test('blockFinished from requests.isFinished', () => {
expect(
mapStateToProps(testState).blockFinished,
).toEqual(selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchBlock }));
).toEqual(
selectors.app.isCreateBlock(testState)
|| selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchBlock }),
);
});
test('advancedSettingsFinished from requests.isFinished', () => {
expect(
mapStateToProps(testState).advancedSettingsFinished,
).toEqual(selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchAdvancedSettings }));
).toEqual(
selectors.app.isCreateBlock(testState)
|| selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchAdvancedSettings }),
);
});
});
describe('mapDispatchToProps', () => {
Expand Down
6 changes: 4 additions & 2 deletions src/editors/containers/ProblemEditor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,13 @@ const ProblemEditor: React.FC<Props> = ({
};

export const mapStateToProps = (state) => ({
blockFinished: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchBlock }),
blockFinished: selectors.app.isCreateBlock(state)
|| selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchBlock }),
blockFailed: selectors.requests.isFailed(state, { requestKey: RequestKeys.fetchBlock }),
problemType: selectors.problem.problemType(state),
blockValue: selectors.app.blockValue(state),
advancedSettingsFinished: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchAdvancedSettings }),
advancedSettingsFinished: selectors.app.isCreateBlock(state)
|| selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchAdvancedSettings }),
});

export const mapDispatchToProps = {
Expand Down
3 changes: 2 additions & 1 deletion src/editors/containers/TextEditor/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ export const mapStateToProps = (state) => ({
blockFailed: selectors.requests.isFailed(state, { requestKey: RequestKeys.fetchBlock }),
blockId: selectors.app.blockId(state),
showRawEditor: selectors.app.showRawEditor(state),
blockFinished: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchBlock }),
blockFinished: selectors.app.isCreateBlock(state)
|| selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchBlock }),
learningContextId: selectors.app.learningContextId(state),
images: selectors.app.images(state),
isLibrary: selectors.app.isLibrary(state),
Expand Down
4 changes: 3 additions & 1 deletion src/editors/containers/TextEditor/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ jest.mock('../../data/redux', () => ({
selectors: {
app: {
blockValue: jest.fn(state => ({ blockValue: state })),
isCreateBlock: jest.fn(state => ({ isCreateBlock: state })),
lmsEndpointUrl: jest.fn(state => ({ lmsEndpointUrl: state })),
studioEndpointUrl: jest.fn(state => ({ studioEndpointUrl: state })),
showRawEditor: jest.fn(state => ({ showRawEditor: state })),
Expand Down Expand Up @@ -126,7 +127,8 @@ describe('TextEditor', () => {
test('blockFinished from requests.isFinished', () => {
expect(
mapStateToProps(testState).blockFinished,
).toEqual(selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchBlock }));
).toEqual(selectors.app.isCreateBlock(testState)
|| selectors.requests.isFinished(testState, { requestKey: RequestKeys.fetchBlock }));
});
test('learningContextId from app.learningContextId', () => {
expect(
Expand Down
3 changes: 2 additions & 1 deletion src/editors/containers/VideoEditor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const VideoEditor: React.FC<EditorComponent> = ({
(state) => selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchStudioView }),
);
const isLibrary = useSelector(selectors.app.isLibrary) as boolean;
const isCreateBlock = useSelector(selectors.app.isCreateBlock) as boolean;
const {
error,
validateEntry,
Expand All @@ -36,7 +37,7 @@ const VideoEditor: React.FC<EditorComponent> = ({
returnFunction={returnFunction}
validateEntry={validateEntry}
>
{studioViewFinished ? (
{(isCreateBlock || studioViewFinished) ? (
<div className="video-editor">
<VideoEditorModal {...{ isLibrary }} />
</div>
Expand Down
1 change: 1 addition & 0 deletions src/editors/data/constants/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const RequestKeys = StrictDict({
fetchImages: 'fetchImages',
fetchUnit: 'fetchUnit',
fetchStudioView: 'fetchStudioView',
createBlock: 'createBlock',
saveBlock: 'saveBlock',
uploadVideo: 'uploadVideo',
allowThumbnailUpload: 'allowThumbnailUpload',
Expand Down
26 changes: 21 additions & 5 deletions src/editors/data/redux/app/selectors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ describe('app selectors unit tests', () => {
simpleSelectors.unitUrl,
simpleSelectors.blockValue,
selectors.isLibrary,
selectors.isCreateBlock,
]);
});
describe('for library blocks', () => {
Expand All @@ -98,8 +99,8 @@ describe('app selectors unit tests', () => {
};

[
[[null, truthy.blockValue, true] as [any, any, any], true] as const,
[[null, null, true] as [any, any, any], false] as const,
[[null, truthy.blockValue, true, false] as [any, any, any, any], true] as const,
[[null, null, true, false] as [any, any, any, any], false] as const,
].map(([args, expected]) => expect(cb(...args)).toEqual(expected));
});
});
Expand All @@ -112,9 +113,19 @@ describe('app selectors unit tests', () => {
};

[
[[null, truthy.blockValue, false] as [any, any, any], false] as const,
[[truthy.unitUrl, null, false] as [any, any, any], false] as const,
[[truthy.unitUrl, truthy.blockValue, false] as [any, any, any], true] as const,
[[null, truthy.blockValue, false, false] as [any, any, any, any], false] as const,
[[truthy.unitUrl, null, false, false] as [any, any, any, any], false] as const,
[[truthy.unitUrl, truthy.blockValue, false, false] as [any, any, any, any], true] as const,
].map(([args, expected]) => expect(cb(...args)).toEqual(expected));
});
});
describe('component creation workflow', () => {
it('returns true if is isCreateBlock is truthy', () => {
const { resultFunc: cb } = selectors.isInitialized;

[
[[null, null, true, true] as [any, any, any, any], true] as const,
[[null, null, true, true] as [any, any, any, any], true] as const,
].map(([args, expected]) => expect(cb(...args)).toEqual(expected));
});
});
Expand Down Expand Up @@ -184,4 +195,9 @@ describe('app selectors unit tests', () => {
});
});
});
describe('isCreateBlock', () => {
it('should return false if the editor is initialized with a blockId', () => {
expect(selectors.isCreateBlock.resultFunc('block-v1:', 'text')).toEqual(false);
});
});
});
24 changes: 21 additions & 3 deletions src/editors/data/redux/app/selectors.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createSelector } from 'reselect';
import type { EditorState } from '..';
import { blockTypes } from '../../constants/app';
import { isLibraryV1Key } from '../../../../generic/key-utils';
import { isLibraryKey, isLibraryV1Key } from '../../../../generic/key-utils';
import * as urls from '../../services/cms/urls';

export const appSelector = (state: EditorState) => state.app;
Expand Down Expand Up @@ -47,7 +47,19 @@ export const isLibrary = createSelector(
if (isLibraryV1Key(learningContextId)) {
return true;
}
if (blockId && blockId.startsWith('lb:')) {
if ((blockId && blockId.startsWith('lb:')) || isLibraryKey(learningContextId)) {
return true;
}
return false;
},
);

export const isCreateBlock = createSelector(
[simpleSelectors.blockId,
simpleSelectors.blockType,
],
(blockId, blockType) => {
if (blockId === '' && blockType) {
return true;
}
return false;
Expand All @@ -59,8 +71,13 @@ export const isInitialized = createSelector(
simpleSelectors.unitUrl,
simpleSelectors.blockValue,
isLibrary,
isCreateBlock,
],
(unitUrl, blockValue, isLibraryBlock) => {
(unitUrl, blockValue, isLibraryBlock, isCreateEditor) => {
if (isCreateEditor) {
return true;
}

if (isLibraryBlock) {
return !!blockValue;
}
Expand Down Expand Up @@ -105,4 +122,5 @@ export default {
displayTitle,
analytics,
isLibrary,
isCreateBlock,
};
10 changes: 9 additions & 1 deletion src/editors/data/redux/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,22 @@ import { AdvancedProblemType, ProblemType } from '../constants/problem';

export { default as thunkActions } from './thunkActions';

const rootReducer = combineReducers({
const editorReducer = combineReducers({
app: app.reducer,
requests: requests.reducer,
video: video.reducer,
problem: problem.reducer,
game: game.reducer,
});

const rootReducer = (state: any, action: any) => {
if (action.type === 'resetEditor') {
return editorReducer(undefined, action);
}

return editorReducer(state, action);
};

const actions = StrictDict({
app: app.actions,
requests: requests.actions,
Expand Down
23 changes: 23 additions & 0 deletions src/editors/data/redux/thunkActions/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,13 @@ export const fetchCourseDetails = () => (dispatch) => {
*/
export const initialize = (data) => (dispatch) => {
const editorType = data.blockType;
dispatch({ type: 'resetEditor' });
dispatch(actions.app.initialize(data));
if (data.blockId === '' && editorType) {
dispatch(actions.app.initializeEditor());
return;
}

dispatch(module.fetchBlock());
if (data.blockId?.startsWith('block-v1:')) {
dispatch(module.fetchUnit());
Expand Down Expand Up @@ -125,6 +131,22 @@ export const saveBlock = (content, returnToUnit) => (dispatch) => {
}));
};

/**
* @param {func} onSuccess
*/
export const createBlock = (content, returnToUnit) => (dispatch) => {
dispatch(requests.createBlock({
onSuccess: (response) => {
dispatch(actions.app.setBlockId(response.id));
dispatch(saveBlock(content, returnToUnit));
},
onFailure: (error) => dispatch(actions.requests.failRequest({
requestKey: RequestKeys.createBlock,
error,
})),
}));
};

export const uploadAsset = ({ file, setSelection }) => (dispatch) => {
dispatch(requests.uploadAsset({
asset: file,
Expand All @@ -142,4 +164,5 @@ export default StrictDict({
saveBlock,
fetchImages,
uploadAsset,
createBlock,
});
Loading
Loading