Unnecessary PHP Notices in lib/weblib.php and type/multichoice/questiontype.php

Noticed these in MOODLE_19_STABLE (2008-08-08) as well - attached 1.9 patch

...and for CVSHEAD - smaller patch, mod/quiz/attempts.php has been reworked.

I should note that I don't have CVS write access, and don't seem to be able to assign it to anyone

Today is testing day (http://docs.moodle.org/en/Development:Weekly_Code_Review) so I'll check these in tomorrow.

Note, you should really only select the latest release from each branch under Affects version/s, although that is not a big deal.

Actually, I am not convinced by your patch. For example, if you call print_user_picture, then $user->picture must be set, otherwise it is an error in the caller. Just putting in the !empty hides a real bug, rather than fixing it.

Similarly, $cmoptions->shuffleanswers and $question->options->shuffleanswers should both be set before calling whatever function that is in the multichoice question type.

So you need to find what people are doing when those errors occur, then we can fix the callers.

Hi Tim,

Not necessarily.

The problem arose from the difference in database schema (on postgresql) between a fresh install of a codebase and the same codebase using a database upgraded from an earlier (1.5) moodle.

In a fresh install, the picture field in mdl_user (and other problem fields) are defined as integer not null default 0, whereas in the upgraded database (which has gone through the various upgrade functions) they do not have the not null or default set.

So you're right in a way, the code is expecting these fields to be not null and therefore to be always set (with a default zero integer value). But in real life people are running upgraded databases where this is not the case, so we need to either make sure that the Moodle code is more robust by not assuming things are necessarily set (which the !empty() check handles) which is much easier, or by going through a complete database regression analysis on all supported database platforms and adding tidy up functions to the upgrade.php of lib as well as all modules and blocks, as well as checking that the install.xml structure is actually reflecting the right choices of not null and default values, which would be nice, but also time consuming, not particularly maintainable in the long term (imo) and somewhat error prone.

Yes, but Moodle is not really un-robust in this situation, it only outputs notices if you have debugging turned up to the hightest level. Moodle still functions correctly. In my opinion those notices are right, they are telling you something minor is not right with your Moodle install. That is exactly what the situation is.

If you go to Admin -> Experimental -> XMLDB editor, you will see some options [Check Indexes] [Check Bigints] at the top of the screen. I think that the correct way forwards here is to make a new option [Check defaults] that checks for the kinds of inconsistencies you are seeing with old sites that have been upgraded.

OK, well I checked this in, just to get it off my open bug list.

By the way, why don't you have CVS commit access?

This fix feels a little hacky but its evidently been running fine for a year now. A more bulletproof solution for ensuring an upgraded schema exactly matches a newly installed schema would be good but is probably too big a task to be done in 1.9. Closing.