Uploaded image for project: '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

          Attachments

            Activity

            Hide
            mudrd8mz David Mudrák 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
            mudrd8mz David Mudrák 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
            davosmith 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
            davosmith 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated (23, 24 & master), thanks!

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

            Works fine Davo, thanks for the patch

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

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

            Regards, Damyon

            Show
            damyon 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:
                  Fix Release Date:
                  11/Mar/13