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.

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?

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...

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.

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.

Hi, Lucas.

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

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.

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

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

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.

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.

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".

Created patch based on Michael's comment.

Sending for peer-review.

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

Thanks

Thanks Ankit.

Submitting for integration review.

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

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

ah, thanks for creating MDL-35138 Michael

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

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.

Linking duplicate MDL-26042

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.

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

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.

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.

Hi Guys,

Thank you for the feedback.

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

Re-submitting for integration review.

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

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

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

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.

works, passing. (not specific to MSSQL btw)

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

(not really)

Closing, thanks!