[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