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.

Sign up for GitHub

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

Streamline onboarding: Hide WordPress.com account connection setting from the onboarding if already connected #2516

Merged
merged 9 commits into from
Aug 22, 2024
Merged

Streamline onboarding: Hide WordPress.com account connection setting from the onboarding if already connected #2516

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
64f51bf
Hides jetpack during onboarding if already connected
dsawardekar Aug 9, 2024
88e0799
Limits jetpack check to only the WPComAccount card
dsawardekar Aug 12, 2024
ad40c2c
Adds e2e tests for jetpack connected & disconnected states
dsawardekar Aug 12, 2024
e2f2420
Refactors variable names in setup flow
dsawardekar Aug 13, 2024
f1d84f6
Update e2e tests
joemcgill Aug 13, 2024
7523a05
Fixes linter warnings
dsawardekar Aug 14, 2024
9e606c1
Merge pull request #2521 from woocommerce/update/2487-hide-jetpack-e2…
dsawardekar Aug 14, 2024
223935e
Updates e2e tests per PR feedack & removes unused code
dsawardekar Aug 19, 2024
ecd25fc
Updates e2e tests per CR feedback
dsawardekar Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions js/src/setup-mc/setup-stepper/setup-accounts/index.js
View file
Open in desktop
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
24 changes: 23 additions & 1 deletion tests/e2e/specs/setup-mc/step-1-accounts.test.js
View file
Open in desktop
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 @@ -79,6 +79,28 @@ test.describe( 'Set up accounts', () => {
await expect( continueButton ).toBeDisabled();
} );

test( 'JetpackConnected: should verify that the Jetpack connect button is hidden if already connected', async () => {
await setUpAccountsPage.goto();

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();

const firstCard = setUpAccountsPage.getWPAccountCard();
await expect( firstCard ).toBeEnabled();
await expect( firstCard ).not.toContainText( 'WordPress.com' );
} );

test.describe( 'FAQ panels', () => {
test( 'should see two questions in FAQ', async () => {
const faqTitles = getFAQPanelTitle( page );
Expand Down