From 00840f6955116463b80f0c21005337a5293bced7 Mon Sep 17 00:00:00 2001
From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com>
Date: Wed, 15 Feb 2023 15:09:15 +1300
Subject: [PATCH] FIX Respect existing query strings and anchors in preview
 link (#1039)

---
 src/Models/BaseElement.php                    |  2 +-
 tests/BaseElementTest.php                     | 20 ++++++++++++++++---
 tests/ElementalAreaDataObjectTest.yml         | 11 ++++++++++
 .../Src/TestPreviewableDataObjectWithLink.php | 10 +++++++++-
 4 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/src/Models/BaseElement.php b/src/Models/BaseElement.php
index 586c34bf..191d95d7 100644
--- a/src/Models/BaseElement.php
+++ b/src/Models/BaseElement.php
@@ -779,7 +779,7 @@ public function PreviewLink($action = null)
             if ($link) {
                 // The ElementalPreview getvar is used in ElementalPageExtension
                 // The anchor must be at the end of the URL to function correctly
-                $link .= '?ElementalPreview=' . mt_rand() . '#' . $this->getAnchor();
+                $link = Controller::join_links($link, '?ElementalPreview=' . mt_rand() . '#' . $this->getAnchor());
             }
         }
 
diff --git a/tests/BaseElementTest.php b/tests/BaseElementTest.php
index b29adaf6..f0f58fd9 100644
--- a/tests/BaseElementTest.php
+++ b/tests/BaseElementTest.php
@@ -14,6 +14,7 @@
 use DNADesign\Elemental\Tests\Src\TestDataObjectWithCMSEditLink;
 use DNADesign\Elemental\Tests\Src\TestMultipleHtmlFieldsElement;
 use DNADesign\Elemental\Tests\Src\TestPage;
+use DNADesign\Elemental\Tests\Src\TestPreviewableDataObjectWithLink;
 use Page;
 use ReflectionClass;
 use SilverStripe\Control\Director;
@@ -48,6 +49,7 @@ class BaseElementTest extends FunctionalTest
         TestDataObjectWithCMSEditLink::class,
         TestElementDataObject::class,
         TestMultipleHtmlFieldsElement::class,
+        TestPreviewableDataObjectWithLink::class,
     ];
 
     public function testSimpleClassName()
@@ -463,6 +465,12 @@ public function previewLinksProvider()
                 'elementDataObject4',
                 'base-link',
             ],
+            // Element in DataObject WITH PreviewLink AND Link which has its own query string and anchor link
+            [
+                TestElement::class,
+                'elementDataObject5',
+                'base-link?something=value&somethingelse=value2',
+            ],
         ];
     }
 
@@ -473,12 +481,18 @@ public function testPreviewLink(string $class, string $elementIdentifier, ?strin
     {
         /** @var BaseElement $element */
         $element = $this->objFromFixture($class, $elementIdentifier);
+        $previewLink = $element->PreviewLink();
 
         if ($link) {
-            $regex = '/^' . preg_quote($link . '?ElementalPreview=', '/') .'\d*#' . $element->getAnchor() . '$/';
-            $this->assertTrue((bool)preg_match($regex, $element->PreviewLink()));
+            $regex = '/^' . preg_quote($link, '/') . '[?&]' . preg_quote('ElementalPreview=', '/')
+                .'\d*#' . $element->getAnchor() . '$/';
+            $this->assertMatchesRegularExpression($regex, $previewLink);
+            // Doesn't try to blindly append query string and anchor - but instead merges intelligently with
+            // whatever's already there
+            $this->assertSame(1, substr_count($previewLink, '?'));
+            $this->assertSame(1, substr_count($previewLink, '#'));
         } else {
-            $this->assertSame($link, $element->PreviewLink());
+            $this->assertSame($link, $previewLink);
         }
     }
 }
diff --git a/tests/ElementalAreaDataObjectTest.yml b/tests/ElementalAreaDataObjectTest.yml
index cfe3799b..43b34f57 100644
--- a/tests/ElementalAreaDataObjectTest.yml
+++ b/tests/ElementalAreaDataObjectTest.yml
@@ -11,6 +11,9 @@ DNADesign\Elemental\Models\ElementalArea:
   areaDataObject4:
     Title: Area 4
     OwnerClassName: DNADesign\Elemental\Tests\Src\TestPreviewableDataObjectWithLink
+  areaDataObject5:
+    Title: Area 5
+    OwnerClassName: DNADesign\Elemental\Tests\Src\TestPreviewableDataObjectWithLink
 
 DNADesign\Elemental\Tests\Src\TestDataObjectWithCMSEditLink:
   dataObject1:
@@ -31,6 +34,10 @@ DNADesign\Elemental\Tests\Src\TestPreviewableDataObjectWithLink:
   dataObject4:
     Title: DataObject with PreviewLink and Link methods
     ElementalAreaID: =>DNADesign\Elemental\Models\ElementalArea.areaDataObject4
+  dataObject5:
+    Title: DataObject with PreviewLink with querystring and anchor
+    LinkData: 'base-link?something=value&somethingelse=value2#some-anchor'
+    ElementalAreaID: =>DNADesign\Elemental\Models\ElementalArea.areaDataObject5
 
 DNADesign\Elemental\Tests\Src\TestElement:
   elementDataObject1:
@@ -49,6 +56,10 @@ DNADesign\Elemental\Tests\Src\TestElement:
     Title: Element 4
     TestValue: 'Hello Test'
     ParentID: =>DNADesign\Elemental\Models\ElementalArea.areaDataObject4
+  elementDataObject5:
+    Title: Element 5
+    TestValue: 'Hello Test'
+    ParentID: =>DNADesign\Elemental\Models\ElementalArea.areaDataObject5
 
 DNADesign\Elemental\Tests\Src\TestElementDataObject:
   testElementDataObject1:
diff --git a/tests/Src/TestPreviewableDataObjectWithLink.php b/tests/Src/TestPreviewableDataObjectWithLink.php
index c60e2244..962afe1d 100644
--- a/tests/Src/TestPreviewableDataObjectWithLink.php
+++ b/tests/Src/TestPreviewableDataObjectWithLink.php
@@ -8,8 +8,16 @@ class TestPreviewableDataObjectWithLink extends TestPreviewableDataObject implem
 {
     private static $table_name = 'TestPreviewableDataObjectWithLink';
 
+    private static $db = [
+        'LinkData' => 'Varchar(255)',
+    ];
+
+    private static $defaults = [
+        'LinkData' => 'base-link',
+    ];
+
     public function Link($action = null)
     {
-        return 'base-link';
+        return $this->LinkData;
     }
 }