Add caching and other enhancements to BP_Signup class.

[dcavins]

While working with the BP_Signup class for network membership requests, I've found a few ways it can be improved to behave more consistently.

• Add caching for Signup SQL queries and BP_Signup objects.
• Synchronize signup result format provided by get() and construct() methods.
• Add action points so that the cache items can be cleaned up.
• Use cached functions rather than making direct database calls.
• Improve behavior of meta-updating method.

Also, I believe that I can resolve #7938 while I'm familiar with the mechanics of this class.

[dcavins]

Improve behavior of BP_Signup class.

[dcavins]

Improve handling of active status in signups class.

[dcavins]

The second patch changes the query behavior so that already activated signups are recognized on the activation screen.

[imath]

Really nice! This looks very promising. Thanks a lot for your work on it. I'll test the patch & give it a deeper look asap.

[imath]

Hi @dcavins

Thanks again for your work on these improvements and sorry about my late review about them.

I've tested 8540.1.patch. I believe there's an issue with the way the sent_date is set.
See the screenshot below:

I believe when the sent_date is not set or equals to '0000-00-00 00:00:00', it should be set with the registered date of the signup. I've included this suggestion and some WP PHP Code standards ones into 8540.1.suggestions.patch.

After applying 8540-change-active-handling.patch, I get a failing test:

1) BP_Tests_BP_Signup::test_objects_should_be_cached
Failed asserting that 48 matches expected 0.

/buddypress/tests/phpunit/testcases/members/class-bp-signup.php:550

In 8540-change-active-handling.suggestions.patch you'll find some other formatting suggestions 😉.

Finally I confirm I can register, activate, and that the admin can see, resend, activate and delete the new signup into the Manage Signups screen. I also confirm an already activated account trying to use the activation key again into the activation screen get the error message you shared into your screenshot 👍

[dcavins]

Thanks for the review @imath!

The reason for my change to the calculation of the sent_date when it is not set is that in the current situation it makes sense to assume the activation was sent at registration time (though we don't record it). However, in the case of the new signup being a *site membership request*, it's important to know that the activation mail was not yet sent. I'm thinking that a data update task when BP10 is installed will be to explicitly set the sent_date and count_sent for existing signups so that we can discern between old signups where we assume the date and number of times sent, and new requests that have never been sent. (A problem with that will be that that data is stored in a serialized array, so I believe we'll have to loop over each BP_Signup in php, ugh.)

In any case, we can apply your logic now, and I'll change it as needed for requests to work. Thanks!

[dcavins]

Improvements on original patch (including imath's suggestions, corrections to tests, and improved documentation in the BP_Signup class)

[imath]

Thanks for updating the patch @dcavins it looks good!

[dcavins]

In 13098:

Improve BP_Signup class.

• Add caching for BP_Signup SQL queries and BP_Signup objects.
• Synchronize signup result format provided by get() and construct() methods.
• Add action points so that the cache items can be cleaned up.
• Use cached functions rather than making direct database calls.
• Improve behavior of meta updating method.

Fixes #8540.

[dcavins]

In 13165:

BP_Signup improvements.

On multisite, when resending actvation emails, send the
right kind (user or user + site). Also ensure that default
meta is set via wpmu meta filters.

introduce bp_members_get_signup_by() and
bp_members_site_requests_enabled().

Correct default value calculation in BP_Signup
to allow backcompat.

See #8540.