From f37935c05518af7c442d8937704a6269c6dbcf47 Mon Sep 17 00:00:00 2001 From: Ken Clary Date: Thu, 22 Jun 2023 11:41:48 -0400 Subject: [PATCH] feat: more/correct v2 url handling. TNL-10742 --- .../containers/EditorContainer/hooks.js | 4 +-- src/editors/data/services/cms/api.js | 2 +- src/editors/data/services/cms/urls.js | 31 ++++++++++++++----- src/editors/data/services/cms/urls.test.js | 19 ++++++++++-- 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/editors/containers/EditorContainer/hooks.js b/src/editors/containers/EditorContainer/hooks.js index 3d245a4f6..b0385b6e5 100644 --- a/src/editors/containers/EditorContainer/hooks.js +++ b/src/editors/containers/EditorContainer/hooks.js @@ -27,7 +27,7 @@ export const handleSaveClicked = ({ returnFunction, }) => { // eslint-disable-next-line react-hooks/rules-of-hooks - const destination = useSelector(selectors.app.returnUrl); + const destination = returnFunction ? '' : useSelector(selectors.app.returnUrl); // eslint-disable-next-line react-hooks/rules-of-hooks const analytics = useSelector(selectors.app.analytics); @@ -57,7 +57,7 @@ export const handleCancel = ({ onClose, returnFunction }) => { return navigateCallback({ returnFunction, // eslint-disable-next-line react-hooks/rules-of-hooks - destination: useSelector(selectors.app.returnUrl), + destination: returnFunction ? '' : useSelector(selectors.app.returnUrl), analyticsEvent: analyticsEvt.editorCancelClick, // eslint-disable-next-line react-hooks/rules-of-hooks analytics: useSelector(selectors.app.analytics), diff --git a/src/editors/data/services/cms/api.js b/src/editors/data/services/cms/api.js index ec0c98a2f..b1e407ad6 100644 --- a/src/editors/data/services/cms/api.js +++ b/src/editors/data/services/cms/api.js @@ -143,7 +143,7 @@ export const apiMethods = { response = { data: content.olx, category: blockType, - couseKey: learningContextId, + courseKey: learningContextId, has_changes: true, id: blockId, metadata: { display_name: title, ...content.settings }, diff --git a/src/editors/data/services/cms/urls.js b/src/editors/data/services/cms/urls.js index f73b255af..ca9fc5c5d 100644 --- a/src/editors/data/services/cms/urls.js +++ b/src/editors/data/services/cms/urls.js @@ -7,26 +7,41 @@ export const unit = ({ studioEndpointUrl, unitUrl }) => ( ); export const returnUrl = ({ studioEndpointUrl, unitUrl, learningContextId }) => { - if (learningContextId && learningContextId.includes('library-v1')) { + if (learningContextId && learningContextId.startsWith('library-v1')) { // when the learning context is a v1 library, return to the library page return libraryV1({ studioEndpointUrl, learningContextId }); } + if (learningContextId && learningContextId.startsWith('lib')) { + // when it's a v2 library, there will be no return url (instead a closed popup) + throw new Error('Return url not available (or needed) for V2 libraries'); + } // when the learning context is a course, return to the unit page - return unitUrl ? unit({ studioEndpointUrl, unitUrl }) : ''; + if (unitUrl) { + return unit({ studioEndpointUrl, unitUrl }); + } else { + throw new Error('No unit url for return url'); + } }; export const block = ({ studioEndpointUrl, blockId }) => ( - blockId.includes('block-v1') + blockId.startsWith('block-v1') ? `${studioEndpointUrl}/xblock/${blockId}` - : `${studioEndpointUrl}/api/xblock/v2/xblocks/${blockId}` + : `${studioEndpointUrl}/api/xblock/v2/xblocks/${blockId}/fields/` ); -export const blockAncestor = ({ studioEndpointUrl, blockId }) => ( - `${block({ studioEndpointUrl, blockId })}?fields=ancestorInfo` -); +export const blockAncestor = ({ studioEndpointUrl, blockId }) => { + if blockId.startsWith('block-v1') { + return `${block({studioEndpointUrl, blockId})}?fields=ancestorInfo`; + } else { + // this url only need to get info to build the return url, which isn't used by V2 blocks + throw new Error('Block ancestor not available (and not needed) for V2 blocks'); + } +}; export const blockStudioView = ({ studioEndpointUrl, blockId }) => ( - `${block({ studioEndpointUrl, blockId })}/studio_view` + blockId.startsWith('block-v1') + ? `${block({ studioEndpointUrl, blockId })}/studio_view` + : `${studioEndpointUrl}/api/xblock/v2/xblocks/${blockId}/view/studio_view/` ); export const courseAssets = ({ studioEndpointUrl, learningContextId }) => ( diff --git a/src/editors/data/services/cms/urls.test.js b/src/editors/data/services/cms/urls.test.js index 2cee79f62..ae2cf40d6 100644 --- a/src/editors/data/services/cms/urls.test.js +++ b/src/editors/data/services/cms/urls.test.js @@ -26,6 +26,7 @@ describe('cms url methods', () => { const learningContextId = 'lEarnIngCOntextId123'; const courseId = 'course-v1:courseId123'; const libraryV1Id = 'library-v1:libaryId123'; + const libraryV2Id = 'lib:libaryId123'; const language = 'la'; const handout = '/aSSet@hANdoUt'; const videoId = '123-SOmeVidEOid-213'; @@ -41,10 +42,14 @@ describe('cms url methods', () => { ], }, }; - it('returns the library page when given the library', () => { + it('returns the library page when given the v1 library', () => { expect(returnUrl({ studioEndpointUrl, unitUrl, learningContextId: libraryV1Id })) .toEqual(`${studioEndpointUrl}/library/${libraryV1Id}`); }); + it('returns empty string when given the v2 library', () => { + expect(returnUrl({ studioEndpointUrl, unitUrl, learningContextId: libraryV2Id })) + .toEqual(''); + }); it('returns url with studioEndpointUrl and unitUrl', () => { expect(returnUrl({ studioEndpointUrl, unitUrl, learningContextId: courseId })) .toEqual(`${studioEndpointUrl}/container/${unitUrl.data.ancestors[0].id}`); @@ -69,7 +74,7 @@ describe('cms url methods', () => { }); it('returns v2 url with studioEndpointUrl and v2BlockId', () => { expect(block({ studioEndpointUrl, blockId: v2BlockId })) - .toEqual(`${studioEndpointUrl}/api/xblock/v2/xblocks/${v2BlockId}`); + .toEqual(`${studioEndpointUrl}/api/xblock/v2/xblocks/${v2BlockId}/fields/`); }); }); describe('blockAncestor', () => { @@ -77,12 +82,20 @@ describe('cms url methods', () => { expect(blockAncestor({ studioEndpointUrl, blockId })) .toEqual(`${block({ studioEndpointUrl, blockId })}?fields=ancestorInfo`); }); + it('returns blank url with studioEndpointUrl, v2 blockId and ancestor query', () => { + expect(blockAncestor({ studioEndpointUrl, blockId: v2BlockId })) + .toEqual(''); + }); }); describe('blockStudioView', () => { - it('returns url with studioEndpointUrl, blockId and studio_view query', () => { + it('returns v1 url with studioEndpointUrl, blockId and studio_view query', () => { expect(blockStudioView({ studioEndpointUrl, blockId })) .toEqual(`${block({ studioEndpointUrl, blockId })}/studio_view`); }); + it('returns v2 url with studioEndpointUrl, v2 blockId and studio_view query', () => { + expect(blockStudioView({ studioEndpointUrl, blockId: v2BlockId })) + .toEqual(`${studioEndpointUrl}/api/xblock/v2/xblocks/${v2BlockId}/view/studio_view/`); + }); }); describe('courseAssets', () => {