Moodle
  1. Moodle
  2. MDL-33362

Missing rule BOOKSTART restoring 19_STABLE book backups

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical 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
    • Rank:
      41227

      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.

        Issue Links

          Activity

          Hide
          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
          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 Agarwal added a comment -

          Requesting a review!
          Thanks

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

          This looks good Ankit.

          +1 for peer review.

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

          Thanks Rosie for the review.
          Sending for integration!!

          Show
          Ankit Agarwal added a comment - Thanks Rosie for the review. Sending for integration!!
          Hide
          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
          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 Agarwal added a comment -

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

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

          Integrated, thanks Ankit!

          Show
          Dan Poltawski added a comment - Integrated, thanks Ankit!
          Hide
          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
          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 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 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
          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
          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
          Eloy Lafuente (stronk7) added a comment -

          YEAR!*

          CAF*, TOT!*

          • Your effort amazingly resulted. (unbelievable :-P)
          • Closing as fixed.
          • Tons of thanks.
          Show
          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: