Fixes a couple segment handling regressions in the UI (#15110)

* When trying to see whether we should decode a segment, favor encoded segment since it appears to be more common.

* Replace existing param in URL if it exists but empty (eg, segment=).

* If editing segment during comparison, remove comparison in case edited segment is one being compared.

* Solve the problem of matching segments that end up decoded a different way to avoid side effects.

* Revert test file change.

* Add test for getStoredSegmentName().

1 files changed, 42 insertions, 0 deletions

diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index 8b23ce59ac..5cb987fdd6 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -1806,6 +1806,48 @@ log_visit.visit_total_actions
         ];
     }
 
+    /**
+     * @dataProvider getTestDataForGetStoredSegmentName
+     */
+    public function test_getStoredSegmentName($segment, $expectedName)
+    {
+        SegmentEditorApi::getInstance()->add('test segment 1', 'browserCode==ff');
+        SegmentEditorApi::getInstance()->add('test segment 2', urlencode('browserCode==ch'));
+        SegmentEditorApi::getInstance()->add('test segment 3', 'pageUrl=@' . urlencode('/a/b?d=blahfty'));
+        SegmentEditorApi::getInstance()->add('test segment 4', 'pageUrl=@' . urlencode(urlencode('/a/b?d=wafty')));
+        SegmentEditorApi::getInstance()->add('test segment 5', urlencode('pageUrl=@' . urlencode(urlencode('/a/b?d=woo'))));
+
+        $segmentObj = new Segment($segment, [1]);
+        $this->assertEquals($expectedName, $segmentObj->getStoredSegmentName(1));
+    }
+
+    public function getTestDataForGetStoredSegmentName()
+    {
+        return [
+            ['browserCode==ff', 'test segment 1'],
+            [urlencode('browserCode==ff'), 'test segment 1'],
+
+            ['browserCode==ch', 'test segment 2'],
+            [urlencode('browserCode==ch'), 'test segment 2'],
+
+            ['pageUrl=@' . urlencode('/a/b?d=blahfty'), 'test segment 3'],
+            ['pageUrl=@' . urlencode(urlencode('/a/b?d=blahfty')), 'test segment 3'],
+
+            ['pageUrl=@' . urlencode(urlencode('/a/b?d=wafty')), 'test segment 4'],
+            [urlencode('pageUrl=@' . urlencode(urlencode('/a/b?d=wafty'))), 'test segment 4'],
+
+            ['pageUrl=@' . urlencode(urlencode('/a/b?d=woo')), 'test segment 5'],
+            [urlencode('pageUrl=@' . urlencode(urlencode('/a/b?d=woo'))), 'test segment 5'],
+
+            // these test cases won't pass because the value is encoded, but the operator isn't in one of the segments. kept here just
+            // so there's a `record that they won't work
+            // ['pageUrl=@' . urlencode('/a/b?d=wafty'), 'test segment 4'],
+            // [urlencode('pageUrl=@' . urlencode('/a/b?d=wafty')), 'test segment 4'],
+            // [urlencode('pageUrl=@' . urlencode('/a/b?d=woo')), 'test segment 5'],
+            // ['pageUrl=@' . urlencode('/a/b?d=woo'), 'test segment 5'],
+        ];
+    }
+
     private function assertWillBeArchived($segmentString)
     {
         $this->assertTrue($this->willSegmentByArchived($segmentString));