Enable canvas assignment creation API #3126

marcospri · 2021-09-20T05:29:36Z

This is the final piece needed for storing all canvas assignments on the DB using the assigments.create endpoint merged already.

Here, the frontend will use this API if the backend provided the details only when creating/editing assignments.

Testing notes

Check already configured (in canvas) assignments can get launched

delete from module_item_configurations ; unless you want to keep some of your db assignments
HTML assignment, https://hypothesis.instructure.com/courses/125/assignments/1513. It won't store any record on the DB
Canvas file assigment, https://hypothesis.instructure.com/courses/125/assignments/875, It will store a new record in the DB, but not using the new code.

Edit existing assignments

Edit https://hypothesis.instructure.com/courses/125/assignments/1513 ( https://en.wikipedia.org/wiki/Margaret_(singer))
New record will show up on the DB ( no resource_link_id)
Launch it, resource_link_id will be filled now.
Edit https://hypothesis.instructure.com/courses/125/assignments/875 with a canvas file
New record will show up on the DB ( no resource_link_id) (note that this is a duplicate of another already exiting assignment. One will have a resource_link_id, the newest will have only ext_lti_assignment_id
Launch it. The record with only resource_link_id will get deleted and the new one will have all information completed.

Create new assignments

Create new assignments and launch them for both URL and canvas file. New row on creating, filled with resource_link_id on launch.

marcospri · 2021-09-20T09:14:43Z

lms/static/scripts/frontend_apps/components/FilePickerApp.js

+      setExtLTIAssignmentId(assignment.ext_lti_assignment_id);
+    }
+
+    if (content && createAssignmentAPI && !extLTIAssignmentId) {


Ony call the API if:

Content already selected

API enabled the config

And we don't have already called it

lms/static/scripts/frontend_apps/components/FilePickerApp.js

lms/static/scripts/frontend_apps/components/FilePickerFormFields.js

lms/static/scripts/frontend_apps/components/FilePickerApp.js

marcospri · 2021-09-20T13:38:55Z

lms/static/scripts/frontend_apps/components/FilePickerApp.js

@@ -94,13 +98,39 @@ export default function FilePickerApp({ onSubmit }) {
  // Submit the form after a selection is made via one of the available
  // methods.
  useEffect(() => {
+    async function createAssignment() {


Should this be defined outside as a const?

You can do, although you'd need to use useCallback to help Preact keep track of the effect's dependencies. This is fine for the moment.

marcospri · 2021-09-20T13:41:20Z

lms/static/scripts/frontend_apps/components/FilePickerApp.js

+
+      data.content = content;
+      data.groupset = groupConfig.groupSet;
+      const assignment = await apiCall({


This doesn't have an expected error response, eg a re-authorization.

Should I then try/catch it? If that's the case how can I show a generic, something went wrong message? (or something more usefull, but this case would likely be a bug).

Should I have boolean creatingAssignment or similar between this an getting the answer?

Should I then try/catch it? If that's the case how can I show a generic, something went wrong message? (or something more usefull, but this case would likely be a bug).

Yes. There is an existing errorInfo state in this component that shows a dialog when set. We can probably re-use that here.

Should I have boolean creatingAssignment or similar between this an getting the answer?

Does this call involve any calls to potentially slow third-party APIs or does all the work happen in our app. If the former, we probably should add a loading state. If the latter, it may be less important.

Added a try and used errorInfo. This is how the UI looks:

robertknight · 2021-09-23T09:17:36Z

lms/static/scripts/frontend_apps/components/FilePickerFormFields.js

@@ -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


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

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

lms/lms/validation/_api.py

Line 99 in 4e26002

"""Canvas only assignment unique identifier"""

to the API view itself:

lms/lms/views/api/assignments.py

Line 22 in 4e26002

def create(self):

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

lms/static/scripts/frontend_apps/components/FilePickerApp.js

marcospri · 2021-11-02T10:20:58Z

lms/static/scripts/frontend_apps/components/test/FilePickerApp-test.js

+      selectContent(wrapper, 'https://example.com');
+
+      await waitFor(() => fakeAPICall.called);
+      await delay(100);


This is the last change since you last looked at it @robertknight

Not sure if necessary of a symptom of something else that's missing/wrong.

Create and edit assignments by first calling the API endpoint and then submit the assignment to the LMS.

Until canvas assignment are stored on the DB on creation/editing this view will only handle launches of already present in the DB canvas file assignments (regular and SpeedGrader launches).

marcospri force-pushed the ext_lti_assignment_id-column-config branch from c94965d to ff0ba9a Compare September 20, 2021 05:44

marcospri force-pushed the ext_lti_assignment_id-column-frontend branch from 646d00e to c6bc581 Compare September 20, 2021 05:53

marcospri force-pushed the ext_lti_assignment_id-column-config branch from ff0ba9a to 664de4a Compare September 20, 2021 08:19

marcospri force-pushed the ext_lti_assignment_id-column-frontend branch from c6bc581 to 6d9499e Compare September 20, 2021 08:27

marcospri linked an issue Sep 20, 2021 that may be closed by this pull request

Move Canvas assignments into the DB #2527

Closed

marcospri commented Sep 20, 2021

View reviewed changes

marcospri mentioned this pull request Sep 20, 2021

Use DB configured canvas assignments in launches #3127

Merged

marcospri force-pushed the ext_lti_assignment_id-column-config branch from 664de4a to 1858755 Compare September 20, 2021 13:01

marcospri force-pushed the ext_lti_assignment_id-column-frontend branch 2 times, most recently from 057dae5 to 48a4c00 Compare September 20, 2021 13:37

marcospri commented Sep 20, 2021

View reviewed changes

marcospri requested a review from robertknight September 20, 2021 13:42

marcospri marked this pull request as ready for review September 20, 2021 13:42

robertknight reviewed Sep 23, 2021

View reviewed changes

lms/static/scripts/frontend_apps/components/FilePickerApp.js Outdated Show resolved Hide resolved

marcospri force-pushed the ext_lti_assignment_id-column-config branch from 1858755 to c673079 Compare September 23, 2021 10:40

marcospri force-pushed the ext_lti_assignment_id-column-frontend branch 4 times, most recently from d1f91f9 to 196834c Compare September 23, 2021 13:31

marcospri force-pushed the ext_lti_assignment_id-column-config branch from ddb9809 to 77c645d Compare September 27, 2021 15:23

marcospri force-pushed the ext_lti_assignment_id-column-frontend branch from 196834c to d46b786 Compare September 27, 2021 15:25

marcospri force-pushed the ext_lti_assignment_id-column-config branch from 77c645d to a225cb2 Compare September 28, 2021 12:49

Base automatically changed from ext_lti_assignment_id-column-config to ext_lti_assignment_id-column-view September 28, 2021 12:49

marcospri force-pushed the ext_lti_assignment_id-column-view branch 3 times, most recently from 74cd9ee to 7b0e418 Compare September 28, 2021 12:57

marcospri force-pushed the ext_lti_assignment_id-column-frontend branch from d46b786 to 9bcc06f Compare September 28, 2021 14:18

marcospri changed the base branch from ext_lti_assignment_id-column-view to ext_lti_assignment_id-column-launches September 28, 2021 14:18

marcospri force-pushed the ext_lti_assignment_id-column-launches branch from 667b97b to a0612b0 Compare September 29, 2021 09:41

marcospri added the ⚠️ do not merge ⚠️ label Oct 20, 2021

marcospri mentioned this pull request Nov 1, 2021

Re-enable canvas_db_configured_basic_lti_launch view #3320

Merged

marcospri removed the ⚠️ do not merge ⚠️ label Nov 2, 2021

marcospri force-pushed the ext_lti_assignment_id-column-frontend branch 4 times, most recently from a519fc7 to 3509a59 Compare November 2, 2021 10:07

marcospri requested a review from robertknight November 2, 2021 10:19

marcospri commented Nov 2, 2021

View reviewed changes

marcospri mentioned this pull request Nov 3, 2021

Handle VitalSource assignments in the creation endpoint schema #3327

Merged

marcospri changed the base branch from master to use-new-canvas-view November 4, 2021 15:46

marcospri force-pushed the ext_lti_assignment_id-column-frontend branch from 2b4b787 to 6565127 Compare November 5, 2021 09:06

robertknight approved these changes Nov 5, 2021

View reviewed changes

Base automatically changed from use-new-canvas-view to master November 9, 2021 09:50

marcospri mentioned this pull request Nov 9, 2021

Revert "Re-enable canvas_db_configured_basic_lti_launch view" #3354

Merged

marcospri force-pushed the ext_lti_assignment_id-column-frontend branch from 6565127 to 45ebf2b Compare November 9, 2021 12:07

Use the new createAssignmentAPI if configured by the backend

cdb1992

Create and edit assignments by first calling the API endpoint and then submit the assignment to the LMS.

marcospri force-pushed the ext_lti_assignment_id-column-frontend branch 2 times, most recently from c68e341 to cdb1992 Compare November 9, 2021 12:10

Enable canvas_db_configured_basic_lti_launch view

b5c730c

Until canvas assignment are stored on the DB on creation/editing this view will only handle launches of already present in the DB canvas file assignments (regular and SpeedGrader launches).

marcospri mentioned this pull request Nov 9, 2021

Store VS assignments on the DB #3341

Merged

marcospri added 4 commits November 12, 2021 12:26

Remove VS view predicate

09e40eb

Remove VS launch view

cbc33f9

Handle VS URL like other document_urls

0e6ba90

SpeedGrader in VS launches

4422ddc

marcospri changed the title ~~Ext lti assignment id column frontend~~ Enable canvas assignment creation API Nov 15, 2021

seanh approved these changes Nov 15, 2021

View reviewed changes

marcospri merged commit 0d95d5b into master Nov 15, 2021

marcospri deleted the ext_lti_assignment_id-column-frontend branch November 15, 2021 12:02

marcospri mentioned this pull request Nov 15, 2021

Revert "Enable canvas assignment creation API" #3390

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable canvas assignment creation API #3126

Enable canvas assignment creation API #3126

marcospri commented Sep 20, 2021 •

edited

marcospri Sep 20, 2021

marcospri Sep 20, 2021

robertknight Sep 23, 2021

marcospri Sep 20, 2021

robertknight Sep 23, 2021

marcospri Sep 23, 2021

robertknight Sep 23, 2021

marcospri Oct 19, 2021

marcospri Nov 2, 2021

Enable canvas assignment creation API #3126

Enable canvas assignment creation API #3126

Conversation

marcospri commented Sep 20, 2021 • edited

Testing notes

Check already configured (in canvas) assignments can get launched

Edit existing assignments

Create new assignments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcospri commented Sep 20, 2021 •

edited