Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-33362

Missing rule BOOKSTART restoring 19_STABLE book backups

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.3
    • Fix Version/s: 2.3.2
    • Component/s: Book
    • Labels:
    • Testing Instructions:
      Hide
      1. Run upgrades on all branches before proceeding with the testing
      2. install book module on your 1.9 install http://moodle.org/plugins/pluginversions.php?plugin=mod_book
      3. create a book instance
      4. create a lesson and add a question page to the lesson, add a link to the book module in the question description
      5. backup the course with all defaults
      6. Restore the course as "new course" in 2.3/master
      7. Go to the lesson and make sure the link to book module now points to the restored book module in the course and doesnt say something like $@BOOKSTART*7@$
      Show
      Run upgrades on all branches before proceeding with the testing install book module on your 1.9 install http://moodle.org/plugins/pluginversions.php?plugin=mod_book create a book instance create a lesson and add a question page to the lesson, add a link to the book module in the question description backup the course with all defaults Restore the course as "new course" in 2.3/master Go to the lesson and make sure the link to book module now points to the restored book module in the course and doesnt say something like $@BOOKSTART*7@$
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-33362-master

      Description

      From https://github.com/skodak/moodle-mod_book/issues/23:

      Petr I found also another problem, this time with backup in 1.9 and restore in 2.2.

      The problem is, that ju didnt handle restore_decode_rule for BOOKSTART, you generate in moodle 1.9 backup for links to books. (line 123 in moodle 1.9/mod/book/backuplib.php).

      I fix this problem adding $rules[] = new restore_decode_rule('BOOKSTART', '/mod/book/view.php?id=$1', 'course_module'); in moodle22/mod/book/backup/moodle2/restore_book_activity_task.class.php in function define_decode_rules()

      Just letting you know, so you can fix it for everybody

      So this needs verification and fix, at least in 2.3, ideally to 2.2 too.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            We'll need to implement this in a STABLE sprint, unless you want to push a change through, Eloy.

            Show
            salvetore Michael de Raadt added a comment - We'll need to implement this in a STABLE sprint, unless you want to push a change through, Eloy.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Requesting a review!
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Requesting a review! Thanks
            Hide
            rwijaya Rossiani Wijaya added a comment -

            This looks good Ankit.

            +1 for peer review.

            Show
            rwijaya Rossiani Wijaya added a comment - This looks good Ankit. +1 for peer review.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Thanks Rosie for the review.
            Sending for integration!!

            Show
            ankit_frenz Ankit Agarwal added a comment - Thanks Rosie for the review. Sending for integration!!
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Ankit,

            I don't think that our rules for this are well defined, but I don't it is wise to increment the version number like you are doing in 2.3.x, because you are setting the module version number in 2.3 and 2.4 exactly equivalent.

            This means if there are any steps in 2.4 right now that aren't applied in 2.3.x, then when someone upgrades to 2.3.2 then the steps will be missed upon upgrading to 2.4 as the version number will be greater than the previous steps.

            So, I think that the version number should only be incremented on the 'release increments' like the main version.php comment says:

            // YYYYMMDD      = weekly release date of this DEV branch
            //         RR    = release increments - 00 in DEV branches
            //           .XX = incremental changes

            The YYYYMMDD should be frozen at the 2.3 version and you use the increments in the middle to do steps.

            Show
            poltawski Dan Poltawski added a comment - Hi Ankit, I don't think that our rules for this are well defined, but I don't it is wise to increment the version number like you are doing in 2.3.x, because you are setting the module version number in 2.3 and 2.4 exactly equivalent. This means if there are any steps in 2.4 right now that aren't applied in 2.3.x, then when someone upgrades to 2.3.2 then the steps will be missed upon upgrading to 2.4 as the version number will be greater than the previous steps. So, I think that the version number should only be incremented on the 'release increments' like the main version.php comment says: // YYYYMMDD = weekly release date of this DEV branch // RR = release increments - 00 in DEV branches // .XX = incremental changes The YYYYMMDD should be frozen at the 2.3 version and you use the increments in the middle to do steps.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Hi Dan,
            I have updated the 2.3 patch as per your suggestion.
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Hi Dan, I have updated the 2.3 patch as per your suggestion. Thanks
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated, thanks Ankit!

            Show
            poltawski Dan Poltawski added a comment - Integrated, thanks Ankit!
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Hello Ankit,

            It seems it's not fixed yet. With Chapterid you get "$@BOOKSTART*2@$" and if you add link to question you get similar result. No proper links are produced.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Hello Ankit, It seems it's not fixed yet. With Chapterid you get "$@BOOKSTART*2@$" and if you add link to question you get similar result. No proper links are produced.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            HI Rajesh,
            The issue you encountered is a generic issue with the backup-restore feature of Moodle. Created MDL-35008 for that. The link should be working fine for all other places beside the "answer text" field. Anyways it is not related to this bug.
            Also created MDL-35007 to handle the missing book chapter rule that you found.

            IMO as long as the link is showing up fine in other places (like lesson description, blog post etc) it should be safe to pass this issue.
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - HI Rajesh, The issue you encountered is a generic issue with the backup-restore feature of Moodle. Created MDL-35008 for that. The link should be working fine for all other places beside the "answer text" field. Anyways it is not related to this bug. Also created MDL-35007 to handle the missing book chapter rule that you found. IMO as long as the link is showing up fine in other places (like lesson description, blog post etc) it should be safe to pass this issue. Thanks
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Ankit,

            Passing this as you have created other issues to take care of this.

            FYI:
            Link is only generated for bookstart and not for chapters. Also, it is not creating any link for lesson options.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Ankit, Passing this as you have created other issues to take care of this. FYI: Link is only generated for bookstart and not for chapters. Also, it is not creating any link for lesson options.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            YEAR!*

            CAF*, TOT!*

            • Your effort amazingly resulted. (unbelievable :-P)
            • Closing as fixed.
            • Tons of thanks.
            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - YEAR!* CAF*, TOT!* Your effort amazingly resulted. (unbelievable :-P) Closing as fixed. Tons of thanks.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Sep/12