Proportional evolution comparison for incomplete periods (#18099)

* Multisite evolution metrics changed to calculate proportionally to percent of the current period which is complete * Use piwik date class, use report generated date if available, added unit test, added tooltip * Improved tooltip detail * Updated unit tested, added tests for evolution metric getRatio(), changes to allow row metadata to be preserved during datatable merges * Additional API test fixes * More test fixes * More test fixes * Remove ts_archived row metadata from final API output * Test fix reversions, added deleteRowsMetadata() method to DataTableInterface * More test fix reversions * Fixed integration test * Trigger Build * Update core/DataTable/Map.php Co-authored-by: Stefan Giehl <[email protected]> * Update core/DataTable.php Co-authored-by: Stefan Giehl <[email protected]> * Update core/Archive/DataCollection.php Co-authored-by: Stefan Giehl <[email protected]> * Update core/DataTable.php Co-authored-by: Stefan Giehl <[email protected]> * Update core/DataTable.php Co-authored-by: Stefan Giehl <[email protected]> * Update plugins/CoreHome/Columns/Metrics/EvolutionMetric.php Co-authored-by: Stefan Giehl <[email protected]> * Improved tooltips for translation, use NumberFormatter for percents, moved additional constructor param to end, null checks * Update plugins/CoreHome/Columns/Metrics/EvolutionMetric.php Co-authored-by: Stefan Giehl <[email protected]> * Update plugins/CoreHome/Columns/Metrics/EvolutionMetric.php Co-authored-by: Stefan Giehl <[email protected]> * Use localized period string, remove unnecessary tooltip percent digits * Formatting fixes * Fix for an issue where evolution values > 999% would be displayed incorrectly * Added data table processor option to provide raw copy of formatted metrics * Update plugins/MultiSites/API.php Fix for row metadata removed too early Co-authored-by: Stefan Giehl <[email protected]> * Replace evolution metrics 'add raw copy' api parameter with _trend column * ensure to use correct metric to check if lower value is better * updates expected test files * fix some more tests * update test file Co-authored-by: Stefan Giehl <[email protected]>
matomo-org · Nov 15, 2021 · 556aada · 556aada
1 parent 2711257
commit 556aada
Show file tree

Hide file tree

Showing 43 changed files with 1,380 additions and 24 deletions.
diff --git a/core/API/DataTablePostProcessor.php b/core/API/DataTablePostProcessor.php
@@ -22,6 +22,7 @@
 use Piwik\Plugin\Report;
 use Piwik\Plugin\ReportsProvider;
 use Piwik\Plugins\API\Filter\DataComparisonFilter;
+use Piwik\Plugins\CoreHome\Columns\Metrics\EvolutionMetric;
 
 /**
  * Processes DataTables that should be served through Piwik's APIs. This processing handles
@@ -453,6 +454,11 @@ public function computeProcessedMetrics(DataTable $dataTable)
                 $computedValue = $processedMetric->compute($row);
                 if ($computedValue !== false) {
                     $row->addColumn($name, $computedValue);
+
+                    // Add a trend column for evolution metrics
+                    if ($processedMetric instanceof EvolutionMetric) {
+                        $row->addColumn($processedMetric->getTrendName(), $processedMetric->getTrendValue($computedValue));
+                    }
                 }
             }
         }

diff --git a/core/Archive.php b/core/Archive.php
@@ -342,7 +342,7 @@ public function getDataTableFromNumeric($names)
      */
     public function getDataTableFromNumericAndMergeChildren($names)
     {
-        $data  = $this->get($names, 'numeric');
+        $data = $this->get($names, 'numeric');
         $resultIndexes = $this->getResultIndices();
         return $data->getMergedDataTable($resultIndexes);
     }
@@ -539,7 +539,7 @@ protected function get($archiveNames, $archiveDataType, $idSubtable = null)
                 $result->addMetadata($row['idsite'], $periodStr, DataTable::ARCHIVED_DATE_METADATA_NAME, $row['ts_archived']);
             }
 
-            $result->set($row['idsite'], $periodStr, $row['name'], $row['value']);
+            $result->set($row['idsite'], $periodStr, $row['name'], $row['value'], [DataTable::ARCHIVED_DATE_METADATA_NAME => $row['ts_archived']]);
         }
 
         return $result;

diff --git a/core/Archive/DataCollection.php b/core/Archive/DataCollection.php
@@ -147,11 +147,17 @@ public function &get($idSite, $period)
      * @param string $period eg, '2012-01-01,2012-01-31'
      * @param string $name eg 'nb_visits'
      * @param string $value eg 5
+     * @param array  $meta Optional metadata to add to the row
      */
-    public function set($idSite, $period, $name, $value)
+    public function set($idSite, $period, $name, $value, array $meta = null)
     {
         $row = & $this->get($idSite, $period);
         $row[$name] = $value;
+        if ($meta) {
+            foreach ($meta as $k => $v) {
+                $row[self::METADATA_CONTAINER_ROW_KEY][$k] = $v;
+            }
+        }
     }
 
     /**

diff --git a/core/Archive/DataTableFactory.php b/core/Archive/DataTableFactory.php
@@ -575,15 +575,22 @@ private function makeMergedWithSiteIndex($index, $useSimpleDataTable, $isNumeric
         $this->setPrettySegmentMetadata($table);
 
         foreach ($index as $idsite => $row) {
+
+            $meta = array();
+            if (isset($row[DataCollection::METADATA_CONTAINER_ROW_KEY])) {
+                $meta = $row[DataCollection::METADATA_CONTAINER_ROW_KEY];
+            }
+            $meta['idsite'] = $idsite;
+
             if (!empty($row)) {
                 $table->addRow(new Row(array(
                     Row::COLUMNS  => $row,
-                    Row::METADATA => array('idsite' => $idsite))
+                    Row::METADATA => $meta)
                 ));
             } elseif ($isNumeric) {
                 $table->addRow(new Row(array(
                     Row::COLUMNS  => $this->defaultRow,
-                    Row::METADATA => array('idsite' => $idsite))
+                    Row::METADATA => $meta)
                 ));
             }
         }

diff --git a/core/DataTable.php b/core/DataTable.php
@@ -1001,6 +1001,31 @@ public function getRowsMetadata($name)
         return $metadataValues;
     }
 
+    /**
+     * Delete row metadata by name in every row.
+     *
+     * @param       $name
+     * @param bool $deleteRecursiveInSubtables
+     */
+    public function deleteRowsMetadata($name, $deleteRecursiveInSubtables = false)
+    {
+        foreach ($this->rows as $row) {
+            $row->deleteMetadata($name);
+
+            $subTable = $row->getSubtable();
+            if ($subTable) {
+                $subTable->deleteRowsMetadata($name, $deleteRecursiveInSubtables);
+            }
+        }
+        if (!is_null($this->summaryRow)) {
+            $this->summaryRow->deleteMetadata($name);
+        }
+        if (!is_null($this->totalsRow)) {
+            $this->totalsRow->deleteMetadata($name);
+        }
+
+    }
+
     /**
      * Returns the number of rows in the table including the summary row.
      *

diff --git a/core/DataTable/DataTableInterface.php b/core/DataTable/DataTableInterface.php
@@ -27,4 +27,5 @@ public function deleteRow($id);
     public function deleteColumn($name);
     public function getColumn($name);
     public function getColumns();
+    public function deleteRowsMetadata($name, $deleteRecursiveInSubtables = false);
 }
diff --git a/core/DataTable/Map.php b/core/DataTable/Map.php
@@ -513,6 +513,19 @@ public function getMetadataIntersectArray($name)
         return array_values($data);
     }
 
+    /**
+     * Delete row metadata by name in every row.
+     *
+     * @param       $name
+     * @param bool $deleteRecursiveInSubtables
+     */
+    public function deleteRowsMetadata($name, $deleteRecursiveInSubtables = false)
+    {
+        foreach ($this->getDataTables() as $table) {
+            $table->deleteRowsMetadata($name, $deleteRecursiveInSubtables);
+        }
+    }
+
     /**
      * See {@link DataTable::getColumns()}.
      *

diff --git a/plugins/CoreHome/Columns/Metrics/EvolutionMetric.php b/plugins/CoreHome/Columns/Metrics/EvolutionMetric.php
@@ -9,8 +9,12 @@
 namespace Piwik\Plugins\CoreHome\Columns\Metrics;
 
 use Piwik\DataTable;
+use Piwik\Archive\DataTableFactory;
 use Piwik\DataTable\Row;
+use Piwik\Date;
 use Piwik\Metrics;
+use Piwik\Plugins\SitesManager\API;
+use Piwik\Site;
 use Piwik\Metrics\Formatter;
 use Piwik\Piwik;
 use Piwik\Plugin\Metric;
@@ -36,6 +40,11 @@ class EvolutionMetric extends ProcessedMetric
      */
     private $evolutionMetricName;
 
+    /**
+     * @var string
+     */
+    private $evolutionMetricTrendName;
+
     /**
      * @var int
      */
@@ -46,6 +55,11 @@ class EvolutionMetric extends ProcessedMetric
      */
     private $pastData;
 
+    /**
+     * @var DataTable
+     */
+    private $currentData;
+
     /**
      * The list of labels leading to the current subtable being processed. Used to get the proper subtable in
      * $pastData.
@@ -62,17 +76,22 @@ class EvolutionMetric extends ProcessedMetric
      * @param string|false $evolutionMetricName The name of the evolution processed metric. Defaults to
      *                                          $wrapped's name with `'_evolution'` appended.
      * @param int $quotientPrecision The percent's quotient precision.
+     * @param DataTable|null $currentData The current datatable, optional but required to calculate the proportionate
+     *                                    evolution values
      */
-    public function __construct($wrapped, DataTable $pastData = null, $evolutionMetricName = false, $quotientPrecision = 0)
+    public function __construct($wrapped, ?DataTable $pastData = null, $evolutionMetricName = false, $quotientPrecision = 0,
+                                ?DataTable $currentData = null)
     {
         $this->wrapped = $wrapped;
         $this->pastData = $pastData;
+        $this->currentData = $currentData;
 
         if (empty($evolutionMetricName)) {
             $wrappedName = $this->getWrappedName();
             $evolutionMetricName = $wrappedName . '_evolution';
         }
 
+        $this->evolutionMetricTrendName = $evolutionMetricName . '_trend';
         $this->evolutionMetricName = $evolutionMetricName;
         $this->quotientPrecision = $quotientPrecision;
     }
@@ -82,6 +101,11 @@ public function getName()
         return $this->evolutionMetricName;
     }
 
+    public function getTrendName()
+    {
+        return $this->evolutionMetricTrendName;
+    }
+
     public function getTranslatedName()
     {
         if ($this->wrapped instanceof Metric) {
@@ -93,6 +117,16 @@ public function getTranslatedName()
         return Piwik::translate('CoreHome_EvolutionMetricName', [$metricName]);
     }
 
+    public function getTrendValue($computedValue = 0)
+    {
+        $isLowerBetter = Metrics::isLowerValueBetter($this->wrapped);
+        if ($isLowerBetter) {
+            return ($computedValue < 0 ? 1 : ($computedValue > 0 ? -1 : 0));
+        }
+
+        return ($computedValue < 0 ? -1 : ($computedValue > 0 ? 1 : 0));
+    }
+
     public function compute(Row $row)
     {
         $columnName = $this->getWrappedName();
@@ -101,6 +135,16 @@ public function compute(Row $row)
         $currentValue = $this->getMetric($row, $columnName);
         $pastValue = $pastRow ? $this->getMetric($pastRow, $columnName) : 0;
 
+        // Reduce past value proportionally to match the percent of the current period which is complete, if applicable
+        $ratio = self::getRatio($this->currentData, $this->pastData, $row);
+        $period = $this->pastData->getMetadata(DataTableFactory::TABLE_METADATA_PERIOD_INDEX);
+        $row->setMetadata('ratio', $ratio);
+        $row->setMetadata('currencySymbol', $row['label'] !== DataTable::ID_SUMMARY_ROW ? Site::getCurrencySymbolFor($row['label']) : API::getInstance()->getDefaultCurrency());
+        $row->setMetadata('previous_'.$columnName, $pastValue);
+        $row->setMetadata('periodName', $period->getLabel());
+        $row->setMetadata('previousRange', $period->getLocalizedShortString());
+        $pastValue = ($pastValue * $ratio);
+
         $dividend = $currentValue - $pastValue;
         $divisor = $pastValue;
 
@@ -170,4 +214,56 @@ private function getPastDataTable()
         }
         return $result;
     }
+
+    /**
+     * Calculate the ratio of time between a past period and current incomplete period
+     *
+     * eg. if today is Thursday at 12:00pm and the past period is a week then the ratio is 0.5, exactly half of the
+     * current incomplete period has passed
+     *
+     * If the current period end is in the past then the ratio will always be 1, since the current period is complete.
+     *
+     * @param DataTable|null $currentData
+     * @param DataTable|null $pastData
+     * @param Row $row
+     * @return float|int
+     * @throws \Exception
+     */
+    public static function getRatio(?DataTable $currentData, ?DataTable $pastData, Row $row)
+    {
+        $ratio = 1;
+
+        if ($currentData != null && $pastData != null) {
+
+            $p = $pastData->getMetadata(DataTableFactory::TABLE_METADATA_PERIOD_INDEX);
+
+            $pStart = $p->getDateStart()->setTime('00:00:00');
+            $pEnd = $p->getDateEnd()->setTime('23:59:59');
+
+            $c = $currentData->getMetadata(DataTableFactory::TABLE_METADATA_PERIOD_INDEX);
+            $cStart = $c->getDateStart()->setTime('00:00:00');
+            $cEnd = $c->getDateEnd()->setTime('23:59:59');
+
+            $nowTS = Date::getNowTimestamp();
+
+            // If we know the date the the datatable data was generated then use that instead of now
+            $archivedDateStr = $row->getMetadata(DataTable::ARCHIVED_DATE_METADATA_NAME);
+
+            if ($archivedDateStr) {
+                $archivedDate = Date::factory($archivedDateStr);
+                if ($archivedDate) {
+                    $nowTS = Date::factory($archivedDate)->getTimestamp();
+                }
+            }
+
+            if ($cStart->getTimestamp() <= $nowTS && $cEnd->getTimestamp() >= $nowTS) {
+                $secsInPastPeriod = $pEnd->getTimestamp() - $pStart->getTimestamp();
+                $secsInCurrentPeriod = $nowTS - $cStart->getTimestamp();
+                $ratio = $secsInCurrentPeriod / $secsInPastPeriod;
+            }
+        }
+
+        return round($ratio, 3);
+
+    }
 }
diff --git a/plugins/CoreHome/tests/Unit/EvolutionMetricTest.php b/plugins/CoreHome/tests/Unit/EvolutionMetricTest.php
@@ -0,0 +1,63 @@
+<?php
+/**
+ * Piwik - 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\CoreHome\tests\Unit;
+
+use PHPUnit\Framework\TestCase;
+use Piwik\DataTable;
+use Piwik\Archive\DataCollection;
+use Piwik\DataTable\Row;
+use Piwik\Date;
+use Piwik\Plugins\CoreHome\Columns\Metrics\EvolutionMetric;
+
+/**
+ * @group CoreHome
+ * @group CoreHomeTest
+ * @group EvolutionMetric
+ */
+class EvolutionMetricTest extends TestCase
+{
+    public function test_shouldDoProportionalComparision_ifCurrentPeriodIncomplete()
+    {
+        $currentData = new DataTable();
+        $cPeriod = new \Piwik\Period\Week(Date::factory('2021-10-10'));
+        $currentData->setMetadata('period', $cPeriod);
+
+        // If the archived date meta data value exists on the row then it will be used
+        // as the current date for calculation purposes, we can use this to consistently test the
+        // ratio calculation by supplying a fixed set of dates that should result in a 0.5 ratio
+
+        $row = new Row();
+        $row->setMetadata(DataTable::ARCHIVED_DATE_METADATA_NAME, '2021-10-07 00:00:00');
+
+        $pastData = new DataTable();
+        $sPeriod = new \Piwik\Period\Week(Date::factory('2021-10-03'));
+        $pastData->setMetadata('period', $sPeriod);
+
+        $ratio = EvolutionMetric::getRatio($currentData, $pastData, $row);
+
+        $this->assertEquals(0.429, $ratio);
+    }
+
+    public function test_shouldNotDoProportionalComparision_ifCurrentPeriodComplete()
+    {
+        $currentData = new DataTable();
+        $cPeriod = new \Piwik\Period\Week(Date::factory('2021-10-10'));
+        $currentData->setMetadata('period', $cPeriod);
+
+        $pastData = new DataTable();
+        $sPeriod = new \Piwik\Period\Week(Date::factory('2021-10-03'));
+        $pastData->setMetadata('period', $sPeriod);
+
+        $ratio = EvolutionMetric::getRatio($currentData, $pastData, new Row());
+
+        $this->assertEquals(1, $ratio);
+    }
+
+}
diff --git a/plugins/MultiSites/API.php b/plugins/MultiSites/API.php
@@ -313,6 +313,12 @@ function ($value) {
             );
         }
 
+        // Remove <ts_archived> row metadata, it's already been used by any filters that needed it
+        $dataTable->queueFilter(function($dataTable) {
+            $dataTable->deleteRowsMetadata(DataTable::ARCHIVED_DATE_METADATA_NAME);
+            $dataTable->deleteColumn('_metadata');
+        });
+
         if ($multipleWebsitesRequested && $dataTable->getRowsCount() === 1 && $dataTable instanceof DataTable\Simple) {
             $simpleTable = $dataTable;
             $dataTable   = $simpleTable->getEmptyClone();
@@ -358,7 +364,8 @@ private function calculateEvolutionPercentages($currentData, $pastData, $apiMetr
                     $metricSettings[self::METRIC_RECORD_NAME_KEY],
                     $pastData,
                     $metricSettings[self::METRIC_EVOLUTION_COL_NAME_KEY],
-                    $quotientPrecision = 1
+                    $quotientPrecision = 1,
+                    $currentData
                 );
             }
             $currentData->setMetadata(DataTable::EXTRA_PROCESSED_METRICS_METADATA_NAME, $extraProcessedMetrics);