Moodle

Custom Authentication Plugins do not get their language strings in User add/editing screens

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Not a bug
  • Affects Version/s: 1.9.4
  • Fix Version/s: 1.9.6
  • Component/s: Authentication
  • Labels:
    None
  • Environment:
    Dev: Win XP, XAMPP 1.6.7, PHP 5.2.6
    Test: Ubuntu, PHP 5.2.6
    Prod: Linux, PHP 5.2.5
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

In user add & editing, the language strings for custom authentication plugins are not searched for correctly.

See http://moodle.org/mod/forum/discuss.php?d=116543

Will create patch and attach in a while...

Issue Links

Activity

Hide
Ian Robotham added a comment -

Patch for this issue, tested on both our local development & our test servers. Both running Moodle 1.9.4

Show
Ian Robotham added a comment - Patch for this issue, tested on both our local development & our test servers. Both running Moodle 1.9.4
Hide
Eloy Lafuente (stronk7) added a comment -

Uhm... it seems that there are a bunch of places doing the search only against 'auth' lang file.

1) Instead of the suggested patch (that only fixes one place) I would do:

  • Move the logic of auth_plugin_base->get_title() to standalone function auth_get_title() somewhere in datalib or similar.
  • Make auth_plugin_base->get_title() to use the new standalone function
  • Make all occurrences of "auth.*title" in moodle core to use the standalone function.

2) Or alternatively:

  • Leave auth_plugin_base->get_title() unmodified.
  • Instantiate auth plugin and use its get_title() method in all occurrences of moodle core

3) Or, also:

  • Leave auth_plugin_base->get_title() unmodified.
  • Create standalone function auth_get_title() somewhere in datalib or similar.
  • Make all occurrences of "auth.*title" in moodle core to use the standalone function.
    (causes logic to be duplicated)

My +1 for 1) above.

Assigning to Jerome, please await Petr's confirmation before doing anything here. TIA!

Ciao

Show
Eloy Lafuente (stronk7) added a comment - Uhm... it seems that there are a bunch of places doing the search only against 'auth' lang file. 1) Instead of the suggested patch (that only fixes one place) I would do:
  • Move the logic of auth_plugin_base->get_title() to standalone function auth_get_title() somewhere in datalib or similar.
  • Make auth_plugin_base->get_title() to use the new standalone function
  • Make all occurrences of "auth.*title" in moodle core to use the standalone function.
2) Or alternatively:
  • Leave auth_plugin_base->get_title() unmodified.
  • Instantiate auth plugin and use its get_title() method in all occurrences of moodle core
3) Or, also:
  • Leave auth_plugin_base->get_title() unmodified.
  • Create standalone function auth_get_title() somewhere in datalib or similar.
  • Make all occurrences of "auth.*title" in moodle core to use the standalone function. (causes logic to be duplicated)
My +1 for 1) above. Assigning to Jerome, please await Petr's confirmation before doing anything here. TIA! Ciao
Hide
Petr Škoda (skodak) added a comment -

I do not like this kind of custom string handling in general.
I think that in 2.0 there should be a lang file for each plugin including the core plugins

For now I would just hack - base->get_title() to make it search the separate files too
and in 2.0 rewrite the string handling there completely

I personally hate having to instantiate object just to get the title - we do that in blocks for everything and it is a nightmare

Show
Petr Škoda (skodak) added a comment - I do not like this kind of custom string handling in general. I think that in 2.0 there should be a lang file for each plugin including the core plugins For now I would just hack - base->get_title() to make it search the separate files too and in 2.0 rewrite the string handling there completely I personally hate having to instantiate object just to get the title - we do that in blocks for everything and it is a nightmare
Hide
Jerome Mouneyrac added a comment -

Hi Petr,
auth_plugin_base->get_title() already search for separate files, I think you mean that we should use the Eloy 1) suggestion? If no, tell me. I'll fix it this avo following Eloy 1) suggestion.

For information: the bug appears in three different places, and the code is well written in two other places.

Show
Jerome Mouneyrac added a comment - Hi Petr, auth_plugin_base->get_title() already search for separate files, I think you mean that we should use the Eloy 1) suggestion? If no, tell me. I'll fix it this avo following Eloy 1) suggestion. For information: the bug appears in three different places, and the code is well written in two other places.
Hide
Jerome Mouneyrac added a comment -

I attached a patch I will commit tomorrow after the build

Show
Jerome Mouneyrac added a comment - I attached a patch I will commit tomorrow after the build
Hide
Martin Dougiamas added a comment -

This accidentally got into the 1.9.5 build, but it looks OK.

What exactly are we doing for this in HEAD?

Show
Martin Dougiamas added a comment - This accidentally got into the 1.9.5 build, but it looks OK. What exactly are we doing for this in HEAD?
Hide
Petr Škoda (skodak) added a comment -

