Moodle
  1. Moodle
  2. MDL-5016

Move the backup XML generation functions to one central place

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 2.0
    • Fix Version/s: None
    • Component/s: RSS
    • Labels:
      None
    • Environment:
      All
    • Affected Branches:
      MOODLE_20_STABLE
    • Rank:
      3183

      Description

      And reuse them completely from other XML generators like:

      • Glossary
      • Quizzes
      • Glossary

      Now we have such functions, practically copied in every place. Backup seems to be the better choice.

        Activity

        Hide
        Penny Leach added a comment -

        I'm going to at least start this by moving the functions out of backuplib.php into a new lib, xmllib.php

        And then I'm going to make the preset code use it.

        Show
        Penny Leach added a comment - I'm going to at least start this by moving the functions out of backuplib.php into a new lib, xmllib.php And then I'm going to make the preset code use it.
        Hide
        Penny Leach added a comment -

        First problem I have encountered is full_tag which calls a backup specific function.

        $content = backup_encode_absolute_links($content);

        I wonder if the way to deal with this is to tell the xml generator the name of a callback function - obviously the best way to do this is make an xmlgenerator object and pass a contentmanipulation callback function name at constructor time. That would be an enormous patch though.

        Perhaps backup can define a constant? XML_CONTENT_CALLBACK and the xml generator can check for that function and behave accordingly ?

        Show
        Penny Leach added a comment - First problem I have encountered is full_tag which calls a backup specific function. $content = backup_encode_absolute_links($content); I wonder if the way to deal with this is to tell the xml generator the name of a callback function - obviously the best way to do this is make an xmlgenerator object and pass a contentmanipulation callback function name at constructor time. That would be an enormous patch though. Perhaps backup can define a constant? XML_CONTENT_CALLBACK and the xml generator can check for that function and behave accordingly ?
        Hide
        Martin Dougiamas added a comment -

        Penny I'm all for centralising that stuff in HEAD!

        Make really sure it's really stable/safe though before applying to 1.8 ...

        Show
        Martin Dougiamas added a comment - Penny I'm all for centralising that stuff in HEAD! Make really sure it's really stable/safe though before applying to 1.8 ...
        Hide
        Martin Dougiamas added a comment -

        About the absolute links ... could this not be kept in there (but just renamed from being a backup specific function) ... and switched on with an ordinary parameters ($encodeabsolutelinks) ? I've not looked at the code, but it sounds like something that lots of Moodle XML might want to do ...

        Show
        Martin Dougiamas added a comment - About the absolute links ... could this not be kept in there (but just renamed from being a backup specific function) ... and switched on with an ordinary parameters ($encodeabsolutelinks) ? I've not looked at the code, but it sounds like something that lots of Moodle XML might want to do ...
        Hide
        Tim Hunt added a comment -

        Following on from the discussion in Moodle HQ chat.

        Penny, how about abase class and two subclasses.

        People writing XML just call the base-class methods.

        One subclass implements these using the backup_... callback, and you them make the functions in backup a wrapper round that class.

        The other subclass does it properly and use used in new code.

        Show
        Tim Hunt added a comment - Following on from the discussion in Moodle HQ chat. Penny, how about abase class and two subclasses. People writing XML just call the base-class methods. One subclass implements these using the backup_... callback, and you them make the functions in backup a wrapper round that class. The other subclass does it properly and use used in new code.
        Hide
        Penny Leach added a comment -

        Hum... That's certainly more elegant, although how do the backup wrapper functions keep a copy of the class around? a global in backuplib.php ?

        Show
        Penny Leach added a comment - Hum... That's certainly more elegant, although how do the backup wrapper functions keep a copy of the class around? a global in backuplib.php ?
        Hide
        Michael de Raadt added a comment -

        Thanks for reporting this issue.

        We have detected that this issue has been inactive for over a year. It was reported as affecting versions that are no longer supported.

        If you believe that this issue is still relevant to current versions (2.5 and beyond), please comment on the issue. Issues left inactive for a further month will be closed.

        Michael d.

        TW9vZGxlDQo=

        Show
        Michael de Raadt added a comment - Thanks for reporting this issue. We have detected that this issue has been inactive for over a year. It was reported as affecting versions that are no longer supported. If you believe that this issue is still relevant to current versions (2.5 and beyond), please comment on the issue. Issues left inactive for a further month will be closed. Michael d. TW9vZGxlDQo=
        Hide
        Michael de Raadt added a comment -

        I'm closing this issue as it has been inactive for over a year has been recorded as affecting versions that are no longer supported.

        This is being done as part of a bulk annual clean-up of issues.

        If you still believe this is an issue in supported versions, please create a new issue.

        Show
        Michael de Raadt added a comment - I'm closing this issue as it has been inactive for over a year has been recorded as affecting versions that are no longer supported. This is being done as part of a bulk annual clean-up of issues. If you still believe this is an issue in supported versions, please create a new issue.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: