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

          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