Fixed POST bookmarks bug when display_name is None.

Added tests & general message to js when parsing fails.

TNL-3989

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

@@ -119,7 +119,7 @@ class @Sequence
       sequence_links = @content_container.find('a.seqnav')
       sequence_links.click @goto
 
-      @el.find('.path').html(@el.find('.nav-item.active').data('path'))
+      @el.find('.path').text(@el.find('.nav-item.active').data('path'))
 
       @sr_container.focus();
       # @$("a.active").blur()

common/lib/xmodule/xmodule/seq_module.py

@@ -191,7 +191,11 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule):
         bookmarks_service = self.runtime.service(self, "bookmarks")
         context["username"] = self.runtime.service(self, "user").get_current_user().opt_attrs['edx-platform.username']
 
-        display_names = [self.get_parent().display_name or '', self.display_name or '']
+        parent_module = self.get_parent()
+        display_names = [
+            parent_module.display_name_with_default,
+            self.display_name_with_default
+        ]
 
         # We do this up here because proctored exam functionality could bypass
         # rendering after this section.
@@ -228,7 +232,7 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule):
                 'type': child.get_icon_class(),
                 'id': child.scope_ids.usage_id.to_deprecated_string(),
                 'bookmarked': is_bookmarked,
-                'path': " > ".join(display_names + [child.display_name or '']),
+                'path': " > ".join(display_names + [child.display_name_with_default]),
             }
             if childinfo['title'] == '':
                 childinfo['title'] = child.display_name_with_default_escaped

lms/static/js/bookmarks/views/bookmark_button.js

@@ -45,9 +45,14 @@
                         view.setBookmarkState(true);
                     },
                     error: function (jqXHR) {
-                        var response = jqXHR.responseText ? JSON.parse(jqXHR.responseText) : '';
-                        var userMessage = response ? response.user_message : '';
-                        view.showError(userMessage);
+                        try {
+                            var response = jqXHR.responseText ? JSON.parse(jqXHR.responseText) : '';
+                            var userMessage = response ? response.user_message : '';
+                            view.showError(userMessage);
+                        }
+                        catch(err) {
+                            view.showError();
+                        }
                     }
                 });
             },

lms/templates/bookmarks/bookmarks-list.underscore

@@ -10,7 +10,7 @@
              <a class="bookmarks-results-list-item" href="<%= bookmark.blockUrl() %>" aria-labelledby="bookmark-link-<%= index %>" data-bookmark-id="<%= bookmark.get('id') %>" data-component-type="<%= bookmark.get('block_type') %>" data-usage-id="<%= bookmark.get('usage_id') %>" aria-describedby="bookmark-type-<%= index %> bookmark-date-<%= index %>">
                 <div class="list-item-content">
                     <div class="list-item-left-section">
-                        <h3 id="bookmark-link-<%= index %>" class="list-item-breadcrumbtrail"> <%= _.pluck(bookmark.get('path'), 'display_name').concat([bookmark.get('display_name')]).join(' <i class="icon fa fa-caret-right" aria-hidden="true"></i><span class="sr">-</span> ') %> </h3>
+                        <h3 id="bookmark-link-<%= index %>" class="list-item-breadcrumbtrail"> <%= _.map(_.pluck(bookmark.get('path'), 'display_name'), _.escape).concat([_.escape(bookmark.get('display_name'))]).join(' <i class="icon fa fa-caret-right" aria-hidden="true"></i><span class="sr">-</span> ') %> </h3>
                         <p id="bookmark-date-<%= index %>" class="list-item-date"> <%= gettext("Bookmarked on") %> <%= humanFriendlyDate(bookmark.get('created')) %> </p>
                     </div>
 

lms/templates/seq_module.html

@@ -19,7 +19,7 @@
            data-element="${idx+1}"
            href="javascript:void(0);"
            data-page-title="${item['page_title']|h}"
-           data-path="${item['path']}"
+           data-path="${item['path']|h}"
            aria-controls="seq_contents_${idx}"
            id="tab_${idx}"
            tabindex="0">

openedx/core/djangoapps/bookmarks/models.py

@@ -81,7 +81,7 @@ class Bookmark(TimeStampedModel):
 
             xblock_cache = XBlockCache.create({
                 'usage_key': usage_key,
-                'display_name': block.display_name,
+                'display_name': block.display_name_with_default,
             })
             data['_path'] = prepare_path_for_serialization(Bookmark.updated_path(usage_key, xblock_cache))
 
@@ -238,7 +238,6 @@ class XBlockCache(TimeStampedModel):
         usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
 
         data['course_key'] = usage_key.course_key
-
         xblock_cache, created = cls.objects.get_or_create(usage_key=usage_key, defaults=data)
 
         if not created:

openedx/core/djangoapps/bookmarks/tests/test_models.py

@@ -229,6 +229,15 @@ class BookmarkModelTests(BookmarksTestsBase):
     """
     Test the Bookmark model.
     """
+    def setUp(self):
+        super(BookmarkModelTests, self).setUp()
+
+        self.vertical_4 = ItemFactory.create(
+            parent_location=self.sequential_2.location,
+            category='vertical',
+            display_name=None
+        )
+
     def get_bookmark_data(self, block, user=None):
         """
         Returns bookmark data for testing.
@@ -297,6 +306,15 @@ class BookmarkModelTests(BookmarksTestsBase):
         self.assertNotEqual(bookmark, bookmark3)
         self.assert_bookmark_model_is_valid(bookmark3, bookmark_data_different_user)
 
+    def test_create_bookmark_successfully_with_display_name_none(self):
+        """
+        Tests creation of bookmark with display_name None.
+        """
+        bookmark_data = self.get_bookmark_data(self.vertical_4)
+        bookmark, __ = Bookmark.create(bookmark_data)
+        bookmark_data['display_name'] = self.vertical_4.display_name_with_default
+        self.assert_bookmark_model_is_valid(bookmark, bookmark_data)
+
     @ddt.data(
         (-30, [[PathItem(EXAMPLE_USAGE_KEY_1, '1')]], 1),
         (30, None, 2),