From 95bea8a4008087c4fd6261fc06542bd3137a0312 Mon Sep 17 00:00:00 2001 From: mattab Date: Mon, 12 Oct 2015 11:37:26 +1300 Subject: SMS report: percentage evolution are prefixed by '+' https://github.com/piwik/piwik/pull/8857 --- core/NumberFormatter.php | 21 ++++++++++++++++-- core/Twig.php | 9 ++++++++ plugins/MobileMessaging/ReportRenderer/Sms.php | 3 ++- plugins/MobileMessaging/templates/SMSReport.twig | 12 +++++------ tests/PHPUnit/Integration/NumberFormatterTest.php | 26 +++++++++++++++++++++++ 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%'), + ); + } } -- cgit v1.2.3