From 41abbbbde53a356526ce2ae04f895bb65e57c30f Mon Sep 17 00:00:00 2001
From: Adrian Greeve <adrian@moodle.com>
Date: Wed, 18 May 2016 09:45:34 +0800
Subject: [PATCH] MDL-45390 gradebook: sql tidy up and version alignment.
---
backup/moodle2/restore_stepslib.php | 6 ++--
...=> grade_letter_boundary_20160518.feature} | 2 +-
lib/db/upgrade.php | 2 +-
lib/db/upgradelib.php | 36 ++++++++-----------
lib/gradelib.php | 2 +-
lib/tests/upgradelib_test.php | 28 +++++++--------
6 files changed, 34 insertions(+), 42 deletions(-)
rename grade/tests/behat/{grade_letter_boundary_20160511.feature => grade_letter_boundary_20160518.feature} (99%)
diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php
index 163ccf92c35..158249b3d1a 100644
--- a/backup/moodle2/restore_stepslib.php
+++ b/backup/moodle2/restore_stepslib.php
@@ -506,10 +506,10 @@ class restore_gradebook_structure_step extends restore_structure_step {
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());
}
diff --git a/grade/tests/behat/grade_letter_boundary_20160511.feature b/grade/tests/behat/grade_letter_boundary_20160518.feature
similarity index 99%
rename from grade/tests/behat/grade_letter_boundary_20160511.feature
rename to grade/tests/behat/grade_letter_boundary_20160518.feature
index 197ba49f47a..47fcb565231 100644
--- a/grade/tests/behat/grade_letter_boundary_20160511.feature
+++ b/grade/tests/behat/grade_letter_boundary_20160518.feature
@@ -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 |
diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php
index 3a0c3cc8e10..22db5bf9567 100644
--- a/lib/db/upgrade.php
+++ b/lib/db/upgrade.php
@@ -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.
diff --git a/lib/db/upgradelib.php b/lib/db/upgradelib.php
index f618de2d910..6de6d846ab5 100644
--- a/lib/db/upgradelib.php
+++ b/lib/db/upgradelib.php
@@ -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();
@@ -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";
}
@@ -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);
}
}
}
diff --git a/lib/gradelib.php b/lib/gradelib.php
index 43d5b5e0cf8..b7906269e36 100644
--- a/lib/gradelib.php
+++ b/lib/gradelib.php
@@ -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.
diff --git a/lib/tests/upgradelib_test.php b/lib/tests/upgradelib_test.php
index 02bc6177084..07bc92261d1 100644
--- a/lib/tests/upgradelib_test.php
+++ b/lib/tests/upgradelib_test.php
@@ -649,13 +649,13 @@ class core_upgradelib_testcase extends advanced_testcase {
// [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');
@@ -673,7 +673,7 @@ class core_upgradelib_testcase extends advanced_testcase {
// [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.
@@ -681,7 +681,7 @@ class core_upgradelib_testcase extends advanced_testcase {
// [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();
@@ -692,27 +692,27 @@ class core_upgradelib_testcase extends advanced_testcase {
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');
@@ -726,11 +726,11 @@ class core_upgradelib_testcase extends advanced_testcase {
// [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.
@@ -738,7 +738,7 @@ class core_upgradelib_testcase extends advanced_testcase {
// [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}));
}