please test/review

just some quick comments (it has been impossible for me to examine the patch deeply).

1) Grades should be restored a bit below, as talked, after all the modules and instances have been properly set, i.e. after the restore_check_instances() block of code.

2) There are some "grrr..." debugs here and there.

3) The double loops, get_records_select() + foreach should be easily transformed into get_recordset_XXX() loops, saving a bunch of memory, specially for user information records.

4) It seems you are using the gradebook API to build grades and so on (instead or raw DB access). Perfect!

5) Also you've differentiated old gradebooks and new gradebooks from the parser. Good idea!

6) I've seen you've hacked the backup_getid() function to avoid current notices about the "needed" thing. Hehe, I've it in my TODO list. And really think that hack shouldn't be there. It's relate with how that info is added to the backup_ids table (unserialised) and I wan't to fix it on insertion. I think it has one TODO defined somewhere in the file and I'll try to insert it properly serialised.

7) Good idea the conditional addslashes, in backup_todb()

8) Finally, one comment about big parts in the backup and restore. Both must include some output to the browser from time to time (to avoid browser timeouts). I guess all the critical parts of the gradebook will have such pieces of code added (haven't really checked that). For now I'd use the current strategy consisting in something like:

$counter++;

//Do some output
if ($counter % XXXXXX == 0) {
if (!defined('RESTORE_SILENTLY')) {
echo ".";
if ($counter % 20*XXXXX == 0) { echo "<br />"; }
}
backup_flush(300);
}

it's only a matter of adjust XXXXX above to a number granting one dot to be sent to output each, say, 1-2 secs (aprox).

Edited: I missed. Of course those "output blocks" should be out for 2.0 with a central function replacing them, but I'd keep current behaviour for stable.

And that's all I've been able to review, hope it helps. Ciao

going to test this a bit more today and commit, then continue with the rest of backup/restore issues

thanks!

• TODO: Change this call inserting to a standard backup_putid() call
• TODO: Also analyse it the "needed" info is really needed for anything. Drop if not.

That I detected when cleaning up a bit users backup http://tracker.moodle.org/browse/MDL-10721

AFAIK, roles assignments and overrides are properly working, I remember I did some tests when fixing that bug and everything worked (with the TODO notes above).

Just FYI, ciao