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

      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

        Gliffy Diagrams

          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: