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.

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

Reduce Jetpack Admin page queries for connected users. #13464

Merged
merged 1 commit into from
Sep 19, 2019

Conversation

dereksmart
Copy link
Member

@dereksmart dereksmart commented Sep 14, 2019

Changes proposed in this Pull Request:

Related to #13463, we are unnecessarily passing the build_connect_url value on every page load of the Jetpack admin. The historical reason for this, is that we used to not reload the page after disconnecting. However, since the new full-screen modal required a page load after disconnect, we can be smarter about when we use the resources to build the url only when we need it.

It prevents about 6-7 unnecessary queries when the current user is connected.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • Faster Jetpack admin page load?

Testing instructions:

Check out query monitor or something similar. Note the queries for a connected user on page load of the main Jetpack Admin dashboard.

Check it out with the patch applied as a connected user. There should be way fewer queries.

Functional testing:
0. From the main Jetpack dashboard:

  • Connect the site to wordpress.com. There should be no problem.
  • Disconnect, and then reconnect. There should be no problem.
  • Create a secondary user while the site is connected.
  • Visit the Jetpack admin page, and click to "create account" as that secondary user. There should be no problem.

Proposed changelog entry for your changes:

  • Faster Jetpack admin page load?

@dereksmart dereksmart added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. Admin Page React-powered dashboard under the Jetpack menu [Type] Janitorial labels Sep 14, 2019
@dereksmart dereksmart added this to the 7.8 milestone Sep 14, 2019
@dereksmart dereksmart requested a review from a team September 14, 2019 14:38
@jetpackbot
Copy link

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: October 1, 2019.
Scheduled code freeze: September 24, 2019

Generated by 🚫 dangerJS against 16968a6

Copy link
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! Merging.

@jeherve jeherve 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 Sep 19, 2019
@jeherve jeherve merged commit 782b203 into master Sep 19, 2019
@jeherve jeherve deleted the update/admin-initial-state-build-connect-url branch September 19, 2019 10:35
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 19, 2019
jeherve added a commit that referenced this pull request Sep 20, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin Page React-powered dashboard under the Jetpack menu [Focus] Performance [Type] Janitorial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants