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 8 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.

Version 1, edited 13 months ago by Jesper800 (previous) (next) (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?

wonderboymusic8 months ago

comment:4 wonderboymusic8 months ago

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

Makes sense. R-r-re-freshed.

comment:5 DrewAPicture8 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.