Skip to:
Content

Opened 3 years ago

Closed 2 years ago

Last modified 3 weeks ago

#2959 closed task (fixed)

Favorites & Subscriptions Roadmap

Reported by: johnjamesjacoby Owned by:
Milestone: 2.6 Priority: high
Severity: major Version: 2.0
Component: API - Subscriptions Keywords: needs-patch needs-testing
Cc: jmdodd@…

Description (last modified by johnjamesjacoby)

With the introduction (and subsequent reinvention) of the per-forum moderator feature for bbPress 2.6, there is opportunity to use this new set of APIs to improve Subscriptions & Favorites.

One problem is favorites & subscriptions are currently stored per-user, serialized in wp_usermeta. The reasoning for this originally, was that this meta-data was specific enough to the user that it was okay to keep there.

Another problem is querying for which user IDs are subscribed to which forums or topics requires a specific FIND_IN_SET MySQL flag, which ends up transversing those serialized wp_usermeta values looking for post IDs.

This logic fails hard with Notifications: when replying to a topic or creating a new topic in a forum, querying for all subscribed users to notify them via email, is a query that does not perform very well when there are millions of rows in wp_usermeta.

So we have two problems that are very uncharacteristic of bbPress's heritage: performance, simplicity.


How to *properly* store these connections inside of WordPress was less obvious in 2010 than it is today.

Using the new per-forum moderator approach as the foundation, the future of Subscriptions & Favorites should be storing a per-feature meta-key in postmeta with a meta-value of the $user_id, like:

add_post_meta( $post_id, '_bbp_subscriber_id', $user_id );

Notice we use add_post_meta() which allows for duplicate meta-keys for the same post (vs. update_post_meta() which will update existing data.)

This allows for easy two-way querying:

Get subscribers for a topic:

get_post_meta( $post_id, '_bbp_subscriber_id', false );

Get subscribed topics for a user:

$topics = get_posts( array(
	'post_type'   => bbp_get_topic_post_type(),
	'meta_key'    => '_bbp_subscriber_id',
	'meta_type'   => 'NUMERIC',
	'meta_value'  => $user_id,
	'numberposts' => -1
) );

Notice that using this approach, we're able to use built in core functions in both directions. Further optimization can be achieved by writing some type of "meta-only" query class that does not require the joining of the wp_posts table, though I believe that is outside of the scope of this first pass.


How do we codify this quickly:

  • Abstract the way user IDs are added to meta-data tables out from per-forum moderators. This should internally allow for hot-wapping the object-type from Posts to Taxonomy Terms.
  • Update the per-forum moderator functions to use this new abstraction
  • Update the Subscription helper & wrapper functions to use this
  • Update the Favorites helper & wrapper functions to use this
  • Build an upgrader to migrate existing wp_usermeta data into the appropriate meta-data tables (posts for now, taxonomy term meta for forums later)

Attachments (3)

2959.diff (8.1 KB) - added by jmdodd 3 years ago.
2959.1.diff (30.5 KB) - added by jmdodd 3 years ago.
Creates meta functions and corresponding unit test coverage; updates moderators, subscriptions, and favorites.
2959-alt-wip.diff (41.0 KB) - added by netweb 2 years ago.

Download all attachments as: .zip

Change History (40)

#1 @johnjamesjacoby
3 years ago

  • Description modified (diff)

#2 @johnjamesjacoby
3 years ago

In my opinion, this should be completed before (or as part of) the mass data migration of WordPress.org's bigger forum installation. As usual, we can (and I believe should) use bbPress.org & BuddyPress.org as our training wheels to get this deployed there first.

#3 @johnjamesjacoby
3 years ago

  • Type changed from defect to task

#4 @johnjamesjacoby
3 years ago

I'll add that a custom pointer table to connect user IDs to post IDs with a "relationship_type" would be the ideal solution. Unfortunately, it's much more work to build this out, and should we ever need to get to this point, it's another simple data migration to pull this out from wp_postmeta and into wp_user_relationships or whatever.

#5 @jmdodd
3 years ago

  • Cc jmdodd@… added

First pass at meta abstraction functions and unit tests.

@jmdodd
3 years ago

@jmdodd
3 years ago

Creates meta functions and corresponding unit test coverage; updates moderators, subscriptions, and favorites.

#6 @netweb
2 years ago

Related: #meta1980 Forums: Subscribe/unsubscribe actions not functioning

#7 @johnjamesjacoby
2 years ago

In 6109:

Favorites/Subscriptions/Moderators: Introduce metadata API for linking multiple users to multiple forums/topics.

Previous to this, connections were stored in usermeta. We knew this would not scale, but bbPress 1 had a friendlier database schema & we expected WordPress's taxonomy/relationship roadmap would be farther along by now.

By storing user ID's in postmeta instead, we gain an ability to query for connections from both directions without custom MySQL, while also leveraging persistent caching in a more sane way.

This commit includes several new helper functions for low-level relationship management, as well as modifications to existing functions to allow them to continue to work as they always have.

See: #2959.

#8 @netweb
2 years ago

In 6110:

Tests: Use assertEqualSets() instead of assertEquals() in favorites and subscriptions tests.

This changeset improves array comparisons where arrays may not be in the same order.

See #2959.

#9 @netweb
2 years ago

  • Milestone changed from 2.7 to 2.6

#10 @netweb
2 years ago

In 6111:

Users: Update PHPDoc's for user functions and capabilities.

See #2959.

#11 @johnjamesjacoby
2 years ago

In 6116:

Roles: Remove overzealous usages of bbp_add_forums_roles().

Now that the wp_roles_init action exists, we can rely on bbPress roles being registered an available, and no longer need to re-reinitalize them before trying to interact with them.

See: #2959.

#12 @johnjamesjacoby
2 years ago

In 6174:

Tools: First pass at upgrade tools for favorites & subscriptions.

  • Registers 2 new repair tools
  • Includes basic looping patterns for user-meta to post-meta

Needs testing and scrutiny.

See #2959.

#13 @johnjamesjacoby
2 years ago

In 6175:

Tools: Use $total count in bbp_admin_migrate_user_favorites().

Also update revision number in function docs.

See #2959.

#14 @johnjamesjacoby
2 years ago

In 6176:

Tools: Massize clean-up

  • Add missing function documentation blocks
  • Rename _migrate_ function to _upgrade_
  • More typo fixes
  • Make overhead values clickable in list table rows
  • Refactor overhead to work more like components, using keys instead of literal strings

See #2959.

#15 @johnjamesjacoby
2 years ago

In 6177:

Tools: Update tools text to include favorites & subscriptions.

See #2959.

#16 @johnjamesjacoby
2 years ago

In 6178:

Tools: Update forum/topic/reply admin classes to avoid screens without post_type parameters.

Fixes edge-case debug notices when tools pages for third-party plugins are doing advanced things.

See #2959.

#17 @johnjamesjacoby
2 years ago

Quick note: the tools introduced in r6174 should probably get switched to faster INSERT INTO queries like we've done for the others. I more just wanted to get a first pass in so we could see the manual process first.

If anyone wants to make a second pass at improving these, please make it so.

#18 @johnjamesjacoby
2 years ago

In 6181:

Tools: Use explode() instead of maybe_unserialize().

User-meta values were strings to enable FIND_IN_SET() usages.

See #2959.

#19 @johnjamesjacoby
2 years ago

In 6182:

Tools: Explicitly pass false into add_post_meta()'s $unique parameter.

Though it's the default, we can't make any mistake that meta-key's to be non-unique for this to function correctly.

See #2959.

#20 @johnjamesjacoby
2 years ago

In 6183:

Core: Upgrade routine for favorites & subscriptions in 2.6.0.

  • Use the new upgrade tools
  • Bump the DB version to 260
  • Improve inline docs in upgrade tool functions

See #2959.

#21 @johnjamesjacoby
2 years ago

In 6184:

Converter: Convert favorites & subscriptions to postmeta vs. usermeta.

  • Update table names
  • Support for comma-separated string values
  • Use strict comparisons where it makes sense to

See #2959, #2668.

#22 @johnjamesjacoby
2 years ago

In 6186:

Admin: Implement new loading sequence for major admin components.

  • Introduce new bbp_current_screen sub-action
  • Hook forums/topics/replies into bbp_current_screen
  • Remove various bail() methods, which were fragile and terrible anyways
  • Revert r6178, thanks to order-of-operation issues with get_current_screen()
  • Remove Comments & Discussion metaboxes if comments is not explicitly supported

