Skip to content

Commit

Permalink
MDL-32932 mod_assign: Coding style clean up and security improvement
Browse files Browse the repository at this point in the history
  • Loading branch information
Sam Hemelryk committed May 22, 2012
1 parent c5b8ae0 commit 2a4fbc3
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 53 deletions.
4 changes: 2 additions & 2 deletions mod/assign/gradingtable.php
View file
Expand Up @@ -229,14 +229,15 @@ function get_rows_per_page() {
*
* @param string $grade
* @param boolean $editable
* @param int $userid The user id of the user this grade belongs to
* @param int $modified Timestamp showing when the grade was last modified
* @return string The formatted grade
*/
function display_grade($grade, $editable, $userid, $modified) {
if ($this->is_downloading()) {
return $grade;
}
$o = $this->assignment->display_grade($grade, $editable, $userid, $modified);

return $o;
}

Expand Down Expand Up @@ -342,7 +343,6 @@ function col_grade(stdClass $row) {

$grade = $this->display_grade($row->grade, $this->quickgrading, $row->userid, $row->timemarked);


//return $grade . $separator . $link;
return $link . $separator . $grade;
}
Expand Down
2 changes: 1 addition & 1 deletion mod/assign/lang/en/assign.php
View file
Expand Up @@ -101,7 +101,7 @@
It is <a href="{$a->url}">available on the web site</a>.';
$string['enabled'] = 'Enabled';
$string['errornosubmissions'] = 'There are no submissions to download';
$string['errorquickgradingnotcompatiblewithadvancedgrading'] = 'The grades were not saved because this assignment is currently using advanced grading';
$string['errorquickgradingvsadvancedgrading'] = 'The grades were not saved because this assignment is currently using advanced grading';
$string['errorrecordmodified'] = 'The grades were not saved because someone has modified one or more records more recently than when you loaded the page.';
$string['feedbackcomments'] = 'Feedback comments';
$string['feedback'] = 'Feedback';
Expand Down
51 changes: 27 additions & 24 deletions mod/assign/locallib.php
View file
Expand Up @@ -889,25 +889,30 @@ public function get_course() {
*
* @param mixed $grade int|null
* @param boolean $editing Are we allowing changes to this grade?
* @param int $userid The user id the grade belongs to
* @param int $modified Timestamp from when the grade was last modified
* @return string User-friendly representation of grade
*/
public function display_grade($grade, $editing, $userid=0, $modified=0) {
global $DB;

static $scalegrades = array();

if ($this->get_instance()->grade >= 0) { // Normal number
if ($this->get_instance()->grade >= 0) {
// Normal number
if ($editing) {
$o = '<input type="text" name="quickgrade_' . $userid . '" value="' . $grade . '" size="6" maxlength="10" class="quickgrade"/>';
$o .= '&nbsp;/&nbsp;' . format_float($this->get_instance()->grade,2);
$o .= '<input type="hidden" name="grademodified_' . $userid . '" value="' . $modified . '"/>';
return $o;
} else if ($grade == -1 || $grade === null) {
return '-';
} else {
return format_float(($grade),2) .'&nbsp;/&nbsp;'. format_float($this->get_instance()->grade,2);
}

} else { // Scale
} else {
// Scale
if (empty($this->cache['scale'])) {
if ($scale = $DB->get_record('scale', array('id'=>-($this->get_instance()->grade)))) {
$this->cache['scale'] = make_menu_from_list($scale->scale);
Expand Down Expand Up @@ -2319,7 +2324,8 @@ private function process_submit_for_grading() {
/**
* save quick grades
*
* @return string - The result of the save operation
* @global moodle_database $DB
* @return string The result of the save operation
*/
private function process_save_quick_grades() {
global $USER, $DB, $CFG;
Expand All @@ -2331,36 +2337,37 @@ private function process_save_quick_grades() {
$gradingmanager = get_grading_manager($this->get_context(), 'mod_assign', 'submissions');
$controller = $gradingmanager->get_active_controller();
if (!empty($controller)) {
return get_string('errorquickgradingnotcompatiblewithadvancedgrading', 'assign');
return get_string('errorquickgradingvsadvancedgrading', 'assign');
}

$users = array();
// first check all the last modified values
$len = strlen('grademodified_');
// Using POST is really unfortunate. A better solution needs to be found here. As we are looking for grades students we could
// gets a list of possible users and look for values based upon that.
foreach ($_POST as $key => $value) {
if (substr($key, 0, $len) === 'grademodified_') {
if (preg_match('#^grademodified_(\d+)$#', $key, $matches)) {
// gather the userid, updated grade and last modified value
$record = new stdClass();
$record->userid = (int)substr($key, $len);
$record->userid = (int)$matches[1];
$record->grade = required_param('quickgrade_' . $record->userid, PARAM_INT);
$record->lastmodified = $value;
$record->lastmodified = clean_param($value, PARAM_INT);
$record->gradinginfo = grade_get_grades($this->get_course()->id, 'mod', 'assign', $this->get_instance()->id, array($record->userid));
$users[$record->userid] = $record;
}
}
list($userids, $useridparams) = $DB->get_in_or_equal(array_keys($users));
if (empty($users)) {
// Quick check to see whether we have any users to update and we don't
return get_string('quickgradingchangessaved', 'assign'); // Technical lie
}

list($userids, $params) = $DB->get_in_or_equal(array_keys($users), SQL_PARAMS_NAMED);
$params['assignment'] = $this->get_instance()->id;
// check them all for currency
$currentgrades = $DB->get_recordset_sql('SELECT u.id as userid,
g.grade as grade,
g.timemodified as lastmodified
FROM {user} u
LEFT JOIN {assign_grades} g ON
u.id = g.userid AND
g.assignment = ? WHERE u.id ' .
$userids,
array_merge(array($this->get_instance()->id),
$useridparams));
$sql = 'SELECT u.id as userid, g.grade as grade, g.timemodified as lastmodified
FROM {user} u
LEFT JOIN {assign_grades} g ON u.id = g.userid AND g.assignment = :assignment
WHERE u.id ' . $userids;
$currentgrades = $DB->get_recordset_sql($sql, $params);

$modifiedusers = array();
foreach ($currentgrades as $current) {
Expand Down Expand Up @@ -2396,6 +2403,7 @@ private function process_save_quick_grades() {
}

}
$currentgrades->close();

// ok - ready to process the updates
foreach ($modifiedusers as $userid => $modified) {
Expand All @@ -2420,8 +2428,6 @@ private function process_save_quick_grades() {
}
}

$user = $DB->get_record('user', array('id' => $userid), '*', MUST_EXIST);

$this->add_to_log('grade submission', $this->format_grade_for_log($grade));
}

Expand All @@ -2442,10 +2448,7 @@ private function process_save_grading_options() {
// Need submit permission to submit an assignment
require_capability('mod/assign:grade', $this->context);



$mform = new mod_assign_grading_options_form(null, array('cm'=>$this->get_course_module()->id, 'contextid'=>$this->context->id, 'userid'=>$USER->id, 'showquickgrading'=>false));

if ($formdata = $mform->get_data()) {
set_user_preference('assign_perpage', $formdata->perpage);
set_user_preference('assign_filter', $formdata->filter);
Expand Down
34 changes: 16 additions & 18 deletions mod/assign/module.js
View file
Expand Up @@ -101,34 +101,32 @@ M.mod_assign.init_grading_table = function(Y) {


});
quickgrade = Y.all('.gradingtable .quickgrade');
var quickgrade = Y.all('.gradingtable .quickgrade');
quickgrade.each(function(quick) {
quick.on('change', function(e) {
parent = this.get('parentNode');
parent.addClass('quickgrademodified');
this.get('parentNode').addClass('quickgrademodified');
});
});
});
};

M.mod_assign.check_dirty_quickgrading_form = function(e) {
if (!M.core_formchangechecker.get_form_dirty_state()) {
// the form is not dirty, so don't display any message
return;
}

if (!M.core_formchangechecker.get_form_dirty_state()) {
// the form is not dirty, so don't display any message
return;
}

// This is the error message that we'll show to browsers which support it
var warningmessage = 'There are unsaved quickgrading changes. Do you really wanto to leave this page?';
// This is the error message that we'll show to browsers which support it
var warningmessage = 'There are unsaved quickgrading changes. Do you really wanto to leave this page?';

// Most browsers are happy with the returnValue being set on the event
// But some browsers do not consistently pass the event
if (e) {
e.returnValue = warningmessage;
}
// Most browsers are happy with the returnValue being set on the event
// But some browsers do not consistently pass the event
if (e) {
e.returnValue = warningmessage;
}

// But some require it to be returned instead
return warningmessage;
// But some require it to be returned instead
return warningmessage;
}
M.mod_assign.init_grading_options = function(Y) {
Y.use('node', function(Y) {
Expand All @@ -153,7 +151,7 @@ M.mod_assign.init_grading_options = function(Y) {
// override the default dirty form behaviour to ignore any input with the class "ignoredirty"
M.mod_assign.set_form_changed = M.core_formchangechecker.set_form_changed;
M.core_formchangechecker.set_form_changed = function(e) {
target = e.currentTarget;
var target = e.currentTarget;
if (!target.hasClass('ignoredirty')) {
M.mod_assign.set_form_changed(e);
}
Expand Down
9 changes: 3 additions & 6 deletions mod/assign/renderer.php
View file
Expand Up @@ -92,15 +92,12 @@ private function add_table_row_tuple(html_table $table, $first, $second) {
* @return string
*/
public function render_assign_quickgrading_result(assign_quickgrading_result $result) {
$url = new moodle_url('/mod/assign/view.php', array('id' => $result->coursemoduleid, 'action'=>'grading'));

$o = '';
$o .= $this->output->heading(get_string('quickgradingresult', 'assign'), 4);

$o .= $this->output->notification($result->message);

$o .= $this->output->continue_button(new moodle_url('/mod/assign/view.php',
array('id' => $result->coursemoduleid,
'action'=>'grading')));

$o .= $this->output->continue_button($url);
return $o;
}

Expand Down
3 changes: 1 addition & 2 deletions mod/assign/styles.css
View file
Expand Up @@ -127,5 +127,4 @@ div.earlysubmission {

#page-mod-assign-view div.gradingtable tr .quickgrademodified {
background-color: #FFCC99;
}

}

0 comments on commit 2a4fbc3

Please sign in to comment.