johnbillion / extended-cpts

A library which provides extended functionality to WordPress custom post types and taxonomies.
GNU General Public License v2.0
979 stars 96 forks source link

admin_filter for taxonomy should only display if taxonomy is associated with post type #151

Closed claytoncollie closed 3 years ago

claytoncollie commented 3 years ago

When using the admin-filter in conjunction with taxonomy, the select field should only display if the taxonomy is associated with that post type.

My use case is that I made a taxonomy that can be associate with any number of post types when configured on a per-client basis. Some clients need post type A and B, others need A and C. In preparation for any of those post types being associated with the taxonomy, I added the admin-filter to post types A, B, C by default with the code below. And now the select field displays on all three post types instead of only the post type that I associated with the taxonomy.

'admin_filters' => array(
    'featured' => array(
        'taxonomy' => 'poa_featured_taxonomy',
    ),
),

I looked through the source code to see where I could insert an if statement to use get_object_taxonomies and in_array to change this behavior. I do not understand the package well enough to do some damage on my local. :)

I am happy to submit a PR if you can point me in the right direction.

johnbillion commented 3 years ago

Thanks for the report.

To be honest I don't think this is the responsibility of Extended CPTs to handle. The same logic applies to filters for post meta fields, and various column types, and there's no way to know whether a post type uses a given meta field so a filter will always be shown if it's declared in the post type options.

Is there a reason you can't conditionally register this filter at the point where the post type is registered?

johnbillion commented 3 years ago

Happy to take a look at your code if you've got a sample to show me. It might just be a case of rearranging the order in which you perform the logic to determine whether the post type supports the taxonomy.

claytoncollie commented 3 years ago

This is great feedback. I use those filters in other places but didn't think to use it here. Now that I have the logic moved to my class and inserting via a filter, my new hurdle is that the filters are set up on a per post type basis instead of having a generic filter for the $args. I have 10 or so post types and also add more based on the client's needs. I would like this filter to apply to all registered post types. Maybe another filter before this filter like below to make it more general. Probably needs to pass the $post_type as a parameter along with $args.

Happy to write a PR for this as well.

$args  = apply_filters( "ext-cpts/args", $args, $post_type );

Here is the proposed filter with the change from above.

add_filter( 'ext-cpts/args', 'admin_filters', 10, 2 );
function admin_filters( array $args, string $post_type ) : array {

    if ( in_array( $post_type, $this->get_post_types(), true ) ) {
        $args['admin_filters']['visibility'] = array( 'taxonomy' => 'poa_visibility_taxonomy' );
    }

    return $args;
}
johnbillion commented 3 years ago

I'm not opposed to adding that filter - it seems useful - but I wonder if you can achieve it already by filtering your arguments with your own filter just before you pass them to register_extended_post_type(). For example:

function register_my_standard_post_type( string $post_type ) {
    // Arguments common to all my CPTs:
    $args = [
        'show_in_rest' => true,
    ];

    // Filter the arguments:
    $args = apply_filters( 'my_standard_cpt_args', $args, $post_type );

    // Register the CPT:
    register_extended_post_type( $post_type, $args );
}

add_filter( 'my_standard_cpt_args', 'admin_filters', 10, 2 );

register_my_standard_post_type( 'foo' );
register_my_standard_post_type( 'bar' );
register_my_standard_post_type( 'baz' );
claytoncollie commented 3 years ago

Yes. I can do it that way for the cpt registered in my main plugin since they are all bundled together. But this does not catch the client-specific post types that are registered in the mu-plugins file or any other client-specific plugin that needs a cpt and uses the register_extended_post_type( $post_type, $args );

I could make a wrapper function for those instances but it is harder for onboarding devs to teach them about this one platform function versus saying, hey here is this standardized repo for registering our cpts and taxonomies that has documentation and community support.

johnbillion commented 3 years ago

In that case, happy to add the suggested generic filter. Do you want to work on a PR for this so you can test it as you go?

claytoncollie commented 3 years ago

Sure, I can write the PR.