Moodle
  1. Moodle
  2. MDL-32931

Forum overview 'new posts' functionality returns incorrect results and is horribly slow

    Details

    • Database:
      Any, PostgreSQL
    • Testing Instructions:
      Hide
      1. As an admin create / go to a course and add 'UserA' as a student and 'UserB' as a teacher
      2. Login as UserA, go to "My moodle" page, enable the course overview block and check the number of new forum posts listed (if there are new forum posts)
      3. Visit the course as UserB and create a new forum
      4. Visit the "My Moodle" page as UserA and check that the number of new forum posts is the same
      5. Visit the course as UserB and create a new forum discussion
      6. Visit the "My Moodle" page as UserA and check that the number of new forum posts has been increased by one
      7. Visit the course as UserB and create a new forum post to the discussion
      8. Visit the "My Moodle" page as UserA and check that the number of new forum posts has been increased by one
      9. Visit the course as UserA, read the forum posts and logout
      10. Login again as UserA and visit the "My Moodle" page to check that there are no pending posts
      Show
      As an admin create / go to a course and add 'UserA' as a student and 'UserB' as a teacher Login as UserA, go to "My moodle" page, enable the course overview block and check the number of new forum posts listed (if there are new forum posts) Visit the course as UserB and create a new forum Visit the "My Moodle" page as UserA and check that the number of new forum posts is the same Visit the course as UserB and create a new forum discussion Visit the "My Moodle" page as UserA and check that the number of new forum posts has been increased by one Visit the course as UserB and create a new forum post to the discussion Visit the "My Moodle" page as UserA and check that the number of new forum posts has been increased by one Visit the course as UserA, read the forum posts and logout Login again as UserA and visit the "My Moodle" page to check that there are no pending posts
    • Workaround:
      Hide

      Avoid log table

      Show
      Avoid log table
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-32931_master
    • Rank:
      40010

      Description

      The forum overview used by the course overview block counts the number of new posts in forum discussions for courses in which the user is enrolled.

      This currently does not return the correct results; its search method is to scan the log table for 'add post' actions - however, posts can be added through other means without an 'add post' action being logged (for example, during the start of a new discussion, a new post is added but only the action 'add discussion' is logged).

      A side consequence of using the database log table for this work is that it can by abysmally slow given a large dataset in the log table. This is especially the case when scanning for new posts in a course which the user has never accessed ($course->lastaccess == 0) as this can then do a scan on the entirety of the log table for all posts since the beginning of the Moodle and can cause cache evictions from the database buffers for other, more useful data.

        Activity

        Tom Lanyon created issue -
        Hide
        Tom Lanyon added a comment -

        Patch to change forum overview to use the authoritative source of forum post information (forum, forum_post and forum_discussion tables) rather than the log table. Fixes course overview block to return accurate results and can significantly improve performance.

        Show
        Tom Lanyon added a comment - Patch to change forum overview to use the authoritative source of forum post information (forum, forum_post and forum_discussion tables) rather than the log table. Fixes course overview block to return accurate results and can significantly improve performance.
        Tom Lanyon made changes -
        Field Original Value New Value
        Pull Master Diff URL https://github.com/tomlanyon/moodle/compare/master...MDL-32931/master
        Pull Master Branch MDL-32931/master
        Pull 2.0 Diff URL https://github.com/tomlanyon/moodle/compare/MOODLE_20_STABLE...MDL-32931/MOODLE_20_STABLE
        Pull 2.0 Branch MDL-32931/MOODLE_20_STABLE
        Database Any,PostgreSQL [ 10000,10002 ]
        Pull 2.2 Diff URL https://github.com/tomlanyon/moodle/compare/MOODLE_22_STABLE...MDL-32931/MOODLE_22_STABLE
        Pull 2.1 Branch MDL-32931/MOODLE_21_STABLE
        Pull 2.2 Branch MDL-32931/MOODLE_22_STABLE
        Pull 2.1 Diff URL https://github.com/tomlanyon/moodle/compare/MOODLE_21_STABLE...MDL-32931/MOODLE_21_STABLE
        Pull from Repository git://github.com/tomlanyon/moodle.git
        Tom Lanyon made changes -
        Labels netspot partner performance
        Hide
        Michael de Raadt added a comment -

        Thanks for reporting that and providing a solution.

        It would help if you could provide a set of testing instructions for your solution.

        Show
        Michael de Raadt added a comment - Thanks for reporting that and providing a solution. It would help if you could provide a set of testing instructions for your solution.
        Michael de Raadt made changes -
        Fix Version/s STABLE backlog [ 10463 ]
        Labels netspot partner performance netspot partner patch triaged
        Hide
        Tom Lanyon added a comment -

        Added test instructions.

        Show
        Tom Lanyon added a comment - Added test instructions.
        Tom Lanyon made changes -
        Testing Instructions 1. Enable the course overview block and ensure it still appears to function (e.g. doesn't throw errors)
        2. Take a course that contains forum discussions and two users ('UserA' and 'UserB').
        3. Visit the course as UserA and check the number of new forum posts listed.
        4. Visit the course as UserB and create a new forum post in an existing discussion, or start a new discussion.
        5. Reload the course page as UserA and check that the number of new forum posts has increased.
        David Monllaó made changes -
        Assignee moodle.com [ moodle.com ] David Monllaó [ davmon ]
        Hide
        David Monllaó added a comment -

        Thanks for the patch Tom. Updating pull branches. Thanks DanP for the comments

        Show
        David Monllaó added a comment - Thanks for the patch Tom. Updating pull branches. Thanks DanP for the comments
        David Monllaó made changes -
        Pull Master Diff URL https://github.com/tomlanyon/moodle/compare/master...MDL-32931/master https://github.com/dmonllao/moodle/compare/master...MDL-32931_master
        Pull Master Branch MDL-32931/master MDL-32931_master
        Pull 2.3 Diff URL https://github.com/dmonllao/moodle/compare/MOODLE_23_STABLE...MDL-32931_23
        Pull 2.0 Diff URL https://github.com/tomlanyon/moodle/compare/MOODLE_20_STABLE...MDL-32931/MOODLE_20_STABLE
        Pull 2.0 Branch MDL-32931/MOODLE_20_STABLE
        Peer reviewer poltawski
        Testing Instructions 1. Enable the course overview block and ensure it still appears to function (e.g. doesn't throw errors)
        2. Take a course that contains forum discussions and two users ('UserA' and 'UserB').
        3. Visit the course as UserA and check the number of new forum posts listed.
        4. Visit the course as UserB and create a new forum post in an existing discussion, or start a new discussion.
        5. Reload the course page as UserA and check that the number of new forum posts has increased.
        # Enable the course overview block and ensure it still appears to function (e.g. doesn't throw errors)
        # Take a course that contains forum discussions and two users ('UserA' and 'UserB').
        # Visit the course as UserA and check the number of new forum posts listed.
        # Visit the course as UserB and create a new forum post in an existing discussion, or start a new discussion.
        # Reload the course page as UserA and check that the number of new forum posts has increased.
        Workaround Avoid log table
        Pull 2.2 Diff URL https://github.com/tomlanyon/moodle/compare/MOODLE_22_STABLE...MDL-32931/MOODLE_22_STABLE https://github.com/dmonllao/moodle/compare/MOODLE_22_STABLE...MDL-32931_22
        Pull 2.1 Branch MDL-32931/MOODLE_21_STABLE
        Pull 2.2 Branch MDL-32931/MOODLE_22_STABLE MDL-32931_22
        Pull 2.1 Diff URL https://github.com/tomlanyon/moodle/compare/MOODLE_21_STABLE...MDL-32931/MOODLE_21_STABLE
        Pull from Repository git://github.com/tomlanyon/moodle.git git://github.com/dmonllao/moodle.git
        Affects Version/s 2.3 [ 10657 ]
        Affects Version/s 2.4 [ 12255 ]
        Pull 2.3 Branch MDL-32931_23
        David Monllaó made changes -
        Status Open [ 1 ] Waiting for peer review [ 10012 ]
        moodle.com made changes -
        Fix Version/s STABLE Sprint 22 [ 12156 ]
        Fix Version/s STABLE backlog [ 10463 ]
        Dan Poltawski made changes -
        Original Estimate 0 minutes [ 0 ]
        Remaining Estimate 0 minutes [ 0 ]
        Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
        Hide
        Dan Poltawski added a comment -

        In general I think the approach seems to make sense. But you need to check if this change is cross-db compatible.

        It looks to me like it might break need both the count and forumid in the group by statement to work on all dbs.

        Show
        Dan Poltawski added a comment - In general I think the approach seems to make sense. But you need to check if this change is cross-db compatible. It looks to me like it might break need both the count and forumid in the group by statement to work on all dbs.
        Dan Poltawski made changes -
        Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
        Hide
        David Monllaó added a comment -

        Thanks for the comments Dan. Query tested with mysql, postgres, oracle and mssql

        Show
        David Monllaó added a comment - Thanks for the comments Dan. Query tested with mysql, postgres, oracle and mssql
        David Monllaó made changes -
        Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
        Dan Poltawski made changes -
        Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
        Hide
        Dan Poltawski added a comment -

        Hi David,

        It looks good for integration, the only comment is that it'd be much easier for us to integrate if could add more extensive testing instructions to cover a more detailed scenario to ensure we are not introducing regressions. (e.g. more users and discussions etc)

        Show
        Dan Poltawski added a comment - Hi David, It looks good for integration, the only comment is that it'd be much easier for us to integrate if could add more extensive testing instructions to cover a more detailed scenario to ensure we are not introducing regressions. (e.g. more users and discussions etc)
        Dan Poltawski made changes -
        Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
        Hide
        David Monllaó added a comment -

        Thanks Dan, adding more instructions and submitting for integration

        Show
        David Monllaó added a comment - Thanks Dan, adding more instructions and submitting for integration
        David Monllaó made changes -
        Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
        Testing Instructions # Enable the course overview block and ensure it still appears to function (e.g. doesn't throw errors)
        # Take a course that contains forum discussions and two users ('UserA' and 'UserB').
        # Visit the course as UserA and check the number of new forum posts listed.
        # Visit the course as UserB and create a new forum post in an existing discussion, or start a new discussion.
        # Reload the course page as UserA and check that the number of new forum posts has increased.
        # As an admin create / go to a course and add 'UserA' as a student and 'UserB' as a teacher
        # Login as UserA, go to "My moodle" page, enable the course overview block and check the number of new forum posts listed (if there are new forum posts)
        # Visit the course as UserB and create a new forum
        # Visit the "My Moodle" page as UserA and check that the number of new forum posts is the same
        # Visit the course as UserB and create a new forum discussion
        # Visit the "My Moodle" page as UserA and check that the number of new forum posts has been increased by one
        # Visit the course as UserB and create a new forum post to the discussion
        # Visit the "My Moodle" page as UserA and check that the number of new forum posts has been increased by one
        # Visit the course as UserA, read the forum posts and logout
        # Login again as UserA and visit the "My Moodle" page to check that there are no pending posts
        Hide
        Aparup Banerjee 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
        Aparup Banerjee 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
        David Monllaó added a comment -

        Pull branches rebased against upstream. Thanks

        Show
        David Monllaó added a comment - Pull branches rebased against upstream. Thanks
        Dan Poltawski made changes -
        Currently in integration Yes [ 10041 ]
        Sam Hemelryk made changes -
        Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
        Integrator samhemelryk
        Hide
        Sam Hemelryk added a comment -

        Thanks David, this has been integrated now.

        Show
        Sam Hemelryk added a comment - Thanks David, this has been integrated now.
        Sam Hemelryk made changes -
        Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
        Fix Version/s 2.2.5 [ 12352 ]
        Fix Version/s 2.3.2 [ 12353 ]
        Michael de Raadt made changes -
        Tester phalacee
        Jason Fowler made changes -
        Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
        Hide
        Jason Fowler added a comment -

        works fine David

        Show
        Jason Fowler added a comment - works fine David
        Jason Fowler made changes -
        Status Testing in progress [ 10011 ] Tested [ 10006 ]
        Hide
        Dan Poltawski added a comment -

        asko, Дзякуй, ধন্যবাদ, Благодаря, Gràcies, 感谢, 謝謝, Hvala, Díky, Tak, Bedankt, Tänan, متشکریم, Salamat, Kiitokset, Merci, Grazas, Danke, Ευχαριστώ, આભાર, תודה, धन्यवाद, Köszönjük, Takk fyrir, Terima Kasih, Grazie, ありがとうございます, Рахмет, សូមអរគុណ, 감사합니다, gratiās, Pateicamies, Ačiū, Благодарам, Tēnā koa, Kia Ora Rawa Atu, आभारी आहोत, Талархал, Takk, Dziękuję, Obrigado, Mulţumesc, Engraziel, Спасибо, Fa'afetai, Хвала, Hvala, ස්තූතියි, Vďaka, Hvala, Mahadsanid, Thanks, Gracias, Tack, Salamat, நன்றி, నెనరులు, ขอบคุณค่ะ!

        Your work has made it into this weeks Moodle release! There are no gold medals available this week - but millions around the world will benefit. Thank you!

        Show
        Dan Poltawski added a comment - asko, Дзякуй, ধন্যবাদ, Благодаря, Gràcies, 感谢, 謝謝, Hvala, Díky, Tak, Bedankt, Tänan, متشکریم, Salamat, Kiitokset, Merci, Grazas, Danke, Ευχαριστώ, આભાર, תודה, धन्यवाद, Köszönjük, Takk fyrir, Terima Kasih, Grazie, ありがとうございます, Рахмет, សូមអរគុណ, 감사합니다, gratiās, Pateicamies, Ačiū, Благодарам, Tēnā koa, Kia Ora Rawa Atu, आभारी आहोत, Талархал, Takk, Dziękuję, Obrigado, Mulţumesc, Engraziel, Спасибо, Fa'afetai, Хвала, Hvala, ස්තූතියි, Vďaka, Hvala, Mahadsanid, Thanks, Gracias, Tack, Salamat, நன்றி, నెనరులు, ขอบคุณค่ะ! Your work has made it into this weeks Moodle release! There are no gold medals available this week - but millions around the world will benefit. Thank you!
        Dan Poltawski made changes -
        Status Tested [ 10006 ] Closed [ 6 ]
        Resolution Fixed [ 1 ]
        Integration date 02/Aug/12
        Dan Poltawski made changes -
        Currently in integration Yes [ 10041 ]
        Eloy Lafuente (stronk7) made changes -
        Fix Version/s STABLE Sprint 22 [ 12156 ]

          People

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

            Dates

            • Created:
              Updated:
              Resolved: