MDL-66079 mod_forum: Add forum grade support
authorAndrew Nicols <andrew@nicols.co.uk>
Tue, 20 Aug 2019 06:48:27 +0000 (14:48 +0800)
committerMathew May <mathewm@hotmail.co.nz>
Wed, 30 Oct 2019 02:23:40 +0000 (10:23 +0800)
Part of MDL-66074

grade/tests/behat/grade_to_pass.feature
grade/tests/coverage.php
lib/form/tests/behat/modgrade_validation.feature
mod/forum/classes/grades/gradeitems.php [new file with mode: 0644]
mod/forum/deprecatedlib.php
mod/forum/externallib.php
mod/forum/lang/en/forum.php
mod/forum/lib.php
mod/forum/mod_form.php
mod/forum/tests/behat/grade_forum.feature [new file with mode: 0644]
mod/forum/tests/grades_gradeitems_test.php [new file with mode: 0644]

index a0e00d7..2cec4d0 100644 (file)
@@ -196,8 +196,8 @@ Feature: We can set the grade to pass value
       | Forum name     | Test Forum 1    |
       | Description    | Test               |
       | Ratings > Aggregate type | Average of ratings |
+      | Ratings > Grade to pass  | 90                 |
       | Ratings > scale[modgrade_point] | 60 |
-      | Grade > Grade to pass  | 90                 |
     Then I should see "The grade to pass can not be greater than the maximum possible grade 60"
 
   Scenario: Set a valid grade to pass for forum activity
@@ -206,10 +206,10 @@ Feature: We can set the grade to pass value
       | Forum name     | Test Forum 1    |
       | Description    | Test               |
       | Ratings > Aggregate type | Average of ratings |
-      | Grade > Grade to pass  | 90                 |
+      | Ratings > Grade to pass  | 90                 |
     And I navigate to "View > Grader report" in the course gradebook
     And I turn editing mode on
-    And I click on "Edit  forum Test Forum 1" "link"
+    And I click on "Edit  forum Rating grade for Test Forum 1" "link"
     And I expand all fieldsets
     Then the field "Grade to pass" matches value "90"
     And I set the field "Grade to pass" to "80"
index fa3128d..959e9c2 100644 (file)
@@ -19,15 +19,16 @@ defined('MOODLE_INTERNAL') || die();
 /**
  * Coverage information for the grades component.
  *
- * @package    grades
- * @category   phpunit
+ * @package    core_grades
+ * @category   test
  * @copyright  2019 Andrew Nicols <andrew@nicols.co.uk>
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
 /**
- * Coverage information for the core subsystem.
+ * Coverage information for the core_grades subsystem.
  *
+ * @package    core_grades
  * @copyright  2019 Andrew Nicols <andrew@nicols.co.uk>
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
index 224c2be..32136bd 100644 (file)
@@ -186,7 +186,7 @@ Feature: Using the activity grade form element
     And I navigate to "Edit settings" in current page administration
     When I expand all fieldsets
     Then I should see "You cannot change the type, as grades already exist for this item."
-    And I set the field "Maximum grade" to "50"
+    And I set the field "Ratings > Maximum grade" to "50"
     And I press "Save and display"
     And I should see "You cannot change the maximum grade when grades already exist for an activity with ratings"
 
diff --git a/mod/forum/classes/grades/gradeitems.php b/mod/forum/classes/grades/gradeitems.php
new file mode 100644 (file)
index 0000000..3e9d098
--- /dev/null
@@ -0,0 +1,63 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Grade item mappings for the activity.
+ *
+ * @package   mod_forum
+ * @copyright Andrew Nicols <andrew@nicols.co.uk>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+declare(strict_types = 1);
+
+namespace mod_forum\grades;
+
+use \core_grades\local\gradeitem\itemnumber_mapping;
+use \core_grades\local\gradeitem\advancedgrading_mapping as advanced_mapping;
+
+/**
+ * Grade item mappings for the activity.
+ *
+ * @package   mod_forum
+ * @copyright Andrew Nicols <andrew@nicols.co.uk>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class gradeitems implements itemnumber_mapping, advanced_mapping {
+
+    /**
+     * Return the list of grade item mappings for the forum.
+     *
+     * @return array
+     */
+    public static function get_itemname_mapping_for_component(): array {
+        return [
+            0 => 'rating',
+            1 => 'forum',
+        ];
+    }
+
+    /**
+     * Get the list of advanced grading item names for this component.
+     *
+     * @return array
+     */
+    public static function get_advancedgrading_itemnames(): array {
+        return [
+            'forum',
+        ];
+    }
+}
index 8e71f22..ae0142c 100644 (file)
@@ -1659,3 +1659,34 @@ function forum_scale_used() {
     throw new coding_exception('forum_scale_used() can not be used anymore. Plugins can implement ' .
         '<modname>_scale_used_anywhere, all implementations of <modname>_scale_used are now ignored');
 }
+
+/**
+ * Return grade for given user or all users.
+ *
+ * @deprecated since Moodle 3.8
+ * @param object $forum
+ * @param int $userid optional user id, 0 means all users
+ * @return array array of grades, false if none
+ */
+function forum_get_user_grades($forum, $userid = 0) {
+    global $CFG;
+
+    require_once($CFG->dirroot.'/rating/lib.php');
+
+    $ratingoptions = (object) [
+        'component' => 'mod_forum',
+        'ratingarea' => 'post',
+        'contextid' => $contextid,
+
+        'modulename' => 'forum',
+        'moduleid  ' => $forum->id,
+        'userid' => $userid,
+        'aggregationmethod' => $forum->assessed,
+        'scaleid' => $forum->scale,
+        'itemtable' => 'forum_posts',
+        'itemtableusercolumn' => 'userid',
+    ];
+
+    $rm = new rating_manager();
+    return $rm->get_user_grades($ratingoptions);
+}
index fab94fa..b692db4 100644 (file)
@@ -137,6 +137,7 @@ class mod_forum_external extends external_api {
                     'assesstimestart' => new external_value(PARAM_INT, 'Assess start time'),
                     'assesstimefinish' => new external_value(PARAM_INT, 'Assess finish time'),
                     'scale' => new external_value(PARAM_INT, 'Scale'),
+                    'grade_forum' => new external_value(PARAM_INT, 'Whole forum grade'),
                     'maxbytes' => new external_value(PARAM_INT, 'Maximum attachment size'),
                     'maxattachments' => new external_value(PARAM_INT, 'Maximum number of attachments'),
                     'forcesubscribe' => new external_value(PARAM_INT, 'Force users to subscribe'),
index 4fb5904..1121409 100644 (file)
@@ -331,6 +331,7 @@ $string['forum:viewrating'] = 'View the total rating you received';
 $string['forum:viewsubscribers'] = 'View subscribers';
 $string['generalforum'] = 'Standard forum for general use';
 $string['generalforums'] = 'General forums';
+$string['gradeitem:forum'] = 'Forum';
 $string['hiddenforumpost'] = 'Hidden forum post';
 $string['hidepreviousrepliescount'] = 'Hide previous replies ({$a})';
 $string['indicator:cognitivedepth'] = 'Forum cognitive';
@@ -714,6 +715,15 @@ $string['forumgrader'] = 'Forum grader';
 $string['grading'] = 'Grading';
 $string['viewconversation'] = 'View conversation';
 
+$string['grade_forum_header'] = 'Whole forum grading';
+$string['grade_forum_title'] = 'Grade';
+$string['gradeforrating'] = 'Grade for rating: {$a->str_long_grade}';
+$string['gradeforratinghidden'] = 'Grade for rating hidden';
+$string['gradeforwholeforum'] = 'Grade for forum: {$a->str_long_grade}';
+$string['gradeforwholeforumhidden'] = 'Grade for forum hidden';
+$string['gradeitemnameforwholeforum'] = 'Whole forum grade for {$a->name}';
+$string['gradeitemnameforrating'] = 'Rating grade for {$a->name}';
+
 // Deprecated since Moodle 3.8.
 $string['cannotdeletediscussioninsinglediscussion'] = 'You cannot delete the first post in a single discussion';
 $string['inpagereplysubject'] = 'Re: {$a}';
index 546c507..eda4a63 100644 (file)
@@ -187,8 +187,25 @@ function forum_update_instance($forum, $mform) {
     // MDL-3942 - if the aggregation type or scale (i.e. max grade) changes then recalculate the grades for the entire forum
     // if  scale changes - do we need to recheck the ratings, if ratings higher than scale how do we want to respond?
     // for count and sum aggregation types the grade we check to make sure they do not exceed the scale (i.e. max score) when calculating the grade
-    if (($oldforum->assessed<>$forum->assessed) or ($oldforum->scale<>$forum->scale)) {
-        forum_update_grades($forum); // recalculate grades for the forum
+    $updategrades = false;
+
+    if ($oldforum->assessed <> $forum->assessed) {
+        // Whether this forum is rated.
+        $updategrades = true;
+    }
+
+    if ($oldforum->scale <> $forum->scale) {
+        // The scale currently in use.
+        $updategrades = true;
+    }
+
+    if (empty($oldforum->grade_forum) || $oldforum->grade_forum <> $forum->grade_forum) {
+        // The whole forum grading.
+        $updategrades = true;
+    }
+
+    if ($updategrades) {
+        forum_update_grades($forum); // Recalculate grades for the forum.
     }
 
     if ($forum->type == 'single') {  // Update related discussion and post.
@@ -350,12 +367,12 @@ function forum_supports($feature) {
         case FEATURE_BACKUP_MOODLE2:          return true;
         case FEATURE_SHOW_DESCRIPTION:        return true;
         case FEATURE_PLAGIARISM:              return true;
+        case FEATURE_ADVANCED_GRADING:        return true;
 
         default: return null;
     }
 }
 
-
 /**
  * Obtains the automatic completion state for this forum based on any conditions
  * in forum settings.
@@ -443,40 +460,55 @@ function forum_get_email_message_id($postid, $usertoid) {
 function forum_user_outline($course, $user, $mod, $forum) {
     global $CFG;
     require_once("$CFG->libdir/gradelib.php");
+
+    $gradeinfo = '';
+    $gradetime = 0;
+
     $grades = grade_get_grades($course->id, 'mod', 'forum', $forum->id, $user->id);
-    if (empty($grades->items[0]->grades)) {
-        $grade = false;
-    } else {
+    if (!empty($grades->items[0]->grades)) {
+        // Item 0 is the rating.
         $grade = reset($grades->items[0]->grades);
+        $gradetime = max($gradetime, grade_get_date_for_user_grade($grade, $user));
+        if (!$grade->hidden || has_capability('moodle/grade:viewhidden', context_course::instance($course->id))) {
+            $gradeinfo .= get_string('gradeforrating', 'forum', $grade) .  html_writer::empty_tag('br');
+        } else {
+            $gradeinfo .= get_string('gradeforratinghidden', 'forum') . html_writer::empty_tag('br');
+        }
     }
 
-    $count = forum_count_user_posts($forum->id, $user->id);
-
-    if ($count && $count->postcount > 0) {
-        $result = new stdClass();
-        $result->info = get_string("numposts", "forum", $count->postcount);
-        $result->time = $count->lastpost;
-        if ($grade) {
-            if (!$grade->hidden || has_capability('moodle/grade:viewhidden', context_course::instance($course->id))) {
-                $result->info .= ', ' . get_string('grade') . ': ' . $grade->str_long_grade;
-            } else {
-                $result->info = get_string('grade') . ': ' . get_string('hidden', 'grades');
-            }
-        }
-        return $result;
-    } else if ($grade) {
-        $result = (object) [
-            'time' => grade_get_date_for_user_grade($grade, $user),
-        ];
+    // Item 1 is the whole-forum grade.
+    if (!empty($grades->items[1]->grades)) {
+        $grade = reset($grades->items[1]->grades);
+        $gradetime = max($gradetime, grade_get_date_for_user_grade($grade, $user));
         if (!$grade->hidden || has_capability('moodle/grade:viewhidden', context_course::instance($course->id))) {
-            $result->info = get_string('grade') . ': ' . $grade->str_long_grade;
+            $gradeinfo .= get_string('gradeforwholeforum', 'forum', $grade) .  html_writer::empty_tag('br');
         } else {
-            $result->info = get_string('grade') . ': ' . get_string('hidden', 'grades');
+            $gradeinfo .= get_string('gradeforwholeforumhidden', 'forum') . html_writer::empty_tag('br');
         }
+    }
 
-        return $result;
+    $count = forum_count_user_posts($forum->id, $user->id);
+    if ($count && $count->postcount > 0) {
+        $info = get_string("numposts", "forum", $count->postcount);
+        $time = $count->lastpost;
+
+        if ($gradeinfo) {
+            $info .= ', ' . $gradeinfo;
+            $time = max($time, $gradetime);
+        }
+
+        return (object) [
+            'info' => $info,
+            'time' => $time,
+        ];
+    } else if ($gradeinfo) {
+        return (object) [
+            'info' => $gradeinfo,
+            'time' => $gradetime,
+        ];
     }
-    return NULL;
+
+    return null;
 }
 
 
@@ -489,24 +521,35 @@ function forum_user_outline($course, $user, $mod, $forum) {
  * @param object $forum
  */
 function forum_user_complete($course, $user, $mod, $forum) {
-    global $CFG,$USER, $OUTPUT;
+    global $CFG, $USER;
     require_once("$CFG->libdir/gradelib.php");
 
-    $grades = grade_get_grades($course->id, 'mod', 'forum', $forum->id, $user->id);
-    if (!empty($grades->items[0]->grades)) {
-        $grade = reset($grades->items[0]->grades);
+    $getgradeinfo = function($grades, string $type) use ($course): string {
+        global $OUTPUT;
+
+        if (empty($grades)) {
+            return '';
+        }
+
+        $result = '';
+        $grade = reset($grades);
         if (!$grade->hidden || has_capability('moodle/grade:viewhidden', context_course::instance($course->id))) {
-            echo $OUTPUT->container(get_string('grade').': '.$grade->str_long_grade);
+            $result .= $OUTPUT->container(get_string("gradefor{$type}", "forum", $grade));
             if ($grade->str_feedback) {
-                echo $OUTPUT->container(get_string('feedback').': '.$grade->str_feedback);
+                $result .= $OUTPUT->container(get_string('feedback').': '.$grade->str_feedback);
             }
         } else {
-            echo $OUTPUT->container(get_string('grade') . ': ' . get_string('hidden', 'grades'));
+            $result .= $OUTPUT->container(get_string("gradefor{$type}hidden", "forum"));
         }
-    }
 
-    if ($posts = forum_get_user_posts($forum->id, $user->id)) {
+        return $result;
+    };
 
+    $grades = grade_get_grades($course->id, 'mod', 'forum', $forum->id, $user->id);
+    echo $getgradeinfo($grades->items[0]->grades, 'rating');
+    echo $getgradeinfo($grades->items[1]->grades, 'wholeforum');
+
+    if ($posts = forum_get_user_posts($forum->id, $user->id)) {
         if (!$cm = get_coursemodule_from_instance('forum', $forum->id, $course->id)) {
             print_error('invalidcoursemodule');
         }
@@ -685,136 +728,133 @@ function forum_print_recent_activity($course, $viewfullnames, $timestart) {
 }
 
 /**
- * Return grade for given user or all users.
- *
- * @global object
- * @global object
- * @param object $forum
- * @param int $userid optional user id, 0 means all users
- * @return array array of grades, false if none
- */
-function forum_get_user_grades($forum, $userid = 0) {
-    global $CFG;
-
-    require_once($CFG->dirroot.'/rating/lib.php');
-
-    $ratingoptions = new stdClass;
-    $ratingoptions->component = 'mod_forum';
-    $ratingoptions->ratingarea = 'post';
-
-    //need these to work backwards to get a context id. Is there a better way to get contextid from a module instance?
-    $ratingoptions->modulename = 'forum';
-    $ratingoptions->moduleid   = $forum->id;
-    $ratingoptions->userid = $userid;
-    $ratingoptions->aggregationmethod = $forum->assessed;
-    $ratingoptions->scaleid = $forum->scale;
-    $ratingoptions->itemtable = 'forum_posts';
-    $ratingoptions->itemtableusercolumn = 'userid';
-
-    $rm = new rating_manager();
-    return $rm->get_user_grades($ratingoptions);
-}
-
-/**
- * Update activity grades
+ * Update activity grades.
  *
- * @category grade
  * @param object $forum
  * @param int $userid specific user only, 0 means all
- * @param boolean $nullifnone return null if grade does not exist
- * @return void
  */
-function forum_update_grades($forum, $userid=0, $nullifnone=true) {
+function forum_update_grades($forum, $userid = 0): void {
     global $CFG, $DB;
     require_once($CFG->libdir.'/gradelib.php');
 
-    if (!$forum->assessed) {
-        forum_grade_item_update($forum);
+    $ratings = null;
+    if ($forum->assessed) {
+        require_once($CFG->dirroot.'/rating/lib.php');
+        $cm = get_coursemodule_from_instance('forum', $forum->id);
 
-    } else if ($grades = forum_get_user_grades($forum, $userid)) {
-        forum_grade_item_update($forum, $grades);
+        $rm = new rating_manager();
+        $ratings = $rm->get_user_grades((object) [
+            'component' => 'mod_forum',
+            'ratingarea' => 'post',
+            'contextid' => \context_module::instance($cm->id)->id,
 
-    } else if ($userid and $nullifnone) {
-        $grade = new stdClass();
-        $grade->userid   = $userid;
-        $grade->rawgrade = NULL;
-        forum_grade_item_update($forum, $grade);
+            'modulename' => 'forum',
+            'moduleid  ' => $forum->id,
+            'userid' => $userid,
+            'aggregationmethod' => $forum->assessed,
+            'scaleid' => $forum->scale,
+            'itemtable' => 'forum_posts',
+            'itemtableusercolumn' => 'userid',
+        ]);
+    }
 
-    } else {
-        forum_grade_item_update($forum);
+    $forumgrades = null;
+    if ($forum->grade_forum) {
+        // TODO MDL-66080.
+        // Need to create a new table for forum_grades with userid, forumid, rawgrade, etc.
     }
+
+    forum_grade_item_update($forum, $ratings, $forumgrades);
 }
 
 /**
- * Create/update grade item for given forum
+ * Create/update grade items for given forum.
  *
- * @category grade
- * @uses GRADE_TYPE_NONE
- * @uses GRADE_TYPE_VALUE
- * @uses GRADE_TYPE_SCALE
  * @param stdClass $forum Forum object with extra cmidnumber
  * @param mixed $grades Optional array/object of grade(s); 'reset' means reset grades in gradebook
- * @return int 0 if ok
  */
-function forum_grade_item_update($forum, $grades=NULL) {
+function forum_grade_item_update($forum, $ratings = null, $forumgrades = null): void {
     global $CFG;
-    if (!function_exists('grade_update')) { //workaround for buggy PHP versions
-        require_once($CFG->libdir.'/gradelib.php');
-    }
-
-    $params = array('itemname'=>$forum->name, 'idnumber'=>$forum->cmidnumber);
+    require_once("{$CFG->libdir}/gradelib.php");
 
-    if (!$forum->assessed or $forum->scale == 0) {
-        $params['gradetype'] = GRADE_TYPE_NONE;
+    // Update the rating.
+    $item = [
+        'itemname' => get_string('gradeitemnameforrating', 'forum', $forum),
+        'idnumber' => $forum->cmidnumber,
+    ];
 
+    if (!$forum->assessed || $forum->scale == 0) {
+        $item['gradetype'] = GRADE_TYPE_NONE;
     } else if ($forum->scale > 0) {
-        $params['gradetype'] = GRADE_TYPE_VALUE;
-        $params['grademax']  = $forum->scale;
-        $params['grademin']  = 0;
-
+        $item['gradetype'] = GRADE_TYPE_VALUE;
+        $item['grademax']  = $forum->scale;
+        $item['grademin']  = 0;
     } else if ($forum->scale < 0) {
-        $params['gradetype'] = GRADE_TYPE_SCALE;
-        $params['scaleid']   = -$forum->scale;
+        $item['gradetype'] = GRADE_TYPE_SCALE;
+        $item['scaleid']   = -$forum->scale;
     }
 
-    if ($grades  === 'reset') {
-        $params['reset'] = true;
-        $grades = NULL;
+    if ($ratings === 'reset') {
+        $item['reset'] = true;
+        $ratings = null;
     }
+    // Itemnumber 0 is the rating.
+    grade_update('mod/forum', $forum->course, 'mod', 'forum', $forum->id, 0, $ratings, $item);
 
-    return grade_update('mod/forum', $forum->course, 'mod', 'forum', $forum->id, 0, $grades, $params);
+    // Whole forum grade.
+    $item = [
+        'itemname' => get_string('gradeitemnameforwholeforum', 'forum', $forum),
+        // Note: We do not need to store the idnumber here.
+    ];
+
+    if (!$forum->grade_forum) {
+        $item['gradetype'] = GRADE_TYPE_NONE;
+    } else if ($forum->grade_forum > 0) {
+        $item['gradetype'] = GRADE_TYPE_VALUE;
+        $item['grademax'] = $forum->grade_forum;
+        $item['grademin'] = 0;
+    } else if ($forum->grade_forum < 0) {
+        $item['gradetype'] = GRADE_TYPE_SCALE;
+        $item['scaleid'] = $forum->grade_forum * -1;
+    }
+
+    if ($forumgrades === 'reset') {
+        $item['reset'] = true;
+        $forumgrades = null;
+    }
+    // Itemnumber 1 is the whole forum grade.
+    grade_update('mod/forum', $forum->course, 'mod', 'forum', $forum->id, 1, $forumgrades, $item);
 }
 
 /**
- * Delete grade item for given forum
+ * Delete grade item for given forum.
  *
- * @category grade
  * @param stdClass $forum Forum object
- * @return grade_item
  */
 function forum_grade_item_delete($forum) {
     global $CFG;
     require_once($CFG->libdir.'/gradelib.php');
 
-    return grade_update('mod/forum', $forum->course, 'mod', 'forum', $forum->id, 0, NULL, array('deleted'=>1));
+    grade_update('mod/forum', $forum->course, 'mod', 'forum', $forum->id, 0, null, ['deleted' => 1]);
+    grade_update('mod/forum', $forum->course, 'mod', 'forum', $forum->id, 1, null, ['deleted' => 1]);
 }
 
 /**
- * Checks if scale is being used by any instance of forum
+ * Checks if scale is being used by any instance of forum.
  *
- * This is used to find out if scale used anywhere
+ * This is used to find out if scale used anywhere.
  *
- * @global object
  * @param $scaleid int
  * @return boolean True if the scale is used by any forum
  */
-function forum_scale_used_anywhere($scaleid) {
+function forum_scale_used_anywhere(int $scaleid): bool {
     global $DB;
-    if ($scaleid and $DB->record_exists('forum', array('scale' => -$scaleid))) {
-        return true;
-    } else {
+
+    if (empty($scaleid)) {
         return false;
     }
+
+    return $DB->record_exists('forum', ['scale' => $scaleid * -1]);
 }
 
 // SQL FUNCTIONS ///////////////////////////////////////////////////////////
@@ -4956,7 +4996,7 @@ function forum_reset_gradebook($courseid, $type='') {
 
     if ($forums = $DB->get_records_sql($sql, $params)) {
         foreach ($forums as $forum) {
-            forum_grade_item_update($forum, 'reset');
+            forum_grade_item_update($forum, 'reset', 'reset');
         }
     }
 }
@@ -6738,3 +6778,14 @@ function mod_forum_user_preferences() {
 
     return $preferences;
 }
+
+/**
+ * Lists all gradable areas for the advanced grading methods gramework.
+ *
+ * @return array('string'=>'string') An array with area names as keys and descriptions as values
+ */
+function forum_grading_areas_list() {
+    return [
+        'forum' => get_string('grade_forum_header', 'forum'),
+    ];
+}
index ae95ac0..64ae3f1 100644 (file)
@@ -27,6 +27,8 @@ if (!defined('MOODLE_INTERNAL')) {
 
 require_once ($CFG->dirroot.'/course/moodleform_mod.php');
 
+use core_grades\component_gradeitems;
+
 class mod_forum_mod_form extends moodleform_mod {
 
     function definition() {
@@ -208,13 +210,100 @@ class mod_forum_mod_form extends moodleform_mod {
 
 //-------------------------------------------------------------------------------
 
-        $this->standard_grading_coursemodule_elements();
+        // Add the whole forum grading options.
+        $this->add_forum_grade_settings($mform, 'forum');
 
         $this->standard_coursemodule_elements();
 //-------------------------------------------------------------------------------
 // buttons
         $this->add_action_buttons();
+    }
 
+    /**
+     * Add the whole forum grade settings to the mform.
+     *
+     * @param   \mform $mform
+     * @param   string $itemname
+     */
+    private function add_forum_grade_settings($mform, string $itemname) {
+        global $COURSE;
+
+        $component = "mod_{$this->_modname}";
+        $defaultgradingvalue = 0;
+
+        $itemnumber = component_gradeitems::get_itemnumber_from_itemname($component, $itemname);
+        $gradefieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'grade');
+        $gradecatfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'gradecat');
+        $gradepassfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'gradepass');
+
+        // The advancedgradingmethod is different in that it is suffixed with an area name... which is not the
+        // itemnumber.
+        $methodfieldname = "advancedgradingmethod_{$itemname}";
+
+        $headername = "{$gradefieldname}_header";
+        $mform->addElement('header', $headername, get_string("grade_{$itemname}_header", $component));
+
+        $isupdate = !empty($this->_cm);
+        $gradeoptions = [
+            'isupdate' => $isupdate,
+            'currentgrade' => false,
+            'hasgrades' => false,
+            'canrescale' => false,
+            'useratings' => false,
+        ];
+
+        if ($isupdate) {
+            $gradeitem = grade_item::fetch([
+                'itemtype' => 'mod',
+                'itemmodule' => $this->_cm->modname,
+                'iteminstance' => $this->_cm->instance,
+                'itemnumber' => $itemnumber,
+                'courseid' => $COURSE->id,
+            ]);
+            if ($gradeitem) {
+                $gradeoptions['currentgrade'] = $gradeitem->grademax;
+                $gradeoptions['currentgradetype'] = $gradeitem->gradetype;
+                $gradeoptions['currentscaleid'] = $gradeitem->scaleid;
+                $gradeoptions['hasgrades'] = $gradeitem->has_grades();
+            }
+        }
+        $mform->addElement(
+            'modgrade',
+            $gradefieldname,
+            get_string("{$gradefieldname}_title", $component),
+            $gradeoptions
+        );
+        $mform->addHelpButton($gradefieldname, 'modgrade', 'grades');
+        $mform->setDefault($gradefieldname, $defaultgradingvalue);
+
+        if (!empty($this->current->_advancedgradingdata['methods']) && !empty($this->current->_advancedgradingdata['areas'])) {
+            $areadata = $this->current->_advancedgradingdata['areas'][$itemname];
+            $mform->addElement(
+                'select',
+                $methodfieldname,
+                get_string('gradingmethod', 'core_grading'),
+                $this->current->_advancedgradingdata['methods']
+            );
+            $mform->addHelpButton($methodfieldname, 'gradingmethod', 'core_grading');
+            $mform->hideIf($methodfieldname, "{$gradefieldname}[modgrade_type]", 'eq', 'none');
+        }
+
+        // Grade category.
+        $mform->addElement(
+            'select',
+            $gradecatfieldname,
+            get_string('gradecategoryonmodform', 'grades'),
+            grade_get_categories_menu($COURSE->id, $this->_outcomesused)
+        );
+        $mform->addHelpButton($gradecatfieldname, 'gradecategoryonmodform', 'grades');
+        $mform->hideIf($gradecatfieldname, "{$gradefieldname}[modgrade_type]", 'eq', 'none');
+
+        // Grade to pass.
+        $mform->addElement('text', $gradepassfieldname, get_string('gradepass', 'grades'));
+        $mform->addHelpButton($gradepassfieldname, 'gradepass', 'grades');
+        $mform->setDefault($gradepassfieldname, '');
+        $mform->setType($gradepassfieldname, PARAM_RAW);
+        $mform->hideIf($gradepassfieldname, "{$gradefieldname}[modgrade_type]", 'eq', 'none');
     }
 
     function definition_after_data() {
@@ -236,7 +325,6 @@ class mod_forum_mod_form extends moodleform_mod {
             $type->freeze();
             $type->setPersistantFreeze(true);
         }
-
     }
 
     public function validation($data, $files) {
@@ -248,9 +336,63 @@ class mod_forum_mod_form extends moodleform_mod {
             }
         }
 
+        $this->validation_forum_grade($data, $files, $errors);
+
         return $errors;
     }
 
+    /**
+     * Handle definition after data for grade settings.
+     *
+     * @param array $data
+     * @param array $files
+     * @param array $errors
+     */
+    private function validation_forum_grade(array $data, array $files, array $errors) {
+        global $COURSE;
+
+        $mform =& $this->_form;
+
+        $component = "mod_forum";
+        $itemname = 'forum';
+        $itemnumber = component_gradeitems::get_itemnumber_from_itemname($component, $itemname);
+        $gradefieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'grade');
+        $gradepassfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'grade');
+
+        $gradeitem = grade_item::fetch([
+            'itemtype' => 'mod',
+            'itemmodule' => $data['modulename'],
+            'iteminstance' => $data['instance'],
+            'itemnumber' => $itemnumber,
+            'courseid' => $COURSE->id,
+        ]);
+
+        if ($mform->elementExists('cmidnumber') && $this->_cm) {
+            if (!grade_verify_idnumber($data['cmidnumber'], $COURSE->id, $gradeitem, $this->_cm)) {
+                $errors['cmidnumber'] = get_string('idnumbertaken');
+            }
+        }
+
+        // Check that the grade pass is a valid number.
+        $gradepassvalid = false;
+        if (isset($data[$gradepassfieldname])) {
+            if (unformat_float($data[$gradepassfieldname], true) === false) {
+                $errors[$gradepassfieldname] = get_string('err_numeric', 'form');
+            } else {
+                $gradepassvalid = true;
+            }
+        }
+
+        // Grade to pass: ensure that the grade to pass is valid for points and scales.
+        // If we are working with a scale, convert into a positive number for validation.
+        if ($gradepassvalid && isset($data[$gradepassfieldname]) && (!empty($data[$gradefieldname]))) {
+            $grade = $data[$gradefieldname];
+            if (unformat_float($data[$gradepassfieldname]) > $grade) {
+                $errors[$gradepassfieldname] = get_string('gradepassgreaterthangrade', 'grades', $grade);
+            }
+        }
+    }
+
     function data_preprocessing(&$default_values) {
         parent::data_preprocessing($default_values);
 
@@ -316,6 +458,32 @@ class mod_forum_mod_form extends moodleform_mod {
             (!empty($data['completionpostsenabled']) && $data['completionposts']!=0);
     }
 
+    /**
+     * Return submitted data if properly submitted or returns NULL if validation fails or
+     * if there is no submitted data.
+     *
+     * Do not override this method, override data_postprocessing() instead.
+     *
+     * @return object submitted data; NULL if not valid or not submitted or cancelled
+     */
+    public function get_data() {
+        $data = parent::get_data();
+        if ($data) {
+            $itemname = 'forum';
+            $component = 'mod_forum';
+            $gradepassfieldname = component_gradeitems::get_field_name_for_itemname($component, $itemname, 'gradepass');
+
+            // Convert the grade pass value - we may be using a language which uses commas,
+            // rather than decimal points, in numbers. These need to be converted so that
+            // they can be added to the DB.
+            if (isset($data->{$gradepassfieldname})) {
+                $data->{$gradepassfieldname} = unformat_float($data->{$gradepassfieldname});
+            }
+        }
+
+        return $data;
+    }
+
     /**
      * Allows module to modify the data returned by form get_data().
      * This method is also called in the bulk activity completion form.
@@ -341,4 +509,3 @@ class mod_forum_mod_form extends moodleform_mod {
         }
     }
 }
-
diff --git a/mod/forum/tests/behat/grade_forum.feature b/mod/forum/tests/behat/grade_forum.feature
new file mode 100644 (file)
index 0000000..5d8e72a
--- /dev/null
@@ -0,0 +1,169 @@
+@mod @mod_forum @core_grades
+Feature: I can grade a students interaction across a forum
+  In order to assess a student's contributions
+  As a teacher
+  I can assign grades to a student based on their contributions
+
+  Background:
+    Given the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher | 1 | teacher1@example.com |
+      | student1 | Student | 1 | student1@example.com |
+    And the following "courses" exist:
+      | fullname | shortname | format | numsections |
+      | Course 1 | C1 | weeks | 5 |
+    And the following "grade categories" exist:
+      | fullname | course |
+      | Tutor | C1 |
+      | Peers | C1 |
+    And the following "course enrolments" exist:
+      | user | course | role |
+      | teacher1 | C1 | editingteacher |
+      | student1 | C1 | student |
+    And the following "scales" exist:
+      | name | scale |
+      | Test Scale 1 | Disappointing, Good, Very good, Excellent |
+    And I log in as "teacher1"
+    And I change window size to "large"
+    And I am on "Course 1" course homepage
+    And I turn editing mode on
+
+  @javascript
+  Scenario: Ensure that forum grade settings do not leak to Ratings
+    Given I add a "Forum" to section "1"
+    And I expand all fieldsets
+    And I set the following fields to these values:
+      | Forum name     | Test Forum 1    |
+      | Description    | Test               |
+
+    # Fields should be hidden when grading is not set.
+    When I set the field "Whole forum grading > Type" to "None"
+    Then "Whole forum grading > Grade to pass" "field" should not be visible
+    And "Whole forum grading > Grade category" "field" should not be visible
+    And "Whole forum grading > Maximum grade" "field" should not be visible
+    And "Ratings > Grade to pass" "field" should not be visible
+    And "Ratings > Grade category" "field" should not be visible
+    And "Ratings > Maximum grade" "field" should not be visible
+
+    # Only Whole forum grading fields should be visible.
+    When I set the field "Whole forum grading > Type" to "Point"
+    Then "Whole forum grading > Grade to pass" "field" should be visible
+    And "Whole forum grading > Grade category" "field" should be visible
+    And "Whole forum grading > Maximum grade" "field" should be visible
+    But "Ratings > Grade to pass" "field" should not be visible
+    And "Ratings > Grade category" "field" should not be visible
+    And "Ratings > Maximum grade" "field" should not be visible
+
+    # Save some values.
+    Given I set the field "Whole forum grading > Maximum grade" to "10"
+    And I set the field "Whole forum grading > Grade category" to "Tutor"
+    And I set the field "Whole forum grading > Grade to pass" to "4"
+    When I press "Save and return to course"
+    And I navigate to "View > Grader report" in the course gradebook
+    And I turn editing mode on
+
+    # There shouldn't be any Ratings grade item.
+    Then I should see "Whole forum grade"
+    But I should not see "Rating grade"
+
+    # The values saved should be reflected here.
+    Given I click on "Edit  forum Whole forum grade for Test Forum 1" "link"
+    When I expand all fieldsets
+    Then the field "Maximum grade" matches value "10"
+    Then the field "Grade to pass" matches value "4"
+    And I should see "Tutor" in the "Parent category" "fieldset"
+
+  @javascript
+  Scenario: Ensure that Ratings settings do not leak to Forum grading
+    Given I add a "Forum" to section "1"
+    And I expand all fieldsets
+    And I set the following fields to these values:
+      | Forum name     | Test Forum 1    |
+      | Description    | Test               |
+
+    # Fields should be hidden when grading is not set.
+    When I set the field "Ratings > Aggregate type" to "No ratings"
+    Then "Ratings > Type" "field" should not be visible
+    And "Ratings > Grade to pass" "field" should not be visible
+    And "Ratings > Grade category" "field" should not be visible
+    And "Ratings > Maximum grade" "field" should not be visible
+    And "Whole forum grading > Grade to pass" "field" should not be visible
+    And "Whole forum grading > Grade category" "field" should not be visible
+    And "Whole forum grading > Maximum grade" "field" should not be visible
+
+    # Set to "Count of ratings"
+    When I set the field "Ratings > Aggregate type" to "Count of ratings"
+    Then "Ratings > Type" "field" should be visible
+    When I set the field "Ratings > Type" to "None"
+    Then "Ratings > Grade to pass" "field" should not be visible
+    And "Ratings > Grade category" "field" should not be visible
+    And "Ratings > Maximum grade" "field" should not be visible
+    And "Whole forum grading > Grade to pass" "field" should not be visible
+    And "Whole forum grading > Grade category" "field" should not be visible
+    And "Whole forum grading > Maximum grade" "field" should not be visible
+
+    # Use point grading
+    When I set the field "Ratings > Type" to "Point"
+    Then "Ratings > Grade to pass" "field" should be visible
+    And "Ratings > Grade category" "field" should be visible
+    And "Ratings > Maximum grade" "field" should be visible
+    And "Whole forum grading > Grade to pass" "field" should not be visible
+    And "Whole forum grading > Grade category" "field" should not be visible
+    And "Whole forum grading > Maximum grade" "field" should not be visible
+
+    # Save some values.
+    Given I set the field "Ratings > Maximum grade" to "10"
+    And I set the field "Ratings > Grade category" to "Tutor"
+    And I set the field "Ratings > Grade to pass" to "4"
+    When I press "Save and return to course"
+    And I navigate to "View > Grader report" in the course gradebook
+    And I turn editing mode on
+
+    # There shouldn't be any Whole forum grade gradeitem.
+    Then I should see "Rating grade"
+    But I should not see "Whole forum grade"
+
+    # The values saved should be reflected here.
+    Given I click on "Edit  forum Rating grade for Test Forum 1" "link"
+    When I expand all fieldsets
+    Then the field "Maximum grade" matches value "10"
+    Then the field "Grade to pass" matches value "4"
+    And I should see "Tutor" in the "Parent category" "fieldset"
+
+  Scenario: Setting both a rating and a whole forum grade does not bleed
+    Given I add a "Forum" to section "1"
+    And I expand all fieldsets
+    And I set the following fields to these values:
+      | Forum name     | Test Forum 1    |
+      | Description    | Test               |
+
+    When I set the field "Ratings > Aggregate type" to "Count of ratings"
+    And I set the field "Ratings > Type" to "Point"
+    And I set the field "Ratings > Maximum grade" to "100"
+    And I set the field "Ratings > Grade category" to "Peers"
+    And I set the field "Ratings > Grade to pass" to "40"
+    And I set the field "Whole forum grading > Type" to "Point"
+    And I set the field "Whole forum grading > Maximum grade" to "10"
+    And I set the field "Whole forum grading > Grade category" to "Tutor"
+    And I set the field "Whole forum grading > Grade to pass" to "4"
+    And I press "Save and return to course"
+    And I navigate to "View > Grader report" in the course gradebook
+    And I turn editing mode on
+
+    # There shouldn't be any Whole forum grade gradeitem.
+    Then I should see "Rating grade"
+    And I should see "Whole forum grade"
+
+    # The values saved should be reflected here.
+    Given I click on "Edit  forum Rating grade for Test Forum 1" "link"
+    When I expand all fieldsets
+    Then the field "Maximum grade" matches value "100"
+    Then the field "Grade to pass" matches value "40"
+    And I should see "Peers" in the "Parent category" "fieldset"
+    And I press "cancel"
+
+    Given I click on "Edit  forum Whole forum grade for Test Forum 1" "link"
+    When I expand all fieldsets
+    Then the field "Maximum grade" matches value "10"
+    Then the field "Grade to pass" matches value "4"
+    And I should see "Tutor" in the "Parent category" "fieldset"
diff --git a/mod/forum/tests/grades_gradeitems_test.php b/mod/forum/tests/grades_gradeitems_test.php
new file mode 100644 (file)
index 0000000..65fbcee
--- /dev/null
@@ -0,0 +1,54 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Unit tests for mod_forum\grades\gradeitems.
+ *
+ * @package   mod_forum
+ * @category  test
+ * @copyright 2019 Andrew Nicols <andrew@nicols.co.uk>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU Public License
+ */
+
+declare(strict_types = 1);
+
+namespace tests\mod_forum\grades;
+
+use advanced_testcase;
+use core_grades\component_gradeitems;
+use coding_exception;
+
+/**
+ * Unit tests for mod_forum\grades\gradeitems.
+ *
+ * @package   mod_forum
+ * @category  test
+ * @copyright 2019 Andrew Nicols <andrew@nicols.co.uk>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class gradeitems_test extends advanced_testcase {
+
+    /**
+     * Ensure that a component which does not implement the mapping class excepts.
+     */
+    public function test_get_mappings() {
+        $mappings = component_gradeitems::get_mappings_for_component('mod_forum');
+        $this->assertIsArray($mappings);
+        $this->assertCount(2, $mappings);
+        $this->assertArraySubset([0 => 'rating'], $mappings);
+        $this->assertArraySubset([1 => 'forum'], $mappings);
+    }
+}