search

CFCommunity-net / buddypress-group-restrictions

Prevent group access to a group based on Member Type (BP 2.2+)
GNU General Public License v3.0
0 stars 1 forks source link

Restricting Group Access #1

Open BoweFrankema opened 9 years ago

BoweFrankema commented 9 years ago

1: Site admin selects which xProfile fields are "available" for a Group Admin to restrict access for.

For example the site admin chooses xProfile field "Your Relationship with CF" as a field that can restrict group access.

In this example the field contains the following values:

"Enable access to members based on the profile field "Your Position in our Company"

[ ] I have CF [ x ] My partner has CF [ ] A family member has CF

*Community members who have filled in My Partner has CF on their profile, are able to join your group!

3: Displaying the Groups/Access Control message

There are a few possible ways this functionality could be implemented.

Hide Restricted Groups - Only the groups that the users has access to are shown. This would requiring filtering the Groups Loop per user. I think this could be performance heavy but it would be an interesting option for sure. This way a user simply does not know what they are missing and would lead to a more pleasant experience (being unable to join an interesting group might be unpleasant)

henrywright commented 9 years ago

Thanks for the detailed spec. I've started to implement the functionality. Just a few quick questions:

  1. Instead of selecting xProfile fields, I think the site admin should select member types. We should try to get rid of the xProfile field (once the data has been migrated) now that the Member Types API is available for use.
  2. Will these options be checkboxes or radio buttons? For example, checkboxes allow group admins to allow > 1 member type to have access to the group. Radio buttons will allow just one member type.
  3. I think we should do bullet point 3 and also hide on the front end. That way, users get the good experience, but if somehow a 'join group' button ever does get displayed on the front-end, then the appropriate server side checks will kick in and the user will be told they're not allowed in the group.
BoweFrankema commented 9 years ago

1: Agreed! I wrote this description before we talked to @imath about the Member Types 2: Good question.. there is a bit of logic involved and maybe this needs a few admin options. But for CFCommunity I want to give users with a certain Member Type the ability to restrict access to a group with the SAME member type. So if their member type was "I have CF" then they could tick a box:

[ ] Only allow access to this group to other community members who filled in "I have CF" on their profile.

This is slightly different then in my description but this would make sense for the users perspective. Then from the admin side of things you could show a radio box?

3: If you could filter the group directory in such a way that would be awesome! Also it would be great if we would stay add a notice to the single group page to make this clear to the users in the group

"This group is only viewable for people who have filled in "I have CF" on their profile"

This would explain the concept to our users and would make it clear which groups are "open" and which are restricted!

henrywright commented 9 years ago

Thanks for the clarifications. I just need a little more clarification on the following:

1: Great :+1:

2: So regarding the following box that can be ticked, would this be displayed at the point of group creation ready for the group creator to tick if they wanted? I'm wondering where to display the option.

[ ] Only allow access to this group to other community members who filled in "I have CF" on their profile

Also, if someone with member type "Other" or "I work with someone who has CF" created a group, would a box be displayed for them to tick?

3: No problem :)

BoweFrankema commented 9 years ago

2: I've looked at the hooks BuddyPress provides during group creation and ideally you would add this checkbox during the "Group Privacy" screen. But there does not seem to be a hook at that step. Not sure if there is a reason for that but maybe an action could be added to core.

If that's not possible it could be added to the first step of group creation process where you could use the hook:

bp_after_group_details_creation_step

It would be nice if we could prevent adding an additional screen to the Group Creation just for this, but maybe there is a reason there are almost no hooks provided during group creation?

henrywright commented 9 years ago

I like your thinking here. I also think the group privacy section would be preferable. also the user could always go back and change their mind at a later date.

I'll investigate if it can be done without the need to request a new hook in core...

henrywright commented 9 years ago

I see what you mean about there being nothing available under the Privacy Options heading but I have an idea (might not be a good idea but let me know what you think)...

Within step 2. Settings, there's an action hook bp_before_group_settings_creation_step. I could introduce a new section using that hook. The section would sit above the Privacy Options heading. We could have a new heading for the section "Group Restrictions" (or something else if you prefer).

Screenshot

BoweFrankema commented 9 years ago

Awesome, that would be close enough to get the job done :-)

On Friday, April 3, 2015, Henry Wright notifications@github.com wrote:

I see what you mean about there being nothing available under the Privacy Options heading but I have an idea (might not be a good idea but let me know what you think)...

Within step 2. Settings, there's an action hook bp_before_group_settings_creation_step. I could introduce a new section using that hook. The section would sit above the Privacy Options heading. We could have a new heading for the section "Group Restrictions" (or something else if you prefer).

[image: Screenshot] https://camo.githubusercontent.com/fa24ba6fec60efeb4825a36b8f7fcb558894d3ce/687474703a2f2f692e696d6775722e636f6d2f52445548426f672e706e67

— Reply to this email directly or view it on GitHub https://github.com/CFCommunity-net/buddypress-group-restrictions/issues/1#issuecomment-89068057 .

henrywright commented 9 years ago

I've just pushed a first draft of the plugin. It's ready for testing once imath has migrated the info from the xProfile field to member type data. You may find things you need to change, so just give me a shout if needed.

I'll need to generate a .pot file too.

imath commented 9 years ago

I think we need to manage this in javascript. The restriction is not needed in case of an hidden group. If i understood well Bowe's need the group restriction feature consist in transforming the public of private groups into hidden groups if current user has a member type that is not concerned by the one set into the group. So javascript would dynamically add a select box if the radio button of private or public are clicked. If hidden is clicked, then the select box disappears.

So the privacy screen is the good place and we don't need any hook to inject the content :) just enqueue some js if create step / manage screen is privacy and let the javascript magic happen ! Then when saving the screen, if the member type is set groups_update_groupmeta else groups_delete_groupmeta.

imath. http://profiles.wordpress.org/imath/

Le 3 avr. 2015 à 01:30, Bowe Frankema notifications@github.com a écrit :

Awesome, that would be close enough to get the job done :-)

On Friday, April 3, 2015, Henry Wright notifications@github.com wrote:

I see what you mean about there being nothing available under the Privacy Options heading but I have an idea (might not be a good idea but let me know what you think)...

Within step 2. Settings, there's an action hook bp_before_group_settings_creation_step. I could introduce a new section using that hook. The section would sit above the Privacy Options heading. We could have a new heading for the section "Group Restrictions" (or something else if you prefer).

[image: Screenshot] https://camo.githubusercontent.com/fa24ba6fec60efeb4825a36b8f7fcb558894d3ce/687474703a2f2f692e696d6775722e636f6d2f52445548426f672e706e67

— Reply to this email directly or view it on GitHub https://github.com/CFCommunity-net/buddypress-group-restrictions/issues/1#issuecomment-89068057 .

— Reply to this email directly or view it on GitHub.

BoweFrankema commented 9 years ago

That is a pretty darn smart solution Mathieu! And you're right.. this would not apply to hidden groups so your solution makes a lot of sense. What do you think @henrywright?

henrywright commented 9 years ago

Hey @imath

If i understood well Bowe's need the group restriction feature consist in transforming the public of private groups into hidden groups if current user has a member type that is not concerned by the one set into the group.

When you say transform a group into a hidden group, do you mean truly hidden as in completely invisible to the subset of users that do not meet the member type criteria? I was under the impression that these users could still view the group and the restriction was just on them 'joining' as a group member.

So javascript would dynamically add a select box if the radio button of private or public are clicked. If hidden is clicked, then the select box disappears.

I think the JavaScript/ajax is a good idea if the group creator clicks hidden in the privacy settings. I'll add that.

BoweFrankema commented 9 years ago

The way I understood it was slightly different, maybe you can clarify again @imath? But I thought you meant:

So the functionality would stay the same as we have now, we would just use javascript to show the checkbox based on Group Privacy selection. Please correct me if I'm wrong :-)

@henrywright: In an ideal scenario we would hide restricted groups inside the directory. But I'm not sure how that would impact performance (if at all?). You would need to make a custom loop that would check the member type and group meta to see which groups would be able to join. This might be a bit much?

The restricted groups should not be "hidden groups" as specified in BuddyPress core. So a Public/Private group can be restricted but have a Privacy level of Public. The reason for this is that it would then be viewable for friends who have the same Member Type (Henry is part of Group "I have CF" just like me. If it was a hidden group I would never see that).

henrywright commented 9 years ago

Hi @BoweFrankema

