Skip to content

Commit

Permalink
MDL-45390 gradebook: sql tidy up and version alignment.
Browse files Browse the repository at this point in the history
  • Loading branch information
@abgreeve
abgreeve committed May 18, 2016
1 parent ece791d commit 41abbbb
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 42 deletions.
6 changes: 3 additions & 3 deletions backup/moodle2/restore_stepslib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -506,10 +506,10 @@ protected function gradebook_calculation_freeze() {
require_once($CFG->libdir . '/db/upgradelib.php');
upgrade_calculated_grade_items($this->get_courseid());
}
// Courses from before 3.1 (20160511) may have a letter boundary problem and should be checked for this issue.
// Backups from before and including 2.9 could have a build number that is greater than 20160511 and should
// Courses from before 3.1 (20160518) may have a letter boundary problem and should be checked for this issue.
// Backups from before and including 2.9 could have a build number that is greater than 20160518 and should
// be checked for this problem.
if (!$gradebookcalculationsfreeze && ($backupbuild < 20160511 || $backuprelease <= 2.9)) {
if (!$gradebookcalculationsfreeze && ($backupbuild < 20160518 || $backuprelease <= 2.9)) {
require_once($CFG->libdir . '/db/upgradelib.php');
upgrade_course_letter_boundary($this->get_courseid());
}
Expand Down
2 changes: 1 addition & 1 deletion ...at/grade_letter_boundary_20160511.feature → ...at/grade_letter_boundary_20160518.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Feature: We can customise the letter boundary of a course.
Given the following "courses" exist:
| fullname | shortname | category |
| Course 1 | C1 | 0 |
And gradebook calculations for the course "C1" are frozen at version "20160511"
And gradebook calculations for the course "C1" are frozen at version "20160518"
And the following "users" exist:
| username | firstname | lastname | email | idnumber | alternatename |
| teacher1 | Teacher | 1 | teacher1@example.com | t1 | Terry |
Expand Down
2 changes: 1 addition & 1 deletion lib/db/upgrade.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2056,7 +2056,7 @@ function xmldb_main_upgrade($oldversion) {
if ($oldversion < 2016051700.01) {
// This script is included in each major version upgrade process (3.0, 3.1) so make sure we don't run it twice.
if (empty($CFG->upgrade_letterboundarycourses)) {
// MDL-21746. If a grade is being displayed with letters and the grade boundaries are not being adhered to properly
// MDL-45390. If a grade is being displayed with letters and the grade boundaries are not being adhered to properly
// then this course will also be frozen.
// If the changes are accepted then the display of some grades may change.
// This is here to freeze the gradebook in affected courses.
Expand Down
36 changes: 14 additions & 22 deletions lib/db/upgradelib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,19 +393,16 @@ function upgrade_course_letter_boundary($courseid = null) {
// 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
FROM {course} c
JOIN {grade_items} gi ON c.id = gi.courseid
JOIN {context} ctx ON ctx.instanceid = c.id AND ctx.contextlevel = :contextlevel
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))
LEFT JOIN {grade_letters} gl ON gl.contextid = ctx.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);
set_config('gradebook_calculations_freeze_' . $courseid->courseid, 20160518);
}
$affectedcourseids->close();

Expand All @@ -414,29 +411,24 @@ function upgrade_course_letter_boundary($courseid = null) {
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 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 = :contextlevel2) gl ON gl.id = c.id
WHERE (gi.display IN (3, 13, 23, 31, 32)
LEFT JOIN {grade_letters} gl ON gl.contextid = ctx.id
WHERE gi.display IN (3, 13, 23, 31, 32)
OR (" . $DB->sql_compare_text('gs.value') . " IN ('3', '13', '23', '31', '32'))
OR gl.id is NOT 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
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";
}

Expand All @@ -454,7 +446,7 @@ function upgrade_course_letter_boundary($courseid = null) {
if (upgrade_letter_boundary_needs_freeze($coursecontext)) {
// We have a course with a possible score standardisation problem. Flag for freeze.
// Flag this course as being frozen.
set_config('gradebook_calculations_freeze_' . $value->courseid, 20160511);
set_config('gradebook_calculations_freeze_' . $value->courseid, 20160518);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/gradelib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ function grade_format_gradevalue_letter($value, $grade_item) {
$gradebookcalculationsfreeze = 'gradebook_calculations_freeze_' . $grade_item->courseid;

foreach ($letters as $boundary => $letter) {
if (property_exists($CFG, $gradebookcalculationsfreeze) && (int)$CFG->{$gradebookcalculationsfreeze} <= 20160511) {
if (property_exists($CFG, $gradebookcalculationsfreeze) && (int)$CFG->{$gradebookcalculationsfreeze} <= 20160518) {
// Do nothing.
} else {
// The boundary is a percentage out of 100 so use 0 as the min and 100 as the max.
Expand Down
28 changes: 14 additions & 14 deletions lib/tests/upgradelib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -649,13 +649,13 @@ public function test_upgrade_course_letter_boundary() {
// [2] A course with letter boundaries which are custom but not affected.
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[2]->id}));
// [3] A course with letter boundaries which are custom and will be affected.
$this->assertEquals(20160511, $CFG->{'gradebook_calculations_freeze_' . $courses[3]->id});
$this->assertEquals(20160518, $CFG->{'gradebook_calculations_freeze_' . $courses[3]->id});
// [4] A course with no letter boundaries, but with a grade item with letter boundaries which are default.
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[4]->id}));
// [5] A course with no letter boundaries, but with a grade item with letter boundaries which are not default, but not affected.
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[5]->id}));
// [6] A course with no letter boundaries, but with a grade item with letter boundaries which are not default which will be affected.
$this->assertEquals(20160511, $CFG->{'gradebook_calculations_freeze_' . $courses[6]->id});
$this->assertEquals(20160518, $CFG->{'gradebook_calculations_freeze_' . $courses[6]->id});

// System setting for grade letter boundaries (default).
set_config('grade_displaytype', '3');
Expand All @@ -673,15 +673,15 @@ public function test_upgrade_course_letter_boundary() {
// [10] A course with grade display settings of letters which are not default, but not affected.
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[10]->id}));
// [11] A course with grade display settings of letters which are not default, which will be affected.
$this->assertEquals(20160511, $CFG->{'gradebook_calculations_freeze_' . $courses[11]->id});
$this->assertEquals(20160518, $CFG->{'gradebook_calculations_freeze_' . $courses[11]->id});
// [12] A grade item with display settings that are not letters.
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[12]->id}));
// [13] A grade item with display settings of letters which are default.
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[13]->id}));
// [14] A grade item with display settings of letters which are not default, but not affected.
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[14]->id}));
// [15] A grade item with display settings of letters which are not default, which will be affected.
$this->assertEquals(20160511, $CFG->{'gradebook_calculations_freeze_' . $courses[15]->id});
$this->assertEquals(20160518, $CFG->{'gradebook_calculations_freeze_' . $courses[15]->id});

// System setting for grade letter boundaries (custom with problem).
$systemcontext = context_system::instance();
Expand All @@ -692,27 +692,27 @@ public function test_upgrade_course_letter_boundary() {
upgrade_course_letter_boundary();

// [16] A course with no grade display settings for the course or grade items.
$this->assertEquals(20160511, $CFG->{'gradebook_calculations_freeze_' . $courses[16]->id});
$this->assertEquals(20160518, $CFG->{'gradebook_calculations_freeze_' . $courses[16]->id});
// [17] A course with grade display settings, but for something that isn't letters.
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[17]->id}));
// [18] A course with grade display settings of letters which are default.
$this->assertEquals(20160511, $CFG->{'gradebook_calculations_freeze_' . $courses[18]->id});
$this->assertEquals(20160518, $CFG->{'gradebook_calculations_freeze_' . $courses[18]->id});
// [19] A course with grade display settings of letters which are not default, but not affected.
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $courses[19]->id}));
// [20] A course with grade display settings of letters which are not default, which will be affected.
$this->assertEquals(20160511, $CFG->{'gradebook_calculations_freeze_' . $courses[20]->id});
$this->assertEquals(20160518, $CFG->{'gradebook_calculations_freeze_' . $courses[20]->id});
// [21] A grade item with display settings which are not letters. Grade total will be affected so should be frozen.
$this->assertEquals(20160511, $CFG->{'gradebook_calculations_freeze_' . $courses[21]->id});
$this->assertEquals(20160518, $CFG->{'gradebook_calculations_freeze_' . $courses[21]->id});
// [22] A grade item with display settings of letters which are default.
$this->assertEquals(20160511, $CFG->{'gradebook_calculations_freeze_' . $courses[22]->id});
$this->assertEquals(20160518, $CFG->{'gradebook_calculations_freeze_' . $courses[22]->id});
// [23] 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[23]->id}));
// [24] A grade item with display settings of letters which are not default, which will be affected.
$this->assertEquals(20160511, $CFG->{'gradebook_calculations_freeze_' . $courses[24]->id});
$this->assertEquals(20160518, $CFG->{'gradebook_calculations_freeze_' . $courses[24]->id});
// [25] A course which is using the default grade display setting, but has updated the grade letter boundary (not 57) Should not be frozen.
$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});
$this->assertEquals(20160518, $CFG->{'gradebook_calculations_freeze_' . $courses[26]->id});

// System setting not showing letters.
set_config('grade_displaytype', '2');
Expand All @@ -726,19 +726,19 @@ public function test_upgrade_course_letter_boundary() {
// [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});
$this->assertEquals(20160518, $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});
$this->assertEquals(20160518, $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});
$this->assertEquals(20160518, $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}));
}
Expand Down

0 comments on commit 41abbbb

Please sign in to comment.