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

Keyring: register Sharing menu page if it has not been registered #13799

Merged
merged 2 commits into from
Oct 28, 2019
Merged

Keyring: register Sharing menu page if it has not been registered #13799

merged 2 commits into from
Oct 28, 2019

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Oct 22, 2019

Fixes #13732

Changes proposed in this Pull Request:

  • When neither Publicize, Sharing, Likes, or Comment Likes are active, nothing in Jetpack registers the Sharing menu that the Keyring service relies on.
    This checks if the menu exists and registers it (although without UI since we don't need one for Keyring services that are not Publicize)

Testing instructions:

  • With this patch on, disable Publicize, Sharing, Likes, and Comment Likes on your site.
  • Go to Jetpack > Settings > Traffic
  • Try to automatically verify your site with Google.
  • The pop up that opens should lead you to Google.

Proposed changelog entry for your changes:

  • Site Verification Tools: ensure that you can connect your site to Google Search Console even when Publicize is disabled.

@jeherve
Keyring: register Sharing menu page if it has not been registered
2ec9737 
Fixes #13732

When neither Publicize, Sharing, Likes, or Comment Likes are active, nothing in Jetpack registers the Sharing menu that the Keyring service relies on.
This checks if the menu exists and registers it (although without UI since we don't need one for Keyring services that are not Publicize)
@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Verification tools [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Oct 22, 2019
@jeherve jeherve added this to the 7.9 milestone Oct 22, 2019
@jeherve jeherve requested a review from a team October 22, 2019 11:30
@jeherve jeherve self-assigned this Oct 22, 2019
@jetpackbot
Copy link

jetpackbot commented Oct 22, 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: November 5, 2019.
Scheduled code freeze: October 29, 2019

Generated by 🚫 dangerJS against ac9dfc4

@ChaosExAnima
Copy link
Contributor

Tried to test but got a timeout from the WPcom API:

Screenshot from 2019-10-23 15-27-35

Could be due to lagging on my localhost?

@jeherve
Copy link
Member Author

jeherve commented Oct 24, 2019

Could be due to lagging on my localhost?

Were you proxied, or did you have public-api.wordpress.com sandboxed? It may explain why you could not reach it.

@ChaosExAnima
Copy link
Contributor

Were you proxied, or did you have public-api.wordpress.com sandboxed? It may explain why you could not reach it.

I was proxied for sure, I double checked. I can debug my local JP installation when I get a sec.

@kraftbj kraftbj added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Oct 24, 2019
@jeherve jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Oct 25, 2019
kraftbj
kraftbj approved these changes Oct 25, 2019
View reviewed changes
Copy link
Contributor

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

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

Tested via JN and worked as advertised.

@kraftbj kraftbj 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 Oct 25, 2019
@jeherve jeherve merged commit 52165ac into master Oct 28, 2019
@jeherve jeherve deleted the fix/missing-sharing-webmaster branch October 28, 2019 12:45
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 28, 2019
@gravityrail
Copy link
Contributor

Thank you so much for fixing this!

jeherve added a commit that referenced this pull request Oct 28, 2019
@jeherve
Changelog: add #13799
114bbc8
jeherve added a commit that referenced this pull request Oct 29, 2019
@jeherve
7.9: Changelog (#13608)
6cd1621 
* 7.9: Changelog

* Update version number

* Update stable tag and tested up to

* Changelog: add #13530

* changelog: add #13578

* Changelog: add #13598

* Changelog: add entry for numerous block preview changes

* Changelog: add #13599

* changelog: add #13541

* Changelog: add #13542

* Changelog: add #13331

* Changelog: add #13558

* Changelog: add #13409

* Changelog: add #13582

* Changelog: add #13600

* Changelog: add #13601

* Changelog: add #13595

* Changelog: add #12695

* Changelog: add #13009

* Changelog: add #13649

* Changelog: add #13450

* Changelog: add #13507

* Changelog: add #13658

* Changelog: add #13687

* changelog: add #13683

* Changelog: add #9323

* Changelog: add #13681

* Fix typos in readme

* Add link to WordPress Beta Tester plugin

* Changelog: add #13630

* Changelog: add #13695

* Changelog: add #13659

* Changelog: add #13716

* Changelog: add #13664

* Changelog: add #13682

* Changelog: add #13362

* Changelog: add #13563

* Add testing list for #13563

* Changelog: add #13735

* Changelog: add #13752

* Changelog: add #13624

* Changelog: add #13756

* Changelog: add #13745

* Changelog: add #13728

* Changelog: add #13779

* Changelog: add #13699

* Changelog: add #13804

* Changelog: add #13761

* Changelog: add #13637

* Changelog: add #13517

* Changelog: add #13521

* Changelog: add #13729

* Testing list: add testing instructions for #13729

* Changelog: add sync changes

* Changelog: add #13807

* Changelog: add #13654

* Changelog: add #13795

* Changelog: add #13801

* Changelog: add #13818

* Changelog: add #13725

* Changelog: add #13831

* Changelog: add #13516

* Testing list: add Twenty Twenty instructions

* Changelog: add #13799

* Changelog: add #13805

* Changelog: add #13688

* Changelog: add #13830
@mmtr mmtr mentioned this pull request Feb 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kraftbj kraftbj kraftbj approved these changes

Assignees

@jeherve jeherve

Labels
[Feature] Verification tools [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
7.9
Development

Successfully merging this pull request may close these issues.

Verifying site using Google doesn't work unless Publicize is enabled
6 participants
@jeherve @jetpackbot @ChaosExAnima @gravityrail @kraftbj @matticbot