WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#16545 closed defect (bug) (fixed)

Direct WP_Query::get_posts() calls can break

Reported by: nacin Owned by:
Milestone: 3.1 Priority: highest omg bbq
Severity: blocker Version: 3.1
Component: Query Keywords: has-patch needs-testing
Focuses: Cc:

Description

global $wp_query;
$wp_query->set('cat', '-13');
$posts = $wp_query->get_posts();

That worked in 3.0, and fails in 3.1. With parse_query() never getting called, now parsed_tax_query() never gets called. Previously enough logic was in get_posts() for this to work. It was inconsistent because parse_query() was circumvented, but it worked.

In IRC he planned out a strategy of sorts and Ryan is going to work up a patch later.

Attachments (4)

16545-parsed_query-flag.diff (1.1 KB) - added by markjaquith 4 years ago.
16545.diff (2.1 KB) - added by ryan 4 years ago.
16545.2.diff (1.4 KB) - added by ryan 4 years ago.
Without parsed_query flag
16545.3.diff (862 bytes) - added by ryan 4 years ago.

Download all attachments as: .zip

Change History (30)

comment:2 @SergeyBiryukov4 years ago

  • Summary changed from Direct WP_Quey::get_posts() calls can break to Direct WP_Query::get_posts() calls can break

comment:3 @scribu4 years ago

  • Cc scribu added

comment:4 @scribu4 years ago

I don't think this warrants 'blocker' status, but I guess that doesn't make much of a difference.

comment:5 @markjaquith4 years ago

There's my suggestion.

comment:6 @jane4 years ago

"That worked in 3.0, and fails in 3.1."

@scribu: an unintentional regression is generally a blocker.

comment:7 @scribu4 years ago

My point was that, like ryan said, it shouldn't have worked ever. parse_query() and get_posts() should have been private from the beginning. (Captain Hindsight to the resque, I know.)

@ryan4 years ago

comment:8 @ryan4 years ago

Avoids calling init() from parse_query() and stomping query vars that have been set(). query() now handles init() and query/query_vars setup. The $query arg to parse_query() is now optional. It is never passed by core code and could probably be gotten rid of, but it is still handled in case a plugin is calling it directly for whatever mysterious reason.

Note that using the set(), get_posts() style requires running init() yourself if you want to do another query with the same query object . This was true in 3.0 as well.

comment:9 @ryan4 years ago

The $parsed_query flag is probably not needed now, but I left it in.

comment:10 @scribu4 years ago

In get_posts(), $this->parsed_query is set to false, just after it's set to true in parse_query(). Is that intentional?

comment:11 @scribu4 years ago

Nevermind, of course it's intentional. But what's the point in having it? Less code = less bugs.

@ryan4 years ago

Without parsed_query flag

comment:12 @scribu4 years ago

  • Keywords has-patch needs-testing added

comment:13 @markjaquith4 years ago

Ryan, what's the test case that your patch passes but mine doesn't?

comment:14 @ryan4 years ago

	$query = new WP_Query;
	$query->set('cat', 4);
	$posts = $query->get_posts();

With your patch all set()s are stomped.

comment:15 @ryan4 years ago

Do the query from an init hook. It will always fail. A query run after the big query could sometimes work (although anyone doing that really should call init() before hand). My patch should work either way.

Version 1, edited 4 years ago by ryan (previous) (next) (diff)

comment:16 @ryan4 years ago

Testing should be done with a clean object and a dirty object that has already run a query.

comment:17 @ryan4 years ago

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

(In [17451]) Run parse_query() in get_posts() so that the query vars are always parsed for set(), get_posts() patterns. fixes #16545 for trunk

comment:18 @ryan4 years ago

(In [17452]) Run parse_query() in get_posts() so that the query vars are always parsed for set(), get_posts() patterns. fixes #16545 for 3.1

comment:19 @nacin4 years ago

This broke get_posts(). #16575

comment:20 @nacin4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@ryan4 years ago

comment:21 @ryan4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [17455]) Use ->query instead of . Always kick off the parse_query action. fixes #16545 for trunk

comment:22 @ryan4 years ago

(In [17456]) Use ->query instead of . Always kick off the parse_query action. fixes #16545 for 3.1

comment:23 @mikeindustries4 years ago

What's the latest on this? Appreciate everyone's work but kind of sucks that WP 3.1 was released with this as a known problem. Everyone who upgraded to 3.1 and had been excluding categories is now unintentionally polluting their public RSS feeds with unexpected content. This is the first time I've upgraded WordPress and regretted it :(.

Thanks in advance for the help.

comment:24 @nacin4 years ago

It's not a known problem. The ticket is closed as fixed.

What is your code?

comment:26 @markjaquith4 years ago

Opened #16622 to track this bug.

Note: See TracTickets for help on using tickets.