Ensure segment definitions doen't regress (#14372)

9 files changed, 65 insertions, 75 deletions

diff --git a/plugins/DevicesDetection/API.php b/plugins/DevicesDetection/API.php
index 8e4625f75a..667affa57b 100644
--- a/plugins/DevicesDetection/API.php
+++ b/plugins/DevicesDetection/API.php
@@ -255,7 +255,7 @@ class API extends \Piwik\Plugin\API
     public function getBrowsers($idSite, $period, $date, $segment = false)
     {
         $dataTable = $this->getDataTable('DevicesDetection_browsers', $idSite, $period, $date, $segment);
-        $dataTable->filter('AddSegmentByLabel', ['browserCode']);
+        $dataTable->filter('AddSegmentValue');
 
         // handle legacy archives
         if ($dataTable instanceof DataTable\Map || !$dataTable->getRowsCount()) {
diff --git a/plugins/DevicesDetection/Columns/BrowserName.php b/plugins/DevicesDetection/Columns/BrowserName.php
index 0c94371316..2b3c205a2f 100644
--- a/plugins/DevicesDetection/Columns/BrowserName.php
+++ b/plugins/DevicesDetection/Columns/BrowserName.php
@@ -20,15 +20,23 @@ class BrowserName extends Base
 {
     protected $columnName = 'config_browser_name';
     protected $columnType = 'VARCHAR(10) NULL';
-    protected $segmentName = 'browserName';
+    protected $segmentName = 'browserCode';
     protected $nameSingular = 'DevicesDetection_ColumnBrowser';
     protected $namePlural = 'DevicesDetection_Browsers';
-    protected $acceptValues = 'FireFox, Internet Explorer, Chrome, Safari, Opera etc.';
+    protected $acceptValues = 'FF, IE, CH, SF, OP etc.';
     protected $type = self::TYPE_TEXT;
 
-    public function __construct()
+    protected function configureSegments()
     {
-        $this->sqlFilterValue = function ($val) {
+        $segment = new Segment();
+        $segment->setName('DevicesDetection_BrowserCode');
+        $this->addSegment($segment);
+
+        $segment = new Segment();
+        $segment->setSegment('browserName');
+        $segment->setName('DevicesDetection_ColumnBrowser');
+        $segment->setAcceptedValues('FireFox, Internet Explorer, Chrome, Safari, Opera etc.');
+        $segment->setSqlFilterValue(function ($val) {
             $browsers = Browser::getAvailableBrowsers();
             array_map(function($val) {
                 return Common::mb_strtolower($val);
@@ -40,22 +48,10 @@ class BrowserName extends Base
             }
 
             return $result;
-        };
-        $this->suggestedValuesCallback = function ($idSite, $maxValuesToReturn) {
+        });
+        $segment->setSuggestedValuesCallback(function ($idSite, $maxValuesToReturn) {
             return array_values(Browser::getAvailableBrowsers() + ['Unknown']);
-        };
-    }
-
-    protected function configureSegments()
-    {
-        parent::configureSegments();
-
-        $segment = new Segment();
-        $segment->setSegment('browserCode');
-        $segment->setName('DevicesDetection_BrowserCode');
-        $segment->setAcceptedValues('FF, IE, CH, SF, OP etc.');
-        $this->suggestedValuesCallback = null;
-        $this->sqlFilterValue = null;
+        });
         $this->addSegment($segment);
     }
 
diff --git a/plugins/DevicesDetection/Columns/Os.php b/plugins/DevicesDetection/Columns/Os.php
index e3003b46fa..a86928279e 100644
--- a/plugins/DevicesDetection/Columns/Os.php
+++ b/plugins/DevicesDetection/Columns/Os.php
@@ -22,15 +22,23 @@ class Os extends Base
 {
     protected $columnName = 'config_os';
     protected $columnType = 'CHAR(3) NULL';
-    protected $segmentName = 'operatingSystemName';
+    protected $segmentName = 'operatingSystemCode';
     protected $nameSingular = 'DevicesDetection_ColumnOperatingSystem';
     protected $namePlural = 'DevicesDetection_OperatingSystems';
-    protected $acceptValues = 'Windows, Linux, Mac, Android, iOS etc.';
+    protected $acceptValues = 'WIN, LIN, MAX, AND, IOS etc.';
     protected $type = self::TYPE_TEXT;
 
-    public function __construct()
+    protected function configureSegments()
     {
-        $this->sqlFilterValue = function ($val) {
+        $segment = new Segment();
+        $segment->setName('DevicesDetection_OperatingSystemCode');
+        $this->addSegment($segment);
+
+        $segment = new Segment();
+        $segment->setSegment('operatingSystemName');
+        $segment->setName('DevicesDetection_ColumnOperatingSystem');
+        $segment->setAcceptedValues('Windows, Linux, Mac, Android, iOS etc.');
+        $segment->setSqlFilterValue(function ($val) {
             $oss = OperatingSystem::getAvailableOperatingSystems();
             array_map(function($val) {
                 return Common::mb_strtolower($val);
@@ -42,22 +50,10 @@ class Os extends Base
             }
 
             return $result;
-        };
-        $this->suggestedValuesCallback = function ($idSite, $maxValuesToReturn) {
+        });
+        $segment->setSuggestedValuesCallback(function ($idSite, $maxValuesToReturn) {
             return array_values(OperatingSystem::getAvailableOperatingSystems() + ['Unknown']);
-        };
-    }
-
-    protected function configureSegments()
-    {
-        parent::configureSegments();
-
-        $segment = new Segment();
-        $segment->setSegment('operatingSystemCode');
-        $segment->setName('DevicesDetection_OperatingSystemCode');
-        $segment->setAcceptedValues('WIN, LIN, MAX, AND, IOS etc.');
-        $this->suggestedValuesCallback = null;
-        $this->sqlFilterValue = null;
+        });
         $this->addSegment($segment);
     }
 
diff --git a/plugins/Ecommerce/tests/System/expected/test_ecommerceOrderWithItems__API.getProcessedReport_day.xml b/plugins/Ecommerce/tests/System/expected/test_ecommerceOrderWithItems__API.getProcessedReport_day.xml
index c0fad7bd09..fed58709d5 100644
--- a/plugins/Ecommerce/tests/System/expected/test_ecommerceOrderWithItems__API.getProcessedReport_day.xml
+++ b/plugins/Ecommerce/tests/System/expected/test_ecommerceOrderWithItems__API.getProcessedReport_day.xml
@@ -74,15 +74,15 @@
 	</reportData>
 	<reportMetadata>
 		<row>
-			<segment>countryCode==pl</segment>
 			<code>pl</code>
 			<logo>plugins/Morpheus/icons/dist/flags/pl.png</logo>
+			<segment>countryCode==pl</segment>
 			<logoHeight>16</logoHeight>
 		</row>
 		<row>
-			<segment>countryCode==fr</segment>
 			<code>fr</code>
 			<logo>plugins/Morpheus/icons/dist/flags/fr.png</logo>
+			<segment>countryCode==fr</segment>
 			<logoHeight>16</logoHeight>
 		</row>
 	</reportMetadata>
diff --git a/plugins/Ecommerce/tests/System/expected/test_ecommerceOrderWithItems__UserCountry.getCountry_day.xml b/plugins/Ecommerce/tests/System/expected/test_ecommerceOrderWithItems__UserCountry.getCountry_day.xml
index 44d4797c79..c30519b3db 100644
--- a/plugins/Ecommerce/tests/System/expected/test_ecommerceOrderWithItems__UserCountry.getCountry_day.xml
+++ b/plugins/Ecommerce/tests/System/expected/test_ecommerceOrderWithItems__UserCountry.getCountry_day.xml
@@ -29,9 +29,9 @@
 		</goals>
 		<nb_conversions>2</nb_conversions>
 		<revenue>3111.11</revenue>
-		<segment>countryCode==pl</segment>
 		<code>pl</code>
 		<logo>plugins/Morpheus/icons/dist/flags/pl.png</logo>
+		<segment>countryCode==pl</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -52,9 +52,9 @@
 		</goals>
 		<nb_conversions>1</nb_conversions>
 		<revenue>10</revenue>
-		<segment>countryCode==fr</segment>
 		<code>fr</code>
 		<logo>plugins/Morpheus/icons/dist/flags/fr.png</logo>
+		<segment>countryCode==fr</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 </result>
\ No newline at end of file
diff --git a/plugins/GeoIp2/tests/System/expected/test_ConvertRegionCodesToIsoTest_testExecute_ShouldConvertRegionCodes__UserCountry.getCountry_month.xml b/plugins/GeoIp2/tests/System/expected/test_ConvertRegionCodesToIsoTest_testExecute_ShouldConvertRegionCodes__UserCountry.getCountry_month.xml
index 908fb0801f..57c00f7924 100644
--- a/plugins/GeoIp2/tests/System/expected/test_ConvertRegionCodesToIsoTest_testExecute_ShouldConvertRegionCodes__UserCountry.getCountry_month.xml
+++ b/plugins/GeoIp2/tests/System/expected/test_ConvertRegionCodesToIsoTest_testExecute_ShouldConvertRegionCodes__UserCountry.getCountry_month.xml
@@ -9,9 +9,9 @@
 		<nb_visits_converted>0</nb_visits_converted>
 		<sum_daily_nb_uniq_visitors>3</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>3</sum_daily_nb_users>
-		<segment>countryCode==ir</segment>
 		<code>ir</code>
 		<logo>plugins/Morpheus/icons/dist/flags/ir.png</logo>
+		<segment>countryCode==ir</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -23,9 +23,9 @@
 		<nb_visits_converted>0</nb_visits_converted>
 		<sum_daily_nb_uniq_visitors>2</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>2</sum_daily_nb_users>
-		<segment>countryCode==jm</segment>
 		<code>jm</code>
 		<logo>plugins/Morpheus/icons/dist/flags/jm.png</logo>
+		<segment>countryCode==jm</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -37,9 +37,9 @@
 		<nb_visits_converted>0</nb_visits_converted>
 		<sum_daily_nb_uniq_visitors>1</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>1</sum_daily_nb_users>
-		<segment>countryCode==ad</segment>
 		<code>ad</code>
 		<logo>plugins/Morpheus/icons/dist/flags/ad.png</logo>
+		<segment>countryCode==ad</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -51,9 +51,9 @@
 		<nb_visits_converted>0</nb_visits_converted>
 		<sum_daily_nb_uniq_visitors>1</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>1</sum_daily_nb_users>
-		<segment>countryCode==bm</segment>
 		<code>bm</code>
 		<logo>plugins/Morpheus/icons/dist/flags/bm.png</logo>
+		<segment>countryCode==bm</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -65,9 +65,9 @@
 		<nb_visits_converted>0</nb_visits_converted>
 		<sum_daily_nb_uniq_visitors>1</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>1</sum_daily_nb_users>
-		<segment>countryCode==cn</segment>
 		<code>cn</code>
 		<logo>plugins/Morpheus/icons/dist/flags/cn.png</logo>
+		<segment>countryCode==cn</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -79,9 +79,9 @@
 		<nb_visits_converted>0</nb_visits_converted>
 		<sum_daily_nb_uniq_visitors>1</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>1</sum_daily_nb_users>
-		<segment>countryCode==gr</segment>
 		<code>gr</code>
 		<logo>plugins/Morpheus/icons/dist/flags/gr.png</logo>
+		<segment>countryCode==gr</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -93,9 +93,9 @@
 		<nb_visits_converted>0</nb_visits_converted>
 		<sum_daily_nb_uniq_visitors>1</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>1</sum_daily_nb_users>
-		<segment>countryCode==gb</segment>
 		<code>gb</code>
 		<logo>plugins/Morpheus/icons/dist/flags/gb.png</logo>
+		<segment>countryCode==gb</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -109,6 +109,7 @@
 		<sum_daily_nb_users>1</sum_daily_nb_users>
 		<code />
 		<logo>plugins/Morpheus/icons/dist/flags/xx.png</logo>
+		<segment>countryCode==</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 </result>
\ No newline at end of file
diff --git a/plugins/UserCountry/API.php b/plugins/UserCountry/API.php
index 72b6c71142..34a00a4add 100644
--- a/plugins/UserCountry/API.php
+++ b/plugins/UserCountry/API.php
@@ -56,7 +56,7 @@ class API extends \Piwik\Plugin\API
         }
 
         // apply filter on the whole datatable in order the inline search to work (searches are done on "beautiful" label)
-        $dataTable->filter('AddSegmentByLabel', ['countryCode']);
+        $dataTable->filter('AddSegmentValue');
         $dataTable->filter('ColumnCallbackAddMetadata', array('label', 'code'));
         $dataTable->filter('ColumnCallbackAddMetadata', array('label', 'logo', __NAMESPACE__ . '\getFlagFromCode'));
         $dataTable->filter('ColumnCallbackReplace', array('label', __NAMESPACE__ . '\countryTranslate'));
diff --git a/plugins/UserCountry/Columns/Country.php b/plugins/UserCountry/Columns/Country.php
index 21c40251ee..9ea19844e1 100644
--- a/plugins/UserCountry/Columns/Country.php
+++ b/plugins/UserCountry/Columns/Country.php
@@ -35,38 +35,35 @@ class Country extends Base
     protected $category =  'UserCountry_VisitLocation';
     protected $nameSingular = 'UserCountry_Country';
     protected $namePlural = 'UserCountryMap_Countries';
-    protected $segmentName = 'countryName';
-    protected $acceptValues = 'Germany, France, Spain, ...';
+    protected $segmentName = 'countryCode';
+    protected $acceptValues = 'ISO 3166-1 alpha-2 country codes (de, us, fr, in, es, etc.)';
 
-    public function __construct()
+    protected function configureSegments()
     {
+        $segment = new Segment();
+        $segment->setName('UserCountry_CountryCode');
+        $this->addSegment($segment);
+
+        $segment = new Segment();
+        $segment->setSegment('countryName');
+        $segment->setName('UserCountry_Country');
+        $segment->setAcceptedValues('Germany, France, Spain, ...');
         $regionDataProvider = StaticContainer::get('Piwik\Intl\Data\Provider\RegionDataProvider');
         $countryList = $regionDataProvider->getCountryList();
         array_walk($countryList, function(&$item, $key) {
             $item = Piwik::translate('Intl_Country_'.strtoupper($key), [], 'en');
         });
 
-        $this->sqlFilterValue = function ($val) use ($countryList) {
+        $segment->setSqlFilterValue(function ($val) use ($countryList) {
             $result   = array_search($val, $countryList);
             if ($result === false) {
                 $result = 'UNK';
             }
             return $result;
-        };
-        $this->suggestedValuesCallback = function ($idSite, $maxValuesToReturn) use ($countryList) {
+        });
+        $segment->setSuggestedValuesCallback(function ($idSite, $maxValuesToReturn) use ($countryList) {
             return array_values($countryList + ['Unknown']);
-        };
-    }
-
-    protected function configureSegments()
-    {
-        parent::configureSegments();
-        $segment = new Segment();
-        $segment->setSegment('countryCode');
-        $segment->setName('UserCountry_CountryCode');
-        $segment->setAcceptedValues('ISO 3166-1 alpha-2 country codes (de, us, fr, in, es, etc.)');
-        $this->suggestedValuesCallback = null;
-        $this->sqlFilterValue = null;
+        });
         $this->addSegment($segment);
     }
 
diff --git a/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData__UserCountry.getCountry_month.xml b/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData__UserCountry.getCountry_month.xml
index 79c5792f66..ade01286fe 100644
--- a/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData__UserCountry.getCountry_month.xml
+++ b/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData__UserCountry.getCountry_month.xml
@@ -22,9 +22,9 @@
 		<revenue>150</revenue>
 		<sum_daily_nb_uniq_visitors>10</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>0</sum_daily_nb_users>
-		<segment>countryCode==cn</segment>
 		<code>cn</code>
 		<logo>plugins/Morpheus/icons/dist/flags/cn.png</logo>
+		<segment>countryCode==cn</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -49,9 +49,9 @@
 		<revenue>45</revenue>
 		<sum_daily_nb_uniq_visitors>3</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>0</sum_daily_nb_users>
-		<segment>countryCode==ca</segment>
 		<code>ca</code>
 		<logo>plugins/Morpheus/icons/dist/flags/ca.png</logo>
+		<segment>countryCode==ca</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -76,9 +76,9 @@
 		<revenue>20</revenue>
 		<sum_daily_nb_uniq_visitors>2</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>1</sum_daily_nb_users>
-		<segment>countryCode==fr</segment>
 		<code>fr</code>
 		<logo>plugins/Morpheus/icons/dist/flags/fr.png</logo>
+		<segment>countryCode==fr</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -103,9 +103,9 @@
 		<revenue>15</revenue>
 		<sum_daily_nb_uniq_visitors>1</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>0</sum_daily_nb_users>
-		<segment>countryCode==id</segment>
 		<code>id</code>
 		<logo>plugins/Morpheus/icons/dist/flags/id.png</logo>
+		<segment>countryCode==id</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -130,9 +130,9 @@
 		<revenue>15</revenue>
 		<sum_daily_nb_uniq_visitors>1</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>0</sum_daily_nb_users>
-		<segment>countryCode==it</segment>
 		<code>it</code>
 		<logo>plugins/Morpheus/icons/dist/flags/it.png</logo>
+		<segment>countryCode==it</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 	<row>
@@ -157,9 +157,9 @@
 		<revenue>15</revenue>
 		<sum_daily_nb_uniq_visitors>1</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>0</sum_daily_nb_users>
-		<segment>countryCode==us</segment>
 		<code>us</code>
 		<logo>plugins/Morpheus/icons/dist/flags/us.png</logo>
+		<segment>countryCode==us</segment>
 		<logoHeight>16</logoHeight>
 	</row>
 </result>
\ No newline at end of file