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 Sub-task
    • Status: Closed
    • Priority: Minor 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
    • Rank:
      15976

      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.

      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
        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
        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
        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
        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
        Adam Barbary added a comment -

        Shown are three new courses added by the IMS Enterprise script

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

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

        Show
        Adam Barbary added a comment - Immediately after creation, a search is performed for one of the new courses. No match is found.
        Hide
        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
        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
        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
        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
        Adam Barbary added a comment -

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

        Show
        Adam Barbary added a comment - This is the same search repeated now that the summary is no longer NULL.
        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        David Monllaó added a comment -

        Branches rebased

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

        Integrated, thanks David

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

        works fine David

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

        Fixed STOP Closed STOP Thanks STOP

        Yay, imagination! Ciao

        Show
        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: