XProfile fields used for signup should be configurable

[johnjamesjacoby]

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.

[johnjamesjacoby]

First pass at simple custom template loop function

[boonebgorges]

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.

[johnjamesjacoby]

​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.

[johnjamesjacoby]

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.

[boonebgorges]

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.

[johnjamesjacoby]

Includes metabox UI

[johnjamesjacoby]

​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 to BP_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

[johnjamesjacoby]

In 9685:

XProfile: Mild clean-up to bp_has_profile():

• Add phpdoc block
• Remove extract() usage
• Small code formatting tweaks

See #6347.

[johnjamesjacoby]

In 9686:

XProfile: Refactor bp_has_profile() stack to support passing arguments as associative arrays.

• Maintains backward compatibility by parsing old-style parameters using bp_core_parse_args_array(), converting to new-style array, and throwing _deprecated_argument() error when old-style is used
• Changes calls to BP_XProfile_Data_Template::__construct() to use the new format
• Hat-tip boonebgorges for similar work to other components previously.

See #6347.

[johnjamesjacoby]

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.

[johnjamesjacoby]

In 9687:

XProfile: General code formatting cleanup to bp-xprofile-template.php. See #6347.

[johnjamesjacoby]

In 9688:

XProfile: Missed a ! in r9687. See #6347.

[johnjamesjacoby]

In 9689:

XProfile: Remove phpdoc block copy/pasta from r9686. See #6347.

[johnjamesjacoby]

In 9690:

XProfile: First pass at saving and looking for signup_position metadata when viewing the main admin editing page. See #6347.

[johnjamesjacoby]

In 9692:

XProfile: Add xprofile_field_save group to BP_Tests_BP_XProfile_Field_TestCases:test_can_delete_save() test for easier testing of this unit. See #6347.

[johnjamesjacoby]

In 9693:

XProfile: Improvements to bp_xprofile_update_meta_cache():

• Cleaner logic for deciding whether a query is necessary
• Bail if: no query necessary, no where conditions, no uncached data, no data retrieved
• Avoid possible debug notice if $where_sql was never defined
• Fewer indentations and less needless execution via improved sanity checks

See #6347.

[sooskriszta]

By default, there should be no xprofile fields required at signup #5583

[johnjamesjacoby]

First pass at refactoring BP_XProfile_Field

[johnjamesjacoby]

​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 for DELETE 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.

[boonebgorges]

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.)

[johnjamesjacoby]

More incomplete iteration; uploading here incase I die before tomorrow

[johnjamesjacoby]

More incomplete iteration, again...

[Offereins]

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?

[DJPaul]

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.

[DJPaul]

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/

[imath]

I believe we should try to carry on the work John started here during the 8.0.0 release.

[imath]

Hi,

I've been working on an alternative way of achieving this. Instead of improving the BP_XProfile_Field object adding methods to fetch/cache/query/delete/add etc.. I suggest to use what we have so far inside the BP_XProfile_Group object.

To build ​this alternative patch I've kept in mind @offereins comment below because I agree it's a very important point.

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?

In the patch I'm editing the xProfile WP Admin Screen to simulate a new Group for the signup fields. This "fake" group is used to enjoy the UI we have to sort and move fields from one Group to another one.
This group doesn't exist into the db table and nothing is changed about the profile fields we drag on it. They are not moving from the field group they are attached to. It's I believe a nice way to pick the fields of the various groups we want to display into the signup form.

I've edited the JavaScript UI so that it simply clones the dragged field into the "Signup Fields" tab. Once cloned into this tab you can reorder the signup fields.

To only fetch the signup fields using the bp_has_profile() loop, I've added a new parameter signup_fields_only to inform BP_XProfile_Group::get() it needs to ignore all other fields. So fields are fetched using the order of their belonging group at first. Then into BP_XProfile_Data_Template just after we got Groups and fields, I'm using a query to build an array of signup field IDs ordered according to their signup_position metadata, I'm then faking a new group looping into the fetched ones to reorder the fields according to this array.

And it works, check ​this demo :)

