Moodle
  1. Moodle
  2. MDL-37801

Glossary 'showentry' links not encoded / decoded during backup & restore

    Details

    • Testing Instructions:
      Hide
      1. Create a glossary with at least one entry in it
      2. Set the glossary display format to 'Entry list'
      3. Copy the link for one of the glossary entries and paste it into a text area (e.g. the 'intro' field for the glossary itself)
      4. Backup & restore the activity
      5. Check that the link now leads to the newly restored entry, not the original entry
      Show
      Create a glossary with at least one entry in it Set the glossary display format to 'Entry list' Copy the link for one of the glossary entries and paste it into a text area (e.g. the 'intro' field for the glossary itself) Backup & restore the activity Check that the link now leads to the newly restored entry, not the original entry
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-37801_glossary_links
    • Rank:
      47542

      Description

      Set the glossary display format to 'Entry list'
      Copy the link for one of the glossary entries and paste it into a text area (e.g. the 'intro' field for the glossary itself)
      Backup & restore the activity

      Expected result:
      The 'showentry' link now links to the entry in the newly-restored glossary

      Actual result:
      The link still links to the entry in the original glossary

      I have a patch which I will attach in a moment

        Activity

        Hide
        David Mudrak added a comment -

        Thanks Davo. The solution is nice and clear. The only thing that caught my eye was that I would expect the ampersand HTTP encoded in the restore_decode_rule call. So the second parameter would read

        '/mod/glossary/showentry.php?courseid=$1&eid=$2'
        

        But then I looked at backup_restore_decode_testcase::test_restore_decode_rule() and it seems that plain ampersands are OK.

        +1

        Show
        David Mudrak added a comment - Thanks Davo. The solution is nice and clear. The only thing that caught my eye was that I would expect the ampersand HTTP encoded in the restore_decode_rule call. So the second parameter would read '/mod/glossary/showentry.php?courseid=$1&eid=$2' But then I looked at backup_restore_decode_testcase::test_restore_decode_rule() and it seems that plain ampersands are OK. +1
        Hide
        Davo Smith added a comment - - edited

        Thanks.

        I thought about that as well, but then went with what was done in other core code that had to handle & / & (in fact there may be a few places in core that break if the original URL to encode has an & in it ...)

        I'll put forward for integration review.

        Show
        Davo Smith added a comment - - edited Thanks. I thought about that as well, but then went with what was done in other core code that had to handle & / & (in fact there may be a few places in core that break if the original URL to encode has an & in it ...) I'll put forward for integration review.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated (23, 24 & master), thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
        Hide
        Jason Fowler added a comment -

        Works fine Davo, thanks for the patch

        Show
        Jason Fowler added a comment - Works fine Davo, thanks for the patch
        Hide
        Damyon Wiese added a comment -

        Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone!

        Regards, Damyon

        Show
        Damyon Wiese added a comment - Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone! Regards, Damyon

          People

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

            Dates

            • Created:
              Updated:
              Resolved: