fix: jump_to should redirect to first unit on invalid key

Previously, it would 404. While accurate, it's not a great user
experience. Users can be offered invalid jump_to paths in the normal
course of things, if course content disappears or they lose access
to it.

In both cases, they might be offered a resume URL in the courseware
that would be to a now-invalid location.

With this change, that invalid link will at least give them
*something* (the first unit in the course) rather than an error
page.

This also (unrelatedly) fixes an exception when the learning MFE
outline page tries to render a course that contains sequences
with no children.

AA-867

common/lib/xmodule/xmodule/contentstore/utils.py

@@ -1,5 +1,6 @@
 # lint-amnesty, pylint: disable=missing-module-docstring
 
+from opaque_keys.edx.keys import CourseKey, UsageKey
 from xmodule.contentstore.content import StaticContent
 
 from .django import contentstore
@@ -45,3 +46,12 @@ def restore_asset_from_trashcan(location):
             store.save(thumbnail_content)
         except Exception:  # lint-amnesty, pylint: disable=broad-except
             pass  # OK if this is left dangling
+
+
+def course_location_from_key(course_key: CourseKey) -> UsageKey:
+    """Creates a usage key for the toplevel course item, handling differences between mongo and newer keys"""
+    if getattr(course_key, 'deprecated', False):
+        block_id = course_key.run
+    else:
+        block_id = 'course'
+    return course_key.make_usage_key('course', block_id)

common/test/acceptance/fixtures/course.py

@@ -14,6 +14,7 @@ from path import Path
 
 from common.test.acceptance.fixtures import STUDIO_BASE_URL
 from common.test.acceptance.fixtures.base import FixtureError, XBlockContainerFixture
+from xmodule.contentstore.utils import course_location_from_key
 
 
 class XBlockFixtureDesc:
@@ -252,11 +253,7 @@ class CourseFixture(XBlockContainerFixture):
         Return the locator string for the course.
         """
         course_key = CourseKey.from_string(self._course_key)
-        if getattr(course_key, 'deprecated', False):
-            block_id = self._course_dict['run']
-        else:
-            block_id = 'course'
-        return str(course_key.make_usage_key('course', block_id))
+        return str(course_location_from_key(course_key))
 
     @property
     def _assets_url(self):

lms/djangoapps/course_home_api/outline/v1/views.py

@@ -310,7 +310,7 @@ class OutlineTabView(RetrieveAPIView):
                         # another type, just skip it (don't filter it out).
                         seq_data['type'] != 'sequential'
                     )
-                ]
+                ] if 'children' in chapter_data else []
 
         data = {
             'access_expiration': access_expiration,

lms/djangoapps/courseware/tests/test_views.py

@@ -186,15 +186,22 @@ class TestJumpTo(ModuleStoreTestCase):
     )
     @ddt.unpack
     def test_jump_to_invalid_location(self, activate_mfe, store_type):
+        """Confirm that invalid locations redirect back to a general course URL"""
         with self.store.default_store(store_type):
             course = CourseFactory.create()
             location = course.id.make_usage_key(None, 'NoSuchPlace')
+        expected_redirect_url = (
+            f'http://learning-mfe/course/{course.id}'
+        ) if activate_mfe else (
+            f'/courses/{course.id}/courseware?' + urlencode({'activate_block_id': str(course.location)})
+        )
         # This is fragile, but unfortunately the problem is that within the LMS we
         # can't use the reverse calls from the CMS
         jumpto_url = f'/courses/{course.id}/jump_to/{location}'
         with _set_mfe_flag(activate_mfe):
             response = self.client.get(jumpto_url)
-        assert response.status_code == 404
+        assert response.status_code == 302
+        assert response.url == expected_redirect_url
 
     @_set_mfe_flag(activate_mfe=False)
     @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)

lms/djangoapps/courseware/views/views.py

@@ -129,6 +129,7 @@ from common.djangoapps.util.cache import cache, cache_if_anonymous
 from common.djangoapps.util.db import outer_atomic
 from common.djangoapps.util.milestones_helpers import get_prerequisite_courses_display
 from common.djangoapps.util.views import ensure_valid_course_key, ensure_valid_usage_key
+from xmodule.contentstore.utils import course_location_from_key
 from xmodule.course_module import COURSE_VISIBILITY_PUBLIC, COURSE_VISIBILITY_PUBLIC_OUTLINE
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
@@ -413,8 +414,8 @@ def jump_to(request, course_id, location):
     try:
         course_key = CourseKey.from_string(course_id)
         usage_key = UsageKey.from_string(location).replace(course_key=course_key)
-    except InvalidKeyError:
-        raise Http404("Invalid course_key or usage_key")  # lint-amnesty, pylint: disable=raise-missing-from
+    except InvalidKeyError as exc:
+        raise Http404("Invalid course_key or usage_key") from exc
 
     experience_param = request.GET.get("experience", "").lower()
     if experience_param == "new":
@@ -430,10 +431,16 @@ def jump_to(request, course_id, location):
             request=request,
             experience=experience,
         )
-    except ItemNotFoundError:
-        raise Http404(f"No data at this location: {usage_key}")  # lint-amnesty, pylint: disable=raise-missing-from
-    except NoPathToItem:
-        raise Http404(f"This location is not in any class: {usage_key}")  # lint-amnesty, pylint: disable=raise-missing-from
+    except (ItemNotFoundError, NoPathToItem):
+        # We used to 404 here, but that's ultimately a bad experience. There are real world use cases where a user
+        # hits a no-longer-valid URL (for example, "resume" buttons that link to no-longer-existing block IDs if the
+        # course changed out from under the user). So instead, let's just redirect to the beginning of the course,
+        # as it is at least a valid page the user can interact with...
+        redirect_url = get_courseware_url(
+            usage_key=course_location_from_key(course_key),
+            request=request,
+            experience=experience,
+        )
 
     return redirect(redirect_url)
 

openedx/core/djangoapps/courseware_api/tests/test_views.py

@@ -427,6 +427,16 @@ class ResumeApiTestViews(BaseCoursewareTests, CompletionWaffleTestMixin):
         assert response.data['unit_id'] == str(self.unit.location)
         assert response.data['section_id'] == str(self.sequence.location)
 
+    def test_resume_invalid_key(self):
+        """A resume key that does not exist should return null IDs (i.e. "redirect to first section")"""
+        self.override_waffle_switch(True)
+        submit_completions_for_testing(self.user, [self.course.id.make_usage_key('html', 'doesnotexist')])
+        response = self.client.get(self.url)
+        assert response.status_code == 200
+        assert response.data['block_id'] is None
+        assert response.data['unit_id'] is None
+        assert response.data['section_id'] is None
+
 
 @ddt.ddt
 class CelebrationApiTestViews(BaseCoursewareTests, MasqueradeMixin):

openedx/core/djangoapps/courseware_api/views.py

@@ -2,8 +2,6 @@
 Course API Views
 """
 
-import json  # lint-amnesty, pylint: disable=unused-import
-
 from completion.exceptions import UnavailableCompletionData
 from completion.utilities import get_key_to_last_completed_block
 from django.conf import settings
@@ -62,6 +60,7 @@ from common.djangoapps.student.models import (
     LinkedInAddToProfileConfiguration
 )
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
 from xmodule.modulestore.search import path_to_location
 from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW
 
@@ -623,8 +622,8 @@ class Resume(DeveloperErrorViewMixin, APIView):
             resp['unit_id'] = str(path[3])
             resp['block_id'] = str(block_key)
 
-        except UnavailableCompletionData:
-            pass
+        except (ItemNotFoundError, NoPathToItem, UnavailableCompletionData):
+            pass  # leaving all the IDs as None indicates a redirect to the first unit in the course, as a backup
 
         return Response(resp)