[buddypress-trac] [BuddyPress Trac] #8167: Nouveau: Inviter cannot remove group invitation

[buddypress-trac]

#8167: Nouveau: Inviter cannot remove group invitation
--------------------------+------------------------------
 Reporter:  dcavins       |       Owner:  dcavins
     Type:  defect (bug)  |      Status:  assigned
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  Groups        |     Version:  5.0.0
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |
--------------------------+------------------------------
Changes (by dcavins):

 * owner:  (none) => dcavins
 * status:  new => assigned


Comment:

 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?

-- 
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/8167#comment:3>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac