mirror of
https://github.com/moodle/moodle.git
synced 2025-01-18 05:58:34 +01:00
MDL-45390 grades: Courses letters using system bad boundaries
Added unit tests to highlight the existing failures and adding unset_config between each set of tests.
This commit is contained in:
parent
405b90bcd2
commit
ece791dbd4
@ -379,39 +379,47 @@ function upgrade_course_letter_boundary($courseid = null) {
|
||||
$params['courseid'] = $courseid;
|
||||
}
|
||||
|
||||
$contextselect = context_helper::get_preload_record_columns_sql('ctx');
|
||||
// Check to see if the system letter boundaries are borked.
|
||||
$systemcontext = context_system::instance();
|
||||
$systemneedsfreeze = upgrade_letter_boundary_needs_freeze($systemcontext);
|
||||
|
||||
// 3, 13, 23, 31, and 32 are the grade display types that incorporate showing letters. See lib/grade/constants/php.
|
||||
if (isset($CFG->grade_displaytype) && in_array($CFG->grade_displaytype, array(3, 13, 23, 31, 32))) {
|
||||
// Check to see if the system letter boundaries are borked.
|
||||
$systemcontext = context_system::instance();
|
||||
if (upgrade_letter_boundary_needs_freeze($systemcontext)) {
|
||||
// Select courses with no grade setting for display and a grade item that is using the default display,
|
||||
// but have not altered the course letter boundary configuration. These courses are definitely affected.
|
||||
$sql = "SELECT DISTINCT c.id AS courseid
|
||||
FROM {grade_items} gi
|
||||
JOIN {course} c ON c.id = gi.courseid
|
||||
LEFT JOIN {grade_settings} gs ON gs.courseid = c.id AND name = 'displaytype'
|
||||
LEFT JOIN (SELECT DISTINCT c.id
|
||||
FROM {grade_letters} gl
|
||||
JOIN {context} ctx ON gl.contextid = ctx.id
|
||||
JOIN {course} c ON ctx.instanceid = c.id
|
||||
WHERE ctx.contextlevel = :contextlevel) gl ON gl.id = c.id
|
||||
WHERE (gi.display = 0 AND (gs.value is NULL))
|
||||
AND gl.id is NULL $coursesql";
|
||||
$affectedcourseids = $DB->get_recordset_sql($sql, $params);
|
||||
foreach ($affectedcourseids as $courseid) {
|
||||
set_config('gradebook_calculations_freeze_' . $courseid->courseid, 20160511);
|
||||
}
|
||||
$affectedcourseids->close();
|
||||
$systemletters = (isset($CFG->grade_displaytype) && in_array($CFG->grade_displaytype, array(3, 13, 23, 31, 32)));
|
||||
|
||||
$contextselect = context_helper::get_preload_record_columns_sql('ctx');
|
||||
|
||||
if ($systemletters && $systemneedsfreeze) {
|
||||
// Select courses with no grade setting for display and a grade item that is using the default display,
|
||||
// but have not altered the course letter boundary configuration. These courses are definitely affected.
|
||||
|
||||
$sql = "SELECT DISTINCT c.id AS courseid
|
||||
FROM {grade_items} gi
|
||||
JOIN {course} c ON c.id = gi.courseid
|
||||
LEFT JOIN {grade_settings} gs ON gs.courseid = c.id AND gs.name = 'displaytype'
|
||||
LEFT JOIN (SELECT DISTINCT c.id
|
||||
FROM {grade_letters} gl
|
||||
JOIN {context} ctx ON gl.contextid = ctx.id
|
||||
JOIN {course} c ON ctx.instanceid = c.id
|
||||
WHERE ctx.contextlevel = :contextlevel) gl ON gl.id = c.id
|
||||
WHERE (gi.display = 0 AND (gs.value is NULL))
|
||||
AND gl.id is NULL $coursesql";
|
||||
$affectedcourseids = $DB->get_recordset_sql($sql, $params);
|
||||
foreach ($affectedcourseids as $courseid) {
|
||||
set_config('gradebook_calculations_freeze_' . $courseid->courseid, 20160516);
|
||||
}
|
||||
$affectedcourseids->close();
|
||||
|
||||
}
|
||||
|
||||
if ($systemletters || $systemneedsfreeze) {
|
||||
|
||||
// If the system letter boundary is okay proceed to check grade item and course grade display settings.
|
||||
$params['contextlevel2'] = CONTEXT_COURSE;
|
||||
$sql = "SELECT DISTINCT c.id AS courseid, $contextselect
|
||||
FROM {course} c
|
||||
JOIN {context} ctx ON ctx.instanceid = c.id AND ctx.contextlevel = :contextlevel
|
||||
JOIN {grade_items} gi ON c.id = gi.courseid
|
||||
LEFT JOIN {grade_settings} gs ON c.id = gs.courseid AND name = 'displaytype'
|
||||
LEFT JOIN {grade_settings} gs ON c.id = gs.courseid AND gs.name = 'displaytype'
|
||||
LEFT JOIN (SELECT DISTINCT c.id
|
||||
FROM {grade_letters} gl
|
||||
JOIN {context} ctx ON gl.contextid = ctx.id
|
||||
@ -422,13 +430,14 @@ function upgrade_course_letter_boundary($courseid = null) {
|
||||
OR gl.id is NOT NULL)
|
||||
$coursesql";
|
||||
} else {
|
||||
|
||||
// There is no site setting for letter grades. Just check the modified letter boundaries.
|
||||
$sql = "SELECT DISTINCT c.id AS courseid, $contextselect
|
||||
FROM {grade_letters} l, {course} c
|
||||
JOIN {context} ctx ON ctx.instanceid = c.id AND ctx.contextlevel = :contextlevel
|
||||
WHERE l.contextid = ctx.id
|
||||
AND ctx.instanceid = c.id
|
||||
$coursesql";
|
||||
FROM {grade_letters} l, {course} c
|
||||
JOIN {context} ctx ON ctx.instanceid = c.id AND ctx.contextlevel = :contextlevel
|
||||
WHERE l.contextid = ctx.id
|
||||
AND ctx.instanceid = c.id
|
||||
$coursesql";
|
||||
}
|
||||
|
||||
$potentialcourses = $DB->get_recordset_sql($sql, $params);
|
||||
|
@ -583,27 +583,29 @@ class core_upgradelib_testcase extends advanced_testcase {
|
||||
global $CFG, $DB;
|
||||
$this->resetAfterTest(true);
|
||||
|
||||
require_once($CFG->libdir . '/db/upgradelib.php');
|
||||
|
||||
// Create a user.
|
||||
$user = $this->getDataGenerator()->create_user();
|
||||
|
||||
// Create some courses.
|
||||
$courses = array();
|
||||
$contexts = array();
|
||||
for ($i = 0; $i < 27; $i++) {
|
||||
for ($i = 0; $i < 37; $i++) {
|
||||
$course = $this->getDataGenerator()->create_course();
|
||||
$context = context_course::instance($course->id);
|
||||
if (in_array($i, array(2, 5, 10, 13, 14, 19, 23, 25))) {
|
||||
if (in_array($i, array(2, 5, 10, 13, 14, 19, 23, 25, 30, 34, 36))) {
|
||||
// Assign good letter boundaries.
|
||||
$this->assign_good_letter_boundary($context->id);
|
||||
}
|
||||
if (in_array($i, array(3, 6, 11, 15, 20, 24, 26))) {
|
||||
if (in_array($i, array(3, 6, 11, 15, 20, 24, 26, 31, 35))) {
|
||||
// Assign bad letter boundaries.
|
||||
$this->assign_bad_letter_boundary($context->id);
|
||||
}
|
||||
|
||||
if (in_array($i, array(9, 10, 11, 18, 19, 20))) {
|
||||
if (in_array($i, array(9, 10, 11, 18, 19, 20, 29, 30, 31))) {
|
||||
grade_set_setting($course->id, 'displaytype', '3');
|
||||
} else if (in_array($i, array(8, 17))) {
|
||||
} else if (in_array($i, array(8, 17, 28))) {
|
||||
grade_set_setting($course->id, 'displaytype', '2');
|
||||
}
|
||||
|
||||
@ -614,10 +616,10 @@ class core_upgradelib_testcase extends advanced_testcase {
|
||||
'itemmodule' => 'assign',
|
||||
'iteminstance' => $assignrow->id,
|
||||
'courseid' => $course->id));
|
||||
if (in_array($i, array(13, 14, 15, 22, 23, 24))) {
|
||||
if (in_array($i, array(13, 14, 15, 23, 24, 34, 35, 36))) {
|
||||
grade_item::set_properties($gi, array('display', 3));
|
||||
$gi->update();
|
||||
} else if (in_array($i, array(12, 21))) {
|
||||
} else if (in_array($i, array(12, 21, 32))) {
|
||||
grade_item::set_properties($gi, array('display', 2));
|
||||
$gi->update();
|
||||
}
|
||||
@ -657,7 +659,11 @@ class core_upgradelib_testcase extends advanced_testcase {
|
||||
|
||||
// System setting for grade letter boundaries (default).
|
||||
set_config('grade_displaytype', '3');
|
||||
for ($i = 0; $i < 37; $i++) {
|
||||
unset_config('gradebook_calculations_freeze_' . $courses[$i]->id);
|
||||
}
|
||||
upgrade_course_letter_boundary();
|
||||
|
||||
// [7] A course with no grade display settings for the course or grade items.
|
||||
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[7]->id}));
|
||||
// [8] A course with grade display settings, but for something that isn't letters.
|
||||
@ -680,6 +686,9 @@ class core_upgradelib_testcase extends advanced_testcase {
|
||||
// System setting for grade letter boundaries (custom with problem).
|
||||
$systemcontext = context_system::instance();
|
||||
$this->assign_bad_letter_boundary($systemcontext->id);
|
||||
for ($i = 0; $i < 37; $i++) {
|
||||
unset_config('gradebook_calculations_freeze_' . $courses[$i]->id);
|
||||
}
|
||||
upgrade_course_letter_boundary();
|
||||
|
||||
// [16] A course with no grade display settings for the course or grade items.
|
||||
@ -704,14 +713,46 @@ class core_upgradelib_testcase extends advanced_testcase {
|
||||
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[25]->id}));
|
||||
// [26] A course that is using the default display setting (letters) and altered the letter boundary with 57. Should be frozen.
|
||||
$this->assertEquals(20160516, $CFG->{'gradebook_calculations_freeze_' . $courses[26]->id});
|
||||
|
||||
// System setting not showing letters.
|
||||
set_config('grade_displaytype', '2');
|
||||
for ($i = 0; $i < 37; $i++) {
|
||||
unset_config('gradebook_calculations_freeze_' . $courses[$i]->id);
|
||||
}
|
||||
upgrade_course_letter_boundary();
|
||||
|
||||
// [27] A course with no grade display settings for the course or grade items.
|
||||
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[27]->id}));
|
||||
// [28] A course with grade display settings, but for something that isn't letters.
|
||||
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[28]->id}));
|
||||
// [29] A course with grade display settings of letters which are default.
|
||||
$this->assertEquals(20160516, $CFG->{'gradebook_calculations_freeze_' . $courses[29]->id});
|
||||
// [30] A course with grade display settings of letters which are not default, but not affected.
|
||||
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[30]->id}));
|
||||
// [31] A course with grade display settings of letters which are not default, which will be affected.
|
||||
$this->assertEquals(20160516, $CFG->{'gradebook_calculations_freeze_' . $courses[31]->id});
|
||||
// [32] A grade item with display settings which are not letters.
|
||||
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[32]->id}));
|
||||
// [33] All system defaults.
|
||||
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[33]->id}));
|
||||
// [34] A grade item with display settings of letters which are not default, but not affected. Course uses new letter boundary setting.
|
||||
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[34]->id}));
|
||||
// [35] A grade item with display settings of letters which are not default, which will be affected.
|
||||
$this->assertEquals(20160516, $CFG->{'gradebook_calculations_freeze_' . $courses[35]->id});
|
||||
// [36] A course with grade display settings of letters with modified and good boundary (not 57) Should not be frozen.
|
||||
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[36]->id}));
|
||||
}
|
||||
|
||||
/**
|
||||
* Test upgrade_letter_boundary_needs_freeze function.
|
||||
*/
|
||||
public function test_upgrade_letter_boundary_needs_freeze() {
|
||||
global $CFG;
|
||||
|
||||
$this->resetAfterTest();
|
||||
|
||||
require_once($CFG->libdir . '/db/upgradelib.php');
|
||||
|
||||
$courses = array();
|
||||
$contexts = array();
|
||||
for ($i = 0; $i < 3; $i++) {
|
||||
@ -753,6 +794,8 @@ class core_upgradelib_testcase extends advanced_testcase {
|
||||
array('contextid' => $contextid, 'lowerboundary' => 25.00000, 'letter' => 'D'),
|
||||
array('contextid' => $contextid, 'lowerboundary' => 0.00000, 'letter' => 'F'),
|
||||
);
|
||||
|
||||
$DB->delete_records('grade_letters', array('contextid' => $contextid));
|
||||
foreach ($newlettersscale as $record) {
|
||||
// There is no API to do this, so we have to manually insert into the database.
|
||||
$DB->insert_record('grade_letters', $record);
|
||||
@ -779,6 +822,8 @@ class core_upgradelib_testcase extends advanced_testcase {
|
||||
array('contextid' => $contextid, 'lowerboundary' => 25.00000, 'letter' => 'D'),
|
||||
array('contextid' => $contextid, 'lowerboundary' => 0.00000, 'letter' => 'F'),
|
||||
);
|
||||
|
||||
$DB->delete_records('grade_letters', array('contextid' => $contextid));
|
||||
foreach ($newlettersscale as $record) {
|
||||
// There is no API to do this, so we have to manually insert into the database.
|
||||
$DB->insert_record('grade_letters', $record);
|
||||
|
Loading…
x
Reference in New Issue
Block a user