FIX Respect existing query strings and anchors in preview link (silve…

…rstripe#1039)
silverstripeltd · Feb 15, 2023 · 00840f6 · 00840f6
1 parent de83bcb
commit 00840f6
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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;
     }
 }