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
    • Rank:
      42039

      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.

        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: