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

Enable canvas assignment creation API #3126

Merged
merged 6 commits into from
Nov 15, 2021
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
22 changes: 9 additions & 13 deletions lms/resources/_js_config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,24 +74,20 @@ def add_document_url(self, document_url):
resource_link_id=self._request.params["resource_link_id"],
),
}
elif document_url.startswith("vitalsource://"):
vitalsource_svc = self._request.find_service(name="vitalsource")
launch_url, launch_params = vitalsource_svc.get_launch_params(
document_url, self._request.lti_user
)
self._config["vitalSource"] = {
"launchUrl": launch_url,
"launchParams": launch_params,
}
else:
self._config["viaUrl"] = via_url(self._request, document_url)

self._add_canvas_speedgrader_settings(document_url=document_url)

def add_vitalsource_launch_config(self, book_id, cfi=None):
vitalsource_svc = self._request.find_service(name="vitalsource")
launch_url, launch_params = vitalsource_svc.get_launch_params(
book_id, cfi, self._request.lti_user
)
self._config["vitalSource"] = {
"launchUrl": launch_url,
"launchParams": launch_params,
}
self._add_canvas_speedgrader_settings(
vitalsource_book_id=book_id, vitalsource_cfi=cfi
)

def asdict(self):
"""
Return the configuration for the app's JavaScript code.
Expand Down
20 changes: 17 additions & 3 deletions lms/services/vitalsource/client.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
import re

import oauthlib
from oauthlib.oauth1 import SIGNATURE_HMAC_SHA1, SIGNATURE_TYPE_BODY

from lms.services.exceptions import ExternalRequestError
from lms.services.vitalsource._schemas import BookInfoSchema, BookTOCSchema

#: A regex for parsing the BOOK_ID and CFI parts out of one of our custom
#: vitalsource://book/bookID/BOOK_ID/cfi/CFI URLs.
DOCUMENT_URL_REGEX = re.compile(
r"vitalsource:\/\/book\/bookID\/(?P<book_id>[^\/]*)\/cfi\/(?P<cfi>.*)"
)


class VitalSourceService:
def __init__(self, http_service, lti_launch_key, lti_launch_secret, api_key):
Expand Down Expand Up @@ -54,7 +62,11 @@ def book_toc(self, book_id: str):

return BookTOCSchema(response).parse()

def get_launch_params(self, book_id, cfi, lti_user):
@staticmethod
def parse_document_url(document_url):
return DOCUMENT_URL_REGEX.search(document_url).groupdict()

def get_launch_params(self, document_url, lti_user):
"""
Return the form params needed to launch the VitalSource book viewer.

Expand All @@ -64,11 +76,13 @@ def get_launch_params(self, book_id, cfi, lti_user):

See https://developer.vitalsource.com/hc/en-us/articles/215612237-POST-LTI-Create-a-Bookshelf-Launch

