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

Cannot edit course settings if course is in hidden category.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5.1, FRONTEND
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide
      1. Create course category
      2. Create course in this category, enrol user as a teacher
      3. Hide this category (course will be hidden as well)
      4. Login as teacher, make sure you can edit this course AND move it to another category

      (Teacher won't be able to move it back though)

      Show
      Create course category Create course in this category, enrol user as a teacher Hide this category (course will be hidden as well) Login as teacher, make sure you can edit this course AND move it to another category (Teacher won't be able to move it back though)
    • Workaround:
      Hide

      Give those users capability to view hidden categories in system context or the particular category context

      Show
      Give those users capability to view hidden categories in system context or the particular category context
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
      wip-MDL-40088-master

      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!

        Gliffy Diagrams

          Activity

          Hide
          millia 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
          millia 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
          poltawski Dan Poltawski added a comment -

          Agreed this is a major regression.

          Show
          poltawski Dan Poltawski added a comment - Agreed this is a major regression.
          Hide
          poltawski 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
          poltawski 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 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 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 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 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
          poltawski 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
          poltawski 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 Marina Glancy added a comment -

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

          Show
          marina Marina Glancy added a comment - ok. Submitting with #1 patch (even though I'd prefer #2)
          Hide
          samhemelryk 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
          samhemelryk 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 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 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 Marina Glancy added a comment -

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

          Closing as fixed!

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

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

          Show
          millia 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:
                Fix Release Date:
                8/Jul/13