Skip to:
Content

Opened 22 months ago

Closed 17 months ago

Last modified 17 months ago

#6784 closed enhancement (fixed)

Group Types API

Reported by: mercime Owned by: boonebgorges
Milestone: 2.6 Priority: normal
Severity: normal Version:
Component: Groups Keywords: has-patch has-unit-tests
Cc: shane@…, bowromir@…, smsjun@…

Description

Akin to Member Types, this feature would be a great addition to the Group components

Attachments (4)

6784.diff (22.1 KB) - added by Mamaduka 21 months ago.
6784.tests.diff (19.4 KB) - added by Mamaduka 21 months ago.
6784.2.diff (42.0 KB) - added by boonebgorges 19 months ago.
6784.metabox.01.patch (3.4 KB) - added by dcavins 18 months ago.
Add meta box to wp-admin group edit screen for changing group type.

Download all attachments as: .zip

Change History (43)

#1 @imath
22 months ago

Should we close this as a duplicate of #4017 ?

#2 @boonebgorges
22 months ago

  • Milestone changed from Awaiting Review to 2.5

No, I think that #4017 is about group tags and the like. This ticket is very specifically about group types, which happen to be implemented as a WP taxonomy.

@Mamaduka posted an initial patch on #4017. His comment there:

This is first pass for Groups Type taxonomy API, it's very similar to Member type API.

  • Introduces API group_*_type() functions.
  • New groups_type_exists() function, which makes it possible for groups_set_type() and groups_remove_type to accept string or array of types.
  • Introduces group_type, group_type__in and group_type__not_in arguments for BP_Groups_Group::get().

Patches: 4017.04.diff 4017.04-tests.diff

Let's put into 2.5 for discussion.

#3 @boonebgorges
22 months ago

Thanks for getting this started, Mamaduka! Initial thoughts.

  • Can we work on function naming? I would prefer not to introduce any new functions with the prefix groups_. For the sake of parity with member type functions, I suggest bp_register_group_type(), bp_get_group_types(), etc. I know that this means ditching the bp_groups_ prefix, but I think that the consistency with member types (and WP core functions) is more important.
  • The idea behind groups_type_exists() is a little dangerous, IMO. If someone passes bad data into groups_set_type(), I think we should tell them about it rather than just eliminating the bad data. bp_set_member_type() only allows for a single member type, and returns false when it's not found, which is at least *some* feedback. If we want to have a multi-set method, we should have more fine-grained error-reporting - or maybe we should just return false if *any* of the supplied types don't exist.
  • In the case of member types, when BP detects that types have been registered, UI appears so that admins can set a member's type. What is our position regarding UI for group types? The situation is a bit more complicated than with member types: I think that in the majority of cases, site admins should be the only one messing with member types, but in the case of groups, you may sometimes want group admins to have control over it. I definitely think we should have some wp-admin UI as part of the first revision, but should we also put something into front-end group creation/editing?

#4 @imath
22 months ago

Before i dive more in detailed into @mamaduka 's very promising patch.

I hardly see the difference between a Group Type and a Group Tag, reason of my first comment to this ticket. But i like the idea of using the "Type" terminology. So i'm fine with it, but maybe we could define what we expect a Group Type to be, if it's not a tag :)

Is it a hierarchical taxonomy ? I guess some users could be interested to "tidy" groups according to a type and a "subtype" for instance.

Point 3 of Boone's latest comment is very important i think: the big question about "Group Types" is: Is it a taxonomy only the Super Admin can control or is it a taxonomy each group admins can control ?

imho: every Super Admins should be able to decide whether to allow group admins to add new group types and/or simply use existing ones. Can a group admin choose more than one type for his group ?

At first sight "Groups" functions are looking like "Members" ones, is there any interest of having a more generic API members and groups (and why not blogs one of this days!) could use ?

Quickly saw a "has_directory" stuff in the patch, i guess just like members this is requiring a kind of dynamic "forbidden name" to avoid slug collision with a group name and probably some adjustments inside BP_Groups_Component->setup_globals() method..

#5 @boonebgorges
22 months ago

I hardly see the difference between a Group Type and a Group Tag, reason of my first comment to this ticket. But i like the idea of using the "Type" terminology. So i'm fine with it, but maybe we could define what we expect a Group Type to be, if it's not a tag :)

Technically, there's probably little (or no) difference. But I see a huge difference in the way they're used, which probably translates to a difference in the UX. Group Tags are folksonomical ways for group admins to describe their groups. Their primary purpose is discovery: if I see a group I like, and the group is tagged 'wordpress', I can easily find related groups. Groups are likely to have many tags.

In contrast, Group Types are an organizational tool, and in many (most?) cases should not be changed by group admins. Groups will generally have a single type. Types could be used in template hierarchies, or to define different URL schemes, or in nav menus (top-level Groups with a child nav item for each Type).

Think about post types and post tags. WordPress stores them differently, of course, but they *could* be stored the same way. Yet they're very different in terms of intended use.

#6 @Mamaduka
22 months ago

Thanks @boonebgorges @imath for feedback.

Facebook uses both and we can all agree that most of the people are more familiar with FB groups feature that any other social network tool. Group Types are pre-defined (in BP case it can be Super Admin or Administrator) and anyone can choose one group. Group Tags have low priority and you can use them same way like in WP.

I'm happy to provide patches for both functionality and maybe we can make API more reusable.

https://cldup.com/zty3eckERm.jpg

#7 @imath
22 months ago

Ah! I trust you @Mamaduka as i don't use Facebook that much :)

Think about post types and post tags

Thanks a lot @boonebgorges i now see why it's different.

@Mamaduka Let's focus on Group Types for 2.5, because i think the UI part will be easier for this feature, as Group Types are registered using code, a select box in a Group Admin meta box + Group create/edit screen should do the job, unless i missed something.

In my previous comment, i think only the last 2 paragraphs can still be considered ;)

#8 @mercime
22 months ago

  • Summary changed from Groups: Add Group Types to Groups Types API

This ticket was mentioned in Slack in #buddypress by boone. View the logs.


22 months ago

#10 follow-up: @Mamaduka
21 months ago

Fist of all sorry for late update.

I started refreshing the patches for back-end API, with suggestions from @boonebgorges and @imath.

Unfortunately we'll have a problem, if we want to use bp_*_group_type(s) naming convention, because BP already has function name bp_get_group_type(). It return group status, while I found it very misleading and we should probably deprecate it with this release, this won't help us to use suggested function name.

Maybe we should stick we original patch function names or use bp_groups_*_type(s) function names?

#11 in reply to: ↑ 10 @boonebgorges
21 months ago

Replying to Mamaduka:

Fist of all sorry for late update.

I started refreshing the patches for back-end API, with suggestions from @boonebgorges and @imath.

Unfortunately we'll have a problem, if we want to use bp_*_group_type(s) naming convention, because BP already has function name bp_get_group_type(). It return group status, while I found it very misleading and we should probably deprecate it with this release, this won't help us to use suggested function name.

Ugh ugh ugh. Can we use bp_groups_get_group_type() then? I hate this, but I think it's better than just groups_. Just pick something and write the patch, and we can discuss (and search and replace) later on :)

@Mamaduka
21 months ago

#12 @Mamaduka
21 months ago

  • Keywords has-patch has-unit-tests added

Patches are updated.

6784.diff remove groups_type_exists() function and adds check for illegal names.

This ticket was mentioned in Slack in #buddypress by mamaduka. View the logs.


21 months ago

#14 @r-a-y
21 months ago

  • Milestone changed from 2.5 to 2.6

@Mamaduka - Sorry we didn't get to this for 2.6.

It looks like it's very close to committing though, so we'll be sure to look at this in the next release.

#15 follow-up: @Mamaduka
21 months ago

Do you think it will make sense to continue working on this as "feature as plugin". Would love to iterated on UI and get some feedback before 2.6.

#16 in reply to: ↑ 15 @boonebgorges
21 months ago

Replying to Mamaduka:

Do you think it will make sense to continue working on this as "feature as plugin". Would love to iterated on UI and get some feedback before 2.6.

Yes, if it's technically possible, it'd be ideal! If you put it on Github, please link to the repo here so that others can follow along.

#17 @shanebp
21 months ago

  • Cc shane@… added

This ticket was mentioned in Slack in #buddypress by mamaduka. View the logs.


19 months ago

#19 @mercime
19 months ago

  • Summary changed from Groups Types API to Group Types API

This ticket was mentioned in Slack in #buddypress by boone. View the logs.


19 months ago

#21 @DJPaul
19 months ago

@Mamaduka Did any work happen on this, or is this the latest patch still waiting for us to review it?

#22 @Mamaduka
19 months ago

@DJPaul latest patches are good for API. Have unfinished separated patches for UI, but any feedback on existing ones would be great. I can refresh those after feedback and add UI patches.

#23 @boonebgorges
19 months ago

@Mamaduka Thanks for your work on this. Looking mostly good.

6784.2.diff updates your patches as follows:

  • In the tests, create the user fixture in setUpBeforeClass() to reduce a bit of overhead.
  • Update documentation to read 2.6.0, along with some other typo fixes
  • Register bp_group_type taxonomy in BP_Groups_Component::register_taxonomies(), so that it's only registered when the Groups component is active.

@Mamaduka Can you say more about the UI stuff you're working on? The API stuff will definitely be committed separately, but we may want to wait until the UI stuff is in patch form, so that any major modifications to the API that may be necessary can be made prior to initial commit.

This ticket was mentioned in Slack in #buddypress by mamaduka. View the logs.


19 months ago

#25 @Bowromir
19 months ago

  • Cc bowromir@… added

#26 @boonebgorges
18 months ago

Bump. @Mamaduka Any progress on the UI end of things? Even partial work would help to move things along. Thanks very much!

#27 @boonebgorges
18 months ago

  • Keywords 2nd-opinion added

It would be a real shame for this ticket to get bumped from 2.6. The API part of it is pretty much ready to go.

I am willing to put some time in over the next week or so to get something workable for this release, but first I'd like to gauge everyone's feelings about what a minimal UI for 2.6 would be.

As a reminder, Member Types shipped in 2.2, and the only admin UI was a Member Type metabox https://bpdevel.wordpress.com/2014/12/08/member-type-api/

Doing the same thing for groups seems like the bare minimum we could get away with. However, groups are a bit more complicated than members in some ways. For instance, we have a front-end group creation process. Should group type selection be included in this? What about on the Manage tab? Certainly there will be many cases where the site admin will not want group admins to be able to change their group types - this was the motivation for not exposing Member Type stuff to users. But I bet it'll be a lot more common of a use case for the Group Type to be user-configurable. I think we should definitely introduce a flag for this in the long run (something like group_admin_can_edit when registering a member type? and then we provide default UI), but I don't know whether people think it's a blocker for 2.6.

This ticket was mentioned in Slack in #buddypress by boone. View the logs.


18 months ago

@dcavins
18 months ago

Add meta box to wp-admin group edit screen for changing group type.

#29 @dcavins
18 months ago

@boonebgorges: There's a tiny typo in bp_groups_get_group_types(), that should be fixed when you commit the changeset. The third line should read:

$types = wp_filter_object_list( $types, $args, $operator );

(The variable is mistyped as $type in 6784.2.diff, so filtering via the $args array fails.)

#30 @boonebgorges
17 months ago

  • Keywords 2nd-opinion removed
  • Owner set to boonebgorges
  • Status changed from new to assigned

Thanks, @dcavins.

So, 6784.2.diff won't work as-is. It allows group types to be registered with 'has_directory', but there is no actual support for 'has_directory' in our router. I'm going to tear it out of the patch and open a separate ticket for it. See #6286, [9723] for the Member Type implementation.

#31 @boonebgorges
17 months ago

In 10766:

Introduce Group Types API.

Modeled on the Member Types API, Group Types allow developers to register
arbitrary group categorizations, and assign one or more of these types to a
given group. BuddyPress stores this information in a custom taxonomy on the
root blog.

Group queries can be filtered by group type, by using the group_type,
group_type__in, and group_type__not_in parameters in the bp_has_groups()
template loop function stack.

Props Mamaduka, boonebgorges, dcavins.
See #6784.

#32 @boonebgorges
17 months ago

In 10767:

Normalize documentation and array definition whitespace after [10766].

See #6784.

#33 @boonebgorges
17 months ago

In 10768:

Declare the $types property on BP_Groups_Component.

This helps to avoid PHP notices when there are no group types registered.

See #6784.

#34 @boonebgorges
17 months ago

In 10769:

Add bp_groups_register_group_types action.

Paralleling the bp_register_member_types action, this hook provides a
convenient place for plugins to register their group types safely.

See #6784.

#35 @boonebgorges
17 months ago

In 10770:

Add Group Type metabox to Group admin panel.

When group types have been registered, the Group Type metabox allows site
administrators to select the group type for a given group.

Props dcavins.
See #6784.

#36 @boonebgorges
17 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

I've opened #7064 to discuss has_directory etc. Other group type features should get their own tickets too.

I think we've pretty much resolved this ticket for 2.6. Thanks for the help, everyone!

#37 @boonebgorges
17 months ago

In 10772:

Specify user_email and user_login for fixture in group type tests.

Earlier versions of the WP test suite were causing users to be created with
duplicate credentials, resulting in unexpected WP_Error objects.
See #WP33968, #WP35199.

See #6784.

#38 @boonebgorges
17 months ago

In 10773:

More adventures in creating unit test fixtures.

Second verse, same as the first. See [10772].

See #6784.

#39 @sharmavishal
17 months ago

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