Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#5296 closed enhancement (fixed)

Add tools for repairing relationships - Friend Counts

Reported by: cmmarslender Owned by: boonebgorges
Milestone: 2.0 Priority: normal
Severity: normal Version:
Component: Core Keywords: has-patch
Cc: chrismarslender@…

Description

It would be really handy to have tools (similar to bbPress) for repairing relationships related to BuddyPress. Recently needed to recount friends for each user after a migration, only to find there wasn't anything to do this, so here is a first pass at it. Tried to keep things similar to bbPress's implementation for simplicity.

https://github.com/cmmarslender/BuddyPress-Tools/

Attachments (3)

5296.patch (10.6 KB) - added by boonebgorges 6 years ago.
5296.02.patch (12.8 KB) - added by boonebgorges 6 years ago.
5296.suggestion.patch (13.9 KB) - added by imath 6 years ago.

Download all attachments as: .zip

Change History (24)

#1 @cmmarslender
6 years ago

  • Cc chrismarslender@… added

#2 follow-up: @boonebgorges
6 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 2.0

Totally awesome!

Couple thoughts:

  • There's no top-level Tools menu in Network Admin, so we have to decide what to do with the submenu when BP is network activated
  • I know that bbPress's migration tools have nice support for AJAX bulk processes. Maybe we could consider implementing this?
  • Let's go ahead and implement other cached counts (group members - are there others?)

#3 @r-a-y
6 years ago

  • Activity mentions count for user
  • Group count for user
  • Blog count for user
  • Total member count (uses a transient and is invalidated on various hooks)

#4 @netweb
6 years ago

  • Repair Imported Group Forum Statuses (Public, Private & Hidden Visibility)

Related: #bbPress2089

The bbPress1 importer doesn't check to see what the BP group status is when it imports the forum. So, the forum visibility for a private or hidden BP group forum will be imported as 'Public'.

The above ticket has been around for a while with no non-hacky way to do this in bbPress so essentially migrating that bbPress ticket to this BuddyPress ticket now. :)

I'll take a look at @cmmarslender's patch shortly and add to it.

At the same time we should also add some contextual help, see #bbPress1920 & attached 1920.diff for starters.

#5 follow-up: @netweb
6 years ago

We also need to cleanup wp_bp_xprofile_data after a user is deleted by bbPress 'Forum Reset' with the 'Delete Imported Users' as this does not touch any BuddyPress data (by design).

#6 in reply to: ↑ 5 ; follow-up: @boonebgorges
6 years ago

Replying to netweb:

We also need to cleanup wp_bp_xprofile_data after a user is deleted by bbPress 'Forum Reset' with the 'Delete Imported Users' as this does not touch any BuddyPress data (by design).

Yes, though the fact that it was deleted by bbPress is probably not relevant to the cleanup process. It sounds like we need (a) a cleanup task that checks for orphaned profile data, and (b) to find the bug (either in bbPress's "Delete Imported Users" or in BP's own xprofile cleanup routines) that is causing the data not to be deleted in the first place.

#7 in reply to: ↑ 6 @netweb
6 years ago

Replying to boonebgorges:

Yes, though the fact that it was deleted by bbPress is probably not relevant to the cleanup process. It sounds like we need (a) a cleanup task that checks for orphaned profile data,

Agreed, Yes, this is what I meant, we need a repair tool to do this.

and (b) to find the bug (either in bbPress's "Delete Imported Users" or in BP's own xprofile cleanup routines) that is causing the data not to be deleted in the first place.

Not a BP xprofile cleanup issue, 100% bbPress edge case issue as when we reset bbPress' forums and delete everything including users we do this directly using SQL DELETE statements and not wp_delete_user. The details on the implementation is here and JJJ's comment:

johnjamesjacoby That's an interesting problem with us doing direct queries. It's probably something BuddyPress should have built into itself eventually, and I'm fine not including BuddyPress specific direct queries in bbPress until then.

#8 follow-up: @boonebgorges
6 years ago

netweb - Excellent, thanks for the additional info. A bit of a side note to the current ticket, but I'm curious to hear why you decided to go with direct queries for the specific task of deleting users. Is it just to reduce overhead? Seems to me like a good long-term goal would be to rework the tools interface (maybe stealing from bbPress's great Import panel) to handle this kind of bulk task in chunks, via AJAX, rather than resorting to direct queries that are bound to bypass important hooks. But maybe there's another reason.

In any case, there's certainly a good argument that BP should have an orphan-xprofile-data cleaner anyway, so let's do it :)

#9 in reply to: ↑ 8 @netweb
6 years ago

Replying to boonebgorges:

I'm curious to hear why you decided to go with direct queries for the specific task of deleting users. Is it just to reduce overhead?

Primarily it was extending the current delete actions for forums, topics, replies, topic tags that were already using SQL to delete everything cause 'Mass Hysteria' [bbPress5175].

Seems to me like a good long-term goal would be to rework the tools interface (maybe stealing from bbPress's great Import panel) to handle this kind of bulk task in chunks, via AJAX, rather than resorting to direct queries that are bound to bypass important hooks. But maybe there's another reason.

No other reason and indeed this has merit and a future goal should be to rewrite the tools to do this.

#10 in reply to: ↑ 2 @netweb
6 years ago

Replying to boonebgorges:

  • There's no top-level Tools menu in Network Admin, so we have to decide what to do with the submenu when BP is network activated

What about as a new tab on the BuddyPress settings page?

That then begs the question should it just be there in the first place then?

This ticket was mentioned in IRC in #buddypress-dev by netweb. View the logs.


6 years ago

#12 @boonebgorges
6 years ago

See also #5128. Assuming we move 'last_activity' usermeta in the way proposed there, we should have a tool for re-running the migration that should auto-trigger during update.

#13 @boonebgorges
6 years ago

5296.patch is an update of cmmarslender's excellent first patch with the following modifications:

  • Moved to a Tools subtab of the BuddyPress admin pages
  • Replaced some direct SQL queries/calls to update_user_meta with existing API functions
  • Lightened the load of some count queries by avoiding get_users(). We don't currently add 0 values for total_group_count and total_friend_count in most cases, so there's no real point to setting those values for users who have no counts. So I've taken a quick-and-dirty approach to ensuring that we only update values for users who will have a value.
  • Removed total_blog_count and activity_mention count tools. As cmmarslender's inline note said, we don't really have anything to count for activity mentions. As for blog counts, we don't store this in usermeta anyway. (We do cache it using wp_cache_set(), but I wasn't sure if it was worth creating a tool just to fix this).

I think this is ready to commit and then iterate on (and to add more tasks to) but I would first like another set of eyes on it. Thanks.

@boonebgorges
6 years ago

#14 @johnjamesjacoby
6 years ago

Moved to a Tools subtab of the BuddyPress admin pages

I'd rather we add a Tools menu to the Network admin than commingle settings and utilities together.

Replaced some direct SQL queries/calls to update_user_meta with existing API functions

The reason bbPress did direct queries was to provide primitive database manipulation tools with a UI, to fix only what we intended to fix, without bumping/breaking anything else. Some of bbPress's meta functions were in flux at the time.

We'll need to hook bp_admin_repair_handler to something more specific than bp_core_admin_hook() as it's causing nonce issues with every other settings page.

Last edited 6 years ago by johnjamesjacoby (previous) (diff)

#15 @boonebgorges
6 years ago

Thanks for the feedback.

I'd rather we add a Tools menu to the Network admin than commingle settings and utilities together.

I'm not sure I'm totally onboard with the distinction, but I don't feel strongly about it. I can make the change.

Some of bbPress's meta functions were in flux at the time.

Yup. Wasn't criticizing, just saying that it worked better for BP's purpose to do this than to use the method that was just copied from bbPress.

We'll need to hook bp_admin_repair_handler to something more specific than bp_core_admin_hook() as it's causing nonce issues with every other settings page.

Oops, sorry about that, my bad. I'll adjust it so it does a better job of looking for the correct POST payload.

#16 follow-up: @boonebgorges
6 years ago

5296.02.patch updates the following:

  • Fix the problem with POST requests
  • Remove the Tools tab from BuddyPress Settings
  • On bp_core_do_network_admin(), register a top-level Tools menu. Create an Available Tools submenu page to serve as the default. Because I don't really have any content to put there, I chose to loop through the submenu items registered under the Tools menu and print a list of links. We could simply default to the BuddyPress Tools submenu, though this might cause problems if other plugins decide to start using our network Tools menu as a parent.
  • Moves the new tools panel to under Tools top level menu

I'd like feedback on the Network Admin approach if anyone has a few minutes.

#17 in reply to: ↑ 16 @imath
6 years ago

Replying to boonebgorges:

I'd like feedback on the Network Admin approach if anyone has a few minutes.

Hi, this is my feedback :)

On one hand, i agree that, on multisite, the best place for the "BuddyPress tools" is a submenu of a network tools menu.
On the other hand, WordPress doesn't provide this menu. The problem is what happens if another plugin register such a menu / submenu feature for its needs ? We'll have two network tools menu ?
I've watched tools.php on single config, there's a hook called 'tool_box', in an ideal world, i think WordPress should create a network-tools page with a specific hook let's say 'network_tool_box'. Then, as WordPress seems to not need any tool in this area, it could check if the net 'network_tool_box' has_action to display in case of false a message like no tools available. Else we would have the list of available tools generated by plugins....

This brings me to my suggestion about the network, i think the "BuddyPress" available tools on multisite config is a bit "straight". I think if we are to do this, we should respect the markup of the tools.php page.
Then we could create a specific hook let's say 'bp_network_tool_box' into the function bp_core_admin_available_tools_page() in order to use it for our need and give an opportunity to plugin developers to also use it in case they need to.
Finally, we could use the hooks 'tool_box' and 'bp_network_tool_box' to display an introduction of the BuddyPress tools in the same way WordPress does it for "Press this" & "Categories and Tags Converter". This introduction would then show on the "regular" and multisite tools pages.

I'm not sure to be clear above, i'm attaching a "suggestion" patch 5296.suggestion.patch that illustrates my feedback ;)

#18 @boonebgorges
6 years ago

I like your suggestion, imath. There's nothing we can do about another plugin trying to register its own Tools menu, of course, but hopefully this won't come up.

#19 @boonebgorges
6 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 8117:

Introduce BuddyPress Tools panel

Modeled on the bbPress panel, the BuddyPress Tools panel contains a number of
tools for repairing BuddyPress data. In this first iteration, the following
actions are supported:

  • Recalculate total_friend_count for each user
  • Recalculate total_group_count for each user
  • Recalculate total site members

On non-Multisite (or when BP is only activated on a single site), the menu
appears as Tools > BuddyPress. When BP is network-activated, we create a stub
Tools menu on the Network Admin to account for the fact that one is not
normally there. If WP ever introduces a menu like this, we can use it instead.

Fixes #5296

Props cmmarslender, boonebgorges, imath

#20 @boonebgorges
6 years ago

Thanks for your work on this ticket, everyone! I'm going to open separate tickets for the creation of one or two more tools for 2.0.

This ticket was mentioned in IRC in #buddypress-dev by netweb. View the logs.


6 years ago

Note: See TracTickets for help on using tickets.