Moodle
  1. Moodle
  2. MDL-12122

User report on 1.9 gradebook should should display something much better to teachers -- with PATCH

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 1.9, 2.0
    • Component/s: Gradebook
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Rank:
      29941

      Description

      I know that someone will call the existing behavior a "feature" of the user report, but when a teacher goes to this report, they get their their own personal grades (even when they are not a graded user). This is useless. They should get the grade reports of all their students instead, which would be extremely helpful. It is true that someone could write a new report, but since this is the default report shipping with 1.9, it should have the desirable behavior, IMO.

      To fix, change in \grade\report\user\index.php
      --------------------------------------
      // Create a report instance
      $report = new grade_report_user($courseid, $gpr, $context, $userid);

      // print the page
      print_heading(get_string('modulename', 'gradereport_user'). ' - '.fullname($report->user));

      if ($report->fill_table())

      { echo $report->print_table(true); }
      -------------------------------------------
      to
      ----------------------------------------
      if (has_capability('moodle/grade:viewall', $context)) { //Teachers will see all student reports
      $gui = new graded_users_iterator($course);
      $gui->init();
      if(!isset($gui->grade_items)) $gui->grade_items = array();
      while ($userdata = $gui->next_user()) {
      $user = $userdata->user;
      $report = new grade_report_user($courseid, $gpr, $context, $user->id);
      print_heading(get_string('modulename', 'gradereport_user'). ' - '.fullname($report->user));
      if ($report->fill_table()) { echo $report->print_table(true); }
      echo "<p style = 'page-break-after: always;'></p>";
      }
      $gui->close();
      }else //Students will see just their own report
      {
      // Create a report instance
      $report = new grade_report_user($courseid, $gpr, $context, $userid);

      // print the page
      print_heading(get_string('modulename', 'gradereport_user'). ' - '.fullname($report->user));

      if ($report->fill_table()) { echo $report->print_table(true); }

      }
      -------------------------------------------
      With thanks to our school's (Seattle Academy) Server Administrator Karlene Clapp for the start of this patch

        Issue Links

          Activity

          Hide
          Martin Dougiamas added a comment - - edited

          Um, and what is going to happen with a course containing 1000 students?

          I think this needs to be modified with some sort of selector ... or paging ...

          Show
          Martin Dougiamas added a comment - - edited Um, and what is going to happen with a course containing 1000 students? I think this needs to be modified with some sort of selector ... or paging ...
          Hide
          Gary Anderson added a comment -

          On the question of large courses, our school fix on this includs a group selector, but I chose to post a simpler fix as this one is easy to understand. It can be added back, but requires patches to the lib file. As is, this fix at least allows for some field testing of the feature. I would like to see a selector for an individual student also, much like logs.

          An institution with a 1000 person course would probably have the resources to write their own report, but this is useful for most schools as is, especially if grouping were added. And without it, the report is not of any use at all to teachers.

          The key thing, however, is that there should be at least one student grade report (probably the default), that can be available to teachers. And this is a start.

          Show
          Gary Anderson added a comment - On the question of large courses, our school fix on this includs a group selector, but I chose to post a simpler fix as this one is easy to understand. It can be added back, but requires patches to the lib file. As is, this fix at least allows for some field testing of the feature. I would like to see a selector for an individual student also, much like logs. An institution with a 1000 person course would probably have the resources to write their own report, but this is useful for most schools as is, especially if grouping were added. And without it, the report is not of any use at all to teachers. The key thing, however, is that there should be at least one student grade report (probably the default), that can be available to teachers. And this is a start.
          Hide
          Gary Anderson added a comment - - edited

          In confirming this patch by installing by hand, I identified a bug where graded_user_interator does not initialize grade_items that is used in a subsequent call to the next_user() method. This produces an warning error in elevated debug modes.

          To fix, after the line $gui->init(); in the patch add

          if(!isset($gui->grade_items)) $gui->grade_items = array();

          This is now fixed in the above code.

          Show
          Gary Anderson added a comment - - edited In confirming this patch by installing by hand, I identified a bug where graded_user_interator does not initialize grade_items that is used in a subsequent call to the next_user() method. This produces an warning error in elevated debug modes. To fix, after the line $gui->init(); in the patch add if(!isset($gui->grade_items)) $gui->grade_items = array(); This is now fixed in the above code.
          Hide
          Petr Škoda added a comment -

          hmm, the default reports should work for any number of students IMO.
          Maybe we should change the default permissions and do not allow teachers to see this report at all.

          Show
          Petr Škoda added a comment - hmm, the default reports should work for any number of students IMO. Maybe we should change the default permissions and do not allow teachers to see this report at all.
          Hide
          Gary Anderson added a comment -

          Sorry, busy teaching!
          Hiding the report from teachers would be one solution, but there should be some type of user report that the teacher can run. There is a benefit of this paricular report as it could be exactly what the student sees.

          If it were, things to look at are:

          Does it work correctly with different permissions, hidden assignments, etc.
          It should work with groups.
          It should list the maximum value (or range) for each activity so a person could -recompute the summary statistics by hand.
          There is probably a need for paging.
          In the teacher view, it one should be able to view a single student.

          Of course, a teacher can "login" as a student (if that capability is set), but being able to see their reports together would be helpful.

          I would be interesting the the path you might take on this; I will look at what might be done this weekend. But the above fix with a group selector is how we will use it at Seattle Academy pending a better solution.

          Show
          Gary Anderson added a comment - Sorry, busy teaching! Hiding the report from teachers would be one solution, but there should be some type of user report that the teacher can run. There is a benefit of this paricular report as it could be exactly what the student sees. If it were, things to look at are: Does it work correctly with different permissions, hidden assignments, etc. It should work with groups. It should list the maximum value (or range) for each activity so a person could -recompute the summary statistics by hand. There is probably a need for paging. In the teacher view, it one should be able to view a single student. Of course, a teacher can "login" as a student (if that capability is set), but being able to see their reports together would be helpful. I would be interesting the the path you might take on this; I will look at what might be done this weekend. But the above fix with a group selector is how we will use it at Seattle Academy pending a better solution.
          Hide
          Gary Anderson added a comment -

          Fixed original code based on subsequent comment.

          Show
          Gary Anderson added a comment - Fixed original code based on subsequent comment.
          Hide
          Martin Dougiamas added a comment -

          Nicolas, can you use this patch but rewrite it so that it has a user selector menu at the top (list of all participant names plus one item to "Show all users on one page")

          For any given user (ie parameter to the script like userid=456) it will show that user only. For all users (userid=all) then show all one one page similar to Gary's patch.

          If userid is not specified then userid default value = $USER->id.

          Show
          Martin Dougiamas added a comment - Nicolas, can you use this patch but rewrite it so that it has a user selector menu at the top (list of all participant names plus one item to "Show all users on one page") For any given user (ie parameter to the script like userid=456) it will show that user only. For all users (userid=all) then show all one one page similar to Gary's patch. If userid is not specified then userid default value = $USER->id.
          Hide
          Nicolas Connault added a comment -

          Patch improved and applied successfully. Please test and give feedback.

          I wrote a new grade API function, so that a selector of graded users would be available from anywhere in Moodle (see grade/lib.php). I also upgraded Petr's graded user iterator with 2-field sorting support (sortorder can be set separately for the two fields also). In addition, the "All" option of the graded users selector has the total number of graded users in parentheses (All users (####)).

          Show
          Nicolas Connault added a comment - Patch improved and applied successfully. Please test and give feedback. I wrote a new grade API function, so that a selector of graded users would be available from anywhere in Moodle (see grade/lib.php). I also upgraded Petr's graded user iterator with 2-field sorting support (sortorder can be set separately for the two fields also). In addition, the "All" option of the graded users selector has the total number of graded users in parentheses (All users (####)).
          Hide
          Petr Škoda added a comment -

          Nicolas, please remove/rewrite the users_count() method from iterator - the problem is that we definitely do not want ->RecordCount() because it may fetch everything into memory, which is what the iterator was supposed to prevent in the first place. There is a bug for that already in tracker, though the removal of RecordCount is not completed yet. I guess I separate counting query should be safer.

          Show
          Petr Škoda added a comment - Nicolas, please remove/rewrite the users_count() method from iterator - the problem is that we definitely do not want ->RecordCount() because it may fetch everything into memory, which is what the iterator was supposed to prevent in the first place. There is a bug for that already in tracker, though the removal of RecordCount is not completed yet. I guess I separate counting query should be safer.
          Hide
          Petr Škoda added a comment -

          Also please rethink the access control, following can not work if userid == 'all'

          $context = get_context_instance(CONTEXT_COURSE, $course->id);
          $usercontext = get_context_instance(CONTEXT_USER, $userid);
          require_capability('gradereport/user:view', $context);

          $access = true;
          if (has_capability('moodle/grade:viewall', $context))

          { //ok - can view all course grades }

          else if ($userid == $USER->id and has_capability('moodle/grade:view', $context) and $course->showgrades)

          { //ok - can view own grades }

          else if (has_capability('moodle/grade:viewall', $usercontext) and $course->showgrades)

          { // ok - can view grades of this user- parent most probably }

          else

          { $access = false; }
          Show
          Petr Škoda added a comment - Also please rethink the access control, following can not work if userid == 'all' $context = get_context_instance(CONTEXT_COURSE, $course->id); $usercontext = get_context_instance(CONTEXT_USER, $userid); require_capability('gradereport/user:view', $context); $access = true; if (has_capability('moodle/grade:viewall', $context)) { //ok - can view all course grades } else if ($userid == $USER->id and has_capability('moodle/grade:view', $context) and $course->showgrades) { //ok - can view own grades } else if (has_capability('moodle/grade:viewall', $usercontext) and $course->showgrades) { // ok - can view grades of this user- parent most probably } else { $access = false; }
          Hide
          Gary Anderson added a comment -

          Nicolas:

          This is a good improvement to allows individuals to be selected in the teacher view.

          This improvment allows another change of a feature that teachers at our school have been asking for. In previous version, when teachers looked at the gradebook, they could click on the student name and see only that student's individual results. This was nice, for example, when you called over a student to discuss something that you are concerned about in your gradebook. In 1.9 Beta it just goes to the student profile, which is not too helpful.

          To add this feature back, change line 687 of /grade/report/grader/lib.php from:

          .'<a href="'.$CFG->wwwroot.'/user/view.php?id='.$user->id.'&course='.$this->course->id.'">'

          to:

          .'<a href="'.$CFG->wwwroot.'/grade/report/'.$CFG->grade_profilereport.'?id='.$this->course->id.'&userid='.$user->id.'">'

          Now it will take you to the prefered grade profile report. This does require that such reports accept id as the course argument and userid as the userid, but this is the default for the standard report so it seems like a good standard even if custom reports are later available.

          --Gary

          Show
          Gary Anderson added a comment - Nicolas: This is a good improvement to allows individuals to be selected in the teacher view. This improvment allows another change of a feature that teachers at our school have been asking for. In previous version, when teachers looked at the gradebook, they could click on the student name and see only that student's individual results. This was nice, for example, when you called over a student to discuss something that you are concerned about in your gradebook. In 1.9 Beta it just goes to the student profile, which is not too helpful. To add this feature back, change line 687 of /grade/report/grader/lib.php from: .'<a href="'.$CFG->wwwroot.'/user/view.php?id='.$user->id.'&course='.$this->course->id.'">' to: .'<a href="'.$CFG->wwwroot.'/grade/report/'.$CFG->grade_profilereport.'?id='.$this->course->id.'&userid='.$user->id.'">' Now it will take you to the prefered grade profile report. This does require that such reports accept id as the course argument and userid as the userid, but this is the default for the standard report so it seems like a good standard even if custom reports are later available. --Gary
          Hide
          Nicolas Connault added a comment -

          Gary, your last point is already mentioned in your big gradebook improvements issue, so I won't address it here. This task is resolved.

          Show
          Nicolas Connault added a comment - Gary, your last point is already mentioned in your big gradebook improvements issue, so I won't address it here. This task is resolved.
          Hide
          Petr Škoda added a comment -

          the iteration does not work after the sorting change introduced here, working on a patch - see linked bug

          Show
          Petr Škoda added a comment - the iteration does not work after the sorting change introduced here, working on a patch - see linked bug

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: