Moodle
  1. Moodle
  2. MDL-28107

Logged in user can't see enrolment icons

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.3
    • Fix Version/s: 2.0.4, 2.1.1
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide

      1. Setup course with PayPal enrolment plugin
      2. As a site admin enrolment plugins should be visible
      3. Log out, enrolment plugins should also be visible
      4. Login as a standard user (student). Make sure enrollment plugins is visible.

      Show
      1. Setup course with PayPal enrolment plugin 2. As a site admin enrolment plugins should be visible 3. Log out, enrolment plugins should also be visible 4. Login as a standard user (student). Make sure enrollment plugins is visible.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull Master Branch:
    • Rank:
      17728

      Description

      Summary

      After seting up a course with ie PayPal enrolment option, either admin or 'not logged in' user can see enrolment icons. In contrary, logged in user with student privileges can't see them.

      Fix

      Why suppress showing enrolment icons? I narrowed condition to check only if icons are present at all. Fix ready to pull from github.

        Activity

        Hide
        Michael de Raadt added a comment -

        Thanks for reporting this and providing a fix.

        There may be a reason for this behaviour. I'm assigning this to Martin to see if he knows.

        Show
        Michael de Raadt added a comment - Thanks for reporting this and providing a fix. There may be a reason for this behaviour. I'm assigning this to Martin to see if he knows.
        Hide
        Rossiani Wijaya added a comment -

        Thanks Bartosz for the patch.

        I also created patches to fix 2.1 and Master versions.

        Show
        Rossiani Wijaya added a comment - Thanks Bartosz for the patch. I also created patches to fix 2.1 and Master versions.
        Hide
        Rajesh Taneja added a comment -

        Patch looks good to me Rossie
        My only concern is why we added that check in first place? It was added as part of MDL-22795.
        +1 for the current patch, as everyone should see the icons

        Show
        Rajesh Taneja added a comment - Patch looks good to me Rossie My only concern is why we added that check in first place? It was added as part of MDL-22795 . +1 for the current patch, as everyone should see the icons
        Hide
        Rossiani Wijaya added a comment -

        Hi Raj,

        It was there because it was thought to be better to not display it for login user. However it creates confusion and inconsistency for the display.

        I also mentioned this issue during scrum meeting and Martin agreed that it should be display at all time.

        Submitting to integration.

        Show
        Rossiani Wijaya added a comment - Hi Raj, It was there because it was thought to be better to not display it for login user. However it creates confusion and inconsistency for the display. I also mentioned this issue during scrum meeting and Martin agreed that it should be display at all time. Submitting to integration.
        Hide
        Sam Hemelryk added a comment -

        Thanks Rosie, this has been integrated now.

        Just mentioning as it hasn't already been mentioned that this was also discussed during our scrum meetings this week and that this is the solution we came to after discussing it.

        Show
        Sam Hemelryk added a comment - Thanks Rosie, this has been integrated now. Just mentioning as it hasn't already been mentioned that this was also discussed during our scrum meetings this week and that this is the solution we came to after discussing it.
        Hide
        Petr Škoda added a comment -

        Thanks everybody, this is now part of the weekly build.

        Show
        Petr Škoda added a comment - Thanks everybody, this is now part of the weekly build.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: