Make WordPress Core

Opened 14 years ago

Closed 13 years ago

#14892 closed enhancement (fixed)

minor improvement on category__in

Reported by: sboisvert's profile sboisvert Owned by: scribu's profile scribu
Milestone: 3.1 Priority: normal
Severity: minor Version: 3.1
Component: Query Keywords:
Focuses: Cc:

Description (last modified by scribu)

if categoryin is only provided an int, it will fail (relatively silently). I find this to be "not friendly"

If I may suggest changing lines 1371-1372 in /wp-includes/query.php to this:

if ( !is_array($qv['category__in']) || empty($qv['category__in']) ) {
	if (is_long($qv['category__in']) || is_int($qv['category__in'])){
		$qv['category__in'] = array($qv['category__in']);
	} else{
		$qv['category__in'] = array();	
	}

I would in the same vein implement the change for categorynot_in

if ( !is_array($qv['category__not_in']) || empty($qv['category__not_in']) ) {
				if (is_long($qv['category__not_in']) || is_int($qv['category__not_in'])){
					$qv['category__not_in'] = array($qv['category__not_in']);
				} else{
					$qv['category__not_in'] = array();	
				}

I would also suggest implementing for tagin and etc etc

Change History (16)

#1 @nacin
14 years ago

We can just cast it to an array, then run absint() on it using array_map().

#2 @sboisvert
14 years ago

That would be even better! Thank you.

#3 @scribu
14 years ago

  • Description modified (diff)

#4 @scribu
14 years ago

  • Milestone changed from Awaiting Review to 3.1
  • Owner set to scribu
  • Status changed from new to accepted

#5 @scribu
14 years ago

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

(In [15626]) Don't require categoryin, tagin etc. to be arrays necessarily. Fixes #14892

#6 @scribu
14 years ago

I also noticed that $is_category was set for tagand instead of $is_tag. This is also fixed in [15626].

#7 @nacin
14 years ago

(In [15627]) Fix logic inversion in r15626. see #14892. Also note that r15626 fixed a s/is_category/is_tag/ typo introduced in r6011, on line 1340.

#8 follow-up: @linuxologos
13 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version changed from 3.0.1 to 3.1

I'm testing 3.1-RC2-17229 and it seems to me that this fix is not working as described. In my case, category_in still requires an array. Tag_in/tag_not_in are working fine with integers though.

#9 in reply to: ↑ 8 @westi
13 years ago

  • Keywords reporter-feedback added

Replying to linuxologos:

I'm testing 3.1-RC2-17229 and it seems to me that this fix is not working as described. In my case, category_in still requires an array. Tag_in/tag_not_in are working fine with integers though.

How are you testing?

An example piece of code that shows the issue will be really helpful in debugging this.

#10 @westi
13 years ago

Reproduced with this:

function query_tests() {
	query_test_one();
	query_test_two();
}

function query_test_one() {
	$one = new WP_Query();
	$one->query( array( 'category__in' => array( 1 ) ) );
}
function query_test_two() {
	$one = new WP_Query();
	$one->query( array( 'category__in' => 1 ) );
}

add_action( 'wp_footer', 'query_tests' , 1);

The first produces a valid query.

The second produces a notice:

WARNING: wp-includes/query.php:1695 - array_unique() expects parameter 1 to be array, integer given

Seeing as this was added in 3.1 and then removed again I think we can move this to 3.2 as an enhancement.

#11 @linuxologos
13 years ago

And even simplier:

The code below should be working according to [15626]:

query_posts( array( 'category__in' => 1 ) )

but it does not, returning unexpected posts and the already mentioned warning:

array_unique() expects parameter 1 to be array, integer given in ...\wp-includes\query.php on line 1695

The code works as expected only if category_in is provided an array (just like in the past), for example:

query_posts( array( 'category__in' => (array)1 ) ) or
query_posts( array( 'category__in' => array(1) ) )

Tag_in/tag_not_in (and possibly other parameters too) are working fine with integers, as they should according to [15626].

Last edited 13 years ago by linuxologos (previous) (diff)

#12 @sboisvert
13 years ago

Just to make it easier for someone else who can actually fix the problem, it seems to be related to the fact that

parse_tax_query()

parses categories differently than tags/tax, the logic that was added for this in line 1695 is as follows:

$q['category__in'] = array_unique( $q['category__in'] );

Which seems to be different than the one in [15626], in that the one in [15626] casts as an array and then traverses and casts the values of the array as ints:
$qv['category__in'] = array_map('absint', (array) $qv['category__in']);

I suspect the behavior in [15626] might be the better one, but I wouldn't be able to say anything with any certainty if there is any valid reason why it was not implemented this way.

Thank you,

Last edited 13 years ago by ocean90 (previous) (diff)

#13 @linuxologos
13 years ago

Replying to westi:

Seeing as this was added in 3.1 and then removed again I think we can move this to 3.2 as an enhancement.

What is a bit annoying is that other parameters of query_posts() (I have not tested them all) are now accepting integers and category_in (only?) is making an odd exception. It seems like an inconsistency.

Last edited 13 years ago by linuxologos (previous) (diff)

#14 @ocean90
13 years ago

  • Keywords query reporter-feedback removed

#15 @linuxologos
13 years ago

Take into consideration this patch by SergeyBiryukov here. It works for me and fixes both tickets (#14892, #16152).

#16 @ryan
13 years ago

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

(In [17245]) Cast category and tag query args to array to allow passing a single ID or slug. Props SergeyBiryukov. fixes #14892

Note: See TracTickets for help on using tickets.