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

Offset not adjusted when using 'saveandnext'

          Details

          • Testing Instructions:
            Hide

            Working with my patch for MDL-30173 applied and using the same base instructions:

            Users 1,2,4,6 have submitted work. It's not yet graded. As a teacher:

            • Click the 'Grade' link for user 1
            • write a comment, give a grade (be generous), and choose 'Save and Next'
            • Verify that the page for User 2 is displayed.
            • write a comment, give a grade (be generous), and choose 'Save and Next'

            Expected Result

            The grading page for user 4 should be displayed

            Actual result

            The grading page for user 6 is displayed

            Cause

            The offset was not reduced after marking in mod/assignment/lib.php::assignment_base->submission(). As a result when display_submission() is called and the offset retrieved, the wrong user is calculated for the next page.

            Show
            Working with my patch for MDL-30173 applied and using the same base instructions: Users 1,2,4,6 have submitted work. It's not yet graded. As a teacher: Click the 'Grade' link for user 1 write a comment, give a grade (be generous), and choose 'Save and Next' Verify that the page for User 2 is displayed. write a comment, give a grade (be generous), and choose 'Save and Next' Expected Result The grading page for user 4 should be displayed Actual result The grading page for user 6 is displayed Cause The offset was not reduced after marking in mod/assignment/lib.php::assignment_base->submission(). As a result when display_submission() is called and the offset retrieved, the wrong user is calculated for the next page.
          • Affected Branches:
            MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
          • Fixed Branches:
            MOODLE_20_STABLE, MOODLE_21_STABLE
          • Pull from Repository:
          • Pull Master Branch:
            MDL-30174-master-2
          • Pull Master Diff URL:

            Description

            When using the "Save and Next" during assignment grading, and after my patch to get filter results to apply in MDL-30173 is applied; a bug with offset calculation becomes apparent.

            When Saving grade information, the offset should be decreased when calculating the URL for the 'next' button.

              Gliffy Diagrams

                Attachments

                  Issue Links

                  has been marked as being related by

                  Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-30173 The assignment submissions() function doesn't pass the filter param when $mode = 'next' or 'saveandnext'

                  • Minor - Minor loss of function where workaround is possible
                  • Closed

                    Activity

                    Ascending order - Click to sort in descending order
                    Hide
                    Permalink
                    salvetore Michael de Raadt added a comment -

                    Thanks again.

                    Show
                    salvetore Michael de Raadt added a comment - Thanks again.
                    Hide
                    Permalink
                    ankit_frenz Ankit Agarwal added a comment -

                    Looks good!
                    Thanks

                    Show
                    ankit_frenz Ankit Agarwal added a comment - Looks good! Thanks
                    Hide
                    Permalink
                    dobedobedoh Andrew Nicols added a comment -

                    Confirmed the presence of the bug and that it cherry-picks cleanly on MOODLE_21_STABLE
                    Will check MOODLE_20_STABLE in a moment

                    Show
                    dobedobedoh Andrew Nicols added a comment - Confirmed the presence of the bug and that it cherry-picks cleanly on MOODLE_21_STABLE Will check MOODLE_20_STABLE in a moment
                    Hide
                    Permalink
                    dobedobedoh Andrew Nicols added a comment -

                    Verified presence of bug and confirmed that it cherry-picks cleanly onto MOODLE_20_STABLE.

                    Show
                    dobedobedoh Andrew Nicols added a comment - Verified presence of bug and confirmed that it cherry-picks cleanly onto MOODLE_20_STABLE.
                    Hide
                    Permalink
                    ankit_frenz Ankit Agarwal added a comment -

                    Hi Andrew,
                    Thanks for confirming that.
                    Submitting for integration

                    Note to integrators: This can be cherry-picked to 20 and 21

                    Thanks

                    Show
                    ankit_frenz Ankit Agarwal added a comment - Hi Andrew, Thanks for confirming that. Submitting for integration Note to integrators: This can be cherry-picked to 20 and 21 Thanks
                    Hide
                    Permalink
                    stronk7 Eloy Lafuente (stronk7) added a comment -

                    The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

                    TIA and ciao

                    PS: Note this is the last message of this type that you will receive along the whole November month, because we are running continuous integration this weeks while QA tests for release of Moodle 2.2 (Dec 1st) are being performed.

                    Show
                    stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao PS: Note this is the last message of this type that you will receive along the whole November month, because we are running continuous integration this weeks while QA tests for release of Moodle 2.2 (Dec 1st) are being performed.
                    Hide
                    Permalink
                    dobedobedoh Andrew Nicols added a comment -

                    Still cherry-picks cleanly on latest MOODLE_20_STABLE, MOODLE_21_STABLE, and master.

                    Show
                    dobedobedoh Andrew Nicols added a comment - Still cherry-picks cleanly on latest MOODLE_20_STABLE, MOODLE_21_STABLE, and master.
                    Hide
                    Permalink
                    dobedobedoh Andrew Nicols added a comment -

                    I discovered during testing that if the filter applied wasn't FILTER_REQUIRED_GRADING, then the offset was not increased.

                    I've adjusted the patch for this and MDL-30173 to use a default filter of FILTER_ALL if none was supplied in the get params, and to increase the offset if the filter wasn't FILTER_REQUIRE_GRADING too.

                    Tested on master

                    Show
                    dobedobedoh Andrew Nicols added a comment - I discovered during testing that if the filter applied wasn't FILTER_REQUIRED_GRADING, then the offset was not increased. I've adjusted the patch for this and MDL-30173 to use a default filter of FILTER_ALL if none was supplied in the get params, and to increase the offset if the filter wasn't FILTER_REQUIRE_GRADING too. Tested on master
                    Hide
                    Permalink
                    nebgor Aparup Banerjee added a comment -

                    Thanks for this fix!

                    Its been integrated now and ready for testing.
                    (cherry-picked to 2.x stable branches and master)

                    Show
                    nebgor Aparup Banerjee added a comment - Thanks for this fix! Its been integrated now and ready for testing. (cherry-picked to 2.x stable branches and master)
                    Hide
                    Permalink
                    samhemelryk Sam Hemelryk added a comment -

                    Passed thanks.

                    Show
                    samhemelryk Sam Hemelryk added a comment - Passed thanks.
                    Hide
                    Permalink
                    stronk7 Eloy Lafuente (stronk7) added a comment -

                    Closing as fixed, many thanks for your effort!

                    Note that the changes related to master (2.2beta) have been already sent upstream. But the stable ones will be part of next weeklies (Wed/Thu) as usual.

                    Ciao

                    Show
                    stronk7 Eloy Lafuente (stronk7) added a comment - Closing as fixed, many thanks for your effort! Note that the changes related to master (2.2beta) have been already sent upstream. But the stable ones will be part of next weeklies (Wed/Thu) as usual. Ciao

                      People

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

                        Dates

                        • Created:
                          Updated:
                          Resolved:
                          Fix Release Date:
                          28/Nov/11