|
[
Permalink
| « Hide
]
Michael Blake added a comment - 01/Dec/08 11:33 AM
Dongsheng, Can you have a look at this issue? It's causing problems for a client of a Moodle Partner. Thanks.
A follow up note on this, in case it helps you track the issue down.
Not that it makes much sense in the process, but I did test the steps using only the admin role. If I set the desired start date in the restore (this role has that option) the resources get the correct corresponding dates. If I then hit reset and set a new course start date, the resources get the "bad" dates. It seems something wrong with course restoring, after restoring the course, I checked the available date of assignment, the due date is not correct.
Start to working on a fix. when we assign a variable to another one, the timestamps string became a int value. Seems this is a bug of php.
This patch could fix this problem, please review. I've tested the patch, and it works, thanks Dongsheng!
One thing I would like to point out is that there are other timestamps in restorelib that could still be affected. They would be "timemodified" of the assignment object, and the "timemodified", "timecreated", and "timemarked" fields of the submission object. Do these need to be fixed as well? For our purposes, the patch works beautifully, but I thought I should mention the other times in case they are a problem for someone in the future. Thanks again, Hi, Kris, I noticed these timestamps as well, but they are correct, so I didn't modify them, it is quite strange.
Please review, the fix is ugly, any suggestion is welcome.
Thanks Hi Dongsheng,
really I don't understand why the patch applied makes things to work. It seems that, simply, you are ignoring the changes in dates performed by restore_log_date_changes(), so dates aren't changed at all. I'm not sure if that's correct behaviour (prevent dates to roll). IMO we should analyse why those dates are being incorrectly rolled. Or am I missing anything? I reopen this for further discussion. Ciao Hi, Eloy, I didn't notice restore_log_date_changes() there, I found a varible named course_startdateoffset which cause this issue.
Hi, Kris, I believe my last patch is incorrect, which ignored the course_startdateoffset, can you try the new patch:
http://cvs.moodle.org/moodle/backup/restore.php?r1=1.44.2.3&r2=1.44.2.4 http://cvs.moodle.org/moodle/mod/assignment/restorelib.php?r1=1.33.2.1&r2=1.33.2.2 Sorry for inconvenience. Eloy, can you confirm my changes, I committed it because last patch is fully wrong. Mark as fixed, need review and testing.
Thanks Sorry for the late reply, but we have tested the patch from our end and it is working as expected. Thanks again for your efforts!
The patch to restore.php causes a problem for me when I restore to a new course.
To reproduce: This patch will allow you to use the restore forms start date to offset the course and it's activities as expected.
If backup A is created from a course then the restore function is used from course B: If a new course is created from backup A it will use the backups start date and the date entered in the form to set the offset for activities or events. If the backup is restoring to the current course and deleting it first then it will use the backups start date and course B's start date. If the backup is restoring to an existing course C and deleting it first then it will use the backup A's start date and C's start date. If the backup is restoring to the current course and adding data to it then for all the added data the offset will be from backup A and the date submitted in the form. If the backup is restoring to an existing course and adding data to it then for all the added data and the offset will be from backup A and the date submitted in the form. This is how most people probably assumed the backup system worked. and by this patch I mean restore.patch ( http://tracker.moodle.org/secure/attachment/17910/restore.patch
I went ahead and created a new issue related to the bug Paul reported MDL-19859
Uh, oh,
I was working with another restore bug fixes ( IMO restore.php isn't the place to perform settings adjustments at all, those sort of things should happen in restore_check.html (where everything is validated before launching restore execution). More yet, this patch is mixing various start dates badly, as Paul commented above, because the course from which the backup file is being restored (id) cannot influence the roll dates thing, it's the target course the only that can have something to do with rolling dates. So I'm going to revert this change in restore.php, trying to apply it properly in restore_check.php, informing about the roll that will be finally happening, trying to cover as many cases/combinations as possible. Ciao Done,
I've reverted the fix applied here to restore.php. Also, as admin, I've performed various tests rolling dates both to new and existing course and they are applied properly (where "properly" means, always performing the roll over the start date existing in the backup file and ignoring the one in the target course). Discussion about that "properly" bit and how the rolling of dates should work (taking into consideration the target course startdate) will continue in MDL-19859. Ciao |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||