Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-59826

context_header() sometimes obscures profile information for users with permission

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide
      • Make sure you have just 1 course on the site with only 1 enrolled student, s1.
      • Edit the manager role (define roles) and set 'viewdetails' and 'viewalldetails' to prevent.
      • Assign manager role to a user at system level.
      • Edit permissions in the course and override the manager role's capabilities, setting 'viewdetails' to 'allow'.
      • Now, log in as the manager user and navigate to the course participants page and click on the single student user's name.
      • Observe you DO now see the user header (the context_header) on the page (i.e. the user pic), and that you DO see the profile information below. 
      • Now, go to your profile page and change the id param to that of the user, s1.
      • Confirm that you can view their profile successfully.
      Show
      Make sure you have just 1 course on the site with only 1 enrolled student, s1. Edit the manager role (define roles) and set 'viewdetails' and 'viewalldetails' to prevent. Assign manager role to a user at system level. Edit permissions in the course and override the manager role's capabilities, setting 'viewdetails' to 'allow'. Now, log in as the manager user and navigate to the course participants page and click on the single student user's name. Observe you DO now see the user header (the context_header) on the page (i.e. the user pic), and that you DO see the profile information below.   Now, go to your profile page and change the id param to that of the user, s1. Confirm that you can view their profile successfully.
    • Affected Branches:
      MOODLE_32_STABLE, MOODLE_33_STABLE, MOODLE_34_STABLE
    • Fixed Branches:
      MOODLE_32_STABLE, MOODLE_33_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-59826-master
    • Sprint:
      3.4 Sprint 4

      Description

      Replication steps as per MDL-58953 (assuming MDL-59825 hasn't landed yet - if it has, you won't be able to replicate the failure, however the patch will still improve performance).

      • Make sure you have just 1 course on the site with only 1 enrolled student, s1.
      • Edit the manager role (define roles) and set 'viewdetails' and 'viewalldetails' to prevent.
      • Assign manager role to a user at system level.
      • Edit permissions in the course and override the manager role's capabilities, setting 'viewdetails' to 'allow'.
      • Now, log in as the manager user and navigate to the course participants page and click on the single student user's name.
      • Observe you don't see any user header (the context_header) on the page (i.e. no user pic), but that you DO see the profile information below. 

      This is a bug, caused by user_can_view_profile() failing to properly check all courses, at least when called from within context_header() without the $course param.

      As mentioned on MDL-58953:

      Now, this can be solved by fixing the user_can_view_profile() so it properly checks all courses (MDL-59825), however, I think a more practical solution (and better performing) would be to pass the $course object to user_can_view_profile() when we're sure the page is a course page. If we're certain we're in a specific course, then we know there's no way we can view the profile of a user within another course, so we don't need to consider this, and can get a performance win by restricting the scope too. 

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                jaked Jake Dallimore
                Reporter:
                jaked Jake Dallimore
                Peer reviewer:
                Adrian Greeve
                Integrator:
                Andrew Nicols
                Tester:
                Ryan Wyllie
                Participants:
                Component watchers:
                Amaia Anabitarte, Bas Brands, Carlos Escobedo, Sara Arjona (@sarjona), Víctor Déniz Falcón, Amaia Anabitarte, Bas Brands, Carlos Escobedo, Sara Arjona (@sarjona), Víctor Déniz Falcón
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/Sep/17