Skip to content
Browse files

MDL-29482: added validation to rubric editor and the 'work in progres…

…s' status
  • Loading branch information...
1 parent 37f391f commit 2ae7faf128cea7b7a86737a388c83bc4fd9ad7e9 @marinaglancy marinaglancy committed Nov 2, 2011
View
13 grade/grading/form/lib.php
@@ -135,6 +135,19 @@ public function is_form_available() {
}
/**
+ * Returns a message why this form is unavailable. Maybe overriden by plugins to give more details.
+ * @see is_form_available()
+ *
+ * @return string
+ */
+ public function form_unavailable_notification() {
+ if ($this->is_form_available()) {
+ return null;
+ }
+ return get_string('gradingformunavailable', 'grades');
+ }
+
+ /**
* Returns URL of a page where the grading form can be defined and edited.
*
* @param moodle_url $returnurl optional URL of a page where the user should be sent once they are finished with editing
View
5 grade/grading/form/rubric/edit.php
@@ -43,16 +43,13 @@
$PAGE->set_url(new moodle_url('/grade/grading/form/rubric/edit.php', array('areaid' => $areaid)));
$PAGE->set_title(get_string('definerubric', 'gradingform_rubric'));
$PAGE->set_heading(get_string('definerubric', 'gradingform_rubric'));
-$PAGE->requires->js('/grade/grading/form/rubric/js/rubriceditor.js');
-//TODO freeze rubric editor if needed
-$mform = new gradingform_rubric_editrubric(null, array('areaid' => $areaid, 'context' => $context, 'freezerubric' => optional_param('freeze', 0, PARAM_INT)));
+$mform = new gradingform_rubric_editrubric(null, array('areaid' => $areaid, 'context' => $context));
$data = $controller->get_definition_for_editing();
$returnurl = optional_param('returnurl', $manager->get_management_url(), PARAM_LOCALURL);
$data->returnurl = $returnurl;
$mform->set_data($data);
if ($mform->is_cancelled()) {
- // todo process editing cancel in a better way
redirect($returnurl);
} else if ($mform->is_submitted() && $mform->is_validated()) {
$data = $mform->get_data();
View
68 grade/grading/form/rubric/edit_form.php
@@ -36,7 +36,7 @@
class gradingform_rubric_editrubric extends moodleform {
/**
- * Form elements definition
+ * Form element definition
*/
public function definition() {
$form = $this->_form;
@@ -56,22 +56,66 @@ public function definition() {
$form->addElement('editor', 'description_editor', get_string('description', 'gradingform_rubric'), null, $options);
$form->setType('description_editor', PARAM_RAW);
+ // rubric completion status
+ $choices = array();
+ $choices[gradingform_controller::DEFINITION_STATUS_WORKINPROGRESS] = get_string('statusworkinprogress', 'gradingform_rubric');
+ $choices[gradingform_controller::DEFINITION_STATUS_PRIVATE] = get_string('statusprivate', 'gradingform_rubric');
+ $choices[gradingform_controller::DEFINITION_STATUS_PUBLIC] = get_string('statuspublic', 'gradingform_rubric');
+ $form->addElement('select', 'status', 'Current rubric status', $choices)->freeze();
+
// rubric editor
$element = $form->addElement('rubriceditor', 'rubric', get_string('rubric', 'gradingform_rubric'));
$form->setType('rubric', PARAM_RAW);
- $form->addRule('rubric', '', 'rubriceditorcompleted'); //TODO how to add this rule automatically?????
- if (array_key_exists('freezerubric', $this->_customdata) && $this->_customdata['freezerubric']) {
- $element->freeze();
- }
+ //$element->freeze(); // TODO freeze rubric editor if needed
- // submit and cancel buttons
- $buttonarray = array(
- $form->createElement('submit', 'submitdraft', get_string('saveandcontinue', 'core_grading')),
- $form->createElement('submit', 'submitfinal', get_string('saveandmakeready', 'core_grading')),
- $form->createElement('cancel')
- );
+ $buttonarray = array();
+ $buttonarray[] = &$form->createElement('submit', 'saverubric', get_string('saverubric', 'gradingform_rubric'));
+ $buttonarray[] = &$form->createElement('submit', 'saverubricdraft', get_string('saverubricdraft', 'gradingform_rubric'));
+ $buttonarray[] = &$form->createElement('cancel');
$form->addGroup($buttonarray, 'buttonar', '', array(' '), false);
- $form->setType('buttonar', PARAM_RAW);
$form->closeHeaderBefore('buttonar');
}
+
+ /**
+ * Form vlidation.
+ * If there are errors return array of errors ("fieldname"=>"error message"),
+ * otherwise true if ok.
+ *
+ * @param array $data array of ("fieldname"=>value) of submitted data
+ * @param array $files array of uploaded files "element_name"=>tmp_file_path
+ * @return array of "element_name"=>"error_description" if there are errors,
+ * or an empty array if everything is OK (true allowed for backwards compatibility too).
+ */
+ function validation($data, $files) {
+ $err = parent::validation($data, $files);
+ $err = array();
+ $form = $this->_form;
+ $rubricel = $form->getElement('rubric');
+ if ($rubricel->non_js_button_pressed($data['rubric'])) {
+ // if JS is disabled and button such as 'Add criterion' is pressed - prevent from submit
+ $err['rubricdummy'] = 1;
+ } else if (isset($data['saverubric']) && $data['saverubric']) {
+ // If user attempts to make rubric active - it needs to be validated
+ if ($rubricel->validate($data['rubric']) !== false) {
+ $err['rubricdummy'] = 1;
+ }
+ }
+ return $err;
+ }
+
+ /**
+ * Return submitted data if properly submitted or returns NULL if validation fails or
+ * if there is no submitted data.
+ *
+ * @return object submitted data; NULL if not valid or not submitted or cancelled
+ */
+ function get_data() {
+ $data = parent::get_data();
+ if (!empty($data->saverubric)) {
+ $data->status = gradingform_controller::DEFINITION_STATUS_PUBLIC; // TODO ???
+ } else if (!empty($data->saverubricdraft)) {
+ $data->status = gradingform_controller::DEFINITION_STATUS_WORKINPROGRESS;
+ }
+ return $data;
+ }
}
View
14 grade/grading/form/rubric/lang/en/gradingform_rubric.php
@@ -55,3 +55,17 @@
$string['showscorestudent'] = 'Display points for each level to those being graded';
$string['enableremarks'] = 'Allow grader to add text remarks for each criteria';
$string['showremarksstudent'] = 'Show remarks to those being graded';
+
+$string['saverubric'] = 'Save rubric and make it available';
+$string['saverubricdraft'] = 'Save as draft';
+
+$string['statusworkinprogress'] = 'Work in progress';
+$string['statusprivate'] = 'Private';
+$string['statuspublic'] = 'Public';
+
+$string['err_nocriteria'] = 'Rubric must contain at least one criterion';
+$string['err_mintwolevels'] = 'Each criterion must have at least two levels';
+$string['err_nodescription'] = 'Criterion description can not be empty';
+$string['err_nodefinition'] = 'Level definition can not be empty';
+$string['err_scoreformat'] = 'Number of points for each level must be a valid non-negative number';
+$string['err_totalscore'] = 'Maximum number of points possible when graded by the rubric must be more than zero';
View
13 grade/grading/form/rubric/lib.php
@@ -136,6 +136,13 @@ public function update_definition(stdClass $newdefinition, $status = null, $user
}
}
foreach ($levelsdata as $levelid => $level) {
+ if (isset($level['score'])) {
+ $level['score'] = (float)$level['score'];
+ if ($level['score']<0) {
+ // TODO why we can't allow negative score for rubric?
+ $level['score'] = 0;
+ }
+ }
if (preg_match('/^NEWID\d+$/', $levelid)) {
// insert level into DB
$data = array('criterionid' => $id, 'definitionformat' => FORMAT_MOODLE); // TODO format is not supported yet
@@ -213,7 +220,11 @@ protected function load_definition() {
// pick the level data
if (!empty($record->rlid)) {
foreach (array('id', 'score', 'definition', 'definitionformat') as $fieldname) {
- $this->definition->rubric_criteria[$record->rcid]['levels'][$record->rlid][$fieldname] = $record->{'rl'.$fieldname};
+ $value = $record->{'rl'.$fieldname};
+ if ($fieldname == 'score') {
+ $value = (float)$value; // To prevent display like 1.00000
+ }
+ $this->definition->rubric_criteria[$record->rcid]['levels'][$record->rlid][$fieldname] = $value;
}
}
}
View
27 grade/grading/form/rubric/renderer.php
@@ -27,7 +27,7 @@
/**
* Grading method plugin renderer
*/
-class gradingform_rubric_renderer {
+class gradingform_rubric_renderer extends plugin_renderer_base {
/**
* This function returns html code for displaying criterion. Depending on $mode it may be the
@@ -82,9 +82,17 @@ public function criterion_template($mode, $options, $elementname = '{NAME}', $cr
}
$description = $criterion['description'];
}
- $criterion_template .= html_writer::tag('td', $description, array('class' => 'description', 'id' => '{NAME}-criteria-{CRITERION-id}-description'));
+ $descriptionclass = 'description';
+ if (isset($criterion['error_description'])) {
+ $descriptionclass .= ' error';
+ }
+ $criterion_template .= html_writer::tag('td', $description, array('class' => $descriptionclass, 'id' => '{NAME}-criteria-{CRITERION-id}-description'));
$levels_str_table = html_writer::tag('table', html_writer::tag('tr', $levels_str, array('id' => '{NAME}-criteria-{CRITERION-id}-levels')));
- $criterion_template .= html_writer::tag('td', $levels_str_table, array('class' => 'levels'));
+ $levelsclass = 'levels';
+ if (isset($criterion['error_levels'])) {
+ $levelsclass .= ' error';
+ }
+ $criterion_template .= html_writer::tag('td', $levels_str_table, array('class' => $levelsclass));
if ($mode == gradingform_rubric_controller::DISPLAY_EDIT_FULL) {
$value = get_string('criterionaddlevel', 'gradingform_rubric');
$button = html_writer::empty_tag('input', array('type' => 'submit', 'name' => '{NAME}[criteria][{CRITERION-id}][levels][addlevel]',
@@ -174,7 +182,11 @@ public function level_template($mode, $options, $elementname = '{NAME}', $criter
$level_template .= html_writer::empty_tag('input', array('type' => 'hidden', 'name' => '{NAME}[criteria][{CRITERION-id}][levelid]', 'value' => $level['id']));
}
$score = html_writer::tag('span', $score, array('id' => '{NAME}-criteria-{CRITERION-id}-levels-{LEVEL-id}-score'));
- $level_template .= html_writer::tag('div', $definition, array('class' => 'definition', 'id' => '{NAME}-criteria-{CRITERION-id}-levels-{LEVEL-id}-definition'));
+ $definitionclass = 'definition';
+ if (isset($level['error_definition'])) {
+ $definitionclass .= ' error';
+ }
+ $level_template .= html_writer::tag('div', $definition, array('class' => $definitionclass, 'id' => '{NAME}-criteria-{CRITERION-id}-levels-{LEVEL-id}-definition'));
$displayscore = true;
if (!$options['showscoreteacher'] && in_array($mode, array(gradingform_rubric_controller::DISPLAY_EVAL, gradingform_rubric_controller::DISPLAY_EVAL_FROZEN, gradingform_rubric_controller::DISPLAY_REVIEW))) {
$displayscore = false;
@@ -183,7 +195,11 @@ public function level_template($mode, $options, $elementname = '{NAME}', $criter
$displayscore = false;
}
if ($displayscore) {
- $level_template .= html_writer::tag('div', $score. get_string('scorepostfix', 'gradingform_rubric'), array('class' => 'score'));
+ $scoreclass = 'score';
+ if (isset($level['error_score'])) {
+ $scoreclass .= ' error';
+ }
+ $level_template .= html_writer::tag('div', $score. get_string('scorepostfix', 'gradingform_rubric'), array('class' => $scoreclass));
}
if ($mode == gradingform_rubric_controller::DISPLAY_EDIT_FULL) {
$value = get_string('leveldelete', 'gradingform_rubric');
@@ -333,7 +349,6 @@ public function display_rubric($criteria, $options, $mode, $elementname = null,
}
foreach ($criterion['levels'] as $levelid => $level) {
$level['id'] = $levelid;
- $level['score'] = (float)$level['score']; // otherwise the display will look like 1.00000
$level['class'] = $this->get_css_class_suffix($levelcnt++, sizeof($criterion['levels']) -1);
$level['checked'] = (isset($criterionvalue['levelid']) && ((int)$criterionvalue['levelid'] === $levelid));
if ($level['checked'] && ($mode == gradingform_rubric_controller::DISPLAY_EVAL_FROZEN || $mode == gradingform_rubric_controller::DISPLAY_REVIEW || $mode == gradingform_rubric_controller::DISPLAY_VIEW)) {
View
140 grade/grading/form/rubric/rubriceditor.php
@@ -26,13 +26,11 @@
require_once("HTML/QuickForm/input.php");
-// register file-related rules
-if (class_exists('HTML_QuickForm')) {
- HTML_QuickForm::registerRule('rubriceditorcompleted', 'callback', '_ruleIsCompleted', 'MoodleQuickForm_rubriceditor');
-}
-
class MoodleQuickForm_rubriceditor extends HTML_QuickForm_input {
public $_helpbutton = '';
+ protected $validationerrors = null; // null - undefined, false - no errors, string - error(s) text
+ protected $wasvalidated = false; // if element has already been validated
+ protected $nonjsbuttonpressed = false; // null - unknown, true/false - button was/wasn't pressed
function MoodleQuickForm_rubriceditor($elementName=null, $elementLabel=null, $attributes=null) {
parent::HTML_QuickForm_input($elementName, $elementLabel, $attributes);
@@ -50,7 +48,7 @@ function toHtml() {
global $PAGE;
$html = $this->_getTabs();
$renderer = $PAGE->get_renderer('gradingform_rubric');
- $data = $this->prepare_non_js_data();
+ $data = $this->prepare_data(null, $this->wasvalidated);
if (!$this->_flagFrozen) {
$mode = gradingform_rubric_controller::DISPLAY_EDIT_FULL;
$module = array('name'=>'gradingform_rubriceditor', 'fullpath'=>'/grade/grading/form/rubric/js/rubriceditor.js',
@@ -71,27 +69,41 @@ function toHtml() {
$mode = gradingform_rubric_controller::DISPLAY_PREVIEW;
}
}
+ if ($this->validationerrors) {
+ $html .= $renderer->notification($this->validationerrors, 'error');
+ }
$html .= $renderer->display_rubric($data['criteria'], $data['options'], $mode, $this->getName());
return $html;
}
/**
* Prepares the data passed in $_POST:
* - processes the pressed buttons 'addlevel', 'addcriterion', 'moveup', 'movedown', 'delete' (when JavaScript is disabled)
+ * sets $this->nonjsbuttonpressed to true/false if such button was pressed
* - if options not passed (i.e. we create a new rubric) fills the options array with the default values
* - if options are passed completes the options array with unchecked checkboxes
+ * - if $withvalidation is set, adds 'error_xxx' attributes to elements that contain errors and creates an error string
+ * and stores it in $this->validationerrors
*
* @param array $value
+ * @param boolean $withvalidation whether to enable data validation
* @return array
*/
- function prepare_non_js_data($value = null) {
+ function prepare_data($value = null, $withvalidation = false) {
if (null === $value) {
$value = $this->getValue();
}
+ if ($this->nonjsbuttonpressed === null) {
+ $this->nonjsbuttonpressed = false;
+ }
+ $totalscore = 0;
+ $errors = array();
$return = array('criteria' => array(), 'options' => gradingform_rubric_controller::get_default_options());
if (!isset($value['criteria'])) {
$value['criteria'] = array();
+ $errors['err_nocriteria'] = 1;
}
+ // If options are present in $value, replace default values with submitted values
if (!empty($value['options'])) {
foreach (array_keys($return['options']) as $option) {
// special treatment for checkboxes
@@ -102,14 +114,18 @@ function prepare_non_js_data($value = null) {
}
}
}
+
+ // iterate through criteria
$lastaction = null;
$lastid = null;
foreach ($value['criteria'] as $id => $criterion) {
if ($id == 'addcriterion') {
$id = $this->get_next_id(array_keys($value['criteria']));
$criterion = array('description' => '');
+ $this->nonjsbuttonpressed = true;
}
$levels = array();
+ $maxscore = null;
if (array_key_exists('levels', $criterion)) {
foreach ($criterion['levels'] as $levelid => $level) {
if ($levelid == 'addlevel') {
@@ -118,13 +134,41 @@ function prepare_non_js_data($value = null) {
'definition' => '',
'score' => 0,
);
+ $this->nonjsbuttonpressed = true;
}
if (!array_key_exists('delete', $level)) {
+ if ($withvalidation) {
+ if (empty($level['definition'])) {
+ $errors['err_nodefinition'] = 1;
+ $level['error_definition'] = true;
+ }
+ if (!preg_match('#^[\+]?\d*$#', trim($level['score'])) && !preg_match('#^[\+]?\d*[\.,]\d+$#', trim($level['score']))) {
+ // TODO why we can't allow negative score for rubric?
+ $errors['err_scoreformat'] = 1;
+ $level['error_score'] = true;
+ }
+ }
$levels[$levelid] = $level;
+ if ($maxscore === null || (float)$level['score'] > $maxscore) {
+ $maxscore = (float)$level['score'];
+ }
+ } else {
+ $this->nonjsbuttonpressed = true;
}
}
}
+ $totalscore += (float)$maxscore;
$criterion['levels'] = $levels;
+ if ($withvalidation && !array_key_exists('delete', $criterion)) {
+ if (count($levels)<2) {
+ $errors['err_mintwolevels'] = 1;
+ $criterion['error_levels'] = true;
+ }
+ if (empty($criterion['description'])) {
+ $errors['err_nodescription'] = 1;
+ $criterion['error_description'] = true;
+ }
+ }
if (array_key_exists('moveup', $criterion) || $lastaction == 'movedown') {
unset($criterion['moveup']);
if ($lastid !== null) {
@@ -137,20 +181,43 @@ function prepare_non_js_data($value = null) {
}
$lastaction = null;
$lastid = $id;
+ $this->nonjsbuttonpressed = true;
} else if (array_key_exists('delete', $criterion)) {
+ $this->nonjsbuttonpressed = true;
} else {
if (array_key_exists('movedown', $criterion)) {
unset($criterion['movedown']);
$lastaction = 'movedown';
+ $this->nonjsbuttonpressed = true;
}
$return['criteria'][$id] = $criterion;
$lastid = $id;
}
}
+
+ if ($totalscore <= 0) {
+ $errors['err_totalscore'] = 1;
+ }
+
+ // add sort order field to criteria
$csortorder = 1;
foreach (array_keys($return['criteria']) as $id) {
$return['criteria'][$id]['sortorder'] = $csortorder++;
}
+
+ // create validation error string (if needed)
+ if ($withvalidation) {
+ if (count($errors)) {
+ $rv = array();
+ foreach ($errors as $error => $v) {
+ $rv[] = get_string($error, 'gradingform_rubric');
+ }
+ $this->validationerrors = join('<br/ >', $rv);
+ } else {
+ $this->validationerrors = false;
+ }
+ $this->wasvalidated = true;
+ }
return $return;
}
@@ -164,52 +231,47 @@ function get_next_id($ids) {
return 'NEWID'.($maxid+1);
}
- function _ruleIsCompleted($elementValue) {
- //echo "_ruleIsCompleted";
- if (isset($elementValue['criteria'])) {
- foreach ($elementValue['criteria'] as $criterionid => $criterion) {
- if ($criterionid == 'addcriterion') {
- return false;
- }
- if (array_key_exists('moveup', $criterion) || array_key_exists('movedown', $criterion) || array_key_exists('delete', $criterion)) {
- return false;
- }
- if (array_key_exists('levels', $criterion) && is_array($criterion['levels'])) {
- foreach ($criterion['levels'] as $levelid => $level) {
- if ($levelid == 'addlevel') {
- return false;
- }
- if (array_key_exists('delete', $level)) {
- return false;
- }
- }
- }
- }
+ /**
+ * Checks if a submit button was pressed which is supposed to be processed on client side by JS
+ * but user seem to have disabled JS in the browser.
+ * (buttons 'add criteria', 'add level', 'move up', 'move down', etc.)
+ * In this case the form containing this element is prevented from being submitted
+ *
+ * @param array $value
+ * @return boolean true if non-submit button was pressed and not processed by JS
+ */
+ function non_js_button_pressed($value) {
+ if ($this->nonjsbuttonpressed === null) {
+ $this->prepare_data($value);
}
- //TODO check everything is filled
- //echo "<pre>";print_r($elementValue);echo "</pre>";
- return true;
+ return $this->nonjsbuttonpressed;
}
- function onQuickFormEvent($event, $arg, &$caller)
- {
- $name = $this->getName();
- if ($name && $caller->elementExists($name)) {
- $caller->addRule($name, '', 'rubriceditorcompleted');
+ /**
+ * Validates that rubric has at least one criterion, at least two levels within one criterion,
+ * each level has a valid score, all levels have filled definitions and all criteria
+ * have filled descriptions
+ *
+ * @param array $value
+ * @return string|false error text or false if no errors found
+ */
+ function validate($value) {
+ if (!$this->wasvalidated) {
+ $this->prepare_data($value, true);
}
- return parent::onQuickFormEvent($event, $arg, $caller);
+ return $this->validationerrors;
}
/**
* Prepares the data for saving
- * @see prepare_non_js_data
+ * @see prepare_data()
*
* @param array $submitValues
* @param boolean $assoc
* @return array
*/
function exportValue(&$submitValues, $assoc = false) {
- $value = $this->prepare_non_js_data($this->_findValue($submitValues));
+ $value = $this->prepare_data($this->_findValue($submitValues));
return $this->_prepareValue($value, $assoc);
}
}
View
5 grade/grading/form/rubric/styles.css
@@ -103,3 +103,8 @@
.gradingform_rubric .options .option {padding-bottom:2px;}
.gradingform_rubric .options .option label {margin-left: 5px;}
.gradingform_rubric .options .option .value {margin-left: 5px;font-weight:bold;}
+
+.gradingform_rubric .criterion .levels.error { border:1px solid red;}
+.gradingform_rubric .criterion .description.error,
+.gradingform_rubric .criterion .levels .level .definition.error,
+.gradingform_rubric .criterion .levels .level .score.error {background:#FFDDDD;}
View
4 grade/grading/manage.php
@@ -220,6 +220,10 @@
}
echo $output->container_end();
+ // display the message if the form is currently not available (if applicable)
+ if ($message = $controller->form_unavailable_notification()) {
+ echo $output->notification($message);
+ }
// display the grading form preview
if ($controller->is_form_defined()) {
if ($definition->status == gradingform_controller::DEFINITION_STATUS_READY) {
View
1 lang/en/grades.php
@@ -289,6 +289,7 @@
Only value and scale grade types may be aggregated. The grade type for an activity-based grade item is set on the activity settings page.';
$string['gradeview'] = 'View grade';
$string['gradeweighthelp'] = 'Grade weight help';
+$string['gradingformunavailable'] = 'Please note: this form is not available for grading at the moment. Simple grading method will be used until form has a valid status.';
$string['groupavg'] = 'Group average';
$string['hidden'] = 'Hidden';
$string['hidden_help'] = 'If ticked, grades are hidden from students. A hidden until date may be set if desired, to release grades after grading is completed.';

0 comments on commit 2ae7faf

Please sign in to comment.
Something went wrong with that request. Please try again.