Opened 6 years ago
Closed 4 years ago
#7882 closed enhancement (fixed)
Stop mirroring user last_activity data to usermeta
Reported by: | boonebgorges | Owned by: | imath |
---|---|---|---|
Milestone: | 7.0.0 | Priority: | normal |
Severity: | normal | Version: | 2.0 |
Component: | Members | Keywords: | has-patch commit |
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 (3)
Change History (13)
#2
in reply to:
↑ 1
@
6 years 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.
#3
@
6 years 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.
#5
@
5 years 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
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
.
#6
@
5 years 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.
#8
@
4 years ago
- Keywords has-patch added; needs-patch removed
02.patch
only allows the 'last_activity'
usermeta mirroring if the 'bp_use_legacy_user_query'
filter is forced to true
. I've added PHPDoc where appropriate.
One other thing I wanted to fix was the BP_Core_User::get_user_extras()
method and how it should use the activity DB table instead of usermeta for last activity. This is now done as well.
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.