Moodle
  1. Moodle
  2. MDL-19421

Dependencies and Activity Links do not restore correctly in backup

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.3, 2.1.4, 2.2.1, 2.3
    • Fix Version/s: 2.1.5, 2.2.2
    • Component/s: Backup, Lesson
    • Labels:
      None
    • Environment:
      Also replicated on Moodle 2.2.1 from self to self
    • Database:
      Any
    • Testing Instructions:
      Hide

      Setup for testing

      In a course, turn editing on and:

      • Create a new lesson
      • Create a second new lesson
        • Toggle the 'Show Advanced' button
        • Under the 'Dependent on' section choose the first lesson
        • Under the 'Link to an Activity' option choose the 'forum - News forum' (or some other activity you have)

      You may like to confirm that the settings are correct in the database with a query like:

      select id, course, name, activitylink, dependency from mdl_lesson order by id asc;
      

      Functionality testing

      Lesson duplication

      • Duplicate the second lesson using the 'x2' button and click through
      • Edit the new lesson
        • Confirm that the 'Dependent on' reports that the lesson is dependent on the first lesson
        • Confirm that the 'Link to an Activity' option points to the 'forum - News forum'

      Lesson backup/restore within the same course

      • Select the lesson
      • Back up just the lesson
      • Restore the lesson
        • Confirm that the 'Dependent on' reports that the lesson is dependent on the first lesson
        • Confirm that the 'Link to an Activity' option points to the 'forum - News forum'

      Lesson backup/restore from coursea to courseb

      • Select the lesson
      • Back up just the lesson
      • Restore the lesson to a different course
        • Confirm that the 'Dependent on' lesson link is not set
        • Confirm that the 'Link to an Activity' option is not set

      Full course backup/restore

      • Back up the entire course
      • Restore the course
        • Confirm that the 'Dependent on' section is correct for each of the lessons
        • Confirm that the 'Link to an Activity' option points to the 'forum - News forum'
      Show
      Setup for testing In a course, turn editing on and: Create a new lesson Create a second new lesson Toggle the 'Show Advanced' button Under the 'Dependent on' section choose the first lesson Under the 'Link to an Activity' option choose the 'forum - News forum' (or some other activity you have) You may like to confirm that the settings are correct in the database with a query like: select id, course, name, activitylink, dependency from mdl_lesson order by id asc; Functionality testing Lesson duplication Duplicate the second lesson using the 'x2' button and click through Edit the new lesson Confirm that the 'Dependent on' reports that the lesson is dependent on the first lesson Confirm that the 'Link to an Activity' option points to the 'forum - News forum' Lesson backup/restore within the same course Select the lesson Back up just the lesson Restore the lesson Confirm that the 'Dependent on' reports that the lesson is dependent on the first lesson Confirm that the 'Link to an Activity' option points to the 'forum - News forum' Lesson backup/restore from coursea to courseb Select the lesson Back up just the lesson Restore the lesson to a different course Confirm that the 'Dependent on' lesson link is not set Confirm that the 'Link to an Activity' option is not set Full course backup/restore Back up the entire course Restore the course Confirm that the 'Dependent on' section is correct for each of the lessons Confirm that the 'Link to an Activity' option points to the 'forum - News forum'
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-19421-master-3
    • Rank:
      2405

      Description

      When restoring a lesson backup, the dependency information is not correctly re-mapped on the new data. As a result, it still points towards the original ID.

        Activity

        Hide
        Chris Collman added a comment -

        Interestingly, the link to activity field in Lesson setting holds but just the dependency field does not. What is so tricky

        Show
        Chris Collman added a comment - Interestingly, the link to activity field in Lesson setting holds but just the dependency field does not. What is so tricky
        Hide
        Martin Dougiamas added a comment -

        Reassigning this to moodle.com for the time being, since Mark Nielsen is not maintaining Lesson any more. Please comment/vote/patch any crucial bugs that affect you for 2.0 to help us prioritise fixes for the upcoming release.

        Show
        Martin Dougiamas added a comment - Reassigning this to moodle.com for the time being, since Mark Nielsen is not maintaining Lesson any more. Please comment/vote/patch any crucial bugs that affect you for 2.0 to help us prioritise fixes for the upcoming release.
        Hide
        Michael de Raadt added a comment -

        Thanks for reporting this issue.

        We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported.

        If you believe that this issue is still relevant to current versions (2.1 and beyond), please comment on the issue. Issues left inactive for a further month will be closed.

        Michael d;

        lqjjLKA0p6

        Show
        Michael de Raadt added a comment - Thanks for reporting this issue. We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported. If you believe that this issue is still relevant to current versions (2.1 and beyond), please comment on the issue. Issues left inactive for a further month will be closed. Michael d; lqjjLKA0p6
        Hide
        Andrew Nicols added a comment -

        This is still an issue in 2.2

        Show
        Andrew Nicols added a comment - This is still an issue in 2.2
        Hide
        Andrew Nicols added a comment -

        Cherry-picks cleanly to:

        • MOODLE_21_STABLE
        • MOODLE_22_STABLE
        Show
        Andrew Nicols added a comment - Cherry-picks cleanly to: MOODLE_21_STABLE MOODLE_22_STABLE
        Hide
        Andrew Nicols added a comment -

        This patch will correct any mappings as expected in a whole course backup/restore
        When duplicating a lesson,the dependency information will be set with the same settings as the lesson being duplicated
        When restoring a single lesson activity, if the dependency matches another lesson in the same course as the lesson being created, then the dependency information is kept; otherwise it is emptied.

        Show
        Andrew Nicols added a comment - This patch will correct any mappings as expected in a whole course backup/restore When duplicating a lesson,the dependency information will be set with the same settings as the lesson being duplicated When restoring a single lesson activity, if the dependency matches another lesson in the same course as the lesson being created, then the dependency information is kept; otherwise it is emptied.
        Hide
        Andrew Nicols added a comment -

        I don't know whether this will ensure correct mappings when restoring Moodle 1.X backups - I'm not familiar with this side of things

        Show
        Andrew Nicols added a comment - I don't know whether this will ensure correct mappings when restoring Moodle 1.X backups - I'm not familiar with this side of things
        Hide
        Andrew Nicols added a comment -

        Hi Michael,

        Any chance we can find a peer reviewer for this bug?

        Cheers,

        Andrew

        Show
        Andrew Nicols added a comment - Hi Michael, Any chance we can find a peer reviewer for this bug? Cheers, Andrew
        Hide
        Rossiani Wijaya added a comment -

        Hi Andrew,

        Thank you for providing patch for this issue.

        However, performing data existence should be done by using $DB->record_exists() instead of $DB->get_record.

        Please update the patch before I submit it for integration.

        Thanks
        Rosie

        Show
        Rossiani Wijaya added a comment - Hi Andrew, Thank you for providing patch for this issue. However, performing data existence should be done by using $DB->record_exists() instead of $DB->get_record. Please update the patch before I submit it for integration. Thanks Rosie
        Hide
        Andrew Nicols added a comment -

        Updated with your suggestion - this now uses record_exists

        Show
        Andrew Nicols added a comment - Updated with your suggestion - this now uses record_exists
        Hide
        Rossiani Wijaya added a comment -

        Thanks for fixing this Andrew.

        It looks good now.

        Submitting for integration.

        Show
        Rossiani Wijaya added a comment - Thanks for fixing this Andrew. It looks good now. Submitting for integration.
        Hide
        Eloy Lafuente (stronk7) 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
        Eloy Lafuente (stronk7) 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
        Eloy Lafuente (stronk7) added a comment -

        Wow, how did I miss that "dependency" column.

        Note that, exactly after your patch there are already some TODOs about the highly similar "activitylink" column.

        So lesson has TWO self-contained exceptions (dependency and activitylink). Just guessing if it would be worth fixing both here. Patch is 99% equivalent if I'm not wrong (I've not been able to find any existing issue about the sister "activitylink" problem).

        Would that be possible, Andrew? NP if not.

        TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Wow, how did I miss that "dependency" column. Note that, exactly after your patch there are already some TODOs about the highly similar "activitylink" column. So lesson has TWO self-contained exceptions (dependency and activitylink). Just guessing if it would be worth fixing both here. Patch is 99% equivalent if I'm not wrong (I've not been able to find any existing issue about the sister "activitylink" problem). Would that be possible, Andrew? NP if not. TIA and ciao
        Hide
        Andrew Nicols added a comment -

        I'll take a look - I'd seen the activitylink TODO but I've never looked at the activitylink side of things.

        Show
        Andrew Nicols added a comment - I'll take a look - I'd seen the activitylink TODO but I've never looked at the activitylink side of things.
        Hide
        Andrew Nicols added a comment -

        Looks pretty trivial to fix. I'm just testing a fix now and will submit a new branch shortly.

        Show
        Andrew Nicols added a comment - Looks pretty trivial to fix. I'm just testing a fix now and will submit a new branch shortly.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Yay, many thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Yay, many thanks!
        Hide
        Andrew Nicols added a comment -

        Eloy, your wish is my command!

        We now handle activitylink restoration and I learned something new about the lesson module.

        Show
        Andrew Nicols added a comment - Eloy, your wish is my command! We now handle activitylink restoration and I learned something new about the lesson module.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Looks perfect! Integrated (21, 22 & master), thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Looks perfect! Integrated (21, 22 & master), thanks!
        Hide
        Gerard Caulfield added a comment -

        Wow... the sooner this sort of testing is automated the better. Test passed and well done.

        Show
        Gerard Caulfield added a comment - Wow... the sooner this sort of testing is automated the better. Test passed and well done.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Some changes to Moodle should be milestones in the project by themselves.

        This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks!

        Closing as fixed, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Some changes to Moodle should be milestones in the project by themselves. This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks! Closing as fixed, ciao

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: