diff --git a/lms/static/scripts/frontend_apps/components/BasicLTILaunchApp.tsx b/lms/static/scripts/frontend_apps/components/BasicLTILaunchApp.tsx index f3c2abe91e..c51b58c4d9 100644 --- a/lms/static/scripts/frontend_apps/components/BasicLTILaunchApp.tsx +++ b/lms/static/scripts/frontend_apps/components/BasicLTILaunchApp.tsx @@ -30,6 +30,19 @@ export type ErrorState = | 'error-reporting-submission' | LTILaunchServerErrorCode; +/** + * Return true if the user can dimiss an error. + * + * This is true for errors which are important, but don't completely prevent + * the user from participating in the assignment. + */ +function canDismissError(e: ErrorState) { + return [ + 'error-reporting-submission', + 'canvas_submission_course_not_available', + ].includes(e); +} + /** * Application displayed when an assignment is launched if the LMS backend * is unable to directly render the content in an iframe. This happens when @@ -88,8 +101,6 @@ export default function BasicLTILaunchApp() { const authWindow = useRef(null); const contentReady = !!contentURL; - const showContent = contentReady && !errorState; - const showSpinner = fetchCount > 0 && !errorState; const incFetchCount = () => { setFetchCount(count => count + 1); @@ -244,7 +255,9 @@ export default function BasicLTILaunchApp() { data: submissionParams, }); } catch (e) { - // If reporting the submission failed, replace the content with an error. + // If reporting the submission failed, show a modal error dialog above + // the content. + // // This avoids the student trying to complete the assignment without // knowing that there was a problem, and the teacher then not seeing a // submission. @@ -318,27 +331,32 @@ export default function BasicLTILaunchApp() { } }, [authToken, authURL, fetchContentURL, fetchGroups]); + const showSpinner = fetchCount > 0 && !errorState; + return (
{showSpinner && } - {errorState && ( - 0} - errorState={errorState} - error={error} - onRetry={authorizeAndFetchURL} - /> - )}
+ {errorState && ( + 0} + errorState={errorState} + error={error} + onDismiss={ + canDismissError(errorState) ? () => setErrorState(null) : undefined + } + onRetry={authorizeAndFetchURL} + /> + )}
); } diff --git a/lms/static/scripts/frontend_apps/components/LaunchErrorDialog.tsx b/lms/static/scripts/frontend_apps/components/LaunchErrorDialog.tsx index f1710d4b23..de5d3cb050 100644 --- a/lms/static/scripts/frontend_apps/components/LaunchErrorDialog.tsx +++ b/lms/static/scripts/frontend_apps/components/LaunchErrorDialog.tsx @@ -5,6 +5,7 @@ import { useConfig } from '../config'; import type { ErrorLike } from '../errors'; import type { ErrorState } from './BasicLTILaunchApp'; import ErrorModal from './ErrorModal'; +import type { ErrorModalProps } from './ErrorModal'; export type LaunchErrorDialogProps = { /** @@ -18,6 +19,9 @@ export type LaunchErrorDialogProps = { error: ErrorLike | null; /** Callback invoked when user clicks the "Try again" button */ onRetry: () => void; + + /** Callback invoked when user clicks the "Dismiss" button */ + onDismiss?: () => void; }; /** @@ -62,6 +66,7 @@ export default function LaunchErrorDialog({ busy, error, errorState, + onDismiss, onRetry, }: LaunchErrorDialogProps) { const { instructorToolbar } = useConfig(); @@ -79,7 +84,7 @@ export default function LaunchErrorDialog({ } // Common properties for error dialog. - const defaultProps = { + const defaultProps: ErrorModalProps = { busy, extraActions, error, @@ -90,6 +95,11 @@ export default function LaunchErrorDialog({ onRetry, }; + if (onDismiss) { + defaultProps.onCancel = onDismiss; + defaultProps.cancelLabel = 'Dismiss'; + } + switch (errorState) { case 'error-authorizing': // nb. There are no error details shown here, since failing to authorize @@ -625,18 +635,42 @@ export default function LaunchErrorDialog({ ); + case 'canvas_submission_course_not_available': + return ( + +

+ Your annotation was saved, but Hypothesis could not record a + submission for grading. +

+

+ This may be because the course has ended and we are no longer + allowed to create submissions. +

+

+ Your instructor is not aware you have saved an annotation. You may + need to reach out to them. +

+ + ); + case 'error-reporting-submission': - // nb. There is no retry action here as we just suggest reloading the entire - // page. return (

- There was a problem submitting this Hypothesis assignment.{' '} - To fix this problem, try reloading the page. + Your annotation was saved, but Hypothesis could not record a + submission for grading. +

+

+ Your instructor is not aware you have saved an annotation. You may + need to reach out to them.

); diff --git a/lms/static/scripts/frontend_apps/components/test/BasicLTILaunchApp-test.js b/lms/static/scripts/frontend_apps/components/test/BasicLTILaunchApp-test.js index 490cd0796a..4ddd4360d0 100644 --- a/lms/static/scripts/frontend_apps/components/test/BasicLTILaunchApp-test.js +++ b/lms/static/scripts/frontend_apps/components/test/BasicLTILaunchApp-test.js @@ -465,14 +465,16 @@ describe('BasicLTILaunchApp', () => { assert.isTrue(wrapper.exists('iframe')); }); - it('displays an error if reporting the submission fails', async () => { + // Make the grading submission API call fail. + function makeSubmissionFail() { const error = new APIError(400, {}); fakeApiCall .withArgs(sinon.match({ path: '/api/lti/submissions' })) .rejects(error); + return error; + } - const wrapper = renderLTILaunchApp(); - + async function waitForErrorDialog(wrapper) { // Wait for the API call to fail and check that an error is displayed. // There should be no "Try again" button in this context, instead we just // ask the user to reload the page. @@ -480,9 +482,27 @@ describe('BasicLTILaunchApp', () => { wrapper, 'LaunchErrorDialog[errorState="error-reporting-submission"]', ); + return errorDialog; + } + + it('displays an error if reporting the submission fails', async () => { + const error = makeSubmissionFail(); + const wrapper = renderLTILaunchApp(); + const errorDialog = await waitForErrorDialog(wrapper); assert.equal(errorDialog.prop('error'), error); }); + it('allows user to dismiss error', async () => { + makeSubmissionFail(); + const wrapper = renderLTILaunchApp(); + const errorDialog = await waitForErrorDialog(wrapper); + + errorDialog.prop('onDismiss')(); + wrapper.update(); + + assert.isFalse(wrapper.exists('LaunchErrorDialog')); + }); + it('does not report a submission if grading is not enabled for the current user', async () => { // If the assignment is not gradable or the user is an instructor, the // `speedGrader` config is omitted. @@ -544,22 +564,28 @@ describe('BasicLTILaunchApp', () => { assert.isFunction(annotationActivityCalls[0].args[1]); }); + // Simulate annotation activity being reported from the client to the + // LMS frontend. + async function reportActivity() { + const annotationActivityCalls = getOnActivityCalls(fakeRpcServer.on); + const callback = annotationActivityCalls[0].callback; + callback('create', { annotation: { isShared: true } }); + await delay(0); + return callback; + } + context( '`annotationActivity` event that qualifies for submission', () => { - it('submits a submission', async () => { + it('creates submission', async () => { renderLTILaunchApp(); assert.isFalse( fakeApiCall.calledWith( sinon.match({ path: '/api/lti/submissions' }), ), ); - const annotationActivityCalls = getOnActivityCalls( - fakeRpcServer.on, - ); - const callback = annotationActivityCalls[0].callback; - callback('create', { annotation: { isShared: true } }); - await delay(0); + + await reportActivity(); assert.calledOnce(fakeApiCall); assert.calledWith( @@ -581,12 +607,7 @@ describe('BasicLTILaunchApp', () => { it('deregisters callback after submission is made', async () => { renderLTILaunchApp(); - const annotationActivityCalls = getOnActivityCalls( - fakeRpcServer.on, - ); - const callback = annotationActivityCalls[0].args[1]; - callback('create', { annotation: { isShared: true } }); - await delay(0); + const callback = await reportActivity(); assert.calledOnce(fakeApiCall); assert.calledWith( @@ -803,7 +824,6 @@ describe('BasicLTILaunchApp', () => { wrapper, 'LaunchErrorDialog[errorState="error-authorizing"]', ); - await contentHidden(wrapper); }); context('when auth fails multiple times in a row', () => { diff --git a/lms/static/scripts/frontend_apps/components/test/LaunchErrorDialog-test.js b/lms/static/scripts/frontend_apps/components/test/LaunchErrorDialog-test.js index 25619a8c93..4b67716f5b 100644 --- a/lms/static/scripts/frontend_apps/components/test/LaunchErrorDialog-test.js +++ b/lms/static/scripts/frontend_apps/components/test/LaunchErrorDialog-test.js @@ -259,11 +259,17 @@ describe('LaunchErrorDialog', () => { }, { errorState: 'error-reporting-submission', - expectedText: 'There was a problem submitting this Hypothesis assignment', - expectedTitle: 'Something went wrong', + expectedText: + 'Your annotation was saved, but Hypothesis could not record a submission for grading.', + expectedTitle: 'Grading submission failed', hasRetry: false, withError: true, }, + { + errorState: 'canvas_submission_course_not_available', + expectedText: 'This may be because the course has ended', + expectedTitle: 'Grading submission failed', + }, ].forEach( ({ errorState, @@ -338,4 +344,22 @@ describe('LaunchErrorDialog', () => { assert.isTrue(editLink.exists()); assert.equal(editLink.prop('href'), '/app/content-item-selection'); }); + + it('does not allow dismissing error if `onDismiss` callback is not provided', () => { + const wrapper = renderDialog(); + const errorModal = wrapper.find('ErrorModal'); + assert.isUndefined(errorModal.prop('onCancel')); + assert.isUndefined(errorModal.prop('cancelLabel')); + }); + + it('allows dismissing error if `onDismiss` callback is provided', () => { + const onDismiss = sinon.stub(); + const wrapper = renderDialog({ onDismiss }); + const errorModal = wrapper.find('ErrorModal'); + + errorModal.prop('onCancel')(); + + assert.equal(errorModal.prop('cancelLabel'), 'Dismiss'); + assert.calledOnce(onDismiss); + }); }); diff --git a/lms/static/scripts/frontend_apps/errors.ts b/lms/static/scripts/frontend_apps/errors.ts index e0fef0d5fc..b975c3f0c6 100644 --- a/lms/static/scripts/frontend_apps/errors.ts +++ b/lms/static/scripts/frontend_apps/errors.ts @@ -23,6 +23,7 @@ export type LTILaunchServerErrorCode = | 'canvas_studio_transcript_unavailable' | 'canvas_studio_media_not_found' | 'canvas_studio_admin_token_refresh_failed' + | 'canvas_submission_course_not_available' | 'd2l_file_not_found_in_course_instructor' | 'd2l_file_not_found_in_course_student' | 'd2l_group_set_empty' @@ -171,6 +172,7 @@ export function isLTILaunchServerError(error: ErrorLike): error is APIError { 'canvas_studio_transcript_unavailable', 'canvas_studio_media_not_found', 'canvas_studio_admin_token_refresh_failed', + 'canvas_submission_course_not_available', 'vitalsource_user_not_found', 'vitalsource_no_book_license', 'moodle_page_not_found_in_course',