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

"Row is out of bounds" error message when grading first student in assignment

    Details

      Description

      Noticed by me when testing the latest Moodle 2.4 and by a forum poster on the latest Moodle 2.3.3+ (which I could replicate)
      Steps to reproduce:
      1. Create an assignment and click on "View/grade all submissions"
      2. Click on "grade" for the first student at the top of the list
      3. You will get the message "Row is out of bounds for the current grading table: -1."

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              grahambowman Graham Bowman added a comment -

              The Grade link for each student is wrong by 1. Clicking to grade student 2 will load the page to grade student 1.

              Show
              grahambowman Graham Bowman added a comment - The Grade link for each student is wrong by 1. Clicking to grade student 2 will load the page to grade student 1.
              Hide
              wcchien Chien Wen-Chang(簡文章) added a comment - - edited

              I replaced line 44 of \mod\assign\gradingtable.php

               
                  private $rownum = -1;

              with

               
                  private $rownum = 0;

              I tested again and no error occured.

              Show
              wcchien Chien Wen-Chang(簡文章) added a comment - - edited I replaced line 44 of \mod\assign\gradingtable.php private $rownum = -1; with private $rownum = 0; I tested again and no error occured.
              Hide
              tsala Helen Foster added a comment -

              Please see MDL-36520 for debug info and linked issue.

              Show
              tsala Helen Foster added a comment - Please see MDL-36520 for debug info and linked issue.
              Hide
              tsala Helen Foster added a comment -

              Increasing priority to blocker, as it is blocking 2.4 QA tests (in addition to being broken functionality in 2.3).

              Show
              tsala Helen Foster added a comment - Increasing priority to blocker, as it is blocking 2.4 QA tests (in addition to being broken functionality in 2.3).
              Hide
              pezsgo Gábor Törley added a comment -

              Hello!

              I realized the same problem like Graham and I did the same like Chen, but this works only if I see all of the students.

              But if I change the Options Assignment per All to for example 10 and I want to check for example the second student's work on the 2., 3., 4., ... page then I got the second student's work on the 1. page.

              Before I did the update to 2.3.3 (from 2.3.2+), everything was fine.

              Thanks for your help!

              Show
              pezsgo Gábor Törley added a comment - Hello! I realized the same problem like Graham and I did the same like Chen, but this works only if I see all of the students. But if I change the Options Assignment per All to for example 10 and I want to check for example the second student's work on the 2., 3., 4., ... page then I got the second student's work on the 1. page. Before I did the update to 2.3.3 (from 2.3.2+), everything was fine. Thanks for your help!
              Hide
              cscurrah Christopher Scurrah added a comment -

              I can confirm with Gábor that the first page is fine but as soon as you go to the next pages it brings up the wrong students

              Show
              cscurrah Christopher Scurrah added a comment - I can confirm with Gábor that the first page is fine but as soon as you go to the next pages it brings up the wrong students
              Hide
              wcchien Chien Wen-Chang(簡文章) added a comment - - edited

              I replaced line 44 of \mod\assign\gradingtable.php

               
                  private $rownum = -1;

              with

               
                  private $rownum = 0;

              and line 344 of \mod\assign\gradingtable.php

               
                                  'rownum'=>$this->rownum ,'action'=>'grade'));

              with

               
                                  'rownum'=>$this->rownum + $this->perpage * $this->currpage ,'action'=>'grade'));

              and line 441 of \mod\assign\gradingtable.php

               
                          $this->rownum += 1;

              with

               
                 //         $this->rownum += 1;

              and line 448 of \mod\assign\gradingtable.php

               
                                          'rownum'=>$this->rownum,'action'=>'grade'));

              with

               
                                          'rownum'=>$this->rownum + $this->perpage * $this->currpage ,'action'=>'grade'));

              Page 2,3,4.. No error occured.

              Show
              wcchien Chien Wen-Chang(簡文章) added a comment - - edited I replaced line 44 of \mod\assign\gradingtable.php private $rownum = -1; with private $rownum = 0; and line 344 of \mod\assign\gradingtable.php 'rownum'=>$this->rownum ,'action'=>'grade')); with 'rownum'=>$this->rownum + $this->perpage * $this->currpage ,'action'=>'grade')); and line 441 of \mod\assign\gradingtable.php $this->rownum += 1; with // $this->rownum += 1; and line 448 of \mod\assign\gradingtable.php 'rownum'=>$this->rownum,'action'=>'grade')); with 'rownum'=>$this->rownum + $this->perpage * $this->currpage ,'action'=>'grade')); Page 2,3,4.. No error occured.
              Hide
              cscurrah Christopher Scurrah added a comment -

              I tried your fix Chien, but got the first user's grading view on all the other users when I clicked on green tick box for each of the students

              That was with line 44 set to:-
              private $rownum = 0;

              When I changed line 44 back to the original setting:-
              private $rownum = -1;

              The original error came up

              Show
              cscurrah Christopher Scurrah added a comment - I tried your fix Chien, but got the first user's grading view on all the other users when I clicked on green tick box for each of the students That was with line 44 set to:- private $rownum = 0; When I changed line 44 back to the original setting:- private $rownum = -1; The original error came up
              Hide
              damyon Damyon Wiese added a comment - - edited

              Moving the order of the columns changes when rownum gets incremented (This refers to the linked issue which caused this regression). Patch incoming.

              Show
              damyon Damyon Wiese added a comment - - edited Moving the order of the columns changes when rownum gets incremented (This refers to the linked issue which caused this regression). Patch incoming.
              Hide
              damyon Damyon Wiese added a comment -

              This patch fixes the issue and makes the order of the columns no longer important.

              Show
              damyon Damyon Wiese added a comment - This patch fixes the issue and makes the order of the columns no longer important.
              Hide
              damyon Damyon Wiese added a comment -

              Also thanks everyone for helping track down this issue.

              Show
              damyon Damyon Wiese added a comment - Also thanks everyone for helping track down this issue.
              Hide
              wcchien Chien Wen-Chang(簡文章) added a comment -

              I tested MDL-36509-m23.
              In page 2, click on "grade" for the first student at the top of the list.
              You should see the grading page for the selected student is first student of Page 1.

              Show
              wcchien Chien Wen-Chang(簡文章) added a comment - I tested MDL-36509 -m23. In page 2, click on "grade" for the first student at the top of the list. You should see the grading page for the selected student is first student of Page 1.
              Hide
              damyon Damyon Wiese added a comment -

              Just updated this patch to fix the docstrings

              Show
              damyon Damyon Wiese added a comment - Just updated this patch to fix the docstrings
              Hide
              damyon Damyon Wiese added a comment -

              Thanks for testing this Chien - but I cannot reproduce this error - can you please provide some more information?

              Show
              damyon Damyon Wiese added a comment - Thanks for testing this Chien - but I cannot reproduce this error - can you please provide some more information?
              Hide
              poltawski Dan Poltawski added a comment -

              Ahha, I hit this myself too

              Show
              poltawski Dan Poltawski added a comment - Ahha, I hit this myself too
              Hide
              poltawski Dan Poltawski added a comment -

              Thanks Damyon, i've integrated this now.

              Show
              poltawski Dan Poltawski added a comment - Thanks Damyon, i've integrated this now.
              Hide
              cscurrah Christopher Scurrah added a comment -

              The patch works for me in Moodle 2.3.3

              Show
              cscurrah Christopher Scurrah added a comment - The patch works for me in Moodle 2.3.3
              Hide
              wcchien Chien Wen-Chang(簡文章) added a comment - - edited

              Sorry, my bad.

              I re-tested MDL-36509-m23.

              It' OK ! No error !

              Show
              wcchien Chien Wen-Chang(簡文章) added a comment - - edited Sorry, my bad. I re-tested MDL-36509 -m23. It' OK ! No error !
              Hide
              koen Koen Roggemans added a comment -

              @Dan: will this end up in the weekly, say on Thursday? It hits us badly (updated this moring to latest ), but I prefer not to dig in the code.

              Show
              koen Koen Roggemans added a comment - @Dan: will this end up in the weekly, say on Thursday? It hits us badly (updated this moring to latest ), but I prefer not to dig in the code.
              Hide
              pezsgo Gábor Törley added a comment -

              I also tested MDL-36509-m23. It is OK, no error! Thanks!

              Show
              pezsgo Gábor Törley added a comment - I also tested MDL-36509 -m23. It is OK, no error! Thanks!
              Hide
              poltawski Dan Poltawski added a comment -

              Koen: yes, assuming it passes testing

              Show
              poltawski Dan Poltawski added a comment - Koen: yes, assuming it passes testing
              Hide
              fred Frédéric Massart added a comment -

              Failing for investigation.

              On master only, I had this notice appearing:

              Notice: Undefined variable: type in /home/fred/www/repositories/im/moodle/lib/pear/HTML/QuickForm/group.php on line 277
               
              Call Stack:
                  0.0001     655760   1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0
                  0.2467   68211408   2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53
                  0.2467   68212144   3. assign->view_single_grade_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:398
                  1.2287  108328728   4. plugin_renderer_base->render() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:2163
                  1.2287  108328880   5. mod_assign_renderer->render_assign_form() /home/fred/www/repositories/im/moodle/lib/outputrenderers.php:215
                  1.2289  108330984   6. mod_assign_renderer->moodleform() /home/fred/www/repositories/im/moodle/mod/assign/renderer.php:115
                  1.2289  108372336   7. moodleform->display() /home/fred/www/repositories/im/moodle/mod/assign/renderer.php:858
                  1.2289  108372416   8. HTML_QuickForm_DHTMLRulesTableless->display() /home/fred/www/repositories/im/moodle/lib/formslib.php:917
                  1.2290  108372416   9. HTML_Common->display() /home/fred/www/repositories/im/moodle/lib/pear/HTML/QuickForm/DHTMLRulesTableless.php:204
                  1.2290  108372416  10. HTML_QuickForm->toHtml() /home/fred/www/repositories/im/moodle/lib/pear/HTML/Common.php:435
                  1.2290  108372496  11. MoodleQuickForm->accept() /home/fred/www/repositories/im/moodle/lib/pear/HTML/QuickForm.php:1674
                  1.2293  108372816  12. HTML_QuickForm->accept() /home/fred/www/repositories/im/moodle/lib/formslib.php:1467
                  1.2319  108391648  13. HTML_QuickForm_group->accept() /home/fred/www/repositories/im/moodle/lib/pear/HTML/QuickForm.php:1631
                  1.2319  108391808  14. MoodleQuickForm_Renderer->startGroup() /home/fred/www/repositories/im/moodle/lib/pear/HTML/QuickForm/group.php:428
                  1.2319  108392400  15. MoodleQuickForm_group->getElementTemplateType() /home/fred/www/repositories/im/moodle/lib/formslib.php:2296
                  1.2320  108392400  16. HTML_QuickForm_group->getGroupType() /home/fred/www/repositories/im/moodle/lib/form/group.php:80

              The only noticeable difference with 2.3 is the 'Cut-off date' that was set.

              Show
              fred Frédéric Massart added a comment - Failing for investigation. On master only, I had this notice appearing: Notice: Undefined variable: type in /home/fred/www/repositories/im/moodle/lib/pear/HTML/QuickForm/group.php on line 277   Call Stack: 0.0001 655760 1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0 0.2467 68211408 2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53 0.2467 68212144 3. assign->view_single_grade_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:398 1.2287 108328728 4. plugin_renderer_base->render() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:2163 1.2287 108328880 5. mod_assign_renderer->render_assign_form() /home/fred/www/repositories/im/moodle/lib/outputrenderers.php:215 1.2289 108330984 6. mod_assign_renderer->moodleform() /home/fred/www/repositories/im/moodle/mod/assign/renderer.php:115 1.2289 108372336 7. moodleform->display() /home/fred/www/repositories/im/moodle/mod/assign/renderer.php:858 1.2289 108372416 8. HTML_QuickForm_DHTMLRulesTableless->display() /home/fred/www/repositories/im/moodle/lib/formslib.php:917 1.2290 108372416 9. HTML_Common->display() /home/fred/www/repositories/im/moodle/lib/pear/HTML/QuickForm/DHTMLRulesTableless.php:204 1.2290 108372416 10. HTML_QuickForm->toHtml() /home/fred/www/repositories/im/moodle/lib/pear/HTML/Common.php:435 1.2290 108372496 11. MoodleQuickForm->accept() /home/fred/www/repositories/im/moodle/lib/pear/HTML/QuickForm.php:1674 1.2293 108372816 12. HTML_QuickForm->accept() /home/fred/www/repositories/im/moodle/lib/formslib.php:1467 1.2319 108391648 13. HTML_QuickForm_group->accept() /home/fred/www/repositories/im/moodle/lib/pear/HTML/QuickForm.php:1631 1.2319 108391808 14. MoodleQuickForm_Renderer->startGroup() /home/fred/www/repositories/im/moodle/lib/pear/HTML/QuickForm/group.php:428 1.2319 108392400 15. MoodleQuickForm_group->getElementTemplateType() /home/fred/www/repositories/im/moodle/lib/formslib.php:2296 1.2320 108392400 16. HTML_QuickForm_group->getGroupType() /home/fred/www/repositories/im/moodle/lib/form/group.php:80 The only noticeable difference with 2.3 is the 'Cut-off date' that was set.
              Hide
              damyon Damyon Wiese added a comment -

              Hi Fred,

              I can't reproduce the error you found - can you please add some more details about the settings for the assignment. The stack trace looks like there is a button group in the form with no elements - but I can't see how that would happen.

              • Damyon
              Show
              damyon Damyon Wiese added a comment - Hi Fred, I can't reproduce the error you found - can you please add some more details about the settings for the assignment. The stack trace looks like there is a button group in the form with no elements - but I can't see how that would happen. Damyon
              Hide
              fred Frédéric Massart added a comment -

              Hi Damyon,

              hard to guide you because I kept all the default values there when creating the new assignment. The only difference that I could think of is that I pretty much enabled all the possible "advanced features", including the activity completion and condition access. I don't know if that could play a role though, probably not.

              Also the student that I am trying to grade has never attempted the assignment. I am attaching a backup of my assignment to that issue.

              Cheers

              Show
              fred Frédéric Massart added a comment - Hi Damyon, hard to guide you because I kept all the default values there when creating the new assignment. The only difference that I could think of is that I pretty much enabled all the possible "advanced features", including the activity completion and condition access. I don't know if that could play a role though, probably not. Also the student that I am trying to grade has never attempted the assignment. I am attaching a backup of my assignment to that issue. Cheers
              Hide
              damyon Damyon Wiese added a comment -

              Thanks Fred, but I am still unable to reproduce this error - I tried using the backup file you provided - and I also went through the backup file but could not see anything unusual in it.

              If you could do any more testing to help narrow this down it would be appreciated (you could try enabling/disabling some of those advanced features and/or grading methods to see if it makes the warning go away).

              Regards, Damyon

              Show
              damyon Damyon Wiese added a comment - Thanks Fred, but I am still unable to reproduce this error - I tried using the backup file you provided - and I also went through the backup file but could not see anything unusual in it. If you could do any more testing to help narrow this down it would be appreciated (you could try enabling/disabling some of those advanced features and/or grading methods to see if it makes the warning go away). Regards, Damyon
              Hide
              fred Frédéric Massart added a comment -

              Well, I have found the issue, and I think the problem was that I only had one student to be graded. Then the group 'navar' was set without any elements in it.

              Here is a patch that seems to do the trick:

              diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php
              index a6dd9bc..4837593 100644
              --- a/mod/assign/locallib.php
              +++ b/mod/assign/locallib.php
              @@ -3966,7 +3966,9 @@ class assign {
                       if (!$last) {
                           $buttonarray[] = $mform->createElement('submit', 'nosaveandnext', get_string('nosavebutnext', 'assign'));
                       }
              -        $mform->addGroup($buttonarray, 'navar', '', array(' '), false);
              +        if (!empty($buttonarray)) {
              +            $mform->addGroup($buttonarray, 'navar', '', array(' '), false);
              +        }
                   }

              We can either pass or fail the test if this needs to be closed quickly. Otherwise I can retest this after the patch has been included.

              Show
              fred Frédéric Massart added a comment - Well, I have found the issue, and I think the problem was that I only had one student to be graded. Then the group 'navar' was set without any elements in it. Here is a patch that seems to do the trick: diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index a6dd9bc..4837593 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -3966,7 +3966,9 @@ class assign { if (!$last) { $buttonarray[] = $mform->createElement('submit', 'nosaveandnext', get_string('nosavebutnext', 'assign')); } - $mform->addGroup($buttonarray, 'navar', '', array(' '), false); + if (!empty($buttonarray)) { + $mform->addGroup($buttonarray, 'navar', '', array(' '), false); + } } We can either pass or fail the test if this needs to be closed quickly. Otherwise I can retest this after the patch has been included.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment - - edited

              Hi guys,

              I'm going to apply Fred's patch to 23 and master in a few hours (5-6) in order to get this passed and be able to release stable weeklies.

              If there is any objection in your side, plz Damyon, drop it here before then. TIA!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - - edited Hi guys, I'm going to apply Fred's patch to 23 and master in a few hours (5-6) in order to get this passed and be able to release stable weeklies. If there is any objection in your side, plz Damyon, drop it here before then. TIA!
              Hide
              fred Frédéric Massart added a comment -

              I have quickly tested this patch and it looks OK. It should be ok to be integrated and tested again.

              Show
              fred Frédéric Massart added a comment - I have quickly tested this patch and it looks OK. It should be ok to be integrated and tested again.
              Hide
              mvgerwen Mitchell van Gerwen added a comment - - edited

              Hello guys,

              I have tested the patch from Frédéric, and so far, it seems to work in version 2.3.3,but not in 2.4 beta.

              EDIT

              strange, I had tested it on a test server, and that did the trick. on the production server, which is the same, the patch didn't work. sorry for any msiunderstandings

              Greetings,

              Mitchell

              Show
              mvgerwen Mitchell van Gerwen added a comment - - edited Hello guys, I have tested the patch from Frédéric, and so far, it seems to work in version 2.3.3,but not in 2.4 beta. EDIT strange, I had tested it on a test server, and that did the trick. on the production server, which is the same, the patch didn't work. sorry for any msiunderstandings Greetings, Mitchell
              Hide
              damyon Damyon Wiese added a comment -

              The patch from Fred looks good to me (thanks Fred) and his description of new the bug makes sense - but this is really a separate issue and much less of a priority than this one. I'll create a new issue for that bug and the original patch for this still looks OK to me.

              Show
              damyon Damyon Wiese added a comment - The patch from Fred looks good to me (thanks Fred) and his description of new the bug makes sense - but this is really a separate issue and much less of a priority than this one. I'll create a new issue for that bug and the original patch for this still looks OK to me.
              Hide
              fred Frédéric Massart added a comment -

              Passing the test as the issue I raised was not relevant. Thanks Damyon for raising a separate issue to take care of it.

              Show
              fred Frédéric Massart added a comment - Passing the test as the issue I raised was not relevant. Thanks Damyon for raising a separate issue to take care of it.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Many, many thanks for your effort!

              Millions of people will enjoy the results of your work, yay!

              Closing as fixed. Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Many, many thanks for your effort! Millions of people will enjoy the results of your work, yay! Closing as fixed. Ciao
              Hide
              koen Koen Roggemans added a comment -

              Yes, I'm one of them . Thank you all for this quick fix

              Show
              koen Koen Roggemans added a comment - Yes, I'm one of them . Thank you all for this quick fix
              Hide
              moodlistov moodlist added a comment -

              This will be fixed only in 2.3.4? In 2.3.3 would not be?

              Show
              moodlistov moodlist added a comment - This will be fixed only in 2.3.4? In 2.3.3 would not be?
              Hide
              tsala Helen Foster added a comment -

              It's fixed in the most recent 2.3.3+ weekly stable build.

              Show
              tsala Helen Foster added a comment - It's fixed in the most recent 2.3.3+ weekly stable build.
              Hide
              alexjeddah alex sykes added a comment -

              Not sure if this is related to this issue - however, there appears to be a further complication relating to the feedback dialog. As follows... NB I am using rowindex in place of rownum due to my security settings.

              view/grade all submissions ->
              Grade ->
              rowindex=0&action=grade -> add comments in feedback textfield -> click 'Save and show next,
              next student submission (rowindex=1 shown in hidden field) has same feedback in textfield (ie feedback for rowindex=0)
              Click to view online text, then 'Back' and correct (previously saved) feedback appears in text field

              Show
              alexjeddah alex sykes added a comment - Not sure if this is related to this issue - however, there appears to be a further complication relating to the feedback dialog. As follows... NB I am using rowindex in place of rownum due to my security settings. view/grade all submissions -> Grade -> rowindex=0&action=grade -> add comments in feedback textfield -> click 'Save and show next, next student submission (rowindex=1 shown in hidden field) has same feedback in textfield (ie feedback for rowindex=0) Click to view online text, then 'Back' and correct (previously saved) feedback appears in text field
              Hide
              damyon Damyon Wiese added a comment -

              Hi Alex,

              That sounds like MDL-35726. Also see MDL-36289 for another recently fixed issue in this area.

              • Damyon
              Show
              damyon Damyon Wiese added a comment - Hi Alex, That sounds like MDL-35726 . Also see MDL-36289 for another recently fixed issue in this area. Damyon

                People

                • Votes:
                  16 Vote for this issue
                  Watchers:
                  23 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    14/Jan/13