Gracefully handle document links with missing ID attributes in rich text

Fixes #4791
Previously, our rich text conversion functions handled the case where a document link specified an ID which is not found in the database. However, they failed with a KeyError when the id attribute was missing completely; links of this second type would occur whenever a link of the first type was re-saved from the Draftail editor. The fix is two-fold:

1) Catch the "missing ID attribute" case - in this case, the resulting link will be missing both the href and id attributes
2) Update the handling of the "ID present but document not found" case so that the id attribute survives the round-trip to the editor and back. The final link as rendered on the front-end will still be an attribute-less <a> element, but the id will be retained in the database (and in the versions rendered within rich text editors) which may be useful for troubleshooting.

CHANGELOG.txt

@@ -22,7 +22,7 @@ Changelog
  * Fix: Disabled autocomplete dropdowns on date/time chooser fields (Janneke Janssen)
  * Fix: Split up `wagtail.admin.forms` to make it less prone to circular imports (Matt Westcott)
  * Fix: Disable linking to root page in rich text, making the page non-functional (Matt Westcott)
- * Fix: Pages should be editable and save-able even if there are broken links in rich text (Matt Westcott)
+ * Fix: Pages should be editable and save-able even if there are broken page or document links in rich text (Matt Westcott)
 
 
 2.2.2 (29.08.2018)

docs/releases/2.3.rst

@@ -45,7 +45,7 @@ Bug fixes
  * Disabled autocomplete dropdowns on date/time chooser fields (Janneke Janssen)
  * Split up ``wagtail.admin.forms`` to make it less prone to circular imports (Matt Westcott)
  * Disable linking to root page in rich text, making the page non-functional (Matt Westcott)
- * Pages should be editable and save-able even if there are broken links in rich text (Matt Westcott)
+ * Pages should be editable and save-able even if there are broken page or document links in rich text (Matt Westcott)
 
 Upgrade considerations
 ======================

wagtail/admin/tests/test_contentstate.py

@@ -390,6 +390,28 @@ class TestHtmlToContentState(TestCase):
             <p>a <a linktype="document" id="9999">document</a> link</p>
             '''
         ))
+        self.assertContentStateEqual(result, {
+            'entityMap': {
+                '0': {
+                    'mutability': 'MUTABLE', 'type': 'DOCUMENT',
+                    'data': {'id': 9999}
+                }
+            },
+            'blocks': [
+                {
+                    'inlineStyleRanges': [], 'text': 'a document link', 'depth': 0, 'type': 'unstyled', 'key': '00000',
+                    'entityRanges': [{'offset': 2, 'length': 8, 'key': 0}]
+                },
+            ]
+        })
+
+    def test_document_link_with_missing_id(self):
+        converter = ContentstateConverter(features=['document-link'])
+        result = json.loads(converter.from_database_format(
+            '''
+            <p>a <a linktype="document">document</a> link</p>
+            '''
+        ))
         self.assertContentStateEqual(result, {
             'entityMap': {
                 '0': {

wagtail/documents/rich_text.py

@@ -13,7 +13,7 @@ def document_linktype_handler(attrs):
     try:
         doc = Document.objects.get(id=attrs['id'])
         return '<a href="%s">' % escape(doc.url)
-    except Document.DoesNotExist:
+    except (Document.DoesNotExist, KeyError):
         return "<a>"
 
 
@@ -31,7 +31,11 @@ class DocumentLinkHandler:
             doc = Document.objects.get(id=attrs['id'])
             return '<a data-linktype="document" data-id="%d" href="%s">' % (doc.id, escape(doc.url))
         except Document.DoesNotExist:
-            return "<a>"
+            # Preserve the ID attribute for troubleshooting purposes, even though it
+            # points to a missing document
+            return '<a data-linktype="document" data-id="%s">' % attrs['id']
+        except KeyError:
+            return '<a data-linktype="document">'
 
 
 EditorHTMLDocumentLinkConversionRule = [
@@ -62,10 +66,15 @@ class DocumentLinkElementHandler(LinkElementHandler):
     def get_attribute_data(self, attrs):
         Document = get_document_model()
         try:
-            doc = Document.objects.get(id=attrs['id'])
-        except Document.DoesNotExist:
+            id = int(attrs['id'])
+        except (KeyError, ValueError):
             return {}
 
+        try:
+            doc = Document.objects.get(id=id)
+        except Document.DoesNotExist:
+            return {'id': id}
+
         return {
             'id': doc.id,
             'url': doc.url,

wagtail/documents/tests/test_rich_text.py

@@ -14,16 +14,29 @@ class TestDocumentRichTextLinkHandler(TestCase):
         self.assertEqual(result,
                          {'id': 'test-id'})
 
+    def test_expand_db_attributes(self):
+        result = document_linktype_handler({'id': 1})
+        self.assertEqual(result,
+                         '<a href="/documents/1/test.pdf">')
+
     def test_expand_db_attributes_document_does_not_exist(self):
         result = document_linktype_handler({'id': 0})
         self.assertEqual(result, '<a>')
 
+    def test_expand_db_attributes_with_missing_id(self):
+        result = document_linktype_handler({})
+        self.assertEqual(result, '<a>')
+
     def test_expand_db_attributes_for_editor(self):
         result = DocumentLinkHandler.expand_db_attributes({'id': 1})
         self.assertEqual(result,
                          '<a data-linktype="document" data-id="1" href="/documents/1/test.pdf">')
 
-    def test_expand_db_attributes_not_for_editor(self):
-        result = document_linktype_handler({'id': 1})
+    def test_expand_db_attributes_for_editor_preserves_id_of_nonexistent_document(self):
+        result = DocumentLinkHandler.expand_db_attributes({'id': 0})
         self.assertEqual(result,
-                         '<a href="/documents/1/test.pdf">')
+                         '<a data-linktype="document" data-id="0">')
+
+    def test_expand_db_attributes_for_editor_with_missing_id(self):
+        result = DocumentLinkHandler.expand_db_attributes({})
+        self.assertEqual(result, '<a data-linktype="document">')