Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-33487

forum_tp_count_discussion_unread_posts() is broken - wrong parameter order

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

          Attachments

            Activity

            Hide
            poltawski Dan Poltawski added a comment -

            Wow, good spotting. Thanks Garret!

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

            By the way, do you know git at all?

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

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

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

            Successful on master, 2.2 and 2.1!

            Show
            fred Frédéric Massart added a comment - Successful on master, 2.2 and 2.1!
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  9/Jul/12