refactor the menu_order tool ("Recalculate the position of each reply")

[tharsheblows]

This refers to #2654 and is about refactoring the menu_order tool here: ​https://bbpress.trac.wordpress.org/browser/branches/2.5/includes/admin/tools.php#L1144

I was working on this this morning and really just need a place to keep my results and (ideally) get feedback or told that I'm doing it all wrong which, honestly, would be super helpful as well.

Currently I have the following times from my last run:

-- the query with LIMIT 500 replies average time: 0.1032 s
-- bbp_update_reply_position( $reply->ID ) average time for each call: 0.01375 s <-- this of course varies with the number of replies to the topic, the std deviation is 0.004111s so quite large and I'm only doing a sample, not sure if I'm catching the really long topics

I am doing a max of 2500 replies (500 at a time) in total right now so the page doesn't time out, otherwise it does -- I haven't fine tuned this though, so I'm not exactly sure at what point it would. Definitely 10,000 replies is too much and 5,000 too, I think.

My first inclination would be to do this in chunks and have the page reload between each chunk, putting the offset in a POST variable and looking for it on the tool page load. Is there a better way? It should also be the case that if you choose to use that tool, that it's the *only* tool you can use but that's straightforward jQuery stuff.

It should be done so that the vast majority of the sites don't get the chunked version, I think that'd be around 1000 replies / around 15 seconds for me. I have no idea how different servers vary or if that's reasonable.

I also had a slightly different way of doing this and will see if I can get the update time down a little.

[tharsheblows]

It would help to know the wp_posts table has about 65k entries for the above numbers, wouldn't it.

Here is the current code for reference:

/**
 * Recalculate reply menu order
 *
 * @since bbPress (r5367)
 *
 * @uses wpdb::query() To run our recount sql queries
 * @uses is_wp_error() To check if the executed query returned {@link WP_Error}
 * @uses bbp_get_reply_post_type() To get the reply post type
 * @uses bbp_update_reply_position() To update the reply position
 * @return array An array of the status code and the message
 */
function bbp_admin_repair_reply_menu_order() {
	global $wpdb;

	$statement = __( 'Recalculating reply menu order … %s', 'bbpress' );
	$result    = __( 'No reply positions to recalculate!',         'bbpress' );

	// Delete cases where `_bbp_reply_to` was accidentally set to itself
	if ( is_wp_error( $wpdb->query( "DELETE FROM `{$wpdb->postmeta}` WHERE `meta_key` = '_bbp_reply_to' AND `post_id` = `meta_value`;" ) ) ) {
		return array( 1, sprintf( $statement, $result ) );
	}

	// Post type
	$rpt = bbp_get_reply_post_type();

	// Get an array of reply id's to update the menu oder for each reply
	$replies = $wpdb->get_results( "SELECT `a`.`ID` FROM `{$wpdb->posts}` AS `a`
										INNER JOIN (
											SELECT `menu_order`, `post_parent`
											FROM `{$wpdb->posts}`
											GROUP BY `menu_order`, `post_parent`
											HAVING COUNT( * ) >1
										)`b`
										ON `a`.`menu_order` = `b`.`menu_order`
										AND `a`.`post_parent` = `b`.`post_parent`
										WHERE `post_type` = '{$rpt}';", OBJECT_K );

	// Bail if no replies returned
	if ( empty( $replies ) ) {
		return array( 1, sprintf( $statement, $result ) );
	}

	// Recalculate the menu order position for each reply
	foreach ( $replies as $reply ) {
		bbp_update_reply_position( $reply->ID );
	}

	// Cleanup
	unset( $replies, $reply );

	// Flush the cache; things are about to get ugly.
	wp_cache_flush();

	return array( 0, sprintf( $statement, __( 'Complete!', 'bbpress' ) ) );
}

[netweb]

Let's move this to 2.7 and rework the repair tools and borrow from bbPress's import/converter panel to handle these tasks and loop through manageable size chunks.

[tharsheblows]

This is what I've been doing. It seems to work although not been tested thoroughly and will time out as discussed. I've just gone ahead and left in all the little timing things because I think they're interesting.

[tharsheblows]

cycles through the function, although incomplete

[tharsheblows]

with working ajax (not yet recursive) and tools.js file - still in progress

[netweb]

Related: #2841

[tharsheblows]

It can only handle about 500 at a time but I just noticed it was ordering by ID rather than post_date in bbp_get_child_ids and that was causing issues. It needs to order by post_date but that's not an index. I'm not great at this sort of thing but could you change the query slightly and use FORCE INDEX type_status_date? It's WHERE post_type=reply AND status=publish ORDER BY post_date? Would that help? Does it matter? I have a feeling some of the larger sites who have never fixed this will have topics with thousands of replies and this will get all of them. What do you think?

Also this is just where I am. I think it's working now though, it'd be great if someone could give it a go -- please imagine it much much prettier with no alerts and lovely notices and the like. This is just for functionality.

[tharsheblows]

latest version (I think)

[netweb]

1st pass of unit test for test_bbp_admin_repair_reply_menu_order()

[netweb]

The patch ​2731-tests.diff is a first pass at verifying bbp_admin_repair_reply_menu_order() repairs reply order correctly.

Note: It doesn't account for the proposed changes in 2731.3.diff

[tharsheblows]

Thoughts on bbp_get_child_ids:

• it's stupid to change bbp_get_child_ids to use post_date for everything, it'd make more to pass something through to change the ORDER BY. That way almost everything could stay the same.
• Also really bad idea for index. It'll be fine, it'll be fine.
• The flush cache is in the for testing. Take it out.
• how long will a super long topic take? 10k replies.

Thoughts on the tool

• one way to mitigate a long bbp_get_child_ids query is to have the function cycle by time. Is this is bad idea?
• where do the responses and cancel button go? I was just going to put it in the td
• it needs a cancel button, an actual button or link
• the cancel button needs to call a kill process. It need to do something that will kill the tool. So maybe a value in wp_options or the cache (is this reliable?) or something that the tool checks each time it runs.

I'm sure there's some oh don't do that it's a really bad idea stuff above, when you notice it, please tell me.

[tharsheblows]

• only do published replies in bbp_get_child_ids when updating menu_order (it's currently doing spam / pending and trash too)

Also

• on pending (not published) -> publish and spam (hmmm) -> publish, re-do menu order at least for the posts after the changed post. Probably for trash too just in case. You'd have to do it the other way too, publish -> pending etc etc

[tharsheblows]

in dire need of docs and a good clean but getting there

[tharsheblows]

with old function for backwards compatibility

[tharsheblows]

more backwards compat, disabled checkbox (for now), slightly saner

[tharsheblows]

Working (but not styled) version of js. The checkbox is disabled, only the run link can be used at the moment. If there is a php only version, then people will need to be strongly encouraged to use the js version as it'll be better. (Do we absolutely need a php-only version?)

PLEASE NOTE: the iterations are alerts. You can kill the process by clicking the "kill process" link but still, the alerts are annoying. I just haven't styled the notifications yet which will *not* be alerts but something much nicer. :)

[tharsheblows]

fixes edge case in bbp_get_reply_positions_published for posts with non-unique post_date (phpunit tests should now work correctly with this)

also lower limit required when using bbp_db()

[tharsheblows]

fixes sort if post_date not unique, lowers limit

[tharsheblows]

with a non-js fallback and the checkbox usable (only for testing, this tool is much much better run with js)

[tharsheblows]

with non-js fallback

[netweb]

#3121 was marked as a duplicate

[johnjamesjacoby]

Along with this, is the idea of removing the menu_order usage entirely (again) and simply counting on the ID or post_date columns to determine the order organically.

menu_order was used to help get accurate pagination counts for private/hidden statuses, but it may not be necessary anymore since recent advancements went into performance improvements in our queries.

This is worth looking early in 2.7, IMO.