diff --git a/reportbuilder/classes/local/helpers/schedule.php b/reportbuilder/classes/local/helpers/schedule.php index 6f23dc39619..9dcfd65c853 100644 --- a/reportbuilder/classes/local/helpers/schedule.php +++ b/reportbuilder/classes/local/helpers/schedule.php @@ -119,10 +119,12 @@ class schedule { // Now convert audiences to SQL for user retrieval. [$wheres, $params] = audience::user_audience_sql($audiences); + [$userorder] = users_order_by_sql('u'); $sql = 'SELECT u.* FROM {user} u - WHERE ' . implode(' OR ', $wheres); + WHERE ' . implode(' OR ', $wheres) . ' + ORDER BY ' . $userorder; return $DB->get_records_sql($sql, $params); } diff --git a/reportbuilder/tests/local/helpers/schedule_test.php b/reportbuilder/tests/local/helpers/schedule_test.php index e71bcd81126..c45c942e374 100644 --- a/reportbuilder/tests/local/helpers/schedule_test.php +++ b/reportbuilder/tests/local/helpers/schedule_test.php @@ -189,13 +189,13 @@ class schedule_test extends advanced_testcase { // Create cohort, with some members. $cohort = $this->getDataGenerator()->create_cohort(); - $cohortuserone = $this->getDataGenerator()->create_user(); + $cohortuserone = $this->getDataGenerator()->create_user(['firstname' => 'Zoe', 'lastname' => 'Zebra']); cohort_add_member($cohort->id, $cohortuserone->id); - $cohortusertwo = $this->getDataGenerator()->create_user(); + $cohortusertwo = $this->getDataGenerator()->create_user(['firstname' => 'Henrietta', 'lastname' => 'Hamster']); cohort_add_member($cohort->id, $cohortusertwo->id); // Create a third user, to be added manually. - $manualuserone = $this->getDataGenerator()->create_user(); + $manualuserone = $this->getDataGenerator()->create_user(['firstname' => 'Bob', 'lastname' => 'Badger']); $audiencecohort = $generator->create_audience([ 'reportid' => $report->get('id'), @@ -220,11 +220,11 @@ class schedule_test extends advanced_testcase { ]); $users = schedule::get_schedule_report_users($schedule); - $this->assertEqualsCanonicalizing([ - $cohortuserone->id, - $cohortusertwo->id, - $manualuserone->id, - ], array_keys($users)); + $this->assertEquals([ + 'Bob', + 'Henrietta', + 'Zoe', + ], array_column($users, 'firstname')); } /** diff --git a/reportbuilder/tests/task/send_schedule_test.php b/reportbuilder/tests/task/send_schedule_test.php index 1170194a19e..4771e4b7e26 100644 --- a/reportbuilder/tests/task/send_schedule_test.php +++ b/reportbuilder/tests/task/send_schedule_test.php @@ -72,8 +72,18 @@ class send_schedule_test extends advanced_testcase { $this->resetAfterTest(); $this->setAdminUser(); - $userone = $this->getDataGenerator()->create_user(['username' => 'userone', 'email' => 'user1@example.com']); - $usertwo = $this->getDataGenerator()->create_user(['username' => 'usertwo', 'email' => 'user2@example.com']); + $userone = $this->getDataGenerator()->create_user([ + 'username' => 'userone', + 'email' => 'user1@example.com', + 'firstname' => 'Zoe', + 'lastname' => 'Zebra', + ]); + $usertwo = $this->getDataGenerator()->create_user([ + 'username' => 'usertwo', + 'email' => 'user2@example.com', + 'firstname' => 'Henrietta', + 'lastname' => 'Hamster', + ]); /** @var core_reportbuilder_generator $generator */ $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); @@ -106,20 +116,17 @@ class send_schedule_test extends advanced_testcase { 'audiences' => json_encode([$audience->get_persistent()->get('id')]), ]); - // Send the schedule, catch emails in sink. + // Send the schedule, catch emails in sink (noting the users are sorted alphabetically). $sink = $this->redirectEmails(); - ob_start(); + $this->expectOutputRegex("/^Sending schedule: My schedule\n" . + " Sending to: " . fullname($usertwo) . "\n" . + " Sending to: " . fullname($userone) . "\n" . + "Sending schedule complete\n/" + ); $sendschedule = new send_schedule(); $sendschedule->set_custom_data(['reportid' => $report->get('id'), 'scheduleid' => $schedule->get('id')]); $sendschedule->execute(); - $output = ob_get_clean(); - - // Assert the output contains the following messages. - $this->assertStringContainsString("Sending schedule: My schedule", $output); - $this->assertStringContainsString("Sending to: " . fullname($userone), $output); - $this->assertStringContainsString("Sending to: " . fullname($usertwo), $output); - $this->assertStringContainsString("Sending schedule complete", $output); $messages = $sink->get_messages(); $this->assertCount(2, $messages);