User roles with differents profile fields

[_DorsVenabili]

This is a ticket for a enhancement of the Extended Profile module.

It would be very useful to be able to create a group of profile fields only for a user role (or some of them).

This way, it would be possible to create custom roles with different fields and to have different registration forms.

[boonebgorges]

See also #5121, which discusses the related project of breaking out capabilities (in part so that they can be linked to user types).

[boonebgorges]

See #6006 for discussion of the underlying user types API.

[Offereins]

A while ago I wrote ​a plugin to assign XProfile field(groups) to BuddyPress groups. Since BP 2.2 my thought was along the lines of this topic to implement the same logic, but for member types. I did not realize it yet, but how would an eventual core implementation differ from the logic present in that plugin?

My plugin distinguishes two implications:

• applicability - who has to provide data for the field(group), at the Edit Profile page?
• visibility - who can see the entered data for the field(group), at the View Profile page?

The group assignment is done on the edit page of the field(group) and group ids are stored as profile field(group) meta.

To be clear: I'd like to help out with some of the code to make this happen. Yet, I'm not sure how it is envisioned by the core devs.

[boonebgorges]

Offereins - Thanks so much for chiming in. I didn't know about your plugin, and it looks like there is indeed some conceptual overlap. (Plugin code looks great, btw.)

First off, I wasn't envisioning any "visibility" implications for this first revision. I can certainly imagine a future enhancement that would allow member types to appear in visibility levels, but I don't think this ticket depends on it. I was imagining it purely in terms of "applicability".

Another conceptual difference I'd imagined is that I'd been imagining this to be a per-field setting, instead of per-group. I think we'd need to go with one or the other - trying to mix the two seems like it would cause weird issues.

At a very basic level, I'd imagined the following:

• When creating/editing a profile field in the Dashboard, admins would have a Member Type metabox. It'd be nice to have three-way toggles for each member type: 'Required', 'Optional', 'Disabled', or something like that. In this way, maybe it would replace the current "required" field.
• Perhaps a simpler (though less flexible) implementation would be a "Hide from Member Types" metabox, with simple toggles for each member type.
• Then we filter fields on the front end, in Edit mode. Similar in concept to your filter_fieldgroups(), though we could do it directly in the class instead of unsetting inside of a filter callback.
• Ideally, we'd also interface with the registration process. This is perhaps too big a task. It'd mean providing a member type selector at registration, and then dynamically showing the proper xprofile fields based on the member type selected.

I'd value your thoughts on the above, given your experience building and using bp-xprofile-for-user-groups.

[Offereins]

@boonebgorges Thank you for your kind words and your clear response. Keeping it to applicability for now, is fine with me. I'm all for the metabox direction, as I've done in my plugin.

Could you elaborate on the foreseen issues with applying this to both fields and fieldgroups? Because I can see great value in it being implemented for both, and my implementation with groups didn't see much/any trouble.

Your first two bullets touch a decision we have to make on what the best user experience would be, I think:

1. selecting member types that the field applies to
2. selecting member types that the field does not apply to
3. per-member-type selecting how the field applies (either optional, required or disabled)

My personal preference goes to option 1, since I imagine fields are more often specific to one member type than to all-but-one. Additionally I'm having difficulties finding a use case where a field would be required for one member type, but optional for another. Note that my plugin wasn't user tested, so I couldn't really tell which road would be best for an implementation in core.

@ Bullet 3: sure!

Concerning your fourth bullet, going beyond my experience, so thinking out loud: either 1) paged registration, 2) AJAX, 3) conditionally showing a member type's field(groups) with CSS: field containers would have a member-type-{all/$type} class and JS would listen for a selected member type, then switch the page's <body> member-type parent class.

I hope these are some constructive thoughts.

[boonebgorges]

Could you elaborate on the foreseen issues with applying this to both fields and fieldgroups? Because I can see great value in it being implemented for both, and my implementation with groups didn't see much/any trouble.

I'm concerned about UX when field and fieldgroup settings overlap. If you have group A with fields 1, 2, and 3; and if you set group A so that it's applicable only to member type 'foo', but set field 2 so that it's applicable only to member type 'bar', who actually sees field 2? In other words: how do settings cascade downward, and how do we show that in the UI? It's not impossible to do this, of course - we can dynamically change settings, and give warnings to users, etc etc - but it's much more complicated than simply having a single layer of settings.

A related idea: member-type-applicability applies only at the field level (not group), but we have a bulk-editing interface that allows admins to apply changes to entire groups at once. I think this might be easier than dealing with the cascading logic described above, and it accomplishes much the same thing.

My personal preference goes to option 1, since I imagine fields are more often specific to one member type than to all-but-one

This sounds sensible, and I think it makes sense to go with something simpler for the first version of the feature - we can build it out to be more robust later on if there is demand.

For registration, something like your option (3) is probably the best for the first version of the feature.

Offereins, if you are interested in taking the first swing at a patch for this - especially given your experience building a sorta-similar plugin - please let me know, and I will give you room to work before diving in myself :-D

[Offereins]

The following issues arise when developing this for a multiple-member-types-per-field situation:

• When multiple member types can be selected per field: how should this be stored in the database?
• When multiple member types can be selected per field: how does this translate to field types (optional/required), when different member types should have different field types?

For both issues counts: storing an array as data (either member types or field types) is bad when we would want to query profile fields per member type (see registration page suggestion).

Any suggestions on this or is it advised to discontinue the multiple-member-types track for now?

[boonebgorges]

I agree that storing an array is a bad idea.

I'm thinking something like this. Use bp_xprofile_add_meta() to add a member type to a given field. This allows you to have multiple member types for a given field:

bp_xprofile_add_meta( $field_id, 'field', 'show_to_member_type', 'student' );
bp_xprofile_add_meta( $field_id, 'field', 'show_to_member_type', 'teacher' );

This means that when you query for a single field's member types, you need to be sure to fetch the whole array ($single = false):

$member_types_for_this_field = bp_xprofile_get_meta( $field_id, 'field', 'show_to_member_type', false );
// array( 0 => 'student', 1 => 'teacher' ) 

In the loop, you will have access to a meta_query. (Not yet fully implemented; see [9722] and #6347.) So if you want to fetch all fields that need to be shown to the current user, it'll look something like:

$current_user_member_type = bp_get_member_type( bp_loggedin_user_id(), false ); // single=false
bp_has_profile( array(
    // ...
    'meta_query' => array(
        array(
            'object' => 'field',
            'key' => 'show_to_member_type',
            'value' => $current_user_member_type,
            'compare' => 'IN',
        ),
    ),
    // ...
) );

The API might look a little different here - you might, eg, pass a 'member_type' param to bp_has_profile(), and then prepare the 'meta_query' clause internally, but this gives you the idea.

When multiple member types can be selected per field: how does this translate to field types (optional/required), when different member types should have different field types?

At a glance, I'd think that assigning a field to multiple member types would mean that a user with at least one of the member type would then see the field. So, in the above situation, a user with member type 'student' would see the field in question. We'll have to make this as clear as possible in the UI, and we'll have to come up with a trick to make sure that fields with *no* member type associations are shown to *all* users.

[Offereins]

Thank you very much for the metadata lecture. How wrong of me to never consider storing array meta values as separate meta instances. Finally using *_add_meta() makes sense to me :)

Considering the last part of your reply, I don't think my point was clear: it concerns field requirement. Yes, only one member type should have to match, but how can we differentiate between field requirement types per member type? For example, how do we mark fields optional for teachers and required for students?

[boonebgorges]

Thank you very much for the metadata lecture. How wrong of me to never consider storing array meta values as separate meta instances. Finally using *_add_meta() makes sense to me :)

Awesome! :)

For example, how do we mark fields optional for teachers and required for students?

I have no idea. If we were going to do this, it should probably be part of the existing "Requirement" metabox, or perhaps it would replace it altogether. Maybe something like:

Setting    Member Type
[optional] Teachers
[required] Students
[none]     Foo

where the part in [brackets] is a dropdown, and the Member Type is a list of all member types? This would probably also need a button that'd quickly enable "require for all" or "optional for all" or something like that, with a single click. You'd also need a row that represents users with *no* member types, I guess?

If any UX mavens have brilliant ideas....

[Offereins]

So I ran into a UX maven the other day... Made me decide to split the requirement from the member-type assignment and maybe, just maybe, have at least the latter finished in time for inclusion in 2.3 (I know the deadline sorta passed).

On the member-type/requirement split: after giving it some thought, it makes sense that in the future, fields may get more member-type specific attributes than just Field Requirement, like Field Visibility for example. This means that tying member-type assignment and requirement together is restricting these future enhancements.

So with this in mind, I think it is best to setup member types for fields first. All other member-type specific field attributes can be added through plugins or later inclusion.

All said, here is the feature plugin for field member-types: ​https://github.com/lmoffereins/bp-xprofile-field-for-member-types

Details:

• Works with BP 2.2
• Assign member types through checkboxes in a metabox on the Edit Field page
• Metabox only shows up when there is any member type registered
• Profile fields are removed on a member's page through the bp_xprofile_get_hidden_fields_for_user filter, where other access restrictions are checked too.
• Plugin has it's own implementation of bp_has_member_type()

[boonebgorges]

Offereins - Thanks for working on this! The general strategy - splitting the metabox from Requirement - does make sense.

Unfortunately, we're past the due date for 2.3 and there's a fair amount of work to be done here (even just in converting it to a BP patch), but let's definitely do something along these lines for 2.4.

A couple of thoughts and questions:

1. When this is turned into a BP patch, should we continue to implement it as a filter on 'bp_xprofile_get_hidden_fields_for_user'? As a filter, it can be easily removed by a developer, which is both a good thing and a bad thing :)
2. The proposed UX doesn't address the case where you want a field to "apply" to users who don't have any member type at all. To be honest, I don't know how to handle this. Maybe an additional checkbox called "[no type]" or something like that?
3. Maybe instead of your "When no member type is selected..." description, all checkboxes could be checked by default? Then we could get rid of the gloss, I think.
4. We probably need some helper text here. Something like: "Make this field available only to the following member types". It's very hard to come up with phrasing that makes this clear, especially because it's easy to confuse with the idea of visibility. IMO, "apply" is not a clear enough word - we need something that means "only users of the following types will have the field appear on their profiles", but obviously that sentence is terrible :)

[Offereins]

Boone - Thanks for your fast reply and feedback. Too bad about the non-inclusion for 2.3, but the feature plugin will be available for those who need it, untill 2.4.

Concerning your questions:

1. I'm not aware of any WP/BP Core policy on internally applied filters. It may also be hardcoded into the function logic before the filter, as is done with visibility levels. I guess using a hook would at least either count for both the metabox and the filter or for none.
2. Missed that one. Will create an additional checkbox as you suggest.
3. Checking all items by default seems a little off for me. It has consequences for a) obligatory storing of field meta for all available member types on field creation, and b) new member type registration, which leads to confusing scenarios (do all fields immediately apply for the new member type?). Perhaps the gloss may seem to be a necessity.
4. Valid point. Will come up with alternatives, but perhaps others within the community may also offer their suggestions.

[Offereins]

Just shipped version 1.0.0 of my plugin: ​https://github.com/lmoffereins/bp-xprofile-field-for-member-types. Implemented are points 2 and 4-ish. Feedback is much appreciated :)

[boonebgorges]

Very cool, Offereins!

I just pushed a mod to my fork of your repo that changes the wording of the description and the "null" option to read like in Screenshot_2015-07-02_14-41-09.png​. The shorter description seems clearer to me; I like putting the "null" option at the end of the list; and I think that "users with no member type" is less ambiguous (to me, "none" could mean "no users").

I'm still concerned about my point 3 above. A few thoughts:

1. In BP 2.3, all fields are shown to all member types. When upgrading to 2.4, the default behavior should be the same. Otherwise, existing fields will disappear until the admin takes action. So either (i) we'll need a migration routine during 2.4 upgrade, or (ii) when get_xprofile_member_types() returns an empty array, we'll need to interpret it as "all" rather than "none", or (iii) we'll need to change the way the data is stored so that a field that has never had the "member types" value set (ie, legacy fields) will be distinguishable from those that have had the "member types" value set to "no types". Each of these options is unpleasant in its own way, but I think we have to do one of them.
2. I think it's correct behavior that all boxes would be checked when creating new fields. It's likely that, for most sites, most new fields will be intended for all member types. That is: limited-access fields will be the exception, rather than the rule.
3. It might be nice to have JS-powered "Select All"/"Select None" links, or maybe a checkbox column header for a similar purpose.

I'm particularly concerned about item (a) here. Offereins, do you have thoughts about which of these techniques is the least unpleasant? Or maybe you have a better idea :)

[Offereins]

Yup. I can see how your labeling clearifies the interaction with it. It just makes me think whether the xprofile meta table would be overloaded with irrelevant data when, as you say, fields will be intended for all member types most of the time. Besides that, I wonder what it means to have none of the boxes checked. Would the field become lost? Don't get me wrong, I like your implementation of the layout. I'm just not sure how it plays out data-wise.

As for point a/1), while that issue wouldn't exist with my implementation (*wink*), I'd like to go with option i, since it would be the least hacky and could also account for installations that have the plugin already installed. It was the first solution that came to my mind as well.

Option ii would be counterintuitive regarding the meaning of the (un)checked status of the member type boxes. This sort-of counts for option i as well.

For option iii I have no idea why/how that would be a good idea.

[boonebgorges]

Just so I'm clear: On your implementation, having no boxes checked means that the field shows for everyone? (I guess that's what your inline comment "no member types were assigned, so user validates" means.) I agree that this makes the technical implementation much simpler, but I find it pretty confusing from a user's point of view. I agree that there's not much purpose to having a field that's available to *no* member types, but at least it makes for a consistent interface.

[Offereins]

Yes, true, agreed. I vote for consistency. Perhaps @tanner can join in on this?

[imath]

@Offereins just tested your plugin, really interesting work, thank you.

About registrations:

1) i think we need to adapt core, your plugin is filtering bp_xprofile_get_hidden_fields_for_user but when no user id is set, this filter is never fired. So we should adapt bp_xprofile_get_hidden_fields_for_user(), else all "restricted to a member type" fields will always be displayed into the signup form.

see the patch attached to this ticket (5192.bp_xprofile_get_hidden_fields_for_user.patch)

But after applying this patch, you'll need to edit your plugin, else when on the xProfile Administration screen "the restricted to a member type" fields won't display.

see ​https://gist.github.com/imath/2360ad6c2b48a5bde160

2) Back on the ticket's description :

and to have different registration forms.

i think this can be done pretty easily and what i'm thinking of would not require css/js manipulation.
Let's imagine this: site.url/register/member_type_slug

See ​https://gist.github.com/imath/60c736673cc3edc804b9
(FYI ^^ is containing ​https://gist.github.com/imath/2360ad6c2b48a5bde160)

[tanner m]

Thanks for all your hard work here @Offereins! The code looks fantastic and is very clear to follow.

After looking this over I would like to propose expanding this functionality to support restrictions in general. I would love to see this functionality be written so that it can be expanded to support user role restrictions, membership type restrictions, etc.

With that in mind I have forked @Offereins plugin and updated to support a more general approach. You can find the fork here: ​https://github.com/tannerm/bp-xprofile-field-for-member-types.

I have updated the verbiage in the admin to use "Restrictions" instead of member types and I think this will make a clearer UI.

Let me know what you all think.

[imath]

The more i think of it, the more i think this feature is really really interesting for 2.4 !

Again, @Offereins did a great work.

Maybe i'm overthinking it, but i think the logic used is improvable. What the plugin's doing is it waits for the visibility to be set on *all* fields before querying again *all* fields to check if some needs to be removed because they don't match with the member's member type.

I think my brain would feel more comfortable (maybe it's working in a strange way :) ) if we would reverse the order of the logic. First remove the fields that don't match the member types and then for *these remaining fields*, check for their visibility.

More globally the registration thing in my previous comment shows that we should also be able to fetch the fields for a given member type *without* needing any user id. Ideally there should be a new argument in bp_has_profile() so it would be possible to use it in specific loops such as the registration one. Maybe something like :

function bp_has_profile( $args = '' ) {
	
	// Parse arguments
	$r = bp_parse_args( $args, array(
		'user_id'                => bp_displayed_user_id(),
		/* other arguments */
		'member_type'            => bp_get_current_member_type(),
	), 'has_profile' );

	if ( ! empty( $r['user_id'] ) ) {
		$r['member_type'] = bp_get_member_type( $r['user_id'] );
	}

In BP_XProfile_Group::get(), we could remove the fields that are not of the requested member types, and then apply the visibility to the remaining fields.

[boonebgorges]

@tanner - Thanks for your thoughts. It looks like your dropbox link has expired - I saw your screenshot the other day, but it's no longer available. Can you upload it directly to Trac?

I think I'm a fan of your UI innovation of separating out "This field is available to all members" from the type-specific selections. This clears up a fair amount of the confusion of the interfaces that Offereins and I had been working on. We'll need some JS that disables fields as appropriate, of course.

As for the question of a more general "Restrictions" field, I am a bit wary. Adding the ability to display content based on more than one criterion means that we're going to have AND/OR confusion. For example, if you have member types Student and Teacher alongside the WP roles, and you check the boxes Student and Author, does that mean that the field should be shown to all Students and all Authors, or only to those members who are both Students and Authors? Presumably the former, but what if one wants to do the latter? The UI here is complex enough when we're only worried about member types. My inclination is that, for 2.4, we should focus on a clear UI for that use case only, and think about making it more general (either in the same metabox, or with additional metaboxes) in future releases.

[tanner m]

Proposed UI

[tanner m]

@boonbgorges, I agree about the confusion that could be introduced with a more general approach to "Restrictions". I am not in any way saying that we should add support for anything other than Member Types. What I am proposing is that as we build it in a way so that plugin developers can extend it how they wish. They will then need to deal with clearing up that And/Or scenario but at least there would be a way for them to hook in and add that functionality.

Really all this means for us is that we store the information in a way that is friendly to other restriction types and add hooks and filters to allow others to do what they need to.

I think what we are doing here really has a lot of potential and I keep hearing from more and more people how they want to setup these kinds of custom restrictions.

Of course I'll be happy to contribute whichever direction we go, just my two cents. :)

In my fork of @Offereins plugins I have adjusted the the way the data is handled and have added the hooks and actions that I think would be needed to make this feature extensible.

[boonebgorges]

Hi everyone,

Thanks for your continued work on this.

What I am proposing is that as we build it in a way so that plugin developers can extend it how they wish.

As things stand, developers are welcome to introduce new metaboxes for additional types of restrictions. Then they can handle data validation, building UI, etc in a way that makes sense for their specific restriction. Let's go with the more limited UI for now, and if there's huge demand for a broader system after the initial release, let's talk about it then.

5192.diff​ is a first attempt at converting this feature to a BP patch. It's based heavily on work done by tanner and especially Offereins. There are a few important places where I've diverged from the previous work. Here's a summary.

• I've moved the CRUD actions to methods on BP_XProfile_Field, which seems logical to me.
• I removed the ability to set this data for XProfile Groups. Offereins, I gather from your code that you were kinda thinking of making it more broadly available, but I think that's a mistake for the first pass - we're then faced with all sorts of problems with cascading, as we've previously discussed. This is another thing we can address with a v2, if there's demand (though tbh I don't think it'll affect the data model - field_group settings will be a UI feature only, which is stored in the DB at the field level).
• I'm using kind of a wacky system for storing data. My primary goal was to build an interface and workflows that make sense. So let me explain those first, and then talk about how it works.
  1. By default, all fields (new and existing) are available to all member types, including the "null" type - users with no type. This means that the checkboxes are filled in for all new and existing fields.
  2. You can uncheck boxes, as you'd expect, to disable a field for given types.
  3. If you activate a new member type (via plugin), here's what happens. If an existing field is available for all types (all checkboxes are checked), then it continues to be available for all types, including the new type. If an existing field has limited availability (at least one checkbox is unchecked), then the new type will be *unchecked*. It seems to me that this is the behavior that most people would expect, since the default state of a field is to be available to everyone.

I've spent some time playing with various scenarios: new fields, existing fields, activating member types, disabling member types, etc, and the workflows described above all feel pretty natural to me. I would really love to get feedback on this.

The data model that makes it possible is, as noted, pretty weird. If all checkboxes are filled, then *nothing* is stored in the database. (This saves space: most fields will be available to everyone.) If *some* checkboxes are filled, then an xprofile fieldmeta entry will be saved for each. If *no* checkboxes are filled, then a *single* fieldmeta entry will be saved, with value '_none'. Yes, this is weird. But the $field->get_member_types() and $field->set_member_types() should make it completely unnecessary for anyone to access these values directly, and these methods behave in exactly the way you would expect.

UI and code feedback welcome!

[boonebgorges]

Oh, one thing I forgot to mention in the above: Offereins had originally used the 'hidden fields' hook to hide fields from users. This is a good technique for a plugin, but I wanted something more direct for BP core. So I'm filtering out the off-limits fields just after they're fetched in BP_XProfile_Group::get(). As it stands, this technique (and Offereins's technique, for that matter) will result in some database overhead during bp_has_profile() queries. If the basic technique looks good to everyone, I can add some cache priming that will minimize the overhead.

[tanner m]

Super, thanks @boonebgorges! I probably won't have time until Monday to check this out fully, but I'll get you some feedback as soon as I can.

[imath]

hi @boonebgorges, just tested it. Great work.

I guess my previous ​suggestion is not a good idea, so i won't insist anymore, although i really think a user_id shouldn't be required to fetch fields for a specific member type.

So to be consistent, as it's not possible to fetch fields for a given member type, i think we have 2 choices :
1- do not display fields restricted to one or more member types on the registration page (i don't think it's great).
2- or give a possibility to filter the fields by member types (see 5192.suggestions.patch)

About the UI: @offereins was adding a really interesting piece of information on the list of fields, i think we should have this. (see 5192.suggestions.patch)

It's not in my suggestions patch, and it's a detail but i think the meta_key should be 'member_type' instead of 'member-type', in other worlds the underscore instead of the dash just like the other meta_key we are using.

[imath]

requires 5192.diff to be applied first.

[boonebgorges]

In 10009:

Improve formatting of BP_XProfile_Group::get() docblock.

See #5192.

[Offereins]

Great job, Boone! For this moment I have the following feedback:

• The field groups were already talked about, so I'm really okay to leave them out for this one.
• I find it to be a clever move, using the array( '_none' ) versus the empty array() approach, considering using the default get/set methods should be sufficient for using the feature. This removes the need for an upgrade procedure. Yet, I have two things to note here:
  • Related to @imath's profile queries, I'm not too sure how this impacts using BP_XProfile_Query when using it with a/multiple user id(s) or a member_type query arg.
  • One thing that may confuse devs at first, is that the 'bp_xprofile_field_member_types' filter may contain '_none' and 'none', both not real member types.
• UI-wise, I think this is a very natural approach. Feels real nice!
• I'm really pleased with copying the i18n text-domain from my plugin :)

Questions:

• Should we disallow registration of member types with the '_none' or 'none' key?

[boonebgorges]

Thanks, all!

I guess my previous suggestion is not a good idea, so i won't insist anymore, although i really think a user_id shouldn't be required to fetch fields for a specific member type.

imath, my apologies. I didn't understand your previous suggestion until now. I think you're right - we should be fetching fields that are applicable to a specific set of member types. Let me work on this in the next patch.

As for how this affects registration: For v1, I think you're right that we should exclude restricted fields from registration. Devs can add custom UI to pull in these fields dynamically if they want. Maybe down the road we can add something like this to core.

One thing that may confuse devs at first, is that the 'bp_xprofile_field_member_types' filter may contain '_none' and 'none', both not real member types.

Ah, right. I think we need to keep 'none', because it's meaningful for devs ("users with no type"), but we should remove '_none' before it hits that filter - devs should never see it.

Should we disallow registration of member types with the '_none' or 'none' key?

Yes, I meant to do this, but I forgot :) Will be in the next patch.

I'm really pleased with copying the i18n text-domain from my plugin :)

Oops :)

I'll work on a second patch.

[Offereins]

Replying to boonebgorges:

One thing that may confuse devs at first, is that the 'bp_xprofile_field_member_types' filter may contain '_none' and 'none', both not real member types.

Ah, right. I think we need to keep 'none', because it's meaningful for devs ("users with no type"), but we should remove '_none' before it hits that filter - devs should never see it.

Err, sorry for the confusion. I took another look at your patch and there's no situation where '_none' hits the filter. So you can ignore it. Thanks.

[imath]

Replying to boonebgorges:

imath, my apologies. I didn't understand your previous suggestion until now. I think you're right - we should be fetching fields that are applicable to a specific set of member types. Let me work on this in the next patch.

No problem :) sorry for the confusion i've introduced. Actually 5192.diff is partly replying to my first concern: get the fields for the member type(s) before applying visibility levels to the remaining fields.
And yes my second concern was the ability to get fields for one or more member types without requiring the user_id. I'm happy we're ok with this possibility as it will be very easy then for a plugin to have registration forms by member types :)

Thanks a lot for your great work on this really interesting feature.

[boonebgorges]

[5192.2.diff] is an update with the following changes:

• Fixed textdomains.
• Switched to 'member_type' instead of 'member-type' for meta_key.
• To decrease confusion, use 'null' to signify members with no member types. Continue to use '_none' internally for fields that should be unavailable to all types.
• In bp_register_member_type(), enforce an array of illegal names.
• Added a "Member Type: Foo, Bar" label to the Profile Fields admin page.
• imath's suggestion: In BP_XProfile_Group::get(), instead of doing a post-query filter of fields to remove those that are not applicable to the current user's member type(s), do a pre-query check to get a list of the user's member_types, and the fields available for those types. The heavy lifting is done by BP_Xprofile_Field::get_fields_for_member_type(), which returns a multidimensional array that looks like this: [3] => ['student', 'teacher'], [5] => ['student', 'null'] etc, where 3 and 5 are field IDs and the array values are the member types for each field.
• Add 'member_type' parameter support to the bp_has_profile() function stack. The default value in bp_has_profile() is 'any', which is a special value that returns only those fields that are available to *all* types. 'member_type' is ignored if 'user_id' is non-zero. This setup means that restricted fields are unavailable during registration by default.

The new queries in get_fields_for_member_type() are a bit ugly. I will add some caching support - maybe even caching in a transient, so that all sites will get it, not just those running a persistent cache - if we are all agreed that the current strategy is a good one.

