Opened 9 years ago
Closed 8 years ago
#34450 closed defect (bug) (fixed)
get_id_from_blogname returns false if the main domain starts with www.
Reported by: | igmoweb | Owned by: | flixos90 |
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | 4.3.1 |
Component: | Networks and Sites | Keywords: | has-patch has-unit-tests |
Focuses: | multisite | Cc: |
Description
I have a local Multisite installation with the following domain:
www.wordpress.dev
I create a new site called: demo1.wordpress.dev, which is fine and works as expected (the www. is trimmed as expected too).
When I try the following code:
$result = get_id_from_blogname( 'demo1' );
$result is false and not the Blog ID.
I think that the issue is located inside the get_id_from_blogname function:
$domain = $slug . '.' . $current_site->domain;
should be
$domain = $slug . '.' . preg_replace( '|^www\.|', '', $current_site->domain );
But I'm not sure if this could affect other areas.
Attachments (3)
Change History (14)
#2
@
9 years ago
I've added the change in 34450.patch though I'm not able to make unit tests under the following conditions:
I've grabbed the regexp from wp-admin/site-new.php where www. is stripped out.
- Subdomains install
- Prefix www. to the test domain without changing WP_TESTS_DOMAIN const in wp-tests-config.php
It would be awesome if someone is capable of that or jsut give me some directions to set the environment with PHPUnit tests bootstrap and I'll make it.
#3
@
9 years ago
Thanks for your work on this, @igmoweb. I think there's a good chance that WP_Site_Query
via #35791 can be used in get_id_from_blogname()
. Testing/feedback is welcome!
This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.
8 years ago
#5
@
8 years ago
- Keywords has-patch needs-refresh added; needs-patch removed
- Milestone changed from Awaiting Review to 4.7
- Owner set to flixos90
- Status changed from new to assigned
#6
@
8 years ago
- Keywords has-unit-tests 2nd-opinion added; needs-unit-tests needs-refresh removed
34450.diff is an updated patch that makes use of WP_Site_Query
(via get_sites()
) in the function. This also allows us to get rid of the get_id_from_blogname_*
cache key, since site query results are cached anyway. An additional benefit we get is that the function now actually returns an integer, as stated in the docs (before the return value would actually be a string, due to wpdb::get_var()
).
The patch also adds unit tests, one for a www-prefixed network, another one without www.
One thing that we should probably improve (but I didn't yet because I wasn't sure): What do we do if the function fails to find an ID? Previously it would have returned null (see wpdb::get_var()
), although this is undocumented. My proposal would be to check if the array returned by get_sites()
is empty, and if so return null. And of course we would then document it as well.
#7
@
8 years ago
@flixos90
I would keep the same return types than before. So yes, I would check if empty( get_sites() ) return null;
to keep backward compatibility and improve documentation for the function.
#8
@
8 years ago
- Keywords 2nd-opinion removed
34450.2.diff is an updated patch that adjusts the return type of the function for backward compatibility and improves the documentation accordingly.
Even if that would be expected behaviour (which I don't think it is), unit tests for
get_id_from_blogname()
would be great.