Skip to:
Content

BuddyPress.org

Opened 16 months ago

Last modified 5 months ago

#7882 new enhancement

Stop mirroring user last_activity data to usermeta

Reported by: boonebgorges Owned by:
Milestone: Up Next Priority: normal
Severity: normal Version: 2.0
Component: Members Keywords: dev-feedback needs-patch
Cc:

Description

Previously: #5128

The primary storage location for user 'last_activity' is the activity table. For backward compatiblity, we mirror that data in usermeta. See https://buddypress.trac.wordpress.org/browser/tags/3.0.0/src/bp-members/bp-members-functions.php?marks=1002-1009#L976

Now that we are moving toward something like semver, we have more of an opportunity to phase this out. I suggest that in 4.0, we stop doing this mirroring by default, but allow plugins to reenable it via filter. We would then publish a notice on the bpdevel blog warning plugin authors about the change.

Attachments (1)

7882.diff (1.9 KB) - added by boonebgorges 15 months ago.

Download all attachments as: .zip

Change History (7)

#1 follow-up: @DJPaul
16 months ago

I think it’s time to stop the sync yes.

But how about add a filter to user meta that intercepts the request before it’s made, and we call activity meta and return that instead? Maybe that’s the filterable option rather than reenabling the sync.

#2 in reply to: ↑ 1 @boonebgorges
16 months ago

Replying to DJPaul:

I think it’s time to stop the sync yes.

But how about add a filter to user meta that intercepts the request before it’s made, and we call activity meta and return that instead? Maybe that’s the filterable option rather than reenabling the sync.

If I'm understanding your suggestion correctly, then we already do this: https://buddypress.trac.wordpress.org/browser/tags/3.0.0/src/bp-members/bp-members-functions.php?marks=1053,1043#L1014

But yes, if we have a "enable legacy mode" filter, we can have it apply to both writing and retrieving.

@boonebgorges
15 months ago

#3 @boonebgorges
15 months ago

  • Keywords has-patch added; 2nd-opinion good-first-bug needs-patch removed

7882.diff is a first pass. The mirroring is disabled by default, but can be reenabled.

I haven't changed anything about the way the get_user_meta( 'last_activity' ) behavior works. It still triggers _doing_it_wrong(), and it still returns the value from the activity table. I don't see a reason why the new filter would affect this. Even if you're not mirroring - perhaps *especially* if you're not mirroring - developers ought to see the notice, and I don't think it causes any problems to continue to provide the correct value out of the activity table.

#4 @boonebgorges
11 months ago

  • Milestone changed from 4.0 to Up Next

#5 @r-a-y
5 months ago

  • Keywords dev-feedback added

This will break legacy user queries that need the entry in the $wpdb->usermeta DB table in order to sort by last active or online:
https://buddypress.trac.wordpress.org/browser/tags/4.2.0/src/bp-core/classes/class-bp-core-user.php#L284
https://buddypress.trac.wordpress.org/browser/tags/4.2.0/src/bp-core/classes/class-bp-core-user.php#L341

(By "break", I mean have an outdated user query!)

Also the get_user_extras() method would need to be updated to fetch the timestamp from the activity DB table:
https://buddypress.trac.wordpress.org/browser/tags/4.2.0/src/bp-core/classes/class-bp-core-user.php#L721

It would almost be like rewriting the legacy user query to use the newer one, which would defeat the purpose of implementing this ticket! ☺

I think a better approach would be to use the existing 'bp_use_legacy_user_query' filter to determine if we should continue mirroring, instead of introducing a newer filter solely for this as in 7882.diff.

Last edited 5 months ago by r-a-y (previous) (diff)

#6 @boonebgorges
5 months ago

  • Keywords needs-patch added; has-patch removed

I think a better approach would be to use the existing 'bp_use_legacy_user_query' filter to determine if we should continue mirroring, instead of introducing a newer filter solely for this as in 7882.diff.

This approach sounds fine to me. The inline docs for that filter should be updated to reflect the fact that using the legacy query will force this data to be mirrored.

Note: See TracTickets for help on using tickets.