Moodle
  1. Moodle
  2. MDL-21374

column averages are in the wrong position in the grader report

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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
    • Rank:
      26550

      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

      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
          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
          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
          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
          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
          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
          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
          Petr Škoda added a comment -

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

          Show
          Petr Škoda added a comment - arrg, another colspan issue, +1 to fix this asap
          Hide
          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
          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
          Petr Škoda added a comment -

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

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

          committed

          Show
          Andrew Davis added a comment - committed

            People

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

              Dates

              • Created:
                Updated:
                Resolved: