Moodle
  1. Moodle
  2. MDL-41594

List of hidden categories is shown to users without the correct permission

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5.1, 2.6
    • Fix Version/s: 2.5.3
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide

      Only way I could reproduce this bug:

      1. Log in as admin/manager
      2. Create hidden category "A" on the top level
      3. Manually update in DB course_categories table and change parent of category "A" to some non-existing id.
      4. As admin purge caches and view the courses list, the category is still shown dimmed
      5. Log out
      6. As guest view the courses list, make sure the hidden category does not appear
      Show
      Only way I could reproduce this bug: Log in as admin/manager Create hidden category "A" on the top level Manually update in DB course_categories table and change parent of category "A" to some non-existing id. As admin purge caches and view the courses list, the category is still shown dimmed Log out As guest view the courses list, make sure the hidden category does not appear
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
      wip-MDL-41594-master

      Description

      In latest version of moodle 2.5.1+ (Build: 20130830) and probably earlier user who has no permission (eg. guest) to view hidden categories of courses can see them as administrator - hidden (dimmed) but when category is clicked error unknowcategory is displayed.
      "Affected categories" are caregories without children and parent=0.
      I spend hours to learn how it works an why that way (upgrading finally from v1.9) and I found a solution of this bug . In method get_tree($id) in file coursecatlib.php

       
      --- /tmp/moodle.org/lib/coursecatlib.php        2013-08-30 03:42:53.000000000 +0200
      +++ coursecatlib.php    2013-09-04 13:49:15.000000000 +0200
      @@ -581,6 +581,9 @@
                   } else {
                       // parent not found. This is data consistency error but next fix_course_sortorder() should fix it
                       $all[0][] = $record->id;
      +                   if (!$record->visible) {
      +                           $all['0i'][] = $record->id;
      +                   }
                   }
                   $count++;
               }
      

      Is there any way to disable permanently cache? - debugging is much simpler then

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Marina Glancy added a comment -

            Hello, thanks for the report.
            I can't reproduce the problem, can you please provide exact sequence of steps?
            Your patch looks reasonable it's just I don't understand yet how can we end up in this part of code at all (why would category sortorder allow child to be before parent)

            BTW it is possible to disable caches, please read cache/README.md

            Show
            Marina Glancy added a comment - Hello, thanks for the report. I can't reproduce the problem, can you please provide exact sequence of steps? Your patch looks reasonable it's just I don't understand yet how can we end up in this part of code at all (why would category sortorder allow child to be before parent) BTW it is possible to disable caches, please read cache/README.md
            Hide
            Marina Glancy added a comment -

            Requesting peer review.

            Show
            Marina Glancy added a comment - Requesting peer review.
            Hide
            Ł.Sanocki added a comment -

            Now I see why I had a problem. I've upgraded from 1.9 -> 2.2 -> 2.5.1+ and my table course_categories looks like:
            query from get_tree:

            SELECT cc.id, cc.parent, cc.visible
                            FROM {course_categories} cc
                            ORDER BY cc.sortorder
            

            id parent visible
            28 0 1
            18 21 0
            6 21 0
            8 22 0
            24 0 1
            9 21 0
            25 0 1
            11 21 0
            4 9 0
            26 21 0
            5 9 0
            23 21 0
            7 0 1
            14 22 0
            22 21 0
            1 0 1
            21 0 0
            12 8 0
            13 8 0

            I didn't modify anything just upgraded. Today I've created a "main" category (id 29) and hide it, and now:

            id parent visible
            28 0 1
            24 0 1
            25 0 1
            7 0 1
            1 0 1
            21 0 0
            6 21 0
            18 21 0
            9 21 0
            4 9 0
            5 9 0
            11 21 0
            23 21 0
            26 21 0
            22 21 0
            8 22 0
            12 8 0
            13 8 0
            14 22 0
            29 0 0

            I suppose that upgrading procedure should be fixed to make course_categories sortorder column correct.
            I didn't took no notice that my categories looks different then before upgrade.
            To reproduce my bug you have perform upgrade maybe from 1.9

            Show
            Ł.Sanocki added a comment - Now I see why I had a problem. I've upgraded from 1.9 -> 2.2 -> 2.5.1+ and my table course_categories looks like: query from get_tree: SELECT cc.id, cc.parent, cc.visible FROM {course_categories} cc ORDER BY cc.sortorder id parent visible 28 0 1 18 21 0 6 21 0 8 22 0 24 0 1 9 21 0 25 0 1 11 21 0 4 9 0 26 21 0 5 9 0 23 21 0 7 0 1 14 22 0 22 21 0 1 0 1 21 0 0 12 8 0 13 8 0 I didn't modify anything just upgraded. Today I've created a "main" category (id 29) and hide it, and now: id parent visible 28 0 1 24 0 1 25 0 1 7 0 1 1 0 1 21 0 0 6 21 0 18 21 0 9 21 0 4 9 0 5 9 0 11 21 0 23 21 0 26 21 0 22 21 0 8 22 0 12 8 0 13 8 0 14 22 0 29 0 0 I suppose that upgrading procedure should be fixed to make course_categories sortorder column correct. I didn't took no notice that my categories looks different then before upgrade. To reproduce my bug you have perform upgrade maybe from 1.9
            Hide
            Marina Glancy added a comment -

            thanks, that explains why you had mess in DB, I'm not sure though that we need to add it as an additional upgrade step. Let's create another issue to deal with it anyway.

            Show
            Marina Glancy added a comment - thanks, that explains why you had mess in DB, I'm not sure though that we need to add it as an additional upgrade step. Let's create another issue to deal with it anyway.
            Hide
            Dan Poltawski added a comment -

            Thanks Marina,

            Looks ok, I suppose it would it would be good to have unit tests for this function but I don't see it as blocking this fix.

            Show
            Dan Poltawski added a comment - Thanks Marina, Looks ok, I suppose it would it would be good to have unit tests for this function but I don't see it as blocking this fix.
            Hide
            Marina Glancy added a comment -

            There are tests already, for example this one deals with hidden categories:
            https://github.com/moodle/moodle/blob/master/lib/tests/coursecatlib_test.php#L329
            but there are no tests for corrupt data

            Show
            Marina Glancy added a comment - There are tests already, for example this one deals with hidden categories: https://github.com/moodle/moodle/blob/master/lib/tests/coursecatlib_test.php#L329 but there are no tests for corrupt data
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Integrated (25 & master), thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - Integrated (25 & master), thanks!
            Hide
            Ankit Agarwal added a comment -

            Works as described.
            Thanks

            Show
            Ankit Agarwal added a comment - Works as described. Thanks
            Hide
            Marina Glancy added a comment -

            And THANK YOU again for making Moodle better every day!

            Another weekly release has been released.

            Show
            Marina Glancy added a comment - And THANK YOU again for making Moodle better every day! Another weekly release has been released.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: