Moodle

Add/ Editing Categories / Courses / Sub courses and it crashes with a coding error detected. "paging_bar requires a perpage value"

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.0.3, 2.2.1, 2.2.5, 2.3.2, 2.4
  • Fix Version/s: 2.2.6, 2.3.3
  • Component/s: Administration
  • Labels:
  • Environment:
    Windows Server 2008
  • Database:
    Any
  • Testing Instructions:
    Hide
    1. access your DB and select the config table, rename 'coursesperpage' to 'coursesperpagexyz'
    2. as admin, access site admin > courses > add/edit courses
    3. select one of the categories
    4. make sure there's no error occurs
    5. access config table again and rename 'coursesperpagexyz' to 'coursesperpage'
    Show
    access your DB and select the config table, rename 'coursesperpage' to 'coursesperpagexyz' as admin, access site admin > courses > add/edit courses select one of the categories make sure there's no error occurs access config table again and rename 'coursesperpagexyz' to 'coursesperpage'
  • Workaround:
    Hide

    Merely to go back up a level and then return down the path but its not really achieving the objective.

    Show
    Merely to go back up a level and then return down the path but its not really achieving the objective.
  • Affected Branches:
    MOODLE_20_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
  • Fixed Branches:
    MOODLE_22_STABLE, MOODLE_23_STABLE
  • Pull from Repository:
  • Pull Master Branch:
  • Pull Master Diff URL:

Description

Coding error detected, it must be fixed by a programmer: paging_bar requires a perpage value.

Stack trace:
line 1936 of \lib\outputcomponents.php: coding_exception thrown
line 2212 of \lib\outputrenderers.php: call to paging_bar->prepare()
line 70 of \lib\outputrenderers.php: call to core_renderer->render_paging_bar()
line 2201 of \lib\outputrenderers.php: call to renderer_base->render()
line 261 of \course\category.php: call to core_renderer->paging_bar()

Stack trace:
line 1936 of \lib\outputcomponents.php: coding_exception thrown
line 2212 of \lib\outputrenderers.php: call to paging_bar->prepare()
line 70 of \lib\outputrenderers.php: call to core_renderer->render_paging_bar()
line 2201 of \lib\outputrenderers.php: call to renderer_base->render()
line 261 of \course\category.php: call to core_renderer->paging_bar()

Home / ⟩ Courses / ⟩ A / ⟩ Course 1

You should really redirect before you start page output
line 562 of \lib\outputrenderers.php: call to debugging()
line 2495 of \lib\weblib.php: call to core_renderer->redirect_message()
line 699 of \mod\scorm\locallib.php: call to redirect()
line 14 of \course\format\scorm\format.php: call to scorm_course_format_display()
line 240 of \course\view.php: call to require()

Issue Links

duplicates

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-26042 Coding error paging bar requires a per page value

  • Minor - Minor loss of function where workaround is possible
  • Open - The issue is open and ready for the assignee to start work on it.
is duplicated by

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-32672 Can't delete course; persistent error message.

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-33790 paging_bar requires a perpage value (when viewing list of courses)

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.
will be (partly) resolved by

Improvement - An enhancement to an existing Moodle feature. MDL-35138 Ensure default values for config settings are set

  • Major - Major loss of function, incorrect output
  • Open - The issue is open and ready for the assignee to start work on it.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Adrian Jones added a comment -

It does make adding and changing courses very trying and difficult .. double the time it should be taking .. so I don't think its that minor a problem.

Show
Adrian Jones added a comment - It does make adding and changing courses very trying and difficult .. double the time it should be taking .. so I don't think its that minor a problem.
Hide
Permalink
Michael de Raadt added a comment -

Hi, Adrian.

Thanks for reporting this. I wasn't able to replicate your problem.

Could you please provide replication instructions and perhaps some more details that would let us determine why this is happening in your case?

Show
Michael de Raadt added a comment - Hi, Adrian. Thanks for reporting this. I wasn't able to replicate your problem. Could you please provide replication instructions and perhaps some more details that would let us determine why this is happening in your case?
Hide
Permalink
Adrian Jones added a comment -

This was a totally clean new install .. on Server 2008 .. with MSSQL and Moodle the lastest build (two weeks ago now .. version number not with Me but I'll check later on).

The problem happened immediately I started to try adding courses...

So.. what details do you need ... ? I'd run the debugging expecting that would give the necessary data...

Show
Adrian Jones added a comment - This was a totally clean new install .. on Server 2008 .. with MSSQL and Moodle the lastest build (two weeks ago now .. version number not with Me but I'll check later on). The problem happened immediately I started to try adding courses... So.. what details do you need ... ? I'd run the debugging expecting that would give the necessary data...
Hide
Permalink
Michael de Raadt added a comment -

This issue has now been replicated and appears to be affecting current versions.

I'm still not able to replicate the problem, so it would be good if someone can help us determine why this is happening to some people and not others.

Show
Michael de Raadt added a comment - This issue has now been replicated and appears to be affecting current versions. I'm still not able to replicate the problem, so it would be good if someone can help us determine why this is happening to some people and not others.
Hide
Permalink
Michael de Raadt added a comment -

Comment from duplicate issue...

Lucas P added a comment - 17/Jun/12 3:56 AM

I've checked course/category.php where it says:
$perpage = optional_param('perpage', $CFG->coursesperpage, PARAM_INT); // how many per page

So, as I understand its an optional parameter, but then the Paging component is enforcing the parameter to have a value? Am I right?
...
That's right.

I hardcoded $perpage = 10 and now the page loads.

So what would be the right way to fix this issue?
...
Hmmm, found this post http://moodle.org/mod/forum/discuss.php?d=163731

Changing this through the web admin interface makes it work.

Show
Michael de Raadt added a comment - Comment from duplicate issue... Lucas P added a comment - 17/Jun/12 3:56 AM I've checked course/category.php where it says: $perpage = optional_param('perpage', $CFG->coursesperpage, PARAM_INT); // how many per page So, as I understand its an optional parameter, but then the Paging component is enforcing the parameter to have a value? Am I right? ... That's right. I hardcoded $perpage = 10 and now the page loads. So what would be the right way to fix this issue? ... Hmmm, found this post http://moodle.org/mod/forum/discuss.php?d=163731 Changing this through the web admin interface makes it work.
Hide
Permalink
Michael de Raadt added a comment -

Hi, Lucas.

I've added you as a watcher on this issue.

Show
Michael de Raadt added a comment - Hi, Lucas. I've added you as a watcher on this issue.
Hide
Permalink
Michael de Raadt added a comment - - edited

Perhaps the best solution is to add a default value in the optional_param() call...

$perpage = optional_param('perpage', $CFG>coursesperpage, PARAM_INT, 10);

No, that won't work.

Show
Michael de Raadt added a comment - - edited Perhaps the best solution is to add a default value in the optional_param() call... $perpage = optional_param('perpage', $CFG > coursesperpage, PARAM_INT, 10); No, that won't work.
Hide
Permalink
Rossiani Wijaya added a comment -

I can't replicate this issue. I tried by installing a fresh copy of moodle.

Show
Rossiani Wijaya added a comment - I can't replicate this issue. I tried by installing a fresh copy of moodle.
Hide
Permalink
Rossiani Wijaya added a comment -

Hi Adrian and Lucas,

If you still getting the error for this, could you try visiting the notification page (site admin > notifications).

This page will check for any missing config setting. For this case it will display the 'new settings - front page settings' page. (attach screenshot). On this page, you can change the value or keep the default value. please make sure to select the 'save changes' button.

Please let me know if the error still occurs after the trying the above steps.

Thank you.
Rosie

Show
Rossiani Wijaya added a comment - Hi Adrian and Lucas, If you still getting the error for this, could you try visiting the notification page (site admin > notifications). This page will check for any missing config setting. For this case it will display the 'new settings - front page settings' page. (attach screenshot). On this page, you can change the value or keep the default value. please make sure to select the 'save changes' button. Please let me know if the error still occurs after the trying the above steps. Thank you. Rosie
Hide
Permalink
Aparup Banerjee added a comment -

Hi,
i've just had a brief look at this and imo:
it seems for some reason (upgrade, install, setting issues?) that $CFG->coursesperpage isn't getting set. We need to investigate this root cause here. any hardcoding of default coursesperpage should be only in the setting for the config, if that default isn't working, lets find out why.

Show
Aparup Banerjee added a comment - Hi, i've just had a brief look at this and imo: it seems for some reason (upgrade, install, setting issues?) that $CFG->coursesperpage isn't getting set. We need to investigate this root cause here. any hardcoding of default coursesperpage should be only in the setting for the config, if that default isn't working, lets find out why.
Hide
Permalink
Michael de Raadt added a comment -

Hi, Apu.

It would be good to know the cause if why this config setting is not set, but I don't think it's something we should invest effort into.

The ultimate problem is that we cannot be assured that config settings are being set to default values, which happens for various reasons and is a fault in Moodle generally. I think that should be tackled as a separate issue and I will elaborate on that by writing such an issue.

For now, I believe we should not avoid putting in a simple fix that can be reversed later (following appropriate mechanisms) when a better fix is implemented.

Show
Michael de Raadt added a comment - Hi, Apu. It would be good to know the cause if why this config setting is not set, but I don't think it's something we should invest effort into. The ultimate problem is that we cannot be assured that config settings are being set to default values, which happens for various reasons and is a fault in Moodle generally. I think that should be tackled as a separate issue and I will elaborate on that by writing such an issue. For now, I believe we should not avoid putting in a simple fix that can be reversed later (following appropriate mechanisms) when a better fix is implemented.
Hide
Permalink
Michael de Raadt added a comment -

I think the reason why such missing settings are coming about is because of upgrades where the user fails to click save on new settings.

That results in errors that are hard to reproduce and are often closed as "cannot reproduce".

Show
Michael de Raadt added a comment - I think the reason why such missing settings are coming about is because of upgrades where the user fails to click save on new settings. That results in errors that are hard to reproduce and are often closed as "cannot reproduce".
Hide
Permalink
Rossiani Wijaya added a comment -

Created patch based on Michael's comment.

Sending for peer-review.

Show
Rossiani Wijaya added a comment - Created patch based on Michael's comment. Sending for peer-review.
Hide
Permalink
Ankit Agarwal added a comment -

Hi Rosie,
This looks good.
You may also want to comment on MDL-35138 to revert your patch once that is integrated.

Thanks

Show
Ankit Agarwal added a comment - Hi Rosie, This looks good. You may also want to comment on MDL-35138 to revert your patch once that is integrated. Thanks
Hide
Permalink
Rossiani Wijaya added a comment -

Thanks Ankit.

Submitting for integration review.

Show
Rossiani Wijaya added a comment - Thanks Ankit. Submitting for integration review.
Hide
Permalink
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

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
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

Show
Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
Hide
Permalink
Aparup Banerjee added a comment -

ah, thanks for creating MDL-35138 Michael

Show
Aparup Banerjee added a comment - ah, thanks for creating MDL-35138 Michael
Hide
Permalink
Aparup Banerjee added a comment -

this looks ok but i'm still unsure about the hardcoding (even if temporary) , i've pinged integrators for opinions incase theres better insight. i can't help thinking i'm not seeing something obvious..

so barring further revelations this looks good to go in , just awaiting any +1's

Show
Aparup Banerjee added a comment - this looks ok but i'm still unsure about the hardcoding (even if temporary) , i've pinged integrators for opinions incase theres better insight. i can't help thinking i'm not seeing something obvious.. so barring further revelations this looks good to go in , just awaiting any +1's
Hide
Permalink
Dan Poltawski added a comment -

My -1 to this solution, because:

1/ It is fine to depend on CFG values being set IMO, this is standard practice all around the code
2/ This issue only handles the case where the CFG value is not set.
3/ It doesn't handle the case for $CFG->coursesperpage set to 0.

It seems to me like the admin UI allows $CFG->coursesperpage to be set to 0, this is the real bug here. We need to prevent that from happening in the admin UI.

Show
Dan Poltawski added a comment - My -1 to this solution, because: 1/ It is fine to depend on CFG values being set IMO, this is standard practice all around the code 2/ This issue only handles the case where the CFG value is not set. 3/ It doesn't handle the case for $CFG->coursesperpage set to 0. It seems to me like the admin UI allows $CFG->coursesperpage to be set to 0, this is the real bug here. We need to prevent that from happening in the admin UI.
Hide
Permalink
Dan Poltawski added a comment -

Linking duplicate MDL-26042

Show
Dan Poltawski added a comment - Linking duplicate MDL-26042
Hide
Permalink
Aparup Banerjee added a comment -

yep ditto, reopening especially for the bit about '0'.

but really as mentioned earlier, we need to see why the default for the config setting hasn't been applied (upgrade?)

i think the best way to go about this bug first of all is to replicate this issue here in the same environment.

Show
Aparup Banerjee added a comment - yep ditto, reopening especially for the bit about '0'. but really as mentioned earlier, we need to see why the default for the config setting hasn't been applied (upgrade?) i think the best way to go about this bug first of all is to replicate this issue here in the same environment.
Hide
Permalink
CiBoT added a comment -

Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

Show
CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
Hide
Permalink
Rajesh Taneja added a comment - - edited

I agree with Dan about "perpage set to 0" case. But setting it to hard-coded value seems reasonable. If CFG is not set, then it should be set. As we do in lot of places like feedback, forum, assign, wiki etc. we should set this value as well. Normally, if user has not set any preference then we define default, this is kinda similar case where value is not set at system level.

In addition to that, we support $CFG->coursesperpage = 0 (Site administration - > Front page - > Front page settings), it might be nice to make sure it can't be set to 0. Also, this bug can be reproduced by setting $CFG- >coursesperpage = 0 on Front page settings page, which can be added in test instructions.

Show
Rajesh Taneja added a comment - - edited I agree with Dan about "perpage set to 0" case. But setting it to hard-coded value seems reasonable. If CFG is not set, then it should be set. As we do in lot of places like feedback, forum, assign, wiki etc. we should set this value as well. Normally, if user has not set any preference then we define default, this is kinda similar case where value is not set at system level. In addition to that, we support $CFG->coursesperpage = 0 (Site administration - > Front page - > Front page settings), it might be nice to make sure it can't be set to 0. Also, this bug can be reproduced by setting $CFG- >coursesperpage = 0 on Front page settings page, which can be added in test instructions.
Hide
Permalink
Michael de Raadt added a comment -

I'm bringing this into the current sprint so we don't loose sight of it.

In relation to relying on the configuration setting, apparently we cannot. This issue addresses a specific problem and should be resolved within its own scope. We have created MDL-35138 to deal with the greater problem.

Dan raised a duplicate issue. It might be worth looking for other places where this perpage value is being used without a check. That is still within the scope of this current issue.

Show
Michael de Raadt added a comment - I'm bringing this into the current sprint so we don't loose sight of it. In relation to relying on the configuration setting, apparently we cannot. This issue addresses a specific problem and should be resolved within its own scope. We have created MDL-35138 to deal with the greater problem. Dan raised a duplicate issue. It might be worth looking for other places where this perpage value is being used without a check. That is still within the scope of this current issue.
Hide
Permalink
Rossiani Wijaya added a comment -

Hi Guys,

Thank you for the feedback.

I updated the patch to use !empty instead of isset().

Re-submitting for integration review.

Show
Rossiani Wijaya added a comment - Hi Guys, Thank you for the feedback. I updated the patch to use !empty instead of isset(). Re-submitting for integration review.
Hide
Permalink
Dan Poltawski added a comment -

The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.
This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.
This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P
Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

Show
Dan Poltawski added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
Hide
Permalink
Aparup Banerjee 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

Show
Aparup Banerjee 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
Hide
Permalink
Dan Poltawski 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

Show
Dan Poltawski 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
Hide
Permalink
Aparup Banerjee added a comment -

ok in the interests of stability for 2.4 release (work is done), ongoing discussion in wider scope (MDL-35138) and further evidence from triaging suggesting this might actually be a broader problem (MDL-36065) ;

I've integrated this into 22, 23 and master(2.4dev).

ps:noting some review commits could've been squashed but no time.

Show
Aparup Banerjee added a comment - ok in the interests of stability for 2.4 release (work is done), ongoing discussion in wider scope ( MDL-35138 ) and further evidence from triaging suggesting this might actually be a broader problem ( MDL-36065 ) ; I've integrated this into 22, 23 and master(2.4dev). ps:noting some review commits could've been squashed but no time.
Hide
Permalink
Aparup Banerjee added a comment - - edited

works, passing. (not specific to MSSQL btw)

Show
Aparup Banerjee added a comment - - edited works, passing. (not specific to MSSQL btw)
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

(not really)

Closing, thanks!

Show
Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: