Moodle
  1. Moodle
  2. MDL-30502

Problem with string lookup in enrol/ldap/settings.php

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.2
    • Fix Version/s: 2.1.4, 2.2.1
    • Component/s: Enrolments
    • Labels:
    • Environment:
      Windows 7
    • Rank:
      33195

      Description

      Someplace around line 59 has the following

      $settings->add(new admin_setting_ldap_rolemapping('enrol_ldap/role_mapping', get_string ('role_mapping_key', 'enrol_ldap', $role->name), get_string ('role_mapping', 'enrol_ldap'), ''));

      The third argument to the second get_string() call is not used, and generates a warning. Apparently it turns out to be a string rather than an object. In any case the string does not require a $a param.

      Replication steps:
      1. Login as administrator
      2. Turn on DEVELOPER debug messages
      3. Go to "Site administration->Plugins->Enrolments->Manage enrol plugin".

      Now you will see an error message like

      Notice: Trying to get property of non-object in C:\Users\mlitzkow\Zend\workspaces\ApacheWorkspace\Moodle_2\enrol\ldap\settings.php on line 59
      

        Activity

        Hide
        Michael de Raadt added a comment -

        Thanks for reporting that.

        What probably needs to be done is to get the name as a string and add a place-holder in the language string.

        Show
        Michael de Raadt added a comment - Thanks for reporting that. What probably needs to be done is to get the name as a string and add a place-holder in the language string.
        Hide
        Petr Škoda added a comment -

        Thanks a lot for the report.

        To integrators: please cherry pick to 2.1 and 2.2 stable.

        Show
        Petr Škoda added a comment - Thanks a lot for the report. To integrators: please cherry pick to 2.1 and 2.2 stable.
        Hide
        Sam Hemelryk added a comment -

        Thanks Petr - this has been integrated now

        Show
        Sam Hemelryk added a comment - Thanks Petr - this has been integrated now
        Hide
        Gerard Caulfield added a comment -

        I have been unable to reproduce the notice. Not sure what might be missing, but I'm passing this back to allow somebody else to attempt to test it.

        Show
        Gerard Caulfield added a comment - I have been unable to reproduce the notice. Not sure what might be missing, but I'm passing this back to allow somebody else to attempt to test it.
        Hide
        Aparup Banerjee added a comment -

        giving this a shot

        Show
        Aparup Banerjee added a comment - giving this a shot
        Hide
        Aparup Banerjee added a comment - - edited

        passing this test. there is no warnings seen.

        The replication instruction were right but inaccurate but the fix is right.

        $role was defined along the way (really in another scope) while all the */settings.php were being called. i got
        $role as
        stdClass Objectn(n [id] => 8n [name] => Authenticated user on frontpagen [shortname] => frontpagen [description] => All logged in users in the frontpage course.n [sortorder] => 8n [archetype] => frontpagen)

        the reporter probably didn't have that roles settings page loaded (perms?)

        any way simple pass for me. thanks all.

        Show
        Aparup Banerjee added a comment - - edited passing this test. there is no warnings seen. The replication instruction were right but inaccurate but the fix is right. $role was defined along the way (really in another scope) while all the */settings.php were being called. i got $role as stdClass Objectn(n [id] => 8n [name] => Authenticated user on frontpagen [shortname] => frontpagen [description] => All logged in users in the frontpage course.n [sortorder] => 8n [archetype] => frontpagen) the reporter probably didn't have that roles settings page loaded (perms?) any way simple pass for me. thanks all.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

        Now... disconnect, relax and enjoy the next days, yay!

        Closing...ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: