Moodle
  1. Moodle
  2. MDL-12922

Course restores for non-admin users appear ~7 years early

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.3, 1.9.3
    • Fix Version/s: 1.8.8, 1.9.4
    • Component/s: Backup
    • Labels:
      None
    • Environment:
      1.8.3+ (2007021534)
    • Database:
      MySQL
    • Affected Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE

      Description

      There is a problem in the restore component of backup that causes the dates on forum posts to be wildly off (7+ years) when a course with forums is restored by a teacher. If a course is restored by an admin, the forum post dates come in correctly.

      What's happening is that /backup/restore_form.html has logic that checks to see if a course has a start date, and if it does, it gives the user the ability to modify that start date.

      However the problem is that this start-date-checking logic is tied to the "course creator" capability – if you don't have the coursecreator capability (and teachers don't) then it doesn't do the check, and assigns the start date to 0, which triggers some date offset calculations, which results in the wonky dates.

      I've created a patch that fixes the problem by moving this logic out of the "coursecreator" portion of "restore_form.html". As a result, teachers will now see the "course startdate" field as well, and everything works as it should. The patch is attached to this bug report.

        Gliffy Diagrams

        1. 192_restore_form_patch.txt
          2 kB
          Kenneth Newquist
        2. backup_restore_fix_1.9.diff
          2 kB
          matt greenwolfe
        3. backup_restore_fix.diff
          0.8 kB
          Kenneth Newquist

          Issue Links

            Activity

            Hide
            Yolanda Ordoñez Rufat added a comment -

            There's also a problem with the post dates in the restore if you have the "course creator" capability. So moving the logic out of the "coursecreator" portion of "restore_form.html" doesn't work, sorry Kenneth.
            I tried a restore as an admin and I change the start date of the course during restore and I get wrong forum post dates (they are set in the future).

            I test it on a 1.8.3 moodle with a 8.2.4 postgres installed in a SUSE Linux Enterprise Server 10 SP1.

            Show
            Yolanda Ordoñez Rufat added a comment - There's also a problem with the post dates in the restore if you have the "course creator" capability. So moving the logic out of the "coursecreator" portion of "restore_form.html" doesn't work, sorry Kenneth. I tried a restore as an admin and I change the start date of the course during restore and I get wrong forum post dates (they are set in the future). I test it on a 1.8.3 moodle with a 8.2.4 postgres installed in a SUSE Linux Enterprise Server 10 SP1.
            Hide
            David Kelly added a comment -

            This is still a bug in 1.9, at least as of build 20080307.

            Show
            David Kelly added a comment - This is still a bug in 1.9, at least as of build 20080307.
            Hide
            Kenneth Newquist added a comment -

            I haven't tried it with the course creator role in Moodle 1.8.x; we don't make use of that particular one. I'll try and spend some time debugging this later this week or early next. I haven't tried it in Moodle 1.9 yet.

            Show
            Kenneth Newquist added a comment - I haven't tried it with the course creator role in Moodle 1.8.x; we don't make use of that particular one. I'll try and spend some time debugging this later this week or early next. I haven't tried it in Moodle 1.9 yet.
            Hide
            matt greenwolfe added a comment -

            Here is a diff file of the changes that worked for me, at least so far, in 1.9.
            Following the diff file for 1.8.3, I edited /backup/restore_form.html by moving several lines out of the if statement. The date line shows up for teachers and seems to work correctly so long as the specified date is greater than or equal to the original course start date.

            Show
            matt greenwolfe added a comment - Here is a diff file of the changes that worked for me, at least so far, in 1.9. Following the diff file for 1.8.3, I edited /backup/restore_form.html by moving several lines out of the if statement. The date line shows up for teachers and seems to work correctly so long as the specified date is greater than or equal to the original course start date.
            Hide
            Kenneth Newquist added a comment -

            I can confirm the problem still exists for teachers in Moodle 1.9.2. Matt's patch failed for me so I've generated a new one based on Moodle 1.9.2 (Build: 20080711).

            I can not recreate the problem with the Course Creator role; when logged in as a course creator I was able to see the start data form (which is the way the original code was supposed to work, as that functionality was reserved for course creators, even though non-course creating teachers needed it as well).

            Show
            Kenneth Newquist added a comment - I can confirm the problem still exists for teachers in Moodle 1.9.2. Matt's patch failed for me so I've generated a new one based on Moodle 1.9.2 (Build: 20080711). I can not recreate the problem with the Course Creator role; when logged in as a course creator I was able to see the start data form (which is the way the original code was supposed to work, as that functionality was reserved for course creators, even though non-course creating teachers needed it as well).
            Hide
            Rahim Virani added a comment -

            I think the patch is broken ". Text File 192_restore_form_patch.txt (2 kb)"

            We tried the ". Text File 192_restore_form_patch.txt (2 kb)" from a 1.9.2 intallation, the patch while it did take into account the current date, it did not honor the settings that we set for course start date, if we set the start-date to May 1st, the course startdate after the restore is still July 20th.

            Show
            Rahim Virani added a comment - I think the patch is broken ". Text File 192_restore_form_patch.txt (2 kb)" We tried the ". Text File 192_restore_form_patch.txt (2 kb)" from a 1.9.2 intallation, the patch while it did take into account the current date, it did not honor the settings that we set for course start date, if we set the start-date to May 1st, the course startdate after the restore is still July 20th.
            Hide
            Kenneth Newquist added a comment -

            I'll re-check the patch later today.

            Show
            Kenneth Newquist added a comment - I'll re-check the patch later today.
            Hide
            Rahim Virani added a comment -

            EXCUSE THE CAPS, BUT I HOPE SOMEONE READS THIS THAT CAN PUT THE PATCH INTO THE DISTRIBUTION WE HAVE NOW A PROBLEM 171 ASSIGNMENTS IN OUR MOODLE INSTALL HAVE SILLY DUE DATES!!!

            The patch has been verified and it works in 1.9.2, We had patched our 1.8.3 distribution but when we did the security upgrade to 1.8.6 we must have lost the patch, it wasnt included in the new distribution so if someone who has access to the main code repository might put this patch in (sorry I dont know how) then this might be avoided.

            We have successfully used this patch in 1.8.3 and in 1.9.2 I really dont want to get bit by this again! - Thanks.

            Show
            Rahim Virani added a comment - EXCUSE THE CAPS, BUT I HOPE SOMEONE READS THIS THAT CAN PUT THE PATCH INTO THE DISTRIBUTION WE HAVE NOW A PROBLEM 171 ASSIGNMENTS IN OUR MOODLE INSTALL HAVE SILLY DUE DATES!!! The patch has been verified and it works in 1.9.2, We had patched our 1.8.3 distribution but when we did the security upgrade to 1.8.6 we must have lost the patch, it wasnt included in the new distribution so if someone who has access to the main code repository might put this patch in (sorry I dont know how) then this might be avoided. We have successfully used this patch in 1.8.3 and in 1.9.2 I really dont want to get bit by this again! - Thanks.
            Hide
            Rahim Virani added a comment -

            So, we checked, this still hasnt made it into Moodle 1.9.3, I'm glad I did my due diligence with this one.

            Martin, what can I do to resolve this?

            Please advise.

            Show
            Rahim Virani added a comment - So, we checked, this still hasnt made it into Moodle 1.9.3, I'm glad I did my due diligence with this one. Martin, what can I do to resolve this? Please advise.
            Hide
            Martin Dougiamas added a comment -

            Eloy can you look at this please?

            Show
            Martin Dougiamas added a comment - Eloy can you look at this please?
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Instead of using the patches above... I've continued with current approach, i.e. non-course-creators cannot roll dates at all.

            I've committed fix for 18_STABLE, 19_STABLE and HEAD. Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Instead of using the patches above... I've continued with current approach, i.e. non-course-creators cannot roll dates at all. I've committed fix for 18_STABLE, 19_STABLE and HEAD. Ciao
            Hide
            Martin Dougiamas added a comment -

            Great, thanks Eloy!

            Show
            Martin Dougiamas added a comment - Great, thanks Eloy!
            Hide
            Tim Hunt added a comment -

            Looks good to me. Thanks Eloy, and thanks for the clear bug report, sorry it took so long to fix.

            Show
            Tim Hunt added a comment - Looks good to me. Thanks Eloy, and thanks for the clear bug report, sorry it took so long to fix.
            Hide
            Mary Parke added a comment -

            Hi Eloy,

            Thank you for this fix. However, I have to disagree with the current practice of making this so only "course creators" are allowed to roll the dates.

            If you follow MDL-9367 (and other related trackers), you'll see that there is an overwhelming need by the community for faculty to be allowed to roll the dates in a course. If it is assumed only the admin is doing this, then an institution has to fund a whole team of folks with admin privileges (or "course creator") to do nothing but course restores from semester to semester. If instead, faculty (teacher role) are allowed to do restore and import with setting the course start date, then the burden is off the team and in the hands of the ones for which this function most effects: the instructors. Otherwise, faculty have to manually edit the dates on every activity every semester, thus defeating the purpose of this "roll forward" feature.

            Logically, as a systems administrator, I would WANT my faculty to be able to set their own course start date (especially as courses start on differing dates throughout the semester), but I wouldn't want my faculty to be assigned to the course creator role in order to make this happen - because I wouldn't want them to have the ability to create whatever courses they wanted on the fly. We only host courses that have gone through a process (such as curriculum committee, dept approval, etc.).

            If this fix could also be applied to at minimum the teacher role, this would greatly assist many institutions in the management of course rollover from semester to semester (term to term).

            Thank you for listening,

            Mary

            Show
            Mary Parke added a comment - Hi Eloy, Thank you for this fix. However, I have to disagree with the current practice of making this so only "course creators" are allowed to roll the dates. If you follow MDL-9367 (and other related trackers), you'll see that there is an overwhelming need by the community for faculty to be allowed to roll the dates in a course. If it is assumed only the admin is doing this, then an institution has to fund a whole team of folks with admin privileges (or "course creator") to do nothing but course restores from semester to semester. If instead, faculty (teacher role) are allowed to do restore and import with setting the course start date, then the burden is off the team and in the hands of the ones for which this function most effects: the instructors. Otherwise, faculty have to manually edit the dates on every activity every semester, thus defeating the purpose of this "roll forward" feature. Logically, as a systems administrator, I would WANT my faculty to be able to set their own course start date (especially as courses start on differing dates throughout the semester), but I wouldn't want my faculty to be assigned to the course creator role in order to make this happen - because I wouldn't want them to have the ability to create whatever courses they wanted on the fly. We only host courses that have gone through a process (such as curriculum committee, dept approval, etc.). If this fix could also be applied to at minimum the teacher role, this would greatly assist many institutions in the management of course rollover from semester to semester (term to term). Thank you for listening, Mary
            Hide
            Rahim Virani added a comment -

            I do agree with Mary,

            If we were to give all the instructors Course-creator privs this would definatly be problematic. and well as Martin L put it "unsustainable"

            We tend to have faculty..... yeah.

            In short I do agree with Mary, I also do know of many other institutions that cannot afford the resources hand-holding such as ours.

            Kind Regards,

            Show
            Rahim Virani added a comment - I do agree with Mary, If we were to give all the instructors Course-creator privs this would definatly be problematic. and well as Martin L put it "unsustainable" We tend to have faculty..... yeah. In short I do agree with Mary, I also do know of many other institutions that cannot afford the resources hand-holding such as ours. Kind Regards,
            Hide
            Barron Koralesky added a comment -

            I agree with Mary and Rahim, if we are to make course restores a valuable self-service function for our instructors, the teacher role needs to be able to restore courses without getting wonky dates. Thank you for your help and work on this.

            Show
            Barron Koralesky added a comment - I agree with Mary and Rahim, if we are to make course restores a valuable self-service function for our instructors, the teacher role needs to be able to restore courses without getting wonky dates. Thank you for your help and work on this.
            Hide
            Marty Gilbert added a comment -

            I second Barron, Mary, and Rahim. I'm not about to give my faculty course-creator roles; that could be disasterous.

            Show
            Marty Gilbert added a comment - I second Barron, Mary, and Rahim. I'm not about to give my faculty course-creator roles; that could be disasterous.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            So, one capability like:

            'moodle/restore:rolldates'

            enabled by default to admins and course creators (to have backward compatibility) but able to be assigned to other roles... sounds like a good solution to everybody here?

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - So, one capability like: 'moodle/restore:rolldates' enabled by default to admins and course creators (to have backward compatibility) but able to be assigned to other roles... sounds like a good solution to everybody here? Ciao
            Hide
            Eloy Lafuente (stronk7) added a comment -

            and that capability should be able to work at course level or only at category level? I ask this because sometimes, on restore we don't know the target course until later in the process (for example, when creating a new course) or when restoring to existing, deleting first, the "teacher" haven't still that capability allowed.

            So, perhaps it should be a "category-based" capability? and not a course one?

            Show
            Eloy Lafuente (stronk7) added a comment - and that capability should be able to work at course level or only at category level? I ask this because sometimes, on restore we don't know the target course until later in the process (for example, when creating a new course) or when restoring to existing, deleting first, the "teacher" haven't still that capability allowed. So, perhaps it should be a "category-based" capability? and not a course one?
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Done. New capability 'moodle/restore:rolldates' is available and allowed to admins and course creators by default (old behaviour). Can be allowed to other roles (at any level) in order to permit them to roll dates on restore.

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Done. New capability 'moodle/restore:rolldates' is available and allowed to admins and course creators by default (old behaviour). Can be allowed to other roles (at any level) in order to permit them to roll dates on restore. Ciao
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Please, test if and report any problem here ASAP, as this new capability will be available in tomorrow's weekly build and in next stable release (Moodle 1.9.6). TIA and re-ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Please, test if and report any problem here ASAP, as this new capability will be available in tomorrow's weekly build and in next stable release (Moodle 1.9.6). TIA and re-ciao

              People

              • Votes:
                19 Vote for this issue
                Watchers:
                9 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: