WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 11 months ago

Last modified 8 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 16 months ago.
wp_includes_post.diff (410 bytes) - added by sphoid 13 months ago.
Improved Patch that checks for existence of $rfields?
22208.2.diff (507 bytes) - added by wonderboymusic 11 months ago.
22208.3.diff (1.3 KB) - added by wonderboymusic 11 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 Viper007Bond21 months ago

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

Jesper80016 months ago

comment:2 Jesper80016 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 16 months ago by Jesper800 (previous) (diff)

comment:3 sphoid13 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.

sphoid13 months ago

Improved Patch that checks for existence of $rfields?

wonderboymusic11 months ago

comment:4 wonderboymusic11 months ago

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

Makes sense. R-r-re-freshed.

comment:5 DrewAPicture11 months ago

  • Cc xoodrew@… added

wonderboymusic11 months ago

comment:6 wonderboymusic11 months ago

  • Keywords commit added

.3.diff has a unit test

comment:7 nacin11 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 11 months ago by nacin (previous) (diff)

comment:8 wonderboymusic11 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 jorgeorpinel8 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 jorgeorpinel8 months ago

  • Keywords needs-refresh added

comment:11 markoheijnen8 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.