[buddypress-trac] [BuddyPress Trac] #6209: Add tests for group invitations and membership request functions

buddypress-trac noreply at wordpress.org
Tue Mar 10 17:45:40 UTC 2015


#6209: Add tests for group invitations and membership request functions
--------------------------------+-----------------------
 Reporter:  dcavins             |       Owner:  dcavins
     Type:  task                |      Status:  assigned
 Priority:  normal              |   Milestone:  2.3
Component:  Component - Groups  |     Version:
 Severity:  normal              |  Resolution:
 Keywords:  has-patch           |
--------------------------------+-----------------------
Changes (by boonebgorges):

 * keywords:  needs-unit-tests has-patch => has-patch


Comment:

 6209-working-tests.02.patch looks good. I'd go ahead and commit it.

 Comments on the second patch:
 - Instead of doing the `$u1_has_request_before` and `after` dance - which
 could mistakenly pass if both were accidentally false instead of both true
 - you should verify truth values directly. `$this->assertTrue(
 groups_check_for_membership_request() )` etc.
 - It seems like a bug that `groups_join_group()` returns true when a user
 is already a member, but I guess this can't be changed for backward
 compatibility reasons.
 - For clarity's sake, when committed, it would be nice if standalone fixes
 (and the tests that demonstrate them) could go in as parts of separate
 changesets. Eg, it looks like a subset of the patch is devoted to
 correcting a bug in `groups_remove_member()` having to do with calling
 from outside a group context. That fix should be separate from the
 `delete_invite` switches.
 - Do you have thoughts about backward compatibility with existing plugins
 that are expecting a certain behavior from functions like
 `groups_delete_membership_request()`? It looks like currently this family
 of functions all uses `groups_uninvite_user()` internally - you call it a
 "loose cannon", I'd call it a "sledge hammer". Making these functions more
 targeted may be more semantic, but it's also going to change existing
 behavior. Are there plugins expecting the current behavior? How badly will
 they break? It may be a good idea to look through the WP plugin repo to
 see if you can find any examples.
 - On a related note: even if we can't find any specific examples of
 plugins breaking, we still ought to put something on bpdevel or elsewhere
 that describes the changes, and warns plugin authors about potential
 breakage.

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


More information about the buddypress-trac mailing list