properly encode segment definitions from table so the hash will be the same as from query params (#17029)

* initial fix

* when creating a Segment or getting a segment hash using a segment definition from the segment table, make sure to urlencode so the hash is the same as when we send the segment as a query parameter

* try to make segment definition change appear in tests (seems like this is just a test fix? if I undo the changes the new test changes still pass)

* add test demonstrating bug

* check access in exampleplugin

* Add missing use

* fix test

* better fix

* fix tests

* fix test

* update expected screenshots

7 files changed, 92 insertions, 6 deletions

diff --git a/plugins/SegmentEditor/API.php b/plugins/SegmentEditor/API.php
index 13f724782b..0565f29cdc 100644
--- a/plugins/SegmentEditor/API.php
+++ b/plugins/SegmentEditor/API.php
@@ -390,6 +390,14 @@ class API extends \Piwik\Plugin\API
 
         $segments = $this->sortSegmentsCreatedByUserFirst($segments);
 
+        $model = new \Piwik\Plugins\SitesManager\Model();
+        $allIdSites = $model->getSitesId();
+        foreach ($segments as &$segmentInfo) {
+            $idSites = !empty($segmentInfo['enable_only_idsite']) ? [(int) $segmentInfo['enable_only_idsite']] : $allIdSites;
+            $segmentObj = new Segment(urlencode($segmentInfo['definition']), $idSites);
+            $segmentInfo['hash'] = $segmentObj->getHash();
+        }
+
         return $segments;
     }
 
diff --git a/plugins/SegmentEditor/SegmentEditor.php b/plugins/SegmentEditor/SegmentEditor.php
index 9a6a4fd615..f9387fcb04 100644
--- a/plugins/SegmentEditor/SegmentEditor.php
+++ b/plugins/SegmentEditor/SegmentEditor.php
@@ -16,6 +16,7 @@ use Piwik\CacheId;
 use Piwik\Common;
 use Piwik\Config;
 use Piwik\Container\StaticContainer;
+use Piwik\CronArchive\SegmentArchiving;
 use Piwik\DataAccess\ArchiveSelector;
 use Piwik\Notification;
 use Piwik\Piwik;
@@ -122,7 +123,20 @@ class SegmentEditor extends \Piwik\Plugin
     public function getKnownSegmentsToArchiveForSite(&$segments, $idSite)
     {
         $model = new Model();
-        $segmentToAutoArchive = $model->getSegmentsToAutoArchive($idSite);
+        $segmentToAutoArchive = $model->getAllSegmentsAndIgnoreVisibility();
+
+        $forceAutoArchive = SegmentArchiving::getShouldForceArchiveAllSegments();
+        foreach ($segmentToAutoArchive as $index => $segmentInfo) {
+            if (!SegmentArchiving::isSegmentForSite($segmentInfo, $idSite)) {
+                unset($segmentToAutoArchive[$index]);
+            }
+
+            if (!$forceAutoArchive
+                && empty($segmentInfo['auto_archive'])
+            ) {
+                unset($segmentToAutoArchive[$index]);
+            }
+        }
 
         foreach ($segmentToAutoArchive as $segmentInfo) {
             $segments[] = $segmentInfo['definition'];
diff --git a/plugins/SegmentEditor/tests/Integration/ApiTest.php b/plugins/SegmentEditor/tests/Integration/ApiTest.php
index 13e0fd3357..86c1710f54 100644
--- a/plugins/SegmentEditor/tests/Integration/ApiTest.php
+++ b/plugins/SegmentEditor/tests/Integration/ApiTest.php
@@ -39,7 +39,6 @@ class ApiTest extends IntegrationTestCase
         if (!Fixture::siteCreated(2)) {
             Fixture::createWebsite('2012-01-01 00:00:00');
         }
-
     }
 
     public function test_getAll_forOneWebsite_returnsSortedSegments()
diff --git a/plugins/SegmentEditor/tests/System/ApiTest.php b/plugins/SegmentEditor/tests/System/ApiTest.php
new file mode 100644
index 0000000000..0ae9dea78c
--- /dev/null
+++ b/plugins/SegmentEditor/tests/System/ApiTest.php
@@ -0,0 +1,65 @@
+<?php
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\SegmentEditor\tests\System;
+
+
+use Piwik\Config;
+use Piwik\Http;
+use Piwik\Plugins\SegmentEditor\API as SegmentEditorApi;
+use Piwik\Tests\Framework\Fixture;
+use Piwik\Tests\Framework\TestCase\SystemTestCase;
+
+class ApiTest extends SystemTestCase
+{
+
+    public function test_segmentHashWorkflow_whenSegmentIsCrazyEncoded()
+    {
+        $segment = 'pageUrl=@%252F1';
+
+        Fixture::createWebsite('2020-03-03 00:00:00');
+
+        Config::getInstance()->General['enable_browser_archiving_triggering'] = 0;
+        self::$fixture->getTestEnvironment()->overrideConfig('General', 'enable_browser_archiving_triggering', 0);
+        self::$fixture->getTestEnvironment()->save();
+
+        $url = Fixture::getTestRootUrl() . '?' . http_build_query([
+            'module' => 'API',
+            'method' => 'SegmentEditor.add',
+            'name' => 'test segment',
+            'definition' => $segment,
+            'idSite' => 1,
+            'autoArchive' => 1,
+            'enabledAllUsers' => 1,
+            'format' => 'json',
+            'token_auth' => Fixture::getTokenAuth(),
+        ]);
+        self::assertStringContainsString(urlencode($segment), $url);
+
+        Http::sendHttpRequest($url, 10);
+
+        $segments = SegmentEditorApi::getInstance()->getAll();
+        $segmentDefinitionHash = end($segments);
+        $segmentDefinitionHash = $segmentDefinitionHash['hash'];
+
+        $url = Fixture::getTestRootUrl() . '?' . http_build_query([
+            'module' => 'API',
+            'method' => 'ExamplePlugin.getSegmentHash',
+            'segment' => $segment,
+            'idSite' => 1,
+            'format' => 'json',
+            'token_auth' => Fixture::getTokenAuth(),
+        ]);
+
+        $segmentApiHash = Http::sendHttpRequest($url, 10);
+        $segmentApiHash = json_decode($segmentApiHash, true);
+        $segmentApiHash = $segmentApiHash['value'];
+
+        $this->assertEquals($segmentApiHash, $segmentDefinitionHash);
+    }
+}
\ No newline at end of file
diff --git a/plugins/SegmentEditor/tests/System/UnprocessedSegmentsTest.php b/plugins/SegmentEditor/tests/System/UnprocessedSegmentsTest.php
index 0d0f78c3c4..28881d034e 100644
--- a/plugins/SegmentEditor/tests/System/UnprocessedSegmentsTest.php
+++ b/plugins/SegmentEditor/tests/System/UnprocessedSegmentsTest.php
@@ -57,7 +57,7 @@ class UnprocessedSegmentsTest extends IntegrationTestCase
         Rules::setBrowserTriggerArchiving(false);
 
         $segments = Rules::getSegmentsToProcess([self::$fixture->idSite]);
-        self::assertTrue(!in_array(self::TEST_SEGMENT, $segments));
+        self::assertTrue(in_array(self::TEST_SEGMENT, $segments)); // auto archive is forced when browser archiving is fully disabled
 
         $this->runAnyApiTest('VisitsSummary.get', 'realTimeSegmentUnprocessed', [
             'idSite' => self::$fixture->idSite,
diff --git a/plugins/SegmentEditor/tests/System/expected/test___VisitsSummary.get_realTimeSegmentUnprocessed.xml b/plugins/SegmentEditor/tests/System/expected/test___VisitsSummary.get_realTimeSegmentUnprocessed.xml
index 0dcab5ab6d..57b75e01db 100644
--- a/plugins/SegmentEditor/tests/System/expected/test___VisitsSummary.get_realTimeSegmentUnprocessed.xml
+++ b/plugins/SegmentEditor/tests/System/expected/test___VisitsSummary.get_realTimeSegmentUnprocessed.xml
@@ -1,4 +1,4 @@
 <?xml version="1.0" encoding="utf-8" ?>
 <result>
-	<error message="The Segment 'testsegment' is set to 'segmented reports are processed in real time' but Matomo is not currently configured to process segmented reports in API requests. To see data for this report in the future, you will need to edit your segment and choose the option labeled 'segmented reports are pre-processed (faster, requires cron)'. Then after a few hours your segment data should become available through the API. (If it does not, there may be a problem.)" />
+	<error message="These reports have no data, because the Segment you requested (testsegment) has not yet been processed by the system. Data for this Segment should become available in a few hours when processing completes. (If it does not, there may be a problem.) Please note that you can test whether your segment will work without having to wait for it to be processed by using the Live.getLastVisitsDetails API. When using this API method, you will see which users and actions were matched by your &amp;segment= parameter. This can help you confirm your Segment matches the users and actions you expect it to." />
 </result>
\ No newline at end of file
diff --git a/plugins/SegmentEditor/tests/UI/expected-screenshots/UnprocessedSegmentTest_unprocessed_segment.png b/plugins/SegmentEditor/tests/UI/expected-screenshots/UnprocessedSegmentTest_unprocessed_segment.png
index 7942d6c6f4..a8a2b568f3 100644
--- a/plugins/SegmentEditor/tests/UI/expected-screenshots/UnprocessedSegmentTest_unprocessed_segment.png
+++ b/plugins/SegmentEditor/tests/UI/expected-screenshots/UnprocessedSegmentTest_unprocessed_segment.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:1b7a3f5c09833cdc3efa54641744465aeaaa3c0ed2f9b3952f43e2b08fa4c291
-size 95094
+oid sha256:427f83e084082ff5007e3c4540108103ec20202532329e8d7dace57c4f8e361d
+size 95013