L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'MDL-82041' of https://github.com/paulholden/moodle

Browse Source
This commit is contained in:
Sara Arjona
2024-06-10 15:17:11 +02:00
parent 8382970cd0 a33e54c08f
commit cbee116728
8 changed files with 191 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.upgradenotes/MDL-82041-2024053011364585.yml
+13
View File
@@ -0,0 +1,13 @@
issueNumber: MDL-82041
notes:
core_reportbuilder:
- message: >-
All time related code has been updated to the PSR-20 Clock interface, as
such the following methods no longer accept a `$timenow` parameter
(instead please use `\core\clock` dependency injection):
- `core_reportbuilder_generator::create_schedule`
-
`core_reportbuilder\local\helpers\schedule::[create_schedule|calculate_next_send_time]`
type: changed
reportbuilder/classes/local/filters/date.php
+5 -5
View File
@@ -18,10 +18,10 @@ declare(strict_types=1);
namespace core_reportbuilder\local\filters;
use DateTimeImmutable;
use core\{clock, di};
use core_reportbuilder\local\helpers\database;
use lang_string;
use MoodleQuickForm;
use core_reportbuilder\local\helpers\database;
/**
* Date report filter
@@ -246,12 +246,12 @@ class date extends base {
case self::DATE_PAST:
$param = database::generate_param_name();
$sql = "{$fieldsql} < :{$param}";
$params[$param] = time();
$params[$param] = di::get(clock::class)->time();
break;
case self::DATE_FUTURE:
$param = database::generate_param_name();
$sql = "{$fieldsql} > :{$param}";
$params[$param] = time();
$params[$param] = di::get(clock::class)->time();
break;
default:
// Invalid or inactive filter.
@@ -271,7 +271,7 @@ class date extends base {
*/
private static function get_relative_timeframe(int $operator, int $dateunitvalue, int $dateunit): array {
// Initialise start/end time to now.
$datestart = $dateend = new DateTimeImmutable();
$datestart = $dateend = di::get(clock::class)->now();
switch ($dateunit) {
case self::DATE_UNIT_HOUR:
reportbuilder/classes/local/helpers/schedule.php
+15 -7
View File
@@ -19,6 +19,7 @@ declare(strict_types=1);
namespace core_reportbuilder\local\helpers;
use context_user;
use core\{clock, di};
use core_user;
use invalid_parameter_exception;
use stdClass;
@@ -43,14 +44,18 @@ class schedule {
* Create report schedule, calculate when it should be next sent
*
* @param stdClass $data
* @param int|null $timenow Time to use as comparison against current date (defaults to current time)
* @param int|null $timenow Deprecated since Moodle 4.5 - please use {@see clock} dependency injection
* @return model
*/
public static function create_schedule(stdClass $data, ?int $timenow = null): model {
if ($timenow !== null) {
debugging('Passing $timenow is deprecated, please use \core\clock dependency injection', DEBUG_DEVELOPER);
}
$data->name = trim($data->name);
$schedule = (new model(0, $data));
$schedule->set('timenextsend', self::calculate_next_send_time($schedule, $timenow));
$schedule->set('timenextsend', self::calculate_next_send_time($schedule));
return $schedule->create();
}
@@ -209,7 +214,7 @@ class schedule {
return false;
}
$timenow = time();
$timenow = di::get(clock::class)->time();
// Ensure we've reached the initial scheduled start time.
$timescheduled = $schedule->get('timescheduled');
@@ -231,13 +236,17 @@ class schedule {
* returned value is after the current date
*
* @param model $schedule
* @param int|null $timenow Time to use as comparison against current date (defaults to current time)
* @param int|null $timenow Deprecated since Moodle 4.5 - please use {@see clock} dependency injection
* @return int
*/
public static function calculate_next_send_time(model $schedule, ?int $timenow = null): int {
global $CFG;
$timenow = $timenow ?? time();
if ($timenow !== null) {
debugging('Passing $timenow is deprecated, please use \core\clock dependency injection', DEBUG_DEVELOPER);
}
$timenow = di::get(clock::class)->time();
$recurrence = $schedule->get('recurrence');
$timescheduled = $schedule->get('timescheduled');
@@ -289,8 +298,7 @@ class schedule {
// Ensure we don't modify anything in the original model.
$scheduleclone = new model(0, $schedule->to_record());
return self::calculate_next_send_time(
$scheduleclone->set('timescheduled', $timestamp), $timenow);
return self::calculate_next_send_time($scheduleclone->set('timescheduled', $timestamp));
} else {
return $timestamp;
}
reportbuilder/classes/task/send_schedule.php
+5 -2
View File
@@ -18,8 +18,9 @@ declare(strict_types=1);
namespace core_reportbuilder\task;
use core_user;
use core\{clock, di};
use core\task\adhoc_task;
use core_user;
use core_reportbuilder\local\helpers\schedule as helper;
use core_reportbuilder\local\models\schedule;
use moodle_exception;
@@ -140,7 +141,9 @@ class send_schedule extends adhoc_task {
}
// Finish, clean up (set persistent property manually to avoid updating it's user/time modified data).
$DB->set_field($schedule::TABLE, 'timelastsent', time(), ['id' => $schedule->get('id')]);
$DB->set_field($schedule::TABLE, 'timelastsent', di::get(clock::class)->time(), [
'id' => $schedule->get('id'),
]);
if ($scheduleattachment !== null) {
$scheduleattachment->delete();
reportbuilder/classes/task/send_schedules.php
+4 -1
View File
@@ -18,6 +18,7 @@ declare(strict_types=1);
namespace core_reportbuilder\task;
use core\{clock, di};
use core\task\scheduled_task;
use core_reportbuilder\local\helpers\schedule;
use core_reportbuilder\local\models\schedule as model;
@@ -46,7 +47,9 @@ class send_schedules extends scheduled_task {
public function execute(): void {
global $DB;
$schedules = model::get_records_select('enabled = 1 AND timenextsend <= :time', ['time' => time()]);
$schedules = model::get_records_select('enabled = 1 AND timenextsend <= :time', [
'time' => di::get(clock::class)->time(),
]);
$schedules = array_filter($schedules, [schedule::class, 'should_send_schedule']);
// Loop over all schedules for sending, execute corresponding task to send each individually.
reportbuilder/tests/generator/lib.php
+3 -5
View File
@@ -16,6 +16,7 @@
declare(strict_types=1);
use core\{clock, di};
use core_reportbuilder\manager;
use core_reportbuilder\local\helpers\report as helper;
use core_reportbuilder\local\helpers\schedule as schedule_helper;
@@ -208,12 +209,9 @@ class core_reportbuilder_generator extends component_generator_base {
$record['message'] = $record['name'] . ' message';
}
if (!array_key_exists('timescheduled', $record)) {
$record['timescheduled'] = usergetmidnight(time() + DAYSECS);
$record['timescheduled'] = usergetmidnight(di::get(clock::class)->time() + DAYSECS);
}
// Time to use as comparison against current date (null means current time).
$timenow = $record['timenow'] ?? null;
return schedule_helper::create_schedule((object) $record, $timenow);
return schedule_helper::create_schedule((object) $record);
}
}
reportbuilder/tests/local/filters/date_test.php
+32 -14
View File
@@ -19,8 +19,9 @@ declare(strict_types=1);
namespace core_reportbuilder\local\filters;
use advanced_testcase;
use lang_string;
use core\clock;
use core_reportbuilder\local\report\filter;
use lang_string;
/**
* Unit tests for date report filter
@@ -31,14 +32,25 @@ use core_reportbuilder\local\report\filter;
* @copyright 2021 Paul Holden <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class date_test extends advanced_testcase {
final class date_test extends advanced_testcase {
/** @var clock $clock */
private readonly clock $clock;
/**
* Mock the clock
*/
protected function setUp(): void {
parent::setUp();
$this->clock = $this->mock_clock_with_frozen(1622502000);
}
/**
* Data provider for {@see test_get_sql_filter_simple}
*
* @return array
*/
public function get_sql_filter_simple_provider(): array {
public static function get_sql_filter_simple_provider(): array {
return [
[date::DATE_ANY, true],
[date::DATE_NOT_EMPTY, true],
@@ -120,7 +132,7 @@ class date_test extends advanced_testcase {
*
* @return array
*/
public function get_sql_filter_current_week_provider(): array {
public static function get_sql_filter_current_week_provider(): array {
return array_map(static function(int $day): array {
return [$day];
}, range(0, 6));
@@ -141,7 +153,8 @@ class date_test extends advanced_testcase {
set_config('calendar_startwday', $startweekday);
$user = $this->getDataGenerator()->create_user(['timecreated' => time()]);
$usertimecreated = $this->clock->time();
$user = $this->getDataGenerator()->create_user(['timecreated' => $usertimecreated]);
$filter = new filter(
date::class,
@@ -165,14 +178,14 @@ class date_test extends advanced_testcase {
*
* @return array
*/
public function get_sql_filter_current_week_no_match_provider(): array {
public static function get_sql_filter_current_week_no_match_provider(): array {
$data = [];
// For each day, create provider data for -/+ 8 days.
// For each day, create provider data for -/+ 7 days.
foreach (range(0, 6) as $day) {
$data = array_merge($data, [
[$day, '-8 day'],
[$day, '+8 day'],
[$day, '-7 day'],
[$day, '+7 day'],
]);
}
@@ -194,7 +207,7 @@ class date_test extends advanced_testcase {
set_config('calendar_startwday', $startweekday);
$usertimecreated = strtotime($timecreated);
$usertimecreated = strtotime($timecreated, $this->clock->time());
$user = $this->getDataGenerator()->create_user(['timecreated' => $usertimecreated]);
$filter = new filter(
@@ -219,7 +232,7 @@ class date_test extends advanced_testcase {
*
* @return array
*/
public function get_sql_filter_relative_provider(): array {
public static function get_sql_filter_relative_provider(): array {
return [
'Before hour' => [date::DATE_BEFORE, 1, date::DATE_UNIT_HOUR, '-90 minute'],
'Before day' => [date::DATE_BEFORE, 1, date::DATE_UNIT_DAY, '-25 hour'],
@@ -271,8 +284,8 @@ class date_test extends advanced_testcase {
'Next two months' => [date::DATE_NEXT, 2, date::DATE_UNIT_MONTH, '+7 week'],
'Next two years' => [date::DATE_NEXT, 2, date::DATE_UNIT_YEAR, '+15 month'],
'In the past' => [date::DATE_PAST, null, null, '-3 hour'],
'In the future' => [date::DATE_FUTURE, null, null, '+3 hour'],
'In the past' => [date::DATE_PAST, null, null, '-1 minute'],
'In the future' => [date::DATE_FUTURE, null, null, '+1 minute'],
];
}
@@ -291,7 +304,12 @@ class date_test extends advanced_testcase {
$this->resetAfterTest();
$usertimecreated = ($timecreated !== null ? strtotime($timecreated) : time());
// Use relative time period if present, otherwise default to current clock time.
$usertimecreated = $this->clock->time();
if ($timecreated !== null) {
$usertimecreated = strtotime($timecreated, $usertimecreated);
}
$user = $this->getDataGenerator()->create_user(['timecreated' => $usertimecreated]);
$filter = new filter(
reportbuilder/tests/local/helpers/schedule_test.php
+114 -75
View File
@@ -20,6 +20,7 @@ namespace core_reportbuilder\local\helpers;
use advanced_testcase;
use invalid_parameter_exception;
use core\clock;
use core_cohort\reportbuilder\audience\cohortmember;
use core_reportbuilder_generator;
use core_reportbuilder\local\models\schedule as model;
@@ -34,7 +35,18 @@ use core_user\reportbuilder\datasource\users;
* @copyright 2021 Paul Holden <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class schedule_test extends advanced_testcase {
final class schedule_test extends advanced_testcase {
/** @var clock $clock */
private readonly clock $clock;
/**
* Mock the clock
*/
protected function setUp(): void {
parent::setUp();
$this->clock = $this->mock_clock_with_frozen(1622847600);
}
/**
* Test create schedule
@@ -47,7 +59,8 @@ class schedule_test extends advanced_testcase {
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
$timescheduled = time() + DAYSECS;
// Create schedule for tomorrow.
$timescheduled = $this->clock->time() + DAYSECS;
$schedule = schedule::create_schedule((object) [
'name' => 'My schedule',
'reportid' => $report->get('id'),
@@ -81,11 +94,12 @@ class schedule_test extends advanced_testcase {
// Update some record properties.
$record = $schedule->to_record();
$record->name = 'My updated schedule';
$record->timescheduled = 1861340400; // 25/12/2028 07:00 UTC.
$record->timescheduled += (12 * HOURSECS);
$schedule = schedule::update_schedule($record);
$this->assertEquals($record->name, $schedule->get('name'));
$this->assertEquals($record->timescheduled, $schedule->get('timescheduled'));
$this->assertEquals($record->timescheduled, $schedule->get('timenextsend'));
}
/**
@@ -294,100 +308,125 @@ class schedule_test extends advanced_testcase {
*
* @return array[]
*/
public function should_send_schedule_provider(): array {
$time = time();
// We just need large offsets for dates in the past/future.
$yesterday = $time - DAYSECS;
$tomorrow = $time + DAYSECS;
public static function should_send_schedule_provider(): array {
return [
'Disabled' => [[
'enabled' => false,
], false],
'Time scheduled in the past' => [[
'recurrence' => model::RECURRENCE_NONE,
'timescheduled' => $yesterday,
], true],
'Time scheduled in the past, already sent prior to schedule' => [[
'recurrence' => model::RECURRENCE_NONE,
'timescheduled' => $yesterday,
'timelastsent' => $yesterday - HOURSECS,
], true],
'Time scheduled in the past, already sent on schedule' => [[
'recurrence' => model::RECURRENCE_NONE,
'timescheduled' => $yesterday,
'timelastsent' => $yesterday,
], false],
'Time scheduled in the future' => [[
'recurrence' => model::RECURRENCE_NONE,
'timescheduled' => $tomorrow,
], false],
'Time scheduled in the future, already sent prior to schedule' => [[
'recurrence' => model::RECURRENCE_NONE,
'timelastsent' => $yesterday,
'timescheduled' => $tomorrow,
], false],
'Next send in the past' => [[
'recurrence' => model::RECURRENCE_DAILY,
'timescheduled' => $yesterday,
'timenextsend' => $yesterday,
], true],
'Next send in the future' => [[
'recurrence' => model::RECURRENCE_DAILY,
'timescheduled' => $yesterday,
'timenextsend' => $tomorrow,
], false],
'Time scheduled in the past' => [
model::RECURRENCE_NONE, '-1 hour', null, null, true,
],
'Time scheduled in the past, already sent prior to schedule' => [
model::RECURRENCE_NONE, '-1 hour', '-2 hour', null, true,
],
'Time scheduled in the past, already sent on schedule' => [
model::RECURRENCE_NONE, '-1 hour', '-1 hour', null, false,
],
'Time scheduled in the future' => [
model::RECURRENCE_NONE, '+1 hour', null, null, false,
],
'Time scheduled in the future, already sent prior to schedule' => [
model::RECURRENCE_NONE, '+1 hour', '-1 hour', null, false,
],
'Next send in the past' => [
model::RECURRENCE_DAILY, '-1 hour', null, '-1 hour', true,
],
'Next send in the future' => [
model::RECURRENCE_DAILY, '-1 hour', null, '+1 hour', false,
],
];
}
/**
* Test for whether a schedule should be sent
*
* @param array $properties
* @param int $recurrence
* @param string $timescheduled Relative time suitable for passing to {@see strtotime}
* @param string|null $timelastsent Relative time suitable for passing to {@see strtotime}, or null to ignore
* @param string|null $timenextsend Relative time suitable for passing to {@see strtotime}, or null to ignore
* @param bool $expected
*
* @dataProvider should_send_schedule_provider
*/
public function test_should_send_schedule(array $properties, bool $expected): void {
public function test_should_send_schedule(
int $recurrence,
string $timescheduled,
?string $timelastsent,
?string $timenextsend,
bool $expected,
): void {
$this->resetAfterTest();
/** @var core_reportbuilder_generator $generator */
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
$schedule = $generator->create_schedule(['reportid' => $report->get('id'), 'name' => 'My schedule'] + $properties);
// Use relative time period if present, otherwise default to zero (never sent).
$scheduletimelastsent = 0;
if ($timelastsent !== null) {
$scheduletimelastsent = strtotime($timelastsent, $this->clock->time());
}
$schedule = $generator->create_schedule([
'reportid' => $report->get('id'),
'name' => 'My schedule',
'recurrence' => $recurrence,
'timescheduled' => strtotime($timescheduled, $this->clock->time()),
'timelastsent' => $scheduletimelastsent,
]);
// If "Time next send" is specified, then override calculated value.
if (array_key_exists('timenextsend', $properties)) {
$schedule->set('timenextsend', $properties['timenextsend']);
if ($timenextsend !== null) {
$schedule->set('timenextsend', strtotime($timenextsend, $this->clock->time()));
}
$this->assertEquals($expected, schedule::should_send_schedule($schedule));
}
/**
* Test for whether a schedule should be sent that has been disabled
*/
public function test_should_send_schedule_disabled(): void {
$this->resetAfterTest();
/** @var core_reportbuilder_generator $generator */
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
$schedule = $generator->create_schedule([
'reportid' => $report->get('id'),
'name' => 'My schedule',
'enabled' => 0,
]);
$this->assertFalse(schedule::should_send_schedule($schedule));
}
/**
* Data provider for {@see test_calculate_next_send_time}
*
* @return array[]
*/
public function calculate_next_send_time_provider(): array {
$timescheduled = 1635865200; // Tue Nov 02 2021 15:00:00 GMT+0000.
$timenow = 1639846800; // Sat Dec 18 2021 17:00:00 GMT+0000.
public static function calculate_next_send_time_provider(): array {
// Times are based on the current clock time (Fri Jun 04 2021 23:00:00 UTC).
return [
'No recurrence' => [model::RECURRENCE_NONE, $timescheduled, $timenow, $timescheduled],
'Recurrence, time scheduled in future' => [model::RECURRENCE_DAILY, $timenow + DAYSECS, $timenow, $timenow + DAYSECS],
// Sun Dec 19 2021 15:00:00 GMT+0000.
'Daily recurrence' => [model::RECURRENCE_DAILY, $timescheduled, $timenow, 1639926000],
// Mon Dec 20 2021 15:00:00 GMT+0000.
'Weekday recurrence' => [model::RECURRENCE_WEEKDAYS, $timescheduled, $timenow, 1640012400],
// Tue Dec 21 2021 15:00:00 GMT+0000.
'Weekly recurrence' => [model::RECURRENCE_WEEKLY, $timescheduled, $timenow, 1640098800],
// Sun Jan 02 2022 15:00:00 GMT+0000.
'Monthy recurrence' => [model::RECURRENCE_MONTHLY, $timescheduled, $timenow, 1641135600],
// Wed Nov 02 2022 15:00:00 GMT+0000.
'Annual recurrence' => [model::RECURRENCE_ANNUALLY, $timescheduled, $timenow, 1667401200],
'No recurrence' => [
model::RECURRENCE_NONE, '2021-06-03 12:00', '2021-06-03 12:00',
],
'Recurrence, time scheduled in future' => [
model::RECURRENCE_DAILY, '2021-06-05 12:00', '2021-06-05 12:00',
],
'Daily recurrence' => [
model::RECURRENCE_DAILY, '2021-06-02 12:00', '2021-06-05 12:00',
],
'Weekday recurrence' => [
model::RECURRENCE_WEEKDAYS, '2021-06-02 12:00', '2021-06-07 12:00',
],
'Weekly recurrence' => [
model::RECURRENCE_WEEKLY, '2021-05-18 12:00', '2021-06-08 12:00',
],
'Monthy recurrence' => [
model::RECURRENCE_MONTHLY, '2021-03-19 12:00', '2021-06-19 12:00',
],
'Annual recurrence' => [
model::RECURRENCE_ANNUALLY, '2019-05-01 12:00', '2022-05-01 12:00',
],
];
}
@@ -395,27 +434,27 @@ class schedule_test extends advanced_testcase {
* Test for calculating next schedule send time
*
* @param int $recurrence
* @param int $timescheduled
* @param int $timenow
* @param int $expected
* @param string $timescheduled Absolute time suitable for passing to {@see strtotime}
* @param string $expected Absolute time suitable for passing to {@see strtotime}
*
* @dataProvider calculate_next_send_time_provider
*/
public function test_calculate_next_send_time(int $recurrence, int $timescheduled, int $timenow, int $expected): void {
public function test_calculate_next_send_time(int $recurrence, string $timescheduled, string $expected): void {
$this->resetAfterTest();
/** @var core_reportbuilder_generator $generator */
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
$schedule = $generator->create_schedule([
// Create model manually, as the generator automatically calculates next send itself.
$schedule = new model(0, (object) [
'reportid' => $report->get('id'),
'name' => 'My schedule',
'recurrence' => $recurrence,
'timescheduled' => $timescheduled,
'timenow' => $timenow,
'timescheduled' => strtotime("{$timescheduled} UTC"),
]);
$this->assertEquals($expected, schedule::calculate_next_send_time($schedule, $timenow));
$scheduleexpected = strtotime("{$expected} UTC");
$this->assertEquals($scheduleexpected, schedule::calculate_next_send_time($schedule));
}
}