Uploaded image for project: '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
    • Status: Closed
    • Priority: 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

      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

        Gliffy Diagrams

          Issue Links

            Activity

            ganderson Gary Anderson created issue -
            Hide
            dougiamas 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
            dougiamas 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
            ganderson 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
            ganderson 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
            ganderson 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
            ganderson 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
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            ganderson 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
            ganderson 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
            ganderson Gary Anderson added a comment -

            Fixed original code based on subsequent comment.

            Show
            ganderson Gary Anderson added a comment - Fixed original code based on subsequent comment.
            ganderson Gary Anderson made changes -
            Field Original Value New Value
            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();
                    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
            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
            Hide
            dougiamas 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
            dougiamas 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.
            dougiamas Martin Dougiamas made changes -
            Assignee Yu Zhang [ lazyfish ] Nicolas Connault [ nicolasconnault ]
            Hide
            nicolasconnault 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
            nicolasconnault 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 (####)).
            nicolasconnault Nicolas Connault made changes -
            Status Open [ 1 ] Resolved [ 5 ]
            Fix Version/s 2.0 [ 10122 ]
            Resolution Fixed [ 1 ]
            Hide
            skodak Petr Skoda 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
            skodak Petr Skoda 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.
            skodak Petr Skoda made changes -
            Resolution Fixed [ 1 ]
            Status Resolved [ 5 ] Reopened [ 4 ]
            Hide
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            ganderson 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
            ganderson 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
            nicolasconnault 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
            nicolasconnault 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.
            nicolasconnault Nicolas Connault made changes -
            Status Reopened [ 4 ] Resolved [ 5 ]
            Resolution Fixed [ 1 ]
            skodak Petr Skoda made changes -
            Link This issue has been marked as being related by MDL-13454 [ MDL-13454 ]
            Hide
            skodak Petr Skoda added a comment -

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

            Show
            skodak Petr Skoda added a comment - the iteration does not work after the sorting change introduced here, working on a patch - see linked bug
            dougiamas Martin Dougiamas made changes -
            Status Resolved [ 5 ] Closed [ 6 ]
            QA Assignee nobody
            dougiamas Martin Dougiamas made changes -
            Workflow jira [ 23344 ] MDL Workflow [ 57846 ]
            dougiamas Martin Dougiamas made changes -
            Workflow MDL Workflow [ 57846 ] MDL Full Workflow [ 86962 ]

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  3/Mar/08