I just got badly bitten by the backup/restore code creating a new course from a backup with the same idnumber as the old course.
I see that shortname and fullname gets _n on the end for the course that gets created - should this happen with idnumber too?
I'm happy to add this & commit it to cvs - imo it should go into both stable and head.
Referring to this thread:
http://moodle.org/mod/forum/discuss.php?d=15485
course.idnumber should be unique but I don't believe that got done. Maybe I could do this at the same time?
Edit: since I had to do this for our deployment, here's the patch:
http://lists.eduforge.org/cgi-bin/archzoom.cgi/arch-eduforge@catalyst.net.nz--2004/moodle--eduforge--1.3.3--patch-445/backup/restorelib.php.diff?diff
In reply to Penny Leach
Re: Restoring into an empty course, idnumber field
Hi Penny,
I agree 100%!! I forgot it in such discussion. Thanks!
But your patch seems to be wrong for me! You've assumed that EVERY course has an idnumber (taking out the get_record() by fullname that I think it's the correct one).
Perhaps a correct approach could be to leave all that code unmodified and just after the do...while loop, check if the idnumber exists and, if yes, leave it blank and show some warning. I think this is better mainly because, as idnumber can be used to relation Moodle with external systems, can be quite dangerous to assume an automatic new idnumber. Resetting it and informing about that could be better, isn't it?
Ciao
I agree 100%!! I forgot it in such discussion. Thanks!
But your patch seems to be wrong for me! You've assumed that EVERY course has an idnumber (taking out the get_record() by fullname that I think it's the correct one).
Perhaps a correct approach could be to leave all that code unmodified and just after the do...while loop, check if the idnumber exists and, if yes, leave it blank and show some warning. I think this is better mainly because, as idnumber can be used to relation Moodle with external systems, can be quite dangerous to assume an automatic new idnumber. Resetting it and informing about that could be better, isn't it?
Ciao
In reply to Eloy Lafuente (stronk7)
Re: Restoring into an empty course, idnumber field
by Penny Leach -
Of course, you're right about my patch assuming every course has an idnumber, that was my bad - in our particular deployment this is the case as every course we have gets populated from an sms unless it's a test course pretty much.
But: the get_record by fullname thing - if I've understood this correctly you're only looping while there's no fullname collision - but we need to also check there is no idnumber collision.
But you're right - maybe the idnumber should be set to blank and a notice shown instead. If the new course needs an idnumber (which is probably actually unlikely) it can be entered manually.
I'll make a new patch for this today.
But: the get_record by fullname thing - if I've understood this correctly you're only looping while there's no fullname collision - but we need to also check there is no idnumber collision.
But you're right - maybe the idnumber should be set to blank and a notice shown instead. If the new course needs an idnumber (which is probably actually unlikely) it can be entered manually.
I'll make a new patch for this today.
More thoughts: I'm starting to think that when restoring into a blank course we shouldn't be populating idnumber at all.
Even if we add _n on the end if there is no collision it could screw up a script that tries to parse the idnumbers and guess stuff from the first n characters or something.
I'm really starting to think we should leave it blank and it can be filled in later if necessary.
Even if we add _n on the end if there is no collision it could screw up a script that tries to parse the idnumbers and guess stuff from the first n characters or something.
I'm really starting to think we should leave it blank and it can be filled in later if necessary.
new patch:
http://lists.eduforge.org/cgi-bin/archzoom.cgi/arch-eduforge@catalyst.net.nz--2004/moodle--eduforge--1.3.3--patch-447?log
this time I'm not using idnumber at all in the new course. If idnumber existed in the old course, we alert the user that we're not using it anymore to avoid collisions. If there was no original idnumber, it's exactly the same as normal.
http://lists.eduforge.org/cgi-bin/archzoom.cgi/arch-eduforge@catalyst.net.nz--2004/moodle--eduforge--1.3.3--patch-447?log
this time I'm not using idnumber at all in the new course. If idnumber existed in the old course, we alert the user that we're not using it anymore to avoid collisions. If there was no original idnumber, it's exactly the same as normal.
In reply to Penny Leach
Re: Restoring into an empty course, idnumber field
Sounds well for me. Feel free to apply it (if nobody has anything against it).
Ciao
Ciao
In reply to Eloy Lafuente (stronk7)
Re: Restoring into an empty course, idnumber field
by Penny Leach -
Just committed now.
Cheers, Eloy
Cheers, Eloy
Ciao Penny and ciao Eloy (and congratulation for the new Lucas in your life (as your skype description says)).
I am getting lost.
I am here redirected by Petr from MDL-12126.
I may agree with you, Penny, about the need of not adding a _n to course ID but... why do you agree to miss the alert too?
Without alert a not experienced user may lost hours before understanding that course ID was not restored.
Can you explain me better the sustaining idea?
Thank you in advance.
I am getting lost.
I am here redirected by Petr from MDL-12126.
I may agree with you, Penny, about the need of not adding a _n to course ID but... why do you agree to miss the alert too?
Without alert a not experienced user may lost hours before understanding that course ID was not restored.
Can you explain me better the sustaining idea?
Thank you in advance.
I'm one of these inexperienced users you're referring to. I'm try to transfer a course between systems, (1.9.1 to 1.9.1). The content doesn't display correctly, complaining about a bad course id. How do I set it if it's wrong everywhere?