Moodle

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

Details

  • Database:
    PostgreSQL, Any
  • 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
  • Pull Master Diff URL:

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

Ascending order - Click to sort in descending order
Hide
Permalink
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.
Hide
Permalink
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.
Hide
Permalink
Tom Lanyon added a comment -

Added test instructions.

Show
Tom Lanyon added a comment - Added test instructions.
Hide
Permalink
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
Hide
Permalink
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.
Hide
Permalink
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
Hide
Permalink
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)
Hide
Permalink
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
Hide
Permalink
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
Permalink
David Monllaó added a comment -

Pull branches rebased against upstream. Thanks

Show
David Monllaó added a comment - Pull branches rebased against upstream. Thanks
Hide
Permalink
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.
Hide
Permalink
Jason Fowler added a comment -

works fine David

Show
Jason Fowler added a comment - works fine David
Hide
Permalink
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!

People

Vote (5)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: