Skip to content

Commit

Permalink
Support displaying restrictions in YouTubePicker (#5483)
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya authored Jun 15, 2023
1 parent 3d967bf commit c756a94
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 7 deletions.
3 changes: 3 additions & 0 deletions lms/static/scripts/frontend_apps/api-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,14 @@ export type JSTORThumbnail = {
image: string;
};

export type YouTubeVideoRestriction = 'age' | 'no_embed';

/**
* Response for an `/api/youtube/videos/{video_id}` call.
*/
export type YouTubeVideoInfo = {
title: string;
channel: string;
image: string;
restrictions: YouTubeVideoRestriction[];
};
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ export type ThumbnailData = {
export type URLFormWithPreviewProps = {
/** Optional extra content to be rendered together with the input and thumbnail */
children?: ComponentChildren;
/** An error message to highlight that something went wrong */
error?: string;
/** Error content to highlight that something went wrong */
error?: ComponentChildren;
/** Thumbnail info to be displayed, if known */
thumbnail?: ThumbnailData;
/** Reference to be set on the URL input */
Expand Down
41 changes: 37 additions & 4 deletions lms/static/scripts/frontend_apps/components/YouTubePicker.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Button, ModalDialog } from '@hypothesis/frontend-shared';
import type { ComponentChildren } from 'preact';
import { useMemo, useRef, useState } from 'preact/hooks';

import type { YouTubeVideoInfo } from '../api-types';
import type { YouTubeVideoInfo, YouTubeVideoRestriction } from '../api-types';
import { isAPIError } from '../errors';
import { urlPath, useAPIFetch } from '../utils/api';
import { videoIdFromYouTubeURL } from '../utils/youtube';
Expand All @@ -17,6 +18,32 @@ export type YouTubePickerProps = {
onSelectURL: (url: string) => void;
};

function formatRestrictionError(
restrictions: YouTubeVideoRestriction[]
): ComponentChildren | undefined {
const mainMessage = 'This video cannot be used in an assignment because';
const restrictionMessages = restrictions.map(restriction =>
restriction === 'age'
? 'it contains age-restricted content'
: "the video's owner does not allow this video to be embedded"
);

if (restrictionMessages.length === 1) {
return `${mainMessage} ${restrictionMessages[0]}`;
}

return (
<>
{mainMessage}:
<ul className="list-disc pl-4 mt-2">
{restrictionMessages.map((message, index) => (
<li key={index}>{message}</li>
))}
</ul>
</>
);
}

export default function YouTubePicker({
onCancel,
defaultURL,
Expand All @@ -42,13 +69,19 @@ export default function YouTubePicker({
: defaultError;
}

// An existing YouTube URL was set, but the video has restrictions
if (videoInfo.data?.restrictions.length) {
const { restrictions } = videoInfo.data;
return formatRestrictionError(restrictions);
}

// A URL was set, but it's not a valid YouTube URL
if (currentURL && !videoId) {
return defaultError;
}

return undefined;
}, [currentURL, videoId, videoInfo.error]);
}, [currentURL, videoId, videoInfo.error, videoInfo.data]);

const onURLChange = (inputURL: string) => setCurrentURL(inputURL);
const resetCurrentURL = () => setCurrentURL(undefined);
Expand All @@ -71,7 +104,7 @@ export default function YouTubePicker({
</Button>,
<Button
data-testid="select-button"
disabled={!videoInfo.data}
disabled={!!error || !videoInfo.data}
key="submit"
onClick={confirmSelection}
variant="primary"
Expand All @@ -95,7 +128,7 @@ export default function YouTubePicker({
orientation: 'landscape',
}}
>
{videoInfo.data?.title && videoInfo.data.channel && (
{!error && videoInfo.data?.title && videoInfo.data.channel && (
<UIMessage data-testid="selected-video" status="success">
<span className="font-bold italic">
{videoInfo.data.title} ({videoInfo.data.channel})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('YouTubePicker', () => {

[
{ data: undefined, isDisabled: true },
{ data: {}, isDisabled: false },
{ data: { restrictions: [] }, isDisabled: false },
].forEach(({ data, isDisabled }) => {
it('disables Continue button as long as no data is loaded from API', () => {
fakeUseAPIFetch.returns({ data });
Expand Down Expand Up @@ -103,6 +103,7 @@ describe('YouTubePicker', () => {
data: {
title: 'The video title',
channel: 'Hypothesis',
restrictions: [],
},
});

Expand All @@ -119,6 +120,7 @@ describe('YouTubePicker', () => {
data: {
title: 'The video title',
image: 'https://i.ytimg.com/vi/9l55oKI_Ch8/mqdefault.jpg',
restrictions: [],
},
});

Expand All @@ -134,6 +136,85 @@ describe('YouTubePicker', () => {
assert.equal(thumbnail.orientation, 'landscape');
});

context('when loaded video has restrictions', () => {
it('does not display video info', () => {
fakeUseAPIFetch.returns({
data: {
title: 'The video title',
channel: 'Hypothesis',
restrictions: ['age'],
},
});

const wrapper = renderComponent();
const metadata = wrapper.find('[data-testid="selected-video"]');

assert.isFalse(metadata.exists());
});

[
{
restriction: 'age',
expectedError:
'This video cannot be used in an assignment because it contains age-restricted content',
},
{
restriction: 'no_embed',
expectedError:
"This video cannot be used in an assignment because the video's owner does not allow this video to be embedded",
},
].forEach(({ restriction, expectedError }) => {
it('displays single error when loaded video has one restriction', () => {
fakeUseAPIFetch.returns({
isLoading: false,
data: {
title: 'The video title',
image: 'https://i.ytimg.com/vi/9l55oKI_Ch8/mqdefault.jpg',
restrictions: [restriction],
},
});

const wrapper = renderComponent();

assert.equal(
wrapper.find('URLFormWithPreview').prop('error'),
expectedError
);
});
});

it('displays multiple errors when video has more than one restriction', () => {
fakeUseAPIFetch.returns({
isLoading: false,
data: {
title: 'The video title',
image: 'https://i.ytimg.com/vi/9l55oKI_Ch8/mqdefault.jpg',
restrictions: ['age', 'no_embed'],
},
});

const wrapper = renderComponent();

// When there's more than one error, the `error` prop contains a component.
// It needs to be wrapped in a div because the top-most node is a fragment,
// which enzyme does not allow
const errorWrapper = mount(
<div>{wrapper.find('URLFormWithPreview').prop('error')}</div>
);
const errorText = errorWrapper.text();

assert.include(
errorText,
'This video cannot be used in an assignment because:'
);
assert.include(errorText, 'it contains age-restricted content');
assert.include(
errorText,
"the video's owner does not allow this video to be embedded"
);
});
});

[
{ errorCode: 'youtube_video_not_found', expectedError: 'Video not found' },
{
Expand Down

0 comments on commit c756a94

Please sign in to comment.