[buddypress-trac] [BuddyPress Trac] #5374: Administation screen to manage signups

[buddypress-trac]

#5374: Administation screen to manage signups
-------------------------------------------------+------------------
 Reporter:  imath                                |       Owner:
     Type:  enhancement                          |      Status:  new
 Priority:  high                                 |   Milestone:  2.0
Component:  Members                              |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  2nd-opinion has-patch needs-testing  |
-------------------------------------------------+------------------

Comment (by boonebgorges):

 imath - I've had a chance to review this patch in some detail. Very nice
 work - I think it's going to be a huge improvement.

 5374.05.patch is a refresh, with the following improvements:

 - Change the name of the new class to BP_Signup and moved to a new file
 bp-members-classes.php. This seems more consistent to me.
 - Eliminated some overly-specific methods in BP_Signup, and added some
 parameters to the main `get()` method so that it'd do all the necessary
 work.
 - Added unit tests for BP_Signup
 - Improved inline docs, codestyling, escaping
 - Changed name of "Mail Count" column to "# Times Resent". I don't love
 either of these phrases, but I think that mine is a little clearer.
 - Added an Activate link to the row-actions. I know that clicking the
 username would lead to the Activate screen, but IMO it was not clear
 enough. Now you have both options :)
 - Cleaned up the text on the Help tab
 - Changed BP_Signup so that it handles serializing/unserializing meta
 values. Arrays in and arrays out - let the database method do the dirty
 work.
 - BP_Signup::add() returns the ID of the new item on success, rather than
 true. Mainly makes unit testing easier.
 - Changed the name of the backward compatibility constant to
 BP_SIGNUPS_SKIP_USER_CREATION, and added some additional inline
 explanatory text. The constant is a very good idea, but the idea is sorta
 confusing, and it's very important that plugin devs etc understand it, so
 I wanted to make it as clear as possible.

 Thoughts for improvements:
 - It's confusing to have the Mail link simply disappear when within 24
 hours of the last resend. Maybe we could change the color or have some
 hover-text or disable it *or something*.

 Aside from that, everything seemed to work well. I tested MS vs non-MS, as
 well as upgrades from 1.9, and I couldn't find any glaring bugs :) See
 what you think of my changes. Once you and I have agreed on a patch that's
 technically viable, let's try to get a couple people to test the patch
 before committing - there are a lot of possible configs to test against,
 and I'd like a rough sense that it's working before putting it into trunk.

 Thanks for your great work on this!

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