#6197 closed defect (bug) (fixed)
Register/Activate pages and bp_core_get_directory_page_ids() since 2.2
Reported by: | imath | Owned by: | imath |
---|---|---|---|
Milestone: | 2.2.1 | Priority: | normal |
Severity: | normal | Version: | 2.2 |
Component: | Registration | Keywords: | has-patch commit |
Cc: |
Description (last modified by )
#6176 is a relative ticket.
Context :
- Multisite config
- User registration is set to 'blog' in other words 'Logged in users may register new sites'
If we go to site.url/wp-signup.php we get :
WordPress displays a message to inform registration are closed.
BuddyPress 2.1.1
Without creating the pages 'register' and 'activate' nor touching the page setting association settings, if we go to site.url/wp-signup.php, we are redirected to site.url/register and we get a 404.
If we create the 'register' and 'activate' pages and set the page association settings, we get :
After Upgrading to BuddyPress 2.2.0
if we go to site.url/wp-signup.php, we are redirected to site.url/register and we get a blank page if the slug of the 'register' page is 'register' if it's something else we get a 404.
Applying the attached patch bring us back to 2.1.0 situation for the activate and register page. I still need to work on unit tests to cover the different possible situations.
Attachments (3)
Change History (21)
#3
@
10 years ago
Awesome - thanks, imath. Regarding the changed test (test_bp_core_get_directory_page_ids_should_not_contain_register_and_activet_pages_when_registration_is_closed()
). You've modified it to show what happens when registration is closed but the pages do not exist. Presumably, part of the problem described in this ticket is that sometimes registration is closed but the pages *do* exist. What should be the behavior in these cases? We should probably have a separate test for it.
#5
@
10 years ago
and bp_core_add_page_mappings() will not create the activate/register page if signups are not allowed. So it must be the admin to set it from the Administration and then we are using bp_core_admin_slugs_setup_handler()
so i think that's what i'm doing in test_bp_core_get_directory_page_ids_register_activate_pages_created_signup_not_allowed()
#6
follow-up:
↓ 9
@
10 years ago
I guess I trust you here, but I'm still confused about what's happening and what you intend to be testing. At a glance, it seems like we need tests for the following:
- Ensure that
bp_core_get_directory_pages()
returns 'register' and 'activate' when registration is active and the pages exist - Ensure that
bp_core_get_directory_pages()
returns 'register' and 'activate' when registration is *inactive* and the pages exist (right?) - Ensure that
bp_core_get_directory_pages()
does not 'register' and 'activate' when registration is active and the pages don't exist - Ensure that a notice is thrown when registration is active and the pages don't exist
- Ensure that the notice is not thrown when registration is active and the pages do exist
Exactly *how* the pages are created during the tests is not terribly important.
Again, though, I don't 100% understand the issue here. For me, the systematic tests are a way to make the problem easier to understand, and in this case I feel like the tests don't cover all the cases. That being said, if you do understand it and are confident with your solution, then go with it :)
#7
@
10 years ago
I think we need to take a bit of time to make sure we're all clear on the issue, and what the unit tests to be covered, especially if we want to ship this in a minor release.
#8
@
10 years ago
Thanks boonebgorges & DJPaul for your feedbacks. I'll review the cases boonebgorges listed and i agree with taking the time on this one. Also, i think it could be interested to ask people who reported problems on the forum if they could test the patch to see if it's solving their problem (as it's just removing a part of an if statement).
#9
in reply to:
↑ 6
@
10 years ago
Replying to boonebgorges:
In 6197.unittests.02.patch, i'm easing the unit tests the way you suggested
- Ensure that
bp_core_get_directory_pages()
returns 'register' and 'activate' when registration is active and the pages exist
This test should cover it : test_bp_core_get_directory_pages_register_activate_page_created_signups_allowed()
- Ensure that
bp_core_get_directory_pages()
returns 'register' and 'activate' when registration is *inactive* and the pages exist (right?)
Yes it's right and it should fix some issues posted in the support forum.
This test should cover it : test_bp_core_get_directory_pages_register_activate_page_created_signups_notallowed()
- Ensure that
bp_core_get_directory_pages()
does not 'register' and 'activate' when registration is active and the pages don't exist
This test should cover it : test_bp_core_get_directory_pages_register_activate_page_notcreated_signups_allowed()
- Ensure that a notice is thrown when registration is active and the pages don't exist
This test should cover it : test_bp_core_activation_notice_register_activate_pages_notcreated_signup_allowed()
- Ensure that the notice is not thrown when registration is active and the pages do exist
This test should cover it : test_bp_core_activation_notice_register_activate_pages_created_signup_allowed()
#10
@
10 years ago
Thanks for bearing with me, imath - it's starting to make more sense to me.
If I understand correctly, test_bp_core_get_directory_page_ids_should_not_contain_register_and_activet_pages_when_registration_is_closed()
- which you have modified - is covering the case where signups are *closed* and where 'register' and 'activate' *do not exist*, right? So it completes the matrix, so to speak?
It's looking to me like https://buddypress.trac.wordpress.org/attachment/ticket/6197/6197.patch is probably the right fix, but it would be great (as imath noted) to get some feedback from people who are experiencing the problem on the forums.
#11
@
10 years ago
I must admit my first tests was over complicated! Sorry for that boonebgorges
I actually added another test in 6197.unittests.02.patch :
test_bp_core_get_directory_pages_register_activate_page_notcreated_signups_notallowed()
it directly covers the case
where signups are *closed* and where 'register' and 'activate' *do not exist*
test_bp_core_get_directory_page_ids_should_not_contain_register_and_activet_pages_when_registration_is_closed()
is creating pages then deleting them then checking register and activate are not in page_ids. So it might be a good idea to remove it. I actually adapted it because he was here before.
I've created a small script to add in bp-custom.php file to retain the activate/register page ids, i think it's safer to ask people testing it instead of asking them to edit a core file.
This ticket was mentioned in Slack in #buddypress by jjj. View the logs.
10 years ago
#13
@
10 years ago
:( the user in support forum cannot (or is afraid to) help. So i'll run some tests with invite-anyone today.
#14
@
10 years ago
Just tested the patch with the invite anyone plugin activated. I confirm it's fixing the issue.
#15
@
10 years ago
- Keywords commit added
Thanks for looking into it more, imath. I think your tests and the fix look fine. Let's go with them for 2.2.1.
Only unit tests