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

        1. ims_file.xml
          1.0 kB
          David Monllaó
        2. summarysupport.patch
          0.6 kB
          Gilles-Philippe Leblanc
        1. Course List.jpg
          28 kB
        2. Search for course returns no results.jpg
          24 kB
        3. Search not NULL.jpg
          22 kB
        4. Summary no NULL.jpg
          103 kB
        5. 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