WordPress.org

Make WordPress Core

Opened 18 months ago

Closed 8 months ago

Last modified 5 months ago

#22208 closed enhancement (fixed)

get_children() doesn't support 'fields' => 'ids' as an argument

Reported by: danielbachhuber Owned by: wonderboymusic
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.4.2
Component: Query Keywords: has-patch
Focuses: Cc:

Description

Not sure whether it should, but it would be nice.

When you pass 'fields' => 'ids' as an argument, the data returned by get_posts() is nooped with this block:

foreach ( $children as $key => $child )
    $kids[$child->ID] = $children[$key];

Attachments (4)

22208.diff (369 bytes) - added by Jesper800 13 months ago.
wp_includes_post.diff (410 bytes) - added by sphoid 10 months ago.
Improved Patch that checks for existence of $rfields?
22208.2.diff (507 bytes) - added by wonderboymusic 9 months ago.
22208.3.diff (1.3 KB) - added by wonderboymusic 8 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 Viper007Bond18 months ago

  • Keywords needs-patch added
  • Type changed from defect (bug) to enhancement
  • Version set to 3.4.2

Jesper80013 months ago

comment:2 Jesper80013 months ago

  • Keywords has-patch added; needs-patch removed

I've added a patch that checks whether the fields parameter is set to either ids or id=>parent, in which case it returns the result from get_posts. Please note that if this is the case, the $output parameter will have no effect. This is something to consider, as it does make the functionality a bit confusing.

Another option would be to ignore the fields parameter for this function and add documentation for that, but it does break functionality currently.

Last edited 13 months ago by Jesper800 (previous) (diff)

comment:3 sphoid10 months ago

I ran into this error "Notice: Trying to get property of non-object in /wp-includes/post.php on line 4617". It was triggered by the plugin WordPress SEO 1.4.10 which uses the 'fields' => 'ids' argument to get_children() in class-metabox.php line 1514. I applied the attached patch and it resolved the issue for me.

sphoid10 months ago

Improved Patch that checks for existence of $rfields?

wonderboymusic9 months ago

comment:4 wonderboymusic9 months ago

  • Keywords dev-feedback removed
  • Milestone changed from Awaiting Review to 3.7

Makes sense. R-r-re-freshed.

comment:5 DrewAPicture9 months ago

  • Cc xoodrew@… added

wonderboymusic8 months ago

comment:6 wonderboymusic8 months ago

  • Keywords commit added

.3.diff has a unit test

comment:7 nacin8 months ago

I imagine more fields might come later on — rather than a blacklist, let's make this a whitelist.

if ( ! empty( $r['fields'] ) )
    return $children;

Thoughts?

Last edited 8 months ago by nacin (previous) (diff)

comment:8 wonderboymusic8 months ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 25160:

Respect the fields arg when passed to get_children().

Fixes #22208.

comment:9 jorgeorpinel5 months ago

I believe I found a small bug with the workaround currently in use for this enhancement. get_children "works similar to get_posts" (quoted from that codex page), but NOT EXACTLY the same.

It seems that certain parameters of the WP_Query are different by default in get_posts, such as 'nopaging' => 'false' (in get_posts) and 'post_status' => 'any' in get_children. Additionally, the format in which each function returns the values is different, where get_post's array is indexed by 0..n and get_children's by the post IDs.

I think It'd be nice to make the fields parameter work in get_children without overriding its calls with get_posts. Otherwise at least, the default WP_Query parameters in the overriding get_posts could be made to match the get_children defaults.

Thanks!

comment:10 jorgeorpinel5 months ago

  • Keywords needs-refresh added

comment:11 markoheijnen5 months ago

  • Keywords commit needs-refresh removed

If you think there is a bug/feature request then you should create a new ticket for it.

Note: See TracTickets for help on using tickets.