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

Connection: Redirect plugins and dashboard connect buttons to the main flow #13599

Merged
merged 5 commits into from
Sep 30, 2019

Conversation

tyxla
Copy link
Member

@tyxla tyxla commented Sep 30, 2019

Currently, the main connection flow starting from /wp-admin/admin.php?page=jetpack#/ has the connect-in-place flow, but the buttons starting from the WP-admin dashboard (/wp-admin/index.php) and plugins screen (/wp-admin/plugins.php) still work the old way.

This PR updates these buttons so they respect the constant and A/B test, and based on them, redirect to either the original connection flow or the connect-in-place flow.

Changes proposed in this Pull Request:

  • Update dashboard and plugins screens connection buttons:
    • Respect the JETPACK_SHOULD_USE_CONNECTION_IFRAME constant.
    • Respect the jetpack_connect_in_place_v2 A/B test.
    • If connect-in-place flow is enabled through the constant or the A/B test, redirect to it and start the flow.
    • If connect-in-place flow is not enabled, the buttons would redirect to the WP.com connection flow, like they used to do before.

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

  • Part of p1HpG7-7nj-p2

Testing instructions:

  • Checkout this branch.
  • Build Jetpack npm build.
  • Follow the instructions below in the following order. I've separated them in sections for clarity.
  • Test the connect in place flow from the Jetpack dashboard
    • Disconnect Jetpack if it was connected.
    • Add this to your wp-config.php: define( 'JETPACK_SHOULD_USE_CONNECTION_IFRAME', true );
    • Go to /wp-admin/admin.php?page=jetpack#/
    • Click the "Set up Jetpack" button.
    • Verify it works the same way as it does on master.
    • Verify you're still able to connect like before.
    • Disconnect Jetpack.
  • Test the connect in place flow from the WP admin dashboard
    • Go to /wp-admin/index.php.
    • Click the "Set up Jetpack" button.
    • Verify it redirects to /wp-admin/admin.php?page=jetpack#/setup
    • Verify you see the loading screen, and after a little, you see either the "Approve" button, or the "Connect with WordPress.com" button.
    • Verify you're still able to connect like before.
    • Disconnect Jetpack.
  • Test the connect in place flow from the plugins screen
    • Go to /wp-admin/plugins.php.
    • Click the "Set up Jetpack" button.
    • Verify it redirects to /wp-admin/admin.php?page=jetpack#/setup
    • Verify you see the loading screen, and after a little, you see either the "Approve" button, or the "Connect with WordPress.com" button.
    • Verify you're still able to connect like before.
    • Disconnect Jetpack.
  • Test the original connection flow from the Jetpack dashboard
    • Remove this from your wp-config.php: define( 'JETPACK_SHOULD_USE_CONNECTION_IFRAME', true );
    • Go to /wp-admin/admin.php?page=jetpack#/
    • Open your Network requests tab.
    • Click the "Set up Jetpack" button.
    • Verify that this performs a request to the A/B test endpoint.
    • Verify you're redirected to the WP.com Jetpack Connect authorization flow.
    • Verify you're still able to connect like before.
    • Disconnect Jetpack.
  • Test the original connection flow from the WP admin dashboard
    • Go to /wp-admin/index.php.
    • Open your Network requests tab.
    • Click the "Set up Jetpack" button.
    • Verify that this performs a request to the A/B test endpoint.
    • Verify you're redirected to the WP.com Jetpack Connect authorization flow.
    • Verify you're still able to connect like before.
    • Disconnect Jetpack.
  • Test the original connection flow from the plugins screen
    • Go to /wp-admin/plugins.php.
    • Open your Network requests tab.
    • Click the "Set up Jetpack" button.
    • Verify that this performs a request to the A/B test endpoint.
    • Verify you're redirected to the WP.com Jetpack Connect authorization flow.
    • Verify you're still able to connect like before.
  • If you got to this point, I wish to thank you for the thorough testing ❤️💪

Proposed changelog entry for your changes:

  • Connection: Redirect plugins and dashboard connect buttons to the main flow

@tyxla tyxla added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Pri] High Connect Flow Connection banners, buttons, ... labels Sep 30, 2019
@tyxla tyxla added this to the 7.9 milestone Sep 30, 2019
@tyxla tyxla requested review from a team September 30, 2019 12:07
@tyxla tyxla self-assigned this Sep 30, 2019
@jetpackbot
Copy link

jetpackbot commented Sep 30, 2019

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 22063d8

@enejb
Copy link
Member

enejb commented Sep 30, 2019

I went though and tested the flows and they worked as expected. Thanks for fixing all the minor things I found a long the way.
This is ready to ship :shipit:

Copy link
Member

@enejb enejb left a comment

Choose a reason for hiding this comment

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

Nicely done.

@enejb enejb added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Sep 30, 2019
@tyxla
Copy link
Member Author

tyxla commented Sep 30, 2019

Thanks for the review and for testing it Enej 🙌

@tyxla tyxla merged commit b6704e7 into master Sep 30, 2019
@tyxla tyxla deleted the update/conn-alt-buttons-redirect-to-cip branch September 30, 2019 14:06
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 30, 2019
jeherve added a commit that referenced this pull request Oct 23, 2019
jeherve added a commit that referenced this pull request Oct 29, 2019
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Connect Flow Connection banners, buttons, ... [Pri] High [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.

5 participants