diff options
| -rw-r--r-- | core/Metrics/Formatter.php | 34 | ||||
| -rw-r--r-- | plugins/Overlay/Controller.php | 3 | ||||
| -rw-r--r-- | tests/PHPUnit/Integration/PiwikTest.php | 83 | ||||
| -rw-r--r-- | tests/PHPUnit/Unit/Metrics/Formatter/HtmlTest.php | 91 | ||||
| -rw-r--r-- | tests/PHPUnit/Unit/Metrics/FormatterTest.php | 232 |
5 files changed, 342 insertions, 101 deletions
diff --git a/core/Metrics/Formatter.php b/core/Metrics/Formatter.php index ac3c6f0278..44103d7e27 100644 --- a/core/Metrics/Formatter.php +++ b/core/Metrics/Formatter.php @@ -23,6 +23,9 @@ use Piwik\Tracker\GoalManager; */ class Formatter { + private $decimalPoint = null; + private $thousandsSeparator = null; + /** * Returns a prettified string representation of a number. The result will have * thousands separators and a decimal point specific to the current locale, eg, @@ -31,19 +34,16 @@ class Formatter * @param number $value * @return string */ - public function getPrettyNumber($value) + public function getPrettyNumber($value, $precision = 0) { - static $decimalPoint = null; - static $thousandsSeparator = null; - - if ($decimalPoint === null) { + if ($this->decimalPoint === null) { $locale = localeconv(); - $decimalPoint = $locale['decimal_point']; - $thousandsSeparator = $locale['thousands_sep']; + $this->decimalPoint = $locale['decimal_point']; + $this->thousandsSeparator = $locale['thousands_sep']; } - return number_format($value, 0, $decimalPoint, $thousandsSeparator); + return number_format($value, $precision, $this->decimalPoint, $this->thousandsSeparator); } /** @@ -51,7 +51,6 @@ class Formatter * * @param int $numberOfSeconds The number of seconds. * @param bool $displayTimeAsSentence If set to true, will output `"5min 17s"`, if false `"00:05:17"`. - * @param bool $isHtml If true, replaces all spaces with `' '`. * @param bool $round Whether to round to the nearest second or not. * @return string */ @@ -132,8 +131,8 @@ class Formatter $units = array('B', 'K', 'M', 'G', 'T'); $currentUnit = null; - foreach ($units as $currentUnit) { - if ($size >= 1024 && $unit != $currentUnit) { + foreach ($units as $idx => $currentUnit) { + if ($size >= 1024 && $unit != $currentUnit && $idx != count($units) - 1) { $size = $size / 1024; } else { break; @@ -148,22 +147,23 @@ class Formatter * * @param int|string $value The monetary value to format. * @param int $idSite The ID of the site whose currency will be used. - * @param bool $isHtml If true, replaces all spaces with `' '`. * @return string */ public function getPrettyMoney($value, $idSite) { - $currencyBefore = self::getCurrencySymbol($idSite); - $space = ' '; + $currencySymbol = self::getCurrencySymbol($idSite); + + $currencyBefore = $currencySymbol . $space; $currencyAfter = ''; + // (maybe more currencies prefer this notation?) $currencySymbolToAppend = array('€', 'kr', 'zł'); // manually put the currency symbol after the amount - if (in_array($currencyBefore, $currencySymbolToAppend)) { - $currencyAfter = $space . $currencyBefore; + if (in_array($currencySymbol, $currencySymbolToAppend)) { + $currencyAfter = $space . $currencySymbol; $currencyBefore = ''; } @@ -179,7 +179,7 @@ class Formatter } } - $prettyMoney = $currencyBefore . $space . $value . $currencyAfter; + $prettyMoney = $currencyBefore . $value . $currencyAfter; return $prettyMoney; } diff --git a/plugins/Overlay/Controller.php b/plugins/Overlay/Controller.php index 603047c286..d55ec7e6e9 100644 --- a/plugins/Overlay/Controller.php +++ b/plugins/Overlay/Controller.php @@ -76,7 +76,8 @@ class Controller extends \Piwik\Plugin\Controller ); $dataTable = $request->process(); - $formatter = new Metrics\Formatter(); + // TODO: move metric formatting logic to Formatter\Html from DataTablePostProcessor + $formatter = new Metrics\Formatter\Html(); $data = array(); if ($dataTable->getRowsCount() > 0) { diff --git a/tests/PHPUnit/Integration/PiwikTest.php b/tests/PHPUnit/Integration/PiwikTest.php index 252c08cd78..6043736e04 100644 --- a/tests/PHPUnit/Integration/PiwikTest.php +++ b/tests/PHPUnit/Integration/PiwikTest.php @@ -9,8 +9,6 @@ namespace Piwik\Tests\Integration; use Piwik\Access; -use Piwik\Filesystem; -use Piwik\MetricsFormatter; use Piwik\Piwik; use Piwik\Plugins\SitesManager\API; use Piwik\Translate; @@ -80,52 +78,6 @@ class PiwikTest extends IntegrationTestCase } /** - * Dataprovider for testGetPrettyTimeFromSeconds - */ - public function getPrettyTimeFromSecondsData() - { - return array( - array(30, array('30s', '00:00:30')), - array(60, array('1 min 0s', '00:01:00')), - array(100, array('1 min 40s', '00:01:40')), - array(3600, array('1 hours 0 min', '01:00:00')), - array(3700, array('1 hours 1 min', '01:01:40')), - array(86400 + 3600 * 10, array('1 days 10 hours', '34:00:00')), - array(86400 * 365, array('365 days 0 hours', '8760:00:00')), - array((86400 * (365.25 + 10)), array('1 years 10 days', '9006:00:00')), - array(1.342, array('1.34s', '00:00:01.34')), - array(.342, array('0.34s', '00:00:00.34')), - array(.02, array('0.02s', '00:00:00.02')), - array(.002, array('0.002s', '00:00:00')), - array(1.002, array('1s', '00:00:01')), - array(1.02, array('1.02s', '00:00:01.02')), - array(1.2, array('1.2s', '00:00:01.20')), - array(122.1, array('2 min 2.1s', '00:02:02.10')), - array(-122.1, array('-2 min 2.1s', '-00:02:02.10')), - array(86400 * -365, array('-365 days 0 hours', '-8760:00:00')) - ); - } - - /** - * @dataProvider getPrettyTimeFromSecondsData - */ - public function testGetPrettyTimeFromSeconds($seconds, $expected) - { - if (($seconds * 100) > PHP_INT_MAX) { - $this->markTestSkipped("Will not pass on 32-bit machine."); - } - - Translate::loadEnglishTranslation(); - - $sentenceExpected = str_replace(' ', ' ', $expected[0]); - $numericExpected = $expected[1]; - $this->assertEquals($sentenceExpected, MetricsFormatter::getPrettyTimeFromSeconds($seconds, $sentence = true)); - $this->assertEquals($numericExpected, MetricsFormatter::getPrettyTimeFromSeconds($seconds, $sentence = false)); - - Translate::unloadEnglishTranslation(); - } - - /** * Dataprovider for testCheckValidLoginString */ public function getInvalidLoginStringData() @@ -184,41 +136,6 @@ class PiwikTest extends IntegrationTestCase } /** - * Dataprovider for testGetPrettyValue - */ - public function getGetPrettyValueTestCases() - { - return array( - array('revenue', 12, '$ 12'), - array('revenue_evolution', '100 %', '100 %'), - array('avg_time_generation', '3.333', '3.33s'), - array('avg_time_generation', '333.333', '5 min 33.33s'), - array('avg_time_on_page', '3', '00:00:03'), - array('avg_time_on_page', '333', '00:05:33'), - ); - } - - /** - * @dataProvider getGetPrettyValueTestCases - */ - public function testGetPrettyValue($columnName, $value, $expected) - { - Translate::loadEnglishTranslation(); - - $access = Access::getInstance(); - $access->setSuperUserAccess(true); - - $idsite = API::getInstance()->addSite("test", "http://test"); - - $this->assertEquals( - $expected, - MetricsFormatter::getPrettyValue($idsite, $columnName, $value, false, false) - ); - - Translate::unloadEnglishTranslation(); - } - - /** * Data provider for testIsAssociativeArray. */ public function getIsAssociativeArrayTestCases() diff --git a/tests/PHPUnit/Unit/Metrics/Formatter/HtmlTest.php b/tests/PHPUnit/Unit/Metrics/Formatter/HtmlTest.php new file mode 100644 index 0000000000..fd9b996f50 --- /dev/null +++ b/tests/PHPUnit/Unit/Metrics/Formatter/HtmlTest.php @@ -0,0 +1,91 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ +namespace Piwik\Tests\Unit\Metrics\Formatter; + +use Piwik\Metrics\Formatter\Html; +use Piwik\Translate; +use Piwik\Plugins\SitesManager\API as SitesManagerAPI; + +/** + * @group Core + */ +class HtmlTest extends \PHPUnit_Framework_TestCase +{ + /** + * @var Html + */ + private $formatter; + + private $sitesInfo; + + public function setUp() + { + $this->sitesInfo = array( + 1 => array( + 'idsite' => '1', + 'currency' => 'EUR' + ) + ); + + $this->formatter = new Html(); + + setlocale(LC_ALL, null); + + Translate::loadEnglishTranslation(); + $this->setSiteManagerApiMock(); + } + + public function tearDown() + { + Translate::unloadEnglishTranslation(); + $this->unsetSiteManagerApiMock(); + + setlocale(LC_ALL, null); + } + + public function test_getPrettyTimeFromSeconds_DefaultsToShowingSentences_AndUsesNonBreakingSpaces() + { + $expected = '1 days 10 hours'; + $value = $this->formatter->getPrettyTimeFromSeconds(86400 + 3600 * 10); + + $this->assertEquals($expected, $value); + } + + public function test_getPrettySizeFromBytes_UsesNonBreakingSpaces() + { + $expected = '1.5 K'; + $value = $this->formatter->getPrettySizeFromBytes(1536); + + $this->assertEquals($expected, $value); + } + + public function test_getPrettyMoney_UsesNonBreakingSpaces() + { + $expected = '1 €'; + $value = $this->formatter->getPrettyMoney(1, 1); + + $this->assertEquals($expected, $value); + } + + private function unsetSiteManagerApiMock() + { + SitesManagerAPI::unsetInstance(); + } + + private function setSiteManagerApiMock() + { + $sitesInfo = $this->sitesInfo; + + $mock = $this->getMock('stdClass', array('getSiteFromId')); + $mock->expects($this->any())->method('getSiteFromId')->willReturnCallback(function ($idSite) use ($sitesInfo) { + return $sitesInfo[$idSite]; + }); + + SitesManagerAPI::setSingletonInstance($mock); + } +}
\ No newline at end of file diff --git a/tests/PHPUnit/Unit/Metrics/FormatterTest.php b/tests/PHPUnit/Unit/Metrics/FormatterTest.php new file mode 100644 index 0000000000..8ef4831299 --- /dev/null +++ b/tests/PHPUnit/Unit/Metrics/FormatterTest.php @@ -0,0 +1,232 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ +namespace Piwik\Tests\Unit\Metrics; + +use Piwik\Metrics\Formatter; +use Piwik\Translate; +use Piwik\Plugins\SitesManager\API as SitesManagerAPI; + +/** + * @group Core + */ +class FormatterTest extends \PHPUnit_Framework_TestCase +{ + /** + * @var Formatter + */ + private $formatter; + + private $sitesInfo; + + public function setUp() + { + $this->sitesInfo = array( + 1 => array( + 'idsite' => '1', + 'currency' => 'EUR' + ), + 2 => array( + 'idsite' => '2', + 'currency' => 'DKK' + ), + 3 => array( + 'idsite' => '3', + 'currency' => 'PLN' + ), + 4 => array( + 'idsite' => '4', + 'currency' => 'NZD' + ), + 5 => array( + 'idsite' => '5', + 'currency' => 'JPY' + ) + ); + + $this->formatter = new Formatter(); + + setlocale(LC_ALL, null); + + Translate::loadEnglishTranslation(); + $this->setSiteManagerApiMock(); + } + + public function tearDown() + { + Translate::unloadEnglishTranslation(); + $this->unsetSiteManagerApiMock(); + + setlocale(LC_ALL, null); + } + + /** + * @dataProvider getPrettyNumberTestData + */ + public function test_getPrettyNumber_ReturnsCorrectResult($number, $expected) + { + $this->assertEquals($expected, $this->formatter->getPrettyNumber($number, 2)); + } + + /** + * @dataProvider getPrettyNumberLocaleTestData + */ + public function test_getPrettyNumber_ReturnsCorrectResult_WhenLocaleIsEuropean($number, $expected) + { + $locale = setlocale(LC_ALL, array('de', 'de_DE', 'ge', 'de_DE.utf8')); + if (empty($locale)) { + $this->markTestSkipped("de_DE locale is not present on this system"); + } + + $this->assertEquals($expected, $this->formatter->getPrettyNumber($number, 2)); + } + + /** + * @dataProvider getPrettySizeFromBytesTestData + */ + public function test_getPrettySizeFromBytes_ReturnsCorrectResult($bytesSize, $unit, $expected) + { + $this->assertEquals($expected, $this->formatter->getPrettySizeFromBytes($bytesSize, $unit)); + } + + /** + * @dataProvider getPrettyMoneyTestData + */ + public function test_getPrettyMoney_ReturnsCorrectResult($value, $idSite, $expected) + { + $this->assertEquals($expected, $this->formatter->getPrettyMoney($value, $idSite)); + } + + /** + * @dataProvider getPrettyPercentFromQuotientTestData + */ + public function test_getPrettyPercentFromQuotient_ReturnsCorrectResult($value, $expected) + { + $this->assertEquals($expected, $this->formatter->getPrettyPercentFromQuotient($value)); + } + + /** + * @dataProvider getPrettyTimeFromSecondsData + */ + public function test_getPrettyTimeFromSeconds_ReturnsCorrectResult($seconds, $expected) + { + if (($seconds * 100) > PHP_INT_MAX) { + $this->markTestSkipped("Will not pass on 32-bit machine."); + } + + $sentenceExpected = $expected[0]; + $this->assertEquals($sentenceExpected, $this->formatter->getPrettyTimeFromSeconds($seconds, $sentence = true)); + + $numericExpected = $expected[1]; + $this->assertEquals($numericExpected, $this->formatter->getPrettyTimeFromSeconds($seconds, $sentence = false)); + } + + public function getPrettyNumberTestData() + { + return array( + array(0.14, '0.14'), + array(0.14567, '0.15'), + array(100.1234, '100.12'), + array(1000.45, '1,000.45'), + array(23456789.00, '23,456,789.00') + ); + } + + public function getPrettyNumberLocaleTestData() + { + return array( + array(0.14, '0,14'), + array(0.14567, '0,15'), + array(100.1234, '100,12'), + array(1000.45, '1.000,45'), + array(23456789.00, '23.456.789,00') + ); + } + + public function getPrettySizeFromBytesTestData() + { + return array( + array(767, null, '767 B'), + array(1024, null, '1 K'), + array(1536, null, '1.5 K'), + array(1024 * 1024, null, '1 M'), + array(1.25 * 1024 * 1024, null, '1.3 M'), + array(1.25 * 1024 * 1024 * 1024, null, '1.3 G'), + array(1.25 * 1024 * 1024 * 1024 * 1024, null, '1.3 T'), + array(1.25 * 1024 * 1024 * 1024 * 1024 * 1024, null, '1280 T'), + array(1.25 * 1024 * 1024, 'M', '1.3 M'), + array(1.25 * 1024 * 1024 * 1024, 'M', '1280 M'), + array(0, null, '0 M') + ); + } + + public function getPrettyMoneyTestData() + { + return array( + array(1, 1, '1 €'), + array(1.045, 2, '1.04 kr'), + array(1000.4445, 3, '1000.44 zł'), + array(1234.56, 4, '$ 1234.56'), + array(234.76, 5, '¥ 234.76') + ); + } + + public function getPrettyPercentFromQuotientTestData() + { + return array( + array(100, '10000%'), + array(1, '100%'), + array(.85, '85%'), + array(.89999, '89.999%'), + array(.0004, '0.04%') + ); + } + + /** + * Dataprovider for testGetPrettyTimeFromSeconds + */ + public function getPrettyTimeFromSecondsData() + { + return array( + array(30, array('30s', '00:00:30')), + array(60, array('1 min 0s', '00:01:00')), + array(100, array('1 min 40s', '00:01:40')), + array(3600, array('1 hours 0 min', '01:00:00')), + array(3700, array('1 hours 1 min', '01:01:40')), + array(86400 + 3600 * 10, array('1 days 10 hours', '34:00:00')), + array(86400 * 365, array('365 days 0 hours', '8760:00:00')), + array((86400 * (365.25 + 10)), array('1 years 10 days', '9006:00:00')), + array(1.342, array('1.34s', '00:00:01.34')), + array(.342, array('0.34s', '00:00:00.34')), + array(.02, array('0.02s', '00:00:00.02')), + array(.002, array('0.002s', '00:00:00')), + array(1.002, array('1s', '00:00:01')), + array(1.02, array('1.02s', '00:00:01.02')), + array(1.2, array('1.2s', '00:00:01.20')), + array(122.1, array('2 min 2.1s', '00:02:02.10')), + array(-122.1, array('-2 min 2.1s', '-00:02:02.10')), + array(86400 * -365, array('-365 days 0 hours', '-8760:00:00')) + ); + } + + private function unsetSiteManagerApiMock() + { + SitesManagerAPI::unsetInstance(); + } + + private function setSiteManagerApiMock() + { + $sitesInfo = $this->sitesInfo; + + $mock = $this->getMock('stdClass', array('getSiteFromId')); + $mock->expects($this->any())->method('getSiteFromId')->willReturnCallback(function ($idSite) use ($sitesInfo) { + return $sitesInfo[$idSite]; + }); + + SitesManagerAPI::setSingletonInstance($mock); + } +}
\ No newline at end of file |