sybrew / the-seo-framework

The SEO Framework WordPress plugin.
https://theseoframework.com/
GNU General Public License v3.0
420 stars 46 forks source link

meta_query conflict #33

Closed ndyjones closed 7 years ago

ndyjones commented 8 years ago

Hi sybrew,

Quick and dirty: Running into a site search conflict when SEO Framework is active that returns 0 content for any s= and from what I can tell it's because of the meta_query arrary that gets set in search.class.php LN 82. I think. Maybe.

Long explanation below, b/c I'm not sure exactly what's going on and code about as well as a caveman with a keyboard. So I manage a site that lives within a WP Multisite within a university environment that has some custom post types and ACF fields. Our multisite admin introduced me to SEO Framework at some point earlier this year and quickly came to appreciate the level of control it provided.

One of our homebrew cpts is a course post that uses ACF for session/date info and an shared all posttype 'Location' taxonomy to create a targeted search for just courses with some options to filter. Initially I was using Ultimate WP Search Query Filter plugin to generate this and embed, but apparently on our multisite AJAX is locked down, and the search won't function for non-University logged in users, which was an issue since our courses are for public at large/industry segments. Sitewide search worked fine, though I could never get UWPSQF to use the non-AJAX search results option, so had to dump it and try building a custom search.

Thus I set to cobbling together a search form that throws filter options via url to a custom search.php for the course post_type and then hits the loop with whichever arguments are provided. Things looked dandy testing in my dev space and when I pushed the theme updates to production, search didn't return anything.

So dumping wp_query on both I ended up with the logs here: multidev-shingo-wp-query.txt production-shingo-wp-query.txt

So I see the main differences between the two as:

` https://multidev.eos.ncsu.edu/ncjones4/?s=shingo MultiDev's request: [request] => SELECT SQL_CALC_FOUND_ROWS DISTINCT wp_17_posts.ID FROM wp_17_posts LEFT JOIN wp_17_postmeta ON wp_17_posts.ID = wp_17_postmeta.post_id WHERE 1=1 AND (((wp_17_posts.post_title LIKE '%shingo%') OR (wp_17_postmeta.meta_value LIKE '%shingo%') OR (wp_17_posts.post_excerpt LIKE '%shingo%') OR (wp_17_posts.post_content LIKE '%shingo%'))) AND wp_17_posts.post_type IN ('post', 'page', 'attachment', 'courses', 'staff', 'project', 'tribe_venue', 'tribe_events') AND (wp_17_posts.post_status = 'publish' OR wp_17_posts.post_status = 'acf-disabled' OR wp_17_posts.post_status = 'private') ORDER BY wp_17_posts.post_title LIKE '%shingo%' DESC, wp_17_posts.post_date DESC LIMIT 0, 5 [posts] => Array ( ...

https://www.ies.ncsu.edu/?s=shingo&debug
Production's request:
[request] => SELECT SQL_CALC_FOUND_ROWS DISTINCT wp_15_posts.ID FROM wp_15_posts LEFT JOIN wp_15_postmeta ON (wp_15_posts.ID = wp_15_postmeta.post_id AND wp_15_postmeta.meta_key = 'exclude_local_search' ) LEFT JOIN wp_15_postmeta ON wp_15_posts.ID = wp_15_postmeta.post_id WHERE 1=1 AND (((wp_15_posts.post_title LIKE '%shingo%') OR (wp_15_postmeta.meta_value LIKE '%shingo%') OR (wp_15_posts.post_excerpt LIKE '%shingo%') OR (wp_15_posts.post_content LIKE '%shingo%'))) AND ( ( wp_15_postmeta.post_id IS NULL ) ) AND wp_15_posts.post_type IN ('post', 'page', 'attachment', 'courses', 'staff', 'project', 'stlr_place', 'tribe_venue', 'tribe_events') AND (wp_15_posts.post_status = 'publish' OR wp_15_posts.post_status = 'acf-disabled' OR wp_15_posts.post_author = 35 AND wp_15_posts.post_status = 'private') GROUP BY wp_15_posts.ID ORDER BY wp_15_posts.post_title LIKE '%shingo%' DESC, wp_15_posts.post_date DESC LIMIT 0, 5 [posts] => Array ( )

Post meta appears in the dump in a couple of spots on Production::

        LN 108
        [meta_query] => Array
            (
                [0] => Array
                    (
                        [0] => Array
                            (
                                [key] => exclude_local_search
                                [value] => 1
                                [type] => CHAR
                                [compare] => NOT EXISTS
                                [relation] => AND
                            )

                    )

            )
        ...
        LN 168
        [meta_query] => WP_Meta_Query Object
            (
                [queries] => Array
                    (
                        [0] => Array
                            (
                                [0] => Array
                                    (
                                        [key] => exclude_local_search
                                        [value] => 1
                                        [type] => CHAR
                                        [compare] => NOT EXISTS
                                        [relation] => AND
                                    )

                                [relation] => OR
                            )

                        [relation] => OR
                    )

                [relation] => AND
                [meta_table] => wp_15_postmeta
                [meta_id_column] => post_id
                [primary_table] => wp_15_posts
                [primary_id_column] => ID
                [table_aliases:protected] => Array
                    (
                        [0] => wp_15_postmeta
                    )

                [clauses:protected] => Array
                    (
                        [wp_15_postmeta] => Array
                            (
                                [key] => exclude_local_search
                                [value] => 1
                                [type] => CHAR
                                [compare] => NOT EXISTS
                                [relation] => AND
                                [alias] => wp_15_postmeta
                                [cast] => CHAR
                            )

                    )

                [has_or_relation:protected] => 
            )

        [date_query] => 
        ...
    `

And when I deactivate SEO Framework and voila, search starts returning things in production again.

So obvs I failed to notice SEO Framework wasn't activated in multidev. Okay. And for some reason &debug doesn't work on multidev, but I dropped a print_r( $wp_query ); in the search.php so I could get the log there.

None of the content in the site has exclude_local_search enabled.

So, not sure why the value of that key is being set as 1. And I'd like to keep using the SEO Framework plugin. I don't anticipate we'd need to hide anything in the site from search, so what I'd kinda like to test is maybe overriding that value to always 0 and testing multidev to see if that's enough to fetch results.

Alternatively I may have done something stupid with search.php and search-courses.php such that's conflicting with SEO Framework, but I don't immediately see it. I've stuck the child theme on my personal git here https://github.com/ndyjones/ies-divi-child if it helps track whatever the hell I've done.

Any insights or help you could provide would be much appreciated.

sybrew commented 8 years ago

Hi NC Jones,

As I'm going to remove the Search class, here's a reference link to what this issue is about: <search.class active lines of code>

Some information: The negative search, as shown below, is added to exclude posts that contain that value:

array(
    'key'      => 'exclude_local_search', // Search for this key.
    'value'    => '1',                    // It being 1, or positive (true).
    'type'     => 'CHAR',                 // As a CHAR, it's serialized as such in the database.
    'compare'  => 'NOT EXISTS',           // It must not exist, or be false.
    'relation' => 'AND',                  // Continue default search behavior.
),

This code has been introduced in version 2.7.0 of The SEO Framework. The previous way of handling this was injecting and filtering all posts (could be thousands) as a search query happens. This was inefficient.

The issue If I'm not mistaken, this is the issue at hand:

I browsed through your Divi child theme, and I saw multiple loops being loaded and edited. As I'm not a theme developer, I took a peek at another theme (i.e. Storefront by WooThemes) and I saw that they simply use:

get_template_part( 'loop' );

I believe there's a lot of complexity going on in your Search Template file. Could you see if excluding some parts of "extra" code to see if the search query acts as it should? If the concerned code is found, could you share it? This way I can determine where this issue is located.

Cheers!

P.S. take a peek at the Ternary Operator for your $_GET variable setting :). Also, I found a bug here (mixed variables).

ndyjones commented 8 years ago

Yes, I'll certainly take a look and try testing a simpler loop. I had been trying about 10 different examples I'd found of custom post type searchs to get it to return relevant posts, so no surprise if I scrambled something up somewhere. I'll cut it down and test on my dev and get back to you.

sybrew commented 8 years ago

Hi NC Jones,

I believe this is what you're looking for: https://gist.githubusercontent.com/galengidman/8b84770a2dcc9abb8bfe/raw/9dae519b202b4ec853bdca8d1689d23f61571b28/search.php

From: https://galengidman.com/2014/06/09/custom-post-type-search-results-templates-in-wordpress/

Best of luck!

Edited code from first link (preventing PHP warnings)

<?php

// store the post type from the URL string
$post_type = ! empty( $_GET['post_type'] ) ? $_GET['post_type'] : '';

// check to see if there was a post type in the
// URL string and if a results template for that
// post type actually exists
if ( 'courses' === $post_type && locate_template( 'search-courses.php' ) ) {

  // if so, load that template
  get_template_part( 'search', 'courses' );

  // and then exit out
  exit;
}

?>

<!-- default search results here -->

On the post type search template, you should simply edit the WordPress query.

Like so:

add_action( 'pre_get_posts', 'my_search_query_edit' );
function my_search_query_edit( $query ) {
    $query->set('post_type', 'courses' );
}

And then simply output the search page as on search.php, without other alterations.

EDIT: Adjusted code example towards your template.

ndyjones commented 8 years ago

Thanks again for you input @sybrew ! The post you linked to was definitely in my reference material when trying to build this as I scoured many cpt search implementations. The tricky part here was for search-courses.php I have have to be able to force some things into the query if specified, like taxonomy and meta key + value.

But ultimately I think that shouldn't have any effect on search.php results, so now that I've finally had a chance to removed the explode piece and edited the search.php so that it's only checking and redirecting if 'course' === $post_type and not modifying the query otherwise, and I still get 0 results when the SEO Framework is enabled.

So this implementation is up on my dev site now which you can see @ https://multidev.eos.ncsu.edu/ncjones4/?s=shingo Where again, I've dumped the wp_query to try and see what going on. I will leave the SEO Framework active right now to give you a chance to see, but the only major difference I see it the 'exclude_local_search key in the meta_query field. When SEO Framework is not active I get two results for that search term.

sybrew commented 8 years ago

Hi NC Jones,

When you add the following code snippet to your theme's functions.php file, does the search results output work again?

function_exists( 'the_seo_framework' ) and remove_action( 'pre_get_posts', array( the_seo_framework(), 'adjust_search_filter' ), 9999 ); 

I'm curious as of if the search adjustment of mine is the actual cause or not. Cheers!

ndyjones commented 8 years ago

@sybrew Have added that to the theme's functions.php and getting results now with SEO Framework active, can see here https://multidev.eos.ncsu.edu/ncjones4/?s=shingo. Also the courses custom post type search returns results with that implemented as well.

This was sort of my first impulse, to remove that tiny bit via theme's functions.php, but again, I couldn't figure out why it seems to think the 'exclude_local_search' is true on all the content even though it's not set on anything, custom post types or otherwise.

So I think I can push the remove_action bit to production to get SEO Framework reactivated there and maintain the other functionality, but happy to continue testing any suggestions you have to further track down the issue.

sybrew commented 8 years ago

Hi NC Jones,

I dug into your query dumps, and I found the following.

The working query is:

SELECT SQL_CALC_FOUND_ROWS  wp_17_posts.ID FROM wp_17_posts  WHERE 1=1  AND (((wp_17_posts.post_title LIKE '%shingo%') OR (wp_17_posts.post_excerpt LIKE '%shingo%') OR (wp_17_posts.post_content LIKE '%shingo%')))  AND (wp_17_posts.post_password = '')  AND wp_17_posts.post_type IN ('post', 'page', 'attachment', 'courses', 'staff', 'project', 'tribe_venue', 'tribe_events') AND (wp_17_posts.post_status = 'publish' OR wp_17_posts.post_status = 'acf-disabled')  ORDER BY wp_17_posts.post_title LIKE '%shingo%' DESC, wp_17_posts.post_date DESC LIMIT 0, 5

