From 68f6eaa66388957c59a04c65de06d9204c0eb221 Mon Sep 17 00:00:00 2001
From: Dylan Wagstaff <dylan@silverstripe.com>
Date: Tue, 16 Jul 2019 15:16:09 +1200
Subject: [PATCH] FIX display recognisable names for dependent content with no
 title

Content blocks and other DataObjects that contain HTMLText fields are
also used in the 'Dependent pages' report for each page in the CMS,
however these objects may have neither a Title field, nor a name to
describe them. Instead of showing an empty field in the Title column for
this table, we can instead show "Untitled " with the localised singular
name appended.
---
 code/Model/SiteTree.php                       | 18 +++++++--
 tests/php/Model/SiteTreeBacklinksTest.php     | 37 +++++++++++++++++++
 tests/php/Model/SiteTreeBacklinksTest.yml     |  3 +-
 .../SiteTreeBacklinksTestContentObject.php    | 18 +++++++++
 4 files changed, 70 insertions(+), 6 deletions(-)
 create mode 100644 tests/php/Model/SiteTreeBacklinksTestContentObject.php

diff --git a/code/Model/SiteTree.php b/code/Model/SiteTree.php
index f0c768cf..b11f0181 100755
--- a/code/Model/SiteTree.php
+++ b/code/Model/SiteTree.php
@@ -1559,7 +1559,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
                 $this->URLSegment = "page-$this->ID";
             }
         }
-        
+
         // need to set the default values of a page e.g."Untitled [Page type]"
         if (empty($this->Title)) {
             $this->Title = _t(
@@ -1978,10 +1978,20 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
                 ->setDisplayFields($dependentColumns)
                 ->setFieldFormatting(array(
                     'Title' => function ($value, &$item) {
+                        $title = $item->Title;
+                        $untitled = _t(
+                            __CLASS__ . '.UntitledDependentObject',
+                            'Untitled {instanceType}',
+                            ['instanceType' => $item->i18n_singular_name()]
+                        );
+                        $tag = $item->hasMethod('CMSEditLink') ? 'a' : 'span';
                         return sprintf(
-                            '<a href="admin/pages/edit/show/%d">%s</a>',
-                            (int)$item->ID,
-                            Convert::raw2xml($item->Title)
+                            '<%s%s class="dependent-content__edit-link %s">%s</%s>',
+                            $tag,
+                            $tag === 'a' ? sprintf(' href="%s"', $item->CMSEditLink()) : '',
+                            $title ? '' : 'dependent-content__edit-link--untitled',
+                            $title ? Convert::raw2xml($title) : $untitled,
+                            $tag
                         );
                     }
                 ));
diff --git a/tests/php/Model/SiteTreeBacklinksTest.php b/tests/php/Model/SiteTreeBacklinksTest.php
index 821c50cf..74bb1c75 100644
--- a/tests/php/Model/SiteTreeBacklinksTest.php
+++ b/tests/php/Model/SiteTreeBacklinksTest.php
@@ -6,6 +6,7 @@ use SilverStripe\CMS\Model\SiteTree;
 use SilverStripe\Control\Director;
 use SilverStripe\Control\HTTP;
 use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Forms\GridField\GridField;
 use SilverStripe\Versioned\Versioned;
 
 /**
@@ -21,6 +22,10 @@ class SiteTreeBacklinksTest extends SapphireTest
         ],
     ];
 
+    protected static $extra_dataobjects = [
+        SiteTreeBacklinksTestContentObject::class,
+    ];
+
     public function setUp()
     {
         parent::setUp();
@@ -28,6 +33,14 @@ class SiteTreeBacklinksTest extends SapphireTest
         // Log in as admin so that we don't run into permission issues.  That's not what we're
         // testing here.
         $this->logInWithPermission('ADMIN');
+
+        $page3 = $this->objFromFixture('Page', 'page3');
+        $page3->Content = str_replace(
+            '$page1.ID',
+            $this->objFromFixture('Page', 'page1')->ID,
+            $page3->Content
+        );
+        $page3->write();
     }
 
     public function testSavingPageWithLinkAddsBacklink()
@@ -251,4 +264,28 @@ class SiteTreeBacklinksTest extends SapphireTest
         // assert backlink to page 2 no longer exists
         $this->assertNotContains($page2->ID, $page1->BackLinkTracking()->column('ID'), 'Assert backlink to page 2 has been removed');
     }
+
+    public function testLinkTrackingWithUntitledObjectsDisplaysAReadableIdentifier()
+    {
+        $page = $this->objFromFixture('Page', 'page2');
+
+        $referencingObject = new SiteTreeBacklinksTestContentObject();
+        $referencingObject->Content = '<p><a href="[sitetree_link,id='
+            . $page->ID
+            . ']">Page 2</a></p>';
+        // Title purposely not set - this is the coverage case for this test
+        $referencingObject->write();
+
+        /** @var GridField $gridField */
+        $gridField = $page->getCMSFields()->dataFieldByName('DependentPages');
+
+        // Prepare and sanity check
+        $this->assertNotNull($gridField);
+        $list = $gridField->getList();
+        $this->assertEquals(1, $list->count());
+
+        $content = $gridField->getColumnContent($list->first(), 'Title');
+
+        $this->assertContains('Untitled Backlink test content object', $content);
+    }
 }
diff --git a/tests/php/Model/SiteTreeBacklinksTest.yml b/tests/php/Model/SiteTreeBacklinksTest.yml
index 4a63c73f..56149a44 100644
--- a/tests/php/Model/SiteTreeBacklinksTest.yml
+++ b/tests/php/Model/SiteTreeBacklinksTest.yml
@@ -1,6 +1,5 @@
 Page:
   page1:
-    ID: 1
     Title: page1
     URLSegment: page1
 
@@ -11,7 +10,7 @@ Page:
   page3:
     Title: page3
     URLSegment: page3
-    Content: '<p><a href="[sitetree_link,id=1]">Testing page 1 link</a></p>'
+    Content: '<p><a href="[sitetree_link,id=$page1.ID]">Testing page 1 link</a></p>'
     LinkTracking: =>Page.page1
 
 
diff --git a/tests/php/Model/SiteTreeBacklinksTestContentObject.php b/tests/php/Model/SiteTreeBacklinksTestContentObject.php
new file mode 100644
index 00000000..9c92316e
--- /dev/null
+++ b/tests/php/Model/SiteTreeBacklinksTestContentObject.php
@@ -0,0 +1,18 @@
+<?php
+
+namespace SilverStripe\CMS\Tests\Model;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+
+class SiteTreeBacklinksTestContentObject extends DataObject implements TestOnly
+{
+    private static $table_name = 'BacklinkContent';
+
+    private static $db = array(
+        'Title' => 'Text',
+        'Content' => 'HTMLText',
+    );
+
+    private static $singular_name = 'Backlink test content object';
+}