Add function to send single invitation

[dcavins]

The current send invitations function, groups_send_invites() takes an inviter ID and a group ID and attempts to send all of the non-sent invitations from that user in that group.

This patch introduces a new function, groups_send_invite_by_invitee(), that sends a single invitation keyed by the invitee ID and group ID. This is useful for sending specific invitations and resending invitations (from a group's "invitations" pane or from the group admin in wp-admin).

I'm attaching two versions: one is built on the current invitation schema, the other is built on the new schema proposed by @imath in #5911. (Short explanation: currently only one invitation to a group should exist for any invitee. Imath proposes that several users could potentially invite the same user, so creates multiple invitations--this method makes for a better user experience in my opinion, because users aren't stumped when they can't create an invitation to a group because someone else already has.)

In my "pre-5911" version, groups_send_invite_by_invitee() only requires the invitee ID and group ID (since only one invitation should exist). In my "post-5911" version, the function requires invitee ID, inviter ID and group ID (so that we send/resend the correct version).

This function is required for progress on #5336.

[dcavins]

Does not require math's changes in #5911

[dcavins]

Requires math's changes in #5911

[boonebgorges]

Thanks for this, dcavins. Looks pretty good, but I have a few questions/requests:

• I'd like to see more unit tests for the various failure conditions: empty group id, empty invitee id, invited user does not have an invite of the right sort, maybe non-existent group/user (though you don't have bail conditions for this latter part).
• Let's return the bool value of `$member->save().
• I understand why you're reusing the 'groups_send_invites' action, but I have mixed feelings about firing it in two different places. Can/should we rewrite one of these functions (groups_send_invites() or groups_send_invite_by_invitee()) to use the other one internally?

[boonebgorges]

Oh, and one more quick comment: can we go with the pre-5911 patch for now, and then create a patch that can be applied when/if 5911 is resolved, which will bring it in line with that change?

[dcavins]

Replying to boonebgorges:

Oh, and one more quick comment: can we go with the pre-5911 patch for now, and then create a patch that can be applied when/if 5911 is resolved, which will bring it in line with that change?

Yes, as long as we require the $inviter_id arg now in either case, then updating for #5911 would only require the internal addition of the $inviter_id argument in groups_check_user_has_invite().

And, yes, groups_send_invites( $user_id, $group_id ) could definitely use the new function internally. I'll write it as such and add some more test cases. Re the groups_send_invites action. It would have to live in the child function (by_invitee), but its arg is an array (I made my arg array( $invitee_id ) so that it would be cast as expected). Can you think of any reason that calling it six times for six single invites would be not acceptable for some action that is expecting a cohort of invited IDs in a single array?

Re tests: I think testing for non-existent groups/users should fall to groups_check_user_has_invite() which is what I'm using to verify that the invitation is legit. I'll add some tests for that function, too.

Thanks for the quick feedback!

[dcavins]

Add groups_send_invite_by_invitee

[dcavins]

This new patch modifies groups_send_invites() to use the new function groups_send_invite_by_invitee() internally.

Adding a number of tests for those functions made me think about when these functions (and the functions they rely on) should return true or false. Here are the decisions that I made in this patch:

groups_notification_group_invites()

• Returns false when the group or member objects don't contain ids.
• Returns false when the submitted inviter_id doesn't match the recorded inviter_id in the groups_members db.
• Returns true if the notification has already been sent, and we haven't specified resend.
• If it completes, it returns the result of wp_mail().

groups_send_invite_by_invitee()

• Returns false if the required parameters aren't provided.
• Returns false if an invitation doesn't exist from that group to that user.
• Returns true if groups_notification_group_invites() returns true and $member->invite_sent gets updated (or doesn't need to be)

groups_send_invites()

• Returns false if the inviter can't send invitations from this group
• Returns true if all calls to groups_send_invite_by_invitee() return true, false if any fail

I also modified bp_groups_user_can_send_invites() to accept a $user_id. (It previously only worked off the logged-in user id.) So I added a number of tests for it as well. (I'd like to apply a bp_groups_user_can_send_invites() check to groups_invite_user() as well. I think the check is happening in the template or js file, but it seems like it should happen at the point of recording the new invite.) Since I modified it, I figured I'd better write tests for it, too.

Please let me know if these return values seem to make sense and are BP-typical.

Thanks in advance for your comments. (Especially about the new tests. I was hoping to cover the major cases, so let me know if I missed any, overdid it or just plain did it wrong.)

[dcavins]

Add groups_send_invite_by_invitee, docs fix

[dcavins]

I've created a separate ticket for the addition of the user_id parameter to bp_groups_user_can_send_invites().

See #6031.

[r-a-y]

I've committed the enhancements dcavins made to bp_groups_user_can_send_invites() from #6031.

6025.04.patch is refreshed to take this into account. I haven't reviewed the rest of this ticket yet.

[DJPaul]

I think this is too late for 2.2 now.

[DJPaul]

:( Much sadness that I have to punt it again. @dcavins, moving this to 2.4, and we can take a look at it early.

[dcavins]

Refreshed patch

[DJPaul]

@dcavins I have no idea what is going on with this ticket, but I've moved it into future release. If you want to progress it, please refresh the patch, and re-summarise what it does and what the next steps are. Thanks.

[dcavins]

This work will happen as part of 6210. At this point, this ticket should be considered a duplicate.