Moodle
  1. Moodle
  2. MDL-33487

forum_tp_count_discussion_unread_posts() is broken - wrong parameter order

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.3
    • Fix Version/s: 2.1.7, 2.2.4
    • Component/s: Forum
    • Labels:
      None
    • Testing Instructions:
      Hide

      This isn't used in core code so doesn't really need testing through interface.

      To test for regressions, create a forum post, view the discussion and copy the discussion id from the url. Run the attached test.php with d param of the dicussion id.

      eg. test.php?d=2

      Show
      This isn't used in core code so doesn't really need testing through interface. To test for regressions, create a forum post, view the discussion and copy the discussion id from the url. Run the attached test.php with d param of the dicussion id. eg. test.php?d=2
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:

      Description

      In mod/forum/lib.php, there is a function called forum_tp_count_discussion_unread_posts() which returns the number of unread messages in a given discussion, for a given user.

      As far as I can tell, this function is not called anywhere in Moodle. But I call it from some custom navigational code (a button that jumps to "more unread messages" in whatever forum you are viewing.)

      The function prepares a database query, and calls count_records_sql(). The call includes the correct parameters, but they are in the wrong order (they don't match the sql query).

      Looking in CVS, I think the mistake crept in when the DB api changed, in revision 1.675 on 5 June 2008.

      It's an obvious, trivial fix.. the call to DB->count_records_sql has the discussionid and cutoffid parameters reversed. Can someone with CVS access fix this please?

      The broken code:

      function forum_tp_count_discussion_unread_posts($userid, $discussionid) {
        global $CFG, $DB;
        $cutoffdate = isset($CFG->forum_oldpostdays) ? (time() - ($CFG->forum_oldpostdays*24*60*60)) : 0;
       
        $sql = 'SELECT COUNT(p.id) '.
               'FROM {forum_posts} p '.
               'LEFT JOIN {forum_read} r ON r.postid = p.id AND r.userid = ? '.
               'WHERE p.discussion = ? '.
               'AND p.modified >= ? AND r.id is NULL';
       
        return $DB->count_records_sql($sql, array($userid, $cutoffdate, $discussionid));
      }

      To fix this, just reverse the $cutoffdata and $discussionid parameters in the count_records_sql call:

        return $DB->count_records_sql($sql, array($userid, $discussionid, $cutoffdate));

        Gliffy Diagrams

        1. test.php
          0.2 kB
          Dan Poltawski

          Activity

          Hide
          Dan Poltawski added a comment -

          Wow, good spotting. Thanks Garret!

          Show
          Dan Poltawski added a comment - Wow, good spotting. Thanks Garret!
          Hide
          Dan Poltawski added a comment -

          By the way, do you know git at all?

          Show
          Dan Poltawski added a comment - By the way, do you know git at all?
          Hide
          Sam Hemelryk added a comment -

          Good spotting and thanks for the fix, this has been integrated now

          Show
          Sam Hemelryk added a comment - Good spotting and thanks for the fix, this has been integrated now
          Hide
          Frédéric Massart added a comment -

          Successful on master, 2.2 and 2.1!

          Show
          Frédéric Massart added a comment - Successful on master, 2.2 and 2.1!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.

          Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.

          Many thanks for your collaboration!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3. Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish. Many thanks for your collaboration! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: