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

      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));
      
      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: