Merge branch '5.2' into 5

code/Controllers/CMSMain.php

@@ -1038,49 +1038,33 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
 
     public function Breadcrumbs($unlinked = false)
     {
-        $items = new ArrayList();
+        $items = ArrayList::create();
 
-        if ($this->TreeIsFiltered()) {
+        if (($this->getAction() !== 'index') && ($record = $this->currentPage())) {
-            $items->push(new ArrayData([
+            // The page is being edited
-                'Title' => CMSPagesController::menu_title(),
+            $this->buildEditFormBreadcrumb($items, $record, $unlinked);
-                'Link' => ($unlinked) ? false : $this->LinkPages()
+        } else {
-            ]));
+            // Ensure we always have the "Pages" crumb first
-            $items->push(new ArrayData([
+            $this->pushCrumb(
-                'Title' => _t('SilverStripe\\CMS\\Controllers\\CMSMain.SEARCHRESULTS', 'Search results'),
+                $items,
-                'Link' => ($unlinked) ? false : $this->LinkPages()
+                CMSPagesController::menu_title(),
-            ]));
+                $unlinked ? false : $this->LinkPages()
+            );
 
-            $this->extend('updateBreadcrumbs', $items);
+            if ($this->TreeIsFiltered()) {
-
+                // Showing search results
-            return $items;
+                $this->pushCrumb(
-        }
+                    $items,
-
+                    _t(CMSMain::class . '.SEARCHRESULTS', 'Search results'),
-        // Check if we are editing a page
+                    ($unlinked) ? false : $this->LinkPages()
-        /** @var SiteTree $record */
+                );
-        $record = $this->currentPage();
+            } elseif ($parentID = $this->getRequest()->getVar('ParentID')) {
-        if (!$record) {
+                // We're navigating the listview. ParentID is the page whose
-            $items->push(new ArrayData([
+                // children are currently displayed.
-                'Title' => CMSPagesController::menu_title(),
+                if ($page = SiteTree::get()->byID($parentID)) {
-                'Link' => ($unlinked) ? false : $this->LinkPages()
+                    $this->buildListViewBreadcrumb($items, $page);
-            ]));
+                }
-
+            }
-            $this->extend('updateBreadcrumbs', $items);
-
-            return $items;
-        }
-
-        // Add all ancestors
-        $ancestors = $record->getAncestors();
-        $ancestors = new ArrayList(array_reverse($ancestors->toArray() ?? []));
-        $ancestors->push($record);
-        /** @var SiteTree $ancestor */
-        foreach ($ancestors as $ancestor) {
-            $items->push(new ArrayData([
-                'Title' => $ancestor->getMenuTitle(),
-                'Link' => ($unlinked)
-                    ? false
-                    : $ancestor->CMSEditLink()
-            ]));
         }
 
         $this->extend('updateBreadcrumbs', $items);
@@ -1088,6 +1072,61 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
         return $items;
     }
 
+    /**
+     * Push the provided an extra breadcrumb crumb at the end of the provided List
+     */
+    private function pushCrumb(ArrayList $items, string $title, string|false $link): void
+    {
+        $items->push(ArrayData::create([
+            'Title' => $title,
+            'Link' => $link
+        ]));
+    }
+
+    /**
+     * Build Breadcrumb for the Edit page form. Each crumb links back to its own edit form.
+     */
+    private function buildEditFormBreadcrumb(ArrayList $items, SiteTree $page, bool $unlinked): void
+    {
+        // Find all ancestors of the provided page
+        $ancestors = $page->getAncestors(true);
+        $ancestors = array_reverse($ancestors->toArray() ?? []);
+        foreach ($ancestors as $ancestor) {
+            // Link to the ancestor's edit form
+            $this->pushCrumb(
+                $items,
+                $ancestor->getMenuTitle(),
+                $unlinked ? false : $ancestor->CMSEditLink()
+            );
+        }
+    }
+
+    /**
+     * Build Breadcrumb for the List view. Each crumb links to the list view for that page.
+     */
+    private function buildListViewBreadcrumb(ArrayList $items, SiteTree $page): void
+    {
+        // Find all ancestors of the provided page
+        $ancestors = $page->getAncestors(true);
+        $ancestors = array_reverse($ancestors->toArray() ?? []);
+        
+        //turns the title and link of the breadcrumbs into template-friendly variables
+        $params = array_filter([
+            'view' => $this->getRequest()->getVar('view'),
+            'q' => $this->getRequest()->getVar('q')
+        ]);
+        
+        foreach ($ancestors as $ancestor) {
+            // Link back to the list view for the current ancestor
+            $params['ParentID'] = $ancestor->ID;
+            $this->pushCrumb(
+                $items,
+                $ancestor->getMenuTitle(),
+                Controller::join_links($this->Link(), '?' . http_build_query($params ?? []))
+            );
+        }
+    }
+
     /**
      * Create serialized JSON string with site tree hints data to be injected into
      * 'data-hints' attribute of root node of jsTree.

code/Controllers/CMSPagesController.php

@@ -31,35 +31,4 @@ class CMSPagesController extends CMSMain
     {
         return false;
     }
-
-    public function Breadcrumbs($unlinked = false)
-    {
-        $this->beforeExtending('updateBreadcrumbs', function (ArrayList $items) {
-            //special case for building the breadcrumbs when calling the listchildren Pages ListView action
-            if ($parentID = $this->getRequest()->getVar('ParentID')) {
-                $page = SiteTree::get()->byID($parentID);
-
-                //build a reversed list of the parent tree
-                $pages = [];
-                while ($page) {
-                    array_unshift($pages, $page); //add to start of array so that array is in reverse order
-                    $page = $page->Parent;
-                }
-
-                //turns the title and link of the breadcrumbs into template-friendly variables
-                $params = array_filter([
-                    'view' => $this->getRequest()->getVar('view'),
-                    'q' => $this->getRequest()->getVar('q')
-                ]);
-                foreach ($pages as $page) {
-                    $params['ParentID'] = $page->ID;
-                    $item = new stdClass();
-                    $item->Title = $page->Title;
-                    $item->Link = Controller::join_links($this->Link(), '?' . http_build_query($params ?? []));
-                    $items->push(new ArrayData($item));
-                }
-            }
-        });
-        return parent::Breadcrumbs($unlinked);
-    }
 }

tests/php/Controllers/CMSMainTest.php

@@ -357,20 +357,99 @@ class CMSMainTest extends FunctionalTest
 
     public function testBreadcrumbs()
     {
-        $page3 = $this->objFromFixture(SiteTree::class, 'page3');
         $page31 = $this->objFromFixture(SiteTree::class, 'page31');
         $this->logInAs('admin');
 
         $response = $this->get('admin/pages/edit/show/' . $page31->ID);
         $parser = new CSSContentParser($response->getBody());
+        $this->assertCrumbs(
+            ['Page 3', 'Page 3.1'],
+            $response,
+            'Edit breadcrumb includes all pages up to the one being edited without a tob level Page'
+        );
+    }
+
+    public function testBreadcrumbsListView()
+    {
+        $page311 = $this->objFromFixture(SiteTree::class, 'page311');
+        $this->logInAs('admin');
+
+        $response = $this->get('admin/pages?ParentID=' . $page311->ID);
+        $this->assertCrumbs(
+            ['Pages', 'Page 3', 'Page 3.1', 'Page 3.1.1'],
+            $response,
+            'List view breadcrumb includes all pages and a Page link back to the root level'
+        );
+    }
+
+    public function testBreadcrumbsListViewTopLevel()
+    {
+        $page311 = $this->objFromFixture(SiteTree::class, 'page311');
+        $this->logInAs('admin');
+
+        $response = $this->get('admin/pages');
+        $this->assertCrumbs(
+            ['Pages'],
+            $response,
+            'Top level of list view includes only a Page crumb'
+        );
+    }
+
+    public function testBreadcrumbsListViewWithPjax()
+    {
+        $page311 = $this->objFromFixture(SiteTree::class, 'page311');
+        $this->logInAs('admin');
+
+        $response = $this->get('admin/pages?ParentID=' . $page311->ID);
+        $this->assertCrumbs(
+            ['Pages', 'Page 3', 'Page 3.1', 'Page 3.1.1'],
+            $response,
+            'List view breadcrumb includes all pages and a Page link back to the root level'
+        );
+    }
+
+    public function testBreadcrumbsSearchView()
+    {
+        $page311 = $this->objFromFixture(SiteTree::class, 'page311');
+        $this->logInAs('admin');
+
+        $response = $this->get(
+            'admin/pages?ParentID=' . $page311->ID,
+            null,
+            [
+                'X-Pjax' => 'ListViewForm,Breadcrumbs',
+                'X-Requested-With' => 'XMLHttpRequest'
+            ]
+        );
+        $jsonStr = $response->getBody();
+        $data = json_decode($jsonStr, true);
+        
+        $parser = new CSSContentParser($data['Breadcrumbs']);
         $crumbs = $parser->getBySelector('.breadcrumbs-wrapper .crumb');
 
-        $this->assertNotNull($crumbs);
+        $crumbs = array_map(function ($crumb) {
-        $this->assertEquals(2, count($crumbs ?? []));
+            return (string)$crumb;
-        $this->assertEquals('Page 3', (string)$crumbs[0]);
+        }, $crumbs);
-        $this->assertEquals('Page 3.1', (string)$crumbs[1]);
 
-        Security::setCurrentUser(null);
+        $this->assertNotNull($crumbs, 'Should have found some crumbs');
+        $this->assertEquals(
+            ['Pages', 'Page 3', 'Page 3.1', 'Page 3.1.1'],
+            $crumbs,
+            'List view breadcrumb includes all pages and a Page link back to the root level when access wia PJAX'
+        );
+    }
+
+    private function assertCrumbs(array $expectedCrumbs, $response, string $message): void
+    {
+        $parser = new CSSContentParser($response->getBody());
+        $crumbs = $parser->getBySelector('.breadcrumbs-wrapper .crumb');
+
+        $crumbs = array_map(function ($crumb) {
+            return (string)$crumb;
+        }, $crumbs);
+
+        $this->assertNotNull($crumbs, $message);
+        $this->assertEquals($expectedCrumbs, $crumbs, $message);
     }
 
     public function testGetNewItem()

tests/php/Controllers/CMSMainTest.yml

@@ -12,6 +12,10 @@ SilverStripe\CMS\Model\SiteTree:
     Title: Page 3.1
     Parent: =>SilverStripe\CMS\Model\SiteTree.page3
     Sort: 1
+  page311:
+    Title: Page 3.1.1
+    Parent: =>SilverStripe\CMS\Model\SiteTree.page31
+    Sort: 1
   page32:
     Title: Page 3.2
     Parent: =>SilverStripe\CMS\Model\SiteTree.page3