:param book_id: The VitalSource book ID ("vbid")
:param cfi: Book location, as a Canonical Fragment Identifier, for deep linking.
:param document_url: `vitalsource://` type URL identifying the document.
:param lti_user: Current LTI user information, from the LTI launch request
:type lti_user: LTIUser
"""
url_params = self.parse_document_url(document_url)
book_id = url_params["book_id"]
cfi = url_params["cfi"]

launch_url = f"https://bc.vitalsource.com/books/{book_id}"
book_location = "/cfi" + cfi
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ export default function BasicLTILaunchApp() {
return;
}

// Don't report a submission until the URL has been successfully fetched.
if (!contentUrl) {
// Don't report a submission until we have the data needed to display the assignment content
if (!contentUrl && !vitalSourceConfig) {
return;
}
try {
Expand All @@ -251,7 +251,7 @@ export default function BasicLTILaunchApp() {
// submission.
handleError(e, 'error-reporting-submission', false);
}
}, [authToken, canvas.speedGrader, contentUrl]);
}, [authToken, canvas.speedGrader, contentUrl, vitalSourceConfig]);

useEffect(() => {
reportSubmission();
Expand Down
40 changes: 37 additions & 3 deletions lms/static/scripts/frontend_apps/components/FilePickerApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from 'preact/hooks';

import { Config } from '../config';
import { apiCall } from '../utils/api';
import { truncateURL } from '../utils/format';

import ContentSelector from './ContentSelector';
Expand Down Expand Up @@ -64,14 +65,17 @@ function contentDescription(content) {
export default function FilePickerApp({ onSubmit }) {
const submitButton = useRef(/** @type {HTMLInputElement|null} */ (null));
const {
api: { authToken },
filePicker: {
formAction,
formFields,
createAssignmentAPI: createAssignmentAPI,
canvas: { groupsEnabled: enableGroupConfig, ltiLaunchUrl },
},
} = useContext(Config);

const [content, setContent] = useState(/** @type {Content|null} */ (null));
const [extLTIAssignmentId, setExtLTIAssignmentId] = useState(null);

const [groupConfig, setGroupConfig] = useState(
/** @type {GroupConfig} */ ({
Expand All @@ -89,7 +93,36 @@ export default function FilePickerApp({ onSubmit }) {
* render.
*/
const [shouldSubmit, setShouldSubmit] = useState(false);
const submit = useCallback(() => setShouldSubmit(true), []);
const submit = useCallback(
async (/** @type {Content} */ content) => {
async function createAssignment() {
const data = {
...createAssignmentAPI.data,
content,
groupset: groupConfig.groupSet,
};
const assignment = await apiCall({
authToken,
path: createAssignmentAPI.path,
data,
});
setExtLTIAssignmentId(assignment.ext_lti_assignment_id);
}
if (content && createAssignmentAPI && !extLTIAssignmentId) {
try {
await createAssignment();
} catch (error) {
setErrorInfo({
message: 'Creating or editing an assignment',
error: error,
});
return;
Copy link
Member Author

Choose a reason for hiding this comment

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

The error is catched here and not in createAssignment to allow for this return and abort the submission.

}
}
setShouldSubmit(true);
},
[authToken, createAssignmentAPI, extLTIAssignmentId, groupConfig.groupSet]
);

// Submit the form after a selection is made via one of the available
// methods.
Expand All @@ -107,7 +140,7 @@ export default function FilePickerApp({ onSubmit }) {
content => {
setContent(content);
if (!enableGroupConfig) {
submit();
submit(content);
}
},
[enableGroupConfig, submit]
Expand Down Expand Up @@ -152,7 +185,7 @@ export default function FilePickerApp({ onSubmit }) {
<LabeledButton
disabled={groupConfig.useGroupSet && !groupConfig.groupSet}
variant="primary"
onClick={submit}
onClick={() => submit(content)}
>
Continue
</LabeledButton>
Expand All @@ -164,6 +197,7 @@ export default function FilePickerApp({ onSubmit }) {
ltiLaunchURL={ltiLaunchUrl}
content={content}
formFields={formFields}
extLTIAssignmentId={extLTIAssignmentId}
groupSet={groupConfig.useGroupSet ? groupConfig.groupSet : null}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { contentItemForContent } from '../utils/content-item';
* @prop {Record<string,string>} formFields - Form fields provided by the backend
* that should be included in the response without any changes
* @prop {string|null} groupSet
* @prop {string|null} extLTIAssignmentId
Copy link
Member

Choose a reason for hiding this comment

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

Is there a good piece documentation somewhere in the backend code about what this parameter means?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's not super explicit block of docs just for this but it can followed from the schema:

"""Canvas only assignment unique identifier"""

to the API view itself:

def create(self):

and in more detail on the comments on .get here https://github.com/hypothesis/lms/blob/4e26002eeba952c96d5edcd4d5314399b07cf1c6/lms/services/assignment.py

*/

/**
Expand All @@ -34,9 +35,13 @@ export default function FilePickerFormFields({
formFields,
groupSet,
ltiLaunchURL,
extLTIAssignmentId,
}) {
/** @type {Record<string,string>} */
const extraParams = groupSet ? { group_set: groupSet } : {};
if (extLTIAssignmentId) {
extraParams.ext_lti_assignment_id = extLTIAssignmentId;
}
marcospri marked this conversation as resolved.
Show resolved Hide resolved
const contentItem = JSON.stringify(
contentItemForContent(ltiLaunchURL, content, extraParams)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Config } from '../../config';
import FilePickerApp, { $imports } from '../FilePickerApp';
import { checkAccessibility } from '../../../test-util/accessibility';
import mockImportedComponents from '../../../test-util/mock-imported-components';
import { waitFor, waitForElement } from '../../../test-util/wait';

function interact(wrapper, callback) {
act(callback);
Expand All @@ -31,6 +32,9 @@ describe('FilePickerApp', () => {

beforeEach(() => {
fakeConfig = {
api: {
authToken: 'dummyAuthToken',
},
filePicker: {
formAction: 'https://www.shinylms.com/',
formFields: { hidden_field: 'hidden_value' },
Expand All @@ -55,12 +59,18 @@ describe('FilePickerApp', () => {
/**
* Check that the expected hidden form fields were set.
*/
function checkFormFields(wrapper, expectedContent, expectedGroupSet) {
function checkFormFields(
wrapper,
expectedContent,
expectedGroupSet,
expectedExtLTIAssignmentId
) {
Copy link
Member

Choose a reason for hiding this comment

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

Since several of these parameters are often null, it may help to change to named arguments, via an object in future.

function checkFormFields(wrapper, { content, groupSet, extLTIAssignmentId })

const formFields = wrapper.find('FilePickerFormFields');
assert.deepEqual(formFields.props(), {
children: [],
content: expectedContent,
formFields: fakeConfig.filePicker.formFields,
extLTIAssignmentId: expectedExtLTIAssignmentId,
groupSet: expectedGroupSet,
ltiLaunchURL: fakeConfig.filePicker.canvas.ltiLaunchUrl,
});
Expand Down Expand Up @@ -104,6 +114,76 @@ describe('FilePickerApp', () => {
});
}

context('when create assignment configuration is enabled', () => {
const authURL = 'https://testlms.hypothes.is/authorize';
const createAssignmentPath = '/api/canvas/assignments';
let fakeAPICall;
let fakeNewAssignment;

beforeEach(() => {
fakeConfig.filePicker.createAssignmentAPI = {
marcospri marked this conversation as resolved.
Show resolved Hide resolved
authURL,
path: createAssignmentPath,
};

fakeAPICall = sinon.stub();
fakeNewAssignment = { ext_lti_assignment_id: 10 };

fakeAPICall
.withArgs(sinon.match({ path: createAssignmentPath }))
.resolves(fakeNewAssignment);

$imports.$mock({
'../utils/api': { apiCall: fakeAPICall },
});
});

it('calls backend api when content is selected', async () => {
const onSubmit = sinon.stub().callsFake(e => e.preventDefault());
const wrapper = renderFilePicker({ onSubmit });

selectContent(wrapper, 'https://example.com');

await waitFor(() => fakeAPICall.called);
assert.calledWith(fakeAPICall, {
authToken: 'dummyAuthToken',
path: createAssignmentPath,
data: {
content: { type: 'url', url: 'https://example.com' },
groupset: null,
},
});

await waitFor(() => onSubmit.called, 100);

wrapper.update();
checkFormFields(
wrapper,
{
type: 'url',
url: 'https://example.com',
},
null /* groupSet */,
fakeNewAssignment.ext_lti_assignment_id
);
});

it('shows an error if creating the assignment fails', async () => {
const error = new Error('Something happened');
const onSubmit = sinon.stub().callsFake(e => e.preventDefault());
fakeAPICall
.withArgs(sinon.match({ path: createAssignmentPath }))
.rejects(error);

const wrapper = renderFilePicker({ onSubmit });

selectContent(wrapper, 'https://example.com');

const errDialog = await waitForElement(wrapper, 'ErrorDialog');
assert.equal(errDialog.length, 1);
assert.equal(errDialog.prop('error'), error);
});
});
context('when groups are not enabled', () => {
it('submits form when content is selected', () => {
const onSubmit = sinon.stub().callsFake(e => e.preventDefault());
Expand All @@ -118,7 +198,8 @@ describe('FilePickerApp', () => {
type: 'url',
url: 'https://example.com',
},
null /* groupSet */
null /* groupSet */,
null /* extLTIAssignmentId */
);
});

Expand Down Expand Up @@ -217,7 +298,8 @@ describe('FilePickerApp', () => {
type: 'url',
url: 'https://example.com',
},
useGroupSet ? 'groupSet1' : null
useGroupSet ? 'groupSet1' : null,
null /* extLTIAssignmentId */
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,23 @@ describe('FilePickerFormFields', () => {
);
});

it('adds `ext_lti_assignment_id` query param to LTI launch URL if `extLTIAssignmentId` prop is specified', () => {
const content = { type: 'url', url: 'https://example.com/' };
const formFields = createComponent({
content,
extLTIAssignmentId: 'EXT_LTI_ASSIGNMENT_ID',
});
const contentItems = JSON.parse(
formFields.find('input[name="content_items"]').prop('value')
);
assert.deepEqual(
contentItems,
contentItemForContent(launchURL, content, {
ext_lti_assignment_id: 'EXT_LTI_ASSIGNMENT_ID',
})
);
});

it('renders `document_url` field for URL content', () => {
const formFields = createComponent({
content: { type: 'url', url: 'https://example.com/' },
Expand Down
1 change: 1 addition & 0 deletions lms/validation/_lti_launch_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ def _decode_url(self, _data, **_kwargs): # pylint:disable=no-self-use
url.lower().startswith("http%3a")
or url.lower().startswith("https%3a")
or url.lower().startswith("canvas%3a")
or url.lower().startswith("vitalsource%3a")
):
url = unquote(url)
_data["url"] = url
Expand Down
Loading