-
Notifications
You must be signed in to change notification settings - Fork 875
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds fallback to main url when the indexable is not available. #21393
Adds fallback to main url when the indexable is not available. #21393
Conversation
Pull Request Test Coverage Report for Build 050032edf51f3e0ae7d3f66c4d62b5acf64fb364Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_url_for_queried_object()
is used for two things:
- For populating the
<link>>
element - For creating the canonical header
While, we correctly fallback to the homepage for the former, I don't think we should do the same for the latter. I think it would be better to not output a canonical header at all, in those cases. At least, that was the final summary of our decisions here, so if we do something different we have to get a new approval.
Also, it would be very nice if we had test instructions for all changes here, not only for the author archives - that would help us also understand when those cases happen for the other feeds, for future reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR + AT is 👍
@tatasha2004 @oksanayoast
There is an issue reported for the case of Author without post. |
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
(in all cases, there should be no PHP notices thrown once you load the feeds)
Author case.
Remove post authors feeds
enabled in de crawl optimizations. And make sure you do have author archives enabled.with
a post. You should see<link>http://basic.wordpress.test/author/{authorname}/</link>
In the network tab make sure you see a header in the request for the page with alink
to<http://basic.wordpress.test/author/{authorname}/>; rel="canonical"
without
a post. You should see<link>http://basic.wordpress.test/</link>
. Also make sure there is no canonical header.Term case
link
points to that category page and the Link header does the same.link
now goes to the homepage. Also make sure there is no canonical header.Post case
/feed/
. Much like the production version: make sure thelink
points to that specific page.link
now goes to the homepage. Also make sure there is no canonical header.CPT archive case
link
points to that specific page. For examplehttp://basic.wordpress.test/yoast-test-movies/feed
link
now goes to the homepage. Also make sure there is no canonical header.Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
link
tag and the Link header point to the homepage. For examplehttp://basic.wordpress.test/
UI changes
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes #
#20436