in HEAD we should split auth.php lang file into multiple files separate for each plugin and use standard get_string() support.

Show
Petr Škoda (skodak) added a comment - in HEAD we should split auth.php lang file into multiple files separate for each plugin and use standard get_string() support.
Hide
Jerome Mouneyrac added a comment -

I'll commit the current change into HEAD and write a new issue for splitting the auth language file.

Show
Jerome Mouneyrac added a comment - I'll commit the current change into HEAD and write a new issue for splitting the auth language file.
Hide
Martin Dougiamas added a comment -

Resolving this then ... Jerome, can you link to the new issue when you make it?

Show
Martin Dougiamas added a comment - Resolving this then ... Jerome, can you link to the new issue when you make it?
Hide
Jerome Mouneyrac added a comment -

sorry I didn't commit in HEAD yet. I do this now, and create a new issue for 2.0.

Show
Jerome Mouneyrac added a comment - sorry I didn't commit in HEAD yet. I do this now, and create a new issue for 2.0.
Hide
Jerome Mouneyrac added a comment -

Ok fixed on Head too.

Show
Jerome Mouneyrac added a comment - Ok fixed on Head too.
Hide
Darko Miletic added a comment -

I see this same issue back in Moodle 1.9.6+

What I did was following:
1. Made a stub for new auth plugin
2. Created supposed language files (<MOODLE>/auth/newplugin/lang/en_utf8/auth_newplugin.php)
3. Added strings:
$string['auth_newplugintitle' ] = 'New title';
$string['auth_newplugdescription'] = 'New description';

And it does not work. The only place where string was picked up was plugin title in the admin block (once the plugin was enabled).

In all other places I saw [[auth_title]] and [[auth_description]] instead of [[auth_newplugintitle]] and [[auth_newplugindescription]].

Show
Darko Miletic added a comment - I see this same issue back in Moodle 1.9.6+ What I did was following: 1. Made a stub for new auth plugin 2. Created supposed language files (<MOODLE>/auth/newplugin/lang/en_utf8/auth_newplugin.php) 3. Added strings: $string['auth_newplugintitle' ] = 'New title'; $string['auth_newplugdescription'] = 'New description'; And it does not work. The only place where string was picked up was plugin title in the admin block (once the plugin was enabled). In all other places I saw [[auth_title]] and [[auth_description]] instead of [[auth_newplugintitle]] and [[auth_newplugindescription]].
Hide
Darko Miletic added a comment -

This was my mistake sorry. Not a bug.

Show
Darko Miletic added a comment - This was my mistake sorry. Not a bug.
Hide
Jerome Mouneyrac added a comment -

No problem Darko

Show
Jerome Mouneyrac added a comment - No problem Darko
Hide
Howard Miller added a comment -

Are you sure this isn't a bug. It's inconvenient at best....

In order to get the title to display in the Site Administration block AND the 'manage authentication' screen proper, it appears that this is required....

(in the enrol plugin's land/en_utf8/auth_pluginname.php folder...

$string['title'] = 'my plugin';
$string['auth_pluginnametitle"] = 'my plugin';

Counter intuitive and/or obscure at best! I tried this in 1.9.7 BTW.

Show
Howard Miller added a comment - Are you sure this isn't a bug. It's inconvenient at best.... In order to get the title to display in the Site Administration block AND the 'manage authentication' screen proper, it appears that this is required.... (in the enrol plugin's land/en_utf8/auth_pluginname.php folder... $string['title'] = 'my plugin'; $string['auth_pluginnametitle"] = 'my plugin'; Counter intuitive and/or obscure at best! I tried this in 1.9.7 BTW.
Hide
Jerome Mouneyrac added a comment -

Hi Howard,
I agree that it sounds weird to have to put two strings for the title into a lang file.

I confirm that you can write $string['auth_newplugintitle' ] = 'New title'; into the lang file named auth_newplugin.php (required for the Manage authentication screen)
I didn't find where $string['title'] is called. Can you give me the screen?
Thanks.

Show
Jerome Mouneyrac added a comment - Hi Howard, I agree that it sounds weird to have to put two strings for the title into a lang file. I confirm that you can write $string['auth_newplugintitle' ] = 'New title'; into the lang file named auth_newplugin.php (required for the Manage authentication screen) I didn't find where $string['title'] is called. Can you give me the screen? Thanks.
Hide
Howard Miller added a comment - - edited

Hi,

Sure... it's the title displayed at the top of the settings page for auth plugin. Of coure, I would suggest that you should only need the title one. The other is, surely, strictly speaking wrong as you don't need auth_... anywhere else.

Show
Howard Miller added a comment - - edited Hi, Sure... it's the title displayed at the top of the settings page for auth plugin. Of coure, I would suggest that you should only need the title one. The other is, surely, strictly speaking wrong as you don't need auth_... anywhere else.

Dates

  • Created:
    Updated:
    Resolved: