Skip to content

Commit

Permalink
Fallback to the deeplinking configuration for url by default
Browse files Browse the repository at this point in the history
The behaviour we had in D2L applies to all LMS where we have tried deep
linking being Canvas the exceptions which overrides get_document.

The behaviour is:

- If we have the assignment in the DB use its document
- If not, but we have a record of the original assignment the current
  launch was copied from, use that.

- If not, check if we have deep linking configuration in the launch and
  use that. This is likely the first launch of the assignment.

- Return None otherwise.
  • Loading branch information
marcospri committed Jan 2, 2024
1 parent bae7907 commit 643573c
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 64 deletions.
10 changes: 0 additions & 10 deletions lms/product/d2l/_plugin/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,6 @@ def get_ltia_aud_claim(self, lti_registration):
# `lti_registration.token_url` as in other LMS
return "https://api.brightspace.com/auth/token"

def get_document_url(self, request, assignment, historical_assignment):
url = super().get_document_url(request, assignment, historical_assignment)

if not url:
# In D2L support both deep linking and DB backed assignment.
# Use the DL url as a fallback.
url = self.get_deep_linked_assignment_configuration(request).get("url")

return url

@classmethod
def factory(cls, _context, request):
return cls()
14 changes: 7 additions & 7 deletions lms/product/plugin/misc.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import Optional

from lms.models import LTIParams, LTIRegistration
from lms.services.html_service import strip_html_tags

Expand Down Expand Up @@ -52,11 +54,8 @@ def get_ltia_aud_claim(self, lti_registration: LTIRegistration) -> str:
return lti_registration.token_url

def get_document_url(
self,
request, # pylint:disable=unused-argument
assignment,
historical_assignment,
):
self, request, assignment, historical_assignment
) -> Optional[str]:
"""Get a document URL from an assignment launch."""

if assignment:
Expand All @@ -65,7 +64,8 @@ def get_document_url(
if historical_assignment:
return historical_assignment.document_url

return None
# For LMSes that support both DL and non-DL assignments fallback to the DL parameters
return self.get_deep_linked_assignment_configuration(request).get("url")

def get_deeplinking_launch_url(self, request, _assignment_configuration: dict):
"""
Expand All @@ -79,7 +79,7 @@ def get_deeplinking_launch_url(self, request, _assignment_configuration: dict):
# The assignment configuration we'll be retrieved by other methods (eg, custom parameters) so that parameter is not used here.
return request.route_url("lti_launches")

def get_deep_linked_assignment_configuration(self, request):
def get_deep_linked_assignment_configuration(self, request) -> dict:
"""Get the configuration of an assignment that was original deep linked."""
params = {}
possible_parameters = ["url", "group_set"]
Expand Down
47 changes: 1 addition & 46 deletions tests/unit/lms/product/d2l/_plugin/misc_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from unittest.mock import patch, sentinel
from unittest.mock import sentinel

import pytest

Expand All @@ -15,55 +15,10 @@ def test_get_ltia_aud_claim(self, plugin):
== "https://api.brightspace.com/auth/token"
)

def test_get_document_default_behaviour(self, plugin, MiscPlugin):
MiscPlugin.get_document_url.return_value = sentinel.url

assert (
plugin.get_document_url(
sentinel.request, sentinel.assignment, sentinel.historical_assignment
)
== sentinel.url
)

def test_get_document_deep_linked_fallback(
self, plugin, MiscPlugin, get_deep_linked_assignment_configuration
):
MiscPlugin.get_document_url.return_value = None
get_deep_linked_assignment_configuration.return_value = {"url": sentinel.url}

assert (
plugin.get_document_url(
sentinel.request, sentinel.assignment, sentinel.historical_assignment
)
== sentinel.url
)

def test_get_document_returns_none(
self, plugin, MiscPlugin, get_deep_linked_assignment_configuration
):
MiscPlugin.get_document_url.return_value = None
get_deep_linked_assignment_configuration.return_value = {}

assert not plugin.get_document_url(
sentinel.request, sentinel.assignment, sentinel.historical_assignment
)

def test_factory(self, pyramid_request):
plugin = D2LMiscPlugin.factory(sentinel.context, pyramid_request)
assert isinstance(plugin, D2LMiscPlugin)

@pytest.fixture
def plugin(self):
return D2LMiscPlugin()

@pytest.fixture
def MiscPlugin(self):
with patch("lms.product.d2l._plugin.misc.super") as patched:
yield patched.return_value

@pytest.fixture
def get_deep_linked_assignment_configuration(self, plugin):
with patch.object(
plugin, "get_deep_linked_assignment_configuration", autospec=True
) as patched:
yield patched
16 changes: 15 additions & 1 deletion tests/unit/lms/product/plugin/misc_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from unittest.mock import sentinel
from unittest.mock import patch, sentinel

import pytest

Expand Down Expand Up @@ -52,6 +52,13 @@ def test_get_document_url_with_assignment_in_db_copied_assignment(

assert result == sentinel.document_url

def test_get_document_deep_linked_fallback(
self, plugin, get_deep_linked_assignment_configuration
):
get_deep_linked_assignment_configuration.return_value = {"url": sentinel.url}

assert plugin.get_document_url(sentinel.request, None, None) == sentinel.url

def test_get_document_url_with_no_document(self, plugin, pyramid_request):
assert not plugin.get_document_url(pyramid_request, None, None)

Expand Down Expand Up @@ -112,6 +119,13 @@ def _with_custom(custom):

return _with_custom

@pytest.fixture
def get_deep_linked_assignment_configuration(self, plugin):
with patch.object(
plugin, "get_deep_linked_assignment_configuration", autospec=True
) as patched:
yield patched

@pytest.fixture
def pyramid_request(self, pyramid_request):
pyramid_request.lti_params = LTIParams({"tool_consumer_instance_guid": "guid"})
Expand Down

0 comments on commit 643573c

Please sign in to comment.