fix: redirect to correct learning context (#83)

For https://2u-internal.atlassian.net/browse/TNL-9955
there are multiple instantiations of the text editor in different contexts, so we need to handle them upfront to not break either experience flow” or something along those lines…
This commit is contained in:
connorhaugh
2022-06-09 09:41:31 -04:00
committed by GitHub
parent ca9f788838
commit cc4e19cd2a
17 changed files with 109 additions and 68 deletions

View File

@@ -9,7 +9,7 @@ import * as hooks from './hooks';
import supportedEditors from './supportedEditors';
export const Editor = ({
courseId,
learningContextId,
blockType,
blockId,
lmsEndpointUrl,
@@ -22,7 +22,7 @@ export const Editor = ({
data: {
blockId,
blockType,
courseId,
learningContextId,
lmsEndpointUrl,
studioEndpointUrl,
},
@@ -44,7 +44,7 @@ export const Editor = ({
);
};
Editor.defaultProps = {
courseId: null,
learningContextId: null,
blockId: null,
lmsEndpointUrl: null,
studioEndpointUrl: null,
@@ -52,7 +52,7 @@ Editor.defaultProps = {
};
Editor.propTypes = {
courseId: PropTypes.string,
learningContextId: PropTypes.string,
blockType: PropTypes.string.isRequired,
blockId: PropTypes.string,
lmsEndpointUrl: PropTypes.string,

View File

@@ -17,13 +17,14 @@ jest.mock('./containers/ProblemEditor/ProblemEditor', () => 'ProblemEditor');
const initData = {
blockId: 'block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4',
blockType: blockTypes.html,
courseId: 'course-v1:edX+DemoX+Demo_Course',
learningContextId: 'course-v1:edX+DemoX+Demo_Course',
lmsEndpointUrl: 'evenfakerurl.com',
studioEndpointUrl: 'fakeurl.com',
};
const props = {
initialize: jest.fn(),
onClose: jest.fn().mockName('props.onClose'),
courseId: 'course-v1:edX+DemoX+Demo_Course',
...initData,
};

View File

@@ -17,7 +17,7 @@ export const EditorPage = ({
<Editor
{...{
onClose,
courseId,
learningContextId: courseId,
blockType,
blockId,
lmsEndpointUrl,

View File

@@ -15,7 +15,7 @@ exports[`Editor Page snapshots props besides blockType default to null 1`] = `
<Editor
blockId={null}
blockType="html"
courseId={null}
learningContextId={null}
lmsEndpointUrl={null}
onClose={null}
studioEndpointUrl={null}
@@ -38,7 +38,7 @@ exports[`Editor Page snapshots rendering correctly with expected Input 1`] = `
<Editor
blockId="block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4"
blockType="html"
courseId="course-v1:edX+DemoX+Demo_Course"
learningContextId="course-v1:edX+DemoX+Demo_Course"
lmsEndpointUrl="evenfakerurl.com"
onClose={[MockFunction props.onClose]}
studioEndpointUrl="fakeurl.com"

View File

@@ -11,7 +11,7 @@ const initialState = {
blockId: null,
blockTitle: null,
blockType: null,
courseId: null,
learningContextId: null,
editorInitialized: false,
studioEndpointUrl: null,
lmsEndpointUrl: null,
@@ -27,7 +27,7 @@ const app = createSlice({
studioEndpointUrl: payload.studioEndpointUrl,
lmsEndpointUrl: payload.lmsEndpointUrl,
blockId: payload.blockId,
courseId: payload.courseId,
learningContextId: payload.learningContextId,
blockType: payload.blockType,
}),
setUnitUrl: (state, { payload }) => ({ ...state, unitUrl: payload }),

View File

@@ -19,7 +19,7 @@ describe('app reducer', () => {
studioEndpointUrl: 'testURL',
lmsEndpointUrl: 'sOmEOtherTestuRl',
blockId: 'anID',
courseId: 'OTHERid',
learningContextId: 'OTHERid',
blockType: 'someTYPE',
};
expect(reducer(

View File

@@ -14,7 +14,7 @@ export const simpleSelectors = {
blockId: mkSimpleSelector(app => app.blockId),
blockType: mkSimpleSelector(app => app.blockType),
blockValue: mkSimpleSelector(app => app.blockValue),
courseId: mkSimpleSelector(app => app.courseId),
learningContextId: mkSimpleSelector(app => app.learningContextId),
editorInitialized: mkSimpleSelector(app => app.editorInitialized),
saveResponse: mkSimpleSelector(app => app.saveResponse),
lmsEndpointUrl: mkSimpleSelector(app => app.lmsEndpointUrl),
@@ -24,8 +24,10 @@ export const simpleSelectors = {
};
export const returnUrl = createSelector(
[module.simpleSelectors.unitUrl, module.simpleSelectors.studioEndpointUrl],
(unitUrl, studioEndpointUrl) => (unitUrl ? urls.unit({ studioEndpointUrl, unitUrl }) : ''),
[module.simpleSelectors.unitUrl, module.simpleSelectors.studioEndpointUrl, module.simpleSelectors.learningContextId],
(unitUrl, studioEndpointUrl, learningContextId) => (
urls.returnUrl({ studioEndpointUrl, unitUrl, learningContextId })
),
);
export const isInitialized = createSelector(
@@ -57,10 +59,10 @@ export const analytics = createSelector(
[
module.simpleSelectors.blockId,
module.simpleSelectors.blockType,
module.simpleSelectors.courseId,
module.simpleSelectors.learningContextId,
],
(blockId, blockType, courseId) => (
{ blockId, blockType, courseId }
(blockId, blockType, learningContextId) => (
{ blockId, blockType, learningContextId }
),
);

View File

@@ -7,7 +7,7 @@ jest.mock('reselect', () => ({
createSelector: jest.fn((preSelectors, cb) => ({ preSelectors, cb })),
}));
jest.mock('../../services/cms/urls', () => ({
unit: (args) => ({ unit: args }),
returnUrl: (args) => ({ returnUrl: args }),
}));
const testState = { some: 'arbitraryValue' };
@@ -39,7 +39,7 @@ describe('app selectors unit tests', () => {
simpleKeys.blockTitle,
simpleKeys.blockType,
simpleKeys.blockValue,
simpleKeys.courseId,
simpleKeys.learningContextId,
simpleKeys.editorInitialized,
simpleKeys.saveResponse,
simpleKeys.studioEndpointUrl,
@@ -53,14 +53,19 @@ describe('app selectors unit tests', () => {
expect(selectors.returnUrl.preSelectors).toEqual([
simpleSelectors.unitUrl,
simpleSelectors.studioEndpointUrl,
simpleSelectors.learningContextId,
]);
});
it('returns urls.unit with the unitUrl if loaded, else an empty string', () => {
it('returns urls.returnUrl with the returnUrl', () => {
const { cb } = selectors.returnUrl;
const studioEndpointUrl = 'baseURL';
const unitUrl = 'some unit url';
expect(cb(null, studioEndpointUrl)).toEqual('');
expect(cb(unitUrl, studioEndpointUrl)).toEqual(urls.unit({ unitUrl, studioEndpointUrl }));
const learningContextId = 'some learning context';
expect(
cb(unitUrl, studioEndpointUrl, learningContextId),
).toEqual(
urls.returnUrl({ unitUrl, studioEndpointUrl, learningContextId }),
);
});
});
describe('isInitialized selector', () => {

View File

@@ -20,7 +20,7 @@ export const fetchUnit = () => (dispatch) => {
/**
* @param {string} studioEndpointUrl
* @param {string} blockId
* @param {string} courseId
* @param {string} learningContextId
* @param {string} blockType
*/
export const initialize = (data) => (dispatch) => {

View File

@@ -79,7 +79,7 @@ export const saveBlock = ({ content, ...rest }) => (dispatch, getState) => {
promise: api.saveBlock({
blockId: selectors.app.blockId(getState()),
blockType: selectors.app.blockType(getState()),
courseId: selectors.app.courseId(getState()),
learningContextId: selectors.app.learningContextId(getState()),
content,
studioEndpointUrl: selectors.app.studioEndpointUrl(getState()),
title: selectors.app.blockTitle(getState()),
@@ -91,7 +91,7 @@ export const uploadImage = ({ image, ...rest }) => (dispatch, getState) => {
dispatch(module.networkRequest({
requestKey: RequestKeys.uploadImage,
promise: api.uploadImage({
courseId: selectors.app.courseId(getState()),
learningContextId: selectors.app.learningContextId(getState()),
image,
studioEndpointUrl: selectors.app.studioEndpointUrl(getState()),
}),
@@ -104,7 +104,7 @@ export const fetchImages = ({ ...rest }) => (dispatch, getState) => {
requestKey: RequestKeys.fetchImages,
promise: api.fetchImages({
studioEndpointUrl: selectors.app.studioEndpointUrl(getState()),
courseId: selectors.app.courseId(getState()),
learningContextId: selectors.app.learningContextId(getState()),
}).then((response) => loadImages(response.data.assets)),
...rest,
}));

View File

@@ -12,7 +12,7 @@ jest.mock('../app/selectors', () => ({
studioEndpointUrl: (state) => ({ studioEndpointUrl: state }),
blockId: (state) => ({ blockId: state }),
blockType: (state) => ({ blockType: state }),
courseId: (state) => ({ courseId: state }),
learningContextId: (state) => ({ learningContextId: state }),
blockTitle: (state) => ({ title: state }),
}));
@@ -198,7 +198,7 @@ describe('requests thunkActions module', () => {
let dispatchedAction;
const expectedArgs = {
studioEndpointUrl: selectors.app.studioEndpointUrl(testState),
courseId: selectors.app.courseId(testState),
learningContextId: selectors.app.learningContextId(testState),
};
beforeEach(() => {
fetchImages = jest.fn((args) => new Promise((resolve) => {
@@ -216,7 +216,7 @@ describe('requests thunkActions module', () => {
expect(dispatchedAction.networkRequest.onSuccess).toEqual(onSuccess);
expect(dispatchedAction.networkRequest.onFailure).toEqual(onFailure);
});
test('api.fetchImages promise called with studioEndpointUrl and courseId', () => {
test('api.fetchImages promise called with studioEndpointUrl and learningContextId', () => {
expect(fetchImages).toHaveBeenCalledWith(expectedArgs);
});
test('promise is chained with api.loadImages', () => {
@@ -236,7 +236,7 @@ describe('requests thunkActions module', () => {
promise: api.saveBlock({
blockId: selectors.app.blockId(testState),
blockType: selectors.app.blockType(testState),
courseId: selectors.app.courseId(testState),
learningContextId: selectors.app.learningContextId(testState),
content,
studioEndpointUrl: selectors.app.studioEndpointUrl(testState),
title: selectors.app.blockTitle(testState),
@@ -255,7 +255,7 @@ describe('requests thunkActions module', () => {
...fetchParams,
requestKey: RequestKeys.uploadImage,
promise: api.uploadImage({
courseId: selectors.app.courseId(testState),
learningContextId: selectors.app.learningContextId(testState),
image,
studioEndpointUrl: selectors.app.studioEndpointUrl(testState),
}),

View File

@@ -11,18 +11,18 @@ export const apiMethods = {
fetchByUnitId: ({ blockId, studioEndpointUrl }) => get(
urls.blockAncestor({ studioEndpointUrl, blockId }),
),
fetchImages: ({ courseId, studioEndpointUrl }) => get(
urls.courseImages({ studioEndpointUrl, courseId }),
fetchImages: ({ learningContextId, studioEndpointUrl }) => get(
urls.courseImages({ studioEndpointUrl, learningContextId }),
),
uploadImage: ({
courseId,
learningContextId,
studioEndpointUrl,
image,
}) => {
const data = new FormData();
data.append('file', image);
return post(
urls.courseAssets({ studioEndpointUrl, courseId }),
urls.courseAssets({ studioEndpointUrl, learningContextId }),
data,
);
},
@@ -30,13 +30,13 @@ export const apiMethods = {
blockId,
blockType,
content,
courseId,
learningContextId,
title,
}) => {
if (blockType === 'html') {
return {
category: blockType,
couseKey: courseId,
couseKey: learningContextId,
data: content,
has_changes: true,
id: blockId,
@@ -49,7 +49,7 @@ export const apiMethods = {
blockId,
blockType,
content,
courseId,
learningContextId,
studioEndpointUrl,
title,
}) => post(
@@ -58,7 +58,7 @@ export const apiMethods = {
blockType,
content,
blockId,
courseId,
learningContextId,
title,
}),
),

View File

@@ -30,7 +30,7 @@ const { apiMethods } = api;
const blockId = 'coursev1:2uX@4345432';
const content = 'Im baby palo santo ugh celiac fashion axe. La croix lo-fi venmo whatever. Beard man braid migas single-origin coffee forage ramps.';
const courseId = 'demo2uX';
const learningContextId = 'demo2uX';
const studioEndpointUrl = 'hortus.coa';
const title = 'remember this needs to go into metadata to save';
@@ -52,8 +52,8 @@ describe('cms api', () => {
describe('fetchImages', () => {
it('should call get with url.courseImages', () => {
apiMethods.fetchImages({ courseId, studioEndpointUrl });
expect(get).toHaveBeenCalledWith(urls.courseImages({ studioEndpointUrl, courseId }));
apiMethods.fetchImages({ learningContextId, studioEndpointUrl });
expect(get).toHaveBeenCalledWith(urls.courseImages({ studioEndpointUrl, learningContextId }));
});
});
@@ -63,11 +63,11 @@ describe('cms api', () => {
blockId,
blockType: 'html',
content,
courseId,
learningContextId,
title,
})).toEqual({
category: 'html',
couseKey: courseId,
couseKey: learningContextId,
data: content,
has_changes: true,
id: blockId,
@@ -86,7 +86,7 @@ describe('cms api', () => {
blockId,
blockType: 'html',
content,
courseId,
learningContextId,
studioEndpointUrl,
title,
});
@@ -96,7 +96,7 @@ describe('cms api', () => {
blockType: 'html',
content,
blockId,
courseId,
learningContextId,
title,
}),
);
@@ -109,12 +109,12 @@ describe('cms api', () => {
const mockFormdata = new FormData();
mockFormdata.append('file', image);
apiMethods.uploadImage({
courseId,
learningContextId,
studioEndpointUrl,
image,
});
expect(post).toHaveBeenCalledWith(
urls.courseAssets({ studioEndpointUrl, courseId }),
urls.courseAssets({ studioEndpointUrl, learningContextId }),
mockFormdata,
);
});

View File

@@ -19,7 +19,7 @@ export const fetchByUnitId = ({ blockId, studioEndpointUrl }) => mockPromise({
data: { ancestors: [{ id: 'unitUrl' }] },
});
// eslint-disable-next-line
export const fetchImages = ({ courseId, studioEndpointUrl }) => mockPromise({
export const fetchImages = ({ learningContextId, studioEndpointUrl }) => mockPromise({
data: {
assets: [
{
@@ -74,13 +74,13 @@ export const normalizeContent = ({
blockId,
blockType,
content,
courseId,
learningContextId,
title,
}) => {
if (blockType === 'html') {
return {
category: blockType,
couseKey: courseId,
couseKey: learningContextId,
data: content,
has_changes: true,
id: blockId,
@@ -94,7 +94,7 @@ export const saveBlock = ({
blockId,
blockType,
content,
courseId,
learningContextId,
studioEndpointUrl,
title,
}) => mockPromise({
@@ -103,17 +103,17 @@ export const saveBlock = ({
blockType,
content,
blockId,
courseId,
learningContextId,
title,
}),
});
export const uploadImage = ({
courseId,
learningContextId,
studioEndpointUrl,
// image,
}) => mockPromise({
url: urls.courseAssets({ studioEndpointUrl, courseId }),
url: urls.courseAssets({ studioEndpointUrl, learningContextId }),
image: {
asset: {
display_name: 'journey_escape.jpg',

View File

@@ -1,7 +1,20 @@
export const libraryV1 = ({ studioEndpointUrl, learningContextId }) => (
`${studioEndpointUrl}/library/${learningContextId}`
);
export const unit = ({ studioEndpointUrl, unitUrl }) => (
`${studioEndpointUrl}/container/${unitUrl.data.ancestors[0].id}`
);
export const returnUrl = ({ studioEndpointUrl, unitUrl, learningContextId }) => {
if (learningContextId && learningContextId.includes('library-v1')) {
// when the learning context is a v1 library, return to the library page
return libraryV1({ studioEndpointUrl, learningContextId });
}
// when the learning context is a course, return to the unit page
return unitUrl ? unit({ studioEndpointUrl, unitUrl }) : '';
};
export const block = ({ studioEndpointUrl, blockId }) => (
`${studioEndpointUrl}/xblock/${blockId}`
);
@@ -10,10 +23,10 @@ export const blockAncestor = ({ studioEndpointUrl, blockId }) => (
`${block({ studioEndpointUrl, blockId })}?fields=ancestorInfo`
);
export const courseAssets = ({ studioEndpointUrl, courseId }) => (
`${studioEndpointUrl}/assets/${courseId}/`
export const courseAssets = ({ studioEndpointUrl, learningContextId }) => (
`${studioEndpointUrl}/assets/${learningContextId}/`
);
export const courseImages = ({ studioEndpointUrl, courseId }) => (
`${courseAssets({ studioEndpointUrl, courseId })}?sort=uploadDate&direction=desc&asset_type=Images`
export const courseImages = ({ studioEndpointUrl, learningContextId }) => (
`${courseAssets({ studioEndpointUrl, learningContextId })}?sort=uploadDate&direction=desc&asset_type=Images`
);

View File

@@ -1,5 +1,7 @@
import {
returnUrl,
unit,
libraryV1,
block,
blockAncestor,
courseAssets,
@@ -9,8 +11,10 @@ import {
describe('cms url methods', () => {
const studioEndpointUrl = 'urLgoeStOstudiO';
const blockId = 'blOckIDTeST123';
const courseId = 'coUrseiD321';
describe('unit', () => {
const learningContextId = 'lEarnIngCOntextId123';
const courseId = 'course-v1:courseId123';
const libraryV1Id = 'library-v1:libaryId123';
describe('return to learning context urls', () => {
const unitUrl = {
data: {
ancestors: [
@@ -20,6 +24,22 @@ describe('cms url methods', () => {
],
},
};
it('returns the library page when given the library', () => {
expect(returnUrl({ studioEndpointUrl, unitUrl, learningContextId: libraryV1Id }))
.toEqual(`${studioEndpointUrl}/library/${libraryV1Id}`);
});
it('returns url with studioEndpointUrl and unitUrl', () => {
expect(returnUrl({ studioEndpointUrl, unitUrl, learningContextId: courseId }))
.toEqual(`${studioEndpointUrl}/container/${unitUrl.data.ancestors[0].id}`);
});
it('returns empty string if no unit url', () => {
expect(returnUrl({ studioEndpointUrl, unitUrl: null, learningContextId: courseId }))
.toEqual('');
});
it('returns the library page when given the library', () => {
expect(libraryV1({ studioEndpointUrl, learningContextId: libraryV1Id }))
.toEqual(`${studioEndpointUrl}/library/${libraryV1Id}`);
});
it('returns url with studioEndpointUrl and unitUrl', () => {
expect(unit({ studioEndpointUrl, unitUrl }))
.toEqual(`${studioEndpointUrl}/container/${unitUrl.data.ancestors[0].id}`);
@@ -38,15 +58,15 @@ describe('cms url methods', () => {
});
});
describe('courseAssets', () => {
it('returns url with studioEndpointUrl and courseId', () => {
expect(courseAssets({ studioEndpointUrl, courseId }))
.toEqual(`${studioEndpointUrl}/assets/${courseId}/`);
it('returns url with studioEndpointUrl and learningContextId', () => {
expect(courseAssets({ studioEndpointUrl, learningContextId }))
.toEqual(`${studioEndpointUrl}/assets/${learningContextId}/`);
});
});
describe('courseImages', () => {
it('returns url with studioEndpointUrl, courseId and courseAssets query', () => {
expect(courseImages({ studioEndpointUrl, courseId }))
.toEqual(`${courseAssets({ studioEndpointUrl, courseId })}?sort=uploadDate&direction=desc&asset_type=Images`);
it('returns url with studioEndpointUrl, learningContextId and courseAssets query', () => {
expect(courseImages({ studioEndpointUrl, learningContextId }))
.toEqual(`${courseAssets({ studioEndpointUrl, learningContextId })}?sort=uploadDate&direction=desc&asset_type=Images`);
});
});
});

View File

@@ -14,7 +14,7 @@ export const EditorGallery = () => {
const type = blockTypes[blockTypeKey];
return { ...obj, [type]: mockBlockIdByType(type) };
}, {});
const courseId = 'fake-course-id';
const courseId = 'library-v1:EDX+apmjoemaonmeonaoenan';
const studioEndpointUrl = 'fake-studio-endpoint-url';
const lmsEndpointUrl = 'https://courses.edx.org'; // this is hardcoded because that is where the image data is from.
const handleChange = (e) => setBlockType(e.target.value);