Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 9 years ago

Last modified 8 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 9 years ago.
6462.02.patch (733 bytes) - added by imath 9 years ago.
6462.03.patch (2.3 KB) - added by dcavins 9 years ago.
Adds unit tests to check draft invitation removal.
6462.04.patch (1.4 KB) - added by dcavins 9 years ago.
Check for draft invites when checking that invites have been removed.

Download all attachments as: .zip

Change History (19)

#1 @imath
9 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
9 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
9 years ago

@imath
9 years ago

#3 @imath
9 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
9 years ago

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

#5 @dcavins
9 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
9 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
9 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
9 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
9 years ago

Adds unit tests to check draft invitation removal.

@dcavins
9 years ago

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

#9 @DJPaul
9 years ago

  • Severity changed from major to blocker

#10 @dcavins
9 years ago

  • Keywords dev-feedback added

#11 @imath
9 years ago

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

#12 @johnjamesjacoby
9 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
9 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.


9 years ago

#15 @DJPaul
8 years ago

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