Moodle
  1. Moodle
  2. MDL-13900

Course-access partially not working correctly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 1.9.1
    • Component/s: Course
    • Labels:
      None
    • Environment:
      MySQL 5.0.45
      PHP 5.2.5
      Linux webhosting with apache
    • Database:
      MySQL
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      30429

      Description

      Hello,

      after upgrading from 1.8.4+ to final 1.9 I have the following problem:
      I am teacher of many courses on our Moodle. Most of them I can access without any problem (visible and hidden courses).

      But there is one special course, category is hidden, course is hidden, password is set, and I am teacher in this course. Every time I try to access this course, I get the message, that login for participants is not possible right now.
      After rechecking, deleting me as teacher and re-adding myself again, no change.

      This course is not shown up in the my-moodle and also cannot be accessed directly by using course-id.

      As administrator I can access this course without any problem.

      Regards, Daniel

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Daniel,

          to be able to see one course, this must be fulfilled AFAIK:

          • course is visible OR user has the "viewhiddencourses" permission (capability) in the course.

          AND

          • category is visible OR user has the "viewhiddencourses" permission (capability) in the category.

          That way, administrators can hide effectively categories and courses to everybody (teachers in course included). To allow teachers to see courses within a hidden category, the "viewhiddencourses" permission must be grated at category level to teachers.

          So, I close this thinking it's not a bug. Feel free to comment / reopen if necessary. Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Daniel, to be able to see one course, this must be fulfilled AFAIK: course is visible OR user has the "viewhiddencourses" permission (capability) in the course. AND category is visible OR user has the "viewhiddencourses" permission (capability) in the category. That way, administrators can hide effectively categories and courses to everybody (teachers in course included). To allow teachers to see courses within a hidden category, the "viewhiddencourses" permission must be grated at category level to teachers. So, I close this thinking it's not a bug. Feel free to comment / reopen if necessary. Ciao
          Hide
          Daniel Schimrik added a comment -

          Hello Eloy,

          I cannot agree with your statement. With default role settings it was with 1.8.x possible to view hidden courses in hidden categories if you are a trainer in this course. Independent from the setting viewhiddencourse.

          You wrote that "administrators can hide effectively categories and courses to everybody (teachers in course included)". In my opinion it makes no sense to hide a course from trainer of that course. The trainer should always see his courses he is trainer in.

          Currently I have a nearly identical problem:
          Under 1.9 I created a new visible category. Inside this category I created a new visible course and assigned a trainer to this course. But: This trainer cannot see this course under mymoodle. The same with students in this course.
          If they go by mydomain.tdl/course or directly by using the course-id, they can access this course.

          So as far as I can see this is a bug in 1.9.

          Today I will do an upgrade to the todays' version of 1.9+ and see if something changes on this topc.

          regards, Daniel

          Show
          Daniel Schimrik added a comment - Hello Eloy, I cannot agree with your statement. With default role settings it was with 1.8.x possible to view hidden courses in hidden categories if you are a trainer in this course. Independent from the setting viewhiddencourse. You wrote that "administrators can hide effectively categories and courses to everybody (teachers in course included)". In my opinion it makes no sense to hide a course from trainer of that course. The trainer should always see his courses he is trainer in. Currently I have a nearly identical problem: Under 1.9 I created a new visible category. Inside this category I created a new visible course and assigned a trainer to this course. But: This trainer cannot see this course under mymoodle. The same with students in this course. If they go by mydomain.tdl/course or directly by using the course-id, they can access this course. So as far as I can see this is a bug in 1.9. Today I will do an upgrade to the todays' version of 1.9+ and see if something changes on this topc. regards, Daniel
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Daniel,

          Thanks for your feedback, I propose you to keep things separated:

          1) About the "mymoodle" bug (that you have reported in MDL-14118), let's continue working on it there. Oki?

          2) About this report "teachers cannot see invisible courses under invisible categories"... lets reopen it and add more people to see the final behavior. My comment above was about HOW it's now performed by code and it seems that, on purpose, that's the behaviour under Moodle 1.9 and that's why I closed as not a bug (the solution is to give viewhidden permisions in the category level).

          But I reopen this for further discussion.... hope it helps!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Daniel, Thanks for your feedback, I propose you to keep things separated: 1) About the "mymoodle" bug (that you have reported in MDL-14118 ), let's continue working on it there. Oki? 2) About this report "teachers cannot see invisible courses under invisible categories"... lets reopen it and add more people to see the final behavior. My comment above was about HOW it's now performed by code and it seems that, on purpose, that's the behaviour under Moodle 1.9 and that's why I closed as not a bug (the solution is to give viewhidden permisions in the category level). But I reopen this for further discussion.... hope it helps! Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Assigning this to Petr, he's working in the mymoodle page these days....

          Thanks for the report, Daniel!

          Show
          Eloy Lafuente (stronk7) added a comment - Assigning this to Petr, he's working in the mymoodle page these days.... Thanks for the report, Daniel!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          grrr, undoing my latest edit... I was editing another bug... sorry.

          Show
          Eloy Lafuente (stronk7) added a comment - grrr, undoing my latest edit... I was editing another bug... sorry.
          Hide
          Martin Dougiamas added a comment - - edited

          Seems to be a logic change from ML in some of the optimisation work for 1.9. I don't think it looks right - so this IS is a bug IMO.

          lib/moodlelib.php:

          // Spaghetti logic construct
          //
          // - able to view course?
          // - able to view category?
          // => if either is missing, course is hidden from this user
          //
          // It's carefully ordered so we run the cheap checks first, and the
          // more costly checks last...
          //
          if (! (($COURSE->visible || has_capability('moodle/course:viewhiddencourses', $COURSE->context))
          && (course_parent_visible($COURSE)) || has_capability('moodle/course:viewhiddencourses',
          get_context_instance(CONTEXT_COURSECAT,
          $COURSE->category))))

          { print_header_simple(); notice(get_string('coursehidden'), $CFG->wwwroot .'/'); }

          I think the last two are supposed to be bracketed together:

          if (! (($COURSE->visible || has_capability('moodle/course:viewhiddencourses', $COURSE->context))
          &&
          ( course_parent_visible($COURSE) || has_capability('moodle/course:viewhiddencourses',
          get_context_instance(CONTEXT_COURSECAT,
          $COURSE->category))))) { print_header_simple(); notice(get_string('coursehidden'), $CFG->wwwroot .'/'); }
          Show
          Martin Dougiamas added a comment - - edited Seems to be a logic change from ML in some of the optimisation work for 1.9. I don't think it looks right - so this IS is a bug IMO. lib/moodlelib.php: // Spaghetti logic construct // // - able to view course? // - able to view category? // => if either is missing, course is hidden from this user // // It's carefully ordered so we run the cheap checks first, and the // more costly checks last... // if (! (($COURSE->visible || has_capability('moodle/course:viewhiddencourses', $COURSE->context)) && (course_parent_visible($COURSE)) || has_capability('moodle/course:viewhiddencourses', get_context_instance(CONTEXT_COURSECAT, $COURSE->category)))) { print_header_simple(); notice(get_string('coursehidden'), $CFG->wwwroot .'/'); } I think the last two are supposed to be bracketed together: if (! (($COURSE->visible || has_capability('moodle/course:viewhiddencourses', $COURSE->context)) && ( course_parent_visible($COURSE) || has_capability('moodle/course:viewhiddencourses', get_context_instance(CONTEXT_COURSECAT, $COURSE->category))))) { print_header_simple(); notice(get_string('coursehidden'), $CFG->wwwroot .'/'); }
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Martin,

          or am I getting blind... or you haven't changed anything in your "bracketed" version above. hehe.

          Anyway... apart from those brackets... I wan't to know if the logic is correct or no:

          Right now, under 1.9, (with brackets in place) the logic is the one I explained some posts above, i.e.:

          "User only can access to course if he's able to view the course AND the parent category" (where they can view the course or category because it's visible OR because they have the "viewhiddencourses" on each context).

          And BOTH (AND) conditions (on course and category) are required right now. The question is if that DOUBLE condition is ok or no.

          Under 1.8 it was enough to have the "viewhiddencourses" at course level (teachers have it by default) to be able to see the course, no matter if the course or the category is hidden or no.

          So, the key seems to be what behaviour is the one to apply to 1.9... apart from the brackets above that seem to be a bug, yup.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Martin, or am I getting blind... or you haven't changed anything in your "bracketed" version above. hehe. Anyway... apart from those brackets... I wan't to know if the logic is correct or no: Right now, under 1.9, (with brackets in place) the logic is the one I explained some posts above, i.e.: "User only can access to course if he's able to view the course AND the parent category" (where they can view the course or category because it's visible OR because they have the "viewhiddencourses" on each context). And BOTH (AND) conditions (on course and category) are required right now. The question is if that DOUBLE condition is ok or no. Under 1.8 it was enough to have the "viewhiddencourses" at course level (teachers have it by default) to be able to see the course, no matter if the course or the category is hidden or no. So, the key seems to be what behaviour is the one to apply to 1.9... apart from the brackets above that seem to be a bug, yup. Ciao
          Hide
          Martin Dougiamas added a comment -

          Yes I see what you mean now, I was seeing the category has_capability as an extra chance rather than an extra restriction.

          I'd agree with the 1.8 logic... there's no need to check for has_capability on the cat level. If it's assigned at that context it'll be picked up on the course level check anyway.

          So I guess :
          if ( !($COURSE->visible && course_parent_visible($COURSE)) &&
          !has_capability('moodle/course:viewhiddencourses', $COURSE->context))

          { notice }

          if that looks OK to you please check it in. I tried reordering the logic a bit but couldn't see anything clearly faster.

          Hmm, get_courses_page seems to retain that old logic too. (no check on the cat)

          Show
          Martin Dougiamas added a comment - Yes I see what you mean now, I was seeing the category has_capability as an extra chance rather than an extra restriction. I'd agree with the 1.8 logic... there's no need to check for has_capability on the cat level. If it's assigned at that context it'll be picked up on the course level check anyway. So I guess : if ( !($COURSE->visible && course_parent_visible($COURSE)) && !has_capability('moodle/course:viewhiddencourses', $COURSE->context)) { notice } if that looks OK to you please check it in. I tried reordering the logic a bit but couldn't see anything clearly faster. Hmm, get_courses_page seems to retain that old logic too. (no check on the cat)
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Change applied, now behaviour should be the same than in Moodle 1.8.

          Any inconsistency about this will be really welcome (in different lists of courses like mymoodle, main page...)

          Thanks for the report, Daniel!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Change applied, now behaviour should be the same than in Moodle 1.8. Any inconsistency about this will be really welcome (in different lists of courses like mymoodle, main page...) Thanks for the report, Daniel! Ciao
          Hide
          Petr Škoda added a comment -

          reviewed closing

          Show
          Petr Škoda added a comment - reviewed closing
          Hide
          Brett Hinton added a comment -

          This issue should be reopened or I can open a new one if needed, but the behavior is repeated in the main page "My Courses" list and in the list of courses on the user profile screen. This is evident in a client who is running 1.9.1+

          Show
          Brett Hinton added a comment - This issue should be reopened or I can open a new one if needed, but the behavior is repeated in the main page "My Courses" list and in the list of courses on the user profile screen. This is evident in a client who is running 1.9.1+
          Hide
          Ann Adamcik added a comment -

          Brett, see MDL-14478.

          Show
          Ann Adamcik added a comment - Brett, see MDL-14478 .

            People

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

              Dates

              • Created:
                Updated:
                Resolved: