Elasticpress: WooCommerce feature and Yoast "is multiple terms query" logic conflict

[kallehauge]

Have you searched for similar issues before submitting this one? Yes

Is this a bug, question or feature request? Feature request, I guess

Describe the issue you encountered:

There is a logic conflict when the WooCommerce feature is enabled with Yoast. The conflict will cause issues like "all product category archives will be converted to robots => noindex" due to a conflict with Yoasts WPSEO_Frontend::is_multiple_terms_query logic that then states that the page is not a single term archive and should therefor not be indexed by robots.

Example: _If we have a parent category called Clothes with a child category named Hoodies, then the parent (Clothes) will be shown as noindex but the child category (Hoodies) will not._

TL;DR Parent term archives with child terms is being set to robots noindex by Yoast.

Current WordPress version: 4.8.1

Current ElasticPress version: Develop branch

Current Elasticsearch version: 2.3

Where do you host your Elasticsearch server: AWS

Other plugins installed (WooCommerce, Simple Redirect Manager, etc..):

• WooCommerce
• Yoast

Steps to reproduce:

1. Install and enable WordPress + WooCommerce + Yoast + ElasticPress.
2. Enable EP's WooCommerce feature.
3. Go to a product category with child terms and inspect the DOM. The parent's term archive is now changed from robots => indexed to robots => noindex.

Why this happens:

This happens because WPSEO_Frontend::is_multiple_terms_query assumes an internal WP Tax Query logic to "include_children" but the way we support shorthand taxonomy queries, we fetch all child terms and converting them into a WP_Tax_Query instead:

_ep_wc_translate_args() in woocommerce.php:_

/**
 * Next check if any taxonomies are in the root of query vars (shorthand form)
 */
foreach ( $supported_taxonomies as $taxonomy ) {
    $term = $query->get( $taxonomy, false );

    if ( ! empty( $term ) ) {
        $integrate = true;

        $terms = array( $term );

        // to add child terms to the tax query
        if ( is_taxonomy_hierarchical( $taxonomy ) ) {
            $term_object = get_term_by( 'slug', $term, $taxonomy );
            $children    = get_term_children( $term_object->term_id, $taxonomy );
            if ( $children ) {
                foreach ( $children as $child ) {
                    $child_object = get_term( $child, $taxonomy );
                    $terms[]      = $child_object->slug;
                }
            }

        }

        $tax_query[] = array(
            'taxonomy' => $taxonomy,
            'field'    => 'slug',
            'terms'    => $terms,
        );
    }
}

_Simplified WPSEO_Frontend::is_multiple_terms_query:_

protected function is_multiple_terms_query() {
    global $wp_query;

    $term = get_queried_object();
    $queried_terms = $wp_query->tax_query->queried_terms;
    return count( $queried_terms[ $term->taxonomy ]['terms'] ) > 1;
}

Solution?

A solution could be to remove our Tax_query after we have fetched posts in EP_WP_Query_Integration::filter_the_posts if we add an action hook?

The reason I think that this is something we could/should solve in this plugin is that I feel like it should be as seamless as possible to use it without changing the logic more than necessary. So a process of "Clean up after ourselves" after we make changes seems logical to me. We should, at the very least, find a solution to be able to fit right in with Yoast as one of the most used plugins.

_Experimental code that I currently use to support Yoast: Pull-Request. This is not ment to be a finial solution but it currently works for me._

[kallehauge]

This PR could be a suggestion to how we could make support for Yoast on taxonomy archives (rough draft). It kinda cleans up the tax query, in the WP_Query.

This should also work with https://github.com/10up/ElasticPress/pull/801 out of the box.

[kallehauge]

@tlovett1 @ivankristianto is it just me who feels like this is a bit critical, since Yosat is as popular as it is?

[ivankristianto]

Sorry for late response.
@kallehauge most of our team are on company summit currently.
We will discuss this matter when we are back.
Thanks for following up.

[kallehauge]

@ivankristianto What's up doc? :smile:

[ivankristianto]

@kallehauge
I test this with latest EP ( branch develop ), Woocommerce v3.3, and WordPress SEO (Yoast) v5.5.1.
And cannot replicate the issue.
And I'm wondering, can you make sure the "Search Engine Visibility" setting is set to false?

[kallehauge]

Hey @ivankristianto !
I just tried again with the latest versions on a Storefront install with dummy-data given from WooCommerce. It still occurs for me when I look at a parent term archive. Example:
_If we have a parent category called Clothes with a child category named Hoodies, then the parent (Clothes) will be shown as noindex but the child category (Hoodies) will not. This is due to the logic in the code-examples I provided in the main description of the issue._

BTW: I can validate that Search Engine Visibility is not set to false :)

WooCommerce status report with environment information:

