Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-26385 fix enrol/imsenterprise
  3. MDL-25983

Allow course summary to be imported using IMS Enterprise enrolment plugin

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0, 2.0.1, 2.2, 2.3, 2.4
    • Fix Version/s: 2.2.5, 2.3.2
    • Component/s: Enrolments
    • Labels:
      None
    • Environment:
      Moodle 2.0, Moodle 2.0.1, Moodle 2.0.2 but could be applied to all Moodle versions.
    • Testing Instructions:
      Hide
      1. Enable IMS enterprise enrolment type with options createnewusers, createnewcourses and createnewcategories enabled and selecting the IMS file path (you can find an IMS file example attached to the issue) and save changes
      2. Copy the attached file to the specified path
      3. Execute the cron or the lower link of the IMS enrolment settings page
      4. Verify that the course has been created and the summary value is the full tag value
      Show
      Enable IMS enterprise enrolment type with options createnewusers, createnewcourses and createnewcategories enabled and selecting the IMS file path (you can find an IMS file example attached to the issue) and save changes Copy the attached file to the specified path Execute the cron or the lower link of the IMS enrolment settings page Verify that the course has been created and the summary value is the full tag value
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-25983_master

      Description

      IMS enterprise use the <short> tag, child of the <description> tag to import the title of the course.
      the patch add the IMS Enterprise valid tag <full> to allow to insert the summary of the course.

      Here's in attached file a patch to provide the course summary using IMS Enterprise enrolment plugin.
      The patch is for the 2.0.1 version of Moodle.

        Gliffy Diagrams

          Attachments

          1. Course List.jpg
            Course List.jpg
            28 kB
          2. ims_file.xml
            1.0 kB
          3. Search for course returns no results.jpg
            Search for course returns no results.jpg
            24 kB
          4. Search not NULL.jpg
            Search not NULL.jpg
            22 kB
          5. Summary no NULL.jpg
            Summary no NULL.jpg
            103 kB
          6. summarysupport.patch
            0.6 kB
          7. View of database immediately after course creation.jpg
            View of database immediately after course creation.jpg
            200 kB

            Activity

            Hide
            acspike Aaron C Spike added a comment -

            I have a similar patch to our Moodle instance. What I think would be most useful is if the configuration would list the applicable course fields and allow an administrator to choose fields from the IMS file to fill in each of these blanks. I would be interested in implementing such a feature and submitting a polished patch, but I could use some mentoring on working with the Moodle configuration codebase.

            diff --git a/enrol/imsenterprise/lib.php b/enrol/imsenterprise/lib.php
            index 5a6c098..b8c7cec 100644
            — a/enrol/imsenterprise/lib.php
            +++ b/enrol/imsenterprise/lib.php
            @@ -318,6 +318,17 @@ function process_group_tag($tagcontents){
            if(preg_match('

            {<description>.*?<short>(.*?)</short>.*?</description>}is', $tagcontents, $matches)){ $group->description = trim($matches[1]); }
            +
            + if(preg_match('{<description>.*?<long>(.*?)</long>.*?</description>}is', $tagcontents, $matches)){ + $group->longname = trim($matches[1]); + }
            + if(preg_match('{<description>.*?<short>(.*?)</short>.*?</description>}

            is', $tagcontents, $matches))

            { + $group->shortname = trim($matches[1]); + }

            + if(preg_match('

            {<description>.*?<full>(.*?)</full>.*?</description>}

            is', $tagcontents, $matches))

            { + $group->full = trim($matches[1]); + }

            +
            if(preg_match('

            {<org>.*?<orgunit>(.*?)</orgunit>.*?</org>}

            is', $tagcontents, $matches))

            { $group->category = trim($matches[1]); }

            @@ -357,8 +368,9 @@ function process_group_tag($tagcontents){
            } else {
            // Create the (hidden) course(s) if not found
            $course = new stdClass();

            • $course->fullname = $group->description;
            • $course->shortname = $coursecode;
              + $course->fullname = $group->longname;
              + $course->shortname = $group->shortname;
              + $course->summary = $group->full;
              $course->idnumber = $coursecode;
              $course->format = 'topics';
              $course->visible = 0;
            Show
            acspike Aaron C Spike added a comment - I have a similar patch to our Moodle instance. What I think would be most useful is if the configuration would list the applicable course fields and allow an administrator to choose fields from the IMS file to fill in each of these blanks. I would be interested in implementing such a feature and submitting a polished patch, but I could use some mentoring on working with the Moodle configuration codebase. diff --git a/enrol/imsenterprise/lib.php b/enrol/imsenterprise/lib.php index 5a6c098..b8c7cec 100644 — a/enrol/imsenterprise/lib.php +++ b/enrol/imsenterprise/lib.php @@ -318,6 +318,17 @@ function process_group_tag($tagcontents){ if(preg_match(' {<description>.*?<short>(.*?)</short>.*?</description>}is', $tagcontents, $matches)){ $group->description = trim($matches[1]); } + + if(preg_match('{<description>.*?<long>(.*?)</long>.*?</description>}is', $tagcontents, $matches)){ + $group->longname = trim($matches[1]); + } + if(preg_match('{<description>.*?<short>(.*?)</short>.*?</description>} is', $tagcontents, $matches)) { + $group->shortname = trim($matches[1]); + } + if(preg_match(' {<description>.*?<full>(.*?)</full>.*?</description>} is', $tagcontents, $matches)) { + $group->full = trim($matches[1]); + } + if(preg_match(' {<org>.*?<orgunit>(.*?)</orgunit>.*?</org>} is', $tagcontents, $matches)) { $group->category = trim($matches[1]); } @@ -357,8 +368,9 @@ function process_group_tag($tagcontents){ } else { // Create the (hidden) course(s) if not found $course = new stdClass(); $course->fullname = $group->description; $course->shortname = $coursecode; + $course->fullname = $group->longname; + $course->shortname = $group->shortname; + $course->summary = $group->full; $course->idnumber = $coursecode; $course->format = 'topics'; $course->visible = 0;
            Hide
            abarbary Adam Barbary added a comment -

            This is a good start, however there is still the issue of if the <full> tag is not provided, then the default will be NULL. In Moodle 2+ a NULL description will not be searchable. If you could include a check to see if <full> is absent or empty, then make it something other than NULL, i.e. ''. That would fix this part of the code.

            Show
            abarbary Adam Barbary added a comment - This is a good start, however there is still the issue of if the <full> tag is not provided, then the default will be NULL. In Moodle 2+ a NULL description will not be searchable. If you could include a check to see if <full> is absent or empty, then make it something other than NULL, i.e. ''. That would fix this part of the code.
            Hide
            abarbary Adam Barbary added a comment -

            Shown are three new courses added by the IMS Enterprise script

            Show
            abarbary Adam Barbary added a comment - Shown are three new courses added by the IMS Enterprise script
            Hide
            abarbary Adam Barbary added a comment -

            Immediately after creation, a search is performed for one of the new courses. No match is found.

            Show
            abarbary Adam Barbary added a comment - Immediately after creation, a search is performed for one of the new courses. No match is found.
            Hide
            abarbary Adam Barbary added a comment -

            You can see in the database shot above, the three IMS Enterprise created course have a value of NULL for the summary field.

            Show
            abarbary Adam Barbary added a comment - You can see in the database shot above, the three IMS Enterprise created course have a value of NULL for the summary field.
            Hide
            abarbary Adam Barbary added a comment -

            Editing the course settings, altering nothing, but just saving the settings change the summary to blank. Note highlighted course, as opposed to the other newly created courses.

            Show
            abarbary Adam Barbary added a comment - Editing the course settings, altering nothing, but just saving the settings change the summary to blank. Note highlighted course, as opposed to the other newly created courses.
            Hide
            abarbary Adam Barbary added a comment -

            This is the same search repeated now that the summary is no longer NULL.

            Show
            abarbary Adam Barbary added a comment - This is the same search repeated now that the summary is no longer NULL.
            Hide
            dmonllao David Monllaó added a comment -

            Hi Aaron and Adam,

            Thanks for the report and the proposed patches. I've added pull branches and submitted the issue to peer review, the course summary is obtained from the IMS enterprise description->full tag, if is empty it will copy the value from the course description.

            By the way, I've been unable to replicate the course search problem with NULL values on the summary column in a Moodle 2.3 stable, I suppose it has been solved by another issue.

            Show
            dmonllao David Monllaó added a comment - Hi Aaron and Adam, Thanks for the report and the proposed patches. I've added pull branches and submitted the issue to peer review, the course summary is obtained from the IMS enterprise description->full tag, if is empty it will copy the value from the course description. By the way, I've been unable to replicate the course search problem with NULL values on the summary column in a Moodle 2.3 stable, I suppose it has been solved by another issue.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi David,

            On line ~380, it seems like we are initializing the course summary to have some value. Course summary is not a required field if created manually. So perhaps, it should also applied for IMS enterprise.

            As for issue on course search, I can't reproduce the error when the course summary is set to null.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi David, On line ~380, it seems like we are initializing the course summary to have some value. Course summary is not a required field if created manually. So perhaps, it should also applied for IMS enterprise. As for issue on course search, I can't reproduce the error when the course summary is set to null.
            Hide
            dmonllao David Monllaó added a comment -

            Thanks for the comments Rossiani, now the course summary is only filled with the full tag value so admins can leave the summary field without value. I also was unable to reproduce the search issue, it should had been fixed by another issue.

            Pull branches updated

            Show
            dmonllao David Monllaó added a comment - Thanks for the comments Rossiani, now the course summary is only filled with the full tag value so admins can leave the summary field without value. I also was unable to reproduce the search issue, it should had been fixed by another issue. Pull branches updated
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            dmonllao David Monllaó added a comment -

            Branches rebased

            Show
            dmonllao David Monllaó added a comment - Branches rebased
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated, thanks David

            Show
            poltawski Dan Poltawski added a comment - Integrated, thanks David
            Hide
            phalacee Jason Fowler added a comment -

            works fine David

            Show
            phalacee Jason Fowler added a comment - works fine David
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Fixed STOP Closed STOP Thanks STOP

            Yay, imagination! Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Fixed STOP Closed STOP Thanks STOP Yay, imagination! Ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Sep/12