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

Consolidate accounts onboarding: Add edit mode to the combo card to enable it to disconnect accounts #2660

Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
b33484d
Add editing mode.
asvinb Oct 21, 2024
1b52069
Add placeholder text.
asvinb Oct 21, 2024
4c493a0
Merge branch 'feature/2509-consolidate-google-account-cards' into upd…
asvinb Oct 31, 2024
f63e731
REmove unused component.
asvinb Oct 31, 2024
78e18b9
Merge branch 'update/2597-connect-mc-account' into update/2605-edit-a…
asvinb Oct 31, 2024
e90aaca
Merge branch 'update/2597-connect-mc-account' into update/2605-edit-a…
asvinb Nov 5, 2024
b9f41b1
Merge branch 'update/2597-connect-mc-account' into update/2605-edit-a…
asvinb Nov 5, 2024
05580c3
Fix linting errors.
asvinb Nov 5, 2024
0a6c386
Merge branch 'update/2603-create-ads-account' into update/2605-edit-a…
asvinb Nov 5, 2024
10cdd81
Merge branch 'update/2597-connect-mc-account' into update/2605-edit-a…
asvinb Nov 5, 2024
218149d
Merge branch 'update/2603-create-ads-account' into update/2605-edit-a…
asvinb Nov 5, 2024
ad78ecb
Merge branch 'update/2582-claim-ads-account' into update/2605-edit-ac…
asvinb Nov 6, 2024
3b1dc9d
Merge branch 'update/2603-create-ads-account' into update/2605-edit-a…
asvinb Nov 6, 2024
65e6571
Add E2E tests.
asvinb Nov 6, 2024
442b478
Merge branch 'feature/2509-consolidate-google-account-cards' into upd…
asvinb Nov 6, 2024
7ded84d
Merge branch 'update/2603-create-ads-account' into update/2605-edit-a…
joemcgill Nov 6, 2024
36a13e4
Use actions prop.
asvinb Nov 7, 2024
80fd522
Refine E2E tests.
asvinb Nov 7, 2024
6de9cc0
Merge branch 'update/2603-create-ads-account' into update/2605-edit-a…
asvinb Nov 7, 2024
1e8a618
Merge branch 'update/2603-create-ads-account' into update/2605-edit-a…
asvinb Nov 8, 2024
846f975
Always hide conversion notice when clicking cancel.
asvinb Nov 8, 2024
fcf69f2
Remove autoadded code.
asvinb Nov 8, 2024
d0b30d2
Merge branch 'update/2603-create-ads-account' into update/2605-edit-a…
asvinb Nov 8, 2024
328a264
Merge branch 'update/2603-create-ads-account' into update/2605-edit-a…
asvinb Nov 8, 2024
0f0aa16
Update Connect to different Google account button text
joemcgill Nov 12, 2024
ba22a30
Eliminate border radius between combo account cards
joemcgill Nov 12, 2024
df93210
Revert title change in GoogleAdsAccountCard
joemcgill Nov 12, 2024
660ea2a
Rename onClick callback handlers
joemcgill Nov 12, 2024
3b9dbbb
Disconnect Ads when switching Google accounts
joemcgill Nov 12, 2024
9dbd087
Merge branch 'update/2603-create-ads-account' into update/2605-edit-a…
asvinb Nov 13, 2024
8260f12
Fix linting errors.
asvinb Nov 13, 2024
de1c98c
Remove unused imports.
asvinb Nov 13, 2024
1ace9be
Don't change import order
joemcgill Nov 13, 2024
c8f8be0
Improve card actions related to edit mode
joemcgill Nov 13, 2024
efb1bd3
Fix lint
joemcgill Nov 13, 2024
9795661
Fix inline docs
joemcgill Nov 13, 2024
0d63a35
Pass createMCAccount to useAutoCreateAdsMCAccounts
joemcgill Nov 14, 2024
3e46d07
Merge branch 'feature/2509-consolidate-google-account-cards' into upd…
joemcgill Nov 14, 2024
2ea19b8
Better card handling during auto-creation
joemcgill Nov 15, 2024
8d93a0e
E2E Add post auto-creation test
joemcgill Nov 15, 2024
08f96a7
Dont' show the edit button when showConnectMC is true and the Ads cla…
joemcgill Nov 15, 2024
1b0ad3e
Improve JSDoc
joemcgill Nov 15, 2024
392bdd7
Check existing accounts in the mc reclaim url card
joemcgill Nov 15, 2024
27a9627
Show MC Reclaim card after refresh
joemcgill Nov 15, 2024
767a129
Fix the issue that E2E test can't log in to wp-admin.
eason9487 Nov 11, 2024
109b724
Add external icon to Claim button
joemcgill Nov 17, 2024
17dab80
This may take a few moments, please wait…
joemcgill Nov 17, 2024
0348394
Only show Address Card once MC is ready
joemcgill Nov 17, 2024
ff47280
Reset connection flow if refreshed
joemcgill Nov 18, 2024
6a7d33e
Remove unnecessary ID from ReclaimUrlCard
joemcgill Nov 18, 2024
fa5a2d3
Omit createInterpolateElement
joemcgill Nov 18, 2024
4343210
Improve ConnectAds UI when connected account is not in existing accounts
joemcgill Nov 18, 2024
93d588e
Allow connected Ads account to be edited when claiming
joemcgill Nov 18, 2024
fd0ad0e
Improve edit mode when there are no existing accounts
joemcgill Nov 18, 2024
0e413b9
Remove unused import
joemcgill Nov 18, 2024
aef0d4a
Add edit button tests
joemcgill Nov 18, 2024
97465bc
Improve combo card border radius CSS
joemcgill Nov 18, 2024
d3d4c34
Lint fixes
joemcgill Nov 18, 2024
58820c1
Require an ID for hasGoogleMCConnection
joemcgill Nov 18, 2024
87049d8
Show disconnect button if there is 1 other existing account
joemcgill Nov 19, 2024
2b213f4
Consider the ConnectMCCard connected even when incomplete
joemcgill Nov 19, 2024
883d1db
Disable button if there are no Ads accounts except that the one that …
asvinb Nov 20, 2024
a98898a
Adjust the JSDoc and test case description for the create Google Ads …
eason9487 Nov 21, 2024
68fa52b
Add code comment to the `ConnectedGoogleComboAccountCard` for a speci…
eason9487 Nov 21, 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
Original file line number Diff line number Diff line change
@@ -1,22 +1,10 @@
/**
* External dependencies
*/
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import AccountCard, { APPEARANCE } from '.~/components/account-card';
import AppButton from '.~/components/app-button';
import ConnectedIconLabel from '.~/components/connected-icon-label';
import Section from '.~/wcdl/section';
import useSwitchGoogleAccount from './useSwitchGoogleAccount';

/**
* Clicking on the "connect to a different Google account" button.
*
* @event gla_google_account_connect_different_account_button_click
*/
import SwitchAccountButton from './switch-account-button';

/**
* Renders a Google account card UI with connected account information.
Expand All @@ -26,16 +14,12 @@ import useSwitchGoogleAccount from './useSwitchGoogleAccount';
* @param {{ email: string }} props.googleAccount A data payload object containing the user's Google account email.
* @param {JSX.Element} [props.helper] Helper content below the Google account email.
* @param {boolean} [props.hideAccountSwitch=false] Indicate whether hide the account switch block at the card footer.
*
* @fires gla_google_account_connect_different_account_button_click
*/
const ConnectedGoogleAccountCard = ( {
googleAccount,
helper,
hideAccountSwitch = false,
} ) => {
const [ handleSwitch, { loading } ] = useSwitchGoogleAccount();

return (
<AccountCard
appearance={ APPEARANCE.GOOGLE }
Expand All @@ -45,16 +29,7 @@ const ConnectedGoogleAccountCard = ( {
>
{ ! hideAccountSwitch && (
<Section.Card.Footer>
<AppButton
isLink
disabled={ loading }
text={ __(
'Or, connect to a different Google account',
'google-listings-and-ads'
) }
eventName="gla_google_account_connect_different_account_button_click"
onClick={ handleSwitch }
/>
<SwitchAccountButton />
</Section.Card.Footer>
) }
</AccountCard>
Expand Down
46 changes: 46 additions & 0 deletions js/src/components/google-account-card/switch-account-button.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* External dependencies
*/
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import AppButton from '.~/components/app-button';
import useSwitchGoogleAccount from './useSwitchGoogleAccount';

/**
* Clicking on the "connect to a different Google account" button.
*
* @event gla_google_account_connect_different_account_button_click
*/

/**
* Renders a switch button that lets user connect with another Google account.
*
* @fires gla_google_account_connect_different_account_button_click
* @param {Object} props React props
* @param {string} [props.text="Or, connect to a different Google account"] Text to display on the button
*/
const SwitchAccountButton = ( {
text = __(
'Or, connect to a different Google account',
'google-listings-and-ads'
),
...restProps
} ) => {
const [ handleSwitch, { loading } ] = useSwitchGoogleAccount();
eason9487 marked this conversation as resolved.
Show resolved Hide resolved

return (
<AppButton
isLink
disabled={ loading }
text={ text }
eventName="gla_google_account_connect_different_account_button_click"
onClick={ handleSwitch }
{ ...restProps }
/>
);
};

export default SwitchAccountButton;
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ import { __ } from '@wordpress/i18n';
/**
* Internal dependencies
*/
import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices';
import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount';
import { useAppDispatch } from '.~/data';
import useGoogleAdsAccountReady from '.~/hooks/useGoogleAdsAccountReady';
import AccountCard from '.~/components/account-card';
import ConnectAdsFooter from './connect-ads-footer';
import ConfirmCreateModal from './confirm-create-modal';
import LoadingLabel from '.~/components/loading-label';
import useApiFetchCallback from '.~/hooks/useApiFetchCallback';
import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices';
import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts';
import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount';
import { useAppDispatch } from '.~/data';
import useGoogleAdsAccountReady from '.~/hooks/useGoogleAdsAccountReady';
import AdsAccountSelectControl from '.~/components/ads-account-select-control';
import ConnectedIconLabel from '.~/components/connected-icon-label';
import ConnectButton from '.~/components/google-ads-account-card/connect-ads/connect-button';
Expand All @@ -37,8 +37,7 @@ const ConnectAds = ( { isConnecting, isCreating, onCreate } ) => {
const { fetchGoogleAdsAccountStatus } = useAppDispatch();
const isConnected = useGoogleAdsAccountReady();
const [ showCreateNewModal, setShowCreateNewModal ] = useState( false );
const { existingAccounts: accounts, hasFinishedResolution } =
useExistingGoogleAdsAccounts();
const { hasFinishedResolution } = useExistingGoogleAdsAccounts();
const {
googleAdsAccount,
refetchGoogleAdsAccount,
Expand Down Expand Up @@ -92,10 +91,6 @@ const ConnectAds = ( { isConnecting, isCreating, onCreate } ) => {
}
};

if ( ! accounts?.length ) {
return null;
}

const getIndicator = () => {
if ( ! hasFinishedResolution || ! hasResolvedGoogleAdsAccount ) {
return <LoadingLabel />;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
/**
* External dependencies
*/
import { useEffect, useRef, useState } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { useState, useEffect, useRef } from '@wordpress/element';

/**
* Internal dependencies
*/
import useAutoCreateAdsMCAccounts from '.~/hooks/useAutoCreateAdsMCAccounts';
import { useAppDispatch } from '.~/data';
import AccountCard, { APPEARANCE } from '../account-card';
import ConnectAds from './connect-ads';
Expand All @@ -14,13 +16,14 @@ import ConnectedAdsAccountsActions from './connected-ads-account-actions';
import Indicator from './indicator';
import getAccountCreationTexts from './getAccountCreationTexts';
import SpinnerCard from '.~/components/spinner-card';
import useAutoCreateAdsMCAccounts from '.~/hooks/useAutoCreateAdsMCAccounts';
import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount';
import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts';
import useCreateMCAccount from '.~/hooks/useCreateMCAccount';
import ConnectMC from '.~/components/google-mc-account-card/connect-mc';
import useGoogleAdsAccountReady from '.~/hooks/useGoogleAdsAccountReady';
import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts';
import AppButton from '.~/components/app-button';
import SwitchAccountButton from '.~/components/google-account-card/switch-account-button';
import useGoogleAdsAccountStatus from '.~/hooks/useGoogleAdsAccountStatus';
import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount';
import useUpsertAdsAccount from '.~/hooks/useUpsertAdsAccount';
Expand All @@ -31,11 +34,13 @@ import './connected-google-combo-account-card.scss';
* It will also kickoff Ads and Merchant Center account creation if the user does not have accounts.
*/
const ConnectedGoogleComboAccountCard = () => {
const [ editMode, setEditMode ] = useState( false );
const [
showConversionMeasurementNotice,
setShowConversionMeasurementNotice,
] = useState( false );
const initConnected = useRef( null );
const { hasDetermined, creatingWhich } = useAutoCreateAdsMCAccounts();
eason9487 marked this conversation as resolved.
Show resolved Hide resolved

// We use a single instance of the hook to create a MC (Merchant Center) account,
// ensuring consistent results across both the main component (ConnectedGoogleComboAccountCard) and its child component (ConnectMC).
Expand All @@ -44,8 +49,6 @@ const ConnectedGoogleComboAccountCard = () => {
const [ createMCAccount, resultCreateMCAccount ] = useCreateMCAccount();
const { data: existingGoogleMCAccounts } = useExistingGoogleMCAccounts();
const { isReady: isGoogleMCReady } = useGoogleMCAccount();
const { hasDetermined, creatingWhich } =
useAutoCreateAdsMCAccounts( createMCAccount );
const { text, subText } = getAccountCreationTexts( creatingWhich );
const { existingAccounts: existingGoogleAdsAccounts } =
useExistingGoogleAdsAccounts();
Expand Down Expand Up @@ -91,17 +94,42 @@ const ConnectedGoogleComboAccountCard = () => {
return <SpinnerCard />;
}

// @TODO: edit mode implementation in 2605
const editMode = false;
const shouldClaimGoogleAdsAccount = Boolean(
googleAdsAccount?.id && hasAccess === false
const toggleEditMode = () => {
setEditMode( ! editMode );
};

const cardActions = (
<div className="gla-google-combo-account-card__description-actions">
{ editMode ? (
<>
<SwitchAccountButton
isTertiary
text={ __(
'Connect to a different Google account',
'google-listings-and-ads'
) }
/>
<AppButton isTertiary onClick={ toggleEditMode }>
{ __( 'Cancel', 'google-listings-and-ads' ) }
</AppButton>
</>
) : (
<AppButton
isTertiary
text={ __( 'Edit', 'google-listings-and-ads' ) }
onClick={ toggleEditMode }
/>
) }
</div>
);

const hasExistingGoogleMCAccounts = existingGoogleMCAccounts?.length > 0;
const showConnectMC =
( editMode && hasExistingGoogleMCAccounts ) ||
( ! isGoogleMCReady && hasExistingGoogleMCAccounts );

const shouldClaimGoogleAdsAccount = Boolean(
googleAdsAccount?.id && hasAccess === false
);
const hasExistingGoogleAdsAccounts = existingGoogleAdsAccounts?.length > 0;
const showConnectAds =
( ( editMode && hasExistingGoogleAdsAccounts ) ||
Expand All @@ -122,6 +150,7 @@ const ConnectedGoogleComboAccountCard = () => {
alignIcon="top"
className="gla-google-combo-account-card gla-google-combo-account-card--connected gla-google-combo-service-account-card--google"
description={ text || <AccountDetails /> }
actions={ cardActions }
helper={ subText }
indicator={ <Indicator showSpinner={ showSpinner } /> }
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,9 @@
margin-bottom: calc((var(--large-gap)) / 2);
}
}

.gla-google-combo-account-card__description-actions {
display: flex;
gap: var(--large-gap);
}
}
39 changes: 38 additions & 1 deletion tests/e2e/specs/setup-mc/step-1-accounts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,6 @@ test.describe( 'Set up accounts', () => {
test.beforeAll( async () => {
await setUpAccountsPage.mockAdsAccountConnected();
await setUpAccountsPage.mockMCConnected();
await setUpAccountsPage.mockAdsAccountConnected();
await setUpAccountsPage.mockAdsStatusClaimed();

await setUpAccountsPage.goto();
Expand All @@ -800,6 +799,44 @@ test.describe( 'Set up accounts', () => {
} );
} );

test.describe( 'Edit button', () => {
test.beforeAll( async () => {
await setUpAccountsPage.mockMCConnected();
await setUpAccountsPage.mockAdsAccountConnected();
await setUpAccountsPage.mockAdsStatusClaimed();
await setUpAccountsPage.goto();

const editButton = setUpAccountsPage.getEditButton();
await editButton.click();
joemcgill marked this conversation as resolved.
Show resolved Hide resolved
} );

test( 'should display "Connect to a different Google account" button when clicked', async () => {
const connectDifferentGoogleAccountButton =
setUpAccountsPage.getConnectDifferentGoogleAccountButton();
await expect( connectDifferentGoogleAccountButton ).toBeVisible();
} );

test( 'should render the Google MC account card', async () => {
const selectAccountTitle =
setUpAccountsPage.getSelectExistingMCAccountTitle();
await expect( selectAccountTitle ).toContainText(
'Connect to existing Merchant Center account'
);
} );

test( 'should render the Google Ads account', async () => {
const googleAdsAccountCard =
setUpAccountsPage.getGoogleAdsAccountCard();

await expect(
googleAdsAccountCard.getByText(
'Connect to existing Google Ads account',
{ exact: true }
)
).toBeVisible();
} );
} );

test.describe( 'Links', () => {
test( 'should contain the correct URL for "Google Merchant Center Help" link', async () => {
await setUpAccountsPage.goto();
Expand Down
27 changes: 19 additions & 8 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 @@ -382,15 +382,26 @@ export default class SetUpAccountsPage extends MockRequests {
}

/**
* Register the responses when checking for account-status.
* Get "Edit" button.
*
* @return {Promise<import('@playwright/test').Response>} The response.
* @return {import('@playwright/test').Locator} Get "Edit" button.
*/
registerAdsAccountStatusResponse() {
return this.page.waitForResponse(
( response ) =>
response.url().includes( '/gla/ads/account-status' ) &&
response.status() === 200
);
getEditButton() {
return this.page.getByRole( 'button', {
name: 'Edit',
exact: true,
} );
}

/**
* Get "Connect to a different Google account" button.
*
* @return {import('@playwright/test').Locator} Get "Connect to a different Google account" button.
*/
getConnectDifferentGoogleAccountButton() {
return this.page.getByRole( 'button', {
name: 'Connect to a different Google account',
exact: true,
} );
}
}