Opened 6 weeks ago
Last modified 3 days ago
#8582 new enhancement
Add site membership requests.
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 10.0.0 | Priority: | normal |
Severity: | normal | Version: | 9.1.1 |
Component: | Members | Keywords: | has-patch needs-testing |
Cc: | dcavins |
Description
It would be very useful to be able to screen site members on the way in, rather than dealing with spam users who have registered and gained access to the site immediately.
I've built a first working patch that interrupts the registration flow, so that a site admin must manually intervene to send the activation email to the user. Here's a comparison of the normal registration flow with the proposed request-based flow:
BuddyPress normal sign-up process:
User fills out registration form.
User is created in signups table with active = 0.
User is created in users table with status = 2
BuddyPress sends user an activation email.
User follows link in email to activate account.
User receives welcome email from BP site.
Table updates: signups entry now is active, user status is now 0.
Membership request sign-up process:
Admin disables "Anyone can register" and enables membership requests.
User visits registration form which has been recast as a "Request Membership" form.
Site admins receive email and notifications about the pending request.
Site admin can manage via the "manage signups" interface.
Admin can check the user's registration information via the extended profile view.
Admin approves the user and the activation email is sent.
User follows link in email to activate account.
User receives welcome email from BP site.
Notes:
If an email has been sent, the request is considered to be "approved."
Front-end profiles can also be viewed before the user is activated/validated. Should we change that behavior?
I have added a filter so that when requests are enabled, it is possible to bypass the manual approval process for some users, like if users are using a certain email domain that you know is trustworthy (like your school or organization).
Attachments (23)
Change History (37)
#1
@
6 weeks ago
I would appreciate help testing the first patch. There are some things I know still need to be handled, like marking notifications as read when signups are approved or deleted. If you notice anything (in either Legacy or Nouveau) that seems off, please let me know. It is probably not necessary to do a line-by-line review of the patch yet. :) Thanks for your feedback!
I am attaching a series of images showing the basic steps in the process, for easy review.
#2
@
6 weeks ago
Waou !! This looks really promising !! Thanks a lot for your work on this ticket, the patch and the screenshots. I’ll test the patch asap and will provide more feedbacks soon 👍
#3
follow-up:
↓ 4
@
6 weeks ago
Here are my first feedbacks @dcavins. For now I haven't reviewed the patch in detail to focus on the feature's (needed imho) improvements.
Thanks a lot for this first pass about the feature. It's already very promising and it should work great when the registration process creates a User, but...
1) About the BuddyPress/WordPress signup process
There is only 1 case where a user is created once they submit the registration form: when WordPress is not multisite and when the BP_SIGNUPS_SKIP_USER_CREATION
is not defined or false
.
As a result, I strongly discourage you to use data from the wp_users table for this feature.
2) About the setting:
Would it be better to name the feature "site membership requests" instead of "network membership requests" if WordPress is not multisite or if BuddyPress is not network activated?
- The checkbox should be readonly until the required signup option matches the need for disabling public registration.
- We should probably include a link to where to turn public registration off.
- This setting can't be
1
or0
imho because of Multisite registration possibilities (account, account + blog).
3) What's the strategy to add the link to request a membership?
Should we include a link to the wp-login.php
form at the minimum?
4) Testing the patch:
PS: On purpose (I knew doing so I'll get some errors 😆) I've set the BP_SIGNUPS_SKIP_USER_CREATION
constant to true.
4.1) Once I submit the form I got a success message but also a notice error:
Undefined variable $item_id in /path-to/buddypress/src/bp-members/bp-members-membership-requests.php on line 119
You should probably use $signup->id
instead.
4.2) If i go to the Manage signups screen, the request is listed but:
Warning: Attempt to read property "ID" on bool in /path-to/buddypress/src/bp-members/bp-members-notifications.php on line 85
That's because as I've explained above: the user wasn't created, because I've set the BP_SIGNUPS_SKIP_USER_CREATION
constant to true
.
I don't get the Extended
link for the user requesting a site membership. See screenshot below:
If I click on the notification link, nothing happens unlike what the 6-manage-single-view.png shows. I stay on the Signup list. That's because of the BP_SIGNUPS_SKIP_USER_CREATION
constant, but it would also be the case for Multisite configs. Using the Extended Profile is an interesting idea, but I believe we need more work to use a UI that is available for any WordPress/BuddyPress configs.
4.3) Possible improvements/questions :
- Could we have like it's the case for pending comments a specific yellow background applied to the row?
- What about a row action to reject with an email message the request?
- When clicking on the Activate row action in this case, how is the requesting user informed their account has been activated?
- The approve request row Action should lead to an adapted "Resend Activation Emails" screen:
- page title = "Approve request(s)"
- description = "You are about to approve the following request(s)"
5) About the admin email notice content
To learn more about the site, visit:
site.url
What are we supposed to learn more, as an Admin I know my site, don't I? I'd remove this part from the email. This may only be interesting for multisite configs.
To view {{requesting-user.name}}'s profile, visit:
profile.url
As on multisite or if BP_SIGNUPS_SKIP_USER_CREATION
is true : the User is not created: this should fail (I wasn't able to test emails on my config). It should be the link to open the Admin UI made available for any WordPress/BuddyPress configs.
Conclusion:
As I've said it's very promising and you already did some great work about it. But I believe we need some more work to take in account the 2 other signup processes you haven't covered so far 😁
#4
in reply to:
↑ 3
@
6 weeks ago
Replying to imath:
Thanks for your feedback, imath!
I have a few questions and/or responses:
- This setting can't be
1
or0
imho because of Multisite registration possibilities (account, account + blog).
I'm not sure what you mean. Can you explain why enabling request or not would need more than on/off?
3) What's the strategy to add the link to request a membership?
Should we include a link to the
wp-login.php
form at the minimum?
I've added a link to the BP login form, but sure, it seems like it should be added to the wp-login form as well.
Using the Extended Profile is an interesting idea, but I believe we need more work to use a UI that is available for any WordPress/BuddyPress configs.
Sure. There may be a way to hotwire the extended view for signup IDs. I'll look into it.
- What about a row action to reject with an email message the request?
I agree that we should probably send a "rejected" email. Poor requester. :)
- When clicking on the Activate row action in this case, how is the requesting user informed their account has been activated?
They get the welcome email.
Thanks again for taking a look at this!
#5
@
6 weeks ago
You're welcome 😉.
About:
I'm not sure what you mean. Can you explain why enabling request or not would need more than on/off?
Correct me if I misunderstood: to turn on site membership requests. You need to disallow public registration. On a single site config, the WP option 'users_can_register'
can be 1 or 0 so I agree with you.
But on multisite, the corresponding WP site meta 'registration'
can be 'all'
, 'blog'
, 'user'
or 'none'
, so what the admin needs to select to enjoy the site membership requests feature ? 'blog'
or 'none'
?
How would we handle the case where only user account can be requested?
How would we handle the case where both user account and site can be requested?
(there's a chance I'm overthinking it! 😅)
This ticket was mentioned in Slack in #buddypress by imath. View the logs.
4 weeks ago
#7
@
3 weeks ago
Here's an updated version of the patch that makes a few changes. The biggest difference is that instead of trying to use the user pages (in WP-Admin), any submitted extended profile data is available in the "Manage Signups" screen, which relies on the signup data only, and I've removed any code that refers to users. Other changes:
- Add links to the request membership form to the widget login form, wp-login, and the WP toolbar.
- Send a rejection email to rejected users. :,(
- Change the language on the manage signups > resend email screen (rebranded to "send approval emails").
Images and a fresh patch to follow.
#8
@
2 weeks ago
Really nice! Thanks a lot for your work on these improvements! I'll test the patch asap.
This ticket was mentioned in Slack in #buddypress by imath. View the logs.
2 weeks ago
#10
follow-up:
↓ 11
@
7 days ago
Hi @dcavins
I've tested the patch, and was a bit surprised at the beginning as submitting a membership request was sending an activation email. Then I understood checking your previous patch that one file was missing. So I've used 8582.1.diff to include the missing bp-members-membership-requests.php
file into 8582.4.patch to have it work the way it should. You should probably check again my edits about this file 😆. 8582.4.patch also includes some recommandation (eg: escaping some outputs, or displaying a message when there are no profile field to display into the modal).
I have some remarks/questions 😁.
- I guess the purpose of having a separate
bp-members-membership-requests.php
is to only include it if Membership requests are allowed, so I've edited this part in the 8582.4 patch. But I see a lot of functions that should be moved into this file... (eg: all the ones that are insidesrc/bp-members/bp-members-filters.php
). - Why do we need
bp_is_active( 'members', 'invitations' )
for site membership requests? It seems weird to me. - Is the Approved column necessary into the WP List Table? I don't see the benefit of it.
- The "Manage signups" WP Admin Menu item should probably be renamed "Manage Pending members".
Otherwise, thanks a lot for your great progress about the ticket, we're getting closer to the finish line 🏁.
On a side note and to follow up with our latest dev-chat: I've checked how WordPress uses the inline-edit feature and I don't like it. I believe we can do things better now we have the BP REST API and modern JavaScript. I'll explore this in another ticket.
#11
in reply to:
↑ 10
@
5 days ago
Replying to imath:
That's strange about the patch. It sure looks like new file src/bp-members/bp-members-membership-requests.php
is included in the attached https://buddypress.trac.wordpress.org/attachment/ticket/8582/8582.3.diff. Maybe when you applied the patch, the new file wasn't created because of permissions or similar?
Regarding file organization, I am in favor or moving filters
and actions
to the main file bp-members-membership-requests.php
. Putting filters in a filters file is how BP is often organized, so I respected that, but personally prefer having files organized by content, not part of speech.
I think the Approved column is useful (it's a renaming of an existing signup column, date activation email sent) because some pending memberships will have been "approved" but not activated by the end user yet. The column tells you at a glance when the approval occurred or if it hasn't yet. You could also use it to sort the "not yet approved" requests up to the top (I will enable sorting on it).
The $64,000 question: What should we do with meta for the 10.0 release? Leave it on the "activate" and "approve" screens? Use a modal, but maybe move the access link to the "actions" cell? I think it would be great to add an inline viewer/editor, but let's not let it hold up anything--it can be a nice improvement later.
Thanks!
#12
@
5 days ago
I'm about to upload a new version of the patch with the following changes:
- Incorporate @imath's changes (though I had to manually merge changes in
bp-members-membership-requests.php
so may have missed a new escape addition there). - Support
bp_is_active( 'members', 'membership_requests' )
so that the functionality can be disabled/hidden via code if desired. - Move filters from
members-filters
intobp-members-membership-requests.php
and remove severalbp_get_membership_requests_required()
checks from those functions since that file is only included when membership requests are enabled. - Filter the "Manage Signups" menu item label in WP Admin > Users when requests are enabled.
Thanks!
#13
@
5 days ago
Another small update to delete associated notifications when a signup is approved, activated or deleted.
Add request membership capability. First draft.