jenkins-bot has submitted this change and it was merged.

Change subject: Split code and fix precision edge case in MwTimeIsoFormatter
......................................................................


Split code and fix precision edge case in MwTimeIsoFormatter

Meant as a clean up of existing code. I started this patch with
splitting the big function.

I found an issue where this formatter incorrectly formats values
with hour, minute or second precision as a day. The issue with this
is that you can not even see the hour, minute or second. It's
supressed. This patch introduces a fallback to the full timestamp.

Change-Id: If5974e808f78a5fbac82f0eeb8d825509c9495a9
---
M lib/includes/formatters/MwTimeIsoFormatter.php
M lib/tests/phpunit/formatters/MwTimeIsoFormatterTest.php
2 files changed, 50 insertions(+), 31 deletions(-)

Approvals:
  Daniel Kinzler: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/lib/includes/formatters/MwTimeIsoFormatter.php 
b/lib/includes/formatters/MwTimeIsoFormatter.php
index bc969a2..f89e3b6 100644
--- a/lib/includes/formatters/MwTimeIsoFormatter.php
+++ b/lib/includes/formatters/MwTimeIsoFormatter.php
@@ -76,13 +76,13 @@
         * @return string Formatted date
         */
        private function getLocalizedDate( $isoTimestamp, $precision ) {
-               $dateFormat = $this->getDateFormat( $precision );
                $localizedYear = $this->getLocalizedYear( $isoTimestamp, 
$precision );
 
-               if ( $dateFormat === 'Y' ) {
+               if ( $precision <= TimeValue::PRECISION_YEAR ) {
                        return $localizedYear;
                }
 
+               $dateFormat = $this->getDateFormat( $precision );
                $mwTimestamp = $this->getMwTimestamp( $isoTimestamp, $precision 
);
                $mwYear = $this->language->sprintfDate( 'Y', $mwTimestamp );
                $localizedDate = $this->language->sprintfDate( $dateFormat, 
$mwTimestamp );
@@ -103,17 +103,18 @@
        /**
         * @param int $precision
         *
+        * @throws InvalidArgumentException
         * @return string Date format string to be used by Language::sprintfDate
         */
        private function getDateFormat( $precision ) {
-               if ( $precision <= TimeValue::PRECISION_YEAR ) {
-                       return 'Y';
-               } elseif ( $precision === TimeValue::PRECISION_MONTH ) {
+               if ( $precision === TimeValue::PRECISION_MONTH ) {
                        $format = $this->language->getDateFormatString( 
'monthonly', 'dmy' );
                        return sprintf( '%s Y', $this->getMonthFormat( $format 
) );
-               } else {
+               } elseif ( $precision === TimeValue::PRECISION_DAY ) {
                        $format = $this->language->getDateFormatString( 'date', 
'dmy' );
                        return sprintf( '%s %s Y', $this->getDayFormat( $format 
), $this->getMonthFormat( $format ) );
+               } else {
+                       throw new InvalidArgumentException( 'Unsupported 
precision' );
                }
        }
 
@@ -198,7 +199,6 @@
         * @param string $isoTimestamp
         * @param int $precision
         *
-        * @throws InvalidArgumentException
         * @return string
         */
        private function getLocalizedYear( $isoTimestamp, $precision ) {
@@ -206,8 +206,8 @@
                list( , $sign, $year ) = $matches;
                $isBCE = $sign === '-';
 
-               $shift = 1e+0;
-               $unshift = 1e+0;
+               $shift = 1;
+               $unshift = 1;
                $func = 'round';
 
                switch ( $precision ) {
@@ -257,28 +257,16 @@
                                break;
                }
 
-               if ( $shift !== 1e+0 || $unshift !== 1e+0 ) {
-                       switch ( $func ) {
-                               case 'ceil':
-                                       $shifted = ceil( $year / $shift ) * 
$unshift;
-                                       break;
-                               case 'floor':
-                                       $shifted = floor( $year / $shift ) * 
$unshift;
-                                       break;
-                               default:
-                                       $shifted = round( $year / $shift ) * 
$unshift;
-                       }
-
-                       // Year to small for precision, fall back to year
-                       if ( $shifted == 0
-                               && ( $precision < TimeValue::PRECISION_YEAR
-                                       || ( $isBCE && $precision === 
TimeValue::PRECISION_YEAR )
-                               )
-                       ) {
-                               $msg = null;
-                       } else {
-                               $year = sprintf( '%.0f', $shifted );
-                       }
+               $shifted = $this->shiftNumber( $year, $func, $shift, $unshift );
+               if ( $shifted == 0
+                       && ( $precision < TimeValue::PRECISION_YEAR
+                               || ( $isBCE && $precision === 
TimeValue::PRECISION_YEAR )
+                       )
+               ) {
+                       // Year to small for precision, fall back to year.
+                       $msg = null;
+               } else {
+                       $year = $shifted;
                }
 
                $year = str_pad( ltrim( $year, '0' ), 1, '0', STR_PAD_LEFT );
@@ -295,6 +283,33 @@
        }
 
        /**
+        * @param string $number
+        * @param string $function
+        * @param float $shift
+        * @param float $unshift
+        *
+        * @return string
+        */
+       private function shiftNumber( $number, $function, $shift, $unshift ) {
+               if ( $shift == 1 && $unshift == 1 ) {
+                       return $number;
+               }
+
+               switch ( $function ) {
+                       case 'ceil':
+                               $shifted = ceil( $number / $shift ) * $unshift;
+                               break;
+                       case 'floor':
+                               $shifted = floor( $number / $shift ) * $unshift;
+                               break;
+                       default:
+                               $shifted = round( $number / $shift ) * $unshift;
+               }
+
+               return sprintf( '%.0f', $shifted );
+       }
+
+       /**
         * @param string $key
         * @param string $param
         *
diff --git a/lib/tests/phpunit/formatters/MwTimeIsoFormatterTest.php 
b/lib/tests/phpunit/formatters/MwTimeIsoFormatterTest.php
index 0277353..a20ad81 100644
--- a/lib/tests/phpunit/formatters/MwTimeIsoFormatterTest.php
+++ b/lib/tests/phpunit/formatters/MwTimeIsoFormatterTest.php
@@ -439,6 +439,10 @@
                                '-1-00-00T00:00:00Z', 
TimeValue::PRECISION_YEAR10,
                                '1 BCE',
                        ),
+                       array(
+                               '+2015-01-01T01:00:00Z', 
TimeValue::PRECISION_HOUR,
+                               '+2015-01-01T01:00:00Z',
+                       ),
 
                        // Better than the raw ISO string
                        array(

-- 
To view, visit https://gerrit.wikimedia.org/r/248900
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If5974e808f78a5fbac82f0eeb8d825509c9495a9
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to