From d2f07045f0be692216ecf53c1dfbdc13fdef35df Mon Sep 17 00:00:00 2001 From: Raymond Zhou <56318341+rayzhou-bit@users.noreply.github.com> Date: Wed, 12 Oct 2022 09:38:44 -0700 Subject: [PATCH] Feat video source integration (#125) * feat: video source integration Co-authored-by: KristinAoki --- .../components/TitleHeader/index.jsx | 2 - .../__snapshots__/index.test.jsx.snap | 24 ++++--- .../components/VideoEditorModal.jsx | 12 +--- .../VideoSettingsModal/ErrorSummary.jsx | 33 +++++++++ .../VideoSettingsModal/ErrorSummary.test.jsx | 45 ++++++++++++ .../__snapshots__/ErrorSummary.test.jsx.snap | 45 ++++++++++++ .../components/ErrorSummary.jsx | 40 ----------- .../components/ErrorSummary.test.jsx | 17 ----- .../__snapshots__/index.test.jsx.snap | 12 ++-- .../components/TranscriptWidget/hooks.js | 14 +++- .../components/TranscriptWidget/index.jsx | 5 +- .../TranscriptWidget/index.test.jsx | 5 ++ .../components/VideoSourceWidget/index.jsx | 3 - .../__snapshots__/ErrorSummary.test.jsx.snap | 23 ------- .../components/VideoSettingsModal/index.jsx | 35 +++------- src/editors/containers/VideoEditor/hooks.js | 68 +++++-------------- .../containers/VideoEditor/hooks.test.js | 58 ++++++++-------- src/editors/containers/VideoEditor/index.jsx | 22 +++--- .../containers/VideoEditor/index.test.jsx | 6 +- src/editors/data/redux/thunkActions/video.js | 30 +++++--- .../data/redux/thunkActions/video.test.js | 13 ++-- src/editors/data/redux/video/selectors.js | 19 +++++- src/editors/data/services/cms/api.js | 4 +- src/editors/data/services/cms/mockApi.js | 4 +- 24 files changed, 282 insertions(+), 257 deletions(-) create mode 100644 src/editors/containers/VideoEditor/components/VideoSettingsModal/ErrorSummary.jsx create mode 100644 src/editors/containers/VideoEditor/components/VideoSettingsModal/ErrorSummary.test.jsx create mode 100644 src/editors/containers/VideoEditor/components/VideoSettingsModal/__snapshots__/ErrorSummary.test.jsx.snap delete mode 100644 src/editors/containers/VideoEditor/components/VideoSettingsModal/components/ErrorSummary.jsx delete mode 100644 src/editors/containers/VideoEditor/components/VideoSettingsModal/components/ErrorSummary.test.jsx delete mode 100644 src/editors/containers/VideoEditor/components/VideoSettingsModal/components/__snapshots__/ErrorSummary.test.jsx.snap diff --git a/src/editors/containers/EditorContainer/components/TitleHeader/index.jsx b/src/editors/containers/EditorContainer/components/TitleHeader/index.jsx index 2056a61a4..f32449873 100644 --- a/src/editors/containers/EditorContainer/components/TitleHeader/index.jsx +++ b/src/editors/containers/EditorContainer/components/TitleHeader/index.jsx @@ -60,8 +60,6 @@ export const TitleHeader = ({ TitleHeader.defaultProps = {}; TitleHeader.propTypes = { isInitialized: PropTypes.bool.isRequired, - title: PropTypes.string.isRequired, - setTitle: PropTypes.func.isRequired, // injected intl: intlShape.isRequired, }; diff --git a/src/editors/containers/VideoEditor/__snapshots__/index.test.jsx.snap b/src/editors/containers/VideoEditor/__snapshots__/index.test.jsx.snap index bb656ac34..d0aab7998 100644 --- a/src/editors/containers/VideoEditor/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/VideoEditor/__snapshots__/index.test.jsx.snap @@ -1,17 +1,19 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`VideoEditor snapshots renders as expected with default behavior 1`] = ` - -
- -
-
+
+ +
+ + `; diff --git a/src/editors/containers/VideoEditor/components/VideoEditorModal.jsx b/src/editors/containers/VideoEditor/components/VideoEditorModal.jsx index cff3201c3..89df1bf81 100644 --- a/src/editors/containers/VideoEditor/components/VideoEditorModal.jsx +++ b/src/editors/containers/VideoEditor/components/VideoEditorModal.jsx @@ -17,30 +17,20 @@ export const hooks = { const VideoEditorModal = ({ close, - error, isOpen, }) => { const dispatch = useDispatch(); module.hooks.initialize(dispatch); return ( - + ); // TODO: add logic to show SelectVideoModal if no selection }; VideoEditorModal.defaultProps = { - error: { - duration: {}, - handout: {}, - license: {}, - thumbnail: {}, - transcripts: {}, - videoSource: {}, - }, }; VideoEditorModal.propTypes = { close: PropTypes.func.isRequired, - error: PropTypes.node, isOpen: PropTypes.bool.isRequired, }; export default VideoEditorModal; diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/ErrorSummary.jsx b/src/editors/containers/VideoEditor/components/VideoSettingsModal/ErrorSummary.jsx new file mode 100644 index 000000000..f65da9b2b --- /dev/null +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/ErrorSummary.jsx @@ -0,0 +1,33 @@ +import React from 'react'; +import { FormattedMessage } from '@edx/frontend-platform/i18n'; + +import { Alert } from '@edx/paragon'; +import { Info } from '@edx/paragon/icons'; + +import messages from './components/messages'; +import { ErrorContext } from '../../hooks'; +import * as module from './ErrorSummary'; + +export const hasNoError = (error) => Object.keys(error[0]).length === 0; + +export const showAlert = (errors) => !Object.values(errors).every(module.hasNoError); + +export const ErrorSummary = () => { + const errors = React.useContext(ErrorContext); + return ( + + + + +

+ +

+
+ ); +}; + +export default ErrorSummary; diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/ErrorSummary.test.jsx b/src/editors/containers/VideoEditor/components/VideoSettingsModal/ErrorSummary.test.jsx new file mode 100644 index 000000000..ae6731d73 --- /dev/null +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/ErrorSummary.test.jsx @@ -0,0 +1,45 @@ +import React from 'react'; +import { shallow } from 'enzyme'; + +import * as module from './ErrorSummary'; + +describe('ErrorSummary', () => { + const errors = { + widgetWithError: [{ err1: 'mSg', err2: 'msG2' }, jest.fn()], + widgetWithNoError: [{}, jest.fn()], + }; + afterEach(() => { + jest.restoreAllMocks(); + }); + describe('render', () => { + beforeEach(() => { + jest.spyOn(React, 'useContext').mockReturnValueOnce({}); + }); + test('snapshots: renders as expected when there are no errors', () => { + jest.spyOn(module, 'showAlert').mockReturnValue(false); + expect(shallow()).toMatchSnapshot(); + }); + test('snapshots: renders as expected when there are errors', () => { + jest.spyOn(module, 'showAlert').mockReturnValue(true); + expect(shallow()).toMatchSnapshot(); + }); + }); + describe('hasNoError', () => { + it('returns true', () => { + expect(module.hasNoError(errors.widgetWithError)).toEqual(false); + }); + it('returns false', () => { + expect(module.hasNoError(errors.widgetWithNoError)).toEqual(true); + }); + }); + describe('showAlert', () => { + it('returns true', () => { + jest.spyOn(module, 'hasNoError').mockReturnValue(false); + expect(module.showAlert(errors)).toEqual(true); + }); + it('returns false', () => { + jest.spyOn(module, 'hasNoError').mockReturnValue(true); + expect(module.showAlert(errors)).toEqual(false); + }); + }); +}); diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/__snapshots__/ErrorSummary.test.jsx.snap b/src/editors/containers/VideoEditor/components/VideoSettingsModal/__snapshots__/ErrorSummary.test.jsx.snap new file mode 100644 index 000000000..88baf91bd --- /dev/null +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/__snapshots__/ErrorSummary.test.jsx.snap @@ -0,0 +1,45 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ErrorSummary render snapshots: renders as expected when there are errors 1`] = ` + + + + +

+ +

+
+`; + +exports[`ErrorSummary render snapshots: renders as expected when there are no errors 1`] = ` + + + + +

+ +

+
+`; diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/ErrorSummary.jsx b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/ErrorSummary.jsx deleted file mode 100644 index 5b7ceeea0..000000000 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/ErrorSummary.jsx +++ /dev/null @@ -1,40 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import { FormattedMessage } from '@edx/frontend-platform/i18n'; - -import { Alert } from '@edx/paragon'; -import { Info } from '@edx/paragon/icons'; -import messages from './messages'; - -export const ErrorSummary = ({ - error, -}) => ( - Object.keys(val).length === 0)} - variant="danger" - > - - - -

- -

-
-); - -ErrorSummary.defaultProps = { - error: { - duration: {}, - handout: {}, - license: {}, - thumbnail: {}, - transcripts: {}, - videoSource: {}, - }, -}; -ErrorSummary.propTypes = { - error: PropTypes.node, -}; - -export default ErrorSummary; diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/ErrorSummary.test.jsx b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/ErrorSummary.test.jsx deleted file mode 100644 index c0e985387..000000000 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/ErrorSummary.test.jsx +++ /dev/null @@ -1,17 +0,0 @@ -import React from 'react'; -import { shallow } from 'enzyme'; - -import { ErrorSummary } from './ErrorSummary'; - -describe('ErrorSummary', () => { - const props = { - error: 'eRrOr', - }; - describe('render', () => { - test('snapshots: renders as expected', () => { - expect( - shallow(), - ).toMatchSnapshot(); - }); - }); -}); diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/__snapshots__/index.test.jsx.snap b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/__snapshots__/index.test.jsx.snap index cebc1fff1..e00e4cede 100644 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/__snapshots__/index.test.jsx.snap @@ -2,7 +2,7 @@ exports[`TranscriptWidget snapshots snapshot: renders ErrorAlert with delete error message 1`] = ` @@ -115,7 +115,7 @@ exports[`TranscriptWidget snapshots snapshot: renders ErrorAlert with delete err exports[`TranscriptWidget snapshots snapshot: renders ErrorAlert with upload error message 1`] = ` @@ -228,7 +228,7 @@ exports[`TranscriptWidget snapshots snapshot: renders ErrorAlert with upload err exports[`TranscriptWidget snapshots snapshots: renders as expected with allowTranscriptDownloads true 1`] = ` @@ -341,7 +341,7 @@ exports[`TranscriptWidget snapshots snapshots: renders as expected with allowTra exports[`TranscriptWidget snapshots snapshots: renders as expected with default props 1`] = ` @@ -412,7 +412,7 @@ exports[`TranscriptWidget snapshots snapshots: renders as expected with default exports[`TranscriptWidget snapshots snapshots: renders as expected with showTranscriptByDefault true 1`] = ` @@ -525,7 +525,7 @@ exports[`TranscriptWidget snapshots snapshots: renders as expected with showTran exports[`TranscriptWidget snapshots snapshots: renders as expected with transcripts 1`] = ` diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/hooks.js b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/hooks.js index 39dc1098f..e6601ff38 100644 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/hooks.js +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/hooks.js @@ -2,14 +2,26 @@ import React from 'react'; import { thunkActions, actions } from '../../../../../../data/redux'; import * as module from './hooks'; import { videoTranscriptLanguages } from '../../../../../../data/constants/video'; +import { ErrorContext } from '../../../../hooks'; +import messages from './messages'; export const state = { inDeleteConfirmation: (args) => React.useState(args), }; +export const updateErrors = ({ isUploadError, isDeleteError }) => { + const [error, setError] = React.useContext(ErrorContext).transcripts; + if (isUploadError) { + setError({ ...error, uploadError: messages.uploadTranscriptError.defaultMessage }); + } + if (isDeleteError) { + setError({ ...error, deleteError: messages.deleteTranscriptError.defaultMessage }); + } +}; + export const transcriptLanguages = (transcripts) => { const languages = []; - if (Object.keys(transcripts).length > 0) { + if (transcripts && Object.keys(transcripts).length > 0) { Object.keys(transcripts).forEach(transcript => { languages.push(videoTranscriptLanguages[transcript]); }); diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/index.jsx b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/index.jsx index 3f87fe27c..c94ca6b7b 100644 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/index.jsx +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/index.jsx @@ -28,12 +28,12 @@ import ErrorAlert from '../../../../../../sharedComponents/ErrorAlerts/ErrorAler import CollapsibleFormWidget from '../CollapsibleFormWidget'; import TranscriptListItem from './TranscriptListItem'; +import { ErrorContext } from '../../../../hooks'; /** * Collapsible Form widget controlling video transcripts */ export const TranscriptWidget = ({ - error, // redux transcripts, allowTranscriptDownloads, @@ -42,6 +42,7 @@ export const TranscriptWidget = ({ isUploadError, isDeleteError, }) => { + const [error] = React.useContext(ErrorContext).transcripts; const languagesArr = hooks.transcriptLanguages(transcripts); const fileInput = hooks.fileInput({ onAddFile: hooks.addFileCallback({ dispatch: useDispatch() }) }); const hasTranscripts = hooks.hasTranscripts(transcripts); @@ -124,10 +125,8 @@ export const TranscriptWidget = ({ }; TranscriptWidget.defaultProps = { - error: {}, }; TranscriptWidget.propTypes = { - error: PropTypes.node, // redux transcripts: PropTypes.shape({}).isRequired, allowTranscriptDownloads: PropTypes.bool.isRequired, diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/index.test.jsx b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/index.test.jsx index 01b16484a..28cc81644 100644 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/index.test.jsx +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/index.test.jsx @@ -7,6 +7,11 @@ import { formatMessage } from '../../../../../../../testUtils'; import { actions, selectors } from '../../../../../../data/redux'; import { TranscriptWidget, mapStateToProps, mapDispatchToProps } from '.'; +jest.mock('react', () => ({ + ...jest.requireActual('react'), + useContext: jest.fn(() => ({ transcripts: ['error.transcripts', jest.fn().mockName('error.setTranscripts')] })), +})); + jest.mock('../../../../../../data/redux', () => ({ actions: { video: { diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoSourceWidget/index.jsx b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoSourceWidget/index.jsx index 3f42d97f7..add9674f7 100644 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoSourceWidget/index.jsx +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoSourceWidget/index.jsx @@ -28,7 +28,6 @@ import CollapsibleFormWidget from '../CollapsibleFormWidget'; * Collapsible Form widget controlling video source as well as fallback sources */ export const VideoSourceWidget = ({ - // error, // injected intl, // redux @@ -128,10 +127,8 @@ export const VideoSourceWidget = ({ ); }; VideoSourceWidget.defaultProps = { - // error: {}, }; VideoSourceWidget.propTypes = { - // error: PropTypes.node, // injected intl: intlShape.isRequired, // redux diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/__snapshots__/ErrorSummary.test.jsx.snap b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/__snapshots__/ErrorSummary.test.jsx.snap deleted file mode 100644 index eb911b3f5..000000000 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/__snapshots__/ErrorSummary.test.jsx.snap +++ /dev/null @@ -1,23 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`ErrorSummary render snapshots: renders as expected 1`] = ` - - - - -

- -

-
-`; diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/index.jsx b/src/editors/containers/VideoEditor/components/VideoSettingsModal/index.jsx index 9ca375366..439cd0386 100644 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/index.jsx +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/index.jsx @@ -1,9 +1,8 @@ import React from 'react'; -import PropTypes from 'prop-types'; import { thunkActions } from '../../../../data/redux'; // import VideoPreview from './components/VideoPreview'; -import ErrorSummary from './components/ErrorSummary'; +import ErrorSummary from './ErrorSummary'; import DurationWidget from './components/DurationWidget'; import HandoutWidget from './components/HandoutWidget'; import LicenseWidget from './components/LicenseWidget'; @@ -20,39 +19,23 @@ export const hooks = { }, }; -export const VideoSettingsModal = ({ - error, -}) => ( +export const VideoSettingsModal = () => (
Video Preview goes here {/* */}
- +

Settings

- - - - - - + + + + + +
); -VideoSettingsModal.defaultProps = { - error: { - duration: {}, - handout: {}, - license: {}, - thumbnail: {}, - transcripts: {}, - videoSource: {}, - }, -}; -VideoSettingsModal.propTypes = { - error: PropTypes.node, -}; - export default VideoSettingsModal; diff --git a/src/editors/containers/VideoEditor/hooks.js b/src/editors/containers/VideoEditor/hooks.js index 011b020b8..1e3ca1bfd 100644 --- a/src/editors/containers/VideoEditor/hooks.js +++ b/src/editors/containers/VideoEditor/hooks.js @@ -1,8 +1,10 @@ -import { useState } from 'react'; +import { useState, createContext } from 'react'; import { StrictDict } from '../../utils'; import * as module from './hooks'; +export const ErrorContext = createContext(); + export const state = StrictDict({ durationErrors: (val) => useState(val), handoutErrors: (val) => useState(val), @@ -22,59 +24,21 @@ export const errorsHook = () => { return { error: { - duration: durationErrors, - handout: handoutErrors, - license: licenseErrors, - thumbnail: thumbnailErrors, - transcripts: transcriptsErrors, - videoSource: videoSourceErrors, + duration: [durationErrors, setDurationErrors], + handout: [handoutErrors, setHandoutErrors], + license: [licenseErrors, setLicenseErrors], + thumbnail: [thumbnailErrors, setThumbnailErrors], + transcripts: [transcriptsErrors, setTranscriptsErrors], + videoSource: [videoSourceErrors, setVideoSourceErrors], }, validateEntry: () => { - let validated = true; - if (!module.validateDuration({ setDurationErrors })) { validated = false; } - if (!module.validateHandout({ setHandoutErrors })) { validated = false; } - if (!module.validateLicense({ setLicenseErrors })) { validated = false; } - if (!module.validateThumbnail({ setThumbnailErrors })) { validated = false; } - if (!module.validateTranscripts({ setTranscriptsErrors })) { validated = false; } - if (!module.validateVideoSource({ setVideoSourceErrors })) { validated = false; } - return validated; + if (Object.keys(durationErrors).length > 0) { return false; } + if (Object.keys(handoutErrors).length > 0) { return false; } + if (Object.keys(licenseErrors).length > 0) { return false; } + if (Object.keys(thumbnailErrors).length > 0) { return false; } + if (Object.keys(transcriptsErrors).length > 0) { return false; } + if (Object.keys(videoSourceErrors).length > 0) { return false; } + return true; }, }; }; - -export const validateDuration = ({ setDurationErrors }) => { - setDurationErrors({ - fieldName: 'sample error message', - }); - return false; -}; -export const validateHandout = ({ setHandoutErrors }) => { - setHandoutErrors({ - fieldName: 'sample error message', - }); - return false; -}; -export const validateLicense = ({ setLicenseErrors }) => { - setLicenseErrors({ - fieldName: 'sample error message', - }); - return false; -}; -export const validateThumbnail = ({ setThumbnailErrors }) => { - setThumbnailErrors({ - fieldName: 'sample error message', - }); - return false; -}; -export const validateTranscripts = ({ setTranscriptsErrors }) => { - setTranscriptsErrors({ - fieldName: 'sample error message', - }); - return false; -}; -export const validateVideoSource = ({ setVideoSourceErrors }) => { - setVideoSourceErrors({ - fieldName: 'sample error message', - }); - return false; -}; diff --git a/src/editors/containers/VideoEditor/hooks.test.js b/src/editors/containers/VideoEditor/hooks.test.js index e12a09c6e..f29438cb4 100644 --- a/src/editors/containers/VideoEditor/hooks.test.js +++ b/src/editors/containers/VideoEditor/hooks.test.js @@ -1,6 +1,5 @@ import { MockUseState } from '../../../testUtils'; -import { keyStore } from '../../utils'; import * as module from './hooks'; jest.mock('react', () => ({ @@ -8,7 +7,6 @@ jest.mock('react', () => ({ })); const state = new MockUseState(module); -const moduleKeys = keyStore(module); let hook; @@ -33,38 +31,44 @@ describe('VideoEditorHooks', () => { state.restore(); }); - const mockTrue = () => true; - const mockFalse = () => false; + const fakeDurationError = { + field1: 'field1msg', + field2: 'field2msg', + }; test('error: state values', () => { - expect(module.errorsHook().error).toEqual({ - duration: state.stateVals[state.keys.durationErrors], - handout: state.stateVals[state.keys.handoutErrors], - license: state.stateVals[state.keys.licenseErrors], - thumbnail: state.stateVals[state.keys.thumbnailErrors], - transcripts: state.stateVals[state.keys.transcriptsErrors], - videoSource: state.stateVals[state.keys.videoSourceErrors], - }); + expect(module.errorsHook().error.duration).toEqual([ + state.stateVals[state.keys.durationErrors], + state.setState[state.keys.durationErrors], + ]); + expect(module.errorsHook().error.handout).toEqual([ + state.stateVals[state.keys.handoutErrors], + state.setState[state.keys.handoutErrors], + ]); + expect(module.errorsHook().error.license).toEqual([ + state.stateVals[state.keys.licenseErrors], + state.setState[state.keys.licenseErrors], + ]); + expect(module.errorsHook().error.thumbnail).toEqual([ + state.stateVals[state.keys.thumbnailErrors], + state.setState[state.keys.thumbnailErrors], + ]); + expect(module.errorsHook().error.transcripts).toEqual([ + state.stateVals[state.keys.transcriptsErrors], + state.setState[state.keys.transcriptsErrors], + ]); + expect(module.errorsHook().error.videoSource).toEqual([ + state.stateVals[state.keys.videoSourceErrors], + state.setState[state.keys.videoSourceErrors], + ]); }); describe('validateEntry', () => { - beforeEach(() => { - hook = module.errorsHook(); - }); test('validateEntry: returns true if all validation calls are true', () => { - jest.spyOn(module, moduleKeys.validateDuration).mockImplementationOnce(mockTrue); - jest.spyOn(module, moduleKeys.validateHandout).mockImplementationOnce(mockTrue); - jest.spyOn(module, moduleKeys.validateLicense).mockImplementationOnce(mockTrue); - jest.spyOn(module, moduleKeys.validateThumbnail).mockImplementationOnce(mockTrue); - jest.spyOn(module, moduleKeys.validateTranscripts).mockImplementationOnce(mockTrue); - jest.spyOn(module, moduleKeys.validateVideoSource).mockImplementationOnce(mockTrue); + hook = module.errorsHook(); expect(hook.validateEntry()).toEqual(true); }); test('validateEntry: returns false if any validation calls are false', () => { - jest.spyOn(module, moduleKeys.validateDuration).mockImplementationOnce(mockFalse); - jest.spyOn(module, moduleKeys.validateHandout).mockImplementationOnce(mockTrue); - jest.spyOn(module, moduleKeys.validateLicense).mockImplementationOnce(mockTrue); - jest.spyOn(module, moduleKeys.validateThumbnail).mockImplementationOnce(mockTrue); - jest.spyOn(module, moduleKeys.validateTranscripts).mockImplementationOnce(mockTrue); - jest.spyOn(module, moduleKeys.validateVideoSource).mockImplementationOnce(mockTrue); + state.mockVal(state.keys.durationErrors, fakeDurationError); + hook = module.errorsHook(); expect(hook.validateEntry()).toEqual(false); }); }); diff --git a/src/editors/containers/VideoEditor/index.jsx b/src/editors/containers/VideoEditor/index.jsx index ef1c8e99b..8e22805c8 100644 --- a/src/editors/containers/VideoEditor/index.jsx +++ b/src/editors/containers/VideoEditor/index.jsx @@ -6,7 +6,7 @@ import { selectors } from '../../data/redux'; import EditorContainer from '../EditorContainer'; import VideoEditorModal from './components/VideoEditorModal'; -import { errorsHook } from './hooks'; +import { ErrorContext, errorsHook } from './hooks'; export const VideoEditor = ({ onClose, @@ -19,15 +19,17 @@ export const VideoEditor = ({ } = errorsHook(); return ( - videoSettings} - onClose={onClose} - validateEntry={validateEntry} - > -
- -
-
+ + videoSettings} + onClose={onClose} + validateEntry={validateEntry} + > +
+ +
+
+
); }; diff --git a/src/editors/containers/VideoEditor/index.test.jsx b/src/editors/containers/VideoEditor/index.test.jsx index fb7b8bce4..7c922b6ad 100644 --- a/src/editors/containers/VideoEditor/index.test.jsx +++ b/src/editors/containers/VideoEditor/index.test.jsx @@ -2,13 +2,13 @@ import React from 'react'; import { shallow } from 'enzyme'; import { selectors } from '../../data/redux'; -import { errorsHook } from './hooks'; import { VideoEditor, mapStateToProps, mapDispatchToProps } from '.'; jest.mock('../EditorContainer', () => 'EditorContainer'); jest.mock('./components/VideoEditorModal', () => 'VideoEditorModal'); jest.mock('./hooks', () => ({ + ErrorContext: jest.fn(), errorsHook: jest.fn(() => ({ error: 'hooks.errorsHook.error', validateEntry: jest.fn().mockName('validateEntry'), @@ -29,10 +29,6 @@ describe('VideoEditor', () => { // redux videoSettings: 'vIdEOsETtings', }; - errorsHook.mockReturnValue({ - error: 'errORsHooKErroR', - validateEntry: jest.fn().mockName('validateEntry'), - }); describe('snapshots', () => { test('renders as expected with default behavior', () => { expect(shallow()).toMatchSnapshot(); diff --git a/src/editors/data/redux/thunkActions/video.js b/src/editors/data/redux/thunkActions/video.js index 5184b43b2..8fa5e7d88 100644 --- a/src/editors/data/redux/thunkActions/video.js +++ b/src/editors/data/redux/thunkActions/video.js @@ -23,7 +23,7 @@ export const loadVideoData = () => (dispatch, getState) => { videoId, fallbackVideos, allowVideoDownloads: rawVideoData.download_video, - transcripts: rawVideoData.transcripts, + transcripts: rawVideoData.transcripts || {}, allowTranscriptDownloads: rawVideoData.download_track, showTranscriptByDefault: rawVideoData.show_captions, duration: { // TODO duration is not always sent so they should be calculated. @@ -47,14 +47,28 @@ export const determineVideoSource = ({ youtubeId, html5Sources, }) => { - // videoSource should be the edx_video_id (if present), or the youtube url (if present), or the first fallback url. - // in that order. - // if we are falling back to the first fallback url, remove it from the list of fallback urls for display - const videoSource = edxVideoId || youtubeId || html5Sources[0] || ''; + // videoSource should be the edx_video_id, the youtube url or the first fallback url in that order. + // If we are falling back to the first fallback url, remove it from the list of fallback urls for display. + const youtubeUrl = `https://youtu.be/${youtubeId}`; const videoId = edxVideoId || ''; - const fallbackVideos = (!edxVideoId && !youtubeId) - ? html5Sources.slice(1) - : html5Sources; + let videoSource = ''; + let fallbackVideos = []; + if (edxVideoId) { + [videoSource, fallbackVideos] = [edxVideoId, html5Sources]; + // videoSource = edxVideoId; + // fallbackVideos = html5Sources; + } else if (youtubeId) { + [videoSource, fallbackVideos] = [youtubeUrl, html5Sources]; + // videoSource = youtubeUrl; + // fallbackVideos = html5Sources; + } else if (Array.isArray(html5Sources) && html5Sources[0]) { + [videoSource, fallbackVideos] = [html5Sources[0], html5Sources.slice(1)]; + // videoSource = html5Sources[0]; + // fallbackVideos = html5Sources.slice(1); + } + if (fallbackVideos.length === 0) { + fallbackVideos = ['', '']; + } return { videoSource, videoId, diff --git a/src/editors/data/redux/thunkActions/video.test.js b/src/editors/data/redux/thunkActions/video.test.js index 0b1d74428..b9ddc226e 100644 --- a/src/editors/data/redux/thunkActions/video.test.js +++ b/src/editors/data/redux/thunkActions/video.test.js @@ -107,6 +107,7 @@ describe('video thunkActions', () => { describe('determineVideoSource', () => { const edxVideoId = 'EDxviDEoiD'; const youtubeId = 'yOuTuBEiD'; + const youtubeUrl = `https://youtu.be/${youtubeId}`; const html5Sources = ['htmLOne', 'hTMlTwo', 'htMLthrEE']; describe('when there is an edx video id, youtube id and html5 sources', () => { it('returns the edx video id for video source and html5 sources for fallback videos', () => { @@ -122,13 +123,13 @@ describe('video thunkActions', () => { }); }); describe('when there is no edx video id', () => { - it('returns the youtube id for video source and html5 sources for fallback videos', () => { + it('returns the youtube url for video source and html5 sources for fallback videos', () => { expect(thunkActions.determineVideoSource({ edxVideoId: '', youtubeId, html5Sources, })).toEqual({ - videoSource: youtubeId, + videoSource: youtubeUrl, videoId: '', fallbackVideos: html5Sources, }); @@ -146,7 +147,7 @@ describe('video thunkActions', () => { fallbackVideos: ['hTMlTwo', 'htMLthrEE'], }); }); - it('returns the html5 source for video source and an empty array for fallback videos', () => { + it('returns the html5 source for video source and an array with 2 empty values for fallback videos', () => { expect(thunkActions.determineVideoSource({ edxVideoId: '', youtubeId: '', @@ -154,12 +155,12 @@ describe('video thunkActions', () => { })).toEqual({ videoSource: 'htmlOne', videoId: '', - fallbackVideos: [], + fallbackVideos: ['', ''], }); }); }); describe('when there is no edx video id, no youtube id and no html5 sources', () => { - it('returns an empty string for video source and an empty array for fallback videos', () => { + it('returns an empty string for video source and an array with 2 empty values for fallback videos', () => { expect(thunkActions.determineVideoSource({ edxVideoId: '', youtubeId: '', @@ -167,7 +168,7 @@ describe('video thunkActions', () => { })).toEqual({ videoSource: '', videoId: '', - fallbackVideos: [], + fallbackVideos: ['', ''], }); }); }); diff --git a/src/editors/data/redux/video/selectors.js b/src/editors/data/redux/video/selectors.js index 2501ba706..f89fba0f7 100644 --- a/src/editors/data/redux/video/selectors.js +++ b/src/editors/data/redux/video/selectors.js @@ -30,6 +30,9 @@ export const simpleSelectors = [ export const openLanguages = createSelector( [module.simpleSelectors.transcripts], (transcripts) => { + if (!transcripts) { + return videoTranscriptLanguages; + } const open = Object.entries(videoTranscriptLanguages).filter( ([lang]) => !Object.keys(transcripts).includes(lang), ); @@ -47,7 +50,19 @@ export const getTranscriptDownloadUrl = createSelector( ); export const videoSettings = createSelector( - Object.values(module.simpleSelectors), + [ + module.simpleSelectors.videoSource, + module.simpleSelectors.fallbackVideos, + module.simpleSelectors.allowVideoDownloads, + module.simpleSelectors.thumbnail, + module.simpleSelectors.transcripts, + module.simpleSelectors.allowTranscriptDownloads, + module.simpleSelectors.duration, + module.simpleSelectors.showTranscriptByDefault, + module.simpleSelectors.handout, + module.simpleSelectors.licenseType, + module.simpleSelectors.licenseDetails, + ], ( videoSource, fallbackVideos, @@ -78,8 +93,8 @@ export const videoSettings = createSelector( ); export default { + ...simpleSelectors, openLanguages, getTranscriptDownloadUrl, - ...simpleSelectors, videoSettings, }; diff --git a/src/editors/data/services/cms/api.js b/src/editors/data/services/cms/api.js index 64a27ee28..28e867876 100644 --- a/src/editors/data/services/cms/api.js +++ b/src/editors/data/services/cms/api.js @@ -145,11 +145,11 @@ export const processVideoIds = ({ videoSource, fallbackVideos }) => { edxVideoId = videoSource; } else if (module.parseYoutubeId(videoSource)) { youtubeId = module.parseYoutubeId(videoSource); - } else { + } else if (videoSource) { html5Sources.push(videoSource); } - fallbackVideos.forEach((src) => html5Sources.push(src)); + fallbackVideos.forEach((src) => (src ? html5Sources.push(src) : null)); return { edxVideoId, diff --git a/src/editors/data/services/cms/mockApi.js b/src/editors/data/services/cms/mockApi.js index 7d6bcb3a9..ce9e32754 100644 --- a/src/editors/data/services/cms/mockApi.js +++ b/src/editors/data/services/cms/mockApi.js @@ -23,7 +23,7 @@ export const fetchBlockById = ({ blockId, studioEndpointUrl }) => mockPromise({ sub: '', track: '', transcripts: { - en: 'my-transcript-url', + en: { filename: 'my-transcript-url' }, }, xml_attributes: { source: '', @@ -54,7 +54,7 @@ export const fetchStudioView = ({ blockId, studioEndpointUrl }) => mockPromise({ sub: '', track: '', transcripts: { - en: 'my-transcript-url', + en: { filename: 'my-transcript-url' }, }, xml_attributes: { source: '',