Opened 6 years ago
Last modified 2 weeks ago
#6347 reopened enhancement
XProfile fields used for signup should be configurable
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 8.0.0 | Priority: | high |
Severity: | normal | Version: | 1.0 |
Component: | Extended Profile | Keywords: | needs-patch |
Cc: | vivek@… |
Description
BuddyPress has always used fields in the "Base" field-group as sign-up fields. In our templates, register.php
just calls bp_has_profile()
with custom arguments.
While it is possible to conditionally filter bp_before_has_profile_parse_args
when bp_is_register_page()
returns true, I believe BuddyPress should offer the ability to cherry-pick which fields across all field-groups should appear on the signup page.
Attachments (5)
Change History (35)
#1
@
6 years ago
A huge +1. This is always baffling to clients.
#5192 probably hinges on this ticket.
Please be mindful of backward compatibility when working on patch - we want to be sure to fall back gracefully on the Base group, and we want to be cognizant of the various ways that people customize the registration process. I've done a number of such customizations, so once I have a sense of how you plan to approach this, I'm happy to provide feedback on this point.
#2
@
6 years ago
6347.01.patch simply moves the bp_has_profile()
and its arguments out of register.php
and into a custom function.
The additional bp_parse_args()
call allows explicit filtering of bp_before_signup_fields_parse_args
to more easily target the arguments.
How I plan to approach this:
- New "Signup" metabox for all fields (including base?) with a checkbox to opt-into including
- Checkbox is linked to profile field meta similar to field visibility
- Custom query cached to
bp_xprofile
group to determine if any custom sign-up fields have been set - If so, use them and *not* the old base-group approach
- If not, use the old base-group approach
UX Goals:
- If a user sets any profile fields as sign-up fields, they have opted into customizing the experience, and will naturally want to continue to pick others
- If a user has not set any profile fields yet, the status quo continues as is for backwards compatibility
- Consider improving visual distinction of which fields are: required, primary, sign-up
Stretch goals:
- Maybe begin to think about removing BuddyPress's dependency on the "Base" profile group and field. They aren't really hurting anything, but they start to make less sense when fields and groups become increasingly more flexible.
- We can create another new metabox for telling BuddyPress which profile field to use for the "Base" field, which BuddyPress traditionally uses as another "Display Name" field, which I think is pretty confusing.
#3
@
6 years ago
One part I haven't worked out is if sign-up fields need custom positioning. If so:
- Where does that specific ordering live?
- How in the UI do we make this clear without making it too complicated?
- Do we allow completely avoiding the profile field requirement? With the approach above, having no fields selected will revert to existing behavior, and not show 0 fields.
#4
@
6 years ago
Something like [6347.01.patch] seems like a good start, and definitely doable for 2.3. I find the syntax a little odd - wrapping a template function rather than, say, filtering the bp_has_profile()
arguments - but whatevs.
#5
@
6 years ago
6347.02.patch does the following:
- Introduces metabox UI for saving field-meta keyed
signup_position
and sanitized with an(int)
cast - Adds
$signup_position
variable toBP_XProfile_Field
class - Currently just a checkbox for on/off, which saves the meta as
1
for all fields (for now) while position is experimented with - No query manipulation yet
#8
@
6 years ago
I completely forgot that BuddyPress's profile field querying is married to querying for field-groups, and fetching individual fields could be considered a secondary notion.
r9685 and r9686 tidied a few existing functions and methods, but it looks like BP_XProfile_Field
could use a static getter method just for retrieving & caching fields themselves, unrelated to groups and field data.
Will work on a patch with tests soon.
#15
@
6 years ago
- Cc vivek@… added
By default, there should be no xprofile fields required at signup #5583
#16
@
6 years ago
- Keywords has-patch 2nd-opinion added
- Owner set to johnjamesjacoby
- Priority changed from normal to high
- Status changed from new to accepted
6347.03.patch is a big patch that does the following:
- Introduces smarter
get()
method for free-form querying of field data - Uses this new static method where appropriate
- Modifies
BP_XProfile_Field
methods as appropriate - Introduces field caching, including for field options
- Passes existing unit tests; needs tests to confirm cache invalidation is working correctly
- Works with existing
xprofile_
functions - Works with field groups & options
- Treats fields and options as one and the same, allowing them to use the same CRUD methods
I would appreciate feedback and help with the following:
- Are the query params okay? What can we add/remove/rename to make this better?
- Should we introduce a dedicated
parse_args()
method that can be used forDELETE
queries also? - Anyone want to wire-up the
meta_query
part? - Our existing meta-cache implementation is a bit of a bolt-on, making it difficult to unwind smoothly with multiple layers of caching happening. It would be nice to simplify this eventually.
- Field groups getting Fields getting Field Data is quite the stack. It's not ideal, but I think it will need to be improved not as part of this initiative.
TL;DR - this is a big patch that touches lots of existing code. Similar to attachments, I'd like to commit this into trunk ASAP to continue iterating on the approach before this patch goes too stale.
This ticket was mentioned in Slack in #buddypress by boone. View the logs.
6 years ago
#18
@
6 years ago
Thanks for your work on this so far.
I am a big fan of converting the disparate query methods to get()
wrappers, especially since it builds in cache support for all these methods. I see that you've built in centralized invalidation for these caches - which I like - but let's make sure to check that this covers all the ways in which the field table can be updated. I haven't looked at this specifically, but my knowledge of the bp-xprofile component is such that I wouldn't be surprised if we were doing some direct UPDATE/INSERT queries elsewhere.
It looks to me like the get()
/cache support refactoring comprises a standalone improvement. It would be nice to see it broken out at the time of commit. The signup IDs stuff is pretty straightforward and independent once the get()
refactor is done.
I'm not sure I understand the parse_args()
suggestion. Is it just so that we don't have to repeat the $args
signature? A class property $default_args
or something like that would also work.
The existing meta-cache implementation - by which I assume you mean bp_xprofile_update_meta_cache()
- is indeed pretty clunky, and is built specifically with the needs of bp_has_profile()
and its funky nesting in mind. I agree that it's probably a good idea to make it more flexible. Something like your update_field_caches()
method, except for meta caches, might be good, though we'd want to make sure that in cases where we *do* cascade the hierarchy, we do a single query for each level of the hierarchy to populate the cache. (In other words: if querying for four profile groups, each with two fields, we do a single query to populate the four groupmeta caches, and a single query to populate the eight fieldmeta caches - *not* four separate fieldmeta cache queries, as the normal tree-walking technique would suggest.)
This ticket was mentioned in Slack in #buddypress by jjj. View the logs.
6 years ago
#21
@
6 years ago
The call for getting "this into trunk ASAP to continue iterating on the approach before this patch goes too stale" - does it still count? I was wondering what is needed to get this ticket going on.
The patch contains the signup-position
metabox, but there's no screen where the user could get an overview of all selected fields and manually rearrange them. I think that is a required addition. Can I help with that or should we wait untill all the query/caching-stuff is in?
#22
@
5 years ago
- Milestone changed from 2.4 to 2.5
I'm not sure either what's up -- it sounds like there are some parts that could be pulled into a separate patch, but I know Boone's done some xprofile cache tweaks recently, I don't know if there's any overlap/redundancies.
Reluctantly moving to 2.5 because this hasn't received any love in the last 6 months.
This ticket was mentioned in Slack in #buddypress by jjj. View the logs.
5 years ago
#24
@
5 years ago
- Keywords needs-patch added; has-patch 2nd-opinion removed
- Milestone changed from 2.5 to Future Release
This ticket was mentioned in Slack in #buddypress by johnywhy. View the logs.
5 years ago
#26
@
3 years ago
- Keywords trac-tidy-2018 added
We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.
Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.
If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.
For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/
#27
@
3 years ago
- Milestone Awaiting Contributions deleted
- Resolution set to maybelater
- Status changed from accepted to closed
This ticket was mentioned in Slack in #buddypress by takecaffeine. View the logs.
11 months ago
#29
@
3 weeks ago
- Keywords trac-tidy-2018 removed
- Milestone set to 8.0.0
- Resolution maybelater deleted
- Status changed from closed to reopened
I believe we should try to carry on the work John started here during the 8.0.0 release.
First pass at simple custom template loop function