Yoast / wordpress-seo

Yoast SEO for WordPress
https://yoast.com/wordpress/plugins/seo/
Other
1.77k stars 891 forks source link

Author sitemap is broken when it is registered as a taxonomy #6354

Closed andizer closed 7 years ago

andizer commented 7 years ago

For each sitemap we are matching its name, but this will break for the author sitemap. This will conflict with a the author is a custom taxonomy. The same might happen when author is a custom post type, I didn't test that.

In the pull #6345 this will fixed by changing the file sequence, but the problem should be fixed properly. In this case the author sitemap will still remain, but the custom taxonomy will disappear.

We might rename the author sitemap name in some thing custom when the taxonomy/posttype author already exists.

stodorovic commented 7 years ago

If it's related to #6343 then sitemap for custom taxonomy doesn't exist because it's private taxonomy.

So, there are some inconsistencies between functions handles_type and get_sitemap_links: https://github.com/Yoast/wordpress-seo/blob/trunk/inc/sitemaps/class-taxonomy-sitemap-provider.php#L43-L46 https://github.com/Yoast/wordpress-seo/blob/trunk/inc/sitemaps/class-taxonomy-sitemap-provider.php#L165-L167

Better fix for #6343 is:

        public function handles_type( $type ) {
                $taxonomy = get_taxonomy( $type );

                if ( $taxonomy === false || ! $this->is_valid_taxonomy( $taxonomy->name ) || ! $taxonomy->public ) {
                        return false;
                }

        return true;
    }

In this case, authors of plugins can use filter wpseo_sitemap_exclude_taxonomy if they want to exclude custom taxonomy. (it isn't related only to author taxonomy) or it's already excluded if it's private (which fixes #6343).

Also, it's good because developers can handle custom sitemap (for some taxonomy) if they exclude it via filter. I'm not sure that's easy with current code.

We might rename the author sitemap name in some thing custom when the taxonomy/posttype author already exists.

@andizer I think that's complex solution. You can think over about my proposal. If you want I can make new PR if it's easier to you.

andizer commented 7 years ago

Thanks @stodorovic. You are right, this issue is related to #6343.

I know it's complex. With so many plugins on WordPress, I have no doubt there is a book library plugin (or something like that) that is using author as a posttype or taxonomy. With the change to have this public there will be a sitemap loss.

I think its better to catch the conflicting situation by renaming the author sitemap in something else.

@omarreiss @moorscode @atimmer What do you think of this, any suggestions on this?

atimmer commented 7 years ago

Duplicates #6343