Skip to content

Commit

Permalink
Merge pull request #5135 from google/fix/5052-adsense-account-state-main
Browse files Browse the repository at this point in the history
Refine logic for getting AFC client.
  • Loading branch information
felixarntz committed Apr 26, 2022
2 parents 4dd90d6 + 54f53c5 commit 520f6c2
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 38 deletions.
45 changes: 28 additions & 17 deletions assets/js/modules/adsense/components/setup/v2/SetupAccount.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ import { useEffect } from '@wordpress/element';
import Data from 'googlesitekit-data';
import {
MODULES_ADSENSE,
STATE_NEEDS_ATTENTION,
STATE_REQUIRES_REVIEW,
STATE_GETTING_READY,
API_STATE_NEEDS_ATTENTION,
API_STATE_REQUIRES_REVIEW,
API_STATE_GETTING_READY,
} from '../../../datastore/constants';
import {
ACCOUNT_STATUS_NO_CLIENT,
Expand Down Expand Up @@ -66,41 +66,52 @@ export default function SetupAccount( { account } ) {
select( MODULES_ADSENSE ).getCurrentSite( accountID )
);

const acfClientID = determineClientID( { clients: clients || [] } );
const afcClient = useSelect( ( select ) =>
select( MODULES_ADSENSE ).getAFCClient( accountID )
);

const afcClientID = determineClientID( { clients: clients || [] } );

const { setClientID, setAccountStatus } = useDispatch( MODULES_ADSENSE );

useEffect( () => {
if ( acfClientID && ( ! clientID || clientID !== acfClientID ) ) {
setClientID( acfClientID );
if ( afcClientID && ( ! clientID || clientID !== afcClientID ) ) {
setClientID( afcClientID );
}
}, [ setClientID, clientID, acfClientID ] );
}, [ setClientID, clientID, afcClientID ] );

useEffect( () => {
// Do nothing if clients aren't loaded because we can't determine acfClientID yet.
// Do nothing if clients aren't loaded because we can't determine afcClientID yet.
if ( clients === undefined || site === undefined ) {
return;
}

if ( ! acfClientID ) {
if ( ! afcClientID ) {
setAccountStatus( ACCOUNT_STATUS_NO_CLIENT );
} else if ( accountState === STATE_NEEDS_ATTENTION ) {
} else if ( accountState === API_STATE_NEEDS_ATTENTION ) {
setAccountStatus( ACCOUNT_STATUS_NEEDS_ATTENTION );
} else if ( site?.state === STATE_REQUIRES_REVIEW ) {
} else if ( afcClient?.state === API_STATE_REQUIRES_REVIEW ) {
setAccountStatus( ACCOUNT_STATUS_CLIENT_REQUIRES_REVIEW );
} else if ( site?.state === STATE_GETTING_READY ) {
} else if ( afcClient?.state === API_STATE_GETTING_READY ) {
setAccountStatus( ACCOUNT_STATUS_CLIENT_GETTING_READY );
} else {
setAccountStatus( ACCOUNT_STATUS_READY );
}
}, [ clients, setAccountStatus, acfClientID, site, accountState ] );
}, [
accountState,
afcClient,
afcClientID,
clients,
setAccountStatus,
site,
] );

// Show the progress bar if clients or site aren't loaded yet.
if ( clients === undefined || site === undefined ) {
return <ProgressBar />;
}

if ( ! acfClientID ) {
if ( ! afcClientID ) {
return <SetupAccountNoClient accountID={ accountID } />;
}

Expand All @@ -109,9 +120,9 @@ export default function SetupAccount( { account } ) {
}

if (
accountState === STATE_NEEDS_ATTENTION ||
site?.state === STATE_REQUIRES_REVIEW ||
site?.state === STATE_GETTING_READY
accountState === API_STATE_NEEDS_ATTENTION ||
afcClient?.state === API_STATE_REQUIRES_REVIEW ||
afcClient?.state === API_STATE_GETTING_READY
) {
return <SetupAccountPendingTasks accountID={ accountID } />;
}
Expand Down
26 changes: 13 additions & 13 deletions assets/js/modules/adsense/components/setup/v2/SetupAccountSite.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ import Data from 'googlesitekit-data';
import Button from '../../../../../components/Button';
import {
MODULES_ADSENSE,
SITE_STATE_READY,
SITE_STATE_GETTING_READY,
SITE_STATE_NEEDS_ATTENTION,
SITE_STATE_REQUIRES_REVIEW,
API_STATE_READY,
API_STATE_GETTING_READY,
API_STATE_NEEDS_ATTENTION,
API_STATE_REQUIRES_REVIEW,
} from '../../../datastore/constants';
import {
SITE_STATUS_READY,
Expand Down Expand Up @@ -85,16 +85,16 @@ export default function SetupAccountSite( { accountID, site, finishSetup } ) {
let siteStatus;

switch ( state ) {
case SITE_STATE_NEEDS_ATTENTION:
case API_STATE_NEEDS_ATTENTION:
siteStatus = SITE_STATUS_NEEDS_ATTENTION;
break;
case SITE_STATE_REQUIRES_REVIEW:
case API_STATE_REQUIRES_REVIEW:
siteStatus = SITE_STATUS_REQUIRES_REVIEW;
break;
case SITE_STATE_GETTING_READY:
case API_STATE_GETTING_READY:
siteStatus = SITE_STATUS_GETTING_READY;
break;
case SITE_STATE_READY:
case API_STATE_READY:
siteStatus = autoAdsEnabled
? SITE_STATUS_READY
: SITE_STATUS_READY_NO_AUTO_ADS;
Expand All @@ -108,16 +108,16 @@ export default function SetupAccountSite( { accountID, site, finishSetup } ) {

let notice;
switch ( state ) {
case SITE_STATE_NEEDS_ATTENTION:
case API_STATE_NEEDS_ATTENTION:
notice = `TODO: UI to inform that site ${ domain } (in account ${ accountID }) needs attention`;
break;
case SITE_STATE_REQUIRES_REVIEW:
case API_STATE_REQUIRES_REVIEW:
notice = `TODO: UI to inform that site ${ domain } (in account ${ accountID }) requires review`;
break;
case SITE_STATE_GETTING_READY:
case API_STATE_GETTING_READY:
notice = `TODO: UI to inform that site ${ domain } (in account ${ accountID }) is getting ready`;
break;
case SITE_STATE_READY:
case API_STATE_READY:
notice = autoAdsEnabled
? `TODO: UI to inform that site ${ domain } (in account ${ accountID }) is ready, with auto ads enabled`
: `TODO: UI to inform that site ${ domain } (in account ${ accountID }) is ready, with auto ads disabled`;
Expand All @@ -128,7 +128,7 @@ export default function SetupAccountSite( { accountID, site, finishSetup } ) {
<div>
<p>{ notice }</p>

{ state === SITE_STATE_READY && (
{ state === API_STATE_READY && (
<div>
<Button
onClick={ continueHandler }
Expand Down
36 changes: 36 additions & 0 deletions assets/js/modules/adsense/datastore/clients.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import { isValidAccountID } from '../util';
import { createFetchStore } from '../../../googlesitekit/data/create-fetch-store';
import { actions as errorStoreActions } from '../../../googlesitekit/data/create-error-store';

const { createRegistrySelector } = Data;

// Actions
const RESET_CLIENTS = 'RESET_CLIENTS';

Expand Down Expand Up @@ -155,6 +157,40 @@ const baseSelectors = {

return clients[ accountID ];
},

/**
* Gets the AdSense For Content (AFC) client for the given AdSense account.
*
* @since n.e.x.t
*
* @param {Object} state Data store's state.
* @param {string} accountID The AdSense Account ID to fetch AFC client for.
* @return {(Object|null|undefined)} An AdSense AFC client, or `null` if none exists; `undefined` if not loaded.
*/
getAFCClient: createRegistrySelector(
( select ) => ( state, accountID ) => {
if ( undefined === accountID ) {
return undefined;
}

const clients = select( MODULES_ADSENSE ).getClients( accountID );

if ( clients === undefined ) {
return undefined;
}

const afcClients = clients.filter( ( client ) => {
return 'AFC' === client.productCode;
} );

if ( ! afcClients.length ) {
return null;
}

// Pick the first AFC client. There should only ever be one anyway.
return afcClients[ 0 ];
}
),
};

const store = Data.combineStores( fetchGetClientsStore, {
Expand Down
65 changes: 65 additions & 0 deletions assets/js/modules/adsense/datastore/clients.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,5 +133,70 @@ describe( 'modules/adsense clients', () => {
expect( console ).toHaveErrored();
} );
} );

describe( 'getAFClient', () => {
it( 'returns undefined if clients are not yet resolved', async () => {
fetchMock.getOnce(
/^\/google-site-kit\/v1\/modules\/adsense\/data\/clients/,
{ body: fixtures.clients, status: 200 }
);

const accountID = fixtures.clients[ 0 ]._accountID;

const initialAFCClient = registry
.select( MODULES_ADSENSE )
.getAFCClient( accountID );

expect( initialAFCClient ).toEqual( undefined );

await subscribeUntil(
registry,
() =>
registry
.select( MODULES_ADSENSE )
.getAFCClient( accountID ) !== undefined
);
} );

it( 'returns null if there are no clients', async () => {
const fakeAccountID = 'pub-777888999';
registry
.dispatch( MODULES_ADSENSE )
.receiveGetClients( [], { accountID: fakeAccountID } );

const client = registry
.select( MODULES_ADSENSE )
.getAFCClient( fakeAccountID );

await subscribeUntil(
registry,
() =>
registry
.select( MODULES_ADSENSE )
.isFetchingGetClients( fakeAccountID ) === false
);

expect( client ).toEqual( null );
} );

it( 'returns the first AFC client', async () => {
const accountID = fixtures.clients[ 0 ]._accountID;
registry
.dispatch( MODULES_ADSENSE )
.receiveGetClients( fixtures.clients, { accountID } );

const client = registry
.select( MODULES_ADSENSE )
.getAFCClient( accountID );

await subscribeUntil( registry, () =>
registry
.select( MODULES_ADSENSE )
.hasFinishedResolution( 'getClients', [ accountID ] )
);

expect( client ).toEqual( fixtures.clients[ 0 ] );
} );
} );
} );
} );
12 changes: 4 additions & 8 deletions assets/js/modules/adsense/datastore/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,7 @@ export const MODULES_ADSENSE = 'modules/adsense';
// Date range offset days for AdSense report requests.
export const DATE_RANGE_OFFSET = 1;

export const SITE_STATE_NEEDS_ATTENTION = 'NEEDS_ATTENTION';
export const SITE_STATE_REQUIRES_REVIEW = 'REQUIRES_REVIEW';
export const SITE_STATE_GETTING_READY = 'GETTING_READY';
export const SITE_STATE_READY = 'READY';

export const STATE_NEEDS_ATTENTION = 'NEEDS_ATTENTION';
export const STATE_REQUIRES_REVIEW = 'REQUIRES_REVIEW';
export const STATE_GETTING_READY = 'GETTING_READY';
export const API_STATE_READY = 'READY';
export const API_STATE_NEEDS_ATTENTION = 'NEEDS_ATTENTION';
export const API_STATE_REQUIRES_REVIEW = 'REQUIRES_REVIEW';
export const API_STATE_GETTING_READY = 'GETTING_READY';

0 comments on commit 520f6c2

Please sign in to comment.