Moodle
  1. Moodle
  2. MDL-43497

Allow course language to be overridden for roles with a given capability

    Details

    • Testing Instructions:
      Hide

      Setup for tests 1 and 2:

      • have at least two non-english language pack installed (here: fr and ar)
      • create a course "no force lang" that does not force language
      • create a course "force lang" that forces the language to a certain language (e.g. ar)

      1. Test overriding session lang using a get parameter works as before

      • log out and log in again
      • access the front page of the moodle with a get parameter: /?lang=fr
      • access the "no force lang" course. Moodle strings should be shown in french.
      • access the "force lang" course. Moodle strings should be in the course language

      2. Test that it is possible to force the language, overriding the course language

      • test this with a user account that has the moodle/site:forcelanguage capability allowed, and/or as an admin user
      • log out and log in again
      • access the front page of the moodle with a get parameter: /?forcelang=fr
      • access the "no force lang" course. Moodle strings should be shown in french.
      • access the "force lang" course. Moodle strings should be in french.
      • log out and log in again
      • access the "no force lang" course. Moodle strings should be shown in the user's language
      • access the "force lang" course. Moodle strings should be shown in the course language.

      3. Run any/all the following Behat:

      • group/tests/behat/delete_groups.feature
      • group/tests/behat/create_groups.feature
      • course/tests/behat/restrict_available_activities.feature
      Show
      Setup for tests 1 and 2: have at least two non-english language pack installed (here: fr and ar) create a course "no force lang" that does not force language create a course "force lang" that forces the language to a certain language (e.g. ar) 1. Test overriding session lang using a get parameter works as before log out and log in again access the front page of the moodle with a get parameter: /?lang=fr access the "no force lang" course. Moodle strings should be shown in french. access the "force lang" course. Moodle strings should be in the course language 2. Test that it is possible to force the language, overriding the course language test this with a user account that has the moodle/site:forcelanguage capability allowed, and/or as an admin user log out and log in again access the front page of the moodle with a get parameter: /?forcelang=fr access the "no force lang" course. Moodle strings should be shown in french. access the "force lang" course. Moodle strings should be in french. log out and log in again access the "no force lang" course. Moodle strings should be shown in the user's language access the "force lang" course. Moodle strings should be shown in the course language. 3. Run any/all the following Behat: group/tests/behat/delete_groups.feature group/tests/behat/create_groups.feature course/tests/behat/restrict_available_activities.feature
    • Affected Branches:
      MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_27_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      mdl27_mdl-43497_allow_force_language_02

      Description

      Whose life this improves: Moodle administrators and Help Desk personnel.

      We've been in a situation several times where a problem was reported on a production, in-use Moodle instance, but we had a hard time to investigate the problem because the course-forced language was one we didn't understand. Usually, if it's a european language, we can more or less understand it. However, no one on our team speaks Arabic, for example.

      The fix suggested (patch provided) here allows the course forced language to be overridden if the user's role has that capability. By default, this is only possible for admin users. This is done the same way that it's done for $SESSION->lang : you specify it in a get parameter, e.g. http://moodle.example.com/?forcelang=en .

      While creating the patch, I investigated all the places where existing Moodle code temporarily changes the current language by setting $SESSION->lang. Everyplace where it made sense (e.g. where the intention is to override the current course and session language), I replaced these with calls to a new function moodle_force_language(), which uses $SESSION->forcelang. $SESSION->forcelang is checked before $course->lang and $SESSION->lang in current_language().

        Gliffy Diagrams

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for suggesting this change. It seems like a reasonable rationalisation.

          As this will introduce a behavioural change I've assigned this to Martin for his review. I've also added David Mudrak and (explicitly) Helen Foster, who may have opinions about this change.

          Show
          Michael de Raadt added a comment - Thanks for suggesting this change. It seems like a reasonable rationalisation. As this will introduce a behavioural change I've assigned this to Martin for his review. I've also added David Mudrak and (explicitly) Helen Foster, who may have opinions about this change.
          Hide
          Helen Foster added a comment -

          I know how it is trying to figure out a problem in a course with a forced language such as Arabic or Hebrew, so Brian's suggested improvement sounds good to me.

          The only disadvantage of implementing a subtle improvement such as this is that most admins won't realise that the functionality exists. We'll just have to ensure that it is mentioned in appropriate places in the documentation though, and hope that people read it.

          Show
          Helen Foster added a comment - I know how it is trying to figure out a problem in a course with a forced language such as Arabic or Hebrew, so Brian's suggested improvement sounds good to me. The only disadvantage of implementing a subtle improvement such as this is that most admins won't realise that the functionality exists. We'll just have to ensure that it is mentioned in appropriate places in the documentation though, and hope that people read it.
          Hide
          CiBoT added a comment -

          Results for MDL-43497

          Show
          CiBoT added a comment - Results for MDL-43497 Remote repository: https://github.com/brki/moodle Remote branch mdl27_mdl-43497_allow_force_language to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/367 Warning: The mdl27_mdl-43497_allow_force_language branch at https://github.com/brki/moodle has not been rebased recently. Details: http://integration.moodle.org/job/Precheck%20remote%20branch/367/artifact/work/smurf.html
          Hide
          Dan Poltawski added a comment -

          Hi Brian,

          Thanks a lot for this patch - it certainly seems like a useful support feature to me. I like that you've done some resonable clean up too. I only have a few comments:

          1. I'm not really sold on the name moodle_force_language(), afterall we know we are in Moodle. I think my suggestion would be force_current_language().
          2. I don't think the description of the function should be 'Some nasty hackery to get strings and dates localised in the given language.'. Its sort of being made non-hacky here.
          3. Don't do isset($_GET['forcelang']) . Simply use optional_param. Thats what it was designed for.
          4. Use the PARAM type PARAM_LANG, not PARAM_SAFEDIR and remove the call to get_string_manager()->translation_exists() as PARAM_LANG does it.
          Show
          Dan Poltawski added a comment - Hi Brian, Thanks a lot for this patch - it certainly seems like a useful support feature to me. I like that you've done some resonable clean up too. I only have a few comments: I'm not really sold on the name moodle_force_language() , afterall we know we are in Moodle. I think my suggestion would be force_current_language() . I don't think the description of the function should be 'Some nasty hackery to get strings and dates localised in the given language.'. Its sort of being made non-hacky here. Don't do isset($_GET ['forcelang'] ) . Simply use optional_param. Thats what it was designed for. Use the PARAM type PARAM_LANG, not PARAM_SAFEDIR and remove the call to get_string_manager()->translation_exists() as PARAM_LANG does it.
          Hide
          Brian King added a comment -

          Hi Dan,

          thanks a lot for checking it out!

          I've made the changes you've suggested, except for 4. I don't think we should do this, because if we do, it's not possible to disable forcelang without logging out. As it currently is, you can access a URL with a parameter like ?forcelang=none, and this will disable any forcelang currently in effect.

          I also made a small change to the the force_current_language() function, to allow setting forcelang to an empty value. Without this change, the language pack administration page triggered a forcelang on it's own, which was quite annoying.

          Previous version: https://github.com/brki/moodle/compare/mdl27_mdl-43497_allow_force_language
          Current version: https://github.com/brki/moodle/compare/mdl27_mdl-43497_allow_force_language_01

          Show
          Brian King added a comment - Hi Dan, thanks a lot for checking it out! I've made the changes you've suggested, except for 4. I don't think we should do this, because if we do, it's not possible to disable forcelang without logging out. As it currently is, you can access a URL with a parameter like ?forcelang=none, and this will disable any forcelang currently in effect. I also made a small change to the the force_current_language() function, to allow setting forcelang to an empty value. Without this change, the language pack administration page triggered a forcelang on it's own, which was quite annoying. Previous version: https://github.com/brki/moodle/compare/mdl27_mdl-43497_allow_force_language Current version: https://github.com/brki/moodle/compare/mdl27_mdl-43497_allow_force_language_01
          Hide
          CiBoT added a comment -

          Results for MDL-43497

          Show
          CiBoT added a comment - Results for MDL-43497 Remote repository: https://github.com/brki/moodle Remote branch mdl27_mdl-43497_allow_force_language_01 to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1957 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1957/artifact/work/smurf.html
          Hide
          Dan Poltawski added a comment -

          Thanks a lot Brian, it looks great. I'm submitting the change for integration.

          (Note i've added the api_change label even though this isn't really an API change, but we should note it in the release notes)

          Show
          Dan Poltawski added a comment - Thanks a lot Brian, it looks great. I'm submitting the change for integration. (Note i've added the api_change label even though this isn't really an API change, but we should note it in the release notes)
          Hide
          Dan Poltawski added a comment -

          Although actually, on 4 - could you explain that as a comment (we don't use PARAM_LANG here because...)

          Show
          Dan Poltawski added a comment - Although actually, on 4 - could you explain that as a comment (we don't use PARAM_LANG here because...)
          Hide
          Brian King added a comment -
          Show
          Brian King added a comment - OK, comment added. New version: https://github.com/brki/moodle/compare/mdl27_mdl-43497_allow_force_language_02
          Hide
          CiBoT added a comment -

          Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

          Show
          CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
          Hide
          Sam Hemelryk added a comment -

          Thanks Brian, this has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Brian, this has been integrated now.
          Hide
          Petr Skoda added a comment -

          Invalid get_string() identifier: 'site:forcelanguage' or component 'core_role'. Perhaps you are missing $string['site:forcelanguage'] = ''; in lang/en/core_role.php?
          line 293 of /lib/classes/string_manager_standard.php: call to debugging()
          line 6864 of /lib/moodlelib.php: call to core_string_manager_standard->get_string()
          line 2953 of /lib/accesslib.php: call to get_string()

          this needs to be fixed

          Show
          Petr Skoda added a comment - Invalid get_string() identifier: 'site:forcelanguage' or component 'core_role'. Perhaps you are missing $string ['site:forcelanguage'] = ''; in lang/en/core_role.php? line 293 of /lib/classes/string_manager_standard.php: call to debugging() line 6864 of /lib/moodlelib.php: call to core_string_manager_standard->get_string() line 2953 of /lib/accesslib.php: call to get_string() this needs to be fixed
          Hide
          Jason Fowler added a comment -

          All good, thanks Brian

          Show
          Jason Fowler added a comment - All good, thanks Brian
          Hide
          Jason Fowler added a comment -

          Totally missed Petr's comment, and didn't see the errors myself. Everything works, except the capability needs a name apparently.

          Show
          Jason Fowler added a comment - Totally missed Petr's comment, and didn't see the errors myself. Everything works, except the capability needs a name apparently.
          Hide
          Rajesh Taneja added a comment -

          This has been detected by Behat, so added that to testing instructions.

          Show
          Rajesh Taneja added a comment - This has been detected by Behat, so added that to testing instructions.
          Hide
          Sam Hemelryk added a comment -

          Ah drat, sorry I missed that, I've talked to Helen and added the missing string now.

          Sending this back to testing, thanks guys.

          Show
          Sam Hemelryk added a comment - Ah drat, sorry I missed that, I've talked to Helen and added the missing string now. Sending this back to testing, thanks guys.
          Hide
          Jason Fowler added a comment -

          assuming the behat tests passed, this is good to go

          Show
          Jason Fowler added a comment - assuming the behat tests passed, this is good to go
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The price of success is hard work,
          dedication to the job at hand,
          and the determination that whether we win or lose,
          we have applied the best of ourselves to the task at hand.

          Vince Lombardi

          This is now part of Moodle, your favorite non-frameworkial LMS, LOL. Thanks, closing!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The price of success is hard work, dedication to the job at hand, and the determination that whether we win or lose, we have applied the best of ourselves to the task at hand. Vince Lombardi This is now part of Moodle, your favorite non-frameworkial LMS, LOL. Thanks, closing! Ciao
          Hide
          Mary Cooch added a comment -

          Removing docs_required label as this has been documented here http://docs.moodle.org/27/en/Capabilities/moodle/site:forcelanguage and also http://docs.moodle.org/27/en/Course_settings and http://docs.moodle.org/27/en/Language_settings - please check and amend/improve if needed -thanks.

          Show
          Mary Cooch added a comment - Removing docs_required label as this has been documented here http://docs.moodle.org/27/en/Capabilities/moodle/site:forcelanguage and also http://docs.moodle.org/27/en/Course_settings and http://docs.moodle.org/27/en/Language_settings - please check and amend/improve if needed -thanks.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: