Skip to content
Closed
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 h/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions h/security/policy/_auth_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class AuthClientPolicy:
("api.bulk.annotation", "POST"),
("api.bulk.group", "POST"),
("api.bulk.lms.annotations", "POST"),
("api.bulk.checkpoint", "POST"),
]

@classmethod
Expand Down
106 changes: 104 additions & 2 deletions h/services/checkpoint.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This coalesce keeps the existing reveal_date whenever the incoming one is NULL. I think this should be a plain override instead: {"reveal_date": parsed_reveal_date}

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

Expand Down
59 changes: 59 additions & 0 deletions h/views/api/bulk/checkpoint.py
Original file line number Diff line number Diff line change
@@ -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)
35 changes: 35 additions & 0 deletions h/views/api/bulk/checkpoint_schema.json
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions tests/unit/h/routes_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading