Moodle
  1. Moodle
  2. MDL-40088

Cannot edit course settings if course is in hidden category.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5.1, FRONTEND
    • Component/s: Course
    • Labels:
    • Rank:
      50818

      Description

      Upgraded school system to 2.5.latest. Teachers cannot edit settings in their courses.
      Testing method:
      Make category. Hide.
      Add class to that category.
      Add teacher in teacher role to that class.
      Enroll teacher in class.
      Try to change settings.

      Once you do this, you will receive:
      Category not known!
      on the results page. The error page gives no help:
      http://docs.moodle.org/25/en/error/moodle/unknowcategory

      By comparison:
      1.9 will allow you to do this, even if the teacher is not enrolled.
      2.4 will allow you to do this, if the teacher is enrolled.

      Thanks very much!

        Activity

        Hide
        David Millians added a comment -

        I don't know if this is a minor or major bug. Right now, during the summer, it's minor. Once school starts up, they really want categories hidden, and then it'd be major... So to be safe, I just tagged it as major. If the category offendeth thee, change it out. Thanks.

        Show
        David Millians added a comment - I don't know if this is a minor or major bug. Right now, during the summer, it's minor. Once school starts up, they really want categories hidden, and then it'd be major... So to be safe, I just tagged it as major. If the category offendeth thee, change it out. Thanks.
        Hide
        Dan Poltawski added a comment -

        Agreed this is a major regression.

        Show
        Dan Poltawski added a comment - Agreed this is a major regression.
        Hide
        Dan Poltawski added a comment -

        Looking (and not testing yet), this might be the fix:

        diff --git a/course/edit_form.php b/course/edit_form.php
        index 13ab045..c845331 100644
        --- a/course/edit_form.php
        +++ b/course/edit_form.php
        @@ -81,7 +81,7 @@ class course_edit_form extends moodleform {
                         $displaylist = coursecat::make_categories_list('moodle/course:create');
                         if (!isset($displaylist[$course->category])) {
                             //always keep current
        -                    $displaylist[$course->category] = coursecat::get($course->category)->get_formatted_name();
        +                    $displaylist[$course->category] = coursecat::get($course->category, MUST_EXIST, true)->get_formatted_name();
                         }
                         $mform->addElement('select', 'category', get_string('coursecategory'), $displaylist);
                         $mform->addHelpButton('category', 'coursecategory');
        
        Show
        Dan Poltawski added a comment - Looking (and not testing yet), this might be the fix: diff --git a/course/edit_form.php b/course/edit_form.php index 13ab045..c845331 100644 --- a/course/edit_form.php +++ b/course/edit_form.php @@ -81,7 +81,7 @@ class course_edit_form extends moodleform { $displaylist = coursecat::make_categories_list('moodle/course:create'); if (!isset($displaylist[$course->category])) { //always keep current - $displaylist[$course->category] = coursecat::get($course->category)->get_formatted_name(); + $displaylist[$course->category] = coursecat::get($course->category, MUST_EXIST, true )->get_formatted_name(); } $mform->addElement('select', 'category', get_string('coursecategory'), $displaylist); $mform->addHelpButton('category', 'coursecategory');
        Hide
        Marina Glancy added a comment -

        I agree this is a regression and I will submit a patch today. As a workaround without patching the code you can do the following:

        • Create a new role with just one capability to View hidden categories that can be applied to system or category context.
        • Grant this role to user(s) who teach courses that are inside the hidden category. (You can do it on system or on category level)
        Show
        Marina Glancy added a comment - I agree this is a regression and I will submit a patch today. As a workaround without patching the code you can do the following: Create a new role with just one capability to View hidden categories that can be applied to system or category context. Grant this role to user(s) who teach courses that are inside the hidden category. (You can do it on system or on category level)
        Hide
        Marina Glancy added a comment -

        This situation occurs when user has capability to change course category BUT does not have the capability to view the category this course is currently in (because it is hidden and user does not have capability to view hidden categories).

        So there is a policy question what to do:
        1. Allow user to change category (in this case Dan's patch works great)
        2. Do not allow user to change category and do not display the dropdown form element at all

        Dan Poltawski can you comment please

        Show
        Marina Glancy added a comment - This situation occurs when user has capability to change course category BUT does not have the capability to view the category this course is currently in (because it is hidden and user does not have capability to view hidden categories). So there is a policy question what to do: 1. Allow user to change category (in this case Dan's patch works great) 2. Do not allow user to change category and do not display the dropdown form element at all Dan Poltawski can you comment please
        Hide
        Dan Poltawski added a comment -

        As I just said in chat, 1. is my preferred solution, but then we break the mantra of actions always being reservable. But 2. is loosing functionality.

        Thus, I think that 1. is the only solution. (These category permission problems give us all nightmare).

        Show
        Dan Poltawski added a comment - As I just said in chat, 1. is my preferred solution, but then we break the mantra of actions always being reservable. But 2. is loosing functionality. Thus, I think that 1. is the only solution. (These category permission problems give us all nightmare).
        Hide
        Marina Glancy added a comment -

        ok. Submitting with #1 patch (even though I'd prefer #2)

        Show
        Marina Glancy added a comment - ok. Submitting with #1 patch (even though I'd prefer #2)
        Hide
        Sam Hemelryk added a comment -

        Thanks Marina this has been integrated now.

        Based purely upon the comments here solution 1 sounds like the better way to go presently, it fixes the regression caused by the category changes.

        However its really not the ideal solution, it breaks the undo mantra as Dan notes, and it also negates the capability controlling the ability to view hidden categories in this situation.
        I think at some point we need to make some decisions here, preferably favouring our well defined capability and access control and then properly document and adhere to theme. Perhaps configuration is the best way around this in the future.

        Show
        Sam Hemelryk added a comment - Thanks Marina this has been integrated now. Based purely upon the comments here solution 1 sounds like the better way to go presently, it fixes the regression caused by the category changes. However its really not the ideal solution, it breaks the undo mantra as Dan notes, and it also negates the capability controlling the ability to view hidden categories in this situation. I think at some point we need to make some decisions here, preferably favouring our well defined capability and access control and then properly document and adhere to theme. Perhaps configuration is the best way around this in the future.
        Hide
        Damyon Wiese added a comment -

        Test passed - I would have preferred solution 2 in this case though. This seems like you are moving the course into a category where you have more permissions.

        Show
        Damyon Wiese added a comment - Test passed - I would have preferred solution 2 in this case though. This seems like you are moving the course into a category where you have more permissions.
        Hide
        Marina Glancy added a comment -

        Thanks for your awesome work! This has now become a part of Moodle.

        Closing as fixed!

        Show
        Marina Glancy added a comment - Thanks for your awesome work! This has now become a part of Moodle. Closing as fixed!
        Hide
        David Millians added a comment -

        I realized I forgot to say, thanks to you all!

        Show
        David Millians added a comment - I realized I forgot to say, thanks to you all!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: