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

Hides jetpack during onboarding if already connected #2516

12 changes: 7 additions & 5 deletions js/src/setup-mc/setup-stepper/setup-accounts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,16 @@ const SetupAccounts = ( props ) => {
* and the whole page would display this AppSpinner, which is not desired.
*/
const isLoadingJetpack = ! jetpack;
const isLoadingGoogle = jetpack?.active === 'yes' && ! google;
const isJetpackActive = jetpack?.active === 'yes';

const isLoadingGoogle = isJetpackActive && ! google;
const isLoadingGoogleMCAccount =
google?.active === 'yes' && scope.gmcRequired && ! googleMCAccount;

if ( isLoadingJetpack || isLoadingGoogle || isLoadingGoogleMCAccount ) {
return <AppSpinner />;
}

const isGoogleAccountDisabled = jetpack?.active !== 'yes';
Copy link
Member

Choose a reason for hiding this comment

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

Nice improvement! Thanks!


// Ads is ready when we have a connection and verified and verified access.
// Billing is not required, and the 'link_merchant' step will be resolved
// when the MC the account is connected.
Expand Down Expand Up @@ -155,8 +155,10 @@ const SetupAccounts = ( props ) => {
) }
>
<VerticalGapLayout size="large">
<WPComAccountCard jetpack={ jetpack } />
<GoogleAccountCard disabled={ isGoogleAccountDisabled } />
{ ! isJetpackActive && (
<WPComAccountCard jetpack={ jetpack } />
) }
<GoogleAccountCard disabled={ ! isJetpackActive } />
<GoogleAdsAccountCard />
</VerticalGapLayout>
</Section>
Expand Down
48 changes: 36 additions & 12 deletions tests/e2e/specs/setup-mc/step-1-accounts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ test.describe( 'Set up accounts', () => {
await setUpAccountsPage.closePage();
} );

