Skip to content
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

Make sure we have a URL when the canonical is empty #18604

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

jdevalk
Copy link
Contributor

@jdevalk jdevalk commented Jun 22, 2022

Context

  • In some cases, when plugins or themes add custom URL types to WordPress, we can't detect the proper canonical. This messes up our Schema too, this fix prevents us from outputting broken schema by using the current URL in the schema instead.

Summary

This PR can be summarized in the following changelog entry:

  • When we don't have a canonical, use the current URL in the Schema output.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

  • Install BBPress;
  • go to any user profile page in the BBPress forum;
  • see that the schema is completely broken without this fix and correct with it.

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

  • Please test Schema output on a few more page types.

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unittests to verify the code works as intended

Fixes PRODUCT-190

@jdevalk jdevalk added the changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog label Jun 22, 2022
@jdevalk jdevalk requested a review from herregroen June 22, 2022 12:56
@IreneStr
Copy link
Contributor

@jdevalk I guess this is still relevant, right?
I found a slack conversation with you saying you requested a review by Herre. Shall I send it to Plugin instead?

@jdevalk
Copy link
Contributor Author

jdevalk commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants