Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-21374

column averages are in the wrong position in the grader report

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.8
    • Fix Version/s: 1.9.8
    • Component/s: Gradebook
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE

      Description

      The columns seem to be off by one in the grader report. See the attached screen shot. I've attached a patch that fixes this.

      I'm not quite sure why the th that is pushing everything over by one was added. It was added as part of MDL-21159

        Gliffy Diagrams

        1. 20100119_offbyone.patch
          1 kB
          Andrew Davis
        2. 20100128_offbyone.patch
          4 kB
          Andrew Davis
        1. offbyone.gif
          24 kB

          Issue Links

            Activity

            Hide
            andyjdavis Andrew Davis added a comment -

            I've added Tim as a watched. He fixed MDL-21159

            Tim, do you recall the reason for adding that th? I can't see any negative side effects caused by just removing it but presumably there was a reason why you added it.

            Show
            andyjdavis Andrew Davis added a comment - I've added Tim as a watched. He fixed MDL-21159 Tim, do you recall the reason for adding that th? I can't see any negative side effects caused by just removing it but presumably there was a reason why you added it.
            Hide
            timhunt Tim Hunt added a comment -

            Yuck, what a mess.

            To be sure everything is working correctly, I guess you need to check with at least the following options in all combinations:

            • $this->is_fixed_students() true/false
            • $this->get_pref('showuseridnumber') true/false
            • has_capability('gradereport/'.$CFG->grade_profilereport.':view', $this->context) true/false

            And make sure you have all possible types of header row, student row, and average row visible.

            We really need a better way to handle tables with configurable columns. You might like to look at what I started to do in the question bank: http://git.moodle.org/gw?p=moodle.git;a=blob;f=question/editlib.php;h=af49fae3c8e75e9a7a90a115601f075bc2e7c461;hb=HEAD#l815. Really, that should be refactored to separate out the question-bank specific stuff from the generic stuff. It also needs to be refactored to use renderers now. Anyway, that is just food for thought for the future. We don't want any more work before 2.0 that we can help.

            Show
            timhunt Tim Hunt added a comment - Yuck, what a mess. To be sure everything is working correctly, I guess you need to check with at least the following options in all combinations: $this->is_fixed_students() true/false $this->get_pref('showuseridnumber') true/false has_capability('gradereport/'.$CFG->grade_profilereport.':view', $this->context) true/false And make sure you have all possible types of header row, student row, and average row visible. We really need a better way to handle tables with configurable columns. You might like to look at what I started to do in the question bank: http://git.moodle.org/gw?p=moodle.git;a=blob;f=question/editlib.php;h=af49fae3c8e75e9a7a90a115601f075bc2e7c461;hb=HEAD#l815 . Really, that should be refactored to separate out the question-bank specific stuff from the generic stuff. It also needs to be refactored to use renderers now. Anyway, that is just food for thought for the future. We don't want any more work before 2.0 that we can help.
            Hide
            jrh18 Jason Hardin added a comment -

            We had the opposite problem with moodle 1.9.7. We had all columns shifted to the left with the addition of the userreport column. I fixed this with the colspan value to 2 instead of 1. See patch below:
            Index: lib.php
            ===================================================================
            — lib.php (revision 8680)
            +++ lib.php (working copy)

            @@ -1166,9 +1175,9 @@

            $fixedstudents = $this->is_fixed_students();
            if (!$fixedstudents) {

            • $colspan='';
              + $colspan='colspan="2" ';
              if ($this->get_pref('showuseridnumber')) { - $colspan = 'colspan="2" '; + $colspan = 'colspan="3" '; }

              $avghtml .= '<th class="header c0 range" '.$colspan.' scope="row">'.$straverage.'</th>';
              }

            Show
            jrh18 Jason Hardin added a comment - We had the opposite problem with moodle 1.9.7. We had all columns shifted to the left with the addition of the userreport column. I fixed this with the colspan value to 2 instead of 1. See patch below: Index: lib.php =================================================================== — lib.php (revision 8680) +++ lib.php (working copy) @@ -1166,9 +1175,9 @@ $fixedstudents = $this->is_fixed_students(); if (!$fixedstudents) { $colspan=''; + $colspan='colspan="2" '; if ($this->get_pref('showuseridnumber')) { - $colspan = 'colspan="2" '; + $colspan = 'colspan="3" '; } $avghtml .= '<th class="header c0 range" '.$colspan.' scope="row">'.$straverage.'</th>'; }
            Hide
            skodak Petr Skoda added a comment -

            arrg, another colspan issue, +1 to fix this asap

            Show
            skodak Petr Skoda added a comment - arrg, another colspan issue, +1 to fix this asap
            Hide
            andyjdavis Andrew Davis added a comment -

            Patch attached. I think this works for all of the scenarios Tim outlined. I'll wait for a review before committing this.

            Show
            andyjdavis Andrew Davis added a comment - Patch attached. I think this works for all of the scenarios Tim outlined. I'll wait for a review before committing this.
            Hide
            skodak Petr Skoda added a comment -

            did not actually test it, but the code seems ok, +1 for commit

            Show
            skodak Petr Skoda added a comment - did not actually test it, but the code seems ok, +1 for commit
            Hide
            andyjdavis Andrew Davis added a comment -

            committed

            Show
            andyjdavis Andrew Davis added a comment - committed

              People

              • Votes:
                2 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Mar/10