Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-36504 enrol improvements 2.5 META
  3. MDL-37391

do not pass ldap connection around in enrol_ldap

    XMLWordPrintable

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.5
    • Component/s: Enrolments
    • Labels:

      Description

      Iñaki Arenaza added a comment - 07/Jan/13 9:18 AM

      Sorry for being so late at reviwing this
      While the fix is OK, I'd say we should fix the cause rather than the symptoms. We shouldn't be passing $ldapconnection around. We already have the active ldap connection in $this->ldapconnection, and we should use it everywhere. The $ldapconnection passing is a leftover from the old days when the auth and enrol LDAP plugins were not classes.
      The only places where we should pass $this->ldapconnection are to the lib/ldaplib.php functions calls.
      I guess this should go into a new bug.
      Saludos.
      Iñaki.

      This issue refactors the use of ldap connection inside enrol_ldap plugin. New unit tests should guarantee that there are no regressions. During the testing I have discovered one issue with role assignments and fixed it, I decided it is safer to not backport it because it affects unsupported setups with multiple roles in one course.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              skodak Petr Skoda
              Reporter:
              skodak Petr Skoda
              Integrator:
              Dan Poltawski
              Tester:
              David Monllaó
              Participants:
              Component watchers:
              Amaia Anabitarte, Carlos Escobedo, Ferran Recio, Ilya Tregubov, Sara Arjona (@sarjona)
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                14/May/13