Moodle
  1. Moodle
  2. MDL-23412

LDAP_DEREF_NEVER and LDAP_DEREF_ALWAYS not defined when including enrol/ldap/settings.php

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Enrolments
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      26954

      Description

      Notice: Use of undefined constant LDAP_DEREF_NEVER - assumed 'LDAP_DEREF_NEVER' in /html/enrol/ldap/settings.php on line 67

      Notice: Use of undefined constant LDAP_DEREF_ALWAYS - assumed 'LDAP_DEREF_ALWAYS' in /html/enrol/ldap/settings.php on line 68

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          Hi Inaki,

          I've commit a [very] quick fix for this that simply defines the two to remove the notices, however it will need proper consideration to find a better solution (I'm sure there is one ).

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Inaki, I've commit a [very] quick fix for this that simply defines the two to remove the notices, however it will need proper consideration to find a better solution (I'm sure there is one ). Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Not sure if anyone has filed a bug for this one yet, got it during upgrade:

          Notice: Undefined property: stdClass::$enrol_ldap_version in ../moodle/enrol/ldap/db/install.php on line 64

          Show
          Sam Hemelryk added a comment - Not sure if anyone has filed a bug for this one yet, got it during upgrade: Notice: Undefined property: stdClass::$enrol_ldap_version in ../moodle/enrol/ldap/db/install.php on line 64
          Hide
          Iñaki Arenaza added a comment -

          Sam,

          that last one has been fixed in MDL-23424.

          Saludos.
          Iñaki.

          Show
          Iñaki Arenaza added a comment - Sam, that last one has been fixed in MDL-23424 . Saludos. Iñaki.
          Hide
          Iñaki Arenaza added a comment -

          I've just committed a proper fix for this. We test whether the PHP LDAP extension is present or not, and if not, we only display a notice to the user about the missin extesion and completely skip the settings.

          Saludos.
          Iñaki.

          Show
          Iñaki Arenaza added a comment - I've just committed a proper fix for this. We test whether the PHP LDAP extension is present or not, and if not, we only display a notice to the user about the missin extesion and completely skip the settings. Saludos. Iñaki.
          Hide
          Petr Škoda added a comment -

          I have fixed the install defaults, please review

          Show
          Petr Škoda added a comment - I have fixed the install defaults, please review
          Hide
          Iñaki Arenaza added a comment -

          Looks good to me.

          Saludos.
          Iñaki.

          Show
          Iñaki Arenaza added a comment - Looks good to me. Saludos. Iñaki.
          Hide
          Anthony Borrow added a comment -

          Any reason we should not define the default constant for 1.9? Peace - Anthony

          Show
          Anthony Borrow added a comment - Any reason we should not define the default constant for 1.9? Peace - Anthony
          Hide
          Iñaki Arenaza added a comment -

          Anthony,

          if think defining the constant is just papering over the real issue: the PHP LDAP extension is not present.

          Does it make sense to display the settings page even if we know for sure that we can't use the plugin? (until we install and enable the PHP extension). Right now we do it in 1.9; Moodle 2.0 simply warns the user about the situation and doesn't try to show the rest of the settings page.

          1.9 is also different in that we don't use the adminlib library to manage the enrol LDAP settings.This means we can't break install/upgrade if the constant is not defined. Unless you go to the enrol LDAP settings page, you won't notice at all (and only if you have Notices enabled in your PHP error level settings).

          So I'd either leave it alone (nothing breaks, only an ungly notice is displayed) or do the same 2.0 does: don't try to display the settings if the extension is not available. But the latter is a change from the previous behaviour, in a stable branch. I'm not sure whether that's an acceptable change or not.

          Saludos.
          Iñaki.

          Show
          Iñaki Arenaza added a comment - Anthony, if think defining the constant is just papering over the real issue: the PHP LDAP extension is not present. Does it make sense to display the settings page even if we know for sure that we can't use the plugin? (until we install and enable the PHP extension). Right now we do it in 1.9; Moodle 2.0 simply warns the user about the situation and doesn't try to show the rest of the settings page. 1.9 is also different in that we don't use the adminlib library to manage the enrol LDAP settings.This means we can't break install/upgrade if the constant is not defined. Unless you go to the enrol LDAP settings page, you won't notice at all (and only if you have Notices enabled in your PHP error level settings). So I'd either leave it alone (nothing breaks, only an ungly notice is displayed) or do the same 2.0 does: don't try to display the settings if the extension is not available. But the latter is a change from the previous behaviour, in a stable branch. I'm not sure whether that's an acceptable change or not. Saludos. Iñaki.
          Hide
          Anthony Borrow added a comment -

          Iñaki - I agree it is best to leave well enough alone. Since production sites should not be displaying these errors and it does not really make sense for it to be enabled when LDAP is not present and it is not an issue in 2.0 my +1 for not doing anything. Peace - Anthony

          Show
          Anthony Borrow added a comment - Iñaki - I agree it is best to leave well enough alone. Since production sites should not be displaying these errors and it does not really make sense for it to be enabled when LDAP is not present and it is not an issue in 2.0 my +1 for not doing anything. Peace - Anthony

            People

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

              Dates

              • Created:
                Updated:
                Resolved: