Skip to content

Commit

Permalink
MDL-78082 core_grades: Fix locking inconsistency
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rezaies committed Sep 6, 2023
1 parent 09ef2d0 commit 4cf1115
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 20 deletions.
4 changes: 2 additions & 2 deletions grade/edit/tree/action.php
Expand Up @@ -92,7 +92,7 @@
if ($type == 'grade' and empty($object->id)) {
$object->insert();
}
$object->set_locked(1, true, true);
$object->set_locked(1, false, true);
}
break;

Expand All @@ -104,7 +104,7 @@
if ($type == 'grade' and empty($object->id)) {
$object->insert();
}
$object->set_locked(0, true, true);
$object->set_locked(0, false, true);
}
break;

Expand Down
26 changes: 11 additions & 15 deletions lib/grade/grade_category.php
Expand Up @@ -2536,25 +2536,21 @@ public function set_locked($lockedstate, $cascade=false, $refresh=true) {

$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))) {

foreach ($children as $child) {
$child->set_locked($lockedstate, true, false);
// 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);

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);
}
}

Expand Down
10 changes: 7 additions & 3 deletions lib/grade/tests/grade_category_test.php
Expand Up @@ -784,13 +784,17 @@ protected function sub_test_grade_category_set_locked() {
$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() {
Expand Down

0 comments on commit 4cf1115

Please sign in to comment.