Skip to content

Commit

Permalink
Merge branch 'MDL-44330' of git://github.com/NeillM/moodle
Browse files Browse the repository at this point in the history
  • Loading branch information
David Monllao committed Nov 24, 2015
2 parents 2bc40fc + 3fca693 commit c506af7
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 25 deletions.
1 change: 1 addition & 0 deletions lib/tablelib.php
Expand Up @@ -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();
Expand Down
18 changes: 16 additions & 2 deletions mod/assign/gradingtable.php
Expand Up @@ -828,7 +828,8 @@ public function col_grade(stdClass $row) {
'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;
Expand Down Expand Up @@ -993,7 +994,8 @@ public function col_userid(stdClass $row) {

$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;

Expand Down Expand Up @@ -1396,4 +1398,16 @@ protected function show_hide_link($column, $index) {
}
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();
}
}
1 change: 1 addition & 0 deletions mod/assign/lang/en/assign.php
Expand Up @@ -446,6 +446,7 @@
$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';
Expand Down
66 changes: 52 additions & 14 deletions mod/assign/locallib.php
Expand Up @@ -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();

Expand Down Expand Up @@ -178,6 +181,9 @@ public function __construct($coursemodulecontext, $coursemodule, $course) {

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

/**
Expand Down Expand Up @@ -470,18 +476,18 @@ public function view($action='') {
$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';
Expand Down Expand Up @@ -514,7 +520,7 @@ public function view($action='') {
}

$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.
Expand Down Expand Up @@ -3006,16 +3012,16 @@ protected function view_single_grade_page($mform) {

// 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);
Expand Down Expand Up @@ -3393,6 +3399,14 @@ protected function view_grading_table() {
$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()) {
Expand Down Expand Up @@ -6080,9 +6094,9 @@ public function add_grade_form_elements(MoodleQuickForm $mform, stdClass $data,
$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);
Expand Down Expand Up @@ -6239,7 +6253,7 @@ public function add_grade_form_elements(MoodleQuickForm $mform, stdClass $data,
$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));
Expand Down Expand Up @@ -6934,13 +6948,15 @@ protected function process_save_grade(&$mform) {
$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);
Expand Down Expand Up @@ -7428,6 +7444,28 @@ public function get_grading_status($userid) {
}
}
}

/**
* 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;
}
}

/**
Expand Down
27 changes: 26 additions & 1 deletion mod/assign/tests/locallib_test.php
Expand Up @@ -2143,7 +2143,7 @@ public function test_feedback_comment_commentinline() {
$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);
Expand Down Expand Up @@ -2329,5 +2329,30 @@ public function test_get_shared_group_members() {
$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')));
}
}

15 changes: 7 additions & 8 deletions mod/assign/view.php
Expand Up @@ -27,23 +27,22 @@

$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);
Expand Down

0 comments on commit c506af7

Please sign in to comment.