diff --git a/h/routes.py b/h/routes.py index c8933018e66..ebd846839dd 100644 --- a/h/routes.py +++ b/h/routes.py @@ -159,6 +159,9 @@ def includeme(config): # noqa: PLR0915 config.add_route( "api.bulk.lms.annotations", "/api/bulk/lms/annotations", request_method="POST" ) + config.add_route( + "api.bulk.checkpoint", "/api/bulk/checkpoint", request_method="POST" + ) config.add_route("api.groups", "/api/groups", factory="h.traversal.GroupRoot") config.add_route( diff --git a/h/security/policy/_auth_client.py b/h/security/policy/_auth_client.py index 2cdb79ab630..77913cb42cb 100644 --- a/h/security/policy/_auth_client.py +++ b/h/security/policy/_auth_client.py @@ -39,6 +39,7 @@ class AuthClientPolicy: ("api.bulk.annotation", "POST"), ("api.bulk.group", "POST"), ("api.bulk.lms.annotations", "POST"), + ("api.bulk.checkpoint", "POST"), ] @classmethod diff --git a/h/services/checkpoint.py b/h/services/checkpoint.py index 1cd4383e169..097e0f49a15 100644 --- a/h/services/checkpoint.py +++ b/h/services/checkpoint.py @@ -1,17 +1,20 @@ from dataclasses import dataclass -from datetime import datetime +from datetime import UTC, datetime -from sqlalchemy import or_, select +from sqlalchemy import func, or_, select +from sqlalchemy.dialects.postgresql import insert from h.models import ( Annotation, Checkpoint, Document, DocumentURI, + Group, GroupMembership, User, ) from h.models.group import LMSRole +from h.schemas import ValidationError @dataclass @@ -89,6 +92,105 @@ def hidden_scopes(self, user: User | None) -> list[HiddenScope]: return [self._hidden_scope(user, checkpoint) for checkpoint in checkpoints] + def upsert_checkpoint( + self, + authority: str, + group_authority_provided_id: str, + document_uri: str, + reveal_date: str | None = None, + ) -> Checkpoint | None: + """ + Upsert a checkpoint for a (group, document) pair. + + Resolves the group by authority + authority_provided_id and the + document by URI. Creates the document if it doesn't exist yet. + + Returns the upserted Checkpoint, or None if the group or document + could not be resolved. + """ + group = self.db.scalar( + select(Group).where( + Group.authority == authority, + Group.authority_provided_id == group_authority_provided_id, + ) + ) + if not group: + return None + + document = Document.find_or_create_by_uris( + self.db, claimant_uri=document_uri, uris=[] + ).first() + if not document: + return None + + parsed_reveal_date = None + if reveal_date: + try: + parsed_reveal_date = datetime.fromisoformat(reveal_date) + except ValueError as err: + msg = f"Invalid reveal_date: {reveal_date!r}" + raise ValidationError(msg) from err + # Store naive UTC to match the column and the utcnow() comparisons. + if parsed_reveal_date.tzinfo is not None: + parsed_reveal_date = parsed_reveal_date.astimezone(UTC).replace( + tzinfo=None + ) + + stmt = ( + insert(Checkpoint) + .values( + group_id=group.id, + document_id=document.id, + previous_checkpoint_id=None, + reveal_date=parsed_reveal_date, + ) + # If a checkpoint already exists for this (group, document), update the + # reveal_date. coalesce keeps the existing date if the new one is NULL. + .on_conflict_do_update( + constraint="uq__checkpoint__group_id__document_id__previous_checkpoint_id", + set_={ + "reveal_date": func.coalesce( + parsed_reveal_date, Checkpoint.reveal_date + ) + }, + ) + .returning(Checkpoint.id) + ) + result = self.db.execute(stmt) + self.db.flush() + + checkpoint_id = result.scalar() + return self.db.get(Checkpoint, checkpoint_id) + + def set_instructor_role( + self, authority: str, username: str, group_authority_provided_ids: list[str] + ) -> None: + """ + Mark `username` as the LMS instructor in the given groups. + + Sets `lms_role = LMS_INSTRUCTOR` on the user's existing memberships in the + groups identified by `(authority, authority_provided_id)`. This is what + excludes an instructor's own annotations from Hide & Reveal hiding. + + Memberships that don't exist yet are left alone (the membership is created + by the group sync that precedes the checkpoint sync); a missing user or + group is a no-op. + """ + user = User.get_by_username(self.db, username, authority) + if user is None: + return + + memberships = self.db.scalars( + select(GroupMembership) + .join(Group, Group.id == GroupMembership.group_id) + .where(GroupMembership.user_id == user.id) + .where(Group.authority == authority) + .where(Group.authority_provided_id.in_(group_authority_provided_ids)) + ).all() + + for membership in memberships: + membership.lms_role = LMSRole.LMS_INSTRUCTOR.value + def _hidden_scope(self, user: User, checkpoint: Checkpoint) -> HiddenScope: group_pubid = checkpoint.group.pubid diff --git a/h/views/api/bulk/checkpoint.py b/h/views/api/bulk/checkpoint.py new file mode 100644 index 00000000000..5b938d0c511 --- /dev/null +++ b/h/views/api/bulk/checkpoint.py @@ -0,0 +1,59 @@ +import json + +from importlib_resources import files +from pyramid.response import Response + +from h.schemas.base import JSONSchema +from h.security import Permission +from h.services.checkpoint import CheckpointService +from h.views.api.config import api_config + + +class BulkCheckpointSchema(JSONSchema): + _SCHEMA_FILE = files("h.views.api.bulk") / "checkpoint_schema.json" + schema_version = 7 + schema = json.loads(_SCHEMA_FILE.read_text(encoding="utf-8")) + + +@api_config( + versions=["v1", "v2"], + route_name="api.bulk.checkpoint", + request_method="POST", + link_name="bulk.checkpoint", + description="Upsert checkpoints for hide and reveal", + permission=Permission.API.BULK_ACTION, +) +def upsert_checkpoints(request): + data = BulkCheckpointSchema().validate(request.json) + authority = data["authority"] + + checkpoint_service = request.find_service(CheckpointService) + + if instructor_username := data.get("instructor_username"): + # Mark the instructor role in all groups included in this request, + # since a single sync can cover multiple groups (e.g. several course sections). + checkpoint_service.set_instructor_role( + authority=authority, + username=instructor_username, + group_authority_provided_ids=[ + item["group_authority_provided_id"] for item in data["checkpoints"] + ], + ) + + results = [] + for item in data["checkpoints"]: + checkpoint = checkpoint_service.upsert_checkpoint( + authority=authority, + group_authority_provided_id=item["group_authority_provided_id"], + document_uri=item["document_uri"], + reveal_date=item.get("reveal_date"), + ) + results.append( + { + "group_authority_provided_id": item["group_authority_provided_id"], + "document_uri": item["document_uri"], + "created": checkpoint is not None, + } + ) + + return Response(json=results, status=200) diff --git a/h/views/api/bulk/checkpoint_schema.json b/h/views/api/bulk/checkpoint_schema.json new file mode 100644 index 00000000000..0d6fad79849 --- /dev/null +++ b/h/views/api/bulk/checkpoint_schema.json @@ -0,0 +1,35 @@ +{ + "$schema": "https://json-schema.org/draft-07/schema", + "type": "object", + "title": "Bulk checkpoint upsert", + "properties": { + "authority": { + "type": "string" + }, + "checkpoints": { + "type": "array", + "minItems": 1, + "items": { + "type": "object", + "properties": { + "group_authority_provided_id": { + "type": "string" + }, + "document_uri": { + "type": "string" + }, + "reveal_date": { + "type": ["string", "null"] + } + }, + "required": ["group_authority_provided_id", "document_uri"], + "additionalProperties": false + } + }, + "instructor_username": { + "type": "string" + } + }, + "required": ["authority", "checkpoints"], + "additionalProperties": false +} diff --git a/tests/unit/h/routes_test.py b/tests/unit/h/routes_test.py index ef41b3ccea6..16885351e79 100644 --- a/tests/unit/h/routes_test.py +++ b/tests/unit/h/routes_test.py @@ -153,6 +153,7 @@ def test_includeme(): "/api/bulk/lms/annotations", request_method="POST", ), + call("api.bulk.checkpoint", "/api/bulk/checkpoint", request_method="POST"), call("api.groups", "/api/groups", factory="h.traversal.GroupRoot"), call( "api.group", diff --git a/tests/unit/h/services/checkpoint_test.py b/tests/unit/h/services/checkpoint_test.py index ee633f55ac7..c47bcac5534 100644 --- a/tests/unit/h/services/checkpoint_test.py +++ b/tests/unit/h/services/checkpoint_test.py @@ -3,8 +3,9 @@ import pytest -from h.models import GroupMembership +from h.models import Document, GroupMembership from h.models.group import LMSRole +from h.schemas import ValidationError from h.services.checkpoint import CheckpointService, factory from h.util.uri import normalize as uri_normalize @@ -188,6 +189,150 @@ def null_role_membership(self, group, user): return self.membership(group, user, None) +class TestUpsert: + def test_it_creates_a_new_checkpoint(self, svc, factories): + group = factories.Group() + + checkpoint = svc.upsert_checkpoint( + authority=group.authority, + group_authority_provided_id=group.authority_provided_id, + document_uri="http://example.com/page", + reveal_date="2026-07-01T10:00:00", + ) + + assert checkpoint.group_id == group.id + assert checkpoint.previous_checkpoint_id is None + assert checkpoint.reveal_date == datetime(2026, 7, 1, 10, 0, 0) # noqa: DTZ001 + + def test_it_creates_a_checkpoint_with_no_reveal_date(self, svc, factories): + group = factories.Group() + + checkpoint = svc.upsert_checkpoint( + authority=group.authority, + group_authority_provided_id=group.authority_provided_id, + document_uri="http://example.com/page", + ) + + assert checkpoint.reveal_date is None + + def test_it_stores_a_timezone_aware_reveal_date_as_naive_utc(self, svc, factories): + group = factories.Group() + + checkpoint = svc.upsert_checkpoint( + authority=group.authority, + group_authority_provided_id=group.authority_provided_id, + document_uri="http://example.com/page", + reveal_date="2026-07-01T10:00:00+02:00", + ) + + # 10:00 at +02:00 is 08:00 UTC, stored without tzinfo. + assert checkpoint.reveal_date == datetime(2026, 7, 1, 8, 0, 0) # noqa: DTZ001 + + def test_it_updates_an_existing_checkpoint(self, svc, factories, db_session): + group = factories.Group() + document = factories.Document() + factories.DocumentURI(document=document, uri="http://example.com/page") + existing = factories.Checkpoint( + group=group, document=document, reveal_date=None + ) + # The bulk endpoint loads the checkpoint fresh; expire the cached instance + # so db.get() reloads the row the conflict-update actually wrote. + db_session.expire(existing) + + checkpoint = svc.upsert_checkpoint( + authority=group.authority, + group_authority_provided_id=group.authority_provided_id, + document_uri="http://example.com/page", + reveal_date="2026-07-01T10:00:00", + ) + + assert checkpoint.id == existing.id + assert checkpoint.reveal_date == datetime(2026, 7, 1, 10, 0, 0) # noqa: DTZ001 + + def test_it_raises_for_an_invalid_reveal_date(self, svc, factories): + group = factories.Group() + + with pytest.raises(ValidationError): + svc.upsert_checkpoint( + authority=group.authority, + group_authority_provided_id=group.authority_provided_id, + document_uri="http://example.com/page", + reveal_date="not-a-date", + ) + + def test_it_returns_None_when_the_group_is_not_found(self, svc): + assert ( + svc.upsert_checkpoint( + authority="example.com", + group_authority_provided_id="missing", + document_uri="http://example.com/page", + ) + is None + ) + + def test_it_returns_None_when_the_document_cannot_be_resolved( + self, svc, factories, mocker + ): + group = factories.Group() + query = mocker.Mock() + query.first.return_value = None + mocker.patch.object(Document, "find_or_create_by_uris", return_value=query) + + assert ( + svc.upsert_checkpoint( + authority=group.authority, + group_authority_provided_id=group.authority_provided_id, + document_uri="http://example.com/page", + ) + is None + ) + + +class TestSetInstructorRole: + def test_it_sets_the_instructor_role(self, svc, factories, db_session): + user = factories.User() + group = factories.Group() + membership = GroupMembership(user=user, group=group) + db_session.add(membership) + db_session.flush() + + svc.set_instructor_role( + authority=user.authority, + username=user.username, + group_authority_provided_ids=[group.authority_provided_id], + ) + + assert membership.lms_role == LMSRole.LMS_INSTRUCTOR.value + + def test_it_only_sets_the_role_in_the_given_groups( + self, svc, factories, db_session + ): + user = factories.User() + group = factories.Group() + other_group = factories.Group() + membership = GroupMembership(user=user, group=group) + other_membership = GroupMembership(user=user, group=other_group) + db_session.add_all([membership, other_membership]) + db_session.flush() + + svc.set_instructor_role( + authority=user.authority, + username=user.username, + group_authority_provided_ids=[group.authority_provided_id], + ) + + assert membership.lms_role == LMSRole.LMS_INSTRUCTOR.value + assert other_membership.lms_role is None + + def test_it_does_nothing_when_the_user_is_not_found(self, svc): + # A missing user is a no-op and must not raise. + svc.set_instructor_role( + authority="example.com", + username="nonexistent", + group_authority_provided_ids=["whatever"], + ) + + class TestFactory: def test_it(self, pyramid_request): svc = factory(mock.sentinel.context, pyramid_request) diff --git a/tests/unit/h/views/api/bulk/checkpoint_test.py b/tests/unit/h/views/api/bulk/checkpoint_test.py new file mode 100644 index 00000000000..fcf193d7e2b --- /dev/null +++ b/tests/unit/h/views/api/bulk/checkpoint_test.py @@ -0,0 +1,119 @@ +import pytest + +from h.schemas import ValidationError +from h.services.checkpoint import CheckpointService +from h.views.api.bulk.checkpoint import BulkCheckpointSchema, upsert_checkpoints + + +class TestBulkCheckpointSchema: + def test_it_is_a_valid_schema(self, schema): + # Basic self-check that this is a valid JSON schema. + assert not schema.validator.check_schema(schema.schema) + + @pytest.fixture + def schema(self): + return BulkCheckpointSchema() + + +@pytest.mark.usefixtures("checkpoint_service", "with_auth_client") +class TestUpsertCheckpoints: + def test_it(self, pyramid_request, checkpoint_service): + pyramid_request.json = { + "authority": "lms.hypothes.is", + "instructor_username": "teacher", + "checkpoints": [ + { + "group_authority_provided_id": "group1", + "document_uri": "http://example.com/1", + "reveal_date": "2026-07-01T10:00:00", + }, + { + "group_authority_provided_id": "group2", + "document_uri": "http://example.com/2", + }, + ], + } + + response = upsert_checkpoints(pyramid_request) + + checkpoint_service.set_instructor_role.assert_called_once_with( + authority="lms.hypothes.is", + username="teacher", + group_authority_provided_ids=["group1", "group2"], + ) + checkpoint_service.upsert_checkpoint.assert_any_call( + authority="lms.hypothes.is", + group_authority_provided_id="group1", + document_uri="http://example.com/1", + reveal_date="2026-07-01T10:00:00", + ) + checkpoint_service.upsert_checkpoint.assert_any_call( + authority="lms.hypothes.is", + group_authority_provided_id="group2", + document_uri="http://example.com/2", + reveal_date=None, + ) + assert response.status_code == 200 + assert response.json == [ + { + "group_authority_provided_id": "group1", + "document_uri": "http://example.com/1", + "created": True, + }, + { + "group_authority_provided_id": "group2", + "document_uri": "http://example.com/2", + "created": True, + }, + ] + + def test_it_does_not_set_instructor_role_without_an_instructor( + self, pyramid_request, checkpoint_service + ): + pyramid_request.json = { + "authority": "lms.hypothes.is", + "checkpoints": [ + { + "group_authority_provided_id": "group1", + "document_uri": "http://example.com/1", + }, + ], + } + + upsert_checkpoints(pyramid_request) + + checkpoint_service.set_instructor_role.assert_not_called() + + def test_it_reports_unresolved_items_as_not_created( + self, pyramid_request, checkpoint_service + ): + checkpoint_service.upsert_checkpoint.return_value = None + pyramid_request.json = { + "authority": "lms.hypothes.is", + "checkpoints": [ + { + "group_authority_provided_id": "group1", + "document_uri": "http://example.com/1", + }, + ], + } + + response = upsert_checkpoints(pyramid_request) + + assert response.json == [ + { + "group_authority_provided_id": "group1", + "document_uri": "http://example.com/1", + "created": False, + }, + ] + + def test_it_raises_with_invalid_request(self, pyramid_request): + pyramid_request.json = {"nope": True} + + with pytest.raises(ValidationError): + upsert_checkpoints(pyramid_request) + + @pytest.fixture + def checkpoint_service(self, mock_service): + return mock_service(CheckpointService)