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

get_my_courses returns all courses with permission to access, not just enrolled courses

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.8, 1.9
    • Fix Version/s: 2.0
    • Component/s: Libraries
    • Labels:
      None
    • Affected Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      The datalib function get_my_courses returns all courses that the current user has permission to view. For example, for an admin it returns every course on the site. This can cause performance and also interface problems.

      In the past for a quick fix I added a limit parameter which you can use to resolve performance problems (calling the function as-is for admin requires many thousand database queries if you have thousands of courses). However the real issue is that usually when this function is called, you actually want the enrolled courses i.e. courses which people have an actual role in.

      I would suggest making get_my_courses deprecated and creating two functions get_my_permitted_courses (which is the same as current, and is what get_my_courses calls) and get_my_enrolled_courses. get_my_enrolled_courses would basically restrict the list to include only those courses you have a specific role in (as well as having the course:view permission).

      (note, eloy asked me to file this bug! blame him!

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Two minor things:

            1) +1 for this.
            2) Please don't blame me, I was just talking over Skype while chatting in HQ chat about this. Thanks Sam!

            With this, users will get their REAL list of "enrolled" courses when needed (my moodle page, profile...) and the whole list or a bigger one when viewing other users posts and do on.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Two minor things: 1) +1 for this. 2) Please don't blame me, I was just talking over Skype while chatting in HQ chat about this. Thanks Sam! With this, users will get their REAL list of "enrolled" courses when needed (my moodle page, profile...) and the whole list or a bigger one when viewing other users posts and do on. Ciao
            Hide
            paolooprandi Paolo Oprandi added a comment -

            I agree absolutely - the behaviour of category (and site?) context roles I think is undesirable. See my and Alison's comments on:

            <http://moodle.org/mod/forum/discuss.php?d=67330>

            <http://moodle.org/mod/forum/discuss.php?d=69840>

            Ciao.

            Show
            paolooprandi Paolo Oprandi added a comment - I agree absolutely - the behaviour of category (and site?) context roles I think is undesirable. See my and Alison's comments on: < http://moodle.org/mod/forum/discuss.php?d=67330 > < http://moodle.org/mod/forum/discuss.php?d=69840 > Ciao.
            Hide
            sonniesedge Charlie Owen (SonniesEdge) added a comment -

            The issue has come up again in this thread: http://moodle.org/mod/forum/discuss.php?d=78463

            I'd love to see this fixed as a "sub-admin" role, that could edit anything but the system config, was the first thing I thought of when the roles system was announced.

            Show
            sonniesedge Charlie Owen (SonniesEdge) added a comment - The issue has come up again in this thread: http://moodle.org/mod/forum/discuss.php?d=78463 I'd love to see this fixed as a "sub-admin" role, that could edit anything but the system config, was the first thing I thought of when the roles system was announced.
            Hide
            alisonpope Alison Pope added a comment -

            Wow seems I talked about this last year too (amazing what you can forget). So think these are probably the same.

            Show
            alisonpope Alison Pope added a comment - Wow seems I talked about this last year too (amazing what you can forget). So think these are probably the same.
            Hide
            skodak Petr Skoda added a comment -

            This was finally fixed in Moodle 2.0, thanks for the report.

            Show
            skodak Petr Skoda added a comment - This was finally fixed in Moodle 2.0, thanks for the report.

              People

              • Votes:
                12 Vote for this issue
                Watchers:
                12 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  24/Nov/10