[wp-trac] [WordPress Trac] #40180: Introduce `get_site_by()` function for multisite
WordPress Trac
noreply at wordpress.org
Sun Oct 1 21:37:16 UTC 2017
#40180: Introduce `get_site_by()` function for multisite
-------------------------------------------------+-------------------------
Reporter: flixos90 | Owner: flixos90
Type: enhancement | Status: assigned
Priority: normal | Milestone: 4.9
Component: Networks and Sites | Version:
Severity: normal | Resolution:
Keywords: has-patch dev-feedback has-unit- | Focuses: multisite
tests ms-roadmap needs-dev-note |
-------------------------------------------------+-------------------------
Comment (by flixos90):
Replying to [comment:27 jeremyfelt]:
> Removed the assertions that the returns were `WP_Site` objects. We
should be asserting one thing per test, and if it's not returning the
object, then other things are going to fail.
The reason I put these extra assertions in was that I wanted to have
failures rather than errors (if `get_site_by()` returns null which I'd
consider a test failure, it throws an error without the `WP_Site`
assertion). But I see the one assertion per test argument, so I agree with
that as well.
> * Added 2 explicit tests for retrieving a site by slug on specific
networks. I see that the other tests use different networks, but I think
having the explicit behavior tested is good.
Good catch, that makes sense.
> The tests for `get_site_by( 'id' ...)` gave me pause. I definitely don't
think we need to account for somebody passing `null` as the ID value when
they can just use `get_site()`. I think that test can be removed.
>
> As for `id` itself, are we doing this just for parity with other
functions? I can't imagine this being more useful than `get_site( $id )`.
Right, I agree there's probably nobody who will use `get_site_by( 'id',
... )` over `get_site( ... )`, but I did it for parity which I still think
makes sense.
As far as putting `null` goes, I agree that this test can be removed.
While it would still work because it just calls `get_site()`, trying to
get a site by a specific ID, which the function name implies, and then
passing `null` doesn't make sense. Should we actually catch that behavior
and return `null` in such a case?
--
Ticket URL: <https://core.trac.wordpress.org/ticket/40180#comment:28>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list