Skip to:
Content

BuddyPress.org

Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#6462 closed regression (fixed)

Group invites cannot be removed if not sent

Reported by: imath's profile imath Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 2.3 Priority: highest
Severity: blocker Version:
Component: Core Keywords: has-patch commit dev-feedback
Cc:

Description

@dcavins,

Was testing the group invites and noticed a pretty annoying problem as it's a regression from 2.2.

When you go in the group's send invites screen eg: site.url/groups/single-group/send-invites/

Once you selected your friends using the checkbox list on the left, you cannot change your mind and unselect any friend.
Clicking on the remove link of the member's entry is not working neither.

I know you've been working in this area, so could you check what's going wrong ? I will check on my side.

Attachments (4)

6462.01.patch (480 bytes) - added by imath 10 years ago.
6462.02.patch (733 bytes) - added by imath 10 years ago.
6462.03.patch (2.3 KB) - added by dcavins 10 years ago.
Adds unit tests to check draft invitation removal.
6462.04.patch (1.4 KB) - added by dcavins 10 years ago.
Check for draft invites when checking that invites have been removed.

Download all attachments as: .zip

Change History (19)

#1 @imath
10 years ago

The problem only happens when you have not send the invites. Let's say you select a friend, then change your mind, then the invite is not removed.

#2 @imath
10 years ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

Ok i see the problem is coming from in r9810 where to uninvite a user it has been changed from :
!BP_Groups_Member::delete( $user_id, $group_id )

to

! BP_Groups_Member::delete_invite( $user_id, $group_id )

I understand the logic, but the problem is that BP_Groups_Member::delete_invite is checking for invite_sent = 1 And when i'm changing my mind, invite is not sent yet.

So i thought about coming back to !BP_Groups_Member::delete( $user_id, $group_id ) to solve the issue (see 01.patch), but i think a better plan is to adapt BP_Groups_Member::delete_invite so that we get rid invite_sent = 1 as i think the check upon inviter_id != 0 is enough (see 02.patch).
Actually i'd prefer this second option because i don't knwo if using BP_Groups_Member::delete( $user_id, $group_id ) could have an impact on a request made by a user. It looks like today (and it was the case in 2.2) it's not possible to select a friend who requested to be a member of the group (on a side note, should be best to not display this user in the list of friends or display a more interesting feedback)

@imath
10 years ago

@imath
10 years ago

#3 @imath
10 years ago

FYI 02.patch is passing the unit tests :)

But 01.patch fails on :

  • test_bp_groups_uninvite_user_leave_memberships_intact
  • test_bp_groups_uninvite_user_leave_requests_intact

#4 @imath
10 years ago

  • Summary changed from Group invites cannot be removed anymore! to Group invites cannot be removed if not sent

#5 @dcavins
10 years ago

02.patch seems like the right answer to me. We're still checking that the user is pending (is_confirmed = 0) and isn't a membership request (inviter_id != 0). (Although just inviter_id != 0 would be enough, because the inviter_id is set to 0 when the invitation is accepted. But leaving it in is reassuring. )

Thanks for catching this bad error on my part.

#6 @imath
10 years ago

  • Keywords commit added; 2nd-opinion removed

you're welcome &np we are humans! If you feel 02.patch is the right answer, go ahead and commit :)

#7 @DJPaul
10 years ago

Is this function, who's SQL you are proposing to change, an existing function, or a new one introduced in 2.3?

#8 @dcavins
10 years ago

It's not a new function. The problem was introduced when I made some changes to the various "delete" functions in a separate ticket: #6209.

02.patch does change the behavior of the method from "delete only sent invites" to "delete an invite regardless of whether it's sent or not". If it's a concern, we can add a third argument, BP_Groups_Member::delete_invite( $user_id, $group_id, $type = 'sent' ) to maintain the current behavior for that method (but be able to change it).

However, the functions groups_uninvite_user(), groups_delete_invite() and groups_reject_invite() all used BP_Groups_Member::delete() before, so the proposed change would maintain the current behavior for those functions.

@dcavins
10 years ago

Adds unit tests to check draft invitation removal.

@dcavins
10 years ago

Check for draft invites when checking that invites have been removed.

#9 @DJPaul
10 years ago

  • Severity changed from major to blocker

#10 @dcavins
10 years ago

  • Keywords dev-feedback added

#11 @imath
10 years ago

@dcavins That's ok for me. Thanks a lot for adding the unit tests.

#12 @johnjamesjacoby
10 years ago

In 9900:

Tests/Groups:

  • Introduce test for deleting "draft" invitations
  • Remove inline variable assignments
  • Remove unused variables
  • Move $now calculations out of date() calls
  • Move one-time use $args arrays into function calls

Props dcavins. See #6462.

#13 @johnjamesjacoby
10 years ago

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

In 9901:

Groups: Updates to delete_invite() method:

  • Add brackets around if condition
  • Break query apart into separate lines for better readability
  • Add inviter_id != 0 condition to query (note that this means not using $wpdb->delete())

Fixes a regression (from r9810) causing group membership invitations to be irreversible if the form was not submitted first.

Fixes #6462. Props dcavins, imath.

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


10 years ago

#15 @DJPaul
9 years ago

  • Component changed from API to Core
Note: See TracTickets for help on using tickets.