Remove they has_document_url predicate and use a conditional in the…

… view

This refactor should be functionally equivalent but has a few advantages
that should be more obvious in follow up changes:

- We can remove the whole custom predicate infra as this is the only one
  that uses it

- We have no more control over getting the document as that's part of
  "our code" vs being called by pyramid before picking a view. Based on
  this we'll be able to move document "getting" to an assignment look up.

lms/views/lti/basic_launch.py

@@ -23,16 +23,6 @@
 from lms.validation import BasicLTILaunchSchema, ConfigureAssignmentSchema
 
 
-def has_document_url(_context, request):
-    """
-    Get if the current launch has a resolvable document URL.
-
-    This is imported into `lms.views.predicates` to provide the
-    `has_document_url` predicate.
-    """
-    return bool(request.product.plugin.misc.get_document_url(request))
-
-
 @view_defaults(
     request_method="POST",
     permission=Permissions.LTI_LAUNCH_ASSIGNMENT,
@@ -56,32 +46,28 @@ def __init__(self, context, request):
 
     @view_config(
         route_name="lti_launches",
-        has_document_url=True,
         renderer="lms:templates/lti/basic_launch/basic_launch.html.jinja2",
     )
-    def configured_launch(self):
-        """Display a document if we can resolve one to show."""
-
-        self._show_document(
-            document_url=self.request.product.plugin.misc.get_document_url(self.request)
-        )
-        self.request.registry.notify(
-            LTIEvent(request=self.request, type=LTIEvent.Type.CONFIGURED_LAUNCH)
-        )
-        return {}
+    def lti_launch(self):
+        """Handle regular LTI launches."""
 
-    @view_config(
-        route_name="lti_launches",
-        has_document_url=False,
-        renderer="lms:templates/file_picker.html.jinja2",
-    )
-    def unconfigured_launch(self):
-        """
-        Show the file-picker for the user to choose a document.
+        if document_url := self.request.product.plugin.misc.get_document_url(
+            self.request
+        ):
+            self.request.override_renderer = (
+                "lms:templates/lti/basic_launch/basic_launch.html.jinja2"
+            )
+            self._show_document(document_url=document_url)
+            self.request.registry.notify(
+                LTIEvent(request=self.request, type=LTIEvent.Type.CONFIGURED_LAUNCH)
+            )
+            return {}
 
-        This happens if we cannot resolve a document URL for any reason.
-        """
+        # Show the file-picker for the user to choose a document.
+        # This happens if we cannot resolve a document URL for any reason.
+        self.request.override_renderer = "lms:templates/file_picker.html.jinja2"
         self._configure_js_for_file_picker()
+
         return {}
 
     @view_config(route_name="lti.reconfigure", renderer="json")

tests/unit/lms/views/lti/basic_launch_test.py

@@ -8,21 +8,10 @@
 from lms.resources import LTILaunchResource
 from lms.resources._js_config import JSConfig
 from lms.security import Permissions
-from lms.views.lti.basic_launch import BasicLaunchViews, has_document_url
+from lms.views.lti.basic_launch import BasicLaunchViews
 from tests import factories
 
 
-class TestHasDocumentURL:
-    @pytest.mark.parametrize("document_url", (None, "a_url"))
-    def test_it(self, misc_plugin, pyramid_request, document_url):
-        misc_plugin.get_document_url.return_value = document_url
-
-        result = has_document_url(sentinel.context, pyramid_request)
-
-        misc_plugin.get_document_url.assert_called_once_with(pyramid_request)
-        assert result == bool(document_url)
-
-
 @pytest.mark.usefixtures(
     "assignment_service",
     "course_service",
@@ -171,33 +160,34 @@ def test_edit_assignment_callback(
             assignment_extra=expected_extras,
         )
 
-    def test_configured_launch(
+    def test_lti_launch_configured(
         self,
         svc,
         misc_plugin,
         pyramid_request,
         _show_document,
         LTIEvent,
     ):
-        svc.configured_launch()
+        misc_plugin.get_document_url.return_value = sentinel.document_url
+
+        svc.lti_launch()
 
         misc_plugin.get_document_url.assert_called_once_with(pyramid_request)
 
-        _show_document.assert_called_once_with(
-            document_url=misc_plugin.get_document_url.return_value
-        )
+        _show_document.assert_called_once_with(document_url=sentinel.document_url)
         LTIEvent.assert_called_once_with(
             request=pyramid_request,
             type=LTIEvent.Type.CONFIGURED_LAUNCH,
         )
         pyramid_request.registry.notify.has_call_with(LTIEvent.return_value)
 
-    def test_unconfigured_launch(self, svc, context, pyramid_request):
+    def test_lti_launch_unconfigured(self, svc, context, pyramid_request, misc_plugin):
+        misc_plugin.get_document_url.return_value = None
         pyramid_request.lti_params = mock.create_autospec(
             LTIParams, spec_set=True, instance=True
         )
 
-        svc.unconfigured_launch()
+        svc.lti_launch()
 
         pyramid_request.lti_params.serialize.assert_called_once_with(
             authorization=context.js_config.auth_token
@@ -207,12 +197,13 @@ def test_unconfigured_launch(self, svc, context, pyramid_request):
             form_fields=pyramid_request.lti_params.serialize.return_value,
         )
 
-    def test_unconfigured_launch_not_authorized(
-        self, context, pyramid_request, has_permission
+    def test_lti_launch_unconfigured_launch_not_authorized(
+        self, context, pyramid_request, has_permission, misc_plugin
     ):
         has_permission.return_value = False
+        misc_plugin.get_document_url.return_value = None
 
-        response = BasicLaunchViews(context, pyramid_request).unconfigured_launch()
+        response = BasicLaunchViews(context, pyramid_request).lti_launch()
 
         has_permission.assert_called_once_with(Permissions.LTI_CONFIGURE_ASSIGNMENT)
         assert (