diff --git a/lib/tablelib.php b/lib/tablelib.php
index e2c4b7f2c5e..53b64a6ef54 100644
--- a/lib/tablelib.php
+++ b/lib/tablelib.php
@@ -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();
diff --git a/mod/assign/gradingtable.php b/mod/assign/gradingtable.php
index 6ffecb8f5ff..91a5e429be7 100644
--- a/mod/assign/gradingtable.php
+++ b/mod/assign/gradingtable.php
@@ -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();
+ }
}
diff --git a/mod/assign/lang/en/assign.php b/mod/assign/lang/en/assign.php
index fea73ea735a..8d5d47a2682 100644
--- a/mod/assign/lang/en/assign.php
+++ b/mod/assign/lang/en/assign.php
@@ -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';
diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php
index aad3f96e1ff..dd61e455025 100644
--- a/mod/assign/locallib.php
+++ b/mod/assign/locallib.php
@@ -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;
+ }
}
/**
diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php
index af74eeb293b..30a55b1ba0a 100644
--- a/mod/assign/tests/locallib_test.php
+++ b/mod/assign/tests/locallib_test.php
@@ -2143,7 +2143,7 @@ Anchor link 2:Link text
$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:Link text
$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')));
+ }
}
diff --git a/mod/assign/view.php b/mod/assign/view.php
index 0bcae0c23cd..01d0edd2d46 100644
--- a/mod/assign/view.php
+++ b/mod/assign/view.php
@@ -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);