Moodle
  1. Moodle
  2. MDL-33933

Q and A forum doesn't allow Admin to view all posts without posting

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.2.6, 2.3.2
    • Component/s: Forum
    • Labels:
    • Testing Instructions:
      Hide
      1. Create a course with a teacher and non-editing teacher.
      2. Create a Q and A forum.
      3. Log in as a teacher and start a discussion.
      4. Log in as a student and make a post.
      5. Log in as the admin and view the posts.
      6. Verify that as the admin you can view the posts.
      Show
      Create a course with a teacher and non-editing teacher. Create a Q and A forum. Log in as a teacher and start a discussion. Log in as a student and make a post. Log in as the admin and view the posts. Verify that as the admin you can view the posts.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-33933-master

      Description

      Replication steps

      1. Make sure you have a course with a teacher and non-editing teacher in it.
      2. Create a forum.
      3. Set the forum type to "Q and A forum".
      4. Log in as a teacher and start a discussion.
      5. Log in as a student and make a post.
      6. Log in as the admin and view the posts.
        Expected result: The admin can see all posts.
        Actual result: The admin can only see the discussion topic.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Adam Barbary added a comment -

            I can confirm this for Moodle Moodle 2.2.3+ as well.

            Tried to view a post by student, had to switch to enrolled active teacher to view forum.

            Show
            Adam Barbary added a comment - I can confirm this for Moodle Moodle 2.2.3+ as well. Tried to view a post by student, had to switch to enrolled active teacher to view forum.
            Hide
            Jody Steele added a comment -

            This is caused by line 5363 in mod/forum/lib.php (HEAD at the time of posting):

            5363:  has_capability('mod/forum:viewqandawithoutposting', $modcontext, $user->id, false));
            

            The false shouldn't be there, as it indicates that the administrator role is ignored as per the documentation from has_capability
            $doanything If false, ignores effect of admin role assignment

            This appears to be caused by what looks like some SERIOUSLY legacy code being improperly migrated to the new version of has_capability.

            Prior to Mon, 14 Aug 2006 05:55:40 +0000 (05:55 +0000) the has_capability function was defined thusly

            has_capability($capability, $contextid=NULL, $kill=false, $userid=NULL)

            and afterwards it was defined

            has_capability($capability, $contextid=NULL, $userid=NULL)

            In mod/forum/lib.php, there was one call of has_capability that was not correctly updated (there were actually several, but only one is important to this discussion)

            -                has_capability('mod/forum:viewqandawithoutposting', $context->id, false, $user->id));
            +                has_capability('mod/forum:viewqandawithoutposting', $context, false, $user->id));
            

            Fast forward to Fri, 1 Sep 2006 09:25:34 +0000 (09:25 +0000) and has_capability was again changed this time to include the $doanything variable

            -function has_capability($capability, $context=NULL, $userid=NULL) {
            +function has_capability($capability, $context=NULL, $userid=NULL, $doanything='true') {
            

            Then, later on someone finally noticed that the capabilities were broken and "fixed" them on Wed, 27 Feb 2008 18:50:06 +0000 (18:50 +0000), causing this bug.

            -                has_capability('mod/forum:viewqandawithoutposting', $modcontext, false, $user->id));
            +                has_capability('mod/forum:viewqandawithoutposting', $modcontext, $user->id, false));
            

            Show
            Jody Steele added a comment - This is caused by line 5363 in mod/forum/lib.php (HEAD at the time of posting): 5363: has_capability('mod/forum:viewqandawithoutposting', $modcontext, $user->id, false)); The false shouldn't be there, as it indicates that the administrator role is ignored as per the documentation from has_capability $doanything If false, ignores effect of admin role assignment This appears to be caused by what looks like some SERIOUSLY legacy code being improperly migrated to the new version of has_capability . Prior to Mon, 14 Aug 2006 05:55:40 +0000 (05:55 +0000) the has_capability function was defined thusly has_capability($capability, $contextid=NULL, $kill=false, $userid=NULL) and afterwards it was defined has_capability($capability, $contextid=NULL, $userid=NULL) In mod/forum/lib.php , there was one call of has_capability that was not correctly updated (there were actually several, but only one is important to this discussion) - has_capability('mod/forum:viewqandawithoutposting', $context->id, false, $user->id)); + has_capability('mod/forum:viewqandawithoutposting', $context, false, $user->id)); Fast forward to Fri, 1 Sep 2006 09:25:34 +0000 (09:25 +0000) and has_capability was again changed this time to include the $doanything variable -function has_capability($capability, $context=NULL, $userid=NULL) { +function has_capability($capability, $context=NULL, $userid=NULL, $doanything='true') { Then, later on someone finally noticed that the capabilities were broken and "fixed" them on Wed, 27 Feb 2008 18:50:06 +0000 (18:50 +0000) , causing this bug. - has_capability('mod/forum:viewqandawithoutposting', $modcontext, false, $user->id)); + has_capability('mod/forum:viewqandawithoutposting', $modcontext, $user->id, false));
            Hide
            Charles Fulton added a comment -

            I've pushed this up my repo but the commit is attributed to Jody (great detective work!)

            Show
            Charles Fulton added a comment - I've pushed this up my repo but the commit is attributed to Jody (great detective work!)
            Hide
            Michael de Raadt added a comment -

            Hopefully we can get this peer reviewed soon.

            Show
            Michael de Raadt added a comment - Hopefully we can get this peer reviewed soon.
            Hide
            Andrew Davis added a comment -

            [Y] Syntax
            [Y] Output
            [Y] Whitespace
            [Y] Language
            [Y] Databases
            [Y] Testing
            [Y] Security
            [Y] Documentation
            [N] Git
            [Y] Sanity check

            Looks like a nice simple fix. The only minor thing is the component in the commit message. It should be mod_forum instead of forum. Submit for integration whenever you're ready. Let me know if you don't have the ability to submit stuff for integration and I'll do it for you.

            Show
            Andrew Davis added a comment - [Y] Syntax [Y] Output [Y] Whitespace [Y] Language [Y] Databases [Y] Testing [Y] Security [Y] Documentation [N] Git [Y] Sanity check Looks like a nice simple fix. The only minor thing is the component in the commit message. It should be mod_forum instead of forum. Submit for integration whenever you're ready. Let me know if you don't have the ability to submit stuff for integration and I'll do it for you.
            Hide
            Charles Fulton added a comment -

            Thanks Andrew, I've amended the commit message and rebased for good measure. I don't have the ability to submit for integration so I'd be grateful if you could do it. Thanks again.

            Show
            Charles Fulton added a comment - Thanks Andrew, I've amended the commit message and rebased for good measure. I don't have the ability to submit for integration so I'd be grateful if you could do it. Thanks again.
            Hide
            Andrew Davis added a comment -

            Submitting for integration.

            Show
            Andrew Davis added a comment - Submitting for integration.
            Hide
            Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            Dan Poltawski added a comment -

            Thanks everyone, i've integrated this and cherry-picked to 2.2 and 2.3

            Show
            Dan Poltawski added a comment - Thanks everyone, i've integrated this and cherry-picked to 2.2 and 2.3
            Hide
            Adrian Greeve added a comment -

            Tested on the 2.2, 2.3 and master integration branches.
            The admin can now see all the posts without posting in the Q and A forum.
            Test passed.

            Show
            Adrian Greeve added a comment - Tested on the 2.2, 2.3 and master integration branches. The admin can now see all the posts without posting in the Q and A forum. Test passed.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Closing as fixed, many thanks for your awesome collaboration.

            Show
            Eloy Lafuente (stronk7) added a comment - Closing as fixed, many thanks for your awesome collaboration.

              People

              • Votes:
                8 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: