[17246] Breaks category exclusion

[markjaquith]

[17246] broke category exclusion using this code (change the excluded cats to match something in your install).

function exclude_category($query) {
	if ( $query->is_feed ) {
		$query->set('cat', '-5,-6');
	}
	return $query;
}

add_filter('pre_get_posts', 'exclude_category');

Reported by Mike Davidson ​here.

I use pre_get_posts to only include certain categories in the feed and homepage, like:

add_filter('pre_get_posts', 'featured_posts');
function featured_posts($query)
{
        if (is_single()) return;

	if ($query->is_home) 
	{
	        $query->set('cat', '50');
		return $query;
	}
	
	if ($query->is_feed)
	{
		$query->set('cat', '50,172');
		return $query;
	}
	
	return $query;
}

But none of this code actually works after 3.1.

The quick fix is to lose $parsed_tax_query and take the hit of having to call get_term_children() twice.

There is still a possible problem with that though. Setting 'cat' adds to the category inclusions and exclusions, rather than replacing them. Trying to override a 'cat' query won't work as before. I don't think this is a common usage though.

Take a look at that patch. I suggested this before. Hashes the state of the query args, and compares it to the state of the current state, to determine whether something has changed.

pre_get_posts is an unreliable place to do set(). You are setting a query var after the query vars are parsed. It happens to work for a few things in 3.0, but it isn't uniformly reliable. It also don't set the query flags to match the query. But, I guess these things have become features. :-)

Anyhow, the patch seems a good approach. The query vars array is usually pretty small so I don't think the serialize will be a big hit. Going forward, we might want to

md5() probably isn't necessary, unless I'm missing something.

Replying to ryan:

pre_get_posts is an unreliable place to do set(). You are setting a query var after the query vars are parsed.

The same would be true of parse_query too, then. They're identical hooks. And they're fairly useful in my experience.

Replying to nacin:

md5() probably isn't necessary, unless I'm missing something.

Just trying to not balloon the size of that object. Tradeoff between memory/CPU.

Also, for secondary requests, what other hook is available to go in and do ->set()s? It seems like parse_query/pre_get_posts are the only option.

Hashing the query vars seems like a good solution. Might end up with a few false positives sometimes, but it's better to err on the safe side.

md5() is written for hashing large amounts of data, so that shouldn't be a problem.

Oh, since you're hashing all the query vars, you should rename 'parsed_tax_query_hash' to 'query_vars_hash'.

Updated the patch to reduce the number of serialize() calls.

Workaround that should work for those who need it:

function exclude_category($query) {
	if ( $query->is_feed ) {
		$query->set('category__not_in', array(5,6));
	}
	return $query;
}

add_filter('pre_get_posts', 'exclude_category');

Sergey — In that case you're serializing the array before changes are potentially made to it, so when you stash it at the end, it could already be out of date.

Is there a way for $this->query_vars to be modified between those lines? I don't see one, but I may have missed something. I also checked the patch with both of pre_get_posts filters above.

Replying to SergeyBiryukov:

Is there a way for $this->query_vars to be modified between those lines? I don't see one, but I may have missed something. I also checked the patch with both of pre_get_posts filters above.

Yes. Because $q is a reference to $this->query_vars.

Looks good.

(In [17489]) Reparse the taxonomy query if query vars change. fixes #16622 for trunk

(In [17490]) Reparse the taxonomy query if query vars change. fixes #16622 for 3.1