The restricted groups should not be "hidden groups" as specified in BuddyPress core.

Right. That's the way I originally understood the spec. Thanks for confirming :)

henrywright commented 9 years ago

So in terms of the next step...

I'll go ahead and implement the javascript suggested by @imath. It will detect if a user has clicked on "This is a hidden group" and if that is true, the 'restriction checkbox' will be hidden from view. If the user clicks on either of "This is a public group" or "This is a private group" then the checkbox will be shown on screen.

henrywright commented 9 years ago

Also, just to pick up on

In an ideal scenario we would hide restricted groups inside the directory. But I'm not sure how that would impact performance (if at all?)

Perhaps we could filter the group loop using bp_parse_args() by setting some meta query conditions. I don't think that will impact performance too much. If there is a performance hit, it will happen once when the group query is done. We won't need to re-query groupmeta for each loop item. Thinking ahead, ajax pagination could be an issue - but that could be looked into.

BoweFrankema commented 9 years ago

Sweet! Also one other thing.. Looking at your code it seems you've limited the checkbox to the member type "Has CF". But ideally we would let other member types restrict access in the same way. So if you're member type "Has Family CF" than the checkbox would say "Only allow members Has Family CF to join this group". Basically it's all contextual to the member type of the user :-)

henrywright commented 9 years ago

Right, I see! I'll make sure to add that in :)

BoweFrankema commented 9 years ago

RE: Performance: I am using Redis for database caching and HHVM. The problem would be that all these queries would be unique because of the user_id. So I think Redis would have a hard time caching any of those queries. That being said maybe some core devs know an answer to this. It would be worth giving it a shot I think!

henrywright commented 9 years ago

Never used Redis so please shout if I'm wrong but I also don't think Redis will work well with dynamic stuff like user ids because it does its caching at the page level.

henrywright commented 9 years ago

I've introduced the js we discussed. When the hidden privacy option is clicked, the restriction checkbox disappears from screen. When the public or private options are clicked, the checkbox reappears.

I've also made things context sensitive with regards to restricting a group.

A few things need to be done from my end:

  1. Test
  2. Clean up
  3. Regenerate the .pot

At your end, please feel free to test by adding some member type data to some dummy users in your site. Also, we need to consider the performance aspects we spoke about to see if going ahead with group loop filtering would be a good idea (or not).

henrywright commented 9 years ago

I've now managed to test at my end and clean up any bugs I found. I've also regenerated the .pot so localisation is now possible.

As mentioned over on Slack, I'll be away this weekend but will be around again on Tuesday (after the bank holiday) to pick up where we've left things.

BoweFrankema commented 9 years ago

Thanks so much for all the work you've done so far! Installed and activated the plugin without a problem. I'll create a few new issues to report on some things I've found :-)

imath commented 9 years ago

Ok @henrywright, you're right : the good filter is bp_before_has_groups_parse_args (using the bp_parse_args feature) i'll let you deal with the groups listing, the goal is to build a group meta query ressource : https://codex.wordpress.org/Class_Reference/WP_Meta_Query

I'm sure you'll have some fun playing in this area :)

henrywright commented 9 years ago

As discussed with @BoweFrankema, I've held off removing items from the loop in favour of outputting a simple message to show each group's restriction.

I think that's everything in this thread covered so branch 1.0.2 should now ready for testing.

imath commented 9 years ago

Reason is ? It's best to show a group that won't be reachable or performance considerations (due to the fact each groups query will contain a meta query) ?

henrywright commented 9 years ago

Even though certain groups will be restricted, Bowe wanted all users to know that these groups exist. Performance wasn't really an issue in the end.

I think the plan is to think about introducing some custom group loops at a later date elsewhere on the site. For example "These are the groups available to you". They'll be the ones to make use of a meta query

BoweFrankema commented 9 years ago

In the beginning there's a two reasons for this

  1. Emphasise and make it clear to our community that this is a option when you create a group (this is a much requested feature on our Facebook page and important for a lot of people)
  2. Showing these groups in the directory to everyone will show more activity in our community which would hopefully lead to more activity (first weeks/months are always the hardest)

Once the community takes off and there's a lot of groups/users and people are aware of the functionality then hiding the groups would make more sense :-)

imath commented 9 years ago

Fine with me :)