From c7cbbb29f4a06bc4edf1cee0cad3c4953c11d2b4 Mon Sep 17 00:00:00 2001
From: Andrew O'Neil <andy@andyofniall.net>
Date: Wed, 27 Sep 2017 15:41:08 +1000
Subject: [PATCH] Fix links on paginated lists when there are GET vars

Prior to this change, if there were already GET vars on a page
with a PaginatedList, the links would include a mix of '&' and '&amp;'.
---
 src/ORM/PaginatedList.php           | 28 ++++++++++++---
 tests/php/ORM/PaginatedListTest.php | 53 +++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 4 deletions(-)

diff --git a/src/ORM/PaginatedList.php b/src/ORM/PaginatedList.php
index 4763ec390..646075c94 100644
--- a/src/ORM/PaginatedList.php
+++ b/src/ORM/PaginatedList.php
@@ -436,7 +436,12 @@ class PaginatedList extends ListDecorator
      */
     public function FirstLink()
     {
-        return HTTP::setGetVar($this->getPaginationGetVar(), 0);
+        return HTTP::setGetVar(
+            $this->getPaginationGetVar(),
+            0,
+            $this->request ? $this->request->getURL(true) : null,
+            '&'
+        );
     }
 
     /**
@@ -446,7 +451,12 @@ class PaginatedList extends ListDecorator
      */
     public function LastLink()
     {
-        return HTTP::setGetVar($this->getPaginationGetVar(), ($this->TotalPages() - 1) * $this->getPageLength());
+        return HTTP::setGetVar(
+            $this->getPaginationGetVar(),
+            ($this->TotalPages() - 1) * $this->getPageLength(),
+            $this->request ? $this->request->getURL(true) : null,
+            '&'
+        );
     }
 
     /**
@@ -458,7 +468,12 @@ class PaginatedList extends ListDecorator
     public function NextLink()
     {
         if ($this->NotLastPage()) {
-            return HTTP::setGetVar($this->getPaginationGetVar(), $this->getPageStart() + $this->getPageLength());
+            return HTTP::setGetVar(
+                $this->getPaginationGetVar(),
+                $this->getPageStart() + $this->getPageLength(),
+                $this->request ? $this->request->getURL(true) : null,
+                '&'
+            );
         }
     }
 
@@ -471,7 +486,12 @@ class PaginatedList extends ListDecorator
     public function PrevLink()
     {
         if ($this->NotFirstPage()) {
-            return HTTP::setGetVar($this->getPaginationGetVar(), $this->getPageStart() - $this->getPageLength());
+            return HTTP::setGetVar(
+                $this->getPaginationGetVar(),
+                $this->getPageStart() - $this->getPageLength(),
+                $this->request ? $this->request->getURL(true) : null,
+                '&'
+            );
         }
     }
 
diff --git a/tests/php/ORM/PaginatedListTest.php b/tests/php/ORM/PaginatedListTest.php
index fa222cb77..09919416c 100644
--- a/tests/php/ORM/PaginatedListTest.php
+++ b/tests/php/ORM/PaginatedListTest.php
@@ -9,6 +9,7 @@ use SilverStripe\Dev\SapphireTest;
 use SilverStripe\ORM\Queries\SQLSelect;
 use SilverStripe\ORM\Tests\DataObjectTest\Player;
 use SilverStripe\View\ArrayData;
+use SilverStripe\Control\HTTPRequest;
 
 /**
  * Tests for the {@link SilverStripe\ORM\PaginatedList} class.
@@ -330,6 +331,19 @@ class PaginatedListTest extends SapphireTest
         $this->assertContains('start=0', $list->FirstLink());
     }
 
+    public function testFirstLinkContainsCurrentGetParameters() {
+        $request = new HTTPRequest(
+            'GET',
+            'http://coolsite.com/my-cool-page',
+            ['awesomeness' => 'nextLevel', 'start' => 20]
+        );
+        $list = new PaginatedList(new ArrayList(), $request);
+        $list->setTotalItems(50);
+        $list->setPageLength(10);
+
+        $this->assertEquals($list->FirstLink(), 'http://coolsite.com/my-cool-page?awesomeness=nextLevel&start=0');
+    }
+
     public function testLastLink()
     {
         $list = new PaginatedList(new ArrayList());
@@ -342,6 +356,19 @@ class PaginatedListTest extends SapphireTest
         $this->assertContains('start=0', $list->LastLink());
     }
 
+    public function testLastLinkContainsCurrentGetParameters() {
+        $request = new HTTPRequest(
+            'GET',
+            'http://coolsite.com/my-cool-page',
+            ['awesomeness' => 'nextLevel']
+        );
+        $list = new PaginatedList(new ArrayList(), $request);
+        $list->setTotalItems(50);
+        $list->setPageLength(10);
+
+        $this->assertEquals($list->LastLink(), 'http://coolsite.com/my-cool-page?awesomeness=nextLevel&start=40');
+    }
+
     public function testNextLink()
     {
         $list = new PaginatedList(new ArrayList());
@@ -363,6 +390,19 @@ class PaginatedListTest extends SapphireTest
         $this->assertNull($list->NextLink());
     }
 
+    public function testNextLinkContainsCurrentGetParameters() {
+        $request = new HTTPRequest(
+            'GET',
+            'http://coolsite.com/my-cool-page',
+            ['awesomeness' => 'nextLevel']
+        );
+        $list = new PaginatedList(new ArrayList(), $request);
+        $list->setTotalItems(50);
+        $list->setPageLength(10);
+
+        $this->assertEquals($list->NextLink(), 'http://coolsite.com/my-cool-page?awesomeness=nextLevel&start=10');
+    }
+
     public function testPrevLink()
     {
         $list = new PaginatedList(new ArrayList());
@@ -380,4 +420,17 @@ class PaginatedListTest extends SapphireTest
         $list->setPageLength(0);
         $this->assertNull($list->PrevLink());
     }
+
+    public function testPrevLinkContainsCurrentGetParameters() {
+        $request = new HTTPRequest(
+            'GET',
+            'http://coolsite.com/my-cool-page',
+            ['awesomeness' => 'nextLevel', 'start' => '30']
+        );
+        $list = new PaginatedList(new ArrayList(), $request);
+        $list->setTotalItems(50);
+        $list->setPageLength(10);
+
+        $this->assertEquals($list->PrevLink(), 'http://coolsite.com/my-cool-page?awesomeness=nextLevel&start=20');
+    }
 }