Moodle
  1. Moodle
  2. MDL-34020

Further work on IMS package importing when using Blackboard packages

    Details

    • Testing Instructions:
      Hide

      Please use my provided IMS package for testing. Without the patch you would expect the package to import but there is no content in the right hand side if you click on each item of the TOC. My patch fixes the import process to associated with html content within the IMS package.

      This IMS package that has resources with linked XML documents so it needs to be recursively processed to find the html / htm content :-

      • imsmanifest.xml -
        <resource identifier="URN_X-WEBCT-VISTA-V0_f2729b16-0ae5-0409-0157-e182c1d4e136.17926318167031" type="webct.manifest" webct:coType="webct.page" xsi:type="webct:ResourceType">
        <file href="X-WEBCT-VISTA-V0/f2729b16-0ae5-0409-0157-e182c1d4e136.17926318167031.xml"/>
        </resource>
      • X-WEBCT-VISTA-V0/f2729b16-0ae5-0409-0157-e182c1d4e136.17926318167031.xml -
        <resource identifier="URN_X-WEBCT-VISTA-V0_f2729b16-0ae5-0409-0157-e182c1d4e136.17926318164031" type="webct.manifest" webct:coType="webct.file" xsi:type="webct:ResourceType">
        <file href="X-WEBCT-VISTA-V0/f2729b16-0ae5-0409-0157-e182c1d4e136.17926318164031.xml"/>
        </resource>
      • X-WEBCT-VISTA-V0/f2729b16-0ae5-0409-0157-e182c1d4e136.17926318164031.xml -
        <resource identifier="URN_X-WEBCT-VISTA-V0_f2729b16-0ae5-0409-0157-e182c1d4e136.17926318164031_R" type="webcontent">
        <file href="X-WEBCT-VISTA-V0/f2729b16-0ae5-0409-0157-e182c1d4e136.17926318164031_R.html"/>
        </resource>
      Show
      Please use my provided IMS package for testing. Without the patch you would expect the package to import but there is no content in the right hand side if you click on each item of the TOC. My patch fixes the import process to associated with html content within the IMS package. This IMS package that has resources with linked XML documents so it needs to be recursively processed to find the html / htm content :- imsmanifest.xml - <resource identifier="URN_X-WEBCT-VISTA-V0_f2729b16-0ae5-0409-0157-e182c1d4e136.17926318167031" type="webct.manifest" webct:coType="webct.page" xsi:type="webct:ResourceType"> <file href="X-WEBCT-VISTA-V0/f2729b16-0ae5-0409-0157-e182c1d4e136.17926318167031.xml"/> </resource> X-WEBCT-VISTA-V0/f2729b16-0ae5-0409-0157-e182c1d4e136.17926318167031.xml - <resource identifier="URN_X-WEBCT-VISTA-V0_f2729b16-0ae5-0409-0157-e182c1d4e136.17926318164031" type="webct.manifest" webct:coType="webct.file" xsi:type="webct:ResourceType"> <file href="X-WEBCT-VISTA-V0/f2729b16-0ae5-0409-0157-e182c1d4e136.17926318164031.xml"/> </resource> X-WEBCT-VISTA-V0/f2729b16-0ae5-0409-0157-e182c1d4e136.17926318164031.xml - <resource identifier="URN_X-WEBCT-VISTA-V0_f2729b16-0ae5-0409-0157-e182c1d4e136.17926318164031_R" type="webcontent"> <file href="X-WEBCT-VISTA-V0/f2729b16-0ae5-0409-0157-e182c1d4e136.17926318164031_R.html"/> </resource>
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull 2.6 Branch:
      mdl34020-moodle26
    • Pull Master Branch:
      mdl34020-master

      Description

      Previously in MDL-30419, if a Blackboard / WebCT IMS package was converted using http://elutools.coventry.ac.uk/ImsCleanupBB.aspx you could import the package.

      This patch will convert directly from Blackboard / WebCT IMS package to Moodle now.

        Gliffy Diagrams

          Activity

          Hide
          Tim Lock added a comment -

          Added Pull 2.1/2.2/2.3/master Pull code

          Show
          Tim Lock added a comment - Added Pull 2.1/2.2/2.3/master Pull code
          Hide
          Dan Poltawski added a comment -

          Thanks for the Patch, Tim!

          Requesting peer review.

          Show
          Dan Poltawski added a comment - Thanks for the Patch, Tim! Requesting peer review.
          Hide
          Sam Hemelryk added a comment -

          Hi Tim + Dan,

          I've looked through this now and noted the following things:

          1. There is still a call to imscp_parse_manifestfile that hasn't been updated in mod/imscp/backup/moodle1/lib.php.
          2. Looking at imscp_parse_manifestfile and imscp_parse_structure I can't help but think we should just pass $imscp and $content to imscp_parse_manifestfile and have it collect the contents of manifest file, and essentially deprecate imscp_parse_structure having it just call imscp_parse_manifestfile. Just saves us an otherwise unneeded argument.
          3. The first two calls to pathinfo in imscp_recursive_href could be optionally replaced by dirname() and pathname() respectively.

          Really this looks top notch thanks, I think point 1 needs to be addressed before this gets put up for integration, points 2 and 3 are totally up to you feel free to implement them, or just ignore them

          Finally just noting I have not tested this, only reviewed code.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Tim + Dan, I've looked through this now and noted the following things: There is still a call to imscp_parse_manifestfile that hasn't been updated in mod/imscp/backup/moodle1/lib.php. Looking at imscp_parse_manifestfile and imscp_parse_structure I can't help but think we should just pass $imscp and $content to imscp_parse_manifestfile and have it collect the contents of manifest file, and essentially deprecate imscp_parse_structure having it just call imscp_parse_manifestfile. Just saves us an otherwise unneeded argument. The first two calls to pathinfo in imscp_recursive_href could be optionally replaced by dirname() and pathname() respectively. Really this looks top notch thanks, I think point 1 needs to be addressed before this gets put up for integration, points 2 and 3 are totally up to you feel free to implement them, or just ignore them Finally just noting I have not tested this, only reviewed code. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          ping?

          Show
          Sam Hemelryk added a comment - ping?
          Hide
          Tim Lock added a comment -

          Hi Sam,

          I'll add this to the list of things to followup

          Show
          Tim Lock added a comment - Hi Sam, I'll add this to the list of things to followup
          Hide
          Tim Lock added a comment -

          Updated github links and PR changes.

          Show
          Tim Lock added a comment - Updated github links and PR changes.
          Hide
          CiBoT added a comment -

          Results for MDL-34020

          Show
          CiBoT added a comment - Results for MDL-34020 Remote repository: https://github.com/tlock/moodle.git Remote branch mdl34020-moodle24 to be integrated into upstream MOODLE_24_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/361 Warning: The mdl34020-moodle24 branch at https://github.com/tlock/moodle.git has not been rebased recently. Details: http://integration.moodle.org/job/Precheck%20remote%20branch/361/artifact/work/smurf.html Remote branch mdl34020-moodle25 to be integrated into upstream MOODLE_25_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/362 Warning: The mdl34020-moodle25 branch at https://github.com/tlock/moodle.git has not been rebased recently. Details: http://integration.moodle.org/job/Precheck%20remote%20branch/362/artifact/work/smurf.html Remote branch mdl34020-moodle26 to be integrated into upstream MOODLE_26_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/363 Warning: The mdl34020-moodle26 branch at https://github.com/tlock/moodle.git has not been rebased recently. Details: http://integration.moodle.org/job/Precheck%20remote%20branch/363/artifact/work/smurf.html Remote branch mdl34020-master to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/364 Warning: The mdl34020-master branch at https://github.com/tlock/moodle.git has not been rebased recently. Details: http://integration.moodle.org/job/Precheck%20remote%20branch/364/artifact/work/smurf.html
          Hide
          Sam Hemelryk added a comment -

          Thanks Tim, sorry this got lost in my queue for some time.
          I've just looked at it now and the changes look spot on thank you. I'm pushing this forwards to integration.

          Show
          Sam Hemelryk added a comment - Thanks Tim, sorry this got lost in my queue for some time. I've just looked at it now and the changes look spot on thank you. I'm pushing this forwards to integration.
          Hide
          Dan Poltawski added a comment -

          Thanks Tim!

          I've integrated in master, 26 and 25. Normally we wouldn't allow changes like this to the 'API' - but here both Sam and I are in agreement that the chances of people using this are remote and it is worthwhile changing.

          cheers

          Show
          Dan Poltawski added a comment - Thanks Tim! I've integrated in master, 26 and 25. Normally we wouldn't allow changes like this to the 'API' - but here both Sam and I are in agreement that the chances of people using this are remote and it is worthwhile changing. cheers
          Hide
          Sam Hemelryk added a comment -

          Tested and passed thanks Tim

          Show
          Sam Hemelryk added a comment - Tested and passed thanks Tim
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I won't be saying "Thanks!" this week. I'm tired of it.

          For the good (and the bad), your code is now part of Moodle, the best LMS in the world. Hope you are contributing for that to continue being a fact (and not the opposite), sincerely.

          Just closing this as fixed, ciao

          PS: Just a bit of black/cruel humor, sorry, LOL!

          Show
          Eloy Lafuente (stronk7) added a comment - I won't be saying "Thanks!" this week. I'm tired of it. For the good (and the bad), your code is now part of Moodle, the best LMS in the world. Hope you are contributing for that to continue being a fact (and not the opposite), sincerely. Just closing this as fixed, ciao PS: Just a bit of black/cruel humor, sorry, LOL!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: