Skip to content

Commit

Permalink
Replace query to fetch an assignments course by the new relationship
Browse files Browse the repository at this point in the history
  • Loading branch information
marcospri committed Aug 7, 2024
1 parent 5fa6172 commit dab1822
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 45 deletions.
14 changes: 3 additions & 11 deletions lms/models/assignment.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import sqlalchemy as sa
from sqlalchemy.dialects.postgresql import JSONB
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy.orm import DynamicMapped, Mapped, mapped_column
from sqlalchemy.orm import DynamicMapped, Mapped, mapped_column, relationship

from lms.db import Base
from lms.models._mixins import CreatedUpdatedMixin
Expand Down Expand Up @@ -81,23 +81,15 @@ class Assignment(CreatedUpdatedMixin, Base):
groupings: DynamicMapped[Grouping] = sa.orm.relationship(
secondary="assignment_grouping", viewonly=True, lazy="dynamic"
)
"""Any groupings (courses, sections, groups) we have seen this assignment in during a launch"""

membership = sa.orm.relationship(
"AssignmentMembership", lazy="dynamic", viewonly=True
)

course_id: Mapped[int | None] = mapped_column(sa.ForeignKey(Course.id))

@property
def course(self):
"""Course this assignment belongs to."""
return (
self.groupings.filter_by(type="course")
.order_by(Grouping.created.desc())
# While logically one assignment belongs to only one course our grouping table might have more
# than one row representing the same course. Return the last created one.
.first()
)
course: Mapped[Course | None] = relationship(Course)

def get_canvas_mapped_file_id(self, file_id):
return self.extra.get("canvas_file_mappings", {}).get(file_id, file_id)
Expand Down
2 changes: 2 additions & 0 deletions lms/views/admin/assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ def assignment_dashboard(self):
)
)

# We can only navigate to assignments via their course, assigmentl.course won't be null
assert assignment.course
response = HTTPFound(
location=self.request.route_url(
"dashboard.assignment",
Expand Down
20 changes: 0 additions & 20 deletions tests/unit/lms/models/assignment_test.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import datetime

import pytest

from lms.models import Assignment
from tests import factories


class TestAssignment:
Expand Down Expand Up @@ -33,23 +30,6 @@ def test_get_canvas_mapped_file_id_returns_the_given_file_id_if_no_mapping_exist
):
assert assignment.get_canvas_mapped_file_id("file_id") == "file_id"

def test_course(self, assignment, db_session):
course = factories.Course(created=datetime.datetime(2024, 1, 1))
factories.AssignmentGrouping(
assignment=assignment, grouping=factories.CanvasSection()
)
factories.AssignmentGrouping(
assignment=assignment, grouping=factories.CanvasGroup()
)
factories.AssignmentGrouping(
assignment=assignment,
grouping=factories.Course(created=datetime.datetime(2022, 1, 1)),
)
factories.AssignmentGrouping(assignment=assignment, grouping=course)
db_session.flush()

assert assignment.course == course

@pytest.fixture
def assignment(self, db_session):
assignment = Assignment(
Expand Down
22 changes: 12 additions & 10 deletions tests/unit/lms/services/dashboard_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,13 @@ def test_get_request_assignment_for_admin(
svc,
organization,
db_session,
application_instance,
course,
get_request_admin_organizations,
):
assignment = factories.Assignment()
course = factories.Course(application_instance=application_instance)
factories.AssignmentGrouping(assignment=assignment, grouping=course)
assignment = factories.Assignment(course_id=course.id)
db_session.flush()
assignment_service.get_by_id.return_value = assignment
get_request_admin_organizations.return_value = [organization]
db_session.flush()

pyramid_request.matchdict["assignment_id"] = sentinel.id

Expand Down Expand Up @@ -98,13 +96,11 @@ def test_get_request_course_for_admin(
pyramid_request,
course_service,
svc,
application_instance,
organization,
get_request_admin_organizations,
course,
):
course_service.get_by_id.return_value = factories.Course(
application_instance=application_instance
)
course_service.get_by_id.return_value = course
get_request_admin_organizations.return_value = [organization]
pyramid_request.matchdict["course_id"] = sentinel.id

Expand Down Expand Up @@ -210,7 +206,13 @@ def organization_service(self, organization_service, organization):
organization_service.get_hierarchy_ids.return_value = []
return organization_service

@pytest.fixture
@pytest.fixture()
def course(self, db_session, application_instance):
course = factories.Course(application_instance=application_instance)
db_session.flush()
return course

@pytest.fixture()
def get_request_admin_organizations(self, svc):
with patch.object(
svc, "get_request_admin_organizations"
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/lms/views/admin/assignment_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ def test_assignment_dashboard(

@pytest.fixture
def assignment(self, application_instance, db_session):
assignment = factories.Assignment()
course = factories.Course(application_instance=application_instance)
assignment = factories.Assignment(course=course)
factories.AssignmentGrouping(assignment=assignment, grouping=course)
db_session.flush() # Give the DB objects IDs
return assignment
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/lms/views/dashboard/api/assignment_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def test_get_assignments(
}

def test_assignment(
self, views, pyramid_request, course, assignment, db_session, dashboard_service
self, views, pyramid_request, assignment, db_session, dashboard_service
):
db_session.flush()
pyramid_request.matchdict["assignment_id"] = sentinel.id
Expand All @@ -57,7 +57,7 @@ def test_assignment(
assert response == {
"id": assignment.id,
"title": assignment.title,
"course": {"id": course.id, "title": course.lms_name},
"course": {"id": assignment.course.id, "title": assignment.course.lms_name},
}

def test_course_assignments(
Expand Down Expand Up @@ -162,7 +162,7 @@ def get_page(self, patch):

@pytest.fixture
def assignment(self, course):
assignment = factories.Assignment()
assignment = factories.Assignment(course=course)
factories.AssignmentGrouping(assignment=assignment, grouping=course)

return assignment

0 comments on commit dab1822

Please sign in to comment.