diff --git a/lms/models/assignment.py b/lms/models/assignment.py index 5ea276950b..2888bbb3eb 100644 --- a/lms/models/assignment.py +++ b/lms/models/assignment.py @@ -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"), diff --git a/lms/resources/_js_config/__init__.py b/lms/resources/_js_config/__init__.py index 82102be179..816126c066 100644 --- a/lms/resources/_js_config/__init__.py +++ b/lms/resources/_js_config/__init__.py @@ -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 = ( @@ -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() @@ -781,7 +786,10 @@ 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 @@ -789,53 +797,56 @@ def _configure_groups(self, course, assignment): 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): diff --git a/lms/services/assignment.py b/lms/services/assignment.py index 7e18c8b59a..2f16c94e81 100644 --- a/lms/services/assignment.py +++ b/lms/services/assignment.py @@ -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 diff --git a/lms/services/lti_h.py b/lms/services/lti_h.py index 1e3d56aad8..d0f122f087 100644 --- a/lms/services/lti_h.py +++ b/lms/services/lti_h.py @@ -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, diff --git a/lms/static/scripts/frontend_apps/components/BasicLTILaunchApp.tsx b/lms/static/scripts/frontend_apps/components/BasicLTILaunchApp.tsx index 52f2a72d36..5d60396cd7 100644 --- a/lms/static/scripts/frontend_apps/components/BasicLTILaunchApp.tsx +++ b/lms/static/scripts/frontend_apps/components/BasicLTILaunchApp.tsx @@ -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 @@ -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({ + 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. 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 f3aeb3636d..ad61df7877 100644 --- a/lms/static/scripts/frontend_apps/components/test/BasicLTILaunchApp-test.js +++ b/lms/static/scripts/frontend_apps/components/test/BasicLTILaunchApp-test.js @@ -67,6 +67,7 @@ describe('BasicLTILaunchApp', () => { on: sinon.stub(), off: sinon.stub(), setGroups: sinon.stub(), + getDocumentUri: sinon.stub().resolves(''), }; $imports.$mock(mockImportedComponents()); @@ -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')); diff --git a/lms/static/scripts/frontend_apps/services/client-rpc.ts b/lms/static/scripts/frontend_apps/services/client-rpc.ts index 1df2e820e9..04eba32f66 100644 --- a/lms/static/scripts/frontend_apps/services/client-rpc.ts +++ b/lms/static/scripts/frontend_apps/services/client-rpc.ts @@ -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; + private _resolveDocumentUri: (uri: string) => void; private _server: Server; /** @@ -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(resolve => { + this._resolveDocumentUri = resolve; + }); + this._server.register('reportDocumentInfo', (info: DocumentInfo) => { + this._resolveDocumentUri(info.uri); + }); + this._resolveGroups = () => {}; const groups = new Promise(resolve => { this._resolveGroups = resolve; @@ -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 { + return this._documentUri; + } + /** * Set which user is focused in the client or none if `user` is `null`. * diff --git a/lms/static/scripts/frontend_apps/services/test/client-rpc-test.js b/lms/static/scripts/frontend_apps/services/test/client-rpc-test.js index 23109d659f..9745de246d 100644 --- a/lms/static/scripts/frontend_apps/services/test/client-rpc-test.js +++ b/lms/static/scripts/frontend_apps/services/test/client-rpc-test.js @@ -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(); diff --git a/lms/views/api/checkpoint.py b/lms/views/api/checkpoint.py index 30ed79a668..1ae19937d2 100644 --- a/lms/views/api/checkpoint.py +++ b/lms/views/api/checkpoint.py @@ -27,9 +27,12 @@ def reveal_checkpoint(request): # application instance (guards cross-institution reveal). # - Membership scope: the caller must be a member of the assignment (guards # an instructor of a different course within the same institution). + # - Identity scope: without a client-reported `document_uri` no checkpoint + # can have been synced to h, so there is nothing to reveal. if ( not assignment or not assignment.checkpoint_enabled + or not assignment.document_uri or not assignment.course or assignment.course.application_instance_id != request.lti_user.application_instance_id @@ -56,7 +59,7 @@ def reveal_checkpoint(request): checkpoints = [ { "group_authority_provided_id": grouping.authority_provided_id, - "document_uri": assignment.document_url, + "document_uri": assignment.document_uri, } for grouping in reveal_groupings ] diff --git a/lms/views/api/sync.py b/lms/views/api/sync.py index b1cbe2a41b..41cea88e36 100644 --- a/lms/views/api/sync.py +++ b/lms/views/api/sync.py @@ -1,6 +1,7 @@ from pyramid.view import view_config from webargs import fields +from lms.models import Grouping from lms.product.plugin.grouping import GroupError from lms.security import Permissions from lms.services.lti_h import checkpoint_sync_data @@ -13,6 +14,7 @@ class APISyncSchema(PyramidRequestSchema): group_set_id = fields.Str(required=False, allow_none=True) group_info = fields.Dict(required=True) gradingStudentId = fields.Str(required=False, allow_none=True) # noqa: N815 + document_uri = fields.Str(required=False, allow_none=True) @view_config( @@ -30,7 +32,28 @@ def sync(request): ) grading_student_id = request.parsed_params.get("gradingStudentId") - if group_set_id := request.parsed_params.get("group_set_id"): + assignment = assignment_service.get_assignment( + course.application_instance.tool_consumer_instance_guid, + request.parsed_params["resource_link_id"], + ) + + if ( + (reported_document_uri := request.parsed_params.get("document_uri")) + and assignment + and assignment.checkpoint_enabled + ): + assignment.document_uri = reported_document_uri + + grouping_type = grouping_service.get_launch_grouping_type( + request, course, assignment + ) + + if grouping_type == Grouping.Type.COURSE: + # Course-grouping assignments have no dynamic groupings to fetch. The + # client only calls /api/sync here to report the document identity and + # have us sync the checkpoint against the course group. + groupings = [course] + elif group_set_id := request.parsed_params.get("group_set_id"): course_copy_plugin = request.product.plugin.course_copy # For course copy we might have stored a mapping for this `group_set_id` group_set_id = course.get_mapped_group_set_id(group_set_id) @@ -68,13 +91,6 @@ def sync(request): grading_student_id=grading_student_id, ) - # Look up the assignment so we can sync checkpoint data for the actual - # groupings (sections or canvas groups), not just the course group. - assignment = assignment_service.get_assignment( - course.application_instance.tool_consumer_instance_guid, - request.parsed_params["resource_link_id"], - ) - # Sync the groups over to H so they are ready to be annotated against. # Also sync checkpoint data if the assignment has checkpoint enabled. h_checkpoint_results = request.find_service(name="lti_h").sync( diff --git a/tests/unit/lms/resources/_js_config/__init___test.py b/tests/unit/lms/resources/_js_config/__init___test.py index 3b4a8ec41f..5cffd48586 100644 --- a/tests/unit/lms/resources/_js_config/__init___test.py +++ b/tests/unit/lms/resources/_js_config/__init___test.py @@ -272,6 +272,22 @@ def test_configures_the_client_with_course_group( ] assert not config["api"]["sync"] + def test_configures_the_client_with_course_group_and_checkpoints( + self, js_config, grouping_service, course, assignment + ): + grouping_service.get_launch_grouping_type.return_value = Grouping.Type.COURSE + assignment.checkpoint_enabled = True + + js_config.enable_lti_launch_mode(course, assignment) + config = js_config.asdict() + + assert config["hypothesisClient"]["services"][0]["groups"] == [ + Any.string.matching("^group:.*@lms.hypothes.is") + ] + # Hide & Reveal assignments still need the sync API so the client can + # report the document identity and receive the checkpoint state. + assert config["api"]["sync"]["path"] == "/api/sync" + @pytest.mark.usefixtures("grouping_plugin") @pytest.mark.parametrize( "grouping_type", [Grouping.Type.SECTION, Grouping.Type.GROUP] @@ -560,10 +576,7 @@ def test_non_youtube_url_does_not_set_client_flag( def test_youtube_video_id_from_url_returns_none_on_parse_error(self): """Cover the except (ValueError, AttributeError) branch.""" with patch("lms.resources._js_config.urlparse", side_effect=ValueError): - assert ( - _youtube_video_id_from_url("https://www.youtube.com/watch?v=abc") - is None - ) + assert _youtube_video_id_from_url("https://youtube.com/watch?v=abc") is None def test_youtube_video_id_from_url_is_case_insensitive_for_host(self): """Host is normalized so YouTube.com / YOUTUBE.COM work like the frontend.""" diff --git a/tests/unit/lms/services/assignment_test.py b/tests/unit/lms/services/assignment_test.py index 49a03cd955..aff194029a 100644 --- a/tests/unit/lms/services/assignment_test.py +++ b/tests/unit/lms/services/assignment_test.py @@ -154,6 +154,44 @@ def test_update_assignment( assert assignment.lis_outcome_service_url == "GRADING URL" assert assignment.lti_v13_resource_link_id == v13_resource_link_id + def test_update_assignment_resets_document_uri_when_the_document_changes( + self, svc, pyramid_request, course + ): + assignment = factories.Assignment( + document_url="canvas://file/course/1/file_id/2", + document_uri="urn:x-pdf:FINGERPRINT", + ) + + assignment = svc.update_assignment( + pyramid_request, + assignment, + "canvas://file/course/1/file_id/3", + sentinel.group_set_id, + course, + ) + + # The new document's identity isn't known yet: the client re-reports it + # on the next launch (via /api/sync). + assert assignment.document_uri is None + + def test_update_assignment_keeps_document_uri_when_the_document_is_unchanged( + self, svc, pyramid_request, course + ): + assignment = factories.Assignment( + document_url="canvas://file/course/1/file_id/2", + document_uri="urn:x-pdf:FINGERPRINT", + ) + + assignment = svc.update_assignment( + pyramid_request, + assignment, + "canvas://file/course/1/file_id/2", + sentinel.group_set_id, + course, + ) + + assert assignment.document_uri == "urn:x-pdf:FINGERPRINT" + @pytest.mark.parametrize("with_existing", [True, False]) def test_update_assignment_with_auto_grading_config( self, svc, pyramid_request, course, with_existing diff --git a/tests/unit/lms/services/lti_h_test.py b/tests/unit/lms/services/lti_h_test.py index 6986955bde..3b070701d2 100644 --- a/tests/unit/lms/services/lti_h_test.py +++ b/tests/unit/lms/services/lti_h_test.py @@ -5,7 +5,7 @@ from lms.models import Grouping from lms.services import HAPIError -from lms.services.lti_h import LTIHService +from lms.services.lti_h import LTIHService, checkpoint_sync_data from tests import factories @@ -100,3 +100,46 @@ def h_user(self, pyramid_request): @pytest.fixture def grouping(self): return create_autospec(Grouping, instance=True, spec_set=True) + + +class TestCheckpointSyncData: + def test_it(self, lti_user): + assignment = factories.Assignment( + checkpoint_enabled=True, document_uri="https://example.com/doc" + ) + + assert checkpoint_sync_data(assignment, lti_user) == { + "document_uri": "https://example.com/doc", + "user": { + "username": lti_user.h_user.username, + "role": "student", + }, + } + + @pytest.mark.usefixtures("user_is_instructor") + def test_it_with_instructor(self, lti_user): + assignment = factories.Assignment( + checkpoint_enabled=True, document_uri="https://example.com/doc" + ) + + assert checkpoint_sync_data(assignment, lti_user)["user"]["role"] == ( + "instructor" + ) + + def test_it_returns_None_without_an_assignment(self, lti_user): + assert checkpoint_sync_data(None, lti_user) is None + + def test_it_returns_None_when_checkpoint_is_not_enabled(self, lti_user): + assignment = factories.Assignment( + checkpoint_enabled=False, document_uri="https://example.com/doc" + ) + + assert checkpoint_sync_data(assignment, lti_user) is None + + def test_it_returns_None_when_the_document_uri_is_not_known(self, lti_user): + # E.g. a file assignment whose PDF fingerprint hasn't been computed + # yet: syncing our internal document_url instead would create an h + # document no annotation ever matches. + assignment = factories.Assignment(checkpoint_enabled=True, document_uri=None) + + assert checkpoint_sync_data(assignment, lti_user) is None diff --git a/tests/unit/lms/views/api/checkpoint_test.py b/tests/unit/lms/views/api/checkpoint_test.py index f8fa080e12..f502c3419d 100644 --- a/tests/unit/lms/views/api/checkpoint_test.py +++ b/tests/unit/lms/views/api/checkpoint_test.py @@ -173,6 +173,25 @@ def test_it_reports_not_revealed_when_h_returns_no_results( assert result == {"revealed": False, "reveal_date": None} + @pytest.mark.usefixtures("user_is_instructor") + def test_it_returns_404_when_the_document_uri_is_not_known( + self, pyramid_request, assignment_service, h_api + ): + # If we never resolved an h document identity (e.g. a file assignment + # whose PDF fingerprint hasn't been computed), no checkpoint can have + # been synced, so there's nothing to reveal. + assignment = self._assignment_with_checkpoint( + pyramid_request.lti_user.application_instance_id + ) + assignment.document_uri = None + assignment_service.get_by_id.return_value = assignment + pyramid_request.matchdict = {"assignment_id": "1"} + + with pytest.raises(HTTPNotFound): + reveal_checkpoint(pyramid_request) + + h_api.reveal_checkpoints.assert_not_called() + @pytest.mark.usefixtures("user_is_instructor") def test_it_returns_404_when_no_groupings( self, pyramid_request, assignment_service @@ -190,7 +209,7 @@ def test_it_returns_404_when_no_groupings( def _assignment_with_checkpoint(self, application_instance_id=None): assignment = MagicMock() assignment.checkpoint_enabled = True - assignment.document_url = "https://example.com/doc" + assignment.document_uri = "https://example.com/doc" if application_instance_id is not None: assignment.course.application_instance_id = application_instance_id grouping = MagicMock() diff --git a/tests/unit/lms/views/api/sync_test.py b/tests/unit/lms/views/api/sync_test.py index 1c08a58e3d..bab463b61d 100644 --- a/tests/unit/lms/views/api/sync_test.py +++ b/tests/unit/lms/views/api/sync_test.py @@ -2,6 +2,7 @@ import pytest +from lms.models import Grouping from lms.product.plugin.grouping import GroupError from lms.views.api.sync import sync from tests import factories @@ -196,7 +197,7 @@ def test_it_syncs_checkpoint_data_with_sections( ): assignment = assignment_service.get_assignment.return_value assignment.checkpoint_enabled = True - assignment.document_url = "https://example.com/doc" + assignment.document_uri = "https://example.com/doc" sync(pyramid_request) @@ -222,7 +223,7 @@ def test_it_syncs_checkpoint_data_with_instructor( ): assignment = assignment_service.get_assignment.return_value assignment.checkpoint_enabled = True - assignment.document_url = "https://example.com/doc" + assignment.document_uri = "https://example.com/doc" sync(pyramid_request) @@ -249,7 +250,7 @@ def test_it_syncs_checkpoint_data_with_groups( pyramid_request.parsed_params["group_set_id"] = sentinel.group_set_id assignment = assignment_service.get_assignment.return_value assignment.checkpoint_enabled = True - assignment.document_url = "https://example.com/doc" + assignment.document_uri = "https://example.com/doc" sync(pyramid_request) @@ -274,7 +275,7 @@ def test_it_returns_checkpoint_state_from_h( ): assignment = assignment_service.get_assignment.return_value assignment.checkpoint_enabled = True - assignment.document_url = "https://example.com/doc" + assignment.document_uri = "https://example.com/doc" lti_h_service.sync.return_value = [ {"revealed": True, "reveal_date": "2026-07-01T12:00:00"} ] @@ -295,13 +296,45 @@ def test_it_omits_checkpoint_state_when_h_returns_no_results( ): assignment = assignment_service.get_assignment.return_value assignment.checkpoint_enabled = True - assignment.document_url = "https://example.com/doc" + assignment.document_uri = "https://example.com/doc" lti_h_service.sync.return_value = None result = sync(pyramid_request) assert "checkpoint" not in result + @pytest.mark.usefixtures("grouping_service", "course_service", "lti_h_service") + def test_it_stores_the_client_reported_document_uri( + self, pyramid_request, assignment_service + ): + assignment = assignment_service.get_assignment.return_value + assignment.checkpoint_enabled = True + pyramid_request.parsed_params["document_uri"] = "https://example.com/reported" + + sync(pyramid_request) + + assert assignment.document_uri == "https://example.com/reported" + + def test_it_with_course_grouping( + self, + pyramid_request, + grouping_service, + course_service, + assignment_service, # noqa: ARG002 + lti_h_service, + ): + grouping_service.get_launch_grouping_type.return_value = Grouping.Type.COURSE + course = course_service.get_by_context_id.return_value + + returned_ids = sync(pyramid_request) + + lti_h_service.sync.assert_called_once_with( + [course], + sentinel.group_info, + checkpoint_data=None, + ) + assert returned_ids["groups"] == [course.groupid.return_value] + @pytest.fixture def assignment_service(self, assignment_service): assignment_service.get_assignment.return_value.checkpoint_enabled = False