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
    • Rank:
      30744

      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.

      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: