Skip to content
This repository has been archived by the owner on Sep 14, 2021. It is now read-only.

Redirect sitemap.xml to wp-sitemap.xml if about to return a 404 #149

Merged
merged 3 commits into from Apr 21, 2020

Conversation

kraftbj
Copy link
Contributor

@kraftbj kraftbj commented Mar 23, 2020

Issue Number

Fixes #144

Description

sitemap.xml is a common location of a site's sitemap. In instances where there is not an existing sitemap.xml either on the file system or served by another plugin, the Core WP-Sitemaps plugin will redirect to the wp-sitemap.xml URL to help improve discoverability and allowed those who have submitted sitemap.xml to search engines to be able to drop-in without change.

Type of change

Please select the relevant options:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Enhancement (change which improves an existing feature. E.g., performance improvement, docs update, etc.)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Steps to test

  • Visit your testing site's sitemap.xml (assuming your server is set to have WP serve that URL, e.g. with pretty permalinks on with .htaccess set for Apache or via the server block in Nginx).
  • On master, a WP-served 404 will be returned.
  • On this branch, it will 302 redirect to wp-sitemap.xml

Acceptance criteria

  • My code follows WordPress coding standards.
  • I have performed a self-review of my own code.
  • If the changes are visual, I have cross browser / device tested.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generate no new warnings.
  • I have added test instructions that prove my fix is effective or that my feature works.

@kraftbj kraftbj added the Type: Feature New feature label Mar 23, 2020
@kraftbj kraftbj added this to the 0.3.0 milestone Mar 23, 2020
@kraftbj kraftbj self-assigned this Mar 23, 2020
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no Has not signed the Google CLA label Mar 23, 2020
@kraftbj
Copy link
Contributor Author

kraftbj commented Mar 23, 2020

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes Signed the Google CLA and removed cla: no Has not signed the Google CLA labels Mar 23, 2020
Included part of a route I opted not to take.
@kraftbj
Copy link
Contributor Author

kraftbj commented Mar 24, 2020

Noting from today's Slack meeting, this PR is on ice until we settle on the URL format.

If we keep wp-sitemap.xml as the file name for our sitemaps, this is needed. If we go with a sitemap.xml option that doesn't conflict with existing implementations, this can be closed.

@pfefferle
Copy link
Contributor

should we also remove the 0.3.0 milestone?

@swissspidy swissspidy removed this from the 0.3.0 milestone Mar 24, 2020
Copy link
Contributor

@svandragt svandragt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, do we want any tests here? @swissspidy

@swissspidy
Copy link
Contributor

If it's possible, sure. But let's wait first whether we actually keep the wp- URLs (#149 (comment))

@svandragt svandragt added the blocked Blocked by another task / decision label Apr 1, 2020
@swissspidy swissspidy removed the blocked Blocked by another task / decision label Apr 14, 2020
@swissspidy swissspidy added this to the 0.3.0 milestone Apr 14, 2020
@swissspidy
Copy link
Contributor

@kraftbj just an FYI that last week we decided to stick with wp-sitemap.xml.

Do you think we can add a test here? WP core itself doesn't test handle_404, so maybe it's not easily doable.

@kraftbj
Copy link
Contributor Author

kraftbj commented Apr 14, 2020

Will pick this back up and see what I can out together.

@swissspidy
Copy link
Contributor

Merging this one for now as it works as expected. Not critical to have a test for an HTTP redirect, but could always be added in a new PR.

@swissspidy swissspidy merged commit e545af8 into master Apr 21, 2020
@swissspidy swissspidy deleted the feature/144-sitemap.xml-redirect branch April 21, 2020 12:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Signed the Google CLA Type: Feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle 404 requests to sitemap.xml
5 participants