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

The assignment submissions() function doesn't pass the filter param when $mode = 'next' or 'saveandnext'

    Details

    • Testing Instructions:
      Hide
      • Create a new submittable assignment
      • enrol six users on the course (the more the better)
      • submit assignments for users one, two, four, and six

      As the teacher

      • open the submissions page
      • confirm all six users are displayed
      • note the order of those users
      • Under 'Optional Settings', set Show to 'Require grading' and save preferenecs
      • Verify you only have users 1, 2, 4, and 6 displayed
      • Select 'Grade' on user 1
      • Select 'Next'
      • Verify that the grade screen for user 2 is displayed
      • Select 'Next'

      Expected Result

      The grade screen for User 4 is displayed

      Actual Result

      The grade screen for User 3 is displayed

      Show
      Create a new submittable assignment enrol six users on the course (the more the better) submit assignments for users one, two, four, and six As the teacher open the submissions page confirm all six users are displayed note the order of those users Under 'Optional Settings', set Show to 'Require grading' and save preferenecs Verify you only have users 1, 2, 4, and 6 displayed Select 'Grade' on user 1 Select 'Next' Verify that the grade screen for user 2 is displayed Select 'Next' Expected Result The grade screen for User 4 is displayed Actual Result The grade screen for User 3 is displayed
    • 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-30173-master-2

      Description

      submissions() doesn't check whether a filter is in use and therefore returns results which aren't within that result set when moving to consecutive results within the result set

      Cause

      The filter param is missing in the redirect() for the 'next' (and 'saveandnext') case of submissions().
      As a result, when the grade screen for the second user is displayed, and the URL for the 'next' button is calculated, the wrong user is calculated

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            dobedobedoh Andrew Nicols added a comment -

            Please note that this causes an issue with offset to become apparent. See MDL-30174 for further info.

            These two issues should probably be integrated together but they are separate bugs

            Show
            dobedobedoh Andrew Nicols added a comment - Please note that this causes an issue with offset to become apparent. See MDL-30174 for further info. These two issues should probably be integrated together but they are separate bugs
            Hide
            dobedobedoh Andrew Nicols added a comment -

            I've converted this to use the moodle_url class as it allows for easier addition of the filter URL param if it's required

            Show
            dobedobedoh Andrew Nicols added a comment - I've converted this to use the moodle_url class as it allows for easier addition of the filter URL param if it's required
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for providing these fixes. We're pretty busy preparing for 2.2 release, but I'll see if we can get these two issues peer reviewed soon.

            Show
            salvetore Michael de Raadt added a comment - Thanks for providing these fixes. We're pretty busy preparing for 2.2 release, but I'll see if we can get these two issues peer reviewed soon.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Hi Andrew,
            This is looking good.
            Make sure you take care of stable branches as well ( if the patch cant be cherry-picked)
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Hi Andrew, This is looking good. Make sure you take care of stable branches as well ( if the patch cant be cherry-picked) Thanks
            Hide
            dobedobedoh Andrew Nicols added a comment -

            Confirmed the presence of the bug and that it cherry-picks correctly 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 correctly on MOODLE_21_STABLE Will check MOODLE_20_STABLE in a moment
            Hide
            dobedobedoh Andrew Nicols added a comment -

            Verified presence of bug and confirmed that it cherry-picks cleanly onto MOODLE_20_STABLE.
            I can provide stable pull branches if you prefer - what's easiest for integrators? A pull branch, or report that cherry-picking works cleanly?

            Show
            dobedobedoh Andrew Nicols added a comment - Verified presence of bug and confirmed that it cherry-picks cleanly onto MOODLE_20_STABLE. I can provide stable pull branches if you prefer - what's easiest for integrators? A pull branch, or report that cherry-picking works cleanly?
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            HiAndrew,
            Thanks for confirming that.
            Should work either ways!
            Submitting for integration

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

            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - HiAndrew, Thanks for confirming that. Should work either ways! Submitting for integration Note to integrators: This can be cherry-picked to 20 and 21 Thanks
            Hide
            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
            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
            dobedobedoh Andrew Nicols added a comment -

            I've updated the patch slightly after finding an issue during additional testing

            Show
            dobedobedoh Andrew Nicols added a comment - I've updated the patch slightly after finding an issue during additional testing
            Hide
            nebgor Aparup Banerjee added a comment - - edited

            Thanks for this fix!

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

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

            Passes thanks

            Show
            samhemelryk Sam Hemelryk added a comment - Passes thanks
            Hide
            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