Thanks for playing along at home :)

[tanner m]

Looks good Boone!

Here are a few comments:

• I found the conditional statement on line 750 of class-bp-xprofile-field.php to be pretty confusing. I'm pretty sure all we need is: if ( ! array_intersect( $field_types, $member_types ) ) { ... }
• I like the UI on the profile field page, I think it is simple and clear.
• I think we should remove/change the Member Types label for the Name required profile field since it is not something that can be updated.
• I actually wonder if we should not have Member Type restrictions for the Primary field groups since those fields will be completed on registration before a Member Type is even assigned.

[imath]

@boonebgorges here's my feedbacks :

1/ thanks a lot for adding the possibility to get fields for a given member type even if the user_id is not set.

2/ thanks a lot for using member_type instead of member-type for the meta_key

3/ As discussed on ​slack, the _none value (meaning no checkboxes are checked) should generate a message to inform the field won't be displayed to any member.

4/ The base profile field Name (primary)(required) shouldn't have the metabox to set the member type, as it needs to always be output.

5/ Thanks a lot for BP_XProfile_Field->get_member_type_label(), i think this piece of info in the list of xProfile fields is really interesting, but :

• when a field concerns all member types, as it's the default value somehow, the list of member types labels doesn't need to be output imho.
• in other words, maybe we should only output the labels if not all the checkboxes are checked.

6/ I strongly disagree with @tanner about disabling the possibility to affect a member type to any field belonging to the first group of fields (the "registration" group of fields). As i suggest in point 4, we should only disable this possibility for the Name (primary)(required) field. I think it's important we leave the possibility to plugin authors to create multiple registration forms (eg: one by member type), which is a need specified as the 3rd point of this ticket's description. BTW it's pretty easy to achieve, now that it's possible to get fields for a given member type :)

7/ When no member types are registered :

• i think no labels should be output in the list of xProfile fields (users.php?page=bp-profile-setup) to be consistent with the fact that no metaboxes are displayed.
• any field should be output no matter the value of the member_type into the database, because the member types feature is kind of inactive in this case.

Thanks a lot for your great work so far :)

[Offereins]

I really like where this is going! Huge thanks for your work, Boone :)

In reply to ​@boonebgorges:

• Some tests seem to use $fg in the assertions where I'd expect $the_group to be used (in class-bp-xprofile-group).

In reply to ​@imath:

3. I've read the discussion on Slack and the idea of the message accompanying the all-non-checked state gets a +1 from me.
4. That restriction was already in my plugin and is adopted in the diff (bail on 1 === $field->id). No harm there.
5. +1, only labels for fields that are need an explanation.
6. I agree with imath.
7. Good catch! +1 as well, for both points. This thing we build can become complex, but we shouldn't forget there may be installations without any member type at all.

[boonebgorges]

Again, thanks for the great feedback, everyone - very, very helpful.

Changes in 5192.3.diff​:

• Updated the appearance of labels on the Profile Fields page. When a field is available for all types (unrestricted), I've removed the label. See the screenshot above. I have mixed feelings about this: it does make for less clutter, but I wonder if it will make people wonder whether the field is invisible to all types. I also changed the format of the label shown when a field is available for *no* types - see the red message in the screenshot.
• Added a dynamic notice to the metabox when all boxes are unchecked. See screenshot above. The message appears/disappears as you'd expect when checking/unchecking boxes. For all of these labels, I'd welcome suggestions for improved wording.
• I added a thin layer of caching for get_fields_for_member_type(). It's not straightforward to cache entirely, due to complications in the way that member types are registered with BP at runtime (making cache invalidation complicated), and newly registered fields should be available for all member types at the time tha they're registered.
• Don't show a Member Type message for field 1.
• In BP_XProfile_Group::get(), skip processing 'member_type' param if no member types are registered.

I agree that we should allow admins to restrict fields in the "Base" group. I see tanner's logic here, but I agree with the others that it's an undue restriction, and that there are good use cases for having restricted fields in group 1.

Any thoughts are welcome!

[Offereins]

Just wondering whether we should account for member types that have been unregistered. Picture a scenario were a member type was selected, but now it only spooks around in the meta table without a tie to the real world. It is only relevant when we're fetching labels, but we may need do an exists() check. Or not?

[boonebgorges]

Replying to Offereins:

Just wondering whether we should account for member types that have been unregistered. Picture a scenario were a member type was selected, but now it only spooks around in the meta table without a tie to the real world. It is only relevant when we're fetching labels, but we may need do an exists() check. Or not?

Oh yeah, good call. I think we shouldn't bother with the meta table itself. Plugins should be responsible for cleaning up their own messes, and in any case we wouldn't want to delete content, since admins may toggle a plugin on and off for testing, etc. However, we should definitely have a check before outputting the label. And in fact, the code as it stands will throw an error in this case. See lines 621-622 of get_member_type_label() in ​https://buddypress.trac.wordpress.org/attachment/ticket/5192/5192.3.diff. This should get a sanity check for the next patch.

[imath]

Thanks a lot @boonebgorges just tested .3.patch and imho we're good to go for a first pass :)

• About the labels, as a red message is informing when a field is unavailable, i don't think the user will have any doubt. If nothing is shown, than any member can see the field.

In the list of xProfile fields : i think we could simplify the message "Unavailable to all member types" in favor of "Unavailable to all members". The first one is a bit "not completely exact" as some could understand that a member with no member type should see the field. I can think of a situation when the admin has not used the "Users with no member type" choice yet, or when he's a bit tired :)

Same for "Fields with no members type..." It's more "Fields with none of the above choices..." Or we could simplify with "This field is unavailable to all members"

But i'm not sure to be the best one to suggest english messages :)

[tanner m]

This is looking great! Big thumbs up from me. :)

Also, I understand not limiting restrictions the whole primary field group. I did not really like that restriction, just wanted to make sure we were going to have a consistent registration flow.

[boonebgorges]

5192.4.diff​

• Some fixes to unit tests
• Simplified the red "no member type" messages - thanks for the good advice, imath.
• Cleaned up docblocks
• For get_member_type_label(), make sure that (a) the types are listed in alphabetical order, and (b) the "null" option always appears last.
• Regarding Offereins's note about inactive member types: this was actually already implemented, because $field->get_member_types() doesn't return invalid types :)

I showed the feature off during this week's dev chat, and people seemed to like it, so I guess this is a last call for comments before initial commit.

[Offereins]

Does this need more eyes or should this just get into the open for testing in real admin work flows?

[boonebgorges]

Replying to Offereins:

Does this need more eyes or should this just get into the open for testing in real admin work flows?

I'll commit it later today or tomorrow. I think we're ready for wider testing in trunk.

[boonebgorges]

In 10022:

Allow xProfile fields to be restricted to users belonging to one or more member types.

A new metabox on the profile field edit panel allows the administrator to
select the member types to which the field is applicable. Admins can also
choose to have a field apply to users who do not belong to any member type.
Information about a field's member type associations is displayed on the
Users > Profile Fields page, alongside each field name.

During registration, the only fields that are displayed are those that are
unrestricted - that is, those available to all users, regardless of member type
(or lack thereof).

This changeset introduces a number of new methods on BP_XProfile_Field that
developers should use to manipulate member type information:
get_member_types(), set_member_types(), and the static
get_fields_for_member_type(). In addition to member types that have been
explicitly registered, 'null' is a pseudo-type representing users who do not
belong to a member type.

This changeset introduces a blacklist of illegal member type names. By default,
the blacklist includes 'any', 'null', and '_none'. Use the
'bp_member_type_illegal_names' filter to add names to the blacklist.

Props Offereins, boonebgorges, tanner m, imath.
See #5192.

[imath]

In 10030:

Add missing changes to admin-rtl.css

See #5192