Moodle
  1. Moodle
  2. MDL-38320

The login info doesn't fix role name

    Details

    • Testing Instructions:
      Hide

      1/ use fresh install or empty role names in site config if upgraded from 2.3
      2/ go to course and switch role
      3/ verify the role name is displayed in to top right corner
      4/ modify the role name in course
      5/ switch again and verify the role name

      Show
      1/ use fresh install or empty role names in site config if upgraded from 2.3 2/ go to course and switch role 3/ verify the role name is displayed in to top right corner 4/ modify the role name in course 5/ switch again and verify the role name
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w12_MDL-38320_m25_logininfo

      Description

      With MDL-8249, we cannot use the role.name field for displaying the role name. Looks like core_renderer::login_info() is using this field directly to display "logged in as" information.

      Also, this method is overridden by theme_mymobile_core_renderer::login_info() and it needs to be fixed as well.

        Gliffy Diagrams

          Activity

          Hide
          Mark Nielsen added a comment -

          I probably filed this under the wrong component.

          Show
          Mark Nielsen added a comment - I probably filed this under the wrong component.
          Hide
          Mary Evans added a comment -

          Can you please explain a little more about the actual steps you have taken doing whatever it is you are trying to do, as the above description makes no sense to me.

          Also which theme are you using?

          Show
          Mary Evans added a comment - Can you please explain a little more about the actual steps you have taken doing whatever it is you are trying to do, as the above description makes no sense to me. Also which theme are you using?
          Hide
          Mark Nielsen added a comment -

          Sorry Mary!

          So, specifically, the trouble code in core_renderer::login_info() is:

          $rolename = '';
          if ($role = $DB->get_record('role', array('id'=>$USER->access['rsw'][$context->path]))) {
              $rolename = ': '.format_string($role->name);
          }
          $loggedinas = get_string('loggedinas', 'moodle', $username).$rolename;
          

          See how role name is directly accessed from the role record? After MDL-8249, this shouldn't be done and instead the role name should be retrieved via role_get_name() or role_fix_names() or whatever and then use the localname property or similar.

          Show
          Mark Nielsen added a comment - Sorry Mary! So, specifically, the trouble code in core_renderer::login_info() is: $rolename = ''; if ($role = $DB->get_record('role', array('id'=>$USER->access['rsw'][$context->path]))) { $rolename = ': '.format_string($role->name); } $loggedinas = get_string('loggedinas', 'moodle', $username).$rolename; See how role name is directly accessed from the role record? After MDL-8249 , this shouldn't be done and instead the role name should be retrieved via role_get_name() or role_fix_names() or whatever and then use the localname property or similar.
          Hide
          Mark Nielsen added a comment - - edited

          Adding Petr as a watcher since he was the author of MDL-8249 and might be able to help out.

          Show
          Mark Nielsen added a comment - - edited Adding Petr as a watcher since he was the author of MDL-8249 and might be able to help out.
          Hide
          Petr Skoda added a comment -

          Thanks for the report, I will create a fix for the next integration cycle.

          Show
          Petr Skoda added a comment - Thanks for the report, I will create a fix for the next integration cycle.
          Hide
          Damyon Wiese added a comment -

          Thanks Petr,

          The patch looks good. Integrated to master and 24.

          Show
          Damyon Wiese added a comment - Thanks Petr, The patch looks good. Integrated to master and 24.
          Hide
          Frédéric Massart added a comment -

          Test passed. However please note that the link (Return to my normal role) is glued to the role name (master only).

          Show
          Frédéric Massart added a comment - Test passed. However please note that the link (Return to my normal role) is glued to the role name (master only).
          Hide
          Damyon Wiese added a comment -

          This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).

          Thanks for your contributions!

          Show
          Damyon Wiese added a comment - This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads). Thanks for your contributions!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: