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
    • Rank:
      26060

      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

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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 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 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
          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
          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
          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
          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 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 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
          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
          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
          Andrew Nicols added a comment -

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

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

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

          Show
          Andrew Nicols added a comment - I've updated the patch slightly after finding an issue during additional testing
          Hide
          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
          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
          Sam Hemelryk added a comment -

          Passes thanks

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