From a15cffed8ca8cea424f7ef8b15a05ab5a67ec2e9 Mon Sep 17 00:00:00 2001 From: Ben Burgess <88810029+bx80@users.noreply.github.com> Date: Fri, 17 Nov 2023 04:11:25 +1300 Subject: [PATCH] Hide segments containing expressions from disabled plugins or features (#21521) * Filter out segments for disabled plugins or features * Update plugins/SegmentEditor/API.php Co-authored-by: Michal Kleiner --------- Co-authored-by: Michal Kleiner --- plugins/SegmentEditor/API.php | 26 +++ .../Segment/SegmentUnavailableTest.php | 186 ++++++++++++++++++ 2 files changed, 212 insertions(+) create mode 100644 tests/PHPUnit/Integration/Segment/SegmentUnavailableTest.php diff --git a/plugins/SegmentEditor/API.php b/plugins/SegmentEditor/API.php index e5f0771b5f2..bf6d88a2601 100644 --- a/plugins/SegmentEditor/API.php +++ b/plugins/SegmentEditor/API.php @@ -395,11 +395,37 @@ public function getAll($idSite = false) } } + $segments = $this->filterSegmentsWithDisabledElements($segments, $idSite); $segments = $this->sortSegmentsCreatedByUserFirst($segments); return $segments; } + /** + * Filter out any segments which cannot be initialized due to disable plugins or features + * + * @param array $segments + * @param $idSite + * + * @return array + */ + private function filterSegmentsWithDisabledElements(array $segments, $idSite = false): array + { + $sites = []; + if (!empty($idSite)) { + $sites = is_array($idSite) ? $idSite : [$idSite]; + } + + foreach ($segments as $k => $segment) { + try { + new Segment($segment['definition'], $sites); + } catch (Exception $e) { + unset($segments[$k]); + } + } + return $segments; + } + /** * Sorts segment in a particular order: * diff --git a/tests/PHPUnit/Integration/Segment/SegmentUnavailableTest.php b/tests/PHPUnit/Integration/Segment/SegmentUnavailableTest.php new file mode 100644 index 00000000000..cd0d0f469d0 --- /dev/null +++ b/tests/PHPUnit/Integration/Segment/SegmentUnavailableTest.php @@ -0,0 +1,186 @@ +addSite('test', 'http://example.org'); + + Option::set(Rules::OPTION_BROWSER_TRIGGER_ARCHIVING, 0); + } + + public function tearDown() : void + { + parent::tearDown(); + Option::set(Rules::OPTION_BROWSER_TRIGGER_ARCHIVING, 1); + } + + public function testvisitorIdSegmentIsDisabledWhenVisitorProfileUnavailable() + { + // Create a new segment that uses the visitorId property + $definition = 'visitorId==4b3d9389bae51466'; + $name = 'visitorId'; + $visitorSegmentId = API::getInstance()->add($name, $definition, $this->idSite, $autoArchive = 1, $enabledAllUsers = 1); + + // Check it is returned by the API get all segment call + $this->checkSegmentAvailable($definition, $name, true); + + // Disable the visitor progile + $this->disableVisitorProfile(true); + + // Check that the new segment is no longer returned by the API + $this->checkSegmentAvailable($definition, $name, false); + + // Clean up + API::getInstance()->delete($visitorSegmentId); + $this->disableVisitorProfile(false); + } + + public function testUserIdSegmentIsDisabledWhenVisitorProfileUnavailable() + { + // Create a new segment that uses the userId property + $definition = 'userId==4b3d9389bae51466'; + $name = 'userId'; + $userSegmentId = API::getInstance()->add($name, $definition, $this->idSite, $autoArchive = 1, $enabledAllUsers = 1); + + // Check it is returned by the API get all segment call + $this->checkSegmentAvailable($definition, $name, true); + + // Disable the visitor progile + $this->disableVisitorProfile(true); + + // Check that the new segment is no longer returned by the API + $this->checkSegmentAvailable($definition, $name, false); + + // Clean up + API::getInstance()->delete($userSegmentId); + $this->disableVisitorProfile(false); + } + + public function testPluginSegmentIsDisabledWhenPluginUnavailable() + { + + // Create a new segment that uses the userId property + $definition = 'browserName==Chrome'; + $name = 'browserName'; + $browserNameSegmentId = API::getInstance()->add($name, $definition, $this->idSite, $autoArchive = 1, $enabledAllUsers = 1); + + // Check it is returned by the API get all segment call + $this->checkSegmentAvailable($definition, $name, true); + + // Disable the devices detection plugin so the browser name metric will be unavailable + \Piwik\Plugin\Manager::getInstance()->deactivatePlugin('DevicesDetection'); + \Piwik\Plugin\Manager::getInstance()->unloadPlugin('DevicesDetection'); + $this->flushCaches(); + + // Check that the new segment is no longer returned by the API + $this->checkSegmentAvailable($definition, $name, false); + + // Clean up + \Piwik\Plugin\Manager::getInstance()->loadPlugin('DevicesDetection'); + \Piwik\Plugin\Manager::getInstance()->installLoadedPlugins(); + \Piwik\Plugin\Manager::getInstance()->activatePlugin('DevicesDetection'); + API::getInstance()->delete($browserNameSegmentId); + $this->flushCaches(); + } + + /** + * Disable the visitorlog feature + * + * @param bool $status + * + * @return void + * @throws \Exception + */ + private function disableVisitorProfile(bool $status): void + { + $settings = new SystemSettings(); + $settings->disableVisitorLog->setValue($status); + $settings->disableVisitorProfile->setValue($status); + $settings->save(); + + $this->flushCaches(); + } + + /** + * Flush the caches that contain cached segment data + * + * @return void + */ + private function flushCaches(): void + { + Cache::getLazyCache()->flushAll(); + Cache::getEagerCache()->flushAll(); + Cache::getTransientCache()->flushAll(); + } + + /** + * Check if a segment is available + * + * @param string $definition + * @param string $name + * @param bool $shouldBeEnabled + * + * @return void + */ + private function checkSegmentAvailable(string $definition, string $name, bool $shouldBeEnabled): void + { + $expected = []; + if ($shouldBeEnabled) { + $expected = [0 => + [ + 'idsegment' => 1, + 'name' => $name, + 'definition' => $definition, + 'hash' => md5($definition), + 'login' => 'super user was set', + 'enable_all_users' => 1, + 'enable_only_idsite' => 1, + 'auto_archive' => 1, + 'ts_last_edit' => null, + 'deleted' => 0, + ] + ]; + } + + $segments = API::getInstance()->getAll($this->idSite); + if (isset($segments[0]['ts_created'])) { + unset($segments[0]['ts_created']); + } + + $this->assertEquals($expected, $segments); + } +}