Moodle
  1. Moodle
  2. MDL-5882

Bug with LDAP and Multi-enrolment.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6
    • Fix Version/s: None
    • Component/s: Enrolments
    • Labels:
      None
    • Environment:
      All
    • Affected Branches:
      MOODLE_16_STABLE
    • Rank:
      8724

      Description

      here's the scenario.

      manual enrolment set to default at site level, ldap enrolment also enabled.

      individual course settings:

      Enrolment Plugins: Site default (internal enrolment)

      course enrollable: No

      I thought ldap interactive enrolment still worked when course enrollable is set to No - if you set course enrollable to Yes - then manual enrolment is enabled.

      when a user logs in, it picks up the courses/enrolment correctly - the My courses block displays the correct courses that a student is enrolled in.

      when a user who has been enrolled in a course via Ldap trys to enter one of the courses displayed in the my courses block - they get the error:

      this course is not enrollable at the moment

      this error is thrown by this in courseenrol.php:

      if (!$course->enrollable //

      ($course->enrollable == 2 && $course->enrolstartdate > 0 && $course->enrolstartdate > time()) //

      ($course->enrollable == 2 && $course->enrolenddate > 0 && $course->enrolenddate <= time())

      )

      { print_header_simple(); notice(get_string('notenrollable'), $CFG->wwwroot); }

      I think this line in course/enrol.php is supposed to check to see if a student is already enrolled:

      if ( !empty($USER->student[$course->id]) or !empty($USER->teacher[$course->id]) ) {

      and I'm guessing this is supposed to return the relevant information:

      if (method_exists($enrol, 'get_student_courses')) {

      $enrol->get_student_courses($USER);

      I've added the following lines:

      if (method_exists($enrol, 'get_student_courses')) {

      $enrol->get_student_courses($USER);

      echo enrol:;

      print_object($p);

      echo user:;

      print_object($USER->student);

      which returns:

      enrol:manual

      Array

      (

      [1] => 1

      [153] => ldap

      [10] => ldap

      )

      Array

      (

      [1] => 1

      )

      enrol:ldap

      Array

      (

      [1] => 1

      )

      Array

      (

      [1] => 1

      )

      so when

      if ( !empty($USER->student[$course->id]) or !empty($USER->teacher[$course->id]) ) {

      is called,

      $USER->student is set to:

      Array

      (

      [1] => 1

      )

      which will fail - because the course I'm trying to gain access to has an ID of 153

      hopefully that makes sense! - bounce me on icq or google chat if you need more info!

        Activity

        Hide
        Martin Dougiamas added a comment -

        From Dan Marsden (dan at digitalclay.co.nz) Wednesday, 21 June 2006, 06:44 AM:

        If I replace the code:

        foreach ($plugins as $p) {

        $enrol = enrolment_factory::factory($p);

        if (method_exists($enrol, 'get_student_courses'))

        { $enrol->get_student_courses($USER); }

        if (method_exists($enrol, 'get_teacher_courses')) { $enrol->get_teacher_courses($USER); }

        unset($enrol);

        }





        with:

        //foreach ($plugins as $p) {

        $p = manual;

        $enrol = enrolment_factory::factory($p);

        if (method_exists($enrol, 'get_student_courses')) { $enrol->get_student_courses($USER); }

        if (method_exists($enrol, 'get_teacher_courses'))

        { $enrol->get_teacher_courses($USER); }

        unset($enrol);

        //}

        so that $enrol = enrolment_factory::factory($p);

        is only called once with the manual flag, then enrolment seems to function correctly, although I'm unsure if this is really a proper fix.

        From Iñaki Arenaza (iarenuno at eteo.mondragon.edu) Saturday, 24 June 2006, 10:57 PM:

        No, that's not a proper fix It works for you, but will generally break on other configurations.

        The real question is why is the ldap enrolment plugin deleting the manually enrolled courses (it shouldn't be doing this).

        The second question is why is the manual enrolment plugin returning courses enrolled via ldap. This is clearly a bug in the manual enrolment plugin.

        Which Moodle version are you running? (see the numbers in ...moodle/version.php)

        Saludos. Iñaki.

        From Dan Marsden (dan at digitalclay.co.nz) Monday, 26 June 2006, 08:21 AM:

        Hi Inaki,

        using 1.6stable direct from developer CVS updated 5minutes ago.

        the ldap enrolment isn't deleting the manually enrolled courses - it's deleting the LDAP ones.

        Dan

        From Petr Skoda (skodak at centrum.cz) Monday, 26 June 2006, 02:48 PM:

        assigning to MartinL...

        From Martin Langhoff (martin at catalyst.net.nz) Tuesday, 27 June 2006, 05:48 AM:

        In-te-res-ting.

        A couple of clarifications:

        • LDAP doesn't have an interactive plugin
        • It is by design that ldap enrolment isn't deleting the manually enrolled courses - it's deleting the LDAP ones. Each enrolment plugin minds its own enrolments.

        If you have LDAP enrolments configured correctly, the LDAP lookup happens at login time, and the user should see the relevant courses in 'my courses' immediately. If you are not seeing them immediately, perhaps your LDAP config settings are wrong.

        (For reference, did you have this working with 1.5? If they were working with 1.5 and don't in 1.6 maybe there's a regression.)

        The really odd thing is that you are reporting that $USER->student is getting munged. That's bad news.

        From Dan Marsden (dan at digitalclay.co.nz) Tuesday, 27 June 2006, 05:53 AM:

        Hi Martin,

        ldap lookup happens correctly at login. - it's when you try to enter a course - it fails.

        when a user who has already been enrolled at Login in a course and trys to enter one of those courses - they get the error:

        this course is not enrollable at the moment

        From Dan Marsden (dan at digitalclay.co.nz) Tuesday, 27 June 2006, 07:19 AM:

        should probably clarify - it doesn't delete the enrolments from the database - the student is still enrolled - the reference to deleting was for the arrays that are returned in other words - $USER->student is getting munged

        Dan

        From Michael Johnson (mike.johnson at wagner.edu) Thursday, 6 July 2006, 11:24 PM:

        For the record, I've applied Dan's ill-advised patch, and that has fixed it for us so far. We are gearing up to launch Moodle here on campus by the end of the summer and this is a show stopper for us. Has any progree been made so far? I'd hate to roll our installation back to 1.5. We love the new 1.6 version!

        From (rthomas at uiuc.edu) Tuesday, 11 July 2006, 06:45 AM:

        This patch worked for me too. Could someone explain the implications of this improper fix – i.e., is there a potential for corruption, or will adding another enrolment option fail, etc? Thanks in advance for a proper fix!

        From Iñaki Arenaza (iarenuno at eteo.mondragon.edu) Tuesday, 11 July 2006, 09:07 PM:

        The problem with this fix is that it relies on the current (wrong) behaviour of the manual enrolment plugin. It currently returns all of the enrolments of a user, whether they are internal/manual or not. Which it shouldn't. It should only return the manual enrolments and don't mess with other plugins' enrolments.

        If someone fixes the manual enrolment plugin, the proposed fix breaks again: you'd only get the manually enrolled courses.

        So the proper fix involves tracking the problem in the ldap enrolment plugin (supposing this is not a configuration issue).

        I'll try to setup a test installation with ldap enrolment and see if I can reproduce it and propose a proper fix (I'm a bit busy now, so don't hold your breath

        Saludos. Iñaki.

        From Iñaki Arenaza (iarenuno at eteo.mondragon.edu) Wednesday, 12 July 2006, 04:44 AM:

        I've had a bit of time this afternoon and had a look at the it. Could you please revert your fix and try the attached patch and see if it resolves the problem?

        It does in my test setup.

        Saludos. Iñaki.

        From Dan Marsden (dan at digitalclay.co.nz) Wednesday, 12 July 2006, 07:57 AM:

        thanks Inaki - I'm on leave until next week - I'll try to patch/test then!

        Dan

        From Dan Marsden (dan at digitalclay.co.nz) Monday, 17 July 2006, 04:00 AM:

        Hi Iñaki,

        The patch is good. - It fixes the standard module. - (interesting that it breaks a Hack I'm using though.....)

        I've got a hack that disables students from being removed from a course automatically via ldap when a course is no longer listed in their courses via ldap. - The hack no longer works as when the ldap module returns $USER->student it does an ldap lookup at the same time (which it should!) - so the array returned doesn't include any courses which they have entries for in the moodle database - it only returns the correct entries.... I'll just have to modify my hack!

        go ahead and commit!

        Dan

        From Dan Marsden (dan at digitalclay.co.nz) Monday, 24 July 2006, 08:39 AM:

        now in 1.6_stable and HEAD -

        Show
        Martin Dougiamas added a comment - From Dan Marsden (dan at digitalclay.co.nz) Wednesday, 21 June 2006, 06:44 AM: If I replace the code: foreach ($plugins as $p) { $enrol = enrolment_factory::factory($p); if (method_exists($enrol, 'get_student_courses')) { $enrol->get_student_courses($USER); } if (method_exists($enrol, 'get_teacher_courses')) { $enrol->get_teacher_courses($USER); } unset($enrol); } with: //foreach ($plugins as $p) { $p = manual; $enrol = enrolment_factory::factory($p); if (method_exists($enrol, 'get_student_courses')) { $enrol->get_student_courses($USER); } if (method_exists($enrol, 'get_teacher_courses')) { $enrol->get_teacher_courses($USER); } unset($enrol); //} so that $enrol = enrolment_factory::factory($p); is only called once with the manual flag, then enrolment seems to function correctly, although I'm unsure if this is really a proper fix. From Iñaki Arenaza (iarenuno at eteo.mondragon.edu) Saturday, 24 June 2006, 10:57 PM: No, that's not a proper fix It works for you, but will generally break on other configurations. The real question is why is the ldap enrolment plugin deleting the manually enrolled courses (it shouldn't be doing this). The second question is why is the manual enrolment plugin returning courses enrolled via ldap. This is clearly a bug in the manual enrolment plugin. Which Moodle version are you running? (see the numbers in ...moodle/version.php) Saludos. Iñaki. From Dan Marsden (dan at digitalclay.co.nz) Monday, 26 June 2006, 08:21 AM: Hi Inaki, using 1.6stable direct from developer CVS updated 5minutes ago. the ldap enrolment isn't deleting the manually enrolled courses - it's deleting the LDAP ones. Dan From Petr Skoda (skodak at centrum.cz) Monday, 26 June 2006, 02:48 PM: assigning to MartinL... From Martin Langhoff (martin at catalyst.net.nz) Tuesday, 27 June 2006, 05:48 AM: In-te-res-ting. A couple of clarifications: LDAP doesn't have an interactive plugin It is by design that ldap enrolment isn't deleting the manually enrolled courses - it's deleting the LDAP ones. Each enrolment plugin minds its own enrolments. If you have LDAP enrolments configured correctly, the LDAP lookup happens at login time, and the user should see the relevant courses in 'my courses' immediately. If you are not seeing them immediately, perhaps your LDAP config settings are wrong. (For reference, did you have this working with 1.5? If they were working with 1.5 and don't in 1.6 maybe there's a regression.) The really odd thing is that you are reporting that $USER->student is getting munged. That's bad news. From Dan Marsden (dan at digitalclay.co.nz) Tuesday, 27 June 2006, 05:53 AM: Hi Martin, ldap lookup happens correctly at login. - it's when you try to enter a course - it fails. when a user who has already been enrolled at Login in a course and trys to enter one of those courses - they get the error: this course is not enrollable at the moment From Dan Marsden (dan at digitalclay.co.nz) Tuesday, 27 June 2006, 07:19 AM: should probably clarify - it doesn't delete the enrolments from the database - the student is still enrolled - the reference to deleting was for the arrays that are returned in other words - $USER->student is getting munged Dan From Michael Johnson (mike.johnson at wagner.edu) Thursday, 6 July 2006, 11:24 PM: For the record, I've applied Dan's ill-advised patch, and that has fixed it for us so far. We are gearing up to launch Moodle here on campus by the end of the summer and this is a show stopper for us. Has any progree been made so far? I'd hate to roll our installation back to 1.5. We love the new 1.6 version! From (rthomas at uiuc.edu) Tuesday, 11 July 2006, 06:45 AM: This patch worked for me too. Could someone explain the implications of this improper fix – i.e., is there a potential for corruption, or will adding another enrolment option fail, etc? Thanks in advance for a proper fix! From Iñaki Arenaza (iarenuno at eteo.mondragon.edu) Tuesday, 11 July 2006, 09:07 PM: The problem with this fix is that it relies on the current (wrong) behaviour of the manual enrolment plugin. It currently returns all of the enrolments of a user, whether they are internal/manual or not. Which it shouldn't. It should only return the manual enrolments and don't mess with other plugins' enrolments. If someone fixes the manual enrolment plugin, the proposed fix breaks again: you'd only get the manually enrolled courses. So the proper fix involves tracking the problem in the ldap enrolment plugin (supposing this is not a configuration issue). I'll try to setup a test installation with ldap enrolment and see if I can reproduce it and propose a proper fix (I'm a bit busy now, so don't hold your breath Saludos. Iñaki. From Iñaki Arenaza (iarenuno at eteo.mondragon.edu) Wednesday, 12 July 2006, 04:44 AM: I've had a bit of time this afternoon and had a look at the it. Could you please revert your fix and try the attached patch and see if it resolves the problem? It does in my test setup. Saludos. Iñaki. From Dan Marsden (dan at digitalclay.co.nz) Wednesday, 12 July 2006, 07:57 AM: thanks Inaki - I'm on leave until next week - I'll try to patch/test then! Dan From Dan Marsden (dan at digitalclay.co.nz) Monday, 17 July 2006, 04:00 AM: Hi Iñaki, The patch is good. - It fixes the standard module. - (interesting that it breaks a Hack I'm using though.....) I've got a hack that disables students from being removed from a course automatically via ldap when a course is no longer listed in their courses via ldap. - The hack no longer works as when the ldap module returns $USER->student it does an ldap lookup at the same time (which it should!) - so the array returned doesn't include any courses which they have entries for in the moodle database - it only returns the correct entries.... I'll just have to modify my hack! go ahead and commit! Dan From Dan Marsden (dan at digitalclay.co.nz) Monday, 24 July 2006, 08:39 AM: now in 1.6_stable and HEAD -

          People

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

            Dates

            • Created:
              Updated:
              Resolved: