From 56da66adb6bea2ff523a70468c4b84c456501519 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Thu, 13 Jul 2017 16:07:27 +0100 Subject: [PATCH] MDL-59532 mod_lesson: Fix check_updates_since with groups There were a couple of incorrect SQL queries when the user calling the function is a teacher and the activity uses separate groups. --- mod/lesson/lib.php | 12 +++++-- mod/lesson/tests/lib_test.php | 66 ++++++++++++++++++++++++++++++++--- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/mod/lesson/lib.php b/mod/lesson/lib.php index e838f5d3be3..1006458b412 100644 --- a/mod/lesson/lib.php +++ b/mod/lesson/lib.php @@ -1601,16 +1601,22 @@ function lesson_check_updates_since(cm_info $cm, $from, $filter = array()) { $updates->userpagesviewed->itemids = array_keys($pagesviewed); } - $select = 'lessonid = ? AND completed > ? ' . $insql; + $select = 'lessonid = ? AND completed > ?'; + if (!empty($insql)) { + $select .= ' AND userid ' . $insql; + } $grades = $DB->get_records_select('lesson_grades', $select, $params, '', 'id'); if (!empty($grades)) { $updates->usergrades->updated = true; $updates->usergrades->itemids = array_keys($grades); } - $select = 'lessonid = ? AND (starttime > ? OR lessontime > ? OR timemodifiedoffline > ?) ' . $insql; + $select = 'lessonid = ? AND (starttime > ? OR lessontime > ? OR timemodifiedoffline > ?)'; $params = array($cm->instance, $from, $from, $from); - $params = array_merge($params, $inparams); + if (!empty($insql)) { + $select .= ' AND userid ' . $insql; + $params = array_merge($params, $inparams); + } $timers = $DB->get_records_select('lesson_timer', $select, $params, '', 'id'); if (!empty($timers)) { $updates->usertimers->updated = true; diff --git a/mod/lesson/tests/lib_test.php b/mod/lesson/tests/lib_test.php index f0e8ef10266..5cd91a22856 100644 --- a/mod/lesson/tests/lib_test.php +++ b/mod/lesson/tests/lib_test.php @@ -91,14 +91,27 @@ class mod_lesson_lib_testcase extends advanced_testcase { $this->resetAfterTest(); $this->setAdminUser(); - $course = $this->getDataGenerator()->create_course(); + $course = new stdClass(); + $course->groupmode = SEPARATEGROUPS; + $course->groupmodeforce = true; + $course = $this->getDataGenerator()->create_course($course); // Create user. - $student = self::getDataGenerator()->create_user(); + $studentg1 = self::getDataGenerator()->create_user(); + $teacherg1 = self::getDataGenerator()->create_user(); + $studentg2 = self::getDataGenerator()->create_user(); // User enrolment. $studentrole = $DB->get_record('role', array('shortname' => 'student')); - $this->getDataGenerator()->enrol_user($student->id, $course->id, $studentrole->id, 'manual'); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); + $this->getDataGenerator()->enrol_user($studentg1->id, $course->id, $studentrole->id, 'manual'); + $this->getDataGenerator()->enrol_user($teacherg1->id, $course->id, $teacherrole->id, 'manual'); + $this->getDataGenerator()->enrol_user($studentg2->id, $course->id, $studentrole->id, 'manual'); + + $group1 = $this->getDataGenerator()->create_group(array('courseid' => $course->id)); + $group2 = $this->getDataGenerator()->create_group(array('courseid' => $course->id)); + groups_add_member($group1, $studentg1); + groups_add_member($group2, $studentg2); $this->setCurrentTimeStart(); $record = array( @@ -136,8 +149,23 @@ class mod_lesson_lib_testcase extends advanced_testcase { $this->assertTrue($updates->answers->updated); $this->assertCount(2, $updates->answers->itemids); - // Now, do something in the lesson. - $this->setUser($student); + // Now, do something in the lesson with the two users. + $this->setUser($studentg1); + mod_lesson_external::launch_attempt($lesson->id); + $data = array( + array( + 'name' => 'answerid', + 'value' => $DB->get_field('lesson_answers', 'id', array('pageid' => $tfrecord->id, 'jumpto' => -1)), + ), + array( + 'name' => '_qf__lesson_display_answer_form_truefalse', + 'value' => 1, + ) + ); + mod_lesson_external::process_page($lesson->id, $tfrecord->id, $data); + mod_lesson_external::finish_attempt($lesson->id); + + $this->setUser($studentg2); mod_lesson_external::launch_attempt($lesson->id); $data = array( array( @@ -152,6 +180,7 @@ class mod_lesson_lib_testcase extends advanced_testcase { mod_lesson_external::process_page($lesson->id, $tfrecord->id, $data); mod_lesson_external::finish_attempt($lesson->id); + $this->setUser($studentg1); $updates = lesson_check_updates_since($cm, $onehourago); // Check question attempts, timers and new grades. @@ -163,6 +192,33 @@ class mod_lesson_lib_testcase extends advanced_testcase { $this->assertTrue($updates->timers->updated); $this->assertCount(1, $updates->timers->itemids); + + // Now, as teacher, check that I can see the two users (even in separate groups). + $this->setUser($teacherg1); + $updates = lesson_check_updates_since($cm, $onehourago); + $this->assertTrue($updates->userquestionattempts->updated); + $this->assertCount(2, $updates->userquestionattempts->itemids); + + $this->assertTrue($updates->usergrades->updated); + $this->assertCount(2, $updates->usergrades->itemids); + + $this->assertTrue($updates->usertimers->updated); + $this->assertCount(2, $updates->usertimers->itemids); + + // Now, teacher can't access all groups. + groups_add_member($group1, $teacherg1); + assign_capability('moodle/site:accessallgroups', CAP_PROHIBIT, $teacherrole->id, context_module::instance($cm->id)); + accesslib_clear_all_caches_for_unit_testing(); + $updates = lesson_check_updates_since($cm, $onehourago); + // I will see only the studentg1 updates. + $this->assertTrue($updates->userquestionattempts->updated); + $this->assertCount(1, $updates->userquestionattempts->itemids); + + $this->assertTrue($updates->usergrades->updated); + $this->assertCount(1, $updates->usergrades->itemids); + + $this->assertTrue($updates->usertimers->updated); + $this->assertCount(1, $updates->usertimers->itemids); } public function test_lesson_core_calendar_provide_event_action_open() { -- 2.36.1