Moodle

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

Details

  • Type: Bug Bug
  • Status: Closed 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 from Repository:
  • Pull Master Branch:
    MDL-31818_grader_paging
  • Pull Master Diff URL:

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

will help resolve

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-26275 Large gradebook won't save changes due to POST size limit

  • Critical - Crashes server, loss of data, severe memory leak
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
Andrew Davis added a comment -

Adding a branch for peer review

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

Added testing instructions.

Show
Andrew Davis added a comment - Added testing instructions.
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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

Vote (1)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: