Wrong check for "is_numeric" resulting in faulty admin_filter

[Rayken]

Line 276 @ https://github.com/johnbillion/extended-cpts/blob/master/src/class-extended-cpt-admin.php#L276

Should probably not have !, because right now, if the key is numeric (like a post_id or so) it reverts to using the 'value' and that is really odd.

My use case is creating a dropdown with post_title and its ID as value, right now I'm getting post_title as value as well. Changing the line to:

if ( is_numeric( $k ) ) {

Fixes this issue. However, I do believe you could skip the entire foreach @ line 275-280 and changing line 288 to:

$key = ( is_numeric( $k ) ? $k : $v );

Unless I have misunderstood the use of $use_key. However this is causing issues for me atm!

For options I'm returning something like:

array( 1 => 'Some post', 2 => 'some other post', 3 => 'some third post' )

And the scripts thinks it should use "Some%20post" as filter instead of 1.

[johnbillion]

The reason the ! is_numeric() check is in place is to allow an associative array to be passed and the array keys to be used (is_numeric() is false), and also to allow an indexed array to be passed and for the array values to be used instead (is_numeric() is true).

I can see that if you have an associative array with numeric keys (eg. for post IDs) then this won't behave as expected. I'm not sure how to go about fixing that.

[maxchirkov]

Hi John,

we are running into the same issue - our post types are associated by an arbitrary ID (int value). We need to display a human readable label in the selection list filter, while filtering by the ID. The current implementation doesn't support that.

I looked around and it seems like there are really no good solutions to support the intended behavior. There's no way to determine if current indexes are sequential integers assumed by the system or they are intended by the programmer. There are attempts to solve this issue, but as long as there's an intent to pass sequential integers (even casted as strings) as [0 => "No", 1 => "Yes"] (or ["0" => "No", "1" => "Yes"]) where the keys are expected to be used, the solution will always fail. Heres an excellent stackoverflow answer on this exact subject: https://stackoverflow.com/a/13522545

So, either the solution has to have some sort of "exception" to the way certain scenarios implemented... Or it has to be treated as Key to Value array where the array key is the option value and the array value is the option label. It is a straightforward solution that can't brake. The downside is that the keys will have to be duplicated if they are the same as labels and it will brake your backward compatibility.

To keep the backward compatibility, you'd probably want to add either an alternative options property that would consume keys and values as is, or have extra flag that would indicate that the options keys need to be used i.e. use_options_key => true.

Some food for thought...

Thank you!

[maxchirkov]

After thinking a bit more about it, I think everything could be left the way it is, but change how we express key to value options:

In our case the option tag accepts 2 parameters value and label or (title). If we were to represent option as and object, we would set option.value and option.label. If option.value was not set, then we would default the value to the options.label or vice versa. So, our options array would simply contain array of objects. In our current PHP scenario we could express it as array of arrays:

$filter = [
    'options' => [
        ['value' => 123, 'label' => 'John Doe'],
        ['value' => 124, 'label' => 'Jane Doe']
    ]
];

So, to make it work, all we need to do is to check if our current option item is an array or a string. If string, we use it as option value and label, if array, then we use them as intended.

[tombroucke]

We also need to filter a custom post type by a related post type, and we came up with 2 possible solutions

• add a filter to the $use_key variable after the check if the keys are numeric (apply_filters('ext-cpts/use_key', $use_key, $filter['options']);
• add an extra item to the array

'admin_filters' => array(
    'linked_post_id' => array(
        'title'    => __('Select linked post', 'textdomain'),
        'meta_key' => 'linked_post_id',
        'options' => 'linked_posts',
        'use_key' => false
    ),
),

I assume these solutions wouldn't affect other functionality

[thedavidthomas]

Am also experiencing this issue, any chance it can be resolved?

[tombroucke]

I came up with a workaround (I wouldn't call it a solution) for this. The problem is that $use_key remains false, because the key is an int (numeric), so the value is used, instead of the key.

As a workaround, we can append or prepend the post ID with a string, and later on, remove that string through a filter:

Add filter

Make sure the key for your filter key & meta_key are the same (event_id) in this case.

'admin_filters' => [
    'event_id' => [
        'meta_key' => 'event_id',
        'title' => __('Event', 'textdomain'),
        'options' => function () {
            $events = Event::find();
            $return = array();
            foreach ($events as $key => $event) {
                $return['event_' . $event->getId()] = $event->title();
            }
            return $return;
        },
    ],
],

Remove string

subscription is the post type that has the admin filter, event_id is the meta_key, event_ has been added in the previous step.

add_filter('ext-cpts/subscription/filter-query/event_id', function($return, $query, $filter){
  $return = [];
  $return['meta_query'][] = [
      'key'   => $filter['meta_key'],
      'value' => str_replace('event_', '', wp_unslash($query['event_id'])),
  ];
  return $return;
}, 10, 3);

[LC43]

we tested this:

$use_key  = $filter['use_key'] ?? false;
if ( ! $use_key ) {
	foreach ( $filter['options'] as $k => $v ) {
		if ( ! is_numeric( $k ) ) {
			$use_key = true;
			break;
		}
	}
}

so we can pass:

'foo' => array(
	'title'    => 'fancy foo',
	'meta_key' => 'foo
	'options'  => 'get_foo_values',
	'use_key'  => true,
),

it was the simplest change that didnt break anything and allowed us to force the filter to use the key