Skip to content

Commit

Permalink
Merge branch 'main' into canvas-factory
Browse files Browse the repository at this point in the history
  • Loading branch information
marcospri authored Jan 8, 2025
2 parents ab978f7 + d3d4a05 commit 6a7bf82
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
"""Add start and end dates to courses."""

import sqlalchemy as sa
from alembic import op

revision = "0aa54a98ad39"
down_revision = "02d413b4d212"


def upgrade() -> None:
op.add_column("lms_course", sa.Column("starts_at", sa.DateTime(), nullable=True))
op.add_column("lms_course", sa.Column("ends_at", sa.DateTime(), nullable=True))


def downgrade() -> None:
op.drop_column("lms_course", "ends_at")
op.drop_column("lms_course", "starts_at")
7 changes: 7 additions & 0 deletions lms/models/lms_course.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- LMSCourse membership stores role information, GroupingMembership doesn't.
"""

from datetime import datetime
from typing import TYPE_CHECKING

import sqlalchemy as sa
Expand Down Expand Up @@ -46,6 +47,12 @@ class LMSCourse(CreatedUpdatedMixin, Base):
lti_context_memberships_url: Mapped[str | None] = mapped_column()
"""URL for the Names and Roles endpoint, stored during launch to use it outside the launch context."""

starts_at: Mapped[datetime | None] = mapped_column()
"""The start date of the course. Only for when we get this information directly from the LMS"""

ends_at: Mapped[datetime | None] = mapped_column()
"""The end date of the course. Only for when we get this information directly from the LMS"""


class LMSCourseApplicationInstance(CreatedUpdatedMixin, Base):
"""Record of on which installs (application instances) we have seen one course."""
Expand Down
31 changes: 30 additions & 1 deletion lms/services/course.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import json
from copy import deepcopy
from datetime import datetime
from typing import Mapping

from dateutil import parser
from sqlalchemy import Select, select, union

from lms.db import full_text_match
Expand Down Expand Up @@ -311,6 +314,8 @@ def _upsert_lms_course(self, course: Course, lti_params: LTIParams) -> LMSCourse
"https://purl.imsglobal.org/spec/lti-nrps/claim/namesroleservice", {}
).get("context_memberships_url")

course_starts_at, course_ends_at = self._get_course_dates(lti_params)

lms_course = bulk_upsert(
self._db,
LMSCourse,
Expand All @@ -321,10 +326,18 @@ def _upsert_lms_course(self, course: Course, lti_params: LTIParams) -> LMSCourse
"h_authority_provided_id": course.authority_provided_id,
"name": course.lms_name,
"lti_context_memberships_url": lti_context_membership_url,
"starts_at": course_starts_at,
"ends_at": course_ends_at,
}
],
index_elements=["h_authority_provided_id"],
update_columns=["updated", "name", "lti_context_memberships_url"],
update_columns=[
"updated",
"name",
"lti_context_memberships_url",
"starts_at",
"ends_at",
],
).one()
bulk_upsert(
self._db,
Expand Down Expand Up @@ -415,6 +428,22 @@ def _get_copied_from_course(self, lti_params) -> Course | None:

return None

def _get_course_dates(
self, lti_params: Mapping
) -> tuple[datetime | None, datetime | None]:
"""Get the dates for the current curse, None if not available."""
try:
course_starts_at = parser.isoparse(lti_params.get("custom_course_starts"))
except (TypeError, ValueError):
course_starts_at = None

try:
course_ends_at = parser.isoparse(lti_params.get("custom_course_ends"))
except (TypeError, ValueError):
course_ends_at = None

return course_starts_at, course_ends_at


def course_service_factory(_context, request):
return CourseService(
Expand Down
34 changes: 31 additions & 3 deletions tests/unit/lms/services/course_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from datetime import date, datetime
from datetime import UTC, date, datetime
from unittest.mock import call, patch, sentinel

import pytest
Expand Down Expand Up @@ -161,9 +161,29 @@ def test_get_from_launch_when_new_and_historical_course_exists(
)
assert course == upsert_course.return_value

@pytest.mark.parametrize(
"custom_course_starts, course_starts_at",
[(None, None), ("2022-01-01T00:00:00Z", datetime(2022, 1, 1, tzinfo=UTC))],
)
@pytest.mark.parametrize(
"custom_course_ends, course_ends_at",
[(None, None), ("2022-01-01T00:00:00Z", datetime(2022, 1, 1, tzinfo=UTC))],
)
def test_upsert_course(
self, svc, grouping_service, bulk_upsert, db_session, lti_params
self,
svc,
grouping_service,
bulk_upsert,
db_session,
lti_params,
custom_course_starts,
course_starts_at,
custom_course_ends,
course_ends_at,
):
lti_params["custom_course_starts"] = custom_course_starts
lti_params["custom_course_ends"] = custom_course_ends

course = svc.upsert_course(
lti_params=lti_params,
extra=sentinel.extra,
Expand Down Expand Up @@ -196,10 +216,18 @@ def test_upsert_course(
"h_authority_provided_id": course.authority_provided_id,
"name": course.lms_name,
"lti_context_memberships_url": None,
"starts_at": course_starts_at,
"ends_at": course_ends_at,
}
],
index_elements=["h_authority_provided_id"],
update_columns=["updated", "name", "lti_context_memberships_url"],
update_columns=[
"updated",
"name",
"lti_context_memberships_url",
"starts_at",
"ends_at",
],
),
call().one(),
call(
Expand Down

0 comments on commit 6a7bf82

Please sign in to comment.