From 7cb2b0ca40f63a56c7397cf32ffdb4be402eab20 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 10 Oct 2024 21:19:43 +0800 Subject: [PATCH] MDL-83412 calendartype_gregorian: Use core_date Update timestamp_to_date_string to use core_date. --- admin/tool/uploadcourse/tests/course_test.php | 2 +- calendar/tests/calendartype_test.php | 6 + calendar/type/gregorian/classes/structure.php | 80 +++++--- .../type/gregorian/tests/structure_test.php | 193 ++++++++++++++++++ lib/phpunit/classes/advanced_testcase.php | 14 ++ lib/tests/classes/mocking_string_manager.php | 47 +++++ lib/xapi/tests/privacy/provider_test.php | 9 +- 7 files changed, 312 insertions(+), 39 deletions(-) create mode 100644 calendar/type/gregorian/tests/structure_test.php create mode 100644 lib/tests/classes/mocking_string_manager.php diff --git a/admin/tool/uploadcourse/tests/course_test.php b/admin/tool/uploadcourse/tests/course_test.php index 3da6389681f..e5fc520504e 100644 --- a/admin/tool/uploadcourse/tests/course_test.php +++ b/admin/tool/uploadcourse/tests/course_test.php @@ -1504,7 +1504,7 @@ class course_test extends \advanced_testcase { // Create our custom field. $category = $this->get_customfield_generator()->create_category(); $this->create_custom_field($category, 'date', 'mydate', - ['mindate' => strtotime('2020-04-01'), 'maxdate' => '2020-04-30']); + ['mindate' => strtotime('2020-04-01'), 'maxdate' => strtotime('2020-04-30')]); $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY; $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY; diff --git a/calendar/tests/calendartype_test.php b/calendar/tests/calendartype_test.php index 5ed64b18cda..dcd957331e6 100644 --- a/calendar/tests/calendartype_test.php +++ b/calendar/tests/calendartype_test.php @@ -196,6 +196,12 @@ class calendartype_test extends \advanced_testcase { $this->assertEquals($calendar->timestamp_to_date_string($this->user->timecreated, '', 99, true, true), userdate($this->user->timecreated)); + // Test the userdate function with a timezone. + $this->assertEquals( + $calendar->timestamp_to_date_string($this->user->timecreated, '', 'Australia/Sydney', true, true), + userdate($this->user->timecreated, timezone: 'Australia/Sydney'), + ); + // Test the calendar/lib.php functions. $this->assertEquals($calendar->get_weekdays(), calendar_get_days()); $this->assertEquals($calendar->get_starting_weekday(), calendar_get_starting_weekday()); diff --git a/calendar/type/gregorian/classes/structure.php b/calendar/type/gregorian/classes/structure.php index a01e84b1c1d..093a7b142e6 100644 --- a/calendar/type/gregorian/classes/structure.php +++ b/calendar/type/gregorian/classes/structure.php @@ -276,13 +276,6 @@ class structure extends type_base { /** * Returns a formatted string that represents a date in user time. * - * Returns a formatted string that represents a date in user time - * WARNING: note that the format is for strftime(), not date(). - * Because of a bug in most Windows time libraries, we can't use - * the nicer %e, so we have to use %d which has leading zeroes. - * A lot of the fuss in the function is just getting rid of these leading - * zeroes as efficiently as possible. - * * If parameter fixday = true (default), then take off leading * zero from %d, else maintain it. * @@ -303,43 +296,62 @@ class structure extends type_base { $format = get_string('strftimedaydatetime', 'langconfig'); } - if (!empty($CFG->nofixday)) { // Config.php can force %d not to be fixed. - $fixday = false; - } else if ($fixday) { - $formatnoday = str_replace('%d', 'DD', $format); - $fixday = ($formatnoday != $format); - $format = $formatnoday; + // Note: This historical logic was about fixing 12-hour time to remove + // unnecessary leading zero was required because on Windows, PHP strftime + // function did not support the correct 'hour without leading zero' parameter (%l). + // This is no longer required because we use IntlDateFormatter. + // Unfortunately though the original implementation was done incorrectly. + // The documentation for strftime notes that for the "%l" and "%e" specifiers where + // no leading zero is used, a space is used instead. + // As a result we switch to the new format specifiers "%l" and "%e", wrap them in placeholders + // and then remove the spaces + + if (empty($CFG->nofixday) && $fixday) { + // Config.php can force %d not to be fixed, but only if the format did not specify it. + $format = str_replace( + '%d', + 'DDHH%eHHDD', + $format, + ); } - // Note: This logic about fixing 12-hour time to remove unnecessary leading - // zero is required because on Windows, PHP strftime function does not - // support the correct 'hour without leading zero' parameter (%l). - if (!empty($CFG->nofixhour)) { - // Config.php can force %I not to be fixed. - $fixhour = false; - } else if ($fixhour) { - $formatnohour = str_replace('%I', 'HH', $format); - $fixhour = ($formatnohour != $format); - $format = $formatnohour; + if (empty($CFG->nofixhour) && $fixhour) { + $format = str_replace( + '%I', + 'DDHH%lHHDD', + $format, + ); } - $time = (int)$time; // Moodle allows rubbish in input... - $datestring = date_format_string($time, $format, $timezone); + if (is_string($time) && !is_numeric($time)) { + debugging( + "Invalid time passed to timestamp_to_date_string: '{$time}'", + DEBUG_DEVELOPER, + ); + $time = 0; + } + + if ($time === null || $time === '') { + $time = 0; + } + + $time = new \DateTime("@{$time}", new \DateTimeZone(date_default_timezone_get())); date_default_timezone_set(\core_date::get_user_timezone($timezone)); - if ($fixday) { - $daystring = ltrim(str_replace(array(' 0', ' '), '', date(' d', $time))); - $datestring = str_replace('DD', $daystring, $datestring); - } - if ($fixhour) { - $hourstring = ltrim(str_replace(array(' 0', ' '), '', date(' h', $time))); - $datestring = str_replace('HH', $hourstring, $datestring); - } + $formattedtime = \core_date::strftime( + $format, + $time, + get_string('locale', 'langconfig'), + ); \core_date::set_default_server_timezone(); - return $datestring; + // Use a simple regex to remove the placeholders and any leading spaces to match the historically + // generated format. + $formattedtime = preg_replace('/DDHH ?(\d{1,2})HHDD/', '$1', $formattedtime); + + return $formattedtime; } /** diff --git a/calendar/type/gregorian/tests/structure_test.php b/calendar/type/gregorian/tests/structure_test.php new file mode 100644 index 00000000000..94a6ef14b5e --- /dev/null +++ b/calendar/type/gregorian/tests/structure_test.php @@ -0,0 +1,193 @@ +. + +namespace calendartype_gregorian; + +/** + * Tests for Gregorian calendar type + * + * @package calendartype_gregorian + * @category test + * @copyright Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \calendartype_gregorian\structure + */ +final class structure_test extends \advanced_testcase { + public function tearDown(): void { + parent::tearDown(); + + get_string_manager(true); + } + + /** + * Test the timestamp_to_date_string method with different input values. + * + * @dataProvider timestamp_to_date_string_provider + * @param string $lang + * @param int $timestamp + * @param string $format + * @param string $timezone + * @param bool $fixday + * @param bool $fixhour + * @param string $expected + */ + public function test_timestamp_to_date_string( + string $locale, + int $timestamp, + string $format, + string $timezone, + bool $fixday, + bool $fixhour, + string $expected, + ): void { + $this->resetAfterTest(); + + $stringmanager = $this->get_mocked_string_manager(); + $stringmanager->mock_string('locale', 'langconfig', $locale); + + $structure = new structure(); + $this->assertEquals( + $expected, + $structure->timestamp_to_date_string( + $timestamp, + $format, + $timezone, + $fixday, + $fixhour, + ), + ); + } + + /** + * Data provider for timestamp_to_date_string tests. + * + * @return array + */ + public static function timestamp_to_date_string_provider(): array { + return [ + 'English with UTC timezone' => [ + 'en', + 0, + '%Y-%m-%d %H:%M:%S', + 'UTC', + false, + false, + '1970-01-01 00:00:00', + ], + 'English with London timezone' => [ + 'en', + 1728487003, + "%d %B %Y", + 'Europe/London', + false, + false, + "09 October 2024", + ], + 'English with Sydney (+11) timezone' => [ + 'en', + 1728487003, + "%d %B %Y", + 'Australia/Sydney', + false, + false, + "10 October 2024", + ], + 'Russian with Sydney (+11) timezone' => [ + 'ru', + 1728487003, + "%d %B %Y %H:%M:%S", + 'Australia/Sydney', + false, + false, + '10 октября 2024 02:16:43', + ], + 'Russian %B %Y (Genitive) with Sydney (+11) timezone' => [ + 'ru', + 1728487003, + "%B %Y", + 'Australia/Sydney', + false, + false, + "октябрь 2024", + ], + 'Russian %d %B %Y (Nominative) with London timezone' => [ + 'ru', + 1728487003, + "%d %B %Y", + 'Europe/London', + false, + false, + "09 октября 2024", + ], + 'Russian %d %B %Y (Nominative) with London timezone fixing leading zero' => [ + 'ru', + 1728487003, + "%d %B %Y", + 'Europe/London', + true, + false, + "9 октября 2024", + ], + 'Russian %e %B %Y (Nominative) with London timezone' => [ + 'ru', + 1728487003, + "%e %B %Y", + 'Europe/London', + false, + false, + " 9 октября 2024", + ], + 'Time %I without fixing leading zero' => [ + 'ru', + 1728487003, + "%I:%M:%S", + 'Australia/Sydney', + false, + false + , + "02:16:43", + ], + 'Time %I fixing leading zero' => [ + 'ru', + 1728487003, + "%I:%M:%S", + 'Australia/Sydney', + false, + true + , + "2:16:43", + ], + 'Time %l without fixing leading zero' => [ + 'ru', + 1728487003, + "%l:%M:%S", + 'Australia/Sydney', + false, + false, + " 2:16:43", + ], + 'Time %l fixing leading zero' => [ + 'ru', + 1728487003, + "%l:%M:%S", + 'Australia/Sydney', + false, + true, + " 2:16:43", + ], + ]; + } +} diff --git a/lib/phpunit/classes/advanced_testcase.php b/lib/phpunit/classes/advanced_testcase.php index aa7512f36df..f7a1a357885 100644 --- a/lib/phpunit/classes/advanced_testcase.php +++ b/lib/phpunit/classes/advanced_testcase.php @@ -912,4 +912,18 @@ abstract class advanced_testcase extends base_testcase { 'handlerstack' => $handlerstack, ]; } + + /** + * Get a copy of the mocked string manager. + * + * @return \core\tests\mocking_string_manager + */ + protected function get_mocked_string_manager(): \core\tests\mocking_string_manager { + global $CFG; + + $this->resetAfterTest(); + $CFG->config_php_settings['customstringmanager'] = \core\tests\mocking_string_manager::class; + + return get_string_manager(true); + } } diff --git a/lib/tests/classes/mocking_string_manager.php b/lib/tests/classes/mocking_string_manager.php new file mode 100644 index 00000000000..d932a701bf1 --- /dev/null +++ b/lib/tests/classes/mocking_string_manager.php @@ -0,0 +1,47 @@ +. + +namespace core\tests; + +/** + * A string manager which supports mocking individaul strings. + * + * @package core + * @copyright Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + + class mocking_string_manager extends \core_string_manager_standard { + /** @var array The list of strings */ + private $strings = []; + + #[\Override] + public function get_string($identifier, $component = '', $a = null, $lang = null) { + if (isset($this->strings["{$component}/{$identifier}"])) { + return $this->strings["{$component}/{$identifier}"]; + } + + return parent::get_string($identifier, $component, $a, $lang); + } + + public function mock_string( + string $identifier, + string $component, + string $value, + ): void { + $this->strings["{$component}/{$identifier}"] = $value; + } +} diff --git a/lib/xapi/tests/privacy/provider_test.php b/lib/xapi/tests/privacy/provider_test.php index 94e6535da7b..bd8e5ae38f4 100644 --- a/lib/xapi/tests/privacy/provider_test.php +++ b/lib/xapi/tests/privacy/provider_test.php @@ -158,16 +158,17 @@ class provider_test extends provider_testcase { $info = (object) reset($result); // Ensure the correct data has been returned. $this->assertNotEmpty($info->statedata); - $this->assertNotEmpty(transform::datetime($info->timecreated)); - $this->assertNotEmpty(transform::datetime($info->timemodified)); + + $this->assertNotEmpty($info->timecreated); + $this->assertNotEmpty($info->timemodified); // Get the states info for user2 in the system context. $result = provider::get_xapi_states_for_user($user2->id, 'fake_component', $systemcontext->id); $info = (object) reset($result); // Ensure the correct data has been returned. $this->assertNotEmpty($info->statedata); - $this->assertNotEmpty(transform::datetime($info->timecreated)); - $this->assertNotEmpty(transform::datetime($info->timemodified)); + $this->assertNotEmpty($info->timecreated); + $this->assertNotEmpty($info->timemodified); // Get the states info for user3 in the system context (it should be empty). $info = provider::get_xapi_states_for_user($user3->id, 'fake_component', $systemcontext->id);