MDL-59169 gradebook: Add groupid to gpr tracking & use

Added groupid to grade_plugin_return tracking, modified constructor
to use query params as defaults if passed-in params not supplied,
used groupid tracking in core reports.

grade_get_graded_users_select also fixed to use gpr (otherwise using
it breaks when course current group has changed between render and
submit).

Fixed coding style issue in gpr constructor & documented properties.

Added upgrade.txt noting gpr changes.

grade/lib.php

@@ -453,7 +453,11 @@ function grade_get_graded_users_select($report, $course, $userid, $groupid, $inc
     if (!empty($menususpendedusers)) {
         $menu[] = array(get_string('suspendedusers') => $menususpendedusers);
     }
-    $select = new single_select(new moodle_url('/grade/report/'.$report.'/index.php', array('id'=>$course->id)), 'userid', $menu, $userid);
+    $gpr = new grade_plugin_return(array('type' => 'report', 'course' => $course, 'groupid' => $groupid));
+    $select = new single_select(
+        new moodle_url('/grade/report/'.$report.'/index.php', $gpr->get_options()),
+        'userid', $menu, $userid
+    );
     $select->label = $label;
     $select->formid = 'choosegradeuser';
     return $select;
@@ -1097,30 +1101,82 @@ function print_grade_page_head($courseid, $active_type, $active_plugin=null,
  * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class grade_plugin_return {
+    /**
+     * Type of grade plugin (e.g. 'edit', 'report')
+     *
+     * @var string
+     */
     public $type;
+    /**
+     * Name of grade plugin (e.g. 'grader', 'overview')
+     *
+     * @var string
+     */
     public $plugin;
+    /**
+     * Course id being viewed
+     *
+     * @var int
+     */
     public $courseid;
+    /**
+     * Id of user whose information is being viewed/edited
+     *
+     * @var int
+     */
     public $userid;
+    /**
+     * Id of group for which information is being viewed/edited
+     *
+     * @var int
+     */
+    public $groupid;
+    /**
+     * Current page # within output
+     *
+     * @var int
+     */
     public $page;
 
     /**
      * Constructor
      *
-     * @param array $params - associative array with return parameters, if null parameter are taken from _GET or _POST
+     * @param array $params - associative array with return parameters, if not supplied parameter are taken from _GET or _POST
      */
-    public function __construct($params = null) {
-        if (empty($params)) {
-            $this->type     = optional_param('gpr_type', null, PARAM_SAFEDIR);
-            $this->plugin   = optional_param('gpr_plugin', null, PARAM_PLUGIN);
-            $this->courseid = optional_param('gpr_courseid', null, PARAM_INT);
-            $this->userid   = optional_param('gpr_userid', null, PARAM_INT);
-            $this->page     = optional_param('gpr_page', null, PARAM_INT);
+    public function __construct($params = []) {
+        $this->type     = optional_param('gpr_type', null, PARAM_SAFEDIR);
+        $this->plugin   = optional_param('gpr_plugin', null, PARAM_PLUGIN);
+        $this->courseid = optional_param('gpr_courseid', null, PARAM_INT);
+        $this->userid   = optional_param('gpr_userid', null, PARAM_INT);
+        $this->groupid  = optional_param('gpr_groupid', null, PARAM_INT);
+        $this->page     = optional_param('gpr_page', null, PARAM_INT);
 
+        foreach ($params as $key => $value) {
+            if (property_exists($this, $key)) {
+                $this->$key = $value;
+            }
+        }
+        // Allow course object rather than id to be used to specify course
+        // - avoid unnecessary use of get_course.
+        if (array_key_exists('course', $params)) {
+            $course = $params['course'];
+            $this->courseid = $course->id;
         } else {
-            foreach ($params as $key=>$value) {
-                if (property_exists($this, $key)) {
-                    $this->$key = $value;
+            $course = null;
+        }
+        // If group has been explicitly set in constructor parameters,
+        // we should respect that.
+        if (!array_key_exists('groupid', $params)) {
+            // Otherwise, 'group' in request parameters is a request for a change.
+            // In that case, or if we have no group at all, we should get groupid from
+            // groups_get_course_group, which will do some housekeeping as well as
+            // give us the correct value.
+            $changegroup = optional_param('group', -1, PARAM_INT);
+            if ($changegroup !== -1 or (empty($this->groupid) and !empty($this->courseid))) {
+                if ($course === null) {
+                    $course = get_course($this->courseid);
                 }
+                $this->groupid = groups_get_course_group($course, true);
             }
         }
     }
@@ -1158,6 +1214,10 @@ public function get_options() {
             $params['userid'] = $this->userid;
         }
 
+        if (!empty($this->groupid)) {
+            $params['group'] = $this->groupid;
+        }
+
         if (!empty($this->page)) {
             $params['page'] = $this->page;
         }
@@ -1193,6 +1253,11 @@ public function get_return_url($default, $extras=null) {
             $glue = '&';
         }
 
+        if (!empty($this->groupid)) {
+            $url .= $glue.'group='.$this->groupid;
+            $glue = '&';
+        }
+
         if (!empty($this->page)) {
             $url .= $glue.'page='.$this->page;
             $glue = '&';
@@ -1231,9 +1296,14 @@ public function get_form_fields() {
             $result .= '<input type="hidden" name="gpr_userid" value="'.$this->userid.'" />';
         }
 
+        if (!empty($this->groupid)) {
+            $result .= '<input type="hidden" name="gpr_groupid" value="'.$this->groupid.'" />';
+        }
+
         if (!empty($this->page)) {
             $result .= '<input type="hidden" name="gpr_page" value="'.$this->page.'" />';
         }
+        return $result;
     }
 
     /**
@@ -1266,6 +1336,11 @@ public function add_mform_elements(&$mform) {
             $mform->setType('gpr_userid', PARAM_INT);
         }
 
+        if (!empty($this->groupid)) {
+            $mform->addElement('hidden', 'gpr_groupid', $this->groupid);
+            $mform->setType('gpr_groupid', PARAM_INT);
+        }
+
         if (!empty($this->page)) {
             $mform->addElement('hidden', 'gpr_page', $this->page);
             $mform->setType('gpr_page', PARAM_INT);
@@ -1298,6 +1373,10 @@ public function add_url_params(moodle_url $url) {
             $url->param('gpr_userid', $this->userid);
         }
 
+        if (!empty($this->groupid)) {
+            $url->param('gpr_groupid', $this->groupid);
+        }
+
         if (!empty($this->page)) {
             $url->param('gpr_page', $this->page);
         }

grade/report/grader/index.php

@@ -65,7 +65,14 @@
 require_capability('moodle/grade:viewall', $context);
 
 // return tracking object
-$gpr = new grade_plugin_return(array('type'=>'report', 'plugin'=>'grader', 'courseid'=>$courseid, 'page'=>$page));
+$gpr = new grade_plugin_return(
+    array(
+        'type' => 'report',
+        'plugin' => 'grader',
+        'course' => $course,
+        'page' => $page
+    )
+);
 
 // last selected report session tracking
 if (!isset($USER->grade_last_report)) {
@@ -187,6 +194,7 @@
     echo '<input type="hidden" value="'.time().'" name="timepageload" />';
     echo '<input type="hidden" value="grader" name="report"/>';
     echo '<input type="hidden" value="'.$page.'" name="page"/>';
+    echo $gpr->get_form_fields();
     echo $reporthtml;
     echo '<div class="submit"><input type="submit" id="gradersubmit" class="btn btn-primary"
         value="'.s(get_string('savechanges')).'" /></div>';

grade/report/lib.php

@@ -366,13 +366,16 @@ public static function supports_mygrades() {
     protected function setup_groups() {
         // find out current groups mode
         if ($this->groupmode = groups_get_course_groupmode($this->course)) {
-            $this->currentgroup = groups_get_course_group($this->course, true);
+            if (empty($this->gpr->groupid)) {
+                $this->currentgroup = groups_get_course_group($this->course, true);
+            } else {
+                $this->currentgroup = $this->gpr->groupid;
+            }
             $this->group_selector = groups_print_course_menu($this->course, $this->pbarurl, true);
 
             if ($this->groupmode == SEPARATEGROUPS and !$this->currentgroup and !has_capability('moodle/site:accessallgroups', $this->context)) {
                 $this->currentgroup = -2; // means can not access any groups at all
             }
-
             if ($this->currentgroup) {
                 $group = groups_get_group($this->currentgroup);
                 $this->currentgroupname     = $group->name;

grade/report/overview/index.php

@@ -93,7 +93,7 @@
 if (has_capability('moodle/grade:viewall', $context) && $courseid != SITEID) {
     // Please note this would be extremely slow if we wanted to implement this properly for all teachers.
     $groupmode    = groups_get_course_groupmode($course);   // Groups are being used
-    $currentgroup = groups_get_course_group($course, true);
+    $currentgroup = $gpr->groupid;
 
     if (!$currentgroup) {      // To make some other functions work better later
         $currentgroup = NULL;

grade/report/overview/lib.php

@@ -286,7 +286,7 @@ public function fill_table($activitylink = false, $studentcoursesonly = false) {
                         'user' => $this->user->id)), $coursename);
                 } else {
                     $courselink = html_writer::link(new moodle_url('/grade/report/user/index.php', array('id' => $course->id,
-                        'userid' => $this->user->id)), $coursename);
+                        'userid' => $this->user->id, 'group' => $this->gpr->groupid)), $coursename);
                 }
 
                 $data = array($courselink, grade_format_gradevalue($finalgrade, $courseitem, true));

grade/report/user/index.php

@@ -91,7 +91,7 @@
 
 if (has_capability('moodle/grade:viewall', $context)) { //Teachers will see all student reports
     $groupmode    = groups_get_course_groupmode($course);   // Groups are being used
-    $currentgroup = groups_get_course_group($course, true);
+    $currentgroup = $gpr->groupid;
 
     if (!$currentgroup) {      // To make some other functions work better later
         $currentgroup = NULL;

grade/upgrade.txt

@@ -0,0 +1,19 @@
+This file describes API changes in /grade/* ;
+Information provided here is intended especially for developers.
+
+=== 3.6 ===
+
+* The grade_plugin_return constructor now uses parameters from the
+  request as defaults, which can be overridden by parameters supplied
+  to the constructor. This may lead to a change in behaviour if only
+  some of the possible parameters are supplied.
+* The grade_plugin_return class now tracks groupid as well as the
+  type, plugin, courseid, userid and page parameters that were tracked
+  previously. The groupid parameter will be set using
+  groups_get_course_group for the relevant course if the group is
+  otherwise unspecified.
+* The above changes mean that code using grade_plugin_return objects
+  should generally no longer call groups_get_course_group directly,
+  but should use the gpr->groupid parameter instead.
+* The grade_plugin_return constructor now accepts either course or
+  courseid as a parameter to specify course.