Issue Details (XML | Word | Printable)

Key: MDL-17469
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Critical Critical
Assignee: Dongsheng Cai
Reporter: Kris Stokking
Votes: 1
Watchers: 6
Operations

Add/Edit UI Mockup to this issue
If you were logged in you would be able to see more operations.
Moodle

Problem with Course Reset - Assignment Dates

Created: 30/Nov/08 12:59 PM   Updated: 06/Oct/09 12:43 AM
Component/s: Backup, Course
Affects Version/s: 1.9.3
Fix Version/s: 1.9.4

File Attachments: 1. Text File MDL-17469.patch (1 kB)
2. Text File restore.patch (1 kB)

Issue Links:
Relates
 

Database: MySQL
Participants: Andy Braden, Dongsheng Cai, Eloy Lafuente (stronk7), Kris Stokking, Martin Dougiamas, Michael Blake, Olav Jordan and Paul Grzesina
Security Level: None
QA Assignee: Nicolas Connault
Resolved date: 16/Dec/08
Affected Branches: MOODLE_19_STABLE
Fixed Branches: MOODLE_19_STABLE


 Description  « Hide
We have found a reproducible bug within the Course Restore and Reset functionality. The bug causes any assignments associated with the Course to have their Due Date set oddly. For example, if Course A starts on 11/1/2008 and is restored into Course B, then Course B is reset to start on 1/1/2009, Assignment A which originally started on 12/23/2008 would now start at 3/12/2000 under Course B.

Here are the steps to reproduce:

1. Login as an administrator
2. Create a sample course (Course A) that starts on 11/1/2008, with one assignment that starts on 12/23/2008.
3. Take a backup of the Course A in step 2 (backup.zip).
4. Create a new course (Course B) that starts on 11/1/2008. Assign an instructor to the course.
5. Login as the instructor in step 4, and restore backup.zip to Course B.
6. Reset Course B to start on 1/1/2009
7. Look at the details of the lone assignment. It should have a due date of 2/23/2008 (or around that date). Instead, it has a due date of something far in the past, such as 3/12/2000.

Please let us know if there is a workaround, as this is holding up teachers from completely restoring a course. Thanks!



 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
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.

Andy Braden added a comment - 02/Dec/08 01:44 AM
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.

Dongsheng Cai added a comment - 03/Dec/08 10:33 AM
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.


Martin Dougiamas added a comment - 03/Dec/08 01:34 PM
Thanks Dongsheng!

Dongsheng Cai added a comment - 03/Dec/08 03:25 PM
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.


Kris Stokking added a comment - 03/Dec/08 11:46 PM
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,
Kris


Dongsheng Cai added a comment - 04/Dec/08 04:46 PM
Hi, Kris, I noticed these timestamps as well, but they are correct, so I didn't modify them, it is quite strange.

Dongsheng Cai added a comment - 04/Dec/08 05:11 PM
Please review, the fix is ugly, any suggestion is welcome.
Thanks

Eloy Lafuente (stronk7) added a comment - 08/Dec/08 04:18 AM
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


Dongsheng Cai added a comment - 09/Dec/08 01:52 PM
Hi, Eloy, I didn't notice restore_log_date_changes() there, I found a varible named course_startdateoffset which cause this issue.

Dongsheng Cai added a comment - 11/Dec/08 01:44 PM
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.


Dongsheng Cai added a comment - 16/Dec/08 02:07 PM
Mark as fixed, need review and testing.
Thanks

Kris Stokking added a comment - 16/Dec/08 10:47 PM
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!

Paul Grzesina added a comment - 12/May/09 04:01 AM
The patch to restore.php causes a problem for me when I restore to a new course.

To reproduce:
1. Login as admin
2. Create a sample course (course A) that starts on May 1, 2009 with one assignment due on May 7, 2009.
3. Take a backup of the course and download the backup file.
4. Upload the backup file to another course (course B) with a different start date.
5. Restore the backup to a new course using the default start date (May 1, 2009).
6. Look at the details of the assignment in the new course. The due date should still be May 7, 2009 but will instead be something different. (It will be offset relative to course B instead of using the date provided in the restore.)


Olav Jordan added a comment - 18/Jul/09 04:03 AM
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.


Olav Jordan added a comment - 18/Jul/09 04:10 AM
and by this patch I mean restore.patch ( http://tracker.moodle.org/secure/attachment/17910/restore.patch ) please if anyone is still watching this issue I would like some review and testing done although I have done quite a bit of testing and have not found any errors/side effects

Olav Jordan added a comment - 18/Jul/09 04:21 AM
I went ahead and created a new issue related to the bug Paul reported MDL-19859

Eloy Lafuente (stronk7) added a comment - 05/Oct/09 11:37 PM
Uh, oh,

I was working with another restore bug fixes (MDL-18469) when finally arrived here, because my checksums were detecting all the time that restore information has been changed somewhere. And the change applied here is the responsible!

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


Eloy Lafuente (stronk7) added a comment - 06/Oct/09 12:37 AM
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