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.

Sign up for GitHub

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

Jump to bottom

Sitemaps: Do not include images not published #10194

Merged
merged 4 commits into from
Sep 24, 2018
Merged

Sitemaps: Do not include images not published #10194

merged 4 commits into from
Sep 24, 2018

Conversation

kraftbj
Copy link
Contributor

@kraftbj kraftbj commented Sep 21, 2018

Fixes #8138

Changes proposed in this Pull Request:

  • Before adding an image to the image sitemap, check the post status. When an attachment is checked for status, the following is returned:
  1. If the image is private, it will be returned as private. Not included in the sitemap.
  2. If the image is unattached, it is assumed published. Included in the sitemap.
  3. Otherwise, it will assume the parent status. Any status other than "publish" is not included in the sitemap.

Testing instructions:

  • Create a new draft post
  • Upload a new image to that post. Do not reuse an existing image.
  • Save as draft.
  • Let the sitemap regenerate. Check the image sitemap.
  • Notice the image is included and the link is the parent's permalink (?p=123).
  • This page will 404 if not logged in, as a draft.
  • Apply patch.
  • Try again. Image not included.

Proposed changelog entry for your changes:

  • No longer add images attached to non-published posts to the image sitemap.

@kraftbj kraftbj added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Sitemaps [Status] Needs Testing We need to add this change to the testing call for this month's release labels Sep 21, 2018
@kraftbj kraftbj added this to the 6.6 milestone Sep 21, 2018
@kraftbj kraftbj requested a review from a team as a code owner September 21, 2018 20:13
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Please review this diff before merging: D18698-code. (newly created revision)

@jetpackbot
Copy link

That's a great PR description, thank you so much for your effort!

Generated by 🚫 dangerJS

@kraftbj kraftbj added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Testing We need to add this change to the testing call for this month's release labels Sep 21, 2018
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Please review this diff before merging: D18698-code. (updated diff)

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 24, 2018
jeherve
jeherve approved these changes Sep 24, 2018
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Works well in my tests. 👍

oskosk
oskosk approved these changes Sep 24, 2018
View reviewed changes
Copy link
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

Tested Great. The image sitemap didn't even get generated when only images on a draft were on the site.

@jeherve jeherve merged commit 14eb2a5 into master Sep 24, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Sep 24, 2018
@jeherve jeherve deleted the fix/sitemap-image-private-posts branch September 24, 2018 19:38
jeherve added a commit that referenced this pull request Sep 25, 2018
@jeherve
changelog: add #10194
aaaece8
jeherve added a commit that referenced this pull request Sep 25, 2018
@jeherve
Changelog for 6.6 release (#10107)
581489a 
* Readme: add boilerplate for next release, 6.6

* Add 6.5 to the changelog.txt file

* Set boilerplate testing list for 6.6

* Readme: update stable tag to 6.5

* Add bullets to 6.5 changelog items

* Readme: add link to previous changelogs

This will help folks who want to know more about past releases,
while keeping the readme.txt short so as to not overwhelm translators and site owners only looking for information about the last release.

* Changelog: add information at the top of the changelog file.

* Changelog: add #10054

* Changelog: add #10078

* Changelog: add #10079

* Changelog: add #10064

* Changelog: add #10094

* Changelog: add #10096

* Testing list: add more information based on #10087

* Changelog: add #9847

* Changelog: add #10084

* Changelog: add #9918

* Changelog: add #7614

* Changelog: add #10116

* Changelog: add #10108

* Changelog: add #10041

* Changelog: add #10121

* Changelog: add #10134

* Changelog: add #10130

* Changelog: add #10109

* changelog: add #10137

* changelog: add #9952

* changelog: add #10120

* changelog: add #10162

* Changelog: add #10163

* Changelog: add #10092

* changelog: add #10156

* Changelog: add #10154

* changelog: add #10122

* Changelog: add #10101

* changelog: add #10105

* changelog: add #10190

* Changelog: add #10196

* changelog: add #10152

* Changelog: add #10153

* Testing list: add more details to Site Verification testing steps.

@see #10143 (comment)

* changelog: add #10194

* Changelog: add #10193
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oskosk oskosk oskosk approved these changes

@jeherve jeherve jeherve approved these changes

Assignees

@kraftbj kraftbj

Labels
[Feature] Sitemaps Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
6.6
Development

Successfully merging this pull request may close these issues.
5 participants
@kraftbj @matticbot @jetpackbot @jeherve @oskosk