Skip to content

Sitemaps: Do not include images not published#10194

Merged
jeherve merged 4 commits intomasterfrom
fix/sitemap-image-private-posts
Sep 24, 2018
Merged

Sitemaps: Do not include images not published#10194
jeherve merged 4 commits intomasterfrom
fix/sitemap-image-private-posts

Conversation

@kraftbj
Copy link
Copy Markdown
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 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
Copy Markdown
Contributor

matticbot commented Sep 21, 2018

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
Copy Markdown
Collaborator

jetpackbot commented Sep 21, 2018

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

Generated by 🚫 dangerJS

@kraftbj kraftbj added [Status] Needs Review This PR is ready for review. 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
Copy Markdown
Contributor

matticbot commented Sep 21, 2018

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 This PR is ready for review. labels Sep 24, 2018
jeherve
jeherve approved these changes Sep 24, 2018
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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

@jeherve jeherve jeherve approved these changes

+1 more reviewer

@oskosk oskosk oskosk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kraftbj kraftbj

Labels

Bug When a feature is broken and / or not performing as intended [Feature] Sitemaps Touches WP.com Files

Projects

None yet

Milestone

6.6

Development

Successfully merging this pull request may close these issues.

5 participants

@kraftbj @matticbot @jetpackbot @jeherve @oskosk