Skip to content
This repository has been archived by the owner on Feb 18, 2021. It is now read-only.

Add Jetpack SSO connect flow #984

Merged
merged 11 commits into from
Mar 13, 2018
Merged

Add Jetpack SSO connect flow #984

merged 11 commits into from
Mar 13, 2018

Conversation

brbrr
Copy link
Contributor

@brbrr brbrr commented Feb 27, 2018

adds SSO connect flow as requested in #777

To test this specific flow:

  1. decrypt updated config. make sure subscriberUser is in your confg
  2. Set JETPACKHOST env var to PRESSABLE / JN: e.g export JETPACKHOST=PRESSABLE
  3. run the test: ./node_modules/.bin/mocha ./specs-jetpack-calypso/wp-jetpack-sso-connect-spec.js -g 'Connect via SSO'

@brbrr brbrr self-assigned this Feb 27, 2018
@brbrr brbrr requested review from seear and a team February 27, 2018 14:32
@seear
Copy link
Contributor

seear commented Feb 27, 2018

@brbrr I'm having trouble running this new spec locally. I'm set up using the all new instructions (great by the way!) and can run the jp connect spec ok using:
./node_modules/.bin/mocha specs-jetpack-calypso/wp-jetpack-connect-spec.js
and all those tests pass.

When I run:
./node_modules/.bin/mocha specs-jetpack-calypso/wp-jetpack-sso-connect-spec.js

I get:

1) Jetpack Connect: (desktop) Disconnect expired sites: @parallel @jetpack Can log in:
     Error: Account key 'jetpackUserWPCOM' not found in the configuration

@brbrr
Copy link
Contributor Author

brbrr commented Feb 28, 2018

@seear Ah, sorry. I've added testing steps, so with defined JETPACKHOST - it should be fine.

Copy link
Contributor

@seear seear left a comment

Choose a reason for hiding this comment

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

It's so great to have an e2e test for this flow, because it is one of the most fiddly to test manually and often gets missed out.

The tests pass for me. Code looks fine. Great work 👍

test.it( 'Can disconnect any expired sites', () => {
this.sidebarComponent = new SidebarComponent( driver );

const removeSites = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to factor this removeSites function out so that it can be shared with the jetpack connect spec. That can be for a future PR because this one is large already.

@brbrr
Copy link
Contributor Author

brbrr commented Mar 1, 2018

@Automattic/flowpatrol ready for review :)

@rachelmcr
Copy link
Member

I noticed that this spec is set up to always use Jurassic Ninja as the host, even when JETPACKHOST is set to a different host (e.g. Pressable). What do you think of writing the spec in a way that it can be run on Jetpack sites on different hosts?

@brbrr
Copy link
Contributor Author

brbrr commented Mar 7, 2018

Thanks for your suggestion @rachelmcr
I've updated the flow to re-use host specific site (with dataHelper.getJetpackSiteName()). To make it work - new WP.com account is created for every test. This spec did not clean up the list of subscribers (it may affect only PRESSABLE target)

@brbrr brbrr force-pushed the add/jetpack-sso-connect-flow branch from f2da534 to 5b79fd5 Compare March 12, 2018 08:37
@brbrr brbrr requested a review from alisterscott March 13, 2018 03:03
@alisterscott
Copy link
Contributor

Looks good and worked well for me:

 ./node_modules/.bin/mocha ./specs-jetpack-calypso/ -g 'Connect via SSO:'


  Jetpack Connect: (desktop)
    Connect via SSO: @parallel @jetpack
      ✓ Can register new Subscriber user (27810ms)
      ✓ Can log into WordPress.com (7130ms)
      ✓ Can log into site via Jetpack SSO (9686ms)
      ✓ Add new user as Subscriber in wp-admin (6209ms)
      ✓ Log out from WP Admin (4337ms)
      ✓ Can log in as Subscriber (6806ms)
      ✓ Can login via SSO into WP Admin (29769ms)


  7 passing (2m)

Copy link
Contributor

@alisterscott alisterscott left a comment

Choose a reason for hiding this comment

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

:shipit:

@brbrr brbrr deleted the add/jetpack-sso-connect-flow branch March 13, 2018 03:33
@alisterscott alisterscott restored the add/jetpack-sso-connect-flow branch March 13, 2018 23:25
@alisterscott alisterscott deleted the add/jetpack-sso-connect-flow branch March 29, 2018 06:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants