Skip to content

Commit

Permalink
Change approach to display acquired license for VS
Browse files Browse the repository at this point in the history
Instead of trying to guess the launch from the VitalSource course
materials via LTI parameters that will be different from each LMS show
the message in all cases for:

- Non DL launches
- No configured assignment

In the VS integration we don't use our non-DL filepicker because it
will required teachers selecting the H sku in *every* launch. We'll
always use DL to configure assignments.

That means that every non configured launch must be from the course
materials section.
  • Loading branch information
marcospri committed May 7, 2024
1 parent 0234439 commit a8288e2
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 19 deletions.
21 changes: 15 additions & 6 deletions lms/services/vitalsource/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from urllib.parse import parse_qs, urlencode, urlparse, urlunparse

from lms.error_code import ErrorCode
from lms.models import LTIParams, LTIUser
from lms.models import Assignment, LTIParams, LTIUser
from lms.services.vitalsource._client import VitalSourceClient
from lms.services.vitalsource.exceptions import VitalSourceMalformedRegex
from lms.services.vitalsource.model import VSBookLocation
Expand Down Expand Up @@ -185,17 +185,26 @@ def get_user_reference(self, lti_params: LTIParams) -> str | None:
return value

def check_h_license(
self, lti_user: LTIUser, lti_params: LTIParams
self, lti_user: LTIUser, lti_params: LTIParams, assignment: Assignment | None
) -> ErrorCode | None:
"""Check if the user of the current launch has a license for the H LTI app."""
if not self._student_pay_enabled:
# Not a school using student pay
return None

if lti_params["resource_link_id"] == lti_params["context_id"]:
# This looks like a "course launch"
# This means that the user launched our tool from the course
# "course materials" placement to acquire a license for our SKU.
if not assignment:
# This looks like a launch meant to acquire a license for our SKU
# While in Canvas that type of launch is always made from a "Course placement"
# the best method to detect these across all LMSes is:
# - A non Deep linking launch.
# Students never do DL launches
# We won't check here this is not a DL launch. We'll rely on the caller for that
# As we don't ever check licenses for instructors there's shudn't be a need to check license in DL.
#
# - No assignment in the DB
# We'd expect no assignment in DL launches but for a regular launch that means the instructor is accessing
# us via the Launch Courseware VS button
#
# We can't do anything else from here other than display a message
return (
ErrorCode.VITALSOURCE_STUDENT_PAY_LICENSE_LAUNCH_INSTRUCTOR
Expand Down
8 changes: 4 additions & 4 deletions lms/views/lti/basic_launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,16 @@ def __init__(self, context, request) -> None:
def lti_launch(self):
"""Handle regular LTI launches."""

assignment = self.assignment_service.get_assignment_for_launch(self.request)

if error_code := self.request.find_service(VitalSourceService).check_h_license(
self.request.lti_user, self.request.lti_params
self.request.lti_user, self.request.lti_params, assignment
):
self.request.override_renderer = "lms:templates/error_dialog.html.jinja2"
self.context.js_config.enable_error_dialog_mode(error_code)
return {}

if assignment := self.assignment_service.get_assignment_for_launch(
self.request
):
if assignment:
self.request.override_renderer = (
"lms:templates/lti/basic_launch/basic_launch.html.jinja2"
)
Expand Down
26 changes: 19 additions & 7 deletions tests/unit/lms/services/vitalsource/service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def test_check_h_license_disabled(self, svc, pyramid_request):
svc._student_pay_enabled = False # pylint:disable=protected-access

assert not svc.check_h_license(
pyramid_request.lti_user, pyramid_request.lti_params
pyramid_request.lti_user, pyramid_request.lti_params, sentinel.assignment
)

@pytest.mark.parametrize(
Expand All @@ -121,12 +121,12 @@ def test_check_h_license_course_launch(
self, request, svc, pyramid_request, user_fixture, code
):
svc._student_pay_enabled = True # pylint:disable=protected-access
pyramid_request.lti_params["context_id"] = "COURSE_ID"
pyramid_request.lti_params["resource_link_id"] = "COURSE_ID"
_ = request.getfixturevalue(user_fixture)

assert (
svc.check_h_license(pyramid_request.lti_user, pyramid_request.lti_params)
svc.check_h_license(
pyramid_request.lti_user, pyramid_request.lti_params, None
)
== code
)

Expand All @@ -136,7 +136,11 @@ def test_check_h_license_failure(self, svc, pyramid_request, customer_client):
customer_client.get_user_book_license.return_value = None

assert (
svc.check_h_license(pyramid_request.lti_user, pyramid_request.lti_params)
svc.check_h_license(
pyramid_request.lti_user,
pyramid_request.lti_params,
sentinel.assignment,
)
== ErrorCode.VITALSOURCE_STUDENT_PAY_NO_LICENSE
)

Expand All @@ -150,7 +154,11 @@ def test_check_h_license_success(self, svc, pyramid_request, customer_client):
customer_client.get_user_book_license.return_value = sentinel.license

assert not (
svc.check_h_license(pyramid_request.lti_user, pyramid_request.lti_params)
svc.check_h_license(
pyramid_request.lti_user,
pyramid_request.lti_params,
sentinel.assignment,
)
)

@pytest.mark.usefixtures("user_is_instructor")
Expand All @@ -160,7 +168,11 @@ def test_check_h_license_no_license_check_for_instructors(
svc._student_pay_enabled = True # pylint:disable=protected-access

assert not (
svc.check_h_license(pyramid_request.lti_user, pyramid_request.lti_params)
svc.check_h_license(
pyramid_request.lti_user,
pyramid_request.lti_params,
sentinel.assignment,
)
)
customer_client.get_user_book_license.assert_not_called()

Expand Down
6 changes: 4 additions & 2 deletions tests/unit/lms/views/lti/basic_launch_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,16 @@ def test_lti_launch_unconfigured_launch_not_authorized(
assert not response

def test_lti_launch_check_h_license_fails(
self, context, pyramid_request, vitalsource_service
self, context, pyramid_request, vitalsource_service, assignment_service
):
vitalsource_service.check_h_license.return_value = sentinel.error_code

response = BasicLaunchViews(context, pyramid_request).lti_launch()

vitalsource_service.check_h_license.assert_called_once_with(
pyramid_request.lti_user, pyramid_request.lti_params
pyramid_request.lti_user,
pyramid_request.lti_params,
assignment_service.get_assignment_for_launch.return_value,
)

assert (
Expand Down

0 comments on commit a8288e2

Please sign in to comment.