test( 'should see accounts step header, "Connect your WordPress.com account" & connect button', async () => {
test( 'JetpackDisconnected: should see accounts step header, "Connect your WordPress.com account" & connect button', async () => {
await setUpAccountsPage.goto();

await expect(
Expand Down Expand Up @@ -116,7 +116,7 @@ test.describe( 'Set up accounts', () => {
} );
} );

test.describe( 'Connect Google account', () => {
test.describe( 'Connected WordPress.com account', async () => {
test.beforeAll( async () => {
// Mock Jetpack as connected
await setUpAccountsPage.mockJetpackConnected(
Expand All @@ -133,14 +133,44 @@ test.describe( 'Set up accounts', () => {
await setUpAccountsPage.goto();
} );

test( 'should see their WPORG email, "Google" title & connect button', async () => {
const jetpackDescriptionRow =
setUpAccountsPage.getJetpackDescriptionRow();
test( 'should not show the WP.org connection card when already connected', async () => {
await expect(
page.getByRole( 'heading', { name: 'Set up your accounts' } )
).toBeVisible();

await expect(
page.getByText(
'Connect the accounts required to use Google for WooCommerce.'
)
).toBeVisible();

await expect(
page.getByRole( 'button', { name: 'Connect' } ).first()
).toBeEnabled();
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove this assertion once the once the previous suggestion is updated.

Suggested change
await expect(
page.getByRole( 'button', { name: 'Connect' } ).first()
).toBeEnabled();


const wpAccountCard = setUpAccountsPage.getWPAccountCard();
await expect( wpAccountCard ).not.toBeVisible();
} );
} );

await expect( jetpackDescriptionRow ).toContainText(
test.describe( 'Connect Google account', () => {
test.beforeAll( async () => {
// Mock Jetpack as connected
await setUpAccountsPage.mockJetpackConnected(
'Test user',
'[email protected]'
);

// Mock google as not connected.
// When pending even WPORG will not render yet.
// If not mocked will fail and render nothing,
// as Jetpack is mocked only on the client-side.
await setUpAccountsPage.mockGoogleNotConnected();

await setUpAccountsPage.goto();
} );

test( 'should see their WPORG email, "Google" title & connect button', async () => {
const googleAccountCard = setUpAccountsPage.getGoogleAccountCard();

await expect(
Expand Down Expand Up @@ -393,12 +423,6 @@ test.describe( 'Set up accounts', () => {
} );

test( 'should see their WPORG email, Google email, "Google Merchant Center" title & "Create account" button', async () => {
const jetpackDescriptionRow =
setUpAccountsPage.getJetpackDescriptionRow();
await expect( jetpackDescriptionRow ).toContainText(
'[email protected]'
);

const googleDescriptionRow =
setUpAccountsPage.getGoogleDescriptionRow();
await expect( googleDescriptionRow ).toContainText(
Expand Down
100 changes: 40 additions & 60 deletions tests/e2e/utils/pages/setup-mc/step-1-set-up-accounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,31 +69,15 @@ export default class SetUpAccountsPage extends MockRequests {
return button.locator( ':scope.is-primary' );
}

/**
* Get .gla-account-card__title class.
*
* @return {import('@playwright/test').Locator} Get .gla-account-card__title class.
*/
getCardTitleClass() {
return this.page.locator( '.gla-account-card__title' );
}

/**
* Get .gla-account-card__description class.
*
* @return {import('@playwright/test').Locator} Get .gla-account-card__description class.
*/
getCardDescriptionClass() {
return this.page.locator( '.gla-account-card__description' );
}

/**
* Get Jetpack description row.
*
* @return {import('@playwright/test').Locator} Get Jetpack description row.
*/
getJetpackDescriptionRow() {
return this.getCardDescriptionClass().first();
return this.getWPAccountCard().locator(
'.gla-account-card__description'
);
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -102,7 +86,9 @@ export default class SetUpAccountsPage extends MockRequests {
* @return {import('@playwright/test').Locator} Get Google description row.
*/
getGoogleDescriptionRow() {
return this.getCardDescriptionClass().nth( 1 );
return this.getGoogleAccountCard().locator(
'.gla-account-card__description'
);
}

/**
Expand All @@ -111,7 +97,9 @@ export default class SetUpAccountsPage extends MockRequests {
* @return {import('@playwright/test').Locator} Get Merchant Center description row.
*/
getMCDescriptionRow() {
return this.getCardDescriptionClass().nth( 3 );
return this.getMCAccountCard().locator(
'.gla-account-card__description'
);
}

/**
Expand All @@ -120,7 +108,9 @@ export default class SetUpAccountsPage extends MockRequests {
* @return {import('@playwright/test').Locator} Get Google Ads title.
*/
getAdsTitleRow() {
return this.getCardTitleClass().nth( 2 );
return this.getGoogleAdsAccountCard().locator(
'.gla-account-card__title'
);
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -129,7 +119,7 @@ export default class SetUpAccountsPage extends MockRequests {
* @return {import('@playwright/test').Locator} Get Google Merchant Center title.
*/
getMCTitleRow() {
return this.getCardTitleClass().nth( 3 );
return this.getMCAccountCard().locator( '.gla-account-card__title' );
}

/**
Expand Down Expand Up @@ -177,22 +167,13 @@ export default class SetUpAccountsPage extends MockRequests {
return this.getModal().locator( 'button.is-secondary' );
}

/**
* Get .gla-connected-icon-label class.
*
* @return {import('@playwright/test').Locator} Get .gla-connected-icon-label class.
*/
getConnectedLabelClass() {
return this.page.locator( '.gla-connected-icon-label' );
}

/**
* Get Jetpack connected label.
*
* @return {import('@playwright/test').Locator} Get Jetpack connected label.
*/
getJetpackConnectedLabel() {
return this.getConnectedLabelClass().first();
return this.getWPAccountCard().locator( '.gla-connected-icon-label' );
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -201,7 +182,9 @@ export default class SetUpAccountsPage extends MockRequests {
* @return {import('@playwright/test').Locator} Get Google connected label.
*/
getGoogleConnectedLabel() {
return this.getConnectedLabelClass().nth( 1 );
return this.getGoogleAccountCard().locator(
'.gla-connected-icon-label'
);
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -210,7 +193,7 @@ export default class SetUpAccountsPage extends MockRequests {
* @return {import('@playwright/test').Locator} Get Merchant Center connected label.
*/
getMCConnectedLabel() {
return this.getConnectedLabelClass().nth( 2 );
return this.getMCAccountCard().locator( '.gla-connected-icon-label' );
}

/**
Expand Down Expand Up @@ -246,31 +229,15 @@ export default class SetUpAccountsPage extends MockRequests {
return this.page.locator( 'input#inspector-input-control-0' );
}

/**
* Get sub section title row.
*
* @return {import('@playwright/test').Locator} Get sub section title row.
*/
getSubSectionTitleRow() {
return this.page.locator( '.wcdl-subsection-title' );
}

/**
* Get section footer row.
*
* @return {import('@playwright/test').Locator} Get section footer row.
*/
getSectionFooterRow() {
return this.page.locator( '.wcdl-section-card-footer' );
}

/**
* Get select existing Merchant Center account title.
*
* @return {import('@playwright/test').Locator} Get select existing Merchant Center account title.
*/
getSelectExistingMCAccountTitle() {
return this.getSubSectionTitleRow().nth( 4 );
return this.getMCAccountCard()
.locator( '.wcdl-subsection-title' )
.nth( 1 );
}

/**
Expand All @@ -297,10 +264,11 @@ export default class SetUpAccountsPage extends MockRequests {
/**
* Get account cards.
*
* @param {Object} options
* @return {import('@playwright/test').Locator} Get account cards.
*/
getAccountCards() {
return this.page.locator( '.gla-account-card' );
getAccountCards( options = {} ) {
return this.page.locator( '.gla-account-card', options );
}

/**
Expand All @@ -309,7 +277,7 @@ export default class SetUpAccountsPage extends MockRequests {
* @return {import('@playwright/test').Locator} Get WordPress account card.
*/
getWPAccountCard() {
return this.getAccountCards().first();
return this.getAccountCards( { hasText: 'WordPress.com' } ).first();
}

/**
Expand All @@ -318,7 +286,11 @@ export default class SetUpAccountsPage extends MockRequests {
* @return {import('@playwright/test').Locator} Get Google account card.
*/
getGoogleAccountCard() {
return this.getAccountCards().nth( 1 );
return this.getAccountCards( {
has: this.page.locator( '.gla-account-card__title', {
hasText: 'Google',
} ),
} ).first();
}

/**
Expand All @@ -327,7 +299,11 @@ export default class SetUpAccountsPage extends MockRequests {
* @return {import('@playwright/test').Locator} Get Google Ads account card.
*/
getGoogleAdsAccountCard() {
return this.getAccountCards().nth( 2 );
return this.getAccountCards( {
has: this.page.locator( '.gla-account-card__title', {
hasText: 'Google Ads',
} ),
} ).first();
}

/**
Expand All @@ -336,7 +312,11 @@ export default class SetUpAccountsPage extends MockRequests {
* @return {import('@playwright/test').Locator} Get Merchant Center account card.
*/
getMCAccountCard() {
return this.getAccountCards().nth( 3 );
return this.getAccountCards( {
has: this.page.locator( '.gla-account-card__title', {
hasText: 'Google Merchant Center',
} ),
} ).first();
}

/**
Expand Down