Skip to content

SAL: Move icon logic to Jetpack base, leverage core site icon option consistently#6025

Merged
dereksmart merged 3 commits intomasterfrom
update/r148273-api-sal-icon
Jan 11, 2017
Merged

SAL: Move icon logic to Jetpack base, leverage core site icon option consistently#6025
dereksmart merged 3 commits intomasterfrom
update/r148273-api-sal-icon

Conversation

@aduth
Copy link
Copy Markdown
Member

@aduth aduth commented Jan 2, 2017

Changes proposed in this Pull Request:

The Jetpack Site Icon module was deprecated in 3.6.1. The transition logic deactivates the module for most sites, so the jetpack_site_icon_url function usually does not pass the existence check for most Jetpack sites.

In any case, we plan toward moving to treating Site Icon as the canonical source of site's icon. The changes herein check for this option, falling back to the Jetpack-defined icon option if unset.

This improves consistency between Jetpack shadow sites and the remote site. Previously, /me/sites (shadow site) will show the icon property for my Jetpack site, whereas /sites/%s does not (because the module is not active on the site).

Testing instructions:

With changes applied to your Jetpack site, ensure that an icon property is consistently returned for Jetpack sites between /me/sites and /sites/%s. Authenticated requests can be issued using the Developer Site console to verify presence of icon.media_id when user has REST API media editing capabilities.

Related:

  • D2940-code
  • r148273-wpcom

@dereksmart
Copy link
Copy Markdown
Contributor

Tests well, thanks aduth

@dereksmart dereksmart 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 Jan 11, 2017
@aduth aduth added [Status] In Progress and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jan 11, 2017
@aduth
Copy link
Copy Markdown
Member Author

aduth commented Jan 11, 2017

Pushed 803e119 to improve logic in omitting possible falsey icons.

Reference:

@aduth aduth added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Jan 11, 2017
@dereksmart
Copy link
Copy Markdown
Contributor

👌 latest change.

@dereksmart dereksmart 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 Jan 11, 2017
@dereksmart dereksmart merged commit e4ceb53 into master Jan 11, 2017
@dereksmart
Copy link
Copy Markdown
Contributor

dereksmart commented Jan 11, 2017

Merged to 4.5 943fb39 a0e92cd aa5c07a

@dereksmart dereksmart deleted the update/r148273-api-sal-icon branch January 11, 2017 20:40
dereksmart pushed a commit that referenced this pull request Jan 17, 2017
CHangelog: add #5457

Changelog: add #5487

Changelog: add #5708

Changelog: add #5879

Changelog: add #5932

Changelog: add #5963

Changelog: add #5968

Changelog: add #5996

Changelog: add #5998

Changelog: add #5999

Changelog: add #6012

Changelog: add #6013

Changelog: add #6014

Changelog: add #6015

Changelog: add #6023

Changelog: add #6024

Changelog: add #6030

Changelog: add #5465

CHangelog: add #6063

Changelog: add #6025

Changelog: add #5974

Changelog: add #6059

Changelog: add #6046

Changelog: add #5418

Changelog: move things around and add missing information.

Changelog: add #5565

Changelog: add #6087

Changelog: add #6095
dereksmart pushed a commit that referenced this pull request Jan 17, 2017
Changelog: add #5867

Changelog: add #5874

Changelog: add #5905

Changelog: add #5906

Changelog: add #5931

Changelog: add #5933

Changelog: add #5934

Bring over 4.4.2 changelog from branch-4.4

@see 18012a3

Changelog: add #5976, #5978, #5983

Changelog: add #5917

Changelog: add #5832

Changelog: add 4.4.2 release post link.

CHangelog: add #5457

Changelog: add #5487

Changelog: add #5708

Changelog: add #5879

Changelog: add #5932

Changelog: add #5963

Changelog: add #5968

Changelog: add #5996

Changelog: add #5998

Changelog: add #5999

Changelog: add #6012

Changelog: add #6013

Changelog: add #6014

Changelog: add #6015

Changelog: add #6023

Changelog: add #6024

Changelog: add #6030

Changelog: add #5465

CHangelog: add #6063

Changelog: add #6025

Changelog: add #5974

Changelog: add #6059

Changelog: add #6046

Changelog: add #5418

Changelog: move things around and add missing information.

Changelog: add #5565

Changelog: add #6087

Changelog: add #6095

Readme: add @tyxla to the list of contributors.

Improved changelog for your readability and enjoyment

updated the release date

finalizing the changelog with a few more edits
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants