[wp-trac] [WordPress Trac] #39879: wp_get_sites() with a false limit no longer returns all sites

[WordPress Trac]

#39879: wp_get_sites() with a false limit no longer returns all sites
--------------------------------------------------+------------------------
 Reporter:  iandunn                               |       Owner:
     Type:  defect (bug)                          |      Status:  new
 Priority:  normal                                |   Milestone:  4.8
Component:  Networks and Sites                    |     Version:  4.6
 Severity:  normal                                |  Resolution:
 Keywords:  has-patch has-unit-tests 2nd-opinion  |     Focuses:  multisite
--------------------------------------------------+------------------------
Changes (by flixos90):

 * keywords:  has-patch has-unit-tests => has-patch has-unit-tests 2nd-
               opinion


Comment:

 Thanks for the report and patch @iandunn. I think it is easier to not use
 `PHP_INT_MAX`, but rather an empty value so that the `LIMIT` clause is
 ignored. [attachment:39879.diff] uses that approach.

 Regarding the tests, I'm not sure whether we should thorougly test this.
 While I generally would be a 100% for testing, as you already mentioned
 there are performance issues when creating 100 sites. We could maybe write
 a test for `WP_Site_Query` though to check whether passing an empty value
 to `number` also sets the query argument to an empty value, although this
 would be a "very nitpicking" test. It would be better if we could check
 the query vars from the `WP_Site_Query` instance that `wp_get_sites()`
 instantiates through `get_sites()`, but since that one is incapsulated, it
 wouldn't be possible.

 Ping @jeremyfelt, do you think tests make sense here regarding the
 circumstances, or can we move forward without?

--
Ticket URL: <https://core.trac.wordpress.org/ticket/39879#comment:5>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform