diff --git a/lms/product/d2l/_plugin/misc.py b/lms/product/d2l/_plugin/misc.py index 8a910ee01a..c1acea232f 100644 --- a/lms/product/d2l/_plugin/misc.py +++ b/lms/product/d2l/_plugin/misc.py @@ -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() diff --git a/lms/product/plugin/misc.py b/lms/product/plugin/misc.py index 96013cbfb2..cf77d087d6 100644 --- a/lms/product/plugin/misc.py +++ b/lms/product/plugin/misc.py @@ -1,3 +1,5 @@ +from typing import Optional + from lms.models import LTIParams, LTIRegistration from lms.services.html_service import strip_html_tags @@ -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: @@ -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): """ @@ -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"] diff --git a/tests/unit/lms/product/d2l/_plugin/misc_test.py b/tests/unit/lms/product/d2l/_plugin/misc_test.py index cf372b7f24..d24b5a7e3d 100644 --- a/tests/unit/lms/product/d2l/_plugin/misc_test.py +++ b/tests/unit/lms/product/d2l/_plugin/misc_test.py @@ -1,4 +1,4 @@ -from unittest.mock import patch, sentinel +from unittest.mock import sentinel import pytest @@ -15,39 +15,6 @@ 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) @@ -55,15 +22,3 @@ def test_factory(self, pyramid_request): @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 diff --git a/tests/unit/lms/product/plugin/misc_test.py b/tests/unit/lms/product/plugin/misc_test.py index 6e6a70e15e..d7733009aa 100644 --- a/tests/unit/lms/product/plugin/misc_test.py +++ b/tests/unit/lms/product/plugin/misc_test.py @@ -1,4 +1,4 @@ -from unittest.mock import sentinel +from unittest.mock import patch, sentinel import pytest @@ -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) @@ -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"})