Details

    • Testing Instructions:
      Hide
      1. Get ready, this can take a while
      2. Create a complete range of different Calender events including atleast
        *site events (sitepages->calendar->new event->type>site)
        *site events that are repeated (use the repeat setting while creating a calendar event)
        *user events (sitepages->calendar->new event->type->user)
        *group events
        *course events (gotto a course-> add calendar block-> click on month name->new event)
        *Course events with images in description
        *course events that are repeated (use the repeat setting while creating a calendar event)
        *activity events. (Activity events are automatically created when you create a activity with due dates)
      3. Backup the course, with default option.
      4. Make sure you see the check box "include calender events" during the backup and restore procedures.
      5. Just to be extra safe make sure course and activity folder in the backups have calendar.xml with respective events data
      6. Restore the course as the following
      • on an existing course on same site
      • on a new course on same site
      • on an existing course on a different site
      • on a new course on a different site
      1. Make sure in each case all events except site events and user events are transfered properly. (We are not backing up site events and user events)
      2. Make sure repeated events are properly restored.
      3. Make sure events with images are restored properly and images are present in the restored events.
      4. Also cross check the database to make sure the newly created events are associated with correct ids of activity, user and group.i.e they are tied to the new ids not the old ones
      5. run this test on atleast more than one type of DB
      Show
      Get ready, this can take a while Create a complete range of different Calender events including atleast *site events (sitepages->calendar->new event->type>site) *site events that are repeated (use the repeat setting while creating a calendar event) *user events (sitepages->calendar->new event->type->user) *group events *course events (gotto a course-> add calendar block-> click on month name->new event) *Course events with images in description *course events that are repeated (use the repeat setting while creating a calendar event) *activity events. (Activity events are automatically created when you create a activity with due dates) Backup the course, with default option. Make sure you see the check box "include calender events" during the backup and restore procedures. Just to be extra safe make sure course and activity folder in the backups have calendar.xml with respective events data Restore the course as the following on an existing course on same site on a new course on same site on an existing course on a different site on a new course on a different site Make sure in each case all events except site events and user events are transfered properly. (We are not backing up site events and user events) Make sure repeated events are properly restored. Make sure events with images are restored properly and images are present in the restored events. Also cross check the database to make sure the newly created events are associated with correct ids of activity, user and group.i.e they are tied to the new ids not the old ones run this test on atleast more than one type of DB
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-22895-master
    • Rank:
      257

      Description

      We should be able to backup course events when course backup is performed. And do that controlled by one setting in backup (defaulting to yes).

      See backup_events_info() under Moodle 1.9.

      Then, restore will allow to restore that information if available and also, optionally controlled by one setting.

      There is one open bug related to this since ages ago: MDL-13425 Perhaps we should fix that too, by adding one new setting on backup/restore to control if that piece of information is included. Or, at least, prevent events to be compied on import.

        Issue Links

          Activity

          Hide
          Ashley Holman added a comment -

          Given that this one is marked as an easy fix, and it is a regression in functionality from 1.9, could this be done for next release?

          Show
          Ashley Holman added a comment - Given that this one is marked as an easy fix, and it is a regression in functionality from 1.9, could this be done for next release?
          Hide
          Michael de Raadt added a comment -

          Carrying over to new sprint.

          Show
          Michael de Raadt added a comment - Carrying over to new sprint.
          Hide
          Aparup Banerjee added a comment - - edited

          Hi Ankit,
          i've just had a look and heres some notes.

          • backup_root_task.class.php line 126 : For grade & comments etc - it makes sense to do '$users->add_dependency();' straight off but for calendar events i think we need to check if the events are actually really user specific events. If there are users events then only we add the dependency. If there are no user events we may want to avoid the unnecessary dependency (and its constraints). This may mean you have to parse for any user event types.
          • maybe the comment 'Events (conditionally)' is better of being 'Calendar events (conditionally)' to avoid confusion with more general events.
          • as chatted - whitespaces:
            --backup/moodle2/restore_root_task.class.php:178: trailing whitespace.
            --backup/moodle2/restore_root_task.class.php:180: trailing whitespace.
            --backup/moodle2/restore_stepslib.php:1615: trailing whitespace.
            --backup/moodle2/restore_stepslib.php:1616: trailing whitespace.

          so imo a little more work/discussion might really finish this off

          Show
          Aparup Banerjee added a comment - - edited Hi Ankit, i've just had a look and heres some notes. backup_root_task.class.php line 126 : For grade & comments etc - it makes sense to do '$users->add_dependency();' straight off but for calendar events i think we need to check if the events are actually really user specific events. If there are users events then only we add the dependency. If there are no user events we may want to avoid the unnecessary dependency (and its constraints). This may mean you have to parse for any user event types. maybe the comment 'Events (conditionally)' is better of being 'Calendar events (conditionally)' to avoid confusion with more general events. as chatted - whitespaces: --backup/moodle2/restore_root_task.class.php:178: trailing whitespace. --backup/moodle2/restore_root_task.class.php:180: trailing whitespace. --backup/moodle2/restore_stepslib.php:1615: trailing whitespace. --backup/moodle2/restore_stepslib.php:1616: trailing whitespace. $event->annotate_ids('group', 'groupid') is missing a ';' the annotating seems fine according to http://docs.moodle.org/dev/Backup_2.0_for_developers#Annotating_IDs - as i understand it , we simply need to annotate the Ids and the backup/restore API should handle the IDs from there. (inforef.xml) so imo a little more work/discussion might really finish this off
          Hide
          Ankit Agarwal added a comment -

          Hi Aparup,
          Thanks for the review.
          -> As discussed with you, I have left dependency intact.
          -> Fixed up other issues mentioned.
          -> Noticed that backup is broken at the moment if there is a lesson with attempts in the course. Created MDL-32061 for that

          Thanks

          Show
          Ankit Agarwal added a comment - Hi Aparup, Thanks for the review. -> As discussed with you, I have left dependency intact. -> Fixed up other issues mentioned. -> Noticed that backup is broken at the moment if there is a lesson with attempts in the course. Created MDL-32061 for that Thanks
          Hide
          Aparup Banerjee added a comment -

          oops forgot to finish review

          Show
          Aparup Banerjee added a comment - oops forgot to finish review
          Hide
          Aparup Banerjee added a comment -

          This has been delayed till next week's integration round.

          Show
          Aparup Banerjee added a comment - This has been delayed till next week's integration round.
          Hide
          Sam Hemelryk added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Sam Hemelryk added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Elizabeth Dalton added a comment -

          Will this also affect the Import function?

          Show
          Elizabeth Dalton added a comment - Will this also affect the Import function?
          Hide
          Ankit Agarwal added a comment -

          @Elizabeth
          There is no change in the calendar import/export feature. with this patch you will have additional option to backup calender events during a course backup and they can be imported during a restore from the backup. No existing calender features are changed.

          Thanks

          Show
          Ankit Agarwal added a comment - @Elizabeth There is no change in the calendar import/export feature. with this patch you will have additional option to backup calender events during a course backup and they can be imported during a restore from the backup. No existing calender features are changed. Thanks
          Hide
          Ankit Agarwal added a comment -

          rebased
          Thanks

          Show
          Ankit Agarwal added a comment - rebased Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          @Elizabeth

          just in case you are talking about the course->import feature and not the calendar one... this won't change anything there either, as far as the handling of calendar stuff is dependent of including user information, and course->import never includes user information.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - @Elizabeth just in case you are talking about the course->import feature and not the calendar one... this won't change anything there either, as far as the handling of calendar stuff is dependent of including user information, and course->import never includes user information. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Hi Ankit,

          this looks really 99% perfect. Just some minor coding/details to amend:

          1) Could we use similar naming schema in the backup_calendar_structure_step and restore_calendarevents_structure_step and also in the settings? Surely "calendarevents" everywhere is better, so you only need to rename the backup step.

          2) If for any reason the userid mapping fails, we have two alternatives: a) ignore the event completely b) map it to the user executing the restore. Right now you're assigning it to userid=1 assuming it's admin, and that may not be true. I feel myself inclined to ignore the event completely.

          3) Exactly the same happen with groupid mapping failing. In this case it's clearer that we should ignore the event completely because else, you're modifying it to be a course event. No way we can default to 0.

          4) Does events->timeduration contain one date (seconds since epoch) or one duration? If the later, then there is not point about to offset it as far as it's not a date.

          5) On restore step, instead of using count_records() surely using record_exists() is better (for readability). Also, both the $sql and the $params should be formatted better following coding guidelines.

          6) Please review the patch with some minor coding guidelines fixes:

          • Add 1-space after "//" comment.
          • if syntax, space after "if", not after curling brace
          • backup/moodle2/backup_stepslib.php #769, 1 space at the beginning is incorrect
          • backup/moodle2/restore_stepslib.php #1604, same 1 space problem
          • backup/moodle2/restore_stepslib.php #1648, space after comma required
          • backup/moodle2/restore_settingslib.php #1613, incorrect alignment of "}"
          • ... any other little thing reported by the code checker.

          7) Silly question... can events have files? Images and friends?

          So, as said, all minor, but all needed to make it perfect... so reopening for next week... thanks for the implementation, it looks perfect!

          Ciao

          Edited: grrr, formatting a bit...

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Hi Ankit, this looks really 99% perfect. Just some minor coding/details to amend: 1) Could we use similar naming schema in the backup_calendar_structure_step and restore_calendarevents_structure_step and also in the settings? Surely "calendarevents" everywhere is better, so you only need to rename the backup step. 2) If for any reason the userid mapping fails, we have two alternatives: a) ignore the event completely b) map it to the user executing the restore. Right now you're assigning it to userid=1 assuming it's admin, and that may not be true. I feel myself inclined to ignore the event completely. 3) Exactly the same happen with groupid mapping failing. In this case it's clearer that we should ignore the event completely because else, you're modifying it to be a course event. No way we can default to 0. 4) Does events->timeduration contain one date (seconds since epoch) or one duration? If the later, then there is not point about to offset it as far as it's not a date. 5) On restore step, instead of using count_records() surely using record_exists() is better (for readability). Also, both the $sql and the $params should be formatted better following coding guidelines. 6) Please review the patch with some minor coding guidelines fixes: Add 1-space after "//" comment. if syntax, space after "if", not after curling brace backup/moodle2/backup_stepslib.php #769, 1 space at the beginning is incorrect backup/moodle2/restore_stepslib.php #1604, same 1 space problem backup/moodle2/restore_stepslib.php #1648, space after comma required backup/moodle2/restore_settingslib.php #1613, incorrect alignment of "}" ... any other little thing reported by the code checker. 7) Silly question... can events have files? Images and friends? So, as said, all minor, but all needed to make it perfect... so reopening for next week... thanks for the implementation, it looks perfect! Ciao Edited: grrr, formatting a bit...
          Hide
          Ankit Agarwal added a comment -

          Hi Eloy,
          Thanks for the feedback.

          1-> Name changed to calenderevents.
          2 & 3 -> Made changes. Now the event is ignored if we cannot do the mapping
          4-> removed the offset, since its time in seconds relative to timestart
          5-> using record_exists_sql now
          6)-> Fixed all styling issues, except

          class·backup_calendarevents_setting·extends·backup_anonymize_setting·{}
          
          109: Closing brace must be on a line by itself
          

          Which I thing need not be fixed
          7->AFAIK calendar events cannot have files associated, but they can have images in the description. I guess we will have to annotate_files and than during restore add_related_files for it to work?

          Thanks

          Show
          Ankit Agarwal added a comment - Hi Eloy, Thanks for the feedback. 1-> Name changed to calenderevents. 2 & 3 -> Made changes. Now the event is ignored if we cannot do the mapping 4-> removed the offset, since its time in seconds relative to timestart 5-> using record_exists_sql now 6)-> Fixed all styling issues, except class·backup_calendarevents_setting· extends ·backup_anonymize_setting·{} 109: Closing brace must be on a line by itself Which I thing need not be fixed 7->AFAIK calendar events cannot have files associated, but they can have images in the description. I guess we will have to annotate_files and than during restore add_related_files for it to work? Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          All good, thanks!

          6- yes, it's correct. Will try to add exception to the checker to pass those.

          7- yes, if it's possible to pick files in the description, then for sure wee need to annotate them on backup and also handle them on restore. Let's talk tomorrow. I'm not sure if we have any similar case to this already supported (perhaps comments or tags...).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - All good, thanks! 6- yes, it's correct. Will try to add exception to the checker to pass those. 7- yes, if it's possible to pick files in the description, then for sure wee need to annotate them on backup and also handle them on restore. Let's talk tomorrow. I'm not sure if we have any similar case to this already supported (perhaps comments or tags...). Ciao
          Hide
          Elizabeth Dalton added a comment -

          @Eloy,

          Thanks, that answered my question. Understood that Import will never include user information. However, this raises a different question: sometimes we need to copy forward Teacher content, but not Student content. That should probably be a different bug entry....

          Show
          Elizabeth Dalton added a comment - @Eloy, Thanks, that answered my question. Understood that Import will never include user information. However, this raises a different question: sometimes we need to copy forward Teacher content, but not Student content. That should probably be a different bug entry....
          Hide
          Ankit Agarwal added a comment - - edited

          Sorry for the spam
          Edited the wrong issue

          Show
          Ankit Agarwal added a comment - - edited Sorry for the spam Edited the wrong issue
          Hide
          Ankit Agarwal added a comment -

          I have added support for images.
          Sending this for another round of review.
          Thanks

          Show
          Ankit Agarwal added a comment - I have added support for images. Sending this for another round of review. Thanks
          Hide
          Aparup Banerjee added a comment -

          The main moodle.git repository has just been updated (yesterday) with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Aparup Banerjee added a comment - The main moodle.git repository has just been updated (yesterday) with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Everything looks, perfect, integrated, thanks!

          PS: I've added one minor commit fixing some brackets and one unused global.

          Show
          Eloy Lafuente (stronk7) added a comment - Everything looks, perfect, integrated, thanks! PS: I've added one minor commit fixing some brackets and one unused global.
          Hide
          Rajesh Taneja added a comment -

          Thanks Ankit,

          Course, activity and group events are there in backup file and restored as expected.

          Show
          Rajesh Taneja added a comment - Thanks Ankit, Course, activity and group events are there in backup file and restored as expected.
          Hide
          Chris Follin added a comment -

          This is marked as fixed in 2.3. Is it strictly 2.3 or could it be backported to 2.1 and 2.2?

          Show
          Chris Follin added a comment - This is marked as fixed in 2.3. Is it strictly 2.3 or could it be backported to 2.1 and 2.2?
          Hide
          Ankit Agarwal added a comment -

          Hi Chris,
          Since this is a new feature implementation, we have no plans of back-porting it to stable branches, as all new feature fixes are done only on master as per our policy. However it should work on 2.1 and 2.2 as well without any issues.

          Thanks

          Show
          Ankit Agarwal added a comment - Hi Chris, Since this is a new feature implementation, we have no plans of back-porting it to stable branches, as all new feature fixes are done only on master as per our policy. However it should work on 2.1 and 2.2 as well without any issues. Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          +1 to consider back-porting it in another issue. I think it should apply clean as far as it's well isolated.

          And we can consider it's a missing feature from 1.9... FYC.

          Show
          Eloy Lafuente (stronk7) added a comment - +1 to consider back-porting it in another issue. I think it should apply clean as far as it's well isolated. And we can consider it's a missing feature from 1.9... FYC.
          Hide
          Dan Poltawski added a comment -

          Jolly good show!

          Your changes have made it into the Moodle release - its time to celebrate! I suggest a hot cup of English tea (with milk, no sugar) or a hoppy English ale.

          Tally-ho!

          Show
          Dan Poltawski added a comment - Jolly good show! Your changes have made it into the Moodle release - its time to celebrate! I suggest a hot cup of English tea (with milk, no sugar) or a hoppy English ale. Tally-ho!
          Hide
          Ankit Agarwal added a comment -

          Hi Eloy,
          Thanks for the comments created MDL-32431 to backport this issue.
          Thanks

          Show
          Ankit Agarwal added a comment - Hi Eloy, Thanks for the comments created MDL-32431 to backport this issue. Thanks
          Hide
          Chris Follin added a comment -

          Thank you, Ankit.

          Show
          Chris Follin added a comment - Thank you, Ankit.
          Hide
          Mike Wilday added a comment -

          Why isn't there a fix for 2.2?

          Show
          Mike Wilday added a comment - Why isn't there a fix for 2.2?
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          (Mike: 3 comments above... it's the link about to backporting)

          Show
          Eloy Lafuente (stronk7) added a comment - - edited (Mike: 3 comments above... it's the link about to backporting)
          Hide
          Mike Wilday added a comment - - edited

          — just saw previous post — yes there is a backtracker... but for some reason in 2.23 we are still experiencing the restore process not restoring the calendar dates...

          Show
          Mike Wilday added a comment - - edited — just saw previous post — yes there is a backtracker... but for some reason in 2.23 we are still experiencing the restore process not restoring the calendar dates...
          Hide
          Rosa Garcia added a comment -

          Hi there, still having the same problems above (in backup process, I can't select events if I don't select users enrolled) in Moodle 2.3.2+ (Build: 20121014).

          Can somebody help me to fix the problem in my Moodle version via diff file, patch or something like that?

          Thanks

          Show
          Rosa Garcia added a comment - Hi there, still having the same problems above (in backup process, I can't select events if I don't select users enrolled) in Moodle 2.3.2+ (Build: 20121014). Can somebody help me to fix the problem in my Moodle version via diff file, patch or something like that? Thanks
          Hide
          Tayla Craig added a comment -

          If possible, can this case be re-opened for review? I am still seeing this issue in 2.4.5. See attached screen capture of the first backup screen. None of the calendar events restored into the new course. However, I have noticed, if you go into the assignment check and uncheck 'enable' next to the due date, it appears. However, this is simply reminding the course that the assignment has a due date, not fixing the issue. Also, it's tedious and time consuming to go through every single assignment that needs to appear in the calendar.

          Please review and fix!

          Show
          Tayla Craig added a comment - If possible, can this case be re-opened for review? I am still seeing this issue in 2.4.5. See attached screen capture of the first backup screen. None of the calendar events restored into the new course. However, I have noticed, if you go into the assignment check and uncheck 'enable' next to the due date, it appears. However, this is simply reminding the course that the assignment has a due date, not fixing the issue. Also, it's tedious and time consuming to go through every single assignment that needs to appear in the calendar. Please review and fix!
          Hide
          Christian Thompson added a comment -

          I'm with Tayla on this one - I am having the same problem on Moodle 2.5. There is a related post here: https://moodle.org/mod/forum/discuss.php?d=236601#p1099071

          This may be considered minor, but it's a huge inconvenience for us as we have to manually update 1000 assignments/activities every 10 weeks so that they appear in the calendar.

          Thank you.

          Show
          Christian Thompson added a comment - I'm with Tayla on this one - I am having the same problem on Moodle 2.5. There is a related post here: https://moodle.org/mod/forum/discuss.php?d=236601#p1099071 This may be considered minor, but it's a huge inconvenience for us as we have to manually update 1000 assignments/activities every 10 weeks so that they appear in the calendar. Thank you.
          Hide
          Ankit Agarwal added a comment -

          Hi Christian and Tayla,
          How old are the backups? Any backup that was generated before this patch was created, won't have events. If you still keep facing this issue with new backups/restore, please create a new issue with detail replication instructions(our work flow does not allow us to reopen closed issue).

          Cheers

          Show
          Ankit Agarwal added a comment - Hi Christian and Tayla, How old are the backups? Any backup that was generated before this patch was created, won't have events. If you still keep facing this issue with new backups/restore, please create a new issue with detail replication instructions(our work flow does not allow us to reopen closed issue). Cheers
          Hide
          Christian Thompson added a comment -

          Thank you, Ankit!

          The backups in question are just a few weeks old. I just repeated the same thing with a new backup and restore and got the same result.

          I have created a new tracker issue here: https://tracker.moodle.org/browse/MDL-43970

          I started uploading screenshots to the new tracker, but had to stop halfway due to Internet connectivity issues. I will finish the upload then things start working normally again.

          Any help would be much much appreciated. Please let me know if you require further information.

          Best,

          Chrstian

          Show
          Christian Thompson added a comment - Thank you, Ankit! The backups in question are just a few weeks old. I just repeated the same thing with a new backup and restore and got the same result. I have created a new tracker issue here: https://tracker.moodle.org/browse/MDL-43970 I started uploading screenshots to the new tracker, but had to stop halfway due to Internet connectivity issues. I will finish the upload then things start working normally again. Any help would be much much appreciated. Please let me know if you require further information. Best, Chrstian

            People

            • Votes:
              10 Vote for this issue
              Watchers:
              16 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: