fix: correct studio sequence navigation bar with correct Next and Previous button actions

cms/djangoapps/contentstore/utils.py

@@ -582,62 +582,120 @@ def is_self_paced(course):
     return course and course.self_paced
 
 
-def get_sibling_urls(subsection):
+def get_sibling_urls(subsection, unit_location):    # pylint: disable=too-many-statements
     """
     Given a subsection, returns the urls for the next and previous units.
 
     (the first unit of the next subsection or section, and
     the last unit of the previous subsection/section)
     """
+    def get_unit_location(direction):
+        """
+        Returns the desired location of the adjacent unit in a subsection.
+        """
+        unit_index = subsection.children.index(unit_location)
+        try:
+            if direction == 'previous':
+                if unit_index > 0:
+                    location = subsection.children[unit_index - 1]
+                else:
+                    location = None
+            else:
+                location = subsection.children[unit_index + 1]
+            return location
+        except IndexError:
+            return None
+
+    def get_subsections_in_section():
+        """
+        Returns subsections present in a section.
+        """
+        try:
+            section_subsections = section.get_children()
+            return section_subsections
+        except AttributeError:
+            log.error("URL Retrieval Error: subsection {subsection} included in section {section}".format(
+                section=section.location,
+                subsection=subsection.location
+            ))
+            return None
+
+    def get_sections_in_course():
+        """
+        Returns sections present in course.
+        """
+        try:
+            section_subsections = section.get_parent().get_children()
+            return section_subsections
+        except AttributeError:
+            log.error("URL Retrieval Error: In section {section} in course".format(
+                section=section.location,
+            ))
+            return None
+
+    def get_subsection_location(section_subsections, current_subsection, direction):
+        """
+        Returns the desired location of the adjacent subsections in a section.
+        """
+        location = None
+        subsection_index = section_subsections.index(next(s for s in subsections if s.location ==
+                                                          current_subsection.location))
+        try:
+            if direction == 'previous':
+                if subsection_index > 0:
+                    prev_subsection = subsections[subsection_index - 1]
+                    location = prev_subsection.get_children()[-1].location
+            else:
+                next_subsection = subsections[subsection_index + 1]
+                location = next_subsection.get_children()[0].location
+            return location
+        except IndexError:
+            return None
+
+    def get_section_location(course_sections, current_section, direction):
+        """
+        Returns the desired location of the adjacent sections in a course.
+        """
+        location = None
+        section_index = course_sections.index(next(s for s in sections if s.location == current_section.location))
+        try:
+            if direction == 'previous':
+                if section_index > 0:
+                    prev_section = sections[section_index - 1]
+                    location = prev_section.get_children()[-1].get_children()[-1].location
+            else:
+                next_section = sections[section_index + 1]
+                location = next_section.get_children()[0].get_children()[0].location
+            return location
+        except IndexError:
+            return None
+
     section = subsection.get_parent()
     prev_url = next_url = ''
-    prev_loc = next_loc = None
-    last_block = None
-    siblings = list(section.get_children())
-    for i, block in enumerate(siblings):
-        if block.location == subsection.location:
-            if last_block:
-                try:
-                    prev_loc = last_block.get_children()[0].location
-                except IndexError:
-                    pass
-            try:
-                next_loc = siblings[i + 1].get_children()[0].location
-            except IndexError:
-                pass
-            break
-        last_block = block
+
+    prev_loc = get_unit_location('previous')
+    next_loc = get_unit_location('next')
+
     if not prev_loc:
-        try:
-            # section.get_parent SHOULD return the course, but for some reason, it might not
-            sections = section.get_parent().get_children()
-        except AttributeError:
-            log.error("URL Retrieval Error # 1: subsection {subsection} included in section {section}".format(
-                section=section.location,
-                subsection=subsection.location
-            ))
-            # This should not be a fatal error. The worst case is that the navigation on the unit page
-            # won't display a link to a previous unit.
-        else:
-            try:
-                prev_section = sections[sections.index(next(s for s in sections if s.location == section.location)) - 1]
-                prev_loc = prev_section.get_children()[-1].get_children()[-1].location
-            except IndexError:
-                pass
+        subsections = get_subsections_in_section()
+        if subsections:
+            prev_loc = get_subsection_location(subsections, subsection, 'previous')
+
     if not next_loc:
-        try:
-            sections = section.get_parent().get_children()
-        except AttributeError:
-            log.error("URL Retrieval Error # 2: subsection {subsection} included in section {section}".format(
-                section=section.location,
-                subsection=subsection.location
-            ))
-        else:
-            try:
-                next_section = sections[sections.index(next(s for s in sections if s.location == section.location)) + 1]
-                next_loc = next_section.get_children()[0].get_children()[0].location
-            except IndexError:
-                pass
+        subsections = get_subsections_in_section()
+        if subsections:
+            next_loc = get_subsection_location(subsections, subsection, 'next')
+
+    if not prev_loc:
+        sections = get_sections_in_course()
+        if sections:
+            prev_loc = get_section_location(sections, section, 'previous')
+
+    if not next_loc:
+        sections = get_sections_in_course()
+        if sections:
+            next_loc = get_section_location(sections, section, 'next')
+
     if prev_loc:
         prev_url = reverse_usage_url('container_handler', prev_loc)
     if next_loc:

cms/djangoapps/contentstore/views/component.py

@@ -161,7 +161,7 @@ def container_handler(request, usage_key_string):
             assert section is not None, "Could not determine ancestor section from unit " + str(unit.location)
 
             # for the sequence navigator
-            prev_url, next_url = get_sibling_urls(subsection)
+            prev_url, next_url = get_sibling_urls(subsection, unit.location)
             # these are quoted here because they'll end up in a query string on the page,
             # and quoting with mako will trigger the xss linter...
             prev_url = quote_plus(prev_url) if prev_url else None

common/lib/xmodule/xmodule/js/src/sequence/display.js

@@ -364,7 +364,7 @@
 
             if ((direction === 'next') && (this.position >= this.contents.length)) {
                 targetUrl = this.nextUrl;
-            } else if ((direction === 'previous') && (this.position === 1)) {
+            } else if ((direction === 'previous')) {
                 targetUrl = this.prevUrl;
             }