Other things the patch is handling:

• The array of ordered signup field IDs is cached
• Installation sets the fullname field by default as the first and required signup field.
• A migrate task does the same during 8.0.0 upgrade.
• Template packs has both been updated.

Things I still need to work on:

• I still need to improve it to select all Base group fields if signup are enabled.
• I still need to add a filter if the user has overriden the register template to improve backcompat.
• I still need to improve the JavaScript UI because when you drag a field into the Signup fields tab, as it's not removing the dragged field from the original Groups tab, the group fields can be sorted differently (moving the dragged field at the top).
• I still need to add a metabox into the field's edit screen to enjoy the things @johnjamesjacoby previously added for the signup_position metabox to remove a field from the signup form from this screen.

About this last point I have 2 options:

• use a toggle to swith between sorting/moving fields
• use up/down arrow buttons in field containers like the WordPress Dashboard metaboxes now includes.

What do you think of this alternative?

[imath]

​6347.alt.2.patch improves 6347.alt.patch by taking in charge the previous points I sill had to work on:

• I still need to improve it to select all Base group fields if signup are enabled.
• I still need to add a filter if the user has overriden the register template to improve backcompat.
• I still need to improve the JavaScript UI because when you drag a field into the Signup fields tab, as it's not removing the dragged field from the original Groups tab, the group fields can be sorted differently (moving the dragged field at the top).
• I still need to add a metabox into the field's edit screen to enjoy previously added code for the signup_position metabox to remove a field from the signup form from this screen.

About the last point, it also handles adding a field to the signup form from the xProfile field edit screen. The position in this case defaults to the last one.

I also added the signup_position to the Field Type supported features introduced in [12868]. This way a field type can disable the signup position metabox.
Finally I've improved the JavaScript UI to dynamically add/remove the (Sign-up) span if a field was added/removed using Ajax.

I believe it's ready to be committed. I'll run some complementary tests before doing so early next week, unless someone has an objection about it ;)

[imath]

In 12885:

Improve the Field/Group APIs to pick selected fields as signup ones

• Introduce the bp_xprofile_get_signup_field_ids() function to get an ordered list of field IDs to use as signup fields.
• Introduce the bp_xprofile_signup_args() function to get the signup arguments to use into the registration page xProfile loop. If no signup fields are available, it falls back to the primary fields group.
• Introduce the $signup_fields_only xProfile loop argument to restrict fetched fields to signup ones if set to true.
• Adapt the BP_XProfile_Data_Template class to build a unique xProfile field group out of any possible field groups containing one or more signup fields when the $signup_fields_only argument is set to true and registration is allowed on the site.
• Adapt the BP_XProfile_Field class to get the possible field's signup position and add a new metabox to add a field to signup ones from the xProfile Field Administration screen.
• Adapt the BP_XProfile_Group class to ignore non signup fields if the $signup_fields_only argument is set to true.

Props johnjamesjacoby, boonebgorges, DJPaul, Offereins

See #6347

[imath]

In 12886:

Use drag and drop into the xProfile Admin UI to to set signup fields

• Create a new "Signup Fields" tab to list signup fields. By default this tab will include the primary field (Name).
• The install process has been adapted to make sure this is the case.
• An upgrade task is migrating fields into the primary group as signup fields.
• Make it possible to drag fields from any group into the "Signup Fields" one.
• Make it possible to use drag and drop to reorder signup fields from the corresponding tab.
• Include a link on each field entry (except the primary one) to remove the field from this "Signup Fields" field group.

Props johnjamesjacoby, boonebgorges, DJPaul, Offereins

See #6347

[imath]

In 12887:

Update both Template Pack registration forms to use signup fields

• Nouveau and Legacy are now usinf the bp_xprofile_signup_args() to build the argument of their registration's form xProfile loop.
• Include a backward compatibility mechanism if the register.php page has been overriden from a theme or a plugin.
• This commit also includes PHP Unit tests about the xProfile loop and Signup Fields.

Props johnjamesjacoby, boonebgorges, DJPaul, Offereins

Fixes #6347