From c0e9bda1ca16fdebbd10f81bf97a45d5de780e78 Mon Sep 17 00:00:00 2001 From: Elim Pizza Date: Wed, 3 Jun 2026 23:10:46 -0300 Subject: [PATCH 1/5] Add Checkpoint model and migration for Hide n Reveal --- ...794945d8e88_create_the_checkpoint_table.py | 48 +++++++++++++++++ h/models/__init__.py | 2 + h/models/checkpoint.py | 54 +++++++++++++++++++ 3 files changed, 104 insertions(+) create mode 100644 h/migrations/versions/3794945d8e88_create_the_checkpoint_table.py create mode 100644 h/models/checkpoint.py diff --git a/h/migrations/versions/3794945d8e88_create_the_checkpoint_table.py b/h/migrations/versions/3794945d8e88_create_the_checkpoint_table.py new file mode 100644 index 00000000000..bcfd2c6cefe --- /dev/null +++ b/h/migrations/versions/3794945d8e88_create_the_checkpoint_table.py @@ -0,0 +1,48 @@ +"""Create the checkpoint table.""" + +import sqlalchemy as sa +from alembic import op + +revision = "3794945d8e88" +down_revision = "a1b2c3d4e5f6" + + +def upgrade() -> None: + op.create_table( + "checkpoint", + sa.Column("id", sa.Integer(), autoincrement=True, nullable=False), + sa.Column("group_id", sa.Integer(), nullable=False), + sa.Column("document_uri", sa.UnicodeText(), nullable=False), + sa.Column("previous_checkpoint_id", sa.Integer(), nullable=True), + sa.Column("reveal_date", sa.DateTime(), nullable=True), + sa.Column( + "updated", sa.DateTime(), server_default=sa.text("now()"), nullable=False + ), + sa.Column( + "created", sa.DateTime(), server_default=sa.text("now()"), nullable=False + ), + sa.ForeignKeyConstraint( + ["group_id"], + ["group.id"], + name=op.f("fk__checkpoint__group_id__group"), + ondelete="CASCADE", + ), + sa.ForeignKeyConstraint( + ["previous_checkpoint_id"], + ["checkpoint.id"], + name=op.f("fk__checkpoint__previous_checkpoint_id__checkpoint"), + ondelete="CASCADE", + ), + sa.PrimaryKeyConstraint("id", name=op.f("pk__checkpoint")), + sa.UniqueConstraint( + "group_id", + "document_uri", + "previous_checkpoint_id", + name="uq__checkpoint__group_id__document_uri__previous_checkpoint_id", + postgresql_nulls_not_distinct=True, + ), + ) + + +def downgrade() -> None: + op.drop_table("checkpoint") diff --git a/h/models/__init__.py b/h/models/__init__.py index 9241cf8a887..a191bae89e6 100644 --- a/h/models/__init__.py +++ b/h/models/__init__.py @@ -25,6 +25,7 @@ from h.models.auth_ticket import AuthTicket from h.models.authz_code import AuthzCode from h.models.blocklist import Blocklist +from h.models.checkpoint import Checkpoint from h.models.document import Document, DocumentMeta, DocumentURI from h.models.feature import Feature from h.models.feature_cohort import FeatureCohort, FeatureCohortUser @@ -54,6 +55,7 @@ "AuthTicket", "AuthzCode", "Blocklist", + "Checkpoint", "Document", "DocumentMeta", "DocumentURI", diff --git a/h/models/checkpoint.py b/h/models/checkpoint.py new file mode 100644 index 00000000000..25450f860d1 --- /dev/null +++ b/h/models/checkpoint.py @@ -0,0 +1,54 @@ +from datetime import datetime + +import sqlalchemy as sa +from sqlalchemy.orm import Mapped, mapped_column + +from h.db import Base +from h.db.mixins import Timestamps +from h.models import helpers + + +class Checkpoint(Base, Timestamps): + """A hide/reveal checkpoint, synced from the LMS so h can authorize annotation visibility. + + Checkpoints form a per-(group, uri) linked list: a checkpoint's start date + is derived from its predecessor's ``reveal_date``, and the first one + (``previous_checkpoint_id`` NULL) starts at the assignment creation date. + Annotations stay hidden until their checkpoint's ``reveal_date`` passes. + """ + + __tablename__ = "checkpoint" + + id: Mapped[int] = mapped_column(sa.Integer, autoincrement=True, primary_key=True) + + group_id: Mapped[int] = mapped_column( + sa.Integer, sa.ForeignKey("group.id", ondelete="CASCADE"), nullable=False + ) + group = sa.orm.relationship("Group") + + document_uri: Mapped[str] = mapped_column(sa.UnicodeText, nullable=False) + + previous_checkpoint_id: Mapped[int | None] = mapped_column( + sa.Integer, sa.ForeignKey("checkpoint.id", ondelete="CASCADE"), nullable=True + ) + previous_checkpoint = sa.orm.relationship( + "Checkpoint", remote_side=[id], uselist=False + ) + + reveal_date: Mapped[datetime | None] = mapped_column(sa.DateTime, nullable=True) + """When the instructor reveals this checkpoint; NULL until revealed.""" + + __table_args__ = ( + # NULLS NOT DISTINCT (PG15+) so the NULL-previous root is unique too: + # at most one first checkpoint per (group, uri). + sa.UniqueConstraint( + "group_id", + "document_uri", + "previous_checkpoint_id", + name="uq__checkpoint__group_id__document_uri__previous_checkpoint_id", + postgresql_nulls_not_distinct=True, + ), + ) + + def __repr__(self) -> str: + return helpers.repr_(self, ["id", "group_id", "document_uri", "reveal_date"]) From bb6591ba911c96155d5cb25001acef43c83f644d Mon Sep 17 00:00:00 2001 From: Elim Pizza Date: Thu, 4 Jun 2026 08:58:12 -0300 Subject: [PATCH 2/5] Remove backtics from comment --- h/models/checkpoint.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/h/models/checkpoint.py b/h/models/checkpoint.py index 25450f860d1..147b4d01393 100644 --- a/h/models/checkpoint.py +++ b/h/models/checkpoint.py @@ -12,9 +12,9 @@ class Checkpoint(Base, Timestamps): """A hide/reveal checkpoint, synced from the LMS so h can authorize annotation visibility. Checkpoints form a per-(group, uri) linked list: a checkpoint's start date - is derived from its predecessor's ``reveal_date``, and the first one - (``previous_checkpoint_id`` NULL) starts at the assignment creation date. - Annotations stay hidden until their checkpoint's ``reveal_date`` passes. + is derived from its predecessor's reveal_date, and the first one + (previous_checkpoint_id NULL) starts at the assignment creation date. + Annotations stay hidden until their checkpoint's reveal_date passes. """ __tablename__ = "checkpoint" From 7641212ec7cc16d181942068c5b2df1886ddae75 Mon Sep 17 00:00:00 2001 From: Elim Pizza Date: Thu, 4 Jun 2026 08:59:04 -0300 Subject: [PATCH 3/5] change helper.repr fields for checkpoints --- h/models/checkpoint.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/h/models/checkpoint.py b/h/models/checkpoint.py index 147b4d01393..bc696c684b4 100644 --- a/h/models/checkpoint.py +++ b/h/models/checkpoint.py @@ -51,4 +51,6 @@ class Checkpoint(Base, Timestamps): ) def __repr__(self) -> str: - return helpers.repr_(self, ["id", "group_id", "document_uri", "reveal_date"]) + return helpers.repr_( + self, ["id", "group_id", "previous_checkpoint_id", "reveal_date"] + ) From 52ab9c770775f4e9510992083add90d0b30f0d13 Mon Sep 17 00:00:00 2001 From: Elim Pizza Date: Thu, 4 Jun 2026 12:46:23 -0300 Subject: [PATCH 4/5] bump coverage --- tests/common/factories/__init__.py | 1 + tests/common/factories/checkpoint.py | 15 +++++++++++++++ tests/unit/h/models/checkpoint_test.py | 10 ++++++++++ 3 files changed, 26 insertions(+) create mode 100644 tests/common/factories/checkpoint.py create mode 100644 tests/unit/h/models/checkpoint_test.py diff --git a/tests/common/factories/__init__.py b/tests/common/factories/__init__.py index 2ac8a12e6b5..fc91a41ea44 100644 --- a/tests/common/factories/__init__.py +++ b/tests/common/factories/__init__.py @@ -9,6 +9,7 @@ from tests.common.factories.auth_ticket import AuthTicket from tests.common.factories.authz_code import AuthzCode from tests.common.factories.base import set_session +from tests.common.factories.checkpoint import Checkpoint from tests.common.factories.document import Document, DocumentMeta, DocumentURI from tests.common.factories.feature import Feature from tests.common.factories.feature_cohort import FeatureCohort diff --git a/tests/common/factories/checkpoint.py b/tests/common/factories/checkpoint.py new file mode 100644 index 00000000000..7fb86dc3c31 --- /dev/null +++ b/tests/common/factories/checkpoint.py @@ -0,0 +1,15 @@ +import factory + +from h import models + +from .base import ModelFactory +from .group import Group + + +class Checkpoint(ModelFactory): + class Meta: + model = models.Checkpoint + sqlalchemy_session_persistence = "flush" + + group = factory.SubFactory(Group) + document_uri = factory.Faker("uri") diff --git a/tests/unit/h/models/checkpoint_test.py b/tests/unit/h/models/checkpoint_test.py new file mode 100644 index 00000000000..841b37726e9 --- /dev/null +++ b/tests/unit/h/models/checkpoint_test.py @@ -0,0 +1,10 @@ +class TestCheckpoint: + def test___repr__(self, factories): + checkpoint = factories.Checkpoint() + + assert repr(checkpoint) == ( + f"Checkpoint(id={checkpoint.id!r}, " + f"group_id={checkpoint.group_id!r}, " + f"previous_checkpoint_id={checkpoint.previous_checkpoint_id!r}, " + f"reveal_date={checkpoint.reveal_date!r})" + ) From 6d9d598469e2be3120260879e3195ecc54256d71 Mon Sep 17 00:00:00 2001 From: Elim Pizza Date: Fri, 5 Jun 2026 09:59:57 -0300 Subject: [PATCH 5/5] Key checkpoints by document_id instead of document_uri --- .../3794945d8e88_create_the_checkpoint_table.py | 12 +++++++++--- h/models/checkpoint.py | 13 ++++++++----- tests/common/factories/checkpoint.py | 3 ++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/h/migrations/versions/3794945d8e88_create_the_checkpoint_table.py b/h/migrations/versions/3794945d8e88_create_the_checkpoint_table.py index bcfd2c6cefe..4a1ef81f208 100644 --- a/h/migrations/versions/3794945d8e88_create_the_checkpoint_table.py +++ b/h/migrations/versions/3794945d8e88_create_the_checkpoint_table.py @@ -12,7 +12,7 @@ def upgrade() -> None: "checkpoint", sa.Column("id", sa.Integer(), autoincrement=True, nullable=False), sa.Column("group_id", sa.Integer(), nullable=False), - sa.Column("document_uri", sa.UnicodeText(), nullable=False), + sa.Column("document_id", sa.Integer(), nullable=False), sa.Column("previous_checkpoint_id", sa.Integer(), nullable=True), sa.Column("reveal_date", sa.DateTime(), nullable=True), sa.Column( @@ -27,6 +27,12 @@ def upgrade() -> None: name=op.f("fk__checkpoint__group_id__group"), ondelete="CASCADE", ), + sa.ForeignKeyConstraint( + ["document_id"], + ["document.id"], + name=op.f("fk__checkpoint__document_id__document"), + ondelete="CASCADE", + ), sa.ForeignKeyConstraint( ["previous_checkpoint_id"], ["checkpoint.id"], @@ -36,9 +42,9 @@ def upgrade() -> None: sa.PrimaryKeyConstraint("id", name=op.f("pk__checkpoint")), sa.UniqueConstraint( "group_id", - "document_uri", + "document_id", "previous_checkpoint_id", - name="uq__checkpoint__group_id__document_uri__previous_checkpoint_id", + name="uq__checkpoint__group_id__document_id__previous_checkpoint_id", postgresql_nulls_not_distinct=True, ), ) diff --git a/h/models/checkpoint.py b/h/models/checkpoint.py index bc696c684b4..4a1337e2515 100644 --- a/h/models/checkpoint.py +++ b/h/models/checkpoint.py @@ -11,8 +11,8 @@ class Checkpoint(Base, Timestamps): """A hide/reveal checkpoint, synced from the LMS so h can authorize annotation visibility. - Checkpoints form a per-(group, uri) linked list: a checkpoint's start date - is derived from its predecessor's reveal_date, and the first one + Checkpoints form a per-(group, document) linked list: a checkpoint's start + date is derived from its predecessor's reveal_date, and the first one (previous_checkpoint_id NULL) starts at the assignment creation date. Annotations stay hidden until their checkpoint's reveal_date passes. """ @@ -26,7 +26,10 @@ class Checkpoint(Base, Timestamps): ) group = sa.orm.relationship("Group") - document_uri: Mapped[str] = mapped_column(sa.UnicodeText, nullable=False) + document_id: Mapped[int] = mapped_column( + sa.Integer, sa.ForeignKey("document.id", ondelete="CASCADE"), nullable=False + ) + document = sa.orm.relationship("Document") previous_checkpoint_id: Mapped[int | None] = mapped_column( sa.Integer, sa.ForeignKey("checkpoint.id", ondelete="CASCADE"), nullable=True @@ -43,9 +46,9 @@ class Checkpoint(Base, Timestamps): # at most one first checkpoint per (group, uri). sa.UniqueConstraint( "group_id", - "document_uri", + "document_id", "previous_checkpoint_id", - name="uq__checkpoint__group_id__document_uri__previous_checkpoint_id", + name="uq__checkpoint__group_id__document_id__previous_checkpoint_id", postgresql_nulls_not_distinct=True, ), ) diff --git a/tests/common/factories/checkpoint.py b/tests/common/factories/checkpoint.py index 7fb86dc3c31..5b090cee9ea 100644 --- a/tests/common/factories/checkpoint.py +++ b/tests/common/factories/checkpoint.py @@ -3,6 +3,7 @@ from h import models from .base import ModelFactory +from .document import Document from .group import Group @@ -12,4 +13,4 @@ class Meta: sqlalchemy_session_persistence = "flush" group = factory.SubFactory(Group) - document_uri = factory.Faker("uri") + document = factory.SubFactory(Document)