Skip to content

Post Images: Return attachment info when using from_thumbnail#11451

Merged
kraftbj merged 3 commits intomasterfrom
add/post-image-from-attachement-post-type
Mar 5, 2019
Merged

Post Images: Return attachment info when using from_thumbnail#11451
kraftbj merged 3 commits intomasterfrom
add/post-image-from-attachement-post-type

Conversation

@kraftbj
Copy link
Copy Markdown
Contributor

@kraftbj kraftbj commented Mar 1, 2019 •
edited
Loading

The Jetpack_PostImages class returns no post image for an attachment post type. We should return the image.

I attached it onto the from_thumbnail group since the attachment's "featured image" is itself, which made more sense to me than the other groups, not to mention matches the intent (the from_html and from_attachment is kinda weird and, personally, something I would often opt out of using).

Fixes #11413

Changes proposed in this Pull Request:

  • If the post we're requesting is an attachment, return the attachment's src instead of an empty string.

Testing instructions:

  • Upload an image and note the post_id.
  • Run Jetpack_PostImages::get_image( [the id] ); via wp shell
  • See empty string returned.
  • Apply patch.
  • Run command again and see an array containing image information.

Proposed changelog entry for your changes:

  • Jetpack Post Images: Provide the image itself when requesting an attachment's post image.

@kraftbj kraftbj added Bug When a feature is broken and / or not performing as intended General [Status] Needs Review This PR is ready for review. labels Mar 1, 2019
@kraftbj kraftbj added this to the 7.2 milestone Mar 1, 2019
@kraftbj kraftbj requested a review from a team March 1, 2019 17:13
@matticbot
Copy link
Copy Markdown
Contributor

matticbot commented Mar 1, 2019

Caution: This PR has changes that must be merged to WordPress.com
Hello kraftbj! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D25030-code before merging this PR. Thank you!

@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Mar 1, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: April 2, 2019.
Scheduled code freeze: March 26, 2019

Generated by 🚫 dangerJS against b70947c

jeherve
jeherve requested changes Mar 4, 2019
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.

Nice catch! I just have a minor nitpick.

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Mar 4, 2019
@jeherve @kraftbj
I need my space! (Fix a spacing issue)
b70947c 
Co-Authored-By: kraftbj <public@brandonkraft.com>
@matticbot
Copy link
Copy Markdown
Contributor

matticbot commented Mar 5, 2019

kraftbj, Your synced wpcom patch D25030-code has been updated.

@kraftbj kraftbj added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 5, 2019
@kraftbj kraftbj requested a review from jeherve March 5, 2019 13:07
@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 Mar 5, 2019
jeherve
jeherve approved these changes Mar 5, 2019
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.

Looking good. 🚢

@kraftbj kraftbj merged commit 140ce31 into master Mar 5, 2019
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 5, 2019
@kraftbj kraftbj deleted the add/post-image-from-attachement-post-type branch March 5, 2019 17:33
@kraftbj
Copy link
Copy Markdown
Contributor Author

kraftbj commented Mar 5, 2019

Deployed to wpcom via r188460-wpcom

kraftbj added a commit that referenced this pull request Mar 25, 2019
@kraftbj
Add CL for #11451
5f45bf1
kraftbj added a commit that referenced this pull request Mar 25, 2019
@kraftbj
Add testing instructions for #11451
a5410fe
kraftbj added a commit that referenced this pull request Mar 27, 2019
@kraftbj
Add initial 7.2 changelog to master (#11690)
2227bfd 
* Initial Changelog for 7.2

* Testing list: add mention of IE11 testing

* Initial Changelog for 7.2

* Testing list: add mention of IE11 testing

* Add CL for #11224

* Add CL for #11426

* Add CL for #11442

* Add testing instructions for #11224

* Add CL for #11451

* Reclassify CL item

* Add testing instructions for #11451

* Add CL for #11486

* Add CL for #11418

* Add CL for #11524

* Add CL and testing instructions for #11449

* Add CL for #11460

* Add CL for #11520 and #11582

* Add CL for #11531

* Add CL #11644

* Add testing instructions for #11644

* Add testing instructions for #11644

* Add CL for #11618

* Uniform changelog lines

* CL #11679

* CL #11661

* CL #11654

* CL #11645

* CL #11643

* CL #11636

* CL #11635 and for other PHPCS commits

* CL #11627

* CL #11626

* CL #11598

* CL #11596

* Remove nested items for shortcopy. I don't believe the detailed list is helpful

* CL #11570

* CL #11569

* CL #11560

* CL #11558

* CL #11555

* CL #6704

* CL #11298

* CL #11324

* CL #11443

* CL #11484

* CL #11516

* CL #11529

* Expand Ads block enhancement CL item
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

Assignees

No one assigned

Labels

Bug When a feature is broken and / or not performing as intended General Touches WP.com Files

Projects

None yet

Milestone

7.2

Development

Successfully merging this pull request may close these issues.

4 participants

@kraftbj @matticbot @jetpackbot @jeherve