Moodle
  1. Moodle
  2. MDL-31818

Submitting grades on the grader report (non-ajax) causes the page you're on to be forgotten

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.5, 2.2.2, 2.3
    • Fix Version/s: 2.1.6, 2.2.3
    • Component/s: Gradebook
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      To test this you will need at least 2 students enrolled in a course although more is better. There needs to be at least one gradeable activity.

      Go to the grade book and select My preferences > Grader report from the gradebook navigation.

      Set "students-per-page" to be less than the number of students you have enrolled in your test course.

      Go to the grader report check that the paging bar functions correctly. Both the page numbers and the next/previous buttons.

      Turn editing on, go to the first page and enter a grade for a student. Press "Update". Check that you are returned to the correct page.

      Go to any other page of the grader report and enter a grade. Press "Update" and check that you are returned to the correct page.

      Show
      To test this you will need at least 2 students enrolled in a course although more is better. There needs to be at least one gradeable activity. Go to the grade book and select My preferences > Grader report from the gradebook navigation. Set "students-per-page" to be less than the number of students you have enrolled in your test course. Go to the grader report check that the paging bar functions correctly. Both the page numbers and the next/previous buttons. Turn editing on, go to the first page and enter a grade for a student. Press "Update". Check that you are returned to the correct page. Go to any other page of the grader report and enter a grade. Press "Update" and check that you are returned to the correct page.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      MDL-31818_grader_paging
    • Rank:
      38449

      Description

      If you set the "students-per-page" grader report setting to be less than the number of students you have in a course the grader report will be paged. If you move to a page other than 1, turn editing on, enter some grades and click update you are taken back to page 1. It should return you to the same page.

        Issue Links

          Activity

          Hide
          Andrew Davis added a comment -

          While looking into this I noticed the below in grade/report/grader/index.php

          // Override perpage if set in URL
          if ($perpageurl) {
              $report->user_prefs['studentsperpage'] = $perpageurl;
          }
          

          It doesn't work as there is nothing at $report->user_prefs and most likely hasn't worked since at least 2007 so I'm intending to remove it barring a good reason becoming apparent.

          Show
          Andrew Davis added a comment - While looking into this I noticed the below in grade/report/grader/index.php // Override perpage if set in URL if ($perpageurl) { $report->user_prefs['studentsperpage'] = $perpageurl; } It doesn't work as there is nothing at $report->user_prefs and most likely hasn't worked since at least 2007 so I'm intending to remove it barring a good reason becoming apparent.
          Hide
          Andrew Davis added a comment -

          Adding a branch for peer review

          Show
          Andrew Davis added a comment - Adding a branch for peer review
          Hide
          Andrew Davis added a comment -

          Added testing instructions.

          Show
          Andrew Davis added a comment - Added testing instructions.
          Hide
          Sam Hemelryk added a comment -

          Hi Andrew,

          The changes to add the page param look good.
          As for the removing of the perpageurl it would be nice to clean that up entirely.
          I see in index.php there is still an optional_param for perpage that is unused, and further down at the paging bar it is referring to the studentsperpage preference.
          Btw is would it be easy to get that stuff working again?

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Andrew, The changes to add the page param look good. As for the removing of the perpageurl it would be nice to clean that up entirely. I see in index.php there is still an optional_param for perpage that is unused, and further down at the paging bar it is referring to the studentsperpage preference. Btw is would it be easy to get that stuff working again? Cheers Sam
          Hide
          Andrew Davis added a comment - - edited

          Ive removed that optional_param. Well spotted. The students per page preference is correct. That still exists (and works).

          It would certainly be possible to reimplement the ability to override the students per page setting via a URL parameter. Its possible there is some value to it, generating a report you're going to print perhaps. First though, we need to clarify MDL-26275 as currently its easy to wind up with a grader report that contains more fields that PHP's max_input_vars resulting in much misery and gnashing of teeth Until we have that pinned down I'm wary of creating a three way conflict (the students per page preference, the students per page URL param and the max students per page we can show without reaching max_input_vars)

          Show
          Andrew Davis added a comment - - edited Ive removed that optional_param. Well spotted. The students per page preference is correct. That still exists (and works). It would certainly be possible to reimplement the ability to override the students per page setting via a URL parameter. Its possible there is some value to it, generating a report you're going to print perhaps. First though, we need to clarify MDL-26275 as currently its easy to wind up with a grader report that contains more fields that PHP's max_input_vars resulting in much misery and gnashing of teeth Until we have that pinned down I'm wary of creating a three way conflict (the students per page preference, the students per page URL param and the max students per page we can show without reaching max_input_vars)
          Hide
          Aparup Banerjee added a comment -

          Thanks guys, this has been integrated into 22 and master.
          This test story was a nice read, happy testing.

          Show
          Aparup Banerjee added a comment - Thanks guys, this has been integrated into 22 and master. This test story was a nice read, happy testing.
          Hide
          Aparup Banerjee added a comment -

          Ah Michael has spotted this is affecting 21 as well.
          Thats been picked into 21 now too and gd for testing.

          Show
          Aparup Banerjee added a comment - Ah Michael has spotted this is affecting 21 as well. Thats been picked into 21 now too and gd for testing.
          Hide
          Michael de Raadt added a comment -

          Test result: Success.

          Tested in 2.1, 2.2 and master, with and without JS.

          Show
          Michael de Raadt added a comment - Test result: Success. Tested in 2.1, 2.2 and master, with and without JS.
          Hide
          Sam Hemelryk added a comment -

          Congratulations are in order, you've made it, or at least your code has!
          It's now part of Moodle and both the git and cvs repositories have been updated.

          This issue is being marked as fixed and closed.

          Thank you.

          Show
          Sam Hemelryk added a comment - Congratulations are in order, you've made it, or at least your code has! It's now part of Moodle and both the git and cvs repositories have been updated. This issue is being marked as fixed and closed. Thank you.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: