Merge pull request #355 from openedx/kenclary/TNL-10743

feat: more/correct v2 url handling. TNL-10742

Author: kenclary

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),

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 },

src/editors/data/services/cms/urls.js

@@ -7,26 +7,39 @@ 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 });
+  }
+  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`;
+  }
+  // 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 }) => (

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,17 +42,21 @@ 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('throws error when given the v2 library', () => {
+      expect(() => { returnUrl({ studioEndpointUrl, unitUrl, learningContextId: libraryV2Id }); })
+        .toThrow('Return url not available (or needed) for V2 libraries');
+    });
     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('throws error if no unit url', () => {
+      expect(() => { returnUrl({ studioEndpointUrl, unitUrl: null, learningContextId: courseId }); })
+        .toThrow('No unit url for return url');
     });
     it('returns the library page when given the library', () => {
       expect(libraryV1({ studioEndpointUrl, learningContextId: libraryV1Id }))
@@ -69,20 +74,28 @@ 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', () => {
     it('returns url with studioEndpointUrl, blockId and ancestor query', () => {
       expect(blockAncestor({ studioEndpointUrl, blockId }))
         .toEqual(`${block({ studioEndpointUrl, blockId })}?fields=ancestorInfo`);
     });
+    it('throws error with studioEndpointUrl, v2 blockId and ancestor query', () => {
+      expect(() => { blockAncestor({ studioEndpointUrl, blockId: v2BlockId }); })
+        .toThrow('Block ancestor not available (and not needed) for V2 blocks');
+    });
   });
   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', () => {