Skip to:
Content

Opened 3 years ago

Closed 20 months ago

Last modified 20 months ago

#3127 closed enhancement (duplicate)

Switch to WP_User_Query for BP_Core_User

Reported by: cnorris23 Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Core Keywords: dev-feedback 2nd-opinion
Cc: georgemamadashvili@…

Description

WP 3.1 introduced the WP_User_Query class. If BP utilized this, it would reduce the amount of code that needs to be maintained (no need to reinvent the wheel), and have the added benefit/reduced headache of caching. However, this would require that XProfile data be moved into the user_meta table, rather than it's own table as it is now. Since I didn't join the BP community until 1.2-alpha, I don't know the logic behind a separate table over the user_meta table. Therefore, this ticket may be short-sighted. I've opened this ticket to start (or end if I'm way off base) a discussion on this. I'm more than willing to write up a patch, but I wanted to get feedback first.

Initial thoughts on moving the XProfile fields would be to move them to a CPT, with profile options and any meta being stored in the post_meta table. Again, this would reduce code we need to maintain by giving us access to WP_Query.

Too much caffeine, and having to decide what/what not to say in this ticket has me all verklempt. Talk amongst yourselves.

Change History (14)

comment:1 DJPaul3 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect to enhancement

Move to post types and the benefits thereof are targeted for 1.4.

comment:2 cnorris233 years ago

Cool. I wasn't sure if XProfile fields were slated for CPT. What about moving userdata to the usermeta table?

comment:3 cnorris233 years ago

  • Severity set to normal

Related #3335

comment:4 joevansteen3 years ago

If this is going to be opened for discussion I would advance the logic proposed in #3335 and in my blog post (http://architectedfutures.info/2011/08/23/bp-wp-profile-sync/). I don't see the logic for having 2 user profile facilities in BP, one from WP and one from BP. If BP has a separate standalone implementation without WP, then it makes sense; otherwise not.

WP is the base system, and WP should provide the user profile. BP's profile happens to be better, richer and more sophisticated. But the profile facility should be a feature of WP. I'm new here and learning the scene, but aren't these two systems related and interdependent? Is there a joint planning forum or body? If the BP profile facilities were moved into WP, WP would be a better system with a richer profile, hierarchical fields, better edits, etc. BP would be closer integrated and no longer responsible for the profile proper. WP profile add-on modules and features would apply to BP "users" without the add-on needing to be specifically aware of BP special tables and rules. (Is that good or bad? I don't know.) Obviously, the "social" network integration of the user into groups, etc would remain BP. Features like profile security would be done as WP and apply to both sides. (Hypothetically that would open a wider audience and more opportunity for contributors for enhancements and maintenance at the same time.)

I've seen some comments about the need not to interfere with the WP side of the system and questions about what happens when someone pulls the BP feature off of their site, but I think these arguments are moot. The user that decides to put a BP add-on on to a WP system made the choice to integrate. I think the desire for integration speaks for itself. By pulling BP off they can't automatically distinguish users who were BP from WP anyway. They are all one set of users, the users of the site. It's the attributes that are BP, not the users. If anyone wanted to get rid of the rich profile data at that point, they could just delete the fields, just as they would if BP were still active.

This may not be the way people want to go, but I thought I'd give it a shot. Maybe some other folks will comment. In terms of people not demanding this earlier, which was another comment I heard, I think people by and large are just glad to have the BP feature set and ignore the conflict by focusing on the BP side. I think the system as a whole though would make more sense and there would be a lot of advantage to having a single user profile facility and having it based in the WP part of the system.

comment:5 boonebgorges3 years ago

joevansteen - Thanks for your comment, and for your blog post. This particular ticket is about changing some of our direct wp_users queries so that they use WP core's WP_User_Query API class. It's not really related to #3335, as that ticket is more about what to do with xprofile and user metadata.

I don't disagree with your general point, which is that we should avoid duplication, and send the best of BP's user management features upstream to WP. But BP xprofile, in its current state, is not even in the ballpark of being ready to pass upstream. It's a large issue that should be revisited once we have begun the process of moving our data out of custom tables.

comment:6 boonebgorges3 years ago

  • Milestone changed from Future Release to 1.6

comment:7 cnorris233 years ago

@boonebgorges Just wanted to give my reasoning (because now I feel like I need to explain myself :)) as to why I said they are related. In order to use WP_User_Query in all it's glory, we need BP user_meta to be in the WP user_meta table. Having BP sync with related WP profile fields is an added bonus, but not neccessary. Therefore, what happens with #3335 directly effects the code needed for this ticket. So while not directly related, they are causally related.

@joevansteen boonebgorges is right. This isn't the proper place for this discussion, but feel free to repost this on #3335. I know you have posted in a more limited way on that ticket already, so you may not want to repost your above comment. The only reason there has been no in depth discussion on #3335 is because it wasn't quite time to work on that ticket. It was outside the scope of 1.5 (1.3), but for 1.6+ discussion should pick up.

comment:8 joevansteen3 years ago

@boonebgorges Thanks for the comment and the clarification. I was drawn here by the link. My opinion is from the perspective of an architect. I can write code, but I'm not familiar with this code base and just getting my feet wet, but I generally understand your dialog.

@cnorris23 As you mention, I have already posted on #3335 and both posts link to my blog post which has my overall conclusions so I don't think its necessary to fill the space repeating myself. I assume the link and this thread will stay for a while for reference. Thanks for pursuing this topic though. I think making incremental progress along this path is good for the ultimate health of the system and the community.

comment:9 boonebgorges3 years ago

In order to use WP_User_Query in all it's glory, we need BP user_meta to be in the WP user_meta table.

Maybe. Either that or we need to extend the WP_User_Query class so that we are able to add our own joins, etc as necessary. I think that, in the short-to-medium term, this is what we must and should do.

The fact is that WP key-value usermeta will never really be able to handle BP-style profile data. As Joe suggests in his blog post, architectural changes will have to happen at level of WP's core user management before we can consider using the full glory of WP_User_Query. (For example, we might consider developing user taxonomies for inclusion in WP core, and then add tax_query support to WP_User_Query, and moving xprofile into *those*.) But this is a discussion that should take place on the other ticket.

For the purposes of the nearer refactor, we will need to do some magic for things like sorting, because currently some of our sorts (like 'popular') depend on joins against BP tables. WP_User_Query is pretty light on filters, so we'll probably either have to override some of their methods, or submit some patches for WP that add the necessary apply_filters(), or both.

comment:10 cnorris233 years ago

@boonebgorges You're right. For some reason I had it in me head that it was going to be easier make the move. I went back the other day to look over the code, and remembered that a move like that would be especially difficult. It would take a number of steps (at least according to my mental wireframe) to get it working with the current Xprofile API. Also, since xprofile is still up in the air for CPTs, and with the shorter 1.6 dev cycle discussed in today's dev chat, I'm not sure it's a worthwhile endeavor for 1.6. If you, jjj, or djpaul feel otherwise, I'd be happy to assign myself to this. Otherwise, I think this is a good cadidate for punting.

comment:11 boonebgorges2 years ago

  • Milestone changed from 1.6 to Future Release

Given our dependence on xprofile information, I think that this needs to wait for further maturation of WP_User_Query.

comment:12 DJPaul20 months ago

Boone. Is this something we can touch in 1.7? I think you said you're looking at some WP_User_Query stuff for the database tickets.

comment:13 boonebgorges20 months ago

  • Milestone Future Release deleted
  • Resolution set to duplicate
  • Status changed from new to closed

This ticket is going to get subsumed under #4060. Long story short, regarding the specifics of this ticket: 1.7 will introduce BP_User_Query, which uses WP_User_Query internally for collecting some user information. bp_core_get_users() and a few other user-fetching functions will then be switched over to use the new BP_User_Query, with an option to fall back on BP_Core_User if they need it for backward compatibility. (Some of BP_Core_User's miscellaneous methods may remain in use for 1.7.)

For further discussion, see #4060.

comment:14 Mamaduka20 months ago

  • Cc georgemamadashvili@… added
Note: See TracTickets for help on using tickets.