Merge pull request #20 from marinaglancy/wip-nov02-rubric

grade/grading/form/lib.php

@@ -122,16 +122,20 @@ public function is_form_defined() {
      * @return boolean
      */
     public function is_form_available() {
+        return ($this->is_form_defined() && $this->definition->status == self::DEFINITION_STATUS_READY);
+    }
 
-        if (!$this->is_form_defined()) {
-            return false;
-        }
-
-        if ($this->definition->status == self::DEFINITION_STATUS_READY) {
-            return true;
+    /**
+     * 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 false;
+        return get_string('gradingformunavailable', 'grading');
     }
 
     /**
@@ -204,6 +208,7 @@ public function get_definition_copy(gradingform_controller $target) {
         $new->description = $old->description;
         $new->descriptionformat = $old->descriptionformat;
         $new->options = $old->options;
+        $new->status = $old->status;
 
         return $new;
     }
@@ -216,10 +221,9 @@ public function get_definition_copy(gradingform_controller $target) {
      * and save their data into own tables, too.
      *
      * @param stdClass $definition data containing values for the {grading_definition} table
-     * @param int|null $status optionally overwrite the status field with this value
      * @param int|null $usermodified optional userid of the author of the definition, defaults to the current user
      */
-    public function update_definition(stdClass $definition, $status = null, $usermodified = null) {
+    public function update_definition(stdClass $definition, $usermodified = null) {
         global $DB, $USER;
 
         if (is_null($usermodified)) {
@@ -248,10 +252,6 @@ public function update_definition(stdClass $definition, $status = null, $usermod
             // set the modification flags
             $record->timemodified = time();
             $record->usermodified = $usermodified;
-            // overwrite the status if required
-            if (!is_null($status)) {
-                $record->status = $status;
-            }
 
             $DB->update_record('grading_definitions', $record);
 
@@ -277,11 +277,12 @@ public function update_definition(stdClass $definition, $status = null, $usermod
             $record->usercreated  = $usermodified;
             $record->timemodified = $record->timecreated;
             $record->usermodified = $record->usercreated;
-            if (!is_null($status)) {
-                $record->status = $status;
-            } else {
+            if (empty($record->status)) {
                 $record->status = self::DEFINITION_STATUS_DRAFT;
             }
+            if (empty($record->descriptionformat)) {
+                $record->descriptionformat = FORMAT_MOODLE; // field can not be empty
+            }
 
             $DB->insert_record('grading_definitions', $record);
 
@@ -290,6 +291,18 @@ public function update_definition(stdClass $definition, $status = null, $usermod
         }
     }
 
+    /**
+     * Formats the definition description for display on page
+     *
+     * @return string
+     */
+    public function get_formatted_description() {
+        if (!isset($this->definition->description)) {
+            return '';
+        }
+        return format_text($this->definition->description, $this->definition->descriptionformat);
+    }
+
     /**
      * Returns the ACTIVE instance for this definition for the specified $raterid and $itemid
      * (if multiple raters are allowed, or only for $itemid otherwise).

grade/grading/form/rubric/edit.php

@@ -43,26 +43,18 @@
 $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();
-    if (!empty($data->submitfinal)) {
-        $controller->update_definition($data, gradingform_controller::DEFINITION_STATUS_READY);
-        redirect($returnurl);
-    } else {
-        $controller->update_definition($data, gradingform_controller::DEFINITION_STATUS_DRAFT);
-        redirect($PAGE->url);
-    }
+    $controller->update_definition($data);
+    redirect($returnurl);
 }
 
 echo $OUTPUT->header();

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,65 @@ 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_DRAFT]    = get_string('statusdraft', 'grading');
+        $choices[gradingform_controller::DEFINITION_STATUS_READY]    = get_string('statusready', 'grading');
+        $form->addElement('select', 'status', get_string('rubricstatus', 'gradingform_rubric'), $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_READY;
+        } else if (!empty($data->saverubricdraft)) {
+            $data->status = gradingform_controller::DEFINITION_STATUS_DRAFT;
+        }
+        return $data;
+    }
 }

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 ready';
+$string['saverubricdraft'] = 'Save as draft';
+
+$string['rubricstatus'] = 'Current rubric status';
+$string['statusdraft'] = 'Draft';
+$string['statusready'] = 'Ready';
+
+$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';

grade/grading/form/rubric/lib.php

@@ -62,17 +62,16 @@ public function extend_settings_navigation(settings_navigation $settingsnav, nav
      *
      * @see parent::update_definition()
      * @param stdClass $newdefinition rubric definition data as coming from gradingform_rubric_editrubric::get_data()
-     * @param int|null $status optionally overwrite the status field with this value
      * @param int|null $usermodified optional userid of the author of the definition, defaults to the current user
      */
-    public function update_definition(stdClass $newdefinition, $status = null, $usermodified = null) {
+    public function update_definition(stdClass $newdefinition, $usermodified = null) {
         global $DB;
 
         // firstly update the common definition data in the {grading_definition} table
         if ($this->definition === false) {
             // if definition does not exist yet, create a blank one
             // (we need id to save files embedded in description)
-            parent::update_definition((object)array(), $status, $usermodified);
+            parent::update_definition(new stdClass(), $usermodified);
             parent::load_definition();
         }
         if (!isset($newdefinition->rubric['options'])) {
@@ -82,7 +81,7 @@ public function update_definition(stdClass $newdefinition, $status = null, $user
         $editoroptions = self::description_form_field_options($this->get_context());
         $newdefinition = file_postupdate_standard_editor($newdefinition, 'description', $editoroptions, $this->get_context(),
             'gradingform_rubric', 'definition_description', $this->definition->id);
-        parent::update_definition($newdefinition, $status, $usermodified);
+        parent::update_definition($newdefinition, $usermodified);
 
         // reload the definition from the database
         $currentdefinition = $this->get_definition(true);
@@ -136,6 +135,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 +219,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;
                 }
             }
         }
@@ -325,9 +335,14 @@ public static function description_form_field_options($context) {
         );
     }
 
+    /**
+     * Formats the definition description for display on page
+     *
+     * @return string
+     */
     public function get_formatted_description() {
-        if ($this->definition === false) {
-            return null;
+        if (!isset($this->definition->description)) {
+            return '';
         }
         $context = $this->get_context();
 
@@ -573,10 +588,13 @@ public function get_grade() {
         $minscore = 0;
         $maxscore = 0;
         foreach ($this->get_controller()->get_definition()->rubric_criteria as $id => $criterion) {
-            $keys = array_keys($criterion['levels']);
-            sort($keys);
-            $minscore += $criterion['levels'][$keys[0]]['score'];
-            $maxscore += $criterion['levels'][$keys[sizeof($keys)-1]]['score'];
+            $scores = array();
+            foreach ($criterion['levels'] as $level) {
+                $scores[] = $level['score'];
+            }
+            sort($scores);
+            $minscore += $scores[0];
+            $maxscore += $scores[sizeof($scores)-1];
         }
 
         if ($maxscore <= $minscore) {
@@ -635,4 +653,45 @@ public function render_grading_element($page, $gradingformelement) {
         }
         return $this->get_controller()->get_renderer($page)->display_rubric($criteria, $options, $mode, $gradingformelement->getName(), $value);
     }
-}
+}
+
+
+/**
+ * Processes file requests for the gradingform_rubric
+ *
+ * Required to serve files for this plugin
+ * Called from pluginfile.php
+ *
+ * @global moodle_database $DB
+ * @param stdClass $course
+ * @param null $cm
+ * @param stdClass $context
+ * @param string $filearea
+ * @param array $args
+ * @param bool $forcedownload
+ * @return void|false
+ */
+function gradingform_rubric_pluginfile($course, $cm, $context, $filearea, $args, $forcedownload) {
+    global $CFG, $DB;
+    // First argument should ALWAYS be the itemid
+    $itemid = (int)array_shift($args);
+    // Construct a URL to the file and check it exists
+    $fs = get_file_storage();
+    $relativepath = implode('/', $args);
+    $fullpath = "/$context->id/gradingform_rubric/$filearea/$itemid/$relativepath";
+    if (!$file = $fs->get_file_by_hash(sha1($fullpath)) or $file->is_directory()) {
+        // File doesnt exist anyway no point proceeding.
+        return false;
+    }
+    // Switch by the fileare and check the appropriate information
+    switch ($filearea) {
+        case 'definition_description' :
+            // Make sure the itemid points to a valid definition
+            if ($DB->record_exists('grading_definitions', array('id' => $itemid))) {
+                send_stored_file($file, 0, 0, $forcedownload);
+            }
+            break;
+    }
+    // Obviosly bogus comething or other in there
+    return false;
+}