Merge branch 'MDL-44330' of git://github.com/NeillM/moodle
authorDavid Monllao <davidm@moodle.com>
Tue, 24 Nov 2015 09:03:06 +0000 (17:03 +0800)
committerDavid Monllao <davidm@moodle.com>
Tue, 24 Nov 2015 09:03:06 +0000 (17:03 +0800)
lib/tablelib.php
mod/assign/gradingtable.php
mod/assign/lang/en/assign.php
mod/assign/locallib.php
mod/assign/tests/locallib_test.php
mod/assign/view.php

index e2c4b7f..53b64a6 100644 (file)
@@ -67,6 +67,7 @@ class flexible_table {
     var $column_suppress = array();
     var $column_nosort   = array('userpic');
     private $column_textsort = array();
+    /** @var boolean Stores if setup has already been called on this flixible table. */
     var $setup           = false;
     var $baseurl         = NULL;
     var $request         = array();
index 5ca0f11..ca436bb 100644 (file)
@@ -828,7 +828,8 @@ class assign_grading_table extends table_sql implements renderable {
                                             'mod_assign');
             $urlparams = array('id' => $this->assignment->get_course_module()->id,
                                'rownum'=>$this->rownum,
-                               'action'=>'grade');
+                               'action' => 'grade',
+                               'useridlistid' => $this->assignment->get_useridlist_key_id());
             $url = new moodle_url('/mod/assign/view.php', $urlparams);
             $link = $this->output->action_link($url, $icon);
             $grade .= $link . $separator;
@@ -993,7 +994,8 @@ class assign_grading_table extends table_sql implements renderable {
 
         $urlparams = array('id'=>$this->assignment->get_course_module()->id,
                            'rownum'=>$this->rownum,
-                           'action'=>'grade');
+                           'action' => 'grade',
+                           'useridlistid' => $this->assignment->get_useridlist_key_id());
         $url = new moodle_url('/mod/assign/view.php', $urlparams);
         $noimage = null;
 
@@ -1396,4 +1398,16 @@ class assign_grading_table extends table_sql implements renderable {
         }
         return '';
     }
+
+    /**
+     * Overides setup to ensure it will only run a single time.
+     */
+    public function setup() {
+        // Check if the setup function has been called before, we should not run it twice.
+        // If we do the sortorder of the table will be broken.
+        if (!empty($this->setup)) {
+            return;
+        }
+        parent::setup();
+    }
 }
index fea73ea..8d5d47a 100644 (file)
@@ -446,6 +446,7 @@ $string['updategrade'] = 'Update grade';
 $string['updatetable'] = 'Save and update table';
 $string['upgradenotimplemented'] = 'Upgrade not implemented in plugin ({$a->type} {$a->subtype})';
 $string['userextensiondate'] = 'Extension granted until: {$a}';
+$string['useridlistnotcached'] = 'Aborting save. Moodle could not determine which user to save the grade against.';
 $string['userswhoneedtosubmit'] = 'Users who need to submit: {$a}';
 $string['usergrade'] = 'User grade';
 $string['validmarkingworkflowstates'] = 'Valid marking workflow states';
index aad3f96..dd61e45 100644 (file)
@@ -140,6 +140,9 @@ class assign {
     /** @var bool whether to exclude users with inactive enrolment */
     private $showonlyactiveenrol = null;
 
+    /** @var string A key used to identify cached userlists created by this object. */
+    private $useridlistid = null;
+
     /** @var array cached list of participants for this assignment. The cache key will be group, showactive and the context id */
     private $participants = array();
 
@@ -178,6 +181,9 @@ class assign {
 
         $this->submissionplugins = $this->load_plugins('assignsubmission');
         $this->feedbackplugins = $this->load_plugins('assignfeedback');
+
+        // Extra entropy is required for uniqid() to work on cygwin.
+        $this->useridlistid = clean_param(uniqid('', true), PARAM_ALPHANUM);
     }
 
     /**
@@ -470,18 +476,18 @@ class assign {
                     $action = 'redirect';
                     $nextpageparams['action'] = 'grade';
                     $nextpageparams['rownum'] = optional_param('rownum', 0, PARAM_INT) + 1;
-                    $nextpageparams['useridlistid'] = optional_param('useridlistid', time(), PARAM_INT);
+                    $nextpageparams['useridlistid'] = optional_param('useridlistid', $this->get_useridlist_key_id(), PARAM_ALPHANUM);
                 }
             } else if (optional_param('nosaveandprevious', null, PARAM_RAW)) {
                 $action = 'redirect';
                 $nextpageparams['action'] = 'grade';
                 $nextpageparams['rownum'] = optional_param('rownum', 0, PARAM_INT) - 1;
-                $nextpageparams['useridlistid'] = optional_param('useridlistid', time(), PARAM_INT);
+                $nextpageparams['useridlistid'] = optional_param('useridlistid', $this->get_useridlist_key_id(), PARAM_ALPHANUM);
             } else if (optional_param('nosaveandnext', null, PARAM_RAW)) {
                 $action = 'redirect';
                 $nextpageparams['action'] = 'grade';
                 $nextpageparams['rownum'] = optional_param('rownum', 0, PARAM_INT) + 1;
-                $nextpageparams['useridlistid'] = optional_param('useridlistid', time(), PARAM_INT);
+                $nextpageparams['useridlistid'] = optional_param('useridlistid', $this->get_useridlist_key_id(), PARAM_ALPHANUM);
             } else if (optional_param('savegrade', null, PARAM_RAW)) {
                 // Save changes button.
                 $action = 'grade';
@@ -514,7 +520,7 @@ class assign {
         }
 
         $returnparams = array('rownum'=>optional_param('rownum', 0, PARAM_INT),
-                              'useridlistid' => optional_param('useridlistid', time(), PARAM_INT));
+                              'useridlistid' => optional_param('useridlistid', $this->get_useridlist_key_id(), PARAM_ALPHANUM));
         $this->register_return_link($action, $returnparams);
 
         // Now show the right view page.
@@ -3006,16 +3012,16 @@ class assign {
 
         // If userid is passed - we are only grading a single student.
         $rownum = required_param('rownum', PARAM_INT);
-        $useridlistid = optional_param('useridlistid', time(), PARAM_INT);
+        $useridlistid = optional_param('useridlistid', $this->get_useridlist_key_id(), PARAM_ALPHANUM);
         $userid = optional_param('userid', 0, PARAM_INT);
         $attemptnumber = optional_param('attemptnumber', -1, PARAM_INT);
 
         $cache = cache::make_from_params(cache_store::MODE_SESSION, 'mod_assign', 'useridlist');
         if (!$userid) {
-            if (!$useridlist = $cache->get($this->get_course_module()->id . '_' . $useridlistid)) {
+            if (!$useridlist = $cache->get($this->get_useridlist_key($useridlistid))) {
                 $useridlist = $this->get_grading_userid_list();
             }
-            $cache->set($this->get_course_module()->id . '_' . $useridlistid, $useridlist);
+            $cache->set($this->get_useridlist_key($useridlistid), $useridlist);
         } else {
             $rownum = 0;
             $useridlist = array($userid);
@@ -3393,6 +3399,14 @@ class assign {
             $o .= $this->get_renderer()->render($gradingtable);
         }
 
+        if ($this->can_grade()) {
+            // We need to cache the order of uses in the table as the person may wish to grade them.
+            // This is done based on the row number of the user.
+            $cache = cache::make_from_params(cache_store::MODE_SESSION, 'mod_assign', 'useridlist');
+            $useridlist = $gradingtable->get_column_data('userid');
+            $cache->set($this->get_useridlist_key(), $useridlist);
+        }
+
         $currentgroup = groups_get_activity_group($this->get_course_module(), true);
         $users = array_keys($this->list_participants($currentgroup, true));
         if (count($users) != 0 && $this->can_grade()) {
@@ -6080,9 +6094,9 @@ class assign {
         $attemptnumber = $params['attemptnumber'];
         if (!$userid) {
             $cache = cache::make_from_params(cache_store::MODE_SESSION, 'mod_assign', 'useridlist');
-            if (!$useridlist = $cache->get($this->get_course_module()->id . '_' . $useridlistid)) {
+            if (!$useridlist = $cache->get($this->get_useridlist_key($useridlistid))) {
                 $useridlist = $this->get_grading_userid_list();
-                $cache->set($this->get_course_module()->id . '_' . $useridlistid, $useridlist);
+                $cache->set($this->get_useridlist_key($useridlistid), $useridlist);
             }
         } else {
             $useridlist = array($userid);
@@ -6239,7 +6253,7 @@ class assign {
         $mform->setType('rownum', PARAM_INT);
         $mform->setConstant('rownum', $rownum);
         $mform->addElement('hidden', 'useridlistid', $useridlistid);
-        $mform->setType('useridlistid', PARAM_INT);
+        $mform->setType('useridlistid', PARAM_ALPHANUM);
         $mform->addElement('hidden', 'attemptnumber', $attemptnumber);
         $mform->setType('attemptnumber', PARAM_INT);
         $mform->addElement('hidden', 'ajax', optional_param('ajax', 0, PARAM_INT));
@@ -6934,13 +6948,15 @@ class assign {
         $instance = $this->get_instance();
         $rownum = required_param('rownum', PARAM_INT);
         $attemptnumber = optional_param('attemptnumber', -1, PARAM_INT);
-        $useridlistid = optional_param('useridlistid', time(), PARAM_INT);
+        $useridlistid = optional_param('useridlistid', $this->get_useridlist_key_id(), PARAM_ALPHANUM);
         $userid = optional_param('userid', 0, PARAM_INT);
         $cache = cache::make_from_params(cache_store::MODE_SESSION, 'mod_assign', 'useridlist');
         if (!$userid) {
-            if (!$useridlist = $cache->get($this->get_course_module()->id . '_' . $useridlistid)) {
-                $useridlist = $this->get_grading_userid_list();
-                $cache->set($this->get_course_module()->id . '_' . $useridlistid, $useridlist);
+            if (!$useridlist = $cache->get($this->get_useridlist_key($useridlistid))) {
+                // If the userid list is not cached we must not save, as it is possible that the user in a
+                // given row position may not be the same now as when the grading page was generated.
+                $url = new moodle_url('/mod/assign/view.php', array('id' => $this->get_course_module()->id));
+                throw new moodle_exception('useridlistnotcached', 'mod_assign', $url);
             }
         } else {
             $useridlist = array($userid);
@@ -7428,6 +7444,28 @@ class assign {
             }
         }
     }
+
+    /**
+     * The id used to uniquily identify the cache for this instance of the assign object.
+     *
+     * @return string
+     */
+    public function get_useridlist_key_id() {
+        return $this->useridlistid;
+    }
+
+    /**
+     * Generates the key that should be used for an entry in the useridlist cache.
+     *
+     * @param string $id Generate a key for this instance (optional)
+     * @return string The key for the id, or new entry if no $id is passed.
+     */
+    public function get_useridlist_key($id = null) {
+        if ($id === null) {
+            $id = $this->get_useridlist_key_id();
+        }
+        return $this->get_course_module()->id . '_' . $id;
+    }
 }
 
 /**
index af74eeb..30a55b1 100644 (file)
@@ -2143,7 +2143,7 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
         $pagination = array('userid'=>$this->students[0]->id,
                             'rownum'=>0,
                             'last'=>true,
-                            'useridlistid'=>time(),
+                            'useridlistid' => $assign->get_useridlist_key_id(),
                             'attemptnumber'=>0);
         $formparams = array($assign, $data, $pagination);
         $mform = new mod_assign_grade_form(null, $formparams);
@@ -2329,5 +2329,30 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
         $this->assertTrue(in_array($this->extrastudents[0]->id, $allgroupmembers));
         $this->assertTrue(in_array($this->extrastudents[1]->id , $allgroupmembers));
     }
+
+    /**
+     * Test that the useridlist cache will retive the correct values
+     * when using assign::get_useridlist_key and assign::get_useridlist_key_id.
+     */
+    public function test_useridlist_cache() {
+        // Create an assignment object, we will use this to test the key generation functions.
+        $course = self::getDataGenerator()->create_course();
+        $assign = self::getDataGenerator()->create_module('assign', array('course' => $course->id));
+        list($courserecord, $cm) = get_course_and_cm_from_instance($assign->id, 'assign');
+        $context = context_module::instance($cm->id);
+        $assign = new assign($context, $cm, $courserecord);
+        // Create the cache.
+        $cache = cache::make_from_params(cache_store::MODE_SESSION, 'mod_assign', 'useridlist');
+        // Create an entry that we will insert into the cache.
+        $entry = array(0 => '5', 1 => '6325', 2 => '67783');
+        // Insert the value into the cache.
+        $cache->set($assign->get_useridlist_key(), $entry);
+        // Now test we can retrive the entry.
+        $this->assertEquals($entry, $cache->get($assign->get_useridlist_key()));
+        $useridlistid = clean_param($assign->get_useridlist_key_id(), PARAM_ALPHANUM);
+        $this->assertEquals($entry, $cache->get($assign->get_useridlist_key($useridlistid)));
+        // Check it will not retrive anything on an invalid key.
+        $this->assertFalse($cache->get($assign->get_useridlist_key('notvalid')));
+    }
 }
 
index 0bcae0c..01d0edd 100644 (file)
@@ -27,23 +27,22 @@ require_once($CFG->dirroot . '/mod/assign/locallib.php');
 
 $id = required_param('id', PARAM_INT);
 
-$urlparams = array('id' => $id,
-                  'action' => optional_param('action', '', PARAM_TEXT),
-                  'rownum' => optional_param('rownum', 0, PARAM_INT),
-                  'useridlistid' => optional_param('useridlistid', time(), PARAM_INT));
-
-$url = new moodle_url('/mod/assign/view.php', $urlparams);
-
 list ($course, $cm) = get_course_and_cm_from_cmid($id, 'assign');
 
 require_login($course, true, $cm);
-$PAGE->set_url($url);
 
 $context = context_module::instance($cm->id);
 
 require_capability('mod/assign:view', $context);
 
 $assign = new assign($context, $cm, $course);
+$urlparams = array('id' => $id,
+                  'action' => optional_param('action', '', PARAM_TEXT),
+                  'rownum' => optional_param('rownum', 0, PARAM_INT),
+                  'useridlistid' => optional_param('useridlistid', $assign->get_useridlist_key_id(), PARAM_ALPHANUM));
+
+$url = new moodle_url('/mod/assign/view.php', $urlparams);
+$PAGE->set_url($url);
 
 $completion=new completion_info($course);
 $completion->set_module_viewed($cm);