Skip to content
Draft
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
3 changes: 3 additions & 0 deletions lms/models/assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ class Assignment(CreatedUpdatedMixin, Base):
document_url: Mapped[str] = mapped_column(sa.Unicode, nullable=False)
"""The URL of the document to be annotated for this assignment."""

document_uri: Mapped[str | None] = mapped_column(sa.Unicode, nullable=True)
"""The URI that identifies this assignment's document in h."""

extra: Mapped[MutableDict] = mapped_column(
MutableDict.as_mutable(JSONB()),
server_default=sa.text("'{}'::jsonb"),
Expand Down
101 changes: 56 additions & 45 deletions lms/resources/_js_config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,7 @@ def enable_toolbar_checkpoint(
toolbar_config["assignmentDueDate"] = due_date_iso
toolbar_config["assignmentCheckpointEnabled"] = True
self._config["instructorToolbar"] = toolbar_config
self._enable_document_info_reporting()

def enable_student_checkpoint(self, assignment, *, h_revealed=False):
due_date_iso = (
Expand All @@ -541,6 +542,10 @@ def enable_student_checkpoint(self, assignment, *, h_revealed=False):
"assignmentDueDate": due_date_iso,
"assignmentCheckpointEnabled": True,
}
self._enable_document_info_reporting()

def _enable_document_info_reporting(self):
self._hypothesis_client["reportDocumentInfo"] = True

def enable_toolbar_editing(self):
toolbar_config = self._get_toolbar_config()
Expand Down Expand Up @@ -781,61 +786,67 @@ def _configure_groups(self, course, assignment):
self._config["hypothesisClient"]["services"][0]["groups"] = [
course.groupid(self._authority)
]
self._config["api"]["sync"] = None
if assignment and assignment.checkpoint_enabled:
self._config["api"]["sync"] = self._sync_api_config(course, assignment)
else:
self._config["api"]["sync"] = None

else:
# If not using the default COURSE grouping point the FE
# to the sync API to dynamically get the relevant groupings.
self._config["hypothesisClient"]["services"][0]["groups"] = (
"$rpc:requestGroups"
)
self._config["api"]["sync"] = self._sync_api_config(course, assignment)

req = self._request
self._config["api"]["sync"] = {
"authUrl": (
req.route_url(req.product.route.oauth2_authorize)
if req.product.route.oauth2_authorize
else None
def _sync_api_config(self, course, assignment): # noqa: ARG002
"""Build the `api.sync` config the frontend uses to POST to /api/sync."""
req = self._request
return {
"authUrl": (
req.route_url(req.product.route.oauth2_authorize)
if req.product.route.oauth2_authorize
else None
),
"path": req.route_path("api.sync"),
# This data is consumed by the view in `lms.views.api.sync` which
# defines the arguments it expects. We need to match that
# description. Anything we add here should be echoed back by the
# frontend.
"data": {
"resource_link_id": assignment.resource_link_id,
"context_id": self._request.lti_params["context_id"],
"group_set_id": self._request.product.plugin.grouping.get_group_set_id(
self._request, assignment, historical_assignment=None
),
"path": req.route_path("api.sync"),
# This data is consumed by the view in `lms.views.api.sync` which
# defines the arguments it expects. We need to match that
# description. Anything we add here should be echoed back by the
# frontend.
"data": {
"resource_link_id": assignment.resource_link_id,
"context_id": self._request.lti_params["context_id"],
"group_set_id": self._request.product.plugin.grouping.get_group_set_id(
self._request, assignment, historical_assignment=None
),
"group_info": {
key: value
for key, value in self._request.lti_params.items()
if key
in {
# Most (all) of these are duplicated elsewhere, we'll keep updating for now
# because external analytics query rely on this table.
"context_id",
"context_title",
"context_label",
"tool_consumer_info_product_family_code",
"tool_consumer_info_version",
"tool_consumer_instance_name",
"tool_consumer_instance_description",
"tool_consumer_instance_url",
"tool_consumer_instance_contact_email",
"tool_consumer_instance_guid",
"custom_canvas_api_domain",
"custom_canvas_course_id",
}
},
# The student we are currently grading. In the case of Canvas
# this will be present in the SpeedGrader launch URL and
# available at launch time. When using our own grading bar this
# will be passed by the frontend
"gradingStudentId": req.params.get("learner_canvas_user_id"),
"group_info": {
key: value
for key, value in self._request.lti_params.items()
if key
in {
# Most (all) of these are duplicated elsewhere, we'll keep updating for now
# because external analytics query rely on this table.
"context_id",
"context_title",
"context_label",
"tool_consumer_info_product_family_code",
"tool_consumer_info_version",
"tool_consumer_instance_name",
"tool_consumer_instance_description",
"tool_consumer_instance_url",
"tool_consumer_instance_contact_email",
"tool_consumer_instance_guid",
"custom_canvas_api_domain",
"custom_canvas_course_id",
}
},
}
# The student we are currently grading. In the case of Canvas
# this will be present in the SpeedGrader launch URL and
# available at launch time. When using our own grading bar this
# will be passed by the frontend
"gradingStudentId": req.params.get("learner_canvas_user_id"),
},
}

def _get_user_info(self) -> User:
if self._request.has_permission(Permissions.STAFF):
Expand Down
5 changes: 5 additions & 0 deletions lms/services/assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,12 @@ def update_assignment( # noqa: PLR0913
# https://github.com/instructure/canvas-lms/issues/1952
return assignment

document_url_changed = assignment.document_url != document_url
assignment.document_url = document_url
if document_url_changed:
# If the document changed, any previously reported identity is stale, so
# clear it and let the client re-report it on this launch.
assignment.document_uri = None
assignment.extra["group_set_id"] = group_set_id

# Metadata based on the launch
Expand Down
12 changes: 7 additions & 5 deletions lms/services/lti_h.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,22 @@
def checkpoint_sync_data(assignment: Assignment | None, lti_user) -> dict | None:
"""Build the checkpoint payload to sync to h for a Hide & Reveal assignment.

Returns None when the assignment is missing or doesn't have checkpoint
enabled, so callers can pass the result straight through to
`LTIHService.sync(..., checkpoint_data=...)`.
Syncs against `assignment.document_uri` — the h document identity.
Returns None when the assignment is missing, doesn't have
checkpoint enabled, or the client hasn't reported an identity yet
(`assignment.document_uri` is None), so callers can pass the result straight
through to `LTIHService.sync(..., checkpoint_data=...)`.

reveal_date is not sent — h is the source of truth for the reveal state.
h's upsert uses coalesce to preserve an existing reveal_date when NULL
is sent.
"""
if not (assignment and assignment.checkpoint_enabled):
if not (assignment and assignment.checkpoint_enabled and assignment.document_uri):
return None

role = "instructor" if lti_user.is_instructor else "student"
return {
"document_uri": assignment.document_url,
"document_uri": assignment.document_uri,
"user": {
"username": lti_user.h_user.username,
"role": role,
Expand Down
35 changes: 35 additions & 0 deletions lms/static/scripts/frontend_apps/components/BasicLTILaunchApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,19 @@ export default function BasicLTILaunchApp() {
// Content URL to show in the iframe.
viaUrl: viaURL,
canvas,
instructorToolbar,
studentToolbar,
} = useConfig(['api', 'hypothesisClient']);

const clientRPC = useService(ClientRPC);

// Whether this is a Hide & Reveal assignment. Only then do we need to wait
// for the client to report the document's h identity and sync a checkpoint.
const checkpointEnabled = !!(
instructorToolbar?.assignmentCheckpointEnabled ??
studentToolbar?.assignmentCheckpointEnabled
);

// Canvas only: The presence of a value for this configuration property
// indicates that an empty grading submission should be made only after this
// (student) user performs qualifying annotation activity. Otherwise, a
Expand Down Expand Up @@ -245,6 +254,32 @@ export default function BasicLTILaunchApp() {
fetchGroups();
}, [fetchContentURL, fetchGroups]);

/**
* For Hide & Reveal assignments, report the document's h identity to the
* backend once the Hypothesis client computes it.
*/
useEffect(() => {
if (!syncAPICallInfo || !checkpointEnabled) {
return;
}

clientRPC.getDocumentUri().then(async documentUri => {
if (!documentUri) {
return;
}
try {
const { checkpoint } = await apiCall<SyncResponse>({
authToken,
path: syncAPICallInfo.path,
data: { ...syncAPICallInfo.data, document_uri: documentUri },
});
setSyncCheckpoint(checkpoint ?? null);
} catch {
// The group sync surfaces sync failures already.
}
});
}, [clientRPC, syncAPICallInfo, authToken, checkpointEnabled]);

/**
* Report a submission to the LMS, with the LMS-provided metadata needed for
* later grading of the assignment.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ describe('BasicLTILaunchApp', () => {
on: sinon.stub(),
off: sinon.stub(),
setGroups: sinon.stub(),
getDocumentUri: sinon.stub().resolves(''),
};

$imports.$mock(mockImportedComponents());
Expand Down Expand Up @@ -162,6 +163,69 @@ describe('BasicLTILaunchApp', () => {
});
});

context('when the assignment has Hide & Reveal checkpoints enabled', () => {
beforeEach(() => {
fakeConfig.api.sync = {
data: { course: { context_id: '12345' } },
path: '/api/sync',
};
fakeConfig.instructorToolbar = { assignmentCheckpointEnabled: true };
});

it('reports the document identity and syncs the checkpoint', async () => {
fakeRpcServer.getDocumentUri.resolves('urn:x-pdf:FINGERPRINT');
const checkpoint = { revealed: false, revealDate: null };
fakeApiCall.callsFake(async ({ data }) =>
data.document_uri ? { checkpoint } : { groups: ['group1'] },
);

const wrapper = renderLTILaunchApp();

await waitFor(() =>
fakeApiCall
.getCalls()
.some(
call => call.args[0].data.document_uri === 'urn:x-pdf:FINGERPRINT',
),
);
assert.calledWith(fakeApiCall, {
authToken: 'dummyAuthToken',
path: '/api/sync',
data: {
course: { context_id: '12345' },
document_uri: 'urn:x-pdf:FINGERPRINT',
},
});

await waitFor(() => {
wrapper.update();
return (
wrapper.find('InstructorToolbar').prop('syncCheckpoint') !== null
);
});
assert.deepEqual(
wrapper.find('InstructorToolbar').prop('syncCheckpoint'),
checkpoint,
);
});

it('does not sync a checkpoint when the client reports no document identity', async () => {
fakeRpcServer.getDocumentUri.resolves('');
fakeApiCall.resolves({ groups: ['group1'] });

renderLTILaunchApp();
await waitFor(() => fakeApiCall.called);
// Let the getDocumentUri promise settle before asserting.
await delay(0);

assert.isFalse(
fakeApiCall
.getCalls()
.some(call => 'document_uri' in call.args[0].data),
);
});
});

it('renders the instructor and student toolbars', () => {
const wrapper = renderLTILaunchApp();
assert.isTrue(wrapper.exists('InstructorToolbar'));
Expand Down
25 changes: 25 additions & 0 deletions lms/static/scripts/frontend_apps/services/client-rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,15 @@ export type ClientRPCOptions = {
* - Updating the Hypothesis client configuration in response to input
* in the LMS frontend, such as changing the focused user in grading mode.
*/
/** Argument for the `reportDocumentInfo` message from the client. */
type DocumentInfo = {
uri: string;
};

export class ClientRPC extends TinyEmitter {
private _resolveGroups: (groups: string[]) => void;
private _documentUri: Promise<string>;
private _resolveDocumentUri: (uri: string) => void;
private _server: Server;

/**
Expand Down Expand Up @@ -173,6 +180,14 @@ export class ClientRPC extends TinyEmitter {
// Expose current auth token via RPC
this._server.register('requestAuthToken', () => authToken);

this._resolveDocumentUri = () => {};
this._documentUri = new Promise<string>(resolve => {
this._resolveDocumentUri = resolve;
});
this._server.register('reportDocumentInfo', (info: DocumentInfo) => {
this._resolveDocumentUri(info.uri);
});

this._resolveGroups = () => {};
const groups = new Promise(resolve => {
this._resolveGroups = resolve;
Expand Down Expand Up @@ -200,6 +215,16 @@ export class ClientRPC extends TinyEmitter {
this._resolveGroups(groups);
}

/**
* Resolve with the loaded document's h identity, once the client reports it.
*
* The client reports this asynchronously after the document loads
* — the URI is only required for Hide & Reveal assignments.
*/
getDocumentUri(): Promise<string> {
return this._documentUri;
}

/**
* Set which user is focused in the client or none if `user` is `null`.
*
Expand Down
13 changes: 13 additions & 0 deletions lms/static/scripts/frontend_apps/services/test/client-rpc-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,19 @@ describe('ClientRPC', () => {
});
});

describe('getDocumentUri', () => {
it('resolves with the URI reported via the "reportDocumentInfo" RPC handler', async () => {
const clientRPC = createClientRPC();

const [, callback] = fakeServerInstance.register.args.find(
([method]) => method === 'reportDocumentInfo',
);
callback({ uri: 'https://example.com/doc' });

assert.equal(await clientRPC.getDocumentUri(), 'https://example.com/doc');
});
});

describe('setFocusedUser', () => {
it('sets focused user in client when user is passed', async () => {
const clientRPC = createClientRPC();
Expand Down
Loading
Loading