Nouveau: Inviter cannot remove group invitation

[dcavins]

As reported here:
​https://buddypress.org/support/topic/cannot-remove-group-invites/

Using the Nouveau Template, the inviter cannot delete an invitation they have extended. The issue is that the ajax.php file specifically only allows group admins to delete invitations. My opinion is that site admins, group admins and the inviter should be able to delete invitations. I'll attach a patch that allows those three cases.

[dcavins]

Allows inviter and site admins to delete group invitations.

[imath]

Hi @dcavins thanks a lot for your patch, I believe this might also be the case for BP Legacy. Could you check if it is the case ?

About the patch it looks good, but I’d feel safer if :

• $current_user was renamed $current_user_id
• the if inside the foreach would use strict comparison `===ˋ,
• we had a unit test for this.

Is this a side effect of the BP Invites API we should fix it 5.1.0 ?

[ekadevau]

As the original bug poster I tried to merge the patch (manually) into my test environment. After merging, the group view /groups/ does not load, it stays in "loading group" state. I tried twice to rule out c/p errors. There is, though, no PHP error (what I'd expect when I break the php file). Can anybody reproduce this? Unfortunately I don't have a git environment here to do proper merges.

[dcavins]

Hi @imath-

This issue is limited to Nouveau--Legacy works as expected.

The issue was exposed in 5.0, but only through a series of quirks. In BP 4.4, the user who sent the invite couldn't access the delete button, but should have been able to. In a commit from two years ago, djpaul added strict checking to an in_array() check in bp_nouveau_prepare_group_potential_invites_for_js(): ​https://github.com/buddypress/BuddyPress/commit/e97284f6f8a772e1bb7f899c1fa10ed0db88ca96#diff-1a38eaecaee735b5f109d2057ef15e93

The function: ​https://github.com/buddypress/BuddyPress/blob/e97284f6f8a772e1bb7f899c1fa10ed0db88ca96/src/bp-templates/bp-nouveau/includes/groups/functions.php#L180
What he didn't know was that the inviter_ids param as returned by Nouveau was a string value, but the logged-in user id is an integer, so the check failed, meaning that the inviter couldn't attempt to delete an invitation that he or she sent (though it would have worked at that time because the delete action handler was too permissive).

Boone added the "is_admin" permission check here, which doesn't mirror the logic in the can_edit check in bp_nouveau_prepare_group_potential_invites_for_js(): ​https://github.com/buddypress/BuddyPress/commit/82eb8c3d316fc13c4f5ffcc828726374d24b6ce8

And I changed the logic that returns the inviter_ids to use the new API logic (they are now integers), ​https://github.com/buddypress/BuddyPress/commit/d38562d6334003984d6382a3145f5d48eefe2f69, which, by making the strict check succeed, exposed the mismatch between the prepare and delete logic.

Ha, so it was a series of small changes that added up to the current state.

Anyway, I'll update my patch to add your suggestions, imath, and also ensure that the can_delete logic is the same in the prepare and delete steps for Nouveau.

Re a unit test, I'm not sure how to write a unit test following our typical conventions for this, since this is an interface + AJAX issue. Any ideas?

[dcavins]

Replying to ekadevau:

As the original bug poster I tried to merge the patch (manually) into my test environment. After merging, the group view /groups/ does not load, it stays in "loading group" state. I tried twice to rule out c/p errors. There is, though, no PHP error (what I'd expect when I break the php file). Can anybody reproduce this? Unfortunately I don't have a git environment here to do proper merges.

It sounds like you're having an error in the AJAX handler. Try replacing the entire file's contents (I mean the file bp-templates/bp-nouveau/includes/groups/ajax.php) with this version which is working on my test environment:
​https://raw.githubusercontent.com/dcavins/buddypress-wp-svn/331cd892f535c403770ebc413feb60dd4c4c23fc/src/bp-templates/bp-nouveau/includes/groups/ajax.php

Thanks!

[ekadevau]

Replying to dcavins:

It sounds like you're having an error in the AJAX handler. Try replacing the entire file's contents (I mean the file bp-templates/bp-nouveau/includes/groups/ajax.php) with this version which is working on my test environment:
​https://raw.githubusercontent.com/dcavins/buddypress-wp-svn/331cd892f535c403770ebc413feb60dd4c4c23fc/src/bp-templates/bp-nouveau/includes/groups/ajax.php

Thank you, apparently I was struggling with the merge process, which is a bit embarrassing ;-) Your version works like a charm. I'll wait for your update to test and hopefully deploy it to my prod environment.

[imath]

Awesome, thanks a lot for your work, investigations and explanations about this @dcavins 💪. I think we should fix it in 5.1.0.

[dcavins]

Use the same logic in the "delete invitations" routine as was used in the "can edit" permissions calculation.

[dcavins]

I've just attached a simpler patch that uses the same logic in the "delete" routine as was used in the "can_edit" permissions check.

I also fixed what I suspect was a mistake, where the error message suggests that we are checking for membership, but the logic was checking for an outstanding membership request. Neither case is that likely, because both actions (joining a group or a member both requesting membership and being invited to a group) would cause the invitation to be removed, so I'm not sure what problem that check is avoiding. I did change the check to mirror the error message, though.

[dcavins]

@ekadevau- The updated file is here: ​https://github.com/dcavins/bp-svn-bporg/blob/64b1c0716264fb2207f85de9a98df54cf6199bf4/src/bp-templates/bp-nouveau/includes/groups/ajax.php

[imath]

Hi @dcavins

Thanks a lot for your work on this. I've tested without and with your patch and I confirm 8167.2.diff is fixing the issue.

Let's fix this ;)

[dcavins]

In 12501:

Nouveau: Fix group uninivite AJAX handler logic.

Ensure that site admins, group admins and the original inviter can all delete a pending invitation.

Fixes #8167.

[dcavins]

In 12502:

Nouveau: Fix group uninivite AJAX handler logic.

Ensure that site admins, group admins and the original inviter can all delete a pending invitation.

Fixes #8167.