Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#5032 closed enhancement (fixed)

Sending group invite to member that has requested group membership doesn't work

Reported by: Mike_Cowobo Owned by: boonebgorges
Milestone: 1.9 Priority: normal
Severity: normal Version:
Component: Groups Keywords: has-patch 2nd-opinion
Cc: mike@…

Description

Symptom: when a group admin invites a user to a group for which the user has already requested membership, the user gets notified, but the invitation doesn't show up in her group invitations list.

Cause: When a user requests group membership, groups_send_membership_request() creates an entry in the bp_groups_members table. However, when a group admin sends an invitation to this member, groups_invite_user() only checks if the user is already a member, or already has an invite. It then continues to make a new entry in bp_groups_members. groups_send_invites next updates the first entry (the request) to have invite_sent.

Result is that when a user looks for his invitations and BP_Groups_Member::get_invites() is called, the method checks for an entry that has an inviter ID and invite sent, and will find nothing.

Going around and making sure that groups_send_invites() updates the right entry (by passing in the membership id) won't work, as that will simply relocate the problem to groups_accept_invite(), which opens the first member entry, and confirms member status. If that happens to be the membership request, the subsequent call to BP_Groups_Member::check_for_membership_request() will be false, and we end up with a confirmed membership and still a group invite in the table.

Simplest solution is just to check for an existing membership request when a user is invited to a group, and accepting that request instead of going through the invitation process (after all, the member has requested membership, why ask her again to become a member?)

The problem is the same the other way around - when a user is invited for a group and then requests membership.

I have attached a patch that does the following:

  • Automatically accept membership request when admin invites a user that has requested membership
  • Automatically accept invitation when user requests membership to a group she is invited to
  • Show "Group invite accepted" and redirect user when she accepts an invite by requesting membership

Attachments (3)

5032.001.2.diff (4.9 KB) - added by Mike_Cowobo 7 years ago.
5032.001.diff (4.9 KB) - added by Mike_Cowobo 7 years ago.
5032.003.diff (10.7 KB) - added by Mike_Cowobo 7 years ago.
Changes 'Request Membership' to 'Accept Invite' for invited users

Download all attachments as: .zip

Change History (10)

@Mike_Cowobo
7 years ago

#1 @boonebgorges
7 years ago

  • Keywords 2nd-opinion added

Thanks for the report and patch, Mike_Cowobo. This bug has bothered me before.

I wonder if your solution might be a little *too* transparent. If I'm invited to a group, then I request membership to the group, I'm added directly to it. This is a little bit confusing. The "ideal" solution might be to modify the helper text throughout. So, (a) when I've been invited, I no longer see "Request Membership" buttons - the text changes to "Accept Invitation". (b) In the other direction: if I request membership to a group, and a group admin goes to invite me to the group, she will see some text on the Send Invites screen to the effect of "This user has requested membership. Sending an invitation will automatically add the user to the group." Finally, (c) the message added via bp_core_add_message() would be updated in these cases to reflect the actual state of affairs: "Since you've already received an invitation to join this group, your membership request has automatically been accepted.", etc.

(b) and (c) will be easy. I'm not sure how easy (a) will be in the context of group directories - we want to make sure to fetch request/invitation data all at once, so we don't slow down the pageload.

Am I overthinking this?

#2 @Mike_Cowobo
7 years ago

  • Cc mike@… added

I don't think you are over thinking this, as this would actually solve a greater annoyance, namely that users still see 'Request Membership' when they're already invited.

You're right that changing the button (a) would entail an extra walk to the db for every private group a user is not a member of yet. This is silly, because the only real difference between a request and an invite is that invites have an inviter ID and requests do not. I have collated these checks into a method BP_Member::get_invites_and_requests(), which is called by groups_check_for_membership_request_and_invite(). Checking for either then boils down to $membership_status['has_invite'] or $membership_status['has_request'].

For a group admin inviting a user (b), I'm not sure if that is so straight forward, or necessary for that matter. What happens now, is that when an admin selects a friend to send an invite to, the invite is created in the db in the ajax call, but not sent until the admin clicks the 'Send Invites' button. If a user has requested membership, this request has to be accepted before the invite is added to the db. Changing this behaviour would entail either changing the response from the ajax call (perhaps a separate listing for users-with-membership-requests, with a 'Accept Request' button) or adding separate logic for membership-request users (no new invite is created, but when invites are sent, membership requests are accepted). I think this is overdoing it for such a rare event.
Better alternative is perhaps accepting the request when the invite is made, and returning a notice about how and why the user is now a member of this group, instead of adding her to the invite list...
(Or.. we could filter the friend list to simply not show users that have an outstanding membership request to this group, and completely prevent this from happening in the first place..)
(Or am I over thinking this?)

The message shown to a user when she has accepted an invite (c) does not have to change if (a) is implemented: the buttons will already read "Accept Invitation", and notifying the user that "Because you were already invited [...]" is plain redundant. Only scenario in which this could pose a problem, is when a user is on a view with a 'Request Membership' button, gets invited by somebody, and clicks 'Request Membership'. She will get notified that she has accepted the group invitation. But she will also still have a 'You have been invited ...' message in her notifications. So it should be alright anyway..

Then, lastly, everything now happens through the 'request-membership' action, because in groups, it is quite deeply entrenched as an exception to the access check for non-members. Adding accept-invite would just add clutter. Also, the scenario in the previous paragraph is caught exactly because of this.

In the patch I changed the label for the 'Request Membership' tab to 'Accept Invitation' when a user has a pending invite. However, accepting an invite through group_slug/request-membership is now not nonced, because then we'd need to add another screen for accepting invites. Currently, navigating to the 'Accept Invitation' tab in the group results in accepting the invitation. Because there is nothing else to do when accepting an invite (like adding a comment), it seems highly redundant to add yet another screen ("You are about to accept the group invitation!") with just another 'Accept Invitation' button on it (while the user could just have clicked the 'Accept Invitation' anywhere!)

@Mike_Cowobo
7 years ago

Changes 'Request Membership' to 'Accept Invite' for invited users

#3 @boonebgorges
7 years ago

  • Milestone changed from Awaiting Review to 1.9
  • Type changed from defect (bug) to enhancement

Thanks for putting the thought into this, Mike_Cowobo, and for the patch. I think it's very good, though I do want to think a bit more about my case (b) - I'm less worried about "overdoing it for such a rare event" than about creating a user experience that makes sense. Either way, let's definitely tackle this for 1.9, because your patch looks like it gets us at least 80% of what's needed.

#4 @boonebgorges
7 years ago

Mike_Cowobo - Sorry for the delay on this one, I've been waiting for a time when I could devote adequate attention. It's pretty complex.

I'm going to go pretty much exactly with your workflow, but I'm going to change the logic of it a bit, for efficiency's sake. Instead of adding additional database methods/wrapper functions for fetching requests + invites, I'm adding flags for is_pending (in other words, "has sent request") and is_invited ("has been invited") to group query objects. This'll greatly reduce the overhead required during group loops.

As for my (b) above, I've opted to include a short message in the markup return by the AJAX request when checking the appropriate box on the Send Invites page. I think that's good enough.

Thanks again for your work on this thorny but important issue.

#5 @boonebgorges
7 years ago

In 7440:

Add redirect_to URL param support to groups_screen_group_invites()

This allows custom redirects after the acceptance/rejection of a group
invitation, for increased flexibility when generating Accept/Reject links
throughout BuddyPress.

See #5032

#6 @boonebgorges
7 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 7442:

Improve UX flow around overlapping group invites and requests

This changeset introduces a number of improvements to the flow surrounding
invitations and membership requests to private groups:

  • When a user has previously requested membership in a group, add a note to that effect when the user is being invited to the group via the Send Invites interface.
  • When a user has previously requested membership in a group, automatically accept that invitation and add the user to the group when an invitation is sent by an existing group member.
  • When a user has previously been invited to a group, buttons in the group directory and in the group header should read Accept Invitation instead of Request Membership

The end result is a more logical user experience with regard to joining private
groups.

Fixes #5032

Props Mike_Cowobo

#7 @boonebgorges
7 years ago

In 7453:

Fix logic error introduced into group template setup in r7442

Template variables were getting overwritten because of incorrect placement
in the loop.

See #5032

Note: See TracTickets for help on using tickets.