### WordPress Environment ###

Home URL: http://woocommerce.dev
Site URL: http://woocommerce.dev
WC Version: 3.1.2
Log Directory Writable: ✔
WP Version: 4.8.2
WP Multisite: –
WP Memory Limit: 2 GB
WP Debug Mode: ✔
WP Cron: ✔
Language: en_US

### Server Environment ###

Server Info: Apache/2.4.18 (Unix) PHP/5.6.16 LibreSSL/2.2.6
PHP Version: 5.6.16
PHP Post Max Size: 8 MB
PHP Time Limit: 120
PHP Max Input Vars: 1000
cURL Version: 7.43.0
SecureTransport

SUHOSIN Installed: –
Max Upload Size: 8 MB
Default Timezone is UTC: ✔
fsockopen/cURL: ✔
SoapClient: ✔
DOMDocument: ✔
GZip: ✔
Multibyte String: ✔
Remote Post: ✔
Remote Get: ✔

### Database ###

WC Database Version: 3.1.2
WC Database Prefix: wp_
woocommerce_sessions: ✔
woocommerce_api_keys: ✔
woocommerce_attribute_taxonomies: ✔
woocommerce_downloadable_product_permissions: ✔
woocommerce_order_items: ✔
woocommerce_order_itemmeta: ✔
woocommerce_tax_rates: ✔
woocommerce_tax_rate_locations: ✔
woocommerce_shipping_zones: ✔
woocommerce_shipping_zone_locations: ✔
woocommerce_shipping_zone_methods: ✔
woocommerce_payment_tokens: ✔
woocommerce_payment_tokenmeta: ✔
MaxMind GeoIP Database: ✔

### Security ###

Secure connection (HTTPS): ❌Your store is not using HTTPS. Learn more about HTTPS and SSL Certificates.
Hide errors from visitors: ❌Error messages should not be shown to visitors.

### Active Plugins (3) ###

ElasticPress: by Taylor Lovett
Matt Gross
Aaron Holbrook
10up – 2.4

WooCommerce: by Automattic – 3.1.2
Yoast SEO: by Team Yoast – 5.5.1

### Settings ###

API Enabled: ✔
Force SSL: –
Currency: DKK (DKK)
Currency Position: left
Thousand Separator: .
Decimal Separator: ,
Number of Decimals: 2
Taxonomies: Product Types: external (external)
grouped (grouped)
simple (simple)
variable (variable)

Taxonomies: Product Visibility: exclude-from-catalog (exclude-from-catalog)
exclude-from-search (exclude-from-search)
featured (featured)
outofstock (outofstock)
rated-1 (rated-1)
rated-2 (rated-2)
rated-3 (rated-3)
rated-4 (rated-4)
rated-5 (rated-5)


### WC Pages ###

Shop base: #4 - /
Cart: #5 - /cart/
Checkout: #6 - /checkout/
My account: #7 - /my-account/

### Theme ###

Name: Storefront
Version: 2.2.5
Author URL: https://woocommerce.com/
Child Theme: ❌ – If you are modifying WooCommerce on a parent theme that you did not build personally we recommend using a child theme. See: How to create a child theme
WooCommerce Support: ✔

### Templates ###

Overrides: –

[ivankristianto]

@kallehauge
Ah I see the issue now.
The issue only occur to the parent terms.
cc: @lukaspawlik

[kallehauge]

Brilliant @ivankristianto :relaxed: I've updated the main description of the issue with the provided example for future reference.

[kallehauge]

Hi @ivankristianto / @lukaspawlik ! What is the status of this?

[kallehauge]

Ping @ivankristianto @lukaspawlik :)

[lukaspawlik]

@eugene-manuilov Could you please take over this one?

Thanks

[eugene-manuilov]

Hi @kallehauge,

I tend to consider it as a Yoast SEO issue because is_multiple_terms_query function checks arguments which may not represent original intent. If you look at that function description, you will see that it says Check if term archive query is for multiple terms (/term-1,term2/ or /term-1+term-2/). That means that it wants to exclude custom archive pages "crafted" by visitors. On the other hand, if you look at ep_wc_translate_args function, you will see that it just explicitly lists all children terms, exactly what WordPress does implicitly.

The proper version of is_multiple_terms_query function should look something like this:

protected function is_multiple_terms_query() {
    global $wp_query;

    $term = get_queried_object();
        $value = $wp_query->get( $term->taxonomy );
    $queried_terms = explode( '+', $value );
    return count( $queried_terms ) > 1;
}

So, I would suggest you open a new ticket at Yoast SEO support and ask them to fix the logic in their function.

[eugene-manuilov]

HI @kallehauge! I am going to close this issue for now. Feel free to re-open it, if you have anything else.