See #2959.

#23 @johnjamesjacoby
2 years ago

In 6188:

Tools: Avoid duplicate entries for favorites & subscriptions.

This fixes a bug in the 2.6.0 upgrade routine where running the tool multiple times could add duplicate metadata.

See #2959.

#24 @johnjamesjacoby
2 years ago

In 6189:

Tools: Use get_results(), and don't delete usermeta.

(Maybe we should have a separate clean-up tool once it's confirmed to be OK.)

See #2959.

#25 @netweb
2 years ago

In 6193:

Tools: Add an upgrade tool for forum subscriptions.

  • Adds bbp_admin_upgrade_user_forum_subscriptions()
  • Renames bbp_admin_upgrade_user_subscriptions() to bbp_admin_upgrade_user_topic_subscriptions()
  • Updated inline docs

Previously in r6174 upgrade tools were added for favorites and subscriptions, support for migrating forums subscriptions meta _bbp_forum_subscriptions wasn't included, rather than adding extra overhead to the existing topic subscriptions upgrade tool another upgrade tool was added for forum subscriptions.

See #2959.

#26 @netweb
2 years ago

In 6194:

Tests: Add favorite, topic subscriptions, and forum subscription upgrade tool tests.

  • BBP_Tests_Admin_Tools::test_bbp_admin_upgrade_user_favorites
  • BBP_Tests_Admin_Tools::test_bbp_admin_upgrade_user_topic_subscriptions
  • BBP_Tests_Admin_Tools::test_bbp_admin_upgrade_user_forum_subscriptions

See #2959.

@netweb
2 years ago

#27 @johnjamesjacoby
2 years ago

In 6197:

Metaboxes: Add metaboxes for viewing favorites & subscriptions of topics & replies.

  • New functions for outputting avatars of users who have favved or subbed
  • Use the $post parameter that's passed in, rather than using get_the_ID() again
  • Use require_once as a language construct vs. include_once() as a function
  • Pass $post object through to metabox subsequent filters vs just the ID

See #2959.

#28 @netweb
2 years ago

In 6200:

Metaboxes: Update @since revisions for functions introduced in r6197.

See #2959.

#29 @johnjamesjacoby
2 years ago

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

This seems pretty much complete, as I envisioned it.

Let's close this for 2.6, and work out any kinks during 2.6 beta.

#30 @johnjamesjacoby
2 years ago

In 6215:

Tools: Order fav/sub upgrade by user_id.

This makes it easier to LIMIT later if you need to manually chunk this. See #2959.

#31 @johnjamesjacoby
2 years ago

In 6216:

Users: Add nopaging to WP_Query usages.

Fixes bug with favs/subs being limited to 10 results by default. See #2959.

#32 @johnjamesjacoby
2 years ago

In 6218:

Users: Remove post__in queries from favs/subs template loops.

This is only recently possible thanks to postmeta storage, and should result in a not-insignificant performance boost for those user profile pages.

See #2959.

#33 @johnjamesjacoby
2 years ago

In 6221:

Moderators: Update bbp_get_moderator_forum_ids() to use same approach as favs/subs.

See #459, #2959, #2972.

#34 @johnjamesjacoby
2 years ago

In 6242:

Upgrade: Do not automatically run the upgrade routine on large installations with many users.

This is basically a no-brainer, and exactly what we've gone through across all of the WordPress.org network already.

See #2959.

#35 @johnjamesjacoby
2 years ago

In 6243:

Upgrade: Introduce bbp_is_large_install() to abstract wp_is_large_network() which is a multisite-only function.

See #2959.

#36 @johnjamesjacoby
2 years ago

In 6274:

Tools: Unify the tab/link experience:

  • Introduce function to get tools pages
  • Trust capability checks in core WordPress functions, and remove our own bespoke pre-checks
  • Add tool-box to wp-admin/tools.php linking to tools the user has access to

This change promotes exposure to bbPress's tools pages, and makes adding third-party tools pages easier.

See: #2959.

#37 @johnjamesjacoby
3 weeks ago

Assigning all closed & unassigned tickets in the 2.6 milestone to myself.

Note: See TracTickets for help on using tickets.