Skip to content

Conversation

@leogermani
Copy link
Contributor

@leogermani leogermani commented Jul 29, 2020

Changes the call to fetchSubscriberToken to use the blog token instead of primary user token, as it is not needed

Jetpack product discussion

p9dueE-1Dv-p2#comment-2838

Does this pull request change what data or activity we track or use?

No

Testing instructions:

Test it first on the master branch:

  • Set up a connected site
  • Activate Subscriptions module
  • Make sure you have at least one subscriber
  • Add the "Blog Subscriptions (Jetpack)" widget to your site
  • Visit the site and check the number of subscribers being displayed. "Join XX other subscribers"

Now checkout this branch

  • clear the cache: wp transient delete wpcom_subscribers_total
  • Visit the site again and confirm that you see the same number of subscribers

Proposed changelog entry for your changes:

  • Call to fetchSubscriberToken now uses the blog token

@leogermani leogermani added the [Status] Needs Review This PR is ready for review. label Jul 29, 2020
@leogermani leogermani added this to the 8.9 milestone Jul 29, 2020
@leogermani leogermani self-assigned this Jul 29, 2020
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello leogermani! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D47196-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@jetpackbot
Copy link
Collaborator

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.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16635

Scheduled Jetpack release: August 4, 2020.
Scheduled code freeze: July 28, 2020

Generated by 🚫 dangerJS against 346ee46

@leogermani leogermani changed the title Sse blog token to make the request to fetchSubscriberToken Use blog token to make the request to fetchSubscriberToken Jul 29, 2020
@jeherve jeherve added [Feature] Subscriptions All subscription-related things such as paid and unpaid, user management, and newsletter settings. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Jul 31, 2020
@leogermani leogermani changed the title Use blog token to make the request to fetchSubscriberToken Use blog token to make the request to fetchSubscriberCount Aug 4, 2020
Copy link
Member

@kbrown9 kbrown9 left a comment

Choose a reason for hiding this comment

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

I tested this using the provided testing instructions, and it worked for me!

@kbrown9 kbrown9 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 Aug 4, 2020
@leogermani leogermani merged commit c95e5fd into master Aug 13, 2020
@leogermani leogermani deleted the update/fetch-subscriber-count-use-blog-token branch August 13, 2020 18:25
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 13, 2020
davidlonjon added a commit that referenced this pull request Aug 14, 2020
* master: (41 commits)
  use blog token to make the request (#16635)
  External Media: Add account disconnect button (#16759)
  CI: Try collect js coverage (#16786)
  Sync: Fix nonce action string in theme edit sync (#16702)
  Connect-in-place: hide new heading during connection process (#16703)
  Update dependency eslint-plugin-jsdoc to v30.2.1 (#16765)
  Theme Tools: Resolve PHP 7.4 array offset notice. (#16795)
  New shell command for easier access to the database. (#16761)
  My Plan: Add Offer Reset project new plans (Jetpack Security, Jetpack Complete) (#16739)
  Increase the `editor.MediaUpload` hook priority (#16669)
  External Media: Remove `speak` announcement when inserting media.
  Extensions: make `render_callback` optional when checking block registration against plan (#16746)
  Conditional check for wrapper before giving focus to new page (#16817)
  Docker: Add package testing shortcut (#16810)
  Settings: Recognize valid Akismet keys from wp-config and restrict input (#16542)
  Social Previews: Add Modal (#16704)
  Update dependency preact to v10.4.7 (#16768)
  Improve a11y of amp-social-share (#16737)
  Instant Search: Tweak expanded result path styling (#16762)
  Docker: Add phpmyadmin to the docker-composer.yml (#16806)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Subscriptions All subscription-related things such as paid and unpaid, user management, and newsletter settings. Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants