Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display better error if Canvas Studio video cannot be used with Via #6233

Merged
merged 1 commit into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions lms/services/canvas_studio.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,19 +280,29 @@ def get_canonical_video_url(self, media_id: str) -> str:
# Example: "https://hypothesis.instructuremedia.com/api/public/v1/media/4"
return self._api_url(f"v1/media/{media_id}")

def get_video_download_url(self, media_id: str) -> str:
def get_video_download_url(self, media_id: str) -> str | None:
"""
Return temporary download URL for a video.

This may return `None` if the video is not available for download.
This can happen for videos imported into Canvas Studio from YouTube
or Vimeo.

Security: This method does not check whether the current user should
have access to this video. See `_admin_api_request`.
"""

download_rsp = self._bare_api_request(
f"v1/media/{media_id}/download", as_admin=True, allow_redirects=False
)
download_redirect = download_rsp.headers.get("Location")
try:
download_rsp = self._bare_api_request(
f"v1/media/{media_id}/download", as_admin=True, allow_redirects=False
)
except ExternalRequestError as err:
# Canvas Studio returns 422 if the video is not available for download.
if err.status_code == 422:
return None
raise

download_redirect = download_rsp.headers.get("Location")
if download_rsp.status_code != 302 or not download_redirect:
raise ExternalRequestError(
message="Media download did not return valid redirect",
Expand Down
36 changes: 36 additions & 0 deletions lms/static/scripts/frontend_apps/components/LaunchErrorDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,42 @@ export default function LaunchErrorDialog({
</ErrorModal>
);

case 'canvas_studio_download_unavailable':
return (
<ErrorModal
{...defaultProps}
onRetry={undefined}
title="Unable to fetch Canvas Studio video"
>
<p>
Only videos uploaded directly to Canvas Studio can be used. Videos
hosted on YouTube or Vimeo cannot be used.
</p>
</ErrorModal>
);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something slightly confusing about these error messages is the fact that what you see here is not the entire message. Instead the server-provided message is shown at the top, and the children of the ErrorModal are shown underneath.

For some of these errors, the server-side response includes only a code, for others there is a code and a message. Here I followed the pattern of those errors which include a short summary in the response, and then include extra details as ErrorModal children.

If we want to translate these error messages in future, then we'll need to have them all in one place though.


case 'canvas_studio_media_not_found':
return (
<ErrorModal
{...defaultProps}
onRetry={undefined}
title="Canvas Studio media not found"
/>
);

case 'canvas_studio_transcript_unavailable':
return (
<ErrorModal
{...defaultProps}
onRetry={undefined}
title="Video does not have a published transcript"
>
<p>
To use a video with Hypothesis, you must upload or generate captions
in Canvas Studio <i>and</i> publish them.
</p>
</ErrorModal>
);
case 'blackboard_group_set_not_found':
return (
<ErrorModal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,29 @@ describe('LaunchErrorDialog', () => {
hasRetry: true,
withError: true,
},
{
errorState: 'canvas_studio_download_unavailable',
expectedText:
'Only videos uploaded directly to Canvas Studio can be used. Videos hosted on YouTube or Vimeo cannot be used.',
expectedTitle: 'Unable to fetch Canvas Studio video',
hasRetry: false,
withError: true,
},
{
errorState: 'canvas_studio_media_not_found',
expectedText: '',
expectedTitle: 'Canvas Studio media not found',
hasRetry: false,
withError: true,
},
{
errorState: 'canvas_studio_transcript_unavailable',
expectedText:
'To use a video with Hypothesis, you must upload or generate captions in Canvas Studio and publish them.',
expectedTitle: 'Video does not have a published transcript',
hasRetry: false,
withError: true,
},
{
errorState: 'd2l_file_not_found_in_course_instructor',
expectedText:
Expand Down
6 changes: 6 additions & 0 deletions lms/static/scripts/frontend_apps/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ export type LTILaunchServerErrorCode =
| 'canvas_group_set_not_found'
| 'canvas_page_not_found_in_course'
| 'canvas_student_not_in_group'
| 'canvas_studio_download_unavailable'
| 'canvas_studio_transcript_unavailable'
| 'canvas_studio_media_not_found'
| 'd2l_file_not_found_in_course_instructor'
| 'd2l_file_not_found_in_course_student'
| 'd2l_group_set_empty'
Expand Down Expand Up @@ -162,6 +165,9 @@ export function isLTILaunchServerError(error: ErrorLike): error is APIError {
'canvas_group_set_not_found',
'canvas_group_set_empty',
'canvas_student_not_in_group',
'canvas_studio_download_unavailable',
'canvas_studio_transcript_unavailable',
'canvas_studio_media_not_found',
'vitalsource_user_not_found',
'vitalsource_no_book_license',
'moodle_page_not_found_in_course',
Expand Down
37 changes: 33 additions & 4 deletions lms/views/api/canvas_studio.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,29 @@
See `CanvasStudioService` for more details.
"""

from pyramid.httpexceptions import HTTPBadRequest, HTTPFound
from pyramid.httpexceptions import HTTPFound
from pyramid.view import view_config

from lms.security import Permissions
from lms.services import CanvasStudioService
from lms.services.canvas_studio import replace_localhost_in_url
from lms.services.exceptions import SerializableError
from lms.validation.authentication import OAuthCallbackSchema
from lms.views.helpers import via_video_url


class CanvasStudioLaunchError(SerializableError):
"""
An error occurred while launching a Canvas Studio assignment.

This exception is used for non-authorization errors that prevent a
Canvas Studio assignment from being launched.
"""

def __init__(self, error_code: str, message: str):
super().__init__(error_code=error_code, message=message)


# View for authorization popup which redirects to Canvas Studio's OAuth
# authorization endpoint.
#
Expand Down Expand Up @@ -135,15 +148,31 @@ def via_url(request):
document_url = assignment.document_url
media_id = CanvasStudioService.media_id_from_url(document_url)
if not media_id:
raise HTTPBadRequest("Unable to get Canvas Studio media ID")
raise CanvasStudioLaunchError(
"canvas_studio_media_not_found", "Unable to get Canvas Studio media ID"
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular error should never happen unless the document URL got edited on Canvas's side or we screwed up when generating it. However there is another related error which could happen, which is if the video gets removed in Canvas. I've used an error code which reflects that more general case.


svc = request.find_service(CanvasStudioService)
canonical_url = svc.get_canonical_video_url(media_id)

# Get the video download URL, then the transcript. We do things in this
# order because if the video cannot be used (eg. because it is a Vimeo
# upload), there is no point in the user uploading a transcript, if that is
# also missing.

download_url = svc.get_video_download_url(media_id)
transcript_url = svc.get_transcript_url(media_id)
if not download_url:
raise CanvasStudioLaunchError(
"canvas_studio_download_unavailable",
"Hypothesis was unable to fetch the video",
)

transcript_url = svc.get_transcript_url(media_id)
if not transcript_url:
raise HTTPBadRequest("This video does not have a published transcript")
raise CanvasStudioLaunchError(
"canvas_studio_transcript_unavailable",
"This video does not have a published transcript",
)

return {
"via_url": via_video_url(request, canonical_url, download_url, transcript_url)
Expand Down
8 changes: 8 additions & 0 deletions tests/unit/lms/services/canvas_studio_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ def test_get_video_download_error(self, svc):
svc.get_video_download_url("456")
assert Any.instance_of(exc_info.value.response).with_attrs({"status_code": 404})

def test_get_video_download_url_returns_None_if_video_not_available(self, svc):
assert svc.get_video_download_url("800") is None

def test_get_video_download_url_fails_if_admin_email_not_set(
self, svc, pyramid_request
):
Expand Down Expand Up @@ -412,6 +415,11 @@ def handler(url, allow_redirects=True):
case "media/456/download":
status_code = 404
json_data = {}
case "media/800/download":
# Simulate response in case where video is hosted on
# YouTube or Vimeo, so not available for download.
status_code = 422
json_data = {}

case _: # pragma: nocover
raise ValueError(f"Unexpected URL {url}")
Expand Down
28 changes: 23 additions & 5 deletions tests/unit/lms/views/api/canvas_studio_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import pytest
from h_matchers import Any
from pyramid.httpexceptions import HTTPBadRequest, HTTPFound
from pyramid.httpexceptions import HTTPFound

import lms.views.api.canvas_studio as views

Expand Down Expand Up @@ -104,21 +104,39 @@ def test_it_raises_if_transcript_not_available(
canvas_studio_service.get_transcript_url.return_value = None

with pytest.raises(
HTTPBadRequest, match="This video does not have a published transcript"
):
views.CanvasStudioLaunchError,
match="This video does not have a published transcript",
) as exc_info:
views.via_url(pyramid_request)

assert exc_info.value.error_code == "canvas_studio_transcript_unavailable"

def test_it_raises_if_document_url_not_valid(
self, pyramid_request, assignment_service
):
assignment_service.get_assignment.return_value.document_url = (
"https://not-a-canvas-studio-url.com"
)
with pytest.raises(
HTTPBadRequest, match="Unable to get Canvas Studio media ID"
):
views.CanvasStudioLaunchError, match="Unable to get Canvas Studio media ID"
) as exc_info:
views.via_url(pyramid_request)

assert exc_info.value.error_code == "canvas_studio_media_not_found"

def test_it_raises_if_download_not_available(
self, canvas_studio_service, pyramid_request
):
canvas_studio_service.get_video_download_url.return_value = None

with pytest.raises(
views.CanvasStudioLaunchError,
match="Hypothesis was unable to fetch the video",
) as exc_info:
views.via_url(pyramid_request)

assert exc_info.value.error_code == "canvas_studio_download_unavailable"

@pytest.fixture
def via_video_url(self, patch):
yield patch("lms.views.api.canvas_studio.via_video_url")
Expand Down
Loading