The non-working query (with the meta query adjustment) is:

SELECT SQL_CALC_FOUND_ROWS DISTINCT wp_17_posts.ID FROM wp_17_posts  LEFT JOIN wp_17_postmeta ON (wp_17_posts.ID = wp_17_postmeta.post_id AND wp_17_postmeta.meta_key = 'exclude_local_search' ) LEFT JOIN wp_17_postmeta ON wp_17_posts.ID = wp_17_postmeta.post_id  WHERE 1=1  AND (((wp_17_posts.post_title LIKE '%shingo%') OR (wp_17_postmeta.meta_value LIKE '%shingo%') OR (wp_17_posts.post_excerpt LIKE '%shingo%') OR (wp_17_posts.post_content LIKE '%shingo%')))  AND (wp_17_posts.post_password = '')  AND (           [request] => SELECT SQL_CALC_FOUND_ROWS  wp_17_posts.ID FROM wp_17_posts  WHERE 1=1  AND (((wp_17_posts.post_title LIKE '%shingo%') OR (wp_17_posts.post_excerpt LIKE '%shingo%') OR (wp_17_posts.post_content LIKE '%shingo%')))  AND (wp_17_posts.post_password = '')  AND wp_17_posts.post_type IN ('post', 'page', 'attachment', 'courses', 'staff', 'project', 'tribe_venue', 'tribe_events') AND (wp_17_posts.post_status = 'publish' OR wp_17_posts.post_status = 'acf-disabled')  ORDER BY wp_17_posts.post_title LIKE '%shingo%' DESC, wp_17_posts.post_date DESC LIMIT 0, 5
  (         
    wp_17_postmeta.post_id IS NULL      
  )     
) AND wp_17_posts.post_type IN ('post', 'page', 'attachment', 'courses', 'staff', 'project', 'tribe_venue', 'tribe_events') AND (wp_17_posts.post_status = 'publish' OR wp_17_posts.post_status = 'acf-disabled') GROUP BY wp_17_posts.ID ORDER BY wp_17_posts.post_title LIKE '%shingo%' DESC, wp_17_posts.post_date DESC LIMIT 0, 5

The big difference:

LEFT JOIN wp_17_postmeta ON (wp_17_posts.ID = wp_17_postmeta.post_id AND wp_17_postmeta.meta_key = 'exclude_local_search' )

What's peculiar is that my adjustment definitely asks that it should be "NOT EXISTS", but the output query tries to find "EXISTS" matches here.

The underlying reason still has to be found. But this is definitely a bug; although situational.

sybrew commented 7 years ago

Hi @ndyjones,

I'll attach a small patch (please test 😄) in a moment which I hope should fix this, letting WordPress' WP_Meta_Query figure this out automatically.

Cheers!

sybrew commented 7 years ago

Hi @ndyjones,

I'm closing this issue for now. If you still encounter this issue after using the latest Master version or the 2.8.0 release version, feel free to re-open this issue.

Cheers! 😄

jhned commented 7 years ago

@sybrew, I tried removing the action the way you described above, but it didn't work. I checked against the current version of the plugin and tried a few different methods, but still couldn't get it working.

In the past, I've only been able to remove actions from classes that have static methods. I tried that here too, but still nothing.

sybrew commented 7 years ago

@jhned you need to run the action removal after init priority 0, as at that point the action is created.

i.e.:

add_action( 'init', function() {
    $tsf = function_exists( 'the_seo_framework' ) ? the_seo_framework() : null;

    if ( is_object( $tsf ) ) 
        remove_action( 'pre_get_posts', array( $tsf, 'adjust_search_filter' ), 9999 ); 

}, 1 ); // note the 1, it's higher than 0 :)
jhned commented 7 years ago

Oh, derp! That does it. Thank you!

tripflex commented 7 years ago

Thank you for the code snippet, I had to add this to my site as I have a very large custom post type database, and with this plugin adding the exclude_local_search meta query to all queries, was adding anywhere from 2-5 seconds of extra load time on every page load