Skip to content

Commit

Permalink
Upsert LMSCourse every time we upsert Course
Browse files Browse the repository at this point in the history
Before doing a migration to backfill LMSCourse based on the information on
Course/Grouping start making the code aware of both types while writing to the
DB.
  • Loading branch information
marcospri committed Aug 23, 2024
1 parent c761113 commit 7541f48
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 3 deletions.
41 changes: 40 additions & 1 deletion lms/services/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
CourseGroupsExportedFromH,
Grouping,
GroupingMembership,
LMSCourse,
LMSCourseApplicationInstance,
LTIRole,
Organization,
RoleScope,
Expand All @@ -21,6 +23,7 @@
)
from lms.product.family import Family
from lms.services.grouping import GroupingService
from lms.services.upsert import bulk_upsert


class CourseService:
Expand Down Expand Up @@ -266,7 +269,7 @@ def upsert_course( # noqa: PLR0913
:param copied_from: A reference to the course this one was copied from
"""

return self._grouping_service.upsert_groupings(
course = self._grouping_service.upsert_groupings(
[
{
"lms_id": context_id,
Expand All @@ -279,6 +282,42 @@ def upsert_course( # noqa: PLR0913
copied_from=copied_from,
)[0]

self._upsert_lms_course(course)
return course

def _upsert_lms_course(self, course: Course) -> LMSCourse:
"""Upsert LMSCourse based on a Course object."""
self._db.flush() # Make sure Course has hit the DB on the current transaction

lms_course = bulk_upsert(
self._db,
LMSCourse,
[
{
"tool_consumer_instance_guid": course.application_instance.tool_consumer_instance_guid,
"lti_context_id": course.lms_id,
"h_authority_provided_id": course.authority_provided_id,
"copied_from_id": course.copied_from_id,
"name": course.lms_name,
}
],
index_elements=["h_authority_provided_id"],
update_columns=["updated", "name"],
).one()
bulk_upsert(
self._db,
LMSCourseApplicationInstance,
[
{
"application_instance_id": course.application_instance_id,
"lms_course_id": lms_course.id,
}
],
index_elements=["application_instance_id", "lms_course_id"],
update_columns=["updated"],
)
return lms_course

def find_group_set(self, group_set_id=None, name=None, context_id=None):
"""
Find the first matching group set in this course.
Expand Down
44 changes: 42 additions & 2 deletions tests/unit/lms/services/course_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from datetime import date, datetime
from unittest.mock import patch, sentinel
from unittest.mock import call, patch, sentinel

import pytest
from h_matchers import Any
Expand All @@ -9,6 +9,8 @@
ApplicationSettings,
CourseGroupsExportedFromH,
Grouping,
LMSCourse,
LMSCourseApplicationInstance,
RoleScope,
RoleType,
)
Expand Down Expand Up @@ -189,7 +191,7 @@ def test_get_from_launch_when_new_and_historical_course_exists(
)
assert course == upsert_course.return_value

def test_upsert_course(self, svc, grouping_service):
def test_upsert_course(self, svc, grouping_service, bulk_upsert, db_session):
course = svc.upsert_course(
context_id=sentinel.context_id,
name=sentinel.name,
Expand All @@ -211,6 +213,38 @@ def test_upsert_course(self, svc, grouping_service):
)

assert course == grouping_service.upsert_groupings.return_value[0]
bulk_upsert.assert_has_calls(
[
call(
db_session,
LMSCourse,
[
{
"tool_consumer_instance_guid": course.application_instance.tool_consumer_instance_guid,
"lti_context_id": course.lms_id,
"h_authority_provided_id": course.authority_provided_id,
"copied_from_id": course.copied_from_id,
"name": course.lms_name,
}
],
index_elements=["h_authority_provided_id"],
update_columns=["updated", "name"],
),
call().one(),
call(
db_session,
LMSCourseApplicationInstance,
[
{
"application_instance_id": course.application_instance_id,
"lms_course_id": bulk_upsert.return_value.one.return_value.id,
}
],
index_elements=["application_instance_id", "lms_course_id"],
update_columns=["updated"],
),
]
)

@pytest.mark.parametrize("canvas_sections_enabled", [True, False])
def test_upsert_course_sets_canvas_sections_enabled_based_on_legacy_rows(
Expand All @@ -220,6 +254,7 @@ def test_upsert_course_sets_canvas_sections_enabled_based_on_legacy_rows(
svc,
canvas_sections_enabled,
grouping_service,
bulk_upsert,
):
db_session.add(
CourseGroupsExportedFromH(
Expand All @@ -243,6 +278,7 @@ def test_upsert_course_sets_canvas_sections_enabled_based_on_legacy_rows(
type_=Any(),
copied_from=None,
)
bulk_upsert.assert_called()

@pytest.mark.usefixtures("course_with_group_sets")
@pytest.mark.parametrize(
Expand Down Expand Up @@ -483,6 +519,10 @@ def upsert_course(self, svc):
with patch.object(svc, "upsert_course") as upsert_course:
yield upsert_course

@pytest.fixture
def bulk_upsert(self, patch):
return patch("lms.services.course.bulk_upsert")


class TestCourseServiceFactory:
def test_it(self, pyramid_request, grouping_service, CourseService):
Expand Down

0 comments on commit 7541f48

Please sign in to comment.