'Count replies in each forum' repair tool breaks reply order (aka. Bug From Hell)

[netweb]

Replies are ordered based on the value in wp_posts.menu_order

A new reply is assigned its menu_order value with bbp_get_topic_reply_count( $topic_id, false ) + 1 in the function bbp_new_reply_handler (​/includes/replies/functions.php#L358)

Both repair tools "Count replies in each forum" and "Count replies in each topic" would delete all meta values _bbp_reply_count from wp_postmeta thus you could only ever have a _bbp_reply_count post meta value for either topics or forums, never both, dependant on the last tool to run.

If the "Count replies in each forum" was the last run repair tool (i.e. And/Or after the "Count replies in each topic" was performed) then trying to retrieve the bbp_get_topic_reply_count (​src) would result in a menu_order of 0 + 1 as the post meta key _bbp_reply_count was non existant for the topic being replied to in wp_postmeta.

The first part of this resolution in tools-fix.patch correctly repairs the repair tools to ensure that each of the above repair tools only deletes it's own forum or topic _bbp_reply_count values in wp_postmeta.

The second part of the resolution is to include a repair tool to repair any broken replies menu_order.

Immediate Workaround: Run the "Count replies in each topic" repair tool and DO NOT run the "Count replies in each forum" repair tool. This will ensure that _bbp_reply_count count is accurate for topics so new replies can use bbp_get_topic_reply_count( $topic_id, false ) + 1 to set the reply menu_order accurately.

[netweb]

The tools-fix.patch​ fixes the issue.

The tools-repair.patch​ is a 'work in progress' repair tool to repair replies affected and needs further testing 'to make it work across all permutations and combinations of reply order with or without threaded replies enabled' and refinements et cetera.

Potential Issues: The delete queries in tools-fix.patch​ are a little expensive and could do with breaking down the delete process into chunks.

[johnjamesjacoby]

The fixer patch can be done with a JOIN between the posts table and the postsmeta table, VS looping through each topic individually (which could result in hundreds of thousands of queries.

The repair patch is a great first stab, but it will kill any installation with more than a few replies as calls to bbp_get_all_child_ids() and wp_update_post() well get costly quickly.

One way to counteract this is to switch our tools to use bbPress's converter ajax, and provide a UI to watch the repairs take place.

[netweb]

I also had this is postmeta which is very inception like, each reply is a 'reply' to the other and will cause both replies to be the last two replies in a topic irrespective of their own or other replies menu_order in a topic.

meta_id	post_id	meta_key	meta_value
43736	42248	_bbp_reply_to	42250
43742	42250	_bbp_reply_to	42248

Not sure how this came to be, have not tried to reproduce this yet, putting it here and to think on this some more.

[johnjamesjacoby]

The good thing is we have plenty of meta to work with, should we decide to switch our approach later. The bad thing is we're currently stuck using meta for pretty much everything. :)

[netweb]

In tools-repair.2.patch instead of ​running bbp_update_reply_position on every reply it now only runs for each reply that has a duplicate menu_order for any single topic.

The query to return the above check: (With some extra 'select' fields for readability in phpMyAdmin)

SELECT a.ID, a.post_content, a.post_name, a.post_parent, a.menu_order, a.post_type
FROM wp_posts AS a
INNER JOIN( 
        SELECT menu_order, post_parent 
        FROM wp_posts 
        GROUP BY menu_order, post_parent 
        HAVING count( * ) > 1 
        ) AS b
ON a.menu_order = b.menu_order
AND a.post_parent = b.post_parent
WHERE post_type = 'reply'

I have an interesting sample wp_posts table with ~11,000 entries (11 Forums, 2,500 Topics & 5,000 replies)
These were generated by @MZAweb's ​bbpFauxData plugin (generates bbPress Forums, Topics & Replies) and when a reply is created with this the reply is not given a menu_order value when generated which works in our favor for testing in this ticket.

Showing rows 0 - 29 ( 4,287 total, Query took 0.0313 sec)

So the query is quite fast but as we have no way to determine how many replies may need repairing for bbPress installs 'out in the wild' thus the following still applies though somewhat mitigated:

The repair patch is a great first stab, but it will kill any installation with more than a few replies as calls to bbp_get_all_child_ids() and wp_update_post() well get costly quickly.

I repaired the above 4,287 replies menu_order in 6 passes, each pass ~750 before the PHP 30 second timeout. As stated we have no idea how many replies need to be repaired in the wild and should indeed do the following:

One way to counteract this is to switch our tools to use bbPress's converter ajax, and provide a UI to watch the repairs take place.

[netweb]

Replying to netweb:

I also had this is postmeta which is very inception like, each reply is a 'reply' to the other and will cause both replies to be the last two replies in a topic irrespective of their own or other replies menu_order in a topic.

meta_id	post_id	meta_key	meta_value
43736	42248	_bbp_reply_to	42250
43742	42250	_bbp_reply_to	42248

See #2588 for details and fix for this.

[ThemeTon]

Can we have simple replace files of that fixes instead of patch? I don't want to replace all those detailed function fixes manually.
Also this updates can be included on next versions of the plugin?

Thanks a lot

[netweb]

Replying to ThemeTon:

Can we have simple replace files of that fixes instead of patch? I don't want to replace all those detailed function fixes manually.

The final patch will be a repair tool to be included alongside bbPress' other repair tools.

That said the patch will once we have finalized it update the actual main files so there will not be a need to patch any files manually.

I would also not recommend trying to implement the patch in it's current state as it is pretty much guaranteed to not work for most sites affected. We are refactoring our repair tools to handle this.

Also this updates can be included on next versions of the plugin?

This will be included in the next bbPress release.

p.s "omg sweet tea" is a higher priority than "highest" ;)

[ThemeTon]

Replying to netweb:
Okay I see everything including "omg sweet tea" :)

So do you have any info about date of next version? When it will have? (approx or exactly)
I checked this, ​https://bbpress.trac.wordpress.org/milestone/2.5.4 and there only have completed percent of issues.

Thanks again

[johnjamesjacoby]

In 5367:

Update to tools:

• Update topic & reply count repair tools to only delete the appropriate meta keys from the appropriate parents.
• Include new reply hierarchy repair tool, to fix a possibly poisoned reply hierarchy.

Props netweb. See #2586 (2.5 branch)

[johnjamesjacoby]

In 5368:

Update to tools:

• Update topic & reply count repair tools to only delete the appropriate meta keys from the appropriate parents.
• Include new reply hierarchy repair tool, to fix a possibly poisoned reply hierarchy.

Props netweb. See #2586 (trunk)

[johnjamesjacoby]

In 5372:

Remove extraneous validation in bbp_get_form_reply_to(). Fixes bug causing _bbp_reply_to field to be incorrectly set as the current $reply_id when editing a reply with no reply_to, which can lead to hierarchy issues. See #2586, #2588. (trunk)

[johnjamesjacoby]

In 5373:

Remove extraneous validation in bbp_get_form_reply_to(). Fixes bug causing _bbp_reply_to field to be incorrectly set as the current $reply_id when editing a reply with no reply_to, which can lead to hierarchy issues. See #2586, #2588. (2.5 branch)

[johnjamesjacoby]

This should finally be fixed. Going to close it as such. Reopen if any other quirks pop up related to this, or create a new ticket and mention this one if any new issues arise.

Thanks everyone for being vigilant and persistent.