MDL-25524 workshop: fixed random allocator removing current allocations

The bug was in filter_current_assessments() being applied before get_unkept_assessments() so actually no current assessment records could be detected.
anovikov · Dec 7, 2010 · 95d28f0 · 95d28f0
1 parent 6eee65e
commit 95d28f0
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 6 deletions.
diff --git a/mod/workshop/allocation/random/lang/en/workshopallocation_random.php b/mod/workshop/allocation/random/lang/en/workshopallocation_random.php
@@ -27,6 +27,7 @@
 $string['addselfassessment'] = 'Add self-assessments';
 $string['allocationaddeddetail'] = 'New assessment to be done: <strong>{$a->reviewername}</strong> is reviewer of <strong>{$a->authorname}</strong>';
 $string['allocationdeallocategraded'] = 'Unable to deallocate already graded assessment: reviewer <strong>{$a->reviewername}</strong>, submission author: <strong>{$a->authorname}</strong>';
+$string['allocationreuseddetail'] = 'Reused assessment: <strong>{$a->reviewername}</strong> kept as reviewer of <strong>{$a->authorname}</strong>';
 $string['allocationsettings'] = 'Allocation settings';
 $string['assessmentdeleteddetail'] = 'Assessment deallocated: <strong>{$a->reviewername}</strong> is no longer reviewer of <strong>{$a->authorname}</strong>';
 $string['assesswosubmission'] = 'Participants can assess without having submitted anything';

diff --git a/mod/workshop/allocation/random/lib.php b/mod/workshop/allocation/random/lib.php
@@ -90,17 +90,13 @@ public function init() {
             $newallocations     = array();      // array of array(reviewer => reviewee)
 
             if ($numofreviews) {
-                // TODO MDL-19870 rewrite this part to make it easier to maintain and extend.
-                // $removecurrent -> remove it at the beginning and stop doing the magic with unkept allocation
-                // (leading to possible bugs)
                 if ($removecurrent) {
                     // behave as if there were no current assessments
                     $curassessments = array();
                 } else {
                     $curassessments = $assessments;
                 }
                 $randomallocations  = $this->random_allocation($authors, $reviewers, $curassessments, $numofreviews, $numper, $o);
-                $this->filter_current_assessments($randomallocations, $assessments);
                 $newallocations     = array_merge($newallocations, $randomallocations);
                 $o[] = 'ok::' . get_string('numofrandomlyallocatedsubmissions', 'workshopallocation_random', count($randomallocations));
                 unset($randomallocations);
@@ -114,13 +110,19 @@ public function init() {
             if (empty($newallocations)) {
                 $o[] = 'info::' . get_string('noallocationtoadd', 'workshopallocation_random');
             } else {
-                $this->add_new_allocations($newallocations, $authors, $reviewers);
+                $newnonexistingallocations = $newallocations;
+                $this->filter_current_assessments($newnonexistingallocations, $assessments);
+                $this->add_new_allocations($newnonexistingallocations, $authors, $reviewers);
                 foreach ($newallocations as $newallocation) {
                     list($reviewerid, $authorid) = each($newallocation);
                     $a                  = new stdclass();
                     $a->reviewername    = fullname($reviewers[0][$reviewerid]);
                     $a->authorname      = fullname($authors[0][$authorid]);
-                    $o[] = 'ok::indent::' . get_string('allocationaddeddetail', 'workshopallocation_random', $a);
+                    if (in_array($newallocation, $newnonexistingallocations)) {
+                        $o[] = 'ok::indent::' . get_string('allocationaddeddetail', 'workshopallocation_random', $a);
+                    } else {
+                        $o[] = 'ok::indent::' . get_string('allocationreuseddetail', 'workshopallocation_random', $a);
+                    }
                 }
             }
             if ($removecurrent) {