MDL-78082 core_grades: Fix locking inconsistency

Problem:
In Moodle 4.2, an unintended change in behaviour occurred when locking
and unlocking grade items. Previously, in Moodle 4.1, locking a grade
item and then subsequently unlocking the grade item left specific grades
for students still locked if they were being locked individually.
However, in Moodle 4.2, the unintended change caused the individual
grades to be unlocked along with the grade item.

Solution:
To address this issue, I modified the set_locked calls in action.php to
pass the cascade parameter as false. Now, when locking or unlocking a
grade item, only the grade item itself is affected, leaving individual
grades unchanged. I have also modified the locking behaviour in the
grade_category class. So, similarly, when locking or unlocking a grade
category, the change applies only to the grade items and grade
categories within the category, without expanding to individual grades.

Explanation:
By making this change, we ensure that the locking mechanism behaves as
expected, providing a consistent and desirable user experience. Locking
and unlocking a grade item or category will no longer inadvertently
affect individual grades, preserving their locked status and avoiding
loss of data when unlocking a grade item associated with the total mark
of a category.
This commit is contained in:
Shamim Rezaie
2023-09-07 05:52:30 +10:00
parent 3693a22ed7
commit 6100c0c548
3 changed files with 20 additions and 20 deletions
+2 -2
View File
@@ -92,7 +92,7 @@ switch ($action) {
if ($type == 'grade' and empty($object->id)) {
$object->insert();
}
$object->set_locked(1, true, true);
$object->set_locked(1, false, true);
}
break;
@@ -104,7 +104,7 @@ switch ($action) {
if ($type == 'grade' and empty($object->id)) {
$object->insert();
}
$object->set_locked(0, true, true);
$object->set_locked(0, false, true);
}
break;
+11 -15
View File
@@ -2536,25 +2536,21 @@ class grade_category extends grade_object {
$result = $this->grade_item->set_locked($lockedstate, $cascade, true);
if ($cascade) {
//process all children - items and categories
if ($children = grade_item::fetch_all(array('categoryid'=>$this->id))) {
// Process all children - items and categories.
if ($children = grade_item::fetch_all(['categoryid' => $this->id])) {
foreach ($children as $child) {
$child->set_locked($lockedstate, $cascade, false);
foreach ($children as $child) {
$child->set_locked($lockedstate, true, false);
if (empty($lockedstate) and $refresh) {
//refresh when unlocking
$child->refresh_grades();
}
if (empty($lockedstate) && $refresh) {
// Refresh when unlocking.
$child->refresh_grades();
}
}
}
if ($children = grade_category::fetch_all(array('parent'=>$this->id))) {
foreach ($children as $child) {
$child->set_locked($lockedstate, true, true);
}
if ($children = static::fetch_all(['parent' => $this->id])) {
foreach ($children as $child) {
$child->set_locked($lockedstate, $cascade, true);
}
}
+7 -3
View File
@@ -784,13 +784,17 @@ class grade_category_test extends \grade_base_testcase {
$category = new \grade_category($this->grade_categories[0]);
$this->assertTrue(method_exists($category, 'set_locked'));
// Will return false as cannot lock a grade that needs updating.
$this->assertFalse($category->set_locked(1));
// Even though a grade that needs updating cannot be locked, set_locked will return true because it will successfully
// schedule the locking for as soon as final grades are recalculated.
$this->assertTrue($category->set_locked(1));
// The category should not be locked yet as we are waiting for a recalculation.
$this->assertFalse($category->is_locked());
grade_regrade_final_grades($this->courseid);
// Get the category from the db again.
$category = new \grade_category($this->grade_categories[0]);
$this->assertTrue($category->set_locked(1));
// The category is locked now.
$this->assertTrue($category->is_locked());
}
protected function sub_test_grade_category_is_hidden() {