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

      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.

        Gliffy Diagrams

          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: