SMS report: percentage evolution are prefixed by '+'

https://github.com/piwik/piwik/pull/8857

5 files changed, 62 insertions, 9 deletions

diff --git a/core/NumberFormatter.php b/core/NumberFormatter.php
index f294508185..141bcdae8d 100644
--- a/core/NumberFormatter.php
+++ b/core/NumberFormatter.php
@@ -8,8 +8,6 @@
  */
 namespace Piwik;
 
-use Piwik\Container\StaticContainer;
-
 /**
  * Class NumberFormatter
  *
@@ -148,6 +146,25 @@ class NumberFormatter extends Singleton
         return $this->formatNumberWithPattern($pattern, $newValue, $maximumFractionDigits, $minimumFractionDigits);
     }
 
+
+    /**
+     * Formats given number as percent value, but keep the leading + sign if found
+     *
+     * @param $value
+     * @return string
+     */
+    public function formatPercentEvolution($value)
+    {
+        $isPositiveEvolution = !empty($value) && ($value > 0 || $value[0] == '+');
+
+        $formatted = self::formatPercent($value);
+
+        if($isPositiveEvolution) {
+            return '+' . $formatted;
+        }
+        return $formatted;
+    }
+
     /**
      * Formats given number as percent value
      * @param string|int|float $value
diff --git a/core/Twig.php b/core/Twig.php
index 6d5b91a24c..c806dcf9bb 100755
--- a/core/Twig.php
+++ b/core/Twig.php
@@ -86,6 +86,7 @@ class Twig
         $this->addFilter_notification();
         $this->addFilter_percentage();
         $this->addFilter_percent();
+        $this->addFilter_percentEvolution();
         $this->addFilter_prettyDate();
         $this->addFilter_safeDecodeRaw();
         $this->addFilter_number();
@@ -288,6 +289,14 @@ class Twig
         $this->twig->addFilter($percentage);
     }
 
+    protected function addFilter_percentEvolution()
+    {
+        $percentage = new Twig_SimpleFilter('percentEvolution', function ($string) {
+            return NumberFormatter::getInstance()->formatPercentEvolution($string);
+        });
+        $this->twig->addFilter($percentage);
+    }
+
     protected function addFilter_number()
     {
         $formatter = new Twig_SimpleFilter('number', function ($string, $minFractionDigits = 0, $maxFractionDigits = 0) {
diff --git a/plugins/MobileMessaging/ReportRenderer/Sms.php b/plugins/MobileMessaging/ReportRenderer/Sms.php
index f3c19d77b4..96b7291240 100644
--- a/plugins/MobileMessaging/ReportRenderer/Sms.php
+++ b/plugins/MobileMessaging/ReportRenderer/Sms.php
@@ -95,7 +95,8 @@ class Sms extends ReportRenderer
                  $evolutionMetrics,
                  function ($value) use ($floatRegex) {
                      $matched = preg_match($floatRegex, $value, $matches);
-                     return $matched ? sprintf("%+d", $matches[0]) : $value;
+                     $formatted = $matched ? sprintf("%+d", $matches[0]) : $value;
+                     return \Piwik\NumberFormatter::getInstance()->formatPercentEvolution($formatted);
                  }
             )
         );
diff --git a/plugins/MobileMessaging/templates/SMSReport.twig b/plugins/MobileMessaging/templates/SMSReport.twig
index 3b234bb064..09f41ece80 100644
--- a/plugins/MobileMessaging/templates/SMSReport.twig
+++ b/plugins/MobileMessaging/templates/SMSReport.twig
@@ -12,32 +12,32 @@
 
     {# visits #}
     {{- rowMetrics.nb_visits|number }} {{ 'General_ColumnNbVisits'|translate }}
-    {%- if rowMetrics.visits_evolution != 0 %} ({{ rowMetrics.visits_evolution|percent }}){%- endif -%}
+    {%- if rowMetrics.visits_evolution != 0 %} ({{ rowMetrics.visits_evolution|percentEvolution }}){%- endif -%}
 
     {%- if rowMetrics.nb_visits != 0 -%}
         {#- actions -#}
         , {{ rowMetrics.nb_actions|number }} {{ 'General_ColumnNbActions'|translate }}
-        {%- if rowMetrics.actions_evolution != 0 %} ({{ rowMetrics.actions_evolution|percent }}){%- endif -%}
+        {%- if rowMetrics.actions_evolution != 0 %} ({{ rowMetrics.actions_evolution|percentEvolution }}){%- endif -%}
 
         {%- if isGoalPluginEnabled -%}
 
             {# goal metrics #}
             {%- if rowMetrics.nb_conversions != 0 -%}
                 , {{ 'General_ColumnRevenue'|translate }}: {{ rowMetrics.revenue|raw }}
-                {%- if rowMetrics.revenue_evolution != 0 %} ({{ rowMetrics.revenue_evolution|percent }}){%- endif -%}
+                {%- if rowMetrics.revenue_evolution != 0 %} ({{ rowMetrics.revenue_evolution|percentEvolution }}){%- endif -%}
 
                 , {{ rowMetrics.nb_conversions|number }} {{ 'Goals_GoalConversions'|translate }}
-                {%- if rowMetrics.nb_conversions_evolution != 0 %} ({{ rowMetrics.nb_conversions_evolution|percent }}){%- endif -%}
+                {%- if rowMetrics.nb_conversions_evolution != 0 %} ({{ rowMetrics.nb_conversions_evolution|percentEvolution }}){%- endif -%}
             {%- endif -%}
 
             {# eCommerce metrics #}
             {%- if siteHasECommerce[rowMetadata.idsite] -%}
 
                 , {{ 'General_ProductRevenue'|translate }}: {{ rowMetrics.ecommerce_revenue|raw }}
-                {%- if rowMetrics.ecommerce_revenue_evolution != 0 %} ({{ rowMetrics.ecommerce_revenue_evolution|percent }}){%- endif -%}
+                {%- if rowMetrics.ecommerce_revenue_evolution != 0 %} ({{ rowMetrics.ecommerce_revenue_evolution|percentEvolution }}){%- endif -%}
 
                 , {{ rowMetrics.orders|number }} {{ 'General_EcommerceOrders'|translate }}
-                {%- if rowMetrics.orders_evolution != 0 %} ({{ rowMetrics.orders_evolution|percent }}){%- endif -%}
+                {%- if rowMetrics.orders_evolution != 0 %} ({{ rowMetrics.orders_evolution|percentEvolution }}){%- endif -%}
             {%- endif -%}
 
         {%- endif -%}
diff --git a/tests/PHPUnit/Integration/NumberFormatterTest.php b/tests/PHPUnit/Integration/NumberFormatterTest.php
index 2636e61378..e2ac454b4e 100644
--- a/tests/PHPUnit/Integration/NumberFormatterTest.php
+++ b/tests/PHPUnit/Integration/NumberFormatterTest.php
@@ -82,6 +82,7 @@ class NumberFormatterTest extends \PHPUnit_Framework_TestCase
             array('en', 5.299, 3, 0, '5.299%'),
             array('en', -50, 3, 3, '-50.000%'),
             array('en', 5000, 0, 0, '5,000%'),
+            array('en', +5000, 0, 0, '5,000%'),
             array('en', 5000000, 0, 0, '5,000,000%'),
             array('en', -5000000, 0, 0, '-5,000,000%'),
 
@@ -94,4 +95,29 @@ class NumberFormatterTest extends \PHPUnit_Framework_TestCase
             array('lt', -152551239.56, 0, 0, '−152 551 239 %'),
         );
     }
+
+    /**
+     * @dataProvider getPercentNumberEvolutionFormattingTestData
+     */
+    public function testPercentEvolutionNumberFormatting($language, $value, $expected)
+    {
+        StaticContainer::get('Piwik\Translation\Translator')->setCurrentLanguage($language);
+        $formatter = NumberFormatter::getInstance();
+        $this->assertEquals($expected, $formatter->formatPercentEvolution($value));
+    }
+
+    public function getPercentNumberEvolutionFormattingTestData()
+    {
+        return array(
+            // english formats
+            array('en', 5, '+5%'),
+            array('en', -5, '-5%'),
+            array('en', 5.299, '+5%'),
+            array('en', -50, '-50%'),
+            array('en', 5000, '+5,000%'),
+            array('en', +5000, '+5,000%'),
+            array('en', 5000000, '+5,000,000%'),
+            array('en', -5000000, '-5,000,000%'),
+        );
+    }
 }