feat: Enable the Video editor in content libraries [FC-0062] (#1319)

* feat: enable video editor in libraries * fix: a11y issue in video editor - URL and ID fields were combined * test: tests for video editor
2024-09-24 19:11:06 -07:00
parent 95521d3b8d
commit 5f41db83c2
8 changed files with 143 additions and 102 deletions
--- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoSourceWidget/snapshots/index.test.jsx.snap
+++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoSourceWidget/snapshots/index.test.jsx.snap
@@ -16,10 +16,10 @@ exports[`VideoSourceWidget snapshots snapshots: renders as expected with default
      id="authoring.videoeditor.videoIdChangeAlert.message"
    />
  </ErrorAlert>
-  <Form.Group>
-    <div
-      className="border-primary-100 border-bottom pb-4"
-    >
+  <div
+    className="border-primary-100 border-bottom pb-4"
+  >
+    <Form.Group>
      <Form.Control
        floatingLabel="Video ID"
        onBlur={[Function]}
@@ -35,6 +35,8 @@ exports[`VideoSourceWidget snapshots snapshots: renders as expected with default
          id="authoring.videoeditor.videoSource.videoId.feedback"
        />
      </Form.Control.Feedback>
+    </Form.Group>
+    <Form.Group>
      <Form.Control
        floatingLabel="Video URL"
        onBlur={[Function]}
@@ -51,31 +53,33 @@ exports[`VideoSourceWidget snapshots snapshots: renders as expected with default
          id="authoring.videoeditor.videoSource.videoUrl.feedback"
        />
      </Form.Control.Feedback>
-    </div>
-    <div
-      className="mt-4"
-    >
-      <FormattedMessage
-        defaultMessage="Fallback videos"
-        description="Title for the fallback videos section"
-        id="authoring.videoeditor.videoSource.fallbackVideo.title"
-      />
-    </div>
-    <div
-      className="mt-3"
-    >
-      <FormattedMessage
-        defaultMessage="To be sure all learners can access the video, edX
+    </Form.Group>
+  </div>
+  <div
+    className="mt-4"
+  >
+    <FormattedMessage
+      defaultMessage="Fallback videos"
+      description="Title for the fallback videos section"
+      id="authoring.videoeditor.videoSource.fallbackVideo.title"
+    />
+  </div>
+  <div
+    className="mt-3"
+  >
+    <FormattedMessage
+      defaultMessage="To be sure all learners can access the video, edX
    recommends providing additional videos in both .mp4 and
    .webm formats.  The first listed video compatible with the
    learner's device will play."
-        description="Test explaining reason for fallback videos"
-        id="authoring.videoeditor.videoSource.fallbackVideo.message"
-      />
-    </div>
-    <Form.Row
-      className="mt-3.5 mx-0 flex-nowrap"
-    >
+      description="Test explaining reason for fallback videos"
+      id="authoring.videoeditor.videoSource.fallbackVideo.message"
+    />
+  </div>
+  <Form.Row
+    className="mt-3.5 mx-0 flex-nowrap"
+  >
+    <Form.Group>
      <Form.Control
        floatingLabel="Video URL"
      />
@@ -87,10 +91,12 @@ exports[`VideoSourceWidget snapshots snapshots: renders as expected with default
        tooltipContent="Delete"
        tooltipPlacement="top"
      />
-    </Form.Row>
-    <ActionRow
-      className="mt-4.5"
-    >
+    </Form.Group>
+  </Form.Row>
+  <ActionRow
+    className="mt-4.5"
+  >
+    <Form.Group>
      <Form.Checkbox
        checked={false}
        className="decorative-control-label"
@@ -132,9 +138,9 @@ exports[`VideoSourceWidget snapshots snapshots: renders as expected with default
          }
        />
      </OverlayTrigger>
-      <ActionRow.Spacer />
-    </ActionRow>
-  </Form.Group>
+    </Form.Group>
+    <ActionRow.Spacer />
+  </ActionRow>
  <div
    className="my-4 border-primary-100 border-bottom"
  />
@@ -169,10 +175,10 @@ exports[`VideoSourceWidget snapshots snapshots: renders as expected with videoSh
      id="authoring.videoeditor.videoIdChangeAlert.message"
    />
  </ErrorAlert>
-  <Form.Group>
-    <div
-      className="border-primary-100 border-bottom pb-4"
-    >
+  <div
+    className="border-primary-100 border-bottom pb-4"
+  >
+    <Form.Group>
      <Form.Control
        floatingLabel="Video ID"
        onBlur={[Function]}
@@ -188,6 +194,8 @@ exports[`VideoSourceWidget snapshots snapshots: renders as expected with videoSh
          id="authoring.videoeditor.videoSource.videoId.feedback"
        />
      </Form.Control.Feedback>
+    </Form.Group>
+    <Form.Group>
      <Form.Control
        floatingLabel="Video URL"
        onBlur={[Function]}
@@ -204,31 +212,33 @@ exports[`VideoSourceWidget snapshots snapshots: renders as expected with videoSh
          id="authoring.videoeditor.videoSource.videoUrl.feedback"
        />
      </Form.Control.Feedback>
-    </div>
-    <div
-      className="mt-4"
-    >
-      <FormattedMessage
-        defaultMessage="Fallback videos"
-        description="Title for the fallback videos section"
-        id="authoring.videoeditor.videoSource.fallbackVideo.title"
-      />
-    </div>
-    <div
-      className="mt-3"
-    >
-      <FormattedMessage
-        defaultMessage="To be sure all learners can access the video, edX
+    </Form.Group>
+  </div>
+  <div
+    className="mt-4"
+  >
+    <FormattedMessage
+      defaultMessage="Fallback videos"
+      description="Title for the fallback videos section"
+      id="authoring.videoeditor.videoSource.fallbackVideo.title"
+    />
+  </div>
+  <div
+    className="mt-3"
+  >
+    <FormattedMessage
+      defaultMessage="To be sure all learners can access the video, edX
    recommends providing additional videos in both .mp4 and
    .webm formats.  The first listed video compatible with the
    learner's device will play."
-        description="Test explaining reason for fallback videos"
-        id="authoring.videoeditor.videoSource.fallbackVideo.message"
-      />
-    </div>
-    <Form.Row
-      className="mt-3.5 mx-0 flex-nowrap"
-    >
+      description="Test explaining reason for fallback videos"
+      id="authoring.videoeditor.videoSource.fallbackVideo.message"
+    />
+  </div>
+  <Form.Row
+    className="mt-3.5 mx-0 flex-nowrap"
+  >
+    <Form.Group>
      <Form.Control
        floatingLabel="Video URL"
      />
@@ -240,10 +250,12 @@ exports[`VideoSourceWidget snapshots snapshots: renders as expected with videoSh
        tooltipContent="Delete"
        tooltipPlacement="top"
      />
-    </Form.Row>
-    <ActionRow
-      className="mt-4.5"
-    >
+    </Form.Group>
+  </Form.Row>
+  <ActionRow
+    className="mt-4.5"
+  >
+    <Form.Group>
      <Form.Checkbox
        checked={false}
        className="decorative-control-label"
@@ -285,9 +297,9 @@ exports[`VideoSourceWidget snapshots snapshots: renders as expected with videoSh
          }
        />
      </OverlayTrigger>
-      <ActionRow.Spacer />
-    </ActionRow>
-  </Form.Group>
+    </Form.Group>
+    <ActionRow.Spacer />
+  </ActionRow>
  <div
    className="my-4 border-primary-100 border-bottom"
  />
--- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoSourceWidget/index.jsx
+++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoSourceWidget/index.jsx
@@ -69,8 +69,9 @@ const VideoSourceWidget = ({
      >
        <FormattedMessage {...messages.videoIdChangeAlert} />
      </ErrorAlert>
-      <Form.Group>
-        <div className="border-primary-100 border-bottom pb-4">
+
+      <div className="border-primary-100 border-bottom pb-4">
+        <Form.Group>
          <Form.Control
            floatingLabel={intl.formatMessage(messages.videoIdLabel)}
            onChange={videoId.onChange}
@@ -80,6 +81,8 @@ const VideoSourceWidget = ({
          <Form.Control.Feedback className="text-primary-300 mb-4">
            <FormattedMessage {...messages.videoIdFeedback} />
          </Form.Control.Feedback>
+        </Form.Group>
+        <Form.Group>
          <Form.Control
            floatingLabel={intl.formatMessage(messages.videoUrlLabel)}
            onChange={source.onChange}
@@ -89,15 +92,17 @@ const VideoSourceWidget = ({
          <Form.Control.Feedback className="text-primary-300">
            <FormattedMessage {...messages.videoUrlFeedback} />
          </Form.Control.Feedback>
-        </div>
-        <div className="mt-4">
-          <FormattedMessage {...messages.fallbackVideoTitle} />
-        </div>
-        <div className="mt-3">
-          <FormattedMessage {...messages.fallbackVideoMessage} />
-        </div>
-        {fallbackVideos.formValue.length > 0 ? fallbackVideos.formValue.map((videoUrl, index) => (
-          <Form.Row className="mt-3.5 mx-0 flex-nowrap">
+        </Form.Group>
+      </div>
+      <div className="mt-4">
+        <FormattedMessage {...messages.fallbackVideoTitle} />
+      </div>
+      <div className="mt-3">
+        <FormattedMessage {...messages.fallbackVideoMessage} />
+      </div>
+      {fallbackVideos.formValue.length > 0 ? fallbackVideos.formValue.map((videoUrl, index) => (
+        <Form.Row className="mt-3.5 mx-0 flex-nowrap">
+          <Form.Group>
            <Form.Control
              floatingLabel={intl.formatMessage(messages.fallbackVideoLabel)}
              onChange={fallbackVideos.onChange(index)}
@@ -113,9 +118,11 @@ const VideoSourceWidget = ({
              alt={intl.formatMessage(messages.deleteFallbackVideo)}
              onClick={() => deleteFallbackVideo(videoUrl)}
            />
-          </Form.Row>
-        )) : null}
-        <ActionRow className="mt-4.5">
+          </Form.Group>
+        </Form.Row>
+      )) : null}
+      <ActionRow className="mt-4.5">
+        <Form.Group>
          <Form.Checkbox
            checked={allowDownload.local}
            className="decorative-control-label"
@@ -136,9 +143,10 @@ const VideoSourceWidget = ({
          >
            <Icon src={InfoOutline} style={{ height: '16px', width: '16px' }} />
          </OverlayTrigger>
-          <ActionRow.Spacer />
-        </ActionRow>
-      </Form.Group>
+        </Form.Group>
+        <ActionRow.Spacer />
+      </ActionRow>
+
      <div className="my-4 border-primary-100 border-bottom" />
      <Button
        className="text-primary-500 font-weight-bold pl-0"
--- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoSourceWidget/index.test.jsx
+++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoSourceWidget/index.test.jsx
@@ -100,10 +100,9 @@ describe('VideoSourceWidget', () => {
        .props.onBlur).toEqual(expected);
    });
    test('updateVideoURL is tied to url field onBlur', () => {
-      const { onBlur } = el
-        // eslint-disable-next-line
-        .shallowWrapper.props.children[1].props.children[0].props.children[2].props;
-      onBlur('onBlur event');
+      const control = el.shallowWrapper.props.children[1].props.children[1].props.children[0];
+      expect(control.props.floatingLabel).toEqual('Video URL');
+      control.props.onBlur('onBlur event');
      expect(hook.updateVideoURL).toHaveBeenCalledWith('onBlur event', '');
    });
  });
--- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/index.tsx
+++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/index.tsx
@@ -48,7 +48,9 @@ const VideoSettingsModal: React.FC<Props> = ({
      <SocialShareWidget />
    )}
    <ThumbnailWidget />
-    <TranscriptWidget />
+    {!isLibrary && ( // Since content libraries v2 don't support static assets yet, we can't include transcripts.
+      <TranscriptWidget />
+    )}
    <DurationWidget />
    <HandoutWidget />
    <LicenseWidget />
--- a/src/library-authoring/add-content/AddContentWorkflow.test.tsx
+++ b/src/library-authoring/add-content/AddContentWorkflow.test.tsx
@@ -37,6 +37,7 @@ jest.spyOn(editorCmsApi as any, 'fetchBlockById').mockImplementation(
 );
 jest.spyOn(textEditorHooks, 'getContent').mockImplementation(() => () => '<p>Edited HTML content</p>');
 jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCss: '' }));
+const saveSpy = jest.spyOn(editorCmsApi as any, 'saveBlock');

 const { libraryId } = mockContentLibrary;
 const renderOpts = {
@@ -74,7 +75,7 @@ describe('AddContentWorkflow test', () => {
    // using TinyMCE to enter some new HTML.

    // Mock the save() REST API method:
-    const saveSpy = jest.spyOn(editorCmsApi as any, 'saveBlock').mockImplementationOnce(async () => ({
+    saveSpy.mockReset().mockImplementationOnce(async () => ({
      status: 200, data: { id: mockXBlockFields.usageKeyNewHtml },
    }));

@@ -109,31 +110,43 @@ describe('AddContentWorkflow test', () => {
    fireEvent.change(inputA, { target: { value: '123456' } });

    // Mock the save() REST API method:
-    const saveSpy = jest.spyOn(editorCmsApi as any, 'saveBlock').mockImplementationOnce(async () => ({
+    saveSpy.mockReset().mockImplementationOnce(async () => ({
      status: 200, data: { id: mockXBlockFields.usageKeyNewProblem },
    }));

    // Click Save
    const saveButton = screen.getByLabelText('Save changes and return to learning context');
    fireEvent.click(saveButton);
-    expect(saveSpy).toHaveBeenCalledTimes(2); // TODO: why is this called twice?
+    expect(saveSpy).toHaveBeenCalledTimes(1);
  });

  it('can create a Video component', async () => {
-    const { mockShowToast } = initializeMocks();
+    initializeMocks();
    render(<LibraryLayout />, renderOpts);

    // Click "New [Component]"
    const newComponentButton = await screen.findByRole('button', { name: /New/ });
    fireEvent.click(newComponentButton);

-    // Pre-condition - the success toast is NOT shown yet:
-    expect(mockShowToast).not.toHaveBeenCalled();
-
    // Click "Video" to create a video component
    fireEvent.click(await screen.findByRole('button', { name: /Video/ }));

-    // We haven't yet implemented the video editor, so we expect only a toast to appear
-    await waitFor(() => expect(mockShowToast).toHaveBeenCalledWith('Content created successfully.'));
+    // Then the editor should open - this is the default title of a blank video in our mock
+    expect(await screen.findByRole('heading', { name: /New Video/ })).toBeInTheDocument();
+
+    // Enter the video URL
+    const urlInput = await screen.findByRole('textbox', { name: 'Video URL' });
+    fireEvent.click(urlInput);
+    fireEvent.change(urlInput, { target: { value: 'https://www.youtube.com/watch?v=9KIIlWS4mkg' } });
+
+    // Mock the save() REST API method:
+    saveSpy.mockReset().mockImplementationOnce(async () => ({
+      status: 200, data: { id: mockXBlockFields.usageKeyNewVideo },
+    }));
+
+    // Click Save
+    const saveButton = screen.getByLabelText('Save changes and return to learning context');
+    fireEvent.click(saveButton);
+    expect(saveSpy).toHaveBeenCalledTimes(1);
  });
 });
--- a/src/library-authoring/components/utils.test.ts
+++ b/src/library-authoring/components/utils.test.ts
@@ -10,9 +10,9 @@ describe('component utils', () => {
      const usageKey = 'lb:org:beta:problem:571fe018-f3ce-45c9-8f53-5dafcb422fdd';
      expect(getEditUrl(usageKey)).toStrictEqual(`/library/lib:org:beta/editor/problem/${usageKey}`);
    });
-    it('doesn\'t yet allow editing a video block', () => {
+    it('returns the right URL for editing a Video block', () => {
      const usageKey = 'lb:org:beta:video:571fe018-f3ce-45c9-8f53-5dafcb422fdd';
-      expect(getEditUrl(usageKey)).toBeUndefined();
+      expect(getEditUrl(usageKey)).toStrictEqual(`/library/lib:org:beta/editor/video/${usageKey}`);
    });
    it('doesn\'t yet allow editing a drag-and-drop-v2 block', () => {
      const usageKey = 'lb:org:beta:drag-and-drop-v2:571fe018-f3ce-45c9-8f53-5dafcb422fdd';
--- a/src/library-authoring/components/utils.ts
+++ b/src/library-authoring/components/utils.ts
@@ -12,7 +12,7 @@ export function getEditUrl(usageKey: string): string | undefined {
  }

  // Which XBlock/component types are supported by the 'editors' built in to this repo?
-  const mfeEditorTypes = ['html', 'problem'];
+  const mfeEditorTypes = ['html', 'problem', 'video'];
  if (mfeEditorTypes.includes(blockType)) {
    return `/library/${libraryId}/editor/${blockType}/${usageKey}`;
  }
--- a/src/library-authoring/data/api.mocks.ts
+++ b/src/library-authoring/data/api.mocks.ts
@@ -150,8 +150,8 @@ mockCreateLibraryBlock.newProblemData = {
  tagsCount: 0,
 } satisfies api.LibraryBlockMetadata;
 mockCreateLibraryBlock.newVideoData = {
-  id: 'lb:Axim:TEST:video:prob1',
-  defKey: 'video1',
+  id: 'lb:Axim:TEST:video:vid1',
+  defKey: 'vid1',
  blockType: 'video',
  displayName: 'New Video',
  hasUnpublishedChanges: true,
@@ -180,6 +180,7 @@ export async function mockXBlockFields(usageKey: string): Promise<api.XBlockFiel
    case thisMock.usageKeyHtml: return thisMock.dataHtml;
    case thisMock.usageKeyNewHtml: return thisMock.dataNewHtml;
    case thisMock.usageKeyNewProblem: return thisMock.dataNewProblem;
+    case thisMock.usageKeyNewVideo: return thisMock.dataNewVideo;
    default: throw new Error(`No mock has been set up for usageKey "${usageKey}"`);
  }
 }
@@ -204,6 +205,12 @@ mockXBlockFields.dataNewProblem = {
  data: '',
  metadata: { displayName: 'New Problem Component' },
 } satisfies api.XBlockFields;
+mockXBlockFields.usageKeyNewVideo = 'lb:Axim:TEST:video:vid1';
+mockXBlockFields.dataNewVideo = {
+  displayName: 'New Video',
+  data: '',
+  metadata: { displayName: 'New Video' },
+} satisfies api.XBlockFields;
 /** Apply this mock. Returns a spy object that can tell you if it's been called. */
 mockXBlockFields.applyMock = () => jest.spyOn(api, 'getXBlockFields').mockImplementation(mockXBlockFields);