Add "manage invitations" pane to group admin screen

[dcavins]

It's not obvious to me how to track outstanding group invitations as a site admin. This patch adds a new pane to the single group admin screen (in the WordPress admin area) allowing site admins to view and remove outstanding group invitations.

This could easily be plugin-ized, so let me know if it's not appropriate for core. I could also write the code for adding a second pane for outstanding group membership requests if desired.

-David

[dcavins]

Patch adding single group "manage invitations" pane

[boonebgorges]

Very nice. Let's look at this for 2.0.

[dcavins]

I thought I'd go ahead and add an outstanding membership requests list to the same meta box. However, it looks like the only way to get the outstanding requests for a group is via the bp_group_has_membership_requests() loop. Is that true? (It'd work out, I think, except for handling pagination. Although all I'd be using from the loop is the $requests_template->request->user_id so it's probably overkill.)

It wouldn't be hard to write a new query to get the outstanding requests, but I want to be sure I'm not reinventing the wheel. Thanks for any comments and advice.

[boonebgorges]

The nice thing about the bp_group_has_membership_requests() function is that it sets up the template loop, so you'll have access to the template functions as well as pagination. Is there a reason you don't want to use that?

Related: we have a template loop for group invites too: bp_group_has_invites(). I see that you wrote your own database query to get outstanding group invites, though we already have a this template loop and groups_get_invites_for_group(). Is this just because you needed limits/pagination? All things being equal, I'd prefer to use the functions that we've already got, improving them as necessary, rather than creating new ones (especially in the case of one-off database query methods).

What do you think?

[dcavins]

I agree. I don't like adding new things that'll have to be maintained later. With the invites loop, I seem to remember that I couldn't use it because it is scoped by the group and by the user who is viewing the list (and this list needs to show all outstanding invitations for the group). I'll double-check that. Also, I'm not sure that bp_group_has_invites() handles pagination. I don't see any code for it in the class, but I could be missing it.

I'm a little concerned about passing pagination to the bp_group_has_membership_requests() loop, too. It's looking for $_REQUEST['num'] which seems a little too generic if there are multiple paginated loops on the same page. Maybe the better answer is to modify the class to use a more specifically named parameter so it can be used here as well as in the front-end templates.

Thanks for thinking about this problem.

[boonebgorges]

it is scoped by the group and by the user who is viewing the list

I'm not sure that bp_group_has_invites() handles pagination

It's looking for $_REQUESTnum? which seems a little too generic if there are multiple paginated loops on the same page

Good calls. Let's fix all three of these rather than introducing new stuff.

[dcavins]

Excellent. I'll make some patches.

Thanks for the comments.

[dcavins]

Here's a first pass at adding pagination to bp_group_has_invites.

Full disclosure: I changed some default behaviors. As it is now, when a user visits a group's "send invites" pane, he is only shown invitations that he has created (even if he is a group or site admin). This causes problems for group members who can't figure out why they can't invite someone to a group (because someone else already did, but they can't see that). I changed the behavior so that all outstanding group invitations are show to all members, but only the inviter or group or site admins can "remove" an invite. I hope this is a move that will be OK with everyone.

Also, clicking "send invitations" will try to send all of the unsent invitations for the group. I've gotten some complaints from my users that they don't know to click the "send invites" button, so I'm wondering about that, too. One thought is to remove it, and add a "confirm invite" button in the action div next to "remove" for each invite. That doesn't handle bulk actions very well, though. Another approach is to add a dialog that reminds the user to click the button if they're leaving the page by navigating away. I can add either, if you think they're a good idea.

Finally, since I reordered the arguments get_invite BP_Groups_Group::get_invites I left groups_get_invites_for_group() in place and added a new function, groups_get_all_invites_for_group, because I didn't want to break uses of the old function.

Please offer any comments, and I'll rework the code as necessary.

[dcavins]

First pass at adding pagination to group invites

[boonebgorges]

Hi dcavins -

Thanks very much for the patch. Some comments:

• Your mods to get_invites() will break backward compatibility. We can't change the order of existing parameters (user_id and group_id). And we can't change the structure of the default return values; it's currently returning an array of IDs, so we must continue to do so, at least by default. Either you'll have to build in some backward compatibility stuff for this method, or write a new method that does what you need it to do (and, optionally, migrate existing uses of get_invites() in BP over to the new method, deprecating the old one).
• Related, my gut tells me that you should be able to do what you're trying to do in get_invites() by using BP_Group_Member_Query. That class has a param is_confirmed that might do it; if not, perhaps you could modify that class to add the necessary parameters. Conceptually, that seems like a better place for an invites query than as a static method on the BP_Groups_Group class.

• "I changed the behavior so that all outstanding group invitations are show to all members, but only the inviter or group or site admins can "remove" an invite." - I guess the worry is that there will be some privacy implications, if I know that someone else in my group has invited certain individuals. I have some thoughts about different ways to handle this, but it's really a separate issue from pagination, and I don't see a reason why the one issue should necessarily hold up the other one (tell me if I'm wrong about this). Would you mind removing your changes related to this and then posting them in a separate enhancement ticket?

• Likewise with the "Send Invites" issue. I agree that this is lousy UX, and to be honest I don't totally understand why the process of creating an invitation is so separate from *sending* it. But I think this should be discussed separately, for sanity's sake.

• What's the motivation for creating the new invites-loop.php template? Just consistency with other components?

• The pagination logic itself seems good, so if we can sort out some of the stuff above, I think we can definitely get at least that much in for 2.0.

[dcavins]

Hi boonebgorges-

Yep, one of the things I was looking for was input on the best way to get what we need for the admin interface without breaking compatibility with plugins and themes. I was hoping that leaving the function groups_get_invites_for_group() working would do it. (The only call to BP_Groups_Group::get_invites in BP is groups_get_invites_for_group and my new function. But of course returning an array of arrays instead of an array of user ids will never work, as I found when the send invites function broke.)

Pagination really wasn't the crux of this ticket to begin with. (So I'll move that to another ticket as well.) This ticket was about adding an invitation management piece on the WordPress admin side group management page. It diverged along the way because I realized that I had to be able to get all outstanding invitations for the group and add pagination. The core piece I still need for this to work on the WP admin side is the ability to get all invites for the group (unscoped by user). We'd like to use bp_groups_has_invites() but that loop is scoped by user.

bp_group_has_invites( $args = '' ) {
	global $bp, $invites_template, $group_id;

	$defaults = array(
		'group_id' => false,
		'user_id' => bp_loggedin_user_id()
	);

I could pass in a user_id of null or zero, I guess. Would that be an acceptable way to get around the user scoping?

Invite visibility: Yes, I'll move that out. (Moved to ticket #5422.)

Send invites UI: I can only imagine that the extra step is a confirmation step, so you can fix a mistake before sending an awkward invitation. Yes, I'll open a new ticket. (#5421)

New loop template: I added the new loop template to simplify AJAX page generation when changing pages. (The existing similar AJAX pagination functions use the related template file to create the new markup and then send it back to the browser. The existing template for invitations contains more than just the member list and pagination--the parts that need to be refreshed--which is what I moved to the new template file.)

Thanks for your feedback.

[dcavins]

I've created ticket #5423 to cover pagination.

On the question of optionally scoping the bp_group_has_invites() loop to show all invitations, I was thinking that I could pass in a user ID of 0 or null, but that seems kind of poor. Would it be reasonable to add an optional argument like scope that could be set to user (which would match the current behavior) or admin (which would remove the user scope)? So the function would start out with:

bp_group_has_invites( $args = '' ) {
	global $bp, $invites_template, $group_id;

	$defaults = array(
		'group_id' => false,
		'user_id' => bp_loggedin_user_id(),
                'scope' => 'user'
	);

Please let me know if a change like that would be OK.

[boonebgorges]

See my ​https://buddypress.trac.wordpress.org/attachment/ticket/5423/5423.02.patch for an alternate strategy. 'inviter_id' accepts three types of values:

1. empty value, to get only items with no inviter (0, false, array())
2. array of user IDs, to get invites sent by those users
3. the string 'any' to get memberships where inviter_id != 0

So, it's similar in spirit to what you've suggested, but implemented at a lower level. What do you think?

[boonebgorges]

I do like the idea of adding the admin pane, but we've run out of time for 2.0.

[dcavins]

That's OK. We laid the necessary groundwork in 2.0 by updating the membership requests and invitations code, so adding the group admin pane should be ready for 2.1.

[dcavins]

Uses BP_Group_Member_Query to add invitations and requests to wp-admin groups

[dcavins]

I've rewritten the "unconfirmed members" pane in wp-admin's single group edit screen to take advantage of the work done on BP_Group_Member_Query in BP 2.0.

This adds a new pane that lists outstanding invitations and membership requests for a group. Currently available actions: Remove an invitation, accept or reject a membership request. I'd like to add the ability to send/resend invitations, but the core functions will need to be made more flexible (or add a new specific utility function for which the user id and group id can be specified--at the moment logged-in user id and group id specify the invitations that will be sent: ​https://buddypress.trac.wordpress.org/browser/trunk/src/bp-groups/bp-groups-functions.php#L822 ).

Thanks for your feedback. Also, I'm not expecting this to make it into 2.1.

[DJPaul]

Thanks for the patch, dcavins. I/we'll try to get some feedback to you as soon as we can, hopefully before the end of next week.

[DJPaul]

dcavins, I am extremely sorry but we've run out of time in the 2.1 dev cycle to review your patch and iterate on it and get it into BuddyPress trunk. Thank you for your hard work on this, and we'll look at this ticket as a priority as soon as we start on 2.2.

[dcavins]

Replying to DJPaul:

dcavins, I am extremely sorry but we've run out of time in the 2.1 dev cycle to review your patch and iterate on it and get it into BuddyPress trunk. Thank you for your hard work on this, and we'll look at this ticket as a priority as soon as we start on 2.2.

No problem, DJPaul. I didn't really expect this minor patch to make it into 2.1.

[boonebgorges]

dcavins - Thanks for your patience as we review this patch.

A few comments:

• You note that it's not possible to resend invitations because our functions for doing so are linked to the logged-in user. Which functions are you talking about? (your link to trunk doesn't work because line numbers are changed :) ) If appropriate, we can and should either make the existing functions more flexible or create new ones. Please feel free to roll that in as part of this patch, or open a separate ticket for it.

• Bulk management interface. You've gone with radio buttons for handling these invitations, with the processing taking place when you click "Save Changes". IMO this is a bit clunky. Better would be to have "Send" and "Remove" links - links which would refresh the page (unless you built AJAX support for them, which would be nice but wouldn't have to be part of v1). In addition, you could have checkboxes and a bulk action dropdown, similar to what we see on Dashboard > Posts. (I am aware that you were probably trying to keep parity with the "Manage Members" interface. But (a) that interface is less than stellar, and (b) the dropdowns in the right column refer to user status, *not* to actions/verbs, so the choice of a radio/dropdown is more semantic in that case.)

• I see that you have copied the terrible bp_groups_admin_create_pagination_links() function. I don't remember what I was thinking when I wrote that it will be deprecated soon. But in any case, instead of reproducing just so we can have slightly different text, could you please add the necessary parameters (maybe passed as part of an $args array in the third param position) necessary to make this function usable for your pagination?

[dcavins]

In #6025, I've proposed a function to send a single invitation that is necessary for ultimately sending/re-sending invitations.

Next up is